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 S2758: false positive on conditional operator with interpolated string #828

Closed
BrightLight opened this issue Oct 13, 2017 · 2 comments
Closed
Assignees
Labels
Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@BrightLight
Copy link

Description

The following code gets flagged by S2758: "This operation returns the same value whether the condition is 'true' or 'false'" (this code is part of a class that generates a code file)

image

This seems like a false positive. I think it returns either
"return this.somefieldname;"
or
"return this.somefieldname.Value;"
These are two different strings.

Repro steps

Source code to copy into project for scanning:

var fieldName = this.GetGeneratedFieldName(member, generationStrategy);
return !this.HasDependentForeignKeyProperty(member)
    ? $"return this.{fieldName};"
    : $"return this.{fieldName}.Value;";

Expected behavior

No issue is created as this clearly returns two different strings.

Actual behavior

S2758: "This operation returns the same value whether the condition is 'true' or 'false'" is reported

Related information

  • SonarC# Version 6.4.1 (build 3596)
  • MSBuild 14.0.23107.0
  • Scanner for MSBuild 3.0.2.656 (via Jenkins)
@Evangelink
Copy link
Contributor

Hi @BrightLight,
Thanks for the feedback. We are able to reproduce the issue with latest version using your code sample.

@Evangelink Evangelink added Area: Rules Type: False Positive Rule IS triggered when it shouldn't be. labels Oct 13, 2017
@Evangelink Evangelink changed the title Rule S2758: false positive on conditional operator with interpolated string Fix S2758: false positive on conditional operator with interpolated string Oct 13, 2017
@Evangelink Evangelink self-assigned this Oct 16, 2017
@Evangelink Evangelink added this to the 6.6 milestone Oct 16, 2017
@Evangelink Evangelink assigned Evangelink and unassigned Evangelink Oct 18, 2017
@Evangelink
Copy link
Contributor

Hi @BrightLight,

We currently rely on Roslyn to detect whether the 2 parts are equals or not. The problem is that we are still using Roslyn 1.0 which doesn't handle interpolated strings hence this False Positive.
We are planning to move to newer versions of Roslyn soon (to support .Net Core) and so this ticket will be automatically fixed.

So I am closing the ticket as we won't do anything on older versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

No branches or pull requests

2 participants