S1905 - False Positives generated within c# ternary operator statement

Clean Code Report False-positive / False-negative...
1

The analyzer generates “warning S1125: Remove the unnecessary Boolean literal(s).” on the line of code for the ‘required’ boolean assignment. In the code snippet below, ‘item’ object is an optional (can be null) ‘to be cloned’ item of DataItem type. Code intent is 'use values from the cloned item, if there is one, otherwise use defaults. ‘titleHtml’ property is of string type (and no warning is generated), ‘required’ is a bool type. Looks like S1905 warning is caused by the first (true) part of the ternary statement, but since 2nd (false) ternary statement is not a ‘true’ literal, the warning really should not be shown.

            var newItem = new Datatem()
            {
                // Skipping code lines ....
                titleHtml = item == null ? string.Empty : item.titleHtml,
                required = item == null ? false : item.required
            };
2

Hi @garrick_s,

welcome to our community.

I’ve took a short look and as far as I can tell the solution suggested by the code fix provider required = !(item == null) && item.required respects the rule specifications.

3

Costin - the issue as described is with a false positive on the string assignment line:

4

Hi @garrick_s, I think I don’t understand what you mean.

This snapshot is from my attempt to reproduce the issue using the latest version of the analyzer (8.9):

image
image1221×805 36.2 KB

As you can see, the issue is raised on the false token from required = item == null ? false : item.required. line which is a false literal expression.

image
image941×729 67.2 KB

5

image
image923×150 11.1 KB

@costin.zaharia here is another example of a FP on that.

Yes it is a boolean literal, however that is not the point. Contrary to the rule violation text, this literal is 100% necessary. Removing it makes the code take the wrong condition and creates many possible bugs. For me it reports that its S1125 and no S1905.

I would like to know in both this case and the OP’s, if this code is wrong, then what is SonarSource’s idea of the correct and clearer/easier to read and understand code that should be getting used instead? To me this just looks like the incorrect/broken redundant boolean evaluation of S1125 is leaking further.

6

Hi @StingyJack,

I would say the OP’s case is not clear since he did not reply yet to confirm or not the behavior I’ve managed to reproduce.

Regarding your case, I think there is place for improvement for this rule. The code can be rewritten to:

var x = string.IsNullOrWhiteSpace()
                 ? throw ...
                 : options.Service...

but this is not what the message is suggesting.

Thanks for the feedback. I’ve created Rule S1125: FP when using ternary operator · Issue #4465 · SonarSource/sonar-dotnet · GitHub so you can track the progress on GitHub.