Repository navigation
SA1017: Require spacing after parameter attributes - #4321
Merged
Merged
Conversation
Report missing whitespace after the last attribute list on parameters and type parameters, using the existing token spacing code fix. Preserve comments, line breaks, and adjacent attribute lists. Fixes #3118 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new diagnostic incorrectly instructs users to remove preceding whitespace instead of adding following whitespace.
1 open finding
What changed in this PR
Extends SA1017 to require spacing after parameter and type-parameter attributes, addressing #3118.
Changes:
- Detects missing following whitespace using the existing token-spacing fix.
- Adds regression coverage for declaration forms, comments, and line breaks.
- Updates rule documentation in four languages.
| File | Description |
|---|---|
| test/StyleCop.Analyzers.Test/SpacingRules/SA1017UnitTests.cs | Adds spacing and code-fix regression cases. |
| test/StyleCop.Analyzers.Test.CSharp8/SpacingRules/SA1017CSharp8UnitTests.cs | Covers the nullable out example. |
| src/StyleCop.Analyzers/SpacingRules/SA1017ClosingAttributeBracketsMustBeSpacedCorrectly.cs | Checks whitespace after parameter attributes. |
| documentation/zh-cn/SA1017.md | Updates Simplified Chinese guidance. |
| documentation/SA1017.md | Documents the expanded rule. |
| documentation/ru-ru/SA1017.md | Updates Russian guidance. |
| documentation/cs-cz/SA1017.md | Updates Czech guidance. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4321 +/- ##
==========================================
+ Coverage 97.62% 97.63% +0.01%
==========================================
Files 1319 1321 +2
Lines 144254 144864 +610
Branches 4864 4873 +9
==========================================
+ Hits 140825 141437 +612
Misses 2173 2173
+ Partials 1256 1254 -2 🚀 New features to boost your workflow:
|
Use a separate localized message for missing following whitespace while preserving the existing preceding-space message. Assert both messages in regression tests and regenerate resource accessors. The new message uses English fallback in other cultures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The narrowly scoped change has focused regression coverage and no identified blocking issues.
0 open findings
1 resolved since last review
Files not reviewed (1)
- src/StyleCop.Analyzers/Lightup/.generated/Microsoft.CodeAnalysis.ResxSourceGenerator.CSharp/Microsoft.CodeAnalysis.ResxSourceGenerator.CSharp.CSharpResxGenerator/SpacingResources.Designer.cs: Generated file
🧠 Review effort: Balanced
AArnott
enabled auto-merge (squash)
October 16, 2026 13:49
Add Russian and Simplified Chinese translations for the new diagnostic so localized resource completeness checks pass without weakening coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation and regression coverage leave no unresolved blocking issues; tests were not rerun during this review.
0 open findings
Files not reviewed (1)
- src/StyleCop.Analyzers/Lightup/.generated/Microsoft.CodeAnalysis.ResxSourceGenerator.CSharp/Microsoft.CodeAnalysis.ResxSourceGenerator.CSharp.CSharpResxGenerator/SpacingResources.Designer.cs: Generated file
🧠 Review effort: Balanced
This was referenced Nov 11, 2026
Bump StyleCop.Analyzers from 1.2.0-beta.556 to 1.3.180
microsoftgraph/aspnetcore-webhooks-sample#436
Open
This was referenced Nov 13, 2026
Open
Open
Open
Merged
Open
Bump StyleCop.Analyzers from 1.2.0-beta.556 to 1.3.180
microsoftgraph/msgraph-sample-aspnet-core#256
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SA1017 checked whitespace before closing attribute brackets but missed missing whitespace after them, allowing declarations such as
[NotNullWhen(true)]out string? foo.Fixes #3118
Require whitespace after the last attribute list on a parameter or type parameter when the declaration continues on the same line. Reuse the existing token-spacing code fix, preserving comments, line breaks, and adjacent attribute lists. Multiple spaces remain the responsibility of SA1025. Update the English, Czech, Russian, and Simplified Chinese rule documentation.
Behavior change: SA1017 now reports missing spaces after parameter and type-parameter attributes. Other attribute targets are unchanged.
Verification: The reporter's exact nullable
outexample and additional parameter cases failed against unchanged master with the expected missing-diagnostic failures before the analyzer change. The build completes with zero warnings, and the complete SA1017 suite passes under every C# 6-15 test project on net10.0, plus the oldest and newest projects on net472. Regression coverage includes modifiers, constructors, delegates, indexers, type parameters, adjacent attributes, comments, multiline attributes, and fixes on both sides of a bracket.Performance: Adds syntax and trivia checks only for closing attribute brackets on parameters and type parameters, within the existing shared token walk. No new registrations, semantic-model calls, tree walks, or per-callback allocations; no benchmark was run.