Skip to content
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

Fix T0010 FP: Allow "_" discard like parameter name #9257

Merged
merged 4 commits into from May 8, 2024

Conversation

martin-strecker-sonarsource
Copy link
Contributor

_ => 42 should be allowed, because _ is more descriptive than x in this case and signals: "I do not care about the parameter." Rational: In (_, _) => 42 _ is a real discard, while in _ => 42 it isn't only for back compatibility reasons.

See e.g. here:
https://sonarcloud.io/project/issues?resolved=false&pullRequest=8950&id=sonaranalyzer-dotnet&open=AY9NLK1CHaBGYjI6fPmk

_ is a better name than x in such a case.

@github-actions github-actions bot added this to In progress in Best Kanban May 6, 2024
@martin-strecker-sonarsource martin-strecker-sonarsource changed the title Fix TP0010: Allow "_" discard like paramter name Fix TP0010: Allow "_" discard like parameter name May 6, 2024
@martin-strecker-sonarsource
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@martin-strecker-sonarsource martin-strecker-sonarsource marked this pull request as ready for review May 7, 2024 13:54
Copy link

sonarcloud bot commented May 8, 2024

Quality Gate Passed Quality Gate passed for 'Sonar .NET Java Plugin'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

sonarcloud bot commented May 8, 2024

Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot moved this from Review in progress to Review approved in Best Kanban May 8, 2024
@@ -29,7 +29,7 @@ public sealed class LambdaParameterName : StylingAnalyzer
context.RegisterNodeAction(c =>
{
var parameter = ((SimpleLambdaExpressionSyntax)c.Node).Parameter;
if (parameter.Identifier.ValueText != "x"
if (parameter.Identifier.ValueText is not ("x" or "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

What about leaving a comment here that in C# 13, it will be a real discard and the "_" will not be needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if parsed as a discard, it is still an IdenifierToken:
sharplab.io

So I think it will still be needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if there's a branch available with that feature? I didn't find any in a quick search.

It might not be IdentifierToken, but some kind of discard designator, like here:
https://sharplab.io/#v2:C4LghgzsA0AmIGoA+ABATARgLACgUGYACdQgYUIG9dCbiiUAWQgWQAoBKS62ngNzABOhVgH1ohAB6cAvIQAqHANzcaAXxWENABwEBLfsACmw3QDsYhM8E4KZAPkKxDAMzABXADbBlOVUA===

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not aware of a branch for it. The closest I could find on SharpLab is "Semi Auto-properties," but that one hasn't been updated since Oct 22.

https://sharplab.io/#v2:EYLgZgpghgLgrgJwgZwLTIgWwJaqnGAe1QAcFCTkAaGEKZGKgExAGoAfAAQCYBGAWABQnAMwACHmIDCYgN5CxiieOwA7GGIAK5EnLEBzCBoC8APjFhsEADZMA3GIwwHAXwVL3i0RIAsYgLIAFACUcp5KXrzcADxqMOZQAMYw2ISqYsZiAPoZ5rIuduFugi5AA===

The DiscardDesignation is an interesting find. The significant difference here is that the distinguishing happens on the node level, while for parameters, it would likely need to occur on the token level.

There are cases like this already with, e.g., IdentifierNameSyntax where an Identifier property can be either an IdentiferToken or a GlobalKeyword.
There can be IdentifierToken or the ArgListKeyword for parameters. Maybe we see an UnderScore here as well in the future.

    <Node Name="ParameterSyntax" Base="BaseParameterSyntax">
        <Kind Name="Parameter"/>
        <! -- ... -->
        <Field Name="Identifier" Type="SyntaxToken">
            <PropertyComment>
                <summary>Gets the identifier.</summary>
            </PropertyComment>
            <Kind Name="IdentifierToken"/>
            <Kind Name="ArgListKeyword"/>
        </Field>
        <! -- ... -->
    </Node>

@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Fix TP0010: Allow "_" discard like parameter name Fix T0010: Allow "_" discard like parameter name May 8, 2024
@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Fix T0010: Allow "_" discard like parameter name Fix T0010 FP: Allow "_" discard like parameter name May 8, 2024
@martin-strecker-sonarsource martin-strecker-sonarsource merged commit 507e925 into master May 8, 2024
31 checks passed
@martin-strecker-sonarsource martin-strecker-sonarsource deleted the Martin/Fix_FP_T0010_Discard branch May 8, 2024 14:04
Best Kanban automation moved this from Review approved to Validate Peach May 8, 2024
@martin-strecker-sonarsource martin-strecker-sonarsource moved this from Validate Peach to Done in Best Kanban May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Best Kanban
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants