Repository navigation
Avoid duplicate misplaced 'partial' diagnostics #85013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
333fred
merged 14 commits into
dotnet:main
from
CyrusNajmabadi:avoid-duplicate-misplaced-partial-diagnostics
Aug 27, 2026
+66
−127
Merged
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
88cdb8d
Avoid duplicate misplaced partial diagnostics
CyrusNajmabadi e66493f
Merge remote-tracking branch 'upstream/main' into avoid-duplicate-mis…
CyrusNajmabadi f4e9537
Update partial constructor diagnostic baseline
CyrusNajmabadi ebd33f1
Centralize partial modifier diagnostics
CyrusNajmabadi 0e18b3c
Group modifier flags before diagnostics
CyrusNajmabadi 2c34015
Remove ordinary-method modifier flag
CyrusNajmabadi 557272e
Clarify legal partial modifier positions
CyrusNajmabadi 83f1aeb
Derive partial feature context from syntax
CyrusNajmabadi 7624122
Condense modifier helper calls
CyrusNajmabadi e0553b9
Condense modifier helper signature
CyrusNajmabadi 8cc2aad
Update remaining partial diagnostic baselines
CyrusNajmabadi 593fd8f
Explain partial async compatibility
CyrusNajmabadi fb7eba3
Name async modifier validation method
CyrusNajmabadi ddf8799
Merge branch 'main' into avoid-duplicate-misplaced-partial-diagnostics
CyrusNajmabadi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,15 @@ internal static DeclarationModifiers MakeAndCheckNonTypeMemberModifiers( | |
| out bool modifierErrors, | ||
| out bool hasExplicitAccessModifier) | ||
| { | ||
| var result = modifiers.ToDeclarationModifiers(isForTypeDeclaration: false, diagnostics.DiagnosticBag ?? new DiagnosticBag(), isOrdinaryMethod: isOrdinaryMethod); | ||
| var diagnosticBag = diagnostics.DiagnosticBag ?? new DiagnosticBag(); | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and here we'd do |
||
| { | ||
| modifiers.CheckPartialModifierOrder(diagnosticBag, isOrdinaryMethod); | ||
| } | ||
|
|
||
| result = CheckModifiers(isForTypeDeclaration: false, isForInterfaceMember, result, allowedModifiers, errorLocation, diagnostics, modifiers, out modifierErrors); | ||
|
|
||
| var readonlyToken = modifiers.FirstOrDefault(SyntaxKind.ReadOnlyKeyword); | ||
|
|
@@ -463,7 +471,9 @@ private static DeclarationModifiers GetDeclarationModifiersAndCheckForDuplicateM | |
| } | ||
|
|
||
| public static DeclarationModifiers ToDeclarationModifiers( | ||
| this SyntaxTokenList modifiers, bool isForTypeDeclaration, DiagnosticBag diagnostics, bool isOrdinaryMethod = false) | ||
| this SyntaxTokenList modifiers, | ||
| bool isForTypeDeclaration, | ||
| DiagnosticBag diagnostics) | ||
| { | ||
| var result = GetDeclarationModifiersAndCheckForDuplicateModifiers(modifiers, diagnostics); | ||
| if ((result & DeclarationModifiers.Partial) == DeclarationModifiers.Partial) | ||
|
|
@@ -473,18 +483,6 @@ public static DeclarationModifiers ToDeclarationModifiers( | |
|
|
||
| var messageId = isForTypeDeclaration ? MessageID.IDS_FeaturePartialTypes : MessageID.IDS_FeaturePartialMethod; | ||
| messageId.CheckFeatureAvailability(diagnostics, modifier); | ||
|
|
||
| // `partial` must always be the last modifier according to the language. However, there was a bug | ||
| // where we allowed `partial async` at the end of modifiers on methods. We keep this behavior for | ||
| // backcompat. | ||
| var isLast = i == modifiers.Count - 1; | ||
| var isPartialAsyncMethod = isOrdinaryMethod && i == modifiers.Count - 2 && modifiers[i + 1].ContextualKind() is SyntaxKind.AsyncKeyword; | ||
| if (!isLast && !isPartialAsyncMethod) | ||
| { | ||
| diagnostics.Add( | ||
| ErrorCode.ERR_PartialMisplaced, | ||
| modifier.GetLocation()); | ||
| } | ||
| } | ||
|
|
||
| switch (result & DeclarationModifiers.AccessibilityMask) | ||
|
|
@@ -505,6 +503,27 @@ public static DeclarationModifiers ToDeclarationModifiers( | |
| return result; | ||
| } | ||
|
|
||
| internal static void CheckPartialModifierOrder(this SyntaxTokenList modifiers, DiagnosticBag diagnostics, bool isOrdinaryMethod) | ||
| { | ||
| var i = modifiers.IndexOf(SyntaxKind.PartialKeyword); | ||
| if (i < 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // `partial` must always be the last modifier according to the language. However, there was a bug | ||
| // where we allowed `partial async` at the end of modifiers on methods. We keep this behavior for | ||
| // backcompat. | ||
| var isLast = i == modifiers.Count - 1; | ||
| var isPartialAsyncMethod = isOrdinaryMethod && i == modifiers.Count - 2 && modifiers[i + 1].ContextualKind() is SyntaxKind.AsyncKeyword; | ||
| if (!isLast && !isPartialAsyncMethod) | ||
| { | ||
| diagnostics.Add( | ||
| ErrorCode.ERR_PartialMisplaced, | ||
| modifiers[i].GetLocation()); | ||
| } | ||
| } | ||
|
|
||
| private static void ReportDuplicateModifiers( | ||
| SyntaxToken modifierToken, | ||
| DeclarationModifiers modifierKind, | ||
|
|
||
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case, we'd have
ToDeclarationModifiers(..., checkPartialModifierOrder: kind != DeclarationKind.Extension)