Skip to content

Classify parameter discards earlier, syntactically instead of semantically, resolving classification conflict - #85225

Merged
akhera99 merged 2 commits into
dotnet:mainfrom
jnm2:lambda_discard_color
Sep 9, 2026
Merged

Classify parameter discards earlier, syntactically instead of semantically, resolving classification conflict#85225
akhera99 merged 2 commits into
dotnet:mainfrom
jnm2:lambda_discard_color

Conversation

@jnm2

@jnm2 jnm2 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #51553.

Roslyn classifies discards as keywords for syntax highlighting which shows whether or not _ is referring to a symbol.

image

However, it's not working for lambda parameter discards:
image

There was an attempt to implement it in #40396 for lambda parameter discards:

case ParameterSyntax parameter when parameter.Identifier.Text == "_":
var symbol = semanticModel.GetDeclaredSymbol(parameter, cancellationToken);
if (symbol?.IsDiscard == true)
{
result.Add(new ClassifiedSpan(parameter.Identifier.Span, ClassificationTypeNames.Keyword));
}
break;

But this was insufficient because it was implemented in the semantic classifier. The syntactic classifier already classified it as ParameterName, and the two are merged with ordering rules which cause ParameterName to win over Keyword.

[ClassificationType(ClassificationTypeNames = ClassificationTypeNames.ParameterName)]
[Name(ClassificationTypeNames.ParameterName)]
[Order(After = PredefinedClassificationTypeNames.Identifier)]
[Order(After = PredefinedClassificationTypeNames.Keyword)]

This PR fixes the issue by implementing the proper classification in the syntactic classifier instead of the semantic classifier. This is preferable because the semantic classifier may run after more of a delay, so this is less visual flicker in the IDE.

Microsoft Reviewers: Open in CodeFlow

@jnm2
jnm2 requested a review from a team as a code owner September 9, 2026 06:31
Copilot AI lite review requested due to automatic review settings September 9, 2026 06:31
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is small, aligns with compiler discard rules for lambda parameters, and includes updated/added test coverage for both syntactic and total classification behavior.

Pull request overview

This PR adjusts C# syntax classification so lambda parameter discards (_ used as a discard when duplicated in the same anonymous function parameter list) are classified as Keyword syntactically (instead of relying on semantic classification), avoiding classification-merge precedence issues where ParameterName would previously win.

Changes:

  • Removed semantic classification of discard lambda parameters from DiscardSyntaxClassifier.
  • Added syntactic classification logic to classify _ as Keyword for anonymous-function parameter lists where _ appears more than once.
  • Updated/added classification tests to validate syntactic + total classification behavior for lambda discards.
File summaries
File Description
src/Workspaces/CSharp/Portable/Classification/SyntaxClassification/DiscardSyntaxClassifier.cs Removes the semantic classification path for ParameterSyntax discards so it no longer conflicts with syntactic parameter-name classification.
src/Workspaces/CSharp/Portable/Classification/ClassificationHelpers.cs Implements syntactic detection of lambda parameter discards by checking for another _ in the same anonymous-function parameter list and returning Keyword.
src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests.cs Adds regression coverage to ensure the merged/total classifier produces Keyword for discard _ parameters and Parameter for non-discard _.
src/EditorFeatures/CSharpTest/Classification/SyntacticClassifierTests_Preprocessor.cs Updates syntactic expected classifications so (_, _) => ... now yields Keyword for both _ parameters.
src/EditorFeatures/CSharpTest/Classification/SemanticClassifierTests.cs Updates semantic-classifier expectations to no longer require keyword classification for lambda discard parameters (now handled syntactically).
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 9, 2026 06:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The new discard detection uses Text comparisons for _ instead of ValueText, which can misclassify unicode-escaped underscore identifiers that are semantically equivalent to _.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/Workspaces/CSharp/Portable/Classification/ClassificationHelpers.cs:263

  • Discard detection should use ValueText instead of Text so escaped underscore identifiers (e.g. \u005f) are treated consistently as _ discards. Using Text can cause incorrect classification for syntactically-equivalent identifiers.
    src/Workspaces/CSharp/Portable/Classification/SyntaxClassification/DiscardSyntaxClassifier.cs:36
  • Discard classification for identifier names should use ValueText rather than Text so unicode-escaped underscore identifiers are classified the same as _.
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@jnm2

jnm2 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

The new discard detection uses Text comparisons for _ instead of ValueText, which can misclassify unicode-escaped underscore identifiers that are semantically equivalent to _.

  • \u005F is not semantically equivalent to _. Only the latter can be a discard. Using ValueText would be less precise and introduce this issue and another issue with @_ incorrectly considered a discard too.
  • This approach already existed in DiscardSyntaxClassifier.cs; it is merely moving. The same approach also remains in that file for handling other kinds of discards.

@akhera99
akhera99 merged commit 4a5461d into dotnet:main Sep 9, 2026
23 checks passed
@dotnet-policy-service dotnet-policy-service Bot added this to the Next milestone Sep 9, 2026
@jnm2
jnm2 deleted the lambda_discard_color branch September 9, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Is this a bug that lambda discards aren't highlighted as keyword?

3 participants