search

ASSERT-KTH / sorald

Automatic repair system for static analysis warnings from SonarQube's SonarJava, TDSC 2022 http://arxiv.org/pdf/2103.12033
MIT License
90 stars 27 forks source link

Bug in S1155 #1025

Closed Zuplyx closed 1 year ago

Zuplyx commented 1 year ago

The processor for rule S1155 only checks for the presence of the == operator when deciding whether to negate the call to Collections.isEmpty(). This leads to wrong code being generated for statements like collection.size() < 1:

Repair generated by Sorald:

- collection.size() < 1
+ !collection.isEmpty()

Correct repair:

- collection.size() < 1
+ collection.isEmpty()
algomaster99 commented 1 year ago

Hi @Zuplyx ! Thanks for the bug report, and I agree we should also check for collection.size() < 1 because it is completely inline with their definition:

The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n).

I will work on the fix, meanwhile could you report this to SonarSource as well? They tend to respond quickly. See https://community.sonarsource.com/t/s1481-unused-local-variables-should-be-removed-is-wrongly-documented/89427.

Zuplyx commented 1 year ago

Hi @algomaster99, I am unsure what to report to SonarSource, as collection.size() < 1 is already being detected as a violation by SonarJava. IMHO the issue lies entirely within Sorald, because it's repair wrongly generates a negated call (!collection.isEmpty()) when it should be just collection.isEmpty().

algomaster99 commented 1 year ago

IMHO the issue lies entirely within Sorald

Oh right, if we are repairing it, SonarSource must have reported the warning. I agree there is no need. I will produce a fix.

Zuplyx commented 1 year ago

Thank you.

algomaster99 commented 1 year ago

This issue has been fixed in https://github.com/ASSERT-KTH/sorald/releases/tag/sorald-0.8.3. Thank you for the report!

Zuplyx commented 1 year ago

Thanks for the quick fix.