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

C#: Fix issue with suppress nullable warning directly on a method call. #16186

Merged
merged 4 commits into from
Apr 15, 2024

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Apr 11, 2024

It turns out that the extractor is somewhat broken for suppress nullable warning expressions like System.Console.ReadLine()!.
The extraction worked correctly when the suppressing nullable warnings on identifiers, but not directly on the result of calls.
Even though ! is referred to as an operator, it has no semantical meaning during program execution (this is only an instruction to the compiler) and is just a piece of syntax without any associated method symbol.

Comment to the code change: Note that PostfixUnary is only used for ++, -- and !.

@github-actions github-actions bot added the C# label Apr 11, 2024
@michaelnebel michaelnebel force-pushed the csharp/suppressnullablefix branch from 34781d5 to f208a13 Compare April 12, 2024 07:40
@michaelnebel michaelnebel force-pushed the csharp/suppressnullablefix branch from 5ea9fe4 to 183cc66 Compare April 12, 2024 08:40
@michaelnebel michaelnebel force-pushed the csharp/suppressnullablefix branch from 183cc66 to cbb5d43 Compare April 12, 2024 09:23
@michaelnebel
Copy link
Contributor Author

michaelnebel commented Apr 12, 2024

I think that DCA looks good, but it is hard to judge from all the noise (1) Performance is acceptable (2) The alert diff only shows diffs on non-security related queries and when spot checking the results, it appears that they are not related to the change in this PR.

@michaelnebel michaelnebel marked this pull request as ready for review April 12, 2024 12:44
@michaelnebel michaelnebel requested a review from a team as a code owner April 12, 2024 12:44
Copy link
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

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

Looks plausible.

@michaelnebel michaelnebel merged commit d5073df into github:main Apr 15, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants