Skip to content

Commit

Permalink
SONARJAVA-5111 S5838 Improve quickfix directly to isEmpty()
Browse files Browse the repository at this point in the history
  • Loading branch information
pauloreilly-sonarsource authored Sep 20, 2024
1 parent 9bfb7e6 commit 8333a7d
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S5838",
"hasTruePositives": false,
"falseNegatives": 274,
"falseNegatives": 280,
"falsePositives": 0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ void stringRelatedAssertionChains() {

assertThat(getString().trim()).isNotEmpty(); // Noncompliant {{Use assertThat(actual).isNotBlank() instead.}}
assertThat(getString().trim()).isNotEqualTo(""); // Noncompliant {{Use assertThat(actual).isNotBlank() instead.}}
assertThat(getString().length()).isEqualTo(0); // Noncompliant {{Use isZero() instead.}}
assertThat(getString().length()).isEqualTo(0); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}}
assertThat(getString().length()).isEqualTo(12); // Noncompliant {{Use assertThat(actual).hasSize(expected) instead.}}
assertThat(getString().length()).isEqualTo(x.length()); // Noncompliant {{Use assertThat(actual).hasSameSizeAs(expected) instead.}}
assertThat(getString().length()).isEqualTo(getArray().length); // Noncompliant {{Use assertThat(actual).hasSize(expected) instead.}}
Expand All @@ -258,7 +258,7 @@ void arrayRelatedAssertionChains() {
int i = 12;
MyClassWithLength myClassWithLength = new MyClassWithLength();

assertThat(getArray().length).isEqualTo(0); // Noncompliant {{Use isZero() instead.}}
assertThat(getArray().length).isEqualTo(0); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}}
assertThat(getArray().length).isZero(); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}}
assertThat(getArray().length).isEqualTo(i); // Noncompliant {{Use assertThat(actual).hasSize(expected) instead.}}
assertThat(getArray().length).isPositive(); // Noncompliant {{Use assertThat(actual).isNotEmpty() instead.}}
Expand Down Expand Up @@ -380,7 +380,7 @@ void fileRelatedAssertionChains() {
assertThat(getFile().listFiles()).isNotEmpty(); // Noncompliant {{Use assertThat(actual).isNotEmptyDirectory() instead.}}

// We report only step by step, not the final transformation possible
assertThat(getFile().list().length).isEqualTo(0); // Noncompliant {{Use isZero() instead.}}
assertThat(getFile().list().length).isEqualTo(0); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}}
assertThat(getFile().list().length).isZero(); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}}
assertThat(getFile().list()).isEmpty(); // Noncompliant {{Use assertThat(actual).isEmptyDirectory() instead.}}
assertThat(getFile()).isEmptyDirectory(); // Compliant, 3 iterations to reach a nice assertion
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package checks.tests;

import java.util.ArrayList;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;

public class AssertJChainSimplificationCheckTest_ListQuickFix {

List<String> stringsList = new ArrayList<>();
String[] stringArray = new String[0];
String string = new String();

// test https://sonarsource.atlassian.net/browse/SONARJAVA-5111
void contextFreeQuickFixes() {

assertThat(stringsList).isEmpty(); // Compliant
assertThat(stringsList.hashCode()).isEqualTo(0); // Noncompliant {{Use isZero() instead.}}

assertThat(stringsList.size()).isEqualTo(0); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}} [[quickfixes=qf_context1]]
// ^^^^^^^^^
// fix@qf_context1 {{Use "assertThat(actual).isEmpty()"}}
// edit@qf_context1 [[sc=27;ec=34]] {{}}
// edit@qf_context1 [[sc=36;ec=48]] {{isEmpty()}}

assertThat(string).isEmpty(); // Compliant
assertThat(string.hashCode()).isEqualTo(0); // Noncompliant {{Use isZero() instead.}}

assertThat(string.length()).isEqualTo(0); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}} [[quickfixes=qf_context2]]
// ^^^^^^^^^
// fix@qf_context2 {{Use "assertThat(actual).isEmpty()"}}
// edit@qf_context2 [[sc=22;ec=31]] {{}}
// edit@qf_context2 [[sc=33;ec=45]] {{isEmpty()}}

