"Correct this" should not mean "break this"

[phh...@gmail.com]

Correct this "&" to "&&" is sometimes dangerous advice.

if (nullable != null & nullable.length() > 0) print("exists and not empty (must use short circuit with && to avoid NPE)");

but

if (youMustCallMe() & youMustCallMe2()) print("called you both successfully (short circuit with && would be wrong)");

 

Short-circuit logic "should" be used in boolean contexts (squid:S2178). Not always!

[Michael Gumowski]

Hello Philip,

Can you specify in which programming language your example is, what is your version of SonarLint and, if you are using SonarLint in connected mode, the version of the language analyzer.

i'm also having trouble getting the point with your message. What is your question or expectation?

Thank you,

Michael

PS: Note that we usually appreciate here usual marks of politeness in this ML, such as "hello" or "thank you".

--
You received this message because you are subscribed to the Google Groups "SonarLint" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarlint+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarlint/bb268e1f-3214-4425-a7e9-f3866ba03fe3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Michael Gumowski | SonarSource

Software Developer, Language Team
https://www.sonarsource.com

[Phil H]

Hi Michael

My post is tagged "java". Perhaps I should have mentioned that in the text as well.

The SonarLint plug-in version is of course whatever was currently stable as found in the eclipse marketplace a few days ago. I have not seen an update available notification since installing it.

The issue I am trying to report is that this SonarLint rule is telling me to "correct this", but if I do so, it *breaks* code.
I gave two examples: one where the rule is giving good advice (to avoid an NPE), and one where the advice is bad (method silently not executed). The whole point of using the bitwise logical operator, if it is intentional, and not just a typo, is to combine the result of evaluating *both* operands. The logical short circuit operator does not necessarily evaluate the second operand. That may or may not be desirable. It is *not* always a "correction".

The rule should say something like "Consider using short circuit evaluation with "&&"instead of  "&". Use parentheses or "&=" if evaluating the second operand really is intended.

Apologies for any perceived lack of politeness. It was certainly not intended.

Regards Phil

[Matthias Kolley]

What about:

final boolean callMeResult = youMustCallMe();
final boolean callMe2Result = youMustCallMe2();
if (callMeResult && callMe2Result) {
  //do your work here
}

[Phil H]

@Matthias Kolley

SonarLint just told me bluntly to: Correct "&" to "&&" (silently and catastrophically breaking the original logic in doing so).
It did not tell me first to preserve the logic with the refactoring "extract variable" (which is IMO less elegant, even if we omit final).

Regards & Hello

[Tibor Blenessy]

Hello,

we discussed this again, and agreed that this issue should not be raised when there is possible side-effect. I created ticket to handle this issue

https://jira.sonarsource.com/browse/SONARJAVA-2589 

Regards

Tibor

--
You received this message because you are subscribed to the Google Groups "SonarLint" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarlint+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarlint/cb9ac0bb-f315-4320-b80b-fdce98499dc0%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Tibor Blenessy | SonarSource

SonarJava Developer

https://sonarsource.com