Skip to content

Commit

Permalink
Fix Quality Flaws: Fix issues from rule S1132 (String equality test s…
Browse files Browse the repository at this point in the history
…ide) (#4209)
  • Loading branch information
Wohops authored Nov 1, 2022
1 parent da2ce7c commit 38560b8
Show file tree
Hide file tree
Showing 41 changed files with 54 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static void saveIssue(SensorContext context, ExternalRuleLoader ruleLoade
.message(message)
.on(inputFile);

if (!line.isEmpty() && !line.equals("0")) {
if (!line.isEmpty() && !"0".equals(line)) {
primaryLocation.at(inputFile.selectLine(Integer.parseInt(line)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private void processFileAndCacheIfApplicable(InputFileScannerContext context, @N
writePackageNameToCache(context, packageName == null ? "" : packageName);
}

if (packageName == null || packageName.equals("")) {
if (packageName == null || packageName.isEmpty()) {
// default package
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ private static boolean isAnnotatedNonnull(MethodInvocationTree mit) {
.stream()
.map(SymbolMetadata.AnnotationInstance::symbol)
.map(Symbol::name)
.anyMatch(name -> name.equalsIgnoreCase("nonNull") || name.equalsIgnoreCase("notNull"));
.anyMatch(name -> "nonNull".equalsIgnoreCase(name) || "notNull".equalsIgnoreCase(name));
}

private static List<JavaQuickFix> getQuickFix(ExpressionTree tree, ExpressionTree boxedBoolean) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ private static boolean isStar(ExpressionTree expressionTree) {
if (expressionTree.is(Tree.Kind.NEW_ARRAY)) {
return ((NewArrayTree) expressionTree).initializers().stream().anyMatch(CORSCheck::isStar);
} else {
String value = ExpressionsHelper.getConstantValueAsString(expressionTree).value();
return value != null && value.equals("*");
return "*".equals(ExpressionsHelper.getConstantValueAsString(expressionTree).value());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.Tree;
Expand Down Expand Up @@ -160,8 +159,7 @@ private static boolean isChronoFieldWeek(ExpressionTree argument) {
if (!select.symbolType().is("java.time.temporal.ChronoField")) {
return false;
}
IdentifierTree identifier = select.identifier();
return identifier.name().equals("ALIGNED_WEEK_OF_YEAR");
return "ALIGNED_WEEK_OF_YEAR".equals(select.identifier().name());
}

private static boolean refersToYear(ExpressionTree argument) {
Expand All @@ -179,9 +177,8 @@ private static boolean isWeekBasedYearUsed(ExpressionTree argument) {
private static boolean isChronoFieldYear(ExpressionTree argument) {
if (argument.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree select = (MemberSelectExpressionTree) argument;
IdentifierTree identifier = select.identifier();
return select.symbolType().is("java.time.temporal.ChronoField") &&
(identifier.name().equals("YEAR") || identifier.name().equals("YEAR_OF_ERA"));
String name = select.identifier().name();
return select.symbolType().is("java.time.temporal.ChronoField") && ("YEAR".equals(name) || "YEAR_OF_ERA".equals(name));
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ public class DiamondOperatorCheck extends SubscriptionVisitor implements JavaVer
Tree.Kind.VARIABLE,
Tree.Kind.TYPE_CAST,
Tree.Kind.RETURN_STATEMENT,
Tree.Kind.ASSIGNMENT};
private static final Tree.Kind[] JAVA_8_KINDS = (Tree.Kind[]) ArrayUtils.addAll(JAVA_7_KINDS, new Tree.Kind[] {
Tree.Kind.CONDITIONAL_EXPRESSION});
Tree.Kind.ASSIGNMENT
};
private static final Tree.Kind[] JAVA_8_KINDS = ArrayUtils.add(JAVA_7_KINDS, Tree.Kind.CONDITIONAL_EXPRESSION);
private Tree.Kind[] expressionKindsToCheck = JAVA_7_KINDS;

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ private static boolean isLoopbackAddress(String ip) {
}

private static boolean isNonRoutableAddress(String ip) {
return ip.equals("0.0.0.0") || IP_V6_NON_ROUTABLE.matcher(ip).matches();
return "0.0.0.0".equals(ip) || IP_V6_NON_ROUTABLE.matcher(ip).matches();
}

private static boolean isBroadcastAddress(String ip) {
return ip.equals("255.255.255.255");
return "255.255.255.255".equals(ip);
}

private static Optional<String> extractIPV4(String value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ private static boolean isTrivialIfStatement(StatementTree node) {
protected static boolean hasDefaultClause(SwitchTree switchStatement) {
return switchStatement.cases().stream()
.flatMap(caseGroupTree -> caseGroupTree.labels().stream())
.anyMatch(caseLabelTree -> caseLabelTree.caseOrDefaultKeyword().text().equals("default"));
.anyMatch(caseLabelTree -> "default".equals(caseLabelTree.caseOrDefaultKeyword().text()));
}

protected static boolean hasElseClause(IfStatementTree ifStatement) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ protected MethodMatchers getMethodInvocationMatchers() {
return MethodMatchers.or(
MethodMatchers.create().ofSubTypes("java.util.concurrent.locks.Lock").names("tryLock").addWithoutParametersMatcher().build(),
MethodMatchers.create().ofTypes(FILE)
.name(name -> name.equals("delete") || name.equals("exists") || name.equals("createNewFile") ||
.name(name -> "delete".equals(name) || "exists".equals(name) || "createNewFile".equals(name) ||
name.startsWith("can") || name.startsWith("is"))
.addWithoutParametersMatcher()
.build(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private static boolean mayBeCollectingIntoVariable(MethodInvocationTree mit) {

private static boolean isConstructor(ExpressionTree tree) {
if (tree.is(Tree.Kind.METHOD_REFERENCE)) {
return ((MethodReferenceTree) tree).method().name().equals("new");
return "new".equals(((MethodReferenceTree) tree).method().name());
}
return (tree.is(Tree.Kind.LAMBDA_EXPRESSION))
&& ((LambdaExpressionTree) tree).body().is(Tree.Kind.NEW_CLASS, Tree.Kind.NEW_ARRAY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ private static Optional<String> getClassIdentifier(ExpressionTree expression) {
ExpressionTree originalExpression = ExpressionUtils.skipParentheses(expression);
if (originalExpression.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) originalExpression;
if (memberSelect.identifier().name().equals("class")) {
if ("class".equals(memberSelect.identifier().name())) {
ExpressionTree selectedExpression = ExpressionUtils.skipParentheses(memberSelect.expression());
return getName(selectedExpression);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private static Optional<ExpressionTree> findUseArgument(AnnotationTree annotatio
for (ExpressionTree tree : annotationTree.arguments()) {
if (tree.is(Tree.Kind.ASSIGNMENT)) {
AssignmentExpressionTree assignment = (AssignmentExpressionTree) tree;
if (((IdentifierTree) assignment.variable()).name().equals("use")
if ("use".equals(((IdentifierTree) assignment.variable()).name())
&& isJsonTypeIdEnumValue(assignment.expression())) {
return Optional.of(assignment.expression());
}
Expand All @@ -102,7 +102,7 @@ private static boolean isJsonTypeIdEnumValue(ExpressionTree tree) {
} else {
valueName = ((IdentifierTree) tree).name();
}
return valueName.equals("CLASS") || valueName.equals("MINIMAL_CLASS");
return "CLASS".equals(valueName) || "MINIMAL_CLASS".equals(valueName);
}

private static boolean isJsonTypeId(ExpressionTree tree) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private void checkField(Symbol.TypeSymbol clazz, ExpressionTree initializer) {
private static Symbol classLiteral(ExpressionTree expression) {
if (expression.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree mset = (MemberSelectExpressionTree) expression;
if (mset.identifier().name().equals("class")) {
if ("class".equals(mset.identifier().name())) {
return mset.expression().symbolType().symbol();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.sonar.check.Rule;
import org.sonar.java.model.ExpressionUtils;
import org.sonar.java.model.LiteralUtils;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;
Expand Down Expand Up @@ -148,7 +149,7 @@ public void visitReturnStatement(ReturnStatementTree tree) {
private void checkReturnedExpression(ExpressionTree expression) {
if (expression.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree mse = (MemberSelectExpressionTree) expression;
if (isThis(mse.expression())) {
if (ExpressionUtils.isThis(mse.expression())) {
checkReturnedExpression(mse.identifier());
}
}
Expand All @@ -160,10 +161,6 @@ private void checkReturnedExpression(ExpressionTree expression) {
}
}

private static boolean isThis(ExpressionTree expression) {
return expression.is(Tree.Kind.IDENTIFIER) && ((IdentifierTree) expression).name().equals("this");
}

private static boolean isOnlyAssignedImmutableVariable(Symbol.VariableSymbol symbol) {
VariableTree declaration = symbol.declaration();
if (declaration != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ protected MethodMatchers getMethodInvocationMatchers() {
@Override
protected void onMethodInvocationFound(MethodInvocationTree mit) {
String methodName = mit.symbol().name();
if (methodName.equals("isPresent")) {
if ("isPresent".equals(methodName)) {
handleIsPresent(mit);
} else if (methodName.equals("anyMatch")) {
} else if ("anyMatch".equals(methodName)) {
handleAnyMatch(mit);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) {
// For now ignore everything that is not a .class expression
if (expressionTree.is(Tree.Kind.MEMBER_SELECT)) {
MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) expressionTree;
boolean isClassIdentifier = memberSelect.identifier().name().equals("class");
boolean isClassIdentifier = "class".equals(memberSelect.identifier().name());
Type symbolType = memberSelect.expression().symbolType();
if (isClassIdentifier && !symbolType.isUnknown() && isNotRuntimeAnnotation(symbolType)) {
reportIssue(expressionTree, "\"@" + symbolType.name() + "\" is not available at runtime and cannot be seen with reflection.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void visitNode(Tree tree) {

public static boolean usesColons(SwitchTree tree) {
return !tree.cases().isEmpty() &&
tree.cases().get(0).labels().get(0).colonOrArrowToken().text().equals(":");
":".equals(tree.cases().get(0).labels().get(0).colonOrArrowToken().text());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private void checkThreadLocalField(Symbol field) {
private static boolean usageIsRemove(IdentifierTree usage) {
return MethodTreeUtils.consecutiveMethodInvocation(usage)
// At this point, we know that "usage" is of type ThreadLocal, we don't have to check the full type, the name is enough.
.filter(mit -> ExpressionUtils.methodName(mit).name().equals("remove"))
.filter(mit -> "remove".equals(ExpressionUtils.methodName(mit).name()))
.isPresent();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ public void visitNode(Tree tree) {
} else if (interestingMethodName != null) {
ExpressionTree returnExpression = ExpressionUtils.skipParentheses(((ReturnStatementTree) tree).expression());
if (returnExpression.is(Kind.NULL_LITERAL)) {
if (interestingMethodName.equals("toString")) {
reportIssue(returnExpression, "Return empty string instead.");
} else {
reportIssue(returnExpression, "Return a non null object.");
}
reportIssue(returnExpression, "toString".equals(interestingMethodName) ? "Return empty string instead." : "Return a non null object.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private boolean isImportFromSamePackage(String importName, Tree tree) {
Tree qualifiedIdentifier = ((ImportTree) tree).qualifiedIdentifier();
// Defensive programming, the qualifiedIdentifier should always be a MemberSelectTree.
if (qualifiedIdentifier.is(Tree.Kind.MEMBER_SELECT) &&
((MemberSelectExpressionTree) qualifiedIdentifier).identifier().name().equals("*")) {
"*".equals(((MemberSelectExpressionTree) qualifiedIdentifier).identifier().name())) {
return importName.equals(currentPackage);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private boolean isMultiAssignment(VariableTree variableTree) {
}
typeAssignmentLine = line;
SyntaxToken token = variableTree.endToken();
return token != null && token.text().equals(",");
return token != null && ",".equals(token.text());
}

private static boolean isArrayInitializerWithoutType(ExpressionTree initializer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private static boolean isMisusedVisibleForTesting(Symbol symbol) {
}

private static boolean isVisibleForTestingAnnotation(AnnotationInstance annotationInstance) {
return annotationInstance.symbol().name().equals("VisibleForTesting");
return "VisibleForTesting".equals(annotationInstance.symbol().name());
}

private static boolean inTheSameFile(Symbol symbol) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) {
MethodSymbol methodSymbol = (MethodSymbol) mit.symbol();
Symbol parentClass = Optional.ofNullable(methodSymbol.owner()).orElse(Symbols.unknownTypeSymbol);
Symbol.TypeSymbol returnType = methodSymbol.returnType();
if (!returnType.isUnknown() && parentClass.name().equals("Builder")) {
if (!returnType.isUnknown() && "Builder".equals(parentClass.name())) {
String returnTypeName = returnType.type().fullyQualifiedName();
// only focus on method of "Builder" class returning itself
if (!returnTypeName.equals(parentClass.type().fullyQualifiedName())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,10 @@ private static boolean setsInvocationTypeToAsync(MethodInvocationTree methodCall
if (INVOCATIONTYPE_MATCHERS.matches(methodCall)) {
// From the matcher we know there is an argument and it is a string.
String stringVal = ExpressionsHelper.getConstantValueAsString(arguments.get(0)).value();
if (stringVal != null) {
return stringVal.equals("Event") || stringVal.equals("DryRun");
} else {
return "Event".equals(stringVal)
|| "DryRun".equals(stringVal)
// Could not get the string real value, therefore sync calls are out of the picture.
return true;
}
|| stringVal == null;
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ public final class UnitTestUtils {
"com.jayway.restassured.response.ValidatableResponseOptions", //restassured 2.x
"io.restassured.response.ValidatableResponseOptions" //restassured 3.x and 4.x
)
.name(name -> name.equals("body") ||
name.equals("time") ||
.name(name -> "body".equals(name) ||
"time".equals(name) ||
name.startsWith("time") ||
name.startsWith("content") ||
name.startsWith("status") ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public final void reportIssue(Tree javaTree, String message, @Nullable Integer c
protected static Optional<ExpressionTree> getFlagsTree(ExpressionTree methodInvocationOrAnnotation) {
if (methodInvocationOrAnnotation.is(Tree.Kind.METHOD_INVOCATION)) {
MethodInvocationTree mit = (MethodInvocationTree) methodInvocationOrAnnotation;
if (mit.symbol().name().equals("compile") && mit.arguments().size() == 2) {
if ("compile".equals(mit.symbol().name()) && mit.arguments().size() == 2) {
return Optional.of(mit.arguments().get(1));
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ public void visitNode(Tree tree) {
if (SET_RESPONSE_HEADERS.matches(methodInvocationTree)) {
methodInvocationTree.arguments().get(0).asConstant(String.class)
.ifPresent(header -> {
if (header.equalsIgnoreCase("server") ||
header.equalsIgnoreCase("x-powered-by")) {
if ("server".equalsIgnoreCase(header) ||
"x-powered-by".equalsIgnoreCase(header)) {
reportIssue(methodInvocationTree, MESSAGE);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private void checkConfigurationFactoryExtension(ClassTree tree) {

@Override
protected void onMethodInvocationFound(MethodInvocationTree mit) {
if (mit.symbol().name().equals("setProperty")) {
if ("setProperty".equals(mit.symbol().name())) {
String stringConstant = ExpressionsHelper.getConstantValueAsString(mit.arguments().get(0)).value();
if ("logback.configurationFile".equals(stringConstant)) {
reportIssue(mit, MESSAGE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ private static boolean isSerialVersionUIDField(VariableTree variable) {
Symbol symbol = variable.symbol();
return symbol.isFinal() &&
symbol.type().is("long") &&
symbol.name().equals("serialVersionUID");
"serialVersionUID".equals(symbol.name());
}

private static boolean setsTheValueToZero(VariableTree variable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private static List<Symbol.MethodSymbol> constructors(ClassTree clazzTree) {
return clazzTree.symbol().memberSymbols().stream()
.filter(Symbol::isMethodSymbol)
.map(s -> (Symbol.MethodSymbol) s)
.filter(m -> m.name().equals("<init>"))
.filter(m -> "<init>".equals(m.name()))
.filter(m -> m.declaration() != null)
.collect(Collectors.toList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.sonar.plugins.java.api.tree.BlockTree;
import org.sonar.plugins.java.api.tree.ClassTree;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodTree;
Expand Down Expand Up @@ -121,8 +120,7 @@ private static boolean methodInvocationOnThisInstance(MethodInvocationTree metho
}
ExpressionTree expression = methodInvocation.methodSelect();
if (expression.is(Tree.Kind.MEMBER_SELECT)) {
expression = ((MemberSelectExpressionTree) expression).expression();
return expression.is(Tree.Kind.IDENTIFIER) && ((IdentifierTree) expression).name().equals("this");
return ExpressionUtils.isThis(((MemberSelectExpressionTree) expression).expression());
}
return expression.is(Tree.Kind.IDENTIFIER);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class AssertJConsecutiveAssertionCheck extends IssuableSubscriptionVisito
public static final MethodMatchers ASSERTJ_SET_CONTEXT_METHODS = MethodMatchers.create()
.ofSubTypes("org.assertj.core.api.AbstractAssert")
.name(name -> name.startsWith("extracting") || name.startsWith("using") || name.startsWith("filtered")
|| name.equals("flatExtracting") || name.equals("map") || name.equals("flatMap"))
|| "flatExtracting".equals(name) || "map".equals(name) || "flatMap".equals(name))
.withAnyParameters()
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) {

private void checkBooleanExpressionInAssertMethod(IdentifierTree problematicAssertionCallIdentifier, ExpressionTree argumentExpression) {
Optional<Assertion> replacementAssertionOpt = getReplacementAssertion(argumentExpression);
if (problematicAssertionCallIdentifier.name().equals("assertFalse")) {
if ("assertFalse".equals(problematicAssertionCallIdentifier.name())) {
replacementAssertionOpt = replacementAssertionOpt.map(COMPLEMENTS::get);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ private void checkCompatibleTypes(MethodInvocationTree mit, Argument actual, Arg

private static boolean isNotEqualsInTestRelatedToEquals(MethodInvocationTree mit) {
String methodName = ExpressionUtils.methodName(mit).name();
return (methodName.equals(ASSERT_NOT_EQUALS) || methodName.equals("isNotEqualTo")) &&
return (ASSERT_NOT_EQUALS.equals(methodName) || "isNotEqualTo".equals(methodName)) &&
UnitTestUtils.isInUnitTestRelatedToObjectMethods(mit);
}

Expand Down
Loading

0 comments on commit 38560b8

Please sign in to comment.