assertThat(stringArray).isEmpty(); // Compliant
assertThat(stringArray.hashCode()).isEqualTo(0); // Noncompliant {{Use isZero() instead.}}

assertThat(stringArray.length).isEqualTo(0); // Noncompliant {{Use assertThat(actual).isEmpty() instead.}} [[quickfixes=qf_context3]]
// ^^^^^^^^^
// fix@qf_context3 {{Use "assertThat(actual).isEmpty()"}}
// edit@qf_context3 [[sc=27;ec=34]] {{}}
// edit@qf_context3 [[sc=36;ec=48]] {{isEmpty()}}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ private AssertJChainSimplificationIndex() {
withSubjectArgumentCondition(LiteralUtils::isTrue, AssertJChainSimplificationIndex::isNotObject, "isTrue()"),
withSubjectArgumentCondition(LiteralUtils::isFalse, AssertJChainSimplificationIndex::isNotObject, "isFalse()"),
withSubjectArgumentCondition(LiteralUtils::isEmptyString, AssertJChainSimplificationIndex::isNotObject, "isEmpty()"),
withSubjectArgumentCondition(AssertJChainSimplificationIndex::isZeroIntOrLong, AssertJChainSimplificationIndex::isStringLength, msgWithActual(IS_EMPTY)),
withSubjectArgumentCondition(AssertJChainSimplificationIndex::isZeroIntOrLong, AssertJChainSimplificationIndex::isCollectionSize, msgWithActual(IS_EMPTY)),
withSubjectArgumentCondition(AssertJChainSimplificationIndex::isZeroIntOrLong, AssertJChainSimplificationIndex::isArrayLength, msgWithActual(IS_EMPTY)),
withSubjectArgumentCondition(AssertJChainSimplificationIndex::isZeroIntOrLong, AssertJChainSimplificationIndex::isNotObject, "isZero()"),
methodCallInSubject(Matchers.TO_STRING, msgWithActualCustom("hasToString", "expectedString")),
methodCallInSubject(predicateArg -> hasMethodCallAsArg(predicateArg, Matchers.HASH_CODE), Matchers.HASH_CODE, msgWithActualExpected("hasSameHashCodeAs")),
Expand Down Expand Up @@ -435,7 +438,29 @@ public static boolean isNegOneIntOrLong(ExpressionTree tree) {
private static boolean isArrayLength(ExpressionTree expression) {
if (expression.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree memberSelectExpressionTree = (MemberSelectExpressionTree) expression;
return memberSelectExpressionTree.expression().symbolType().isArray() && LENGTH.equals(memberSelectExpressionTree.identifier().name());
if (memberSelectExpressionTree.expression().symbolType().isArray()) {
return LENGTH.equals(memberSelectExpressionTree.identifier().name());
}
}
return false;
}

private static boolean isCollectionSize(ExpressionTree expression) {
if (expression.is(Tree.Kind.METHOD_INVOCATION)) {
MethodInvocationTree invocation = (MethodInvocationTree) expression;
if (invocation.methodSelect().is(Tree.Kind.MEMBER_SELECT)) {
return Matchers.COLLECTION_SIZE.matches(invocation);
}
}
return false;
}

private static boolean isStringLength(ExpressionTree expression) {
if (expression.is(Tree.Kind.METHOD_INVOCATION)) {
MethodInvocationTree invocation = (MethodInvocationTree) expression;
if (invocation.methodSelect().is(Tree.Kind.MEMBER_SELECT)) {
return Matchers.STRING_LENGTH.matches(invocation);
}
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,13 @@ void test_quick_fixes() {
.withCheck(new AssertJChainSimplificationCheck())
.verifyIssues();
}

@Test
void test_list_quick_fixes() {
CheckVerifier.newVerifier()
.onFile(testCodeSourcesPath("checks/tests/AssertJChainSimplificationCheckTest_ListQuickFix.java"))
.withCheck(new AssertJChainSimplificationCheck())
.verifyIssues();
}

}

0 comments on commit 8333a7d

Please sign in to comment.