-
Notifications
You must be signed in to change notification settings - Fork 227
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 S4457 FP: Argument check after async code #6624
Conversation
argumentExceptionLocations.Add(node.GetLocation()); | ||
} | ||
// "ThrowIfNull" returns void so it cannot be an argument. We can stop. | ||
else |
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.
Without this else
block, the visit recursion stops for this node.
This means that for something like:
whatever.Invocation()
The whatever.Invocation
syntax node is not going to keep visiting inside the whatever
node, in which case if whatever
contains an AwaitExpression
, we will not spot it to stop the SyntaxWalker via:
public override void VisitAwaitExpression(AwaitExpressionSyntax node) => keepWalking = false;
Which produces this the FP in the testcases.
analyzers/src/SonarAnalyzer.CSharp/Rules/ParameterValidationInMethodWalker.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ParameterValidationInMethodWalker.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ParameterValidationInMethodWalker.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/ParameterValidationInMethodWalker.cs
Show resolved
Hide resolved
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.
A few more modifications requested
baa358a
to
97dc7fc
Compare
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
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.
LGTM!
Fixes #6449