Avoid duplicate misplaced 'partial' diagnostics - #85013
Merged
333fred merged 14 commits intoAug 27, 2026
Merged
Conversation
Check partial ordering only for declarations that permit the modifier, leaving invalid declarations to the existing modifier check.
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
dotnet-policy-service
Bot
added
the
Community
CyrusNajmabadi
commented
Aug 24, 2026
| if (kind != DeclarationKind.Extension) | ||
| { | ||
| node.Modifiers.CheckPartialModifierOrder(diagnostics, isOrdinaryMethod: false); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
note: an alternative is to still have CheckPartialModifierOrder inside ToDeclarationModifiers, and then pass in a flag if you don't want them to be checked (because you know it would be be unhelpful with other errors you know will happen).
I'm fine with either approach.
Contributor
Author
There was a problem hiding this comment.
in this case, we'd have ToDeclarationModifiers(..., checkPartialModifierOrder: kind != DeclarationKind.Extension)
CyrusNajmabadi
commented
Aug 24, 2026
| var result = modifiers.ToDeclarationModifiers(isForTypeDeclaration: false, diagnosticBag); | ||
| // There is no need to report an ordering error when 'partial' is not allowed. | ||
| // The invalid modifier is reported below. | ||
| if ((allowedModifiers & DeclarationModifiers.Partial) != 0) |
Contributor
Author
There was a problem hiding this comment.
and here we'd do ToDeclarationModifiers(..., checkPartialModifierOrder: (allowedModifiers & DeclarationModifiers.Partial) != 0)
Contributor
Author
|
@333fred fixing that funky duplicate partial modifier issue. |
This was referenced Aug 24, 2026
Contributor
Author
|
@jjonescz ptal. Thanks! |
…placed-partial-diagnostics
Remove duplicate misplaced-partial expectations after modifier validation begins reporting each occurrence once.
jjonescz
approved these changes
Aug 26, 2026
Let modifier conversion validate both whether partial is allowed and whether it is ordered correctly, so later modifier checks do not report the same token again.
CyrusNajmabadi
commented
Aug 26, 2026
CyrusNajmabadi
commented
Aug 26, 2026
| // (1,8): error CS0267: The 'partial' modifier can only appear immediately before 'class', 'record', 'struct', 'interface', 'event', an instance constructor name, or a method or property return type. | ||
| // public partial delegate void M(); | ||
| Diagnostic(ErrorCode.ERR_PartialMisplaced, "M").WithLocation(1, 30)); | ||
| Diagnostic(ErrorCode.ERR_PartialMisplaced, "partial").WithLocation(1, 8)); |
Contributor
Author
There was a problem hiding this comment.
note: these diagnostics are better. they're now on the partial modifier, not hte member.
333fred
reviewed
Aug 26, 2026
333fred
approved these changes
Aug 26, 2026
333fred
enabled auto-merge (squash)
August 26, 2026 19:55
This was referenced Aug 28, 2026
333fred
pushed a commit
that referenced
this pull request
Sep 4, 2026
## Summary - Parse `partial` as a declaration modifier in any position when the following tokens unambiguously identify a declaration. - Build the intended declaration syntax tree instead of producing cascading parser errors. - Preserve the existing validity rules by reporting `ERR_PartialMisplaced` from binding for every non-canonical position. - Preserve the historical `partial async` ordinary-method behavior. - Preserve the existing `CS0267` order-modifiers code fix; this PR does not introduce or depend on a feature diagnostic. This is a parser-recovery implementation fix. It does not change which programs compile. ## PR chain This is a chain of three PRs. Merge them in this order: - #85013 - #84935 - #83216 ## Test plan - Added permutation and regression coverage for misplaced `partial` on type and member declarations. - Updated affected parser tests to verify declaration-shaped trees and binding diagnostics. - Verified the existing `OrderModifiersCompilerErrorTests` suite still fixes `CS0267` by moving `partial` to its canonical position.
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
3 participants
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.
Summary
Report modifier-order diagnostics only when
partialis valid for the declaration kind. This avoids reporting the same misplacedpartialerror from both modifier conversion and declaration-specific validation.PR chain
This is a chain of three PRs. Merge them in this order:
Test plan
Microsoft Reviewers: Open in CodeFlow