Bug: S1125 code fix does not add ! when transforming x == false.

[urmel9]

• Operating system: Microsoft Visual Studio Community 2022 (64-Bit) - Current
  Version 17.7.0
• SonarLint plugin version: 9.8.0.76515
• Programming language you're coding in: C#
• Is connected mode used: No
• Link to SonarCommunity

Hello,
we use SonarLint with StyleCop in our company. Because there are many small warning that could be autofixed on save, we use the feature "clean-code on save".

Here is the problem:
The Autofix of S1125 is wrong. The rule and the description are definitely correct, but the autofix function is wrong. It changes if (testBool == false) to if (testBool). Screenshot are attached.

This is the fix:
Autofix should have this result:
if (!testBool)

We had some bugs because SonarLint changed it wrong and is missing the exclamation mark on negative boolean literals. Could you please fix this? We will use the rule quick then again. Thank you! :slight_smile:

Before:

After clean-code on save

[mary-georgiou-sonarsource]

Hello @urmel9 and thanks for your report.
I confirm this is a bug.
I added this issue to our backlog to tackle as soon as we can.

[urmel9]

Hello @mary-georgiou-sonarsource are there any updates about this? Is it fixed now, or do I have to wait until the next release?
Thank you! :)

[mary-georgiou-sonarsource]

Hello, @urmel9!
Not yet - we need to plan to fix this during a future sprint.
Please follow the issue so you'll be updated once it's fixed!

[msedi]

@mary-georgiou-sonarsource @pavel-mikula-sonarsource Just a question, why has the Bug label be removed? I think it is a bug, right?

[urmel9]

@msedi I agree. It is unexpected behavior and leads to bug in your own code, when you use autoformat. Even the sugestion is confusing.

[mary-georgiou-sonarsource]

We use the bug label only for issues related to one of our analyzers throwing AD0001- NullReferenceException.

Edit: you can find the label descriptions here https://github.com/SonarSource/sonar-dotnet/labels

[HipDragon]

Any idea of when this will be fixed?

It would be nice if this autofix was disabled while this was being resolved. Better to leave the code alone and working as developers originally wrote it. Then to have developers tempted to use the autofix and have it possibility change the logic of the code.

If there is a way to disable specific autofixes in SonarLint I would like to know. Thanks in advance.

[richardissimo]

I've just discovered this issue myself in the "==false" scenario, where the fixer will simply remove that part of the condition, which inverts the logic, causing the code to not behave as intended.

@mary-georgiou-sonarsource I would suggest that the original title is far more suitable "Autofix of S1125 in C# is wrong and leads to bad results" - that makes it clear that it creates a problem. The current title does not give any urgency to this problem, which is clearly not being given any priority.

To answer the question about how to suppress this, This isn't mentioned at all in the Disabling a rule documentation, and the settings.json file format and location section gives no clues or examples how to format what you need. But you can suppress it in an ".editorconfig", which is potentially a better solution, as it allows the settings to be configured per repository (in version control, used by everyone) rather than per user. You just need the following line in an ".editorconfig" file alongside your solution, in the [*.cs] section:

dotnet_diagnostic.S1125.severity = none

One final point... as much as the "== true" is clearly redundant, and should be removed, I have to say that the "== false" scenario feels like it should be a completely different rule than "==true". For example although the "!" operator is perfectly valid syntax, it can visually disappear into the brackets, making the reader misunderstand. For example, compare:

if (testBool)

if (!testBool)

And this issue is more pronounced when the logic is more complex, and isn't the only thing in the condition. So I'd be happy to have a rule highlighting the "==true" redundancy; but not the "==false".

[urmel9]

@richardissimo Thank you for your detailed answer.

@HipDragon Imho you it feels wrong to turn off a whole rule because the autofix is wrong. Unfortunately, right now, there isn't a possibility to turn off the autofix. I created an issue for Roslyn because SonarLint said it isn't their problem

[mary-georgiou-sonarsource]

Hello everyone and thank you for your comments.
We increased the priority of this issue and we will tackle it in the next 3-4 months during our next planned hardening sprint.
Thank you!

[p1luz]

Hello everyone,
is this bugfix also planned to be released in the VSIX package for Visual Studio extensions?

[costin-zaharia-sonarsource]

Hi @p1luz,

Do you mean SonarLint for Visual Studio?

If yes, this version should be included in the next version. I do not have though an ETA yet.

[p1luz]

@mary-georgiou-sonarsource @costin-zaharia-sonarsource yes, that one.
Ok, thanks