False-positive: Constants should come first in equality tests (cpp:S1772)

Rules and Languages Report False-positive / False-negative...
1
  • What language is this for? C++
  • Which rule? Constants should come first in equality tests (cpp:S1772)
  • Why do you believe it’s a false-positive/false-negative?
    Left side is a (const) RValue! Using only one = (as in issue documentation) already gives a compiler error
  • Are you using
    • SonarCloud?
    • SonarQube - which version?
    • SonarLint - which IDE/version? VSCode, v4.1.0
      • in connected mode with SonarQube or SonarCloud? yes. SQ.
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
size_t count()
{
   return 0;
}
bool empty()
{
   return count() == 0;
}

image
image760×225 10.9 KB

3

Hi @KUGA2,
Thanks for raising this issue, it is indeed a false positive.
Is this rule valuable for you? Currently, the rule is not part of SonarWay and we are not convinced of its interest for C++ projects, as accidental assignments in conditions are already detected. Thus we were thinking about deprecating it altogether.

5

It is active, and as long as it is not removed, we will probably also not remove it even if its deprecated.

Also, we are running on pretty old compilers (gcc 4) for some builds that would not detect the problems mentioned in the SQ description.

So I would rather have it fixed, even though flipping removes the issue for this finding.

6

Thanks for your answer. I added it as a data point for our reflection on the future of this rule.

Regarding the scenario you mention, it is true that because count() is of type size_t, an assignment would fail and in that sense, it is a false positive.

But if count() was returning something else than a builtin, the risk would be back. In the same way, the rule will raise on return myEnum == EnumClass::Value; even though using a = would not compile because an enum class cannot be converted to a boolean.

The value of this rule is to make the code more trustable by adding a simple coding convention. If the convention becomes heterogeneous, depending on whether an assignment would compile or not, that will add a serious cognitive cost. Thus I’m not sure allowing that would go in the right direction.

On the other hand, my position is quite theoretical, since I’m not using this convention in my own code, so feel free to tell me if you disagree with this vision.

1 Like
8

I switched the values. I don`t care anymore. I still think it would be an improvement to the rule if rvalues on the left side are considered equivalent to const. I just wanted to point this out to you and not start another discussion…