From b1d78673f73af9ee158534ff5c406997993f8e98 Mon Sep 17 00:00:00 2001 From: cushon Date: Thu, 12 May 2016 19:43:05 -0700 Subject: [PATCH] Consolidate checks for reference equality RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=122224435 --- .../AbstractReferenceEquality.java | 112 +++++++++++++ .../bugpatterns/NumericEquality.java | 75 +++------ .../bugpatterns/OptionalEquality.java | 76 ++------- .../ProtoStringFieldReferenceEquality.java | 51 ++---- .../bugpatterns/StringEquality.java | 147 ++---------------- .../testdata/StringEqualityPositiveCases.java | 33 +--- 6 files changed, 179 insertions(+), 315 deletions(-) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java new file mode 100644 index 00000000000..02c45fa5aa2 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java @@ -0,0 +1,112 @@ +/* + * Copyright 2015 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NONNULL; +import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULL; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; +import com.google.errorprone.dataflow.nullnesspropagation.Nullness; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; + +import com.sun.source.tree.BinaryTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.util.TreePath; + +/** + * Abstract implementation of a BugPattern that detects the use of reference equality to compare + * classes with value semantics. + * + *

See e.g. {@link NumericEquality}, {@link OptionalEquality}, + * {@link ProtoStringFieldReferenceEquality}, and {@link StringEquality}. + * + * @author cushon@google.com (Liam Miller-Cushon) + */ +public abstract class AbstractReferenceEquality extends BugChecker implements BinaryTreeMatcher { + + protected abstract boolean matchArgument(ExpressionTree tree, VisitorState state); + + @Override + public final Description matchBinary(BinaryTree tree, VisitorState state) { + switch (tree.getKind()) { + case EQUAL_TO: + case NOT_EQUAL_TO: + break; + default: + return Description.NO_MATCH; + } + if (tree.getLeftOperand().getKind() == Kind.NULL_LITERAL + || !matchArgument(tree.getLeftOperand(), state)) { + return Description.NO_MATCH; + } + if (tree.getRightOperand().getKind() == Kind.NULL_LITERAL + || !matchArgument(tree.getRightOperand(), state)) { + return Description.NO_MATCH; + } + + Description.Builder builder = buildDescription(tree); + addFixes(builder, tree, state); + return builder.build(); + } + + protected void addFixes(Description.Builder builder, BinaryTree tree, VisitorState state) { + ExpressionTree lhs = tree.getLeftOperand(); + ExpressionTree rhs = tree.getRightOperand(); + + // Swap the order (e.g. rhs.equals(lhs) if the rhs is a non-null constant, and the lhs is not + if (ASTHelpers.constValue(lhs) == null && ASTHelpers.constValue(rhs) != null) { + ExpressionTree tmp = lhs; + lhs = rhs; + rhs = tmp; + } + + String prefix = tree.getKind() == Kind.NOT_EQUAL_TO ? "!" : ""; + String lhsSource = state.getSourceForNode(lhs); + String rhsSource = state.getSourceForNode(rhs); + + Nullness nullness = getNullness(lhs, state); + + // If the lhs is possibly-null, provide both options. + if (nullness != NONNULL) { + builder.addFix( + SuggestedFix.builder() + .replace( + tree, String.format("%sObjects.equals(%s, %s)", prefix, lhsSource, rhsSource)) + .addImport("java.util.Objects") + .build()); + } + if (nullness != NULL) { + builder.addFix( + SuggestedFix.replace( + tree, + String.format( + "%s%s.equals(%s)", + prefix, + lhs instanceof BinaryTree ? String.format("(%s)", lhsSource) : lhsSource, + rhsSource))); + } + } + + private Nullness getNullness(ExpressionTree expr, VisitorState state) { + TreePath pathToExpr = new TreePath(state.getPath(), expr); + return state.getNullnessAnalysis().getNullness(pathToExpr, state.context); + } +} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/NumericEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/NumericEquality.java index 4bc0791de72..8a6391e9420 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/NumericEquality.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/NumericEquality.java @@ -17,77 +17,42 @@ import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; -import static com.google.errorprone.matchers.Matchers.allOf; -import static com.google.errorprone.matchers.Matchers.anyOf; -import static com.google.errorprone.matchers.Matchers.binaryTree; -import static com.google.errorprone.matchers.Matchers.isSubtypeOf; -import static com.google.errorprone.matchers.Matchers.kindIs; -import static com.google.errorprone.matchers.Matchers.not; -import static com.sun.source.tree.Tree.Kind.EQUAL_TO; -import static com.sun.source.tree.Tree.Kind.NOT_EQUAL_TO; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; -import com.google.errorprone.fixes.Fix; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.matchers.Description; -import com.google.errorprone.matchers.Matcher; import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.BinaryTree; import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.code.Symbol; /** * @author scottjohnson@google.com (Scott Johnson) */ -@BugPattern(name = "NumericEquality", - summary = "Numeric comparison using reference equality instead of value equality", - explanation = "Numbers are compared for reference equality/inequality using == or != " - + "instead of for value equality using .equals()", - category = JDK, severity = ERROR, maturity = EXPERIMENTAL) -public class NumericEquality extends BugChecker implements BinaryTreeMatcher { - - public static final Matcher SUBCLASS_OF_NUMBER = - allOf(isSubtypeOf("java.lang.Number"), not(kindIs(Tree.Kind.NULL_LITERAL))); - public static final Matcher MATCHER = allOf( - anyOf(kindIs(EQUAL_TO), kindIs(NOT_EQUAL_TO)), - binaryTree(SUBCLASS_OF_NUMBER, SUBCLASS_OF_NUMBER)); +@BugPattern( + name = "NumericEquality", + summary = "Numeric comparison using reference equality instead of value equality", + explanation = + "Numbers are compared for reference equality/inequality using == or != " + + "instead of for value equality using .equals()", + category = JDK, + severity = ERROR, + maturity = EXPERIMENTAL +) +public class NumericEquality extends AbstractReferenceEquality { @Override - public Description matchBinary(BinaryTree tree, VisitorState state) { - ExpressionTree leftOperand = tree.getLeftOperand(); - ExpressionTree rightOperand = tree.getRightOperand(); - Symbol left = ASTHelpers.getSymbol(leftOperand); - Symbol right = ASTHelpers.getSymbol(rightOperand); - if (left == null || right == null) { - return Description.NO_MATCH; - } - // Using a static final object as a sentinel is OK - if ((isFinal(left) && left.isStatic()) || (isFinal(right) && right.isStatic())) { - return Description.NO_MATCH; - } - // Match left and right operand to subclasses of java.lang.Number and not null - if (!MATCHER.matches(tree, state)) { - return Description.NO_MATCH; + protected boolean matchArgument(ExpressionTree tree, VisitorState state) { + if (!ASTHelpers.isSubtype( + ASTHelpers.getType(tree), state.getTypeFromString("java.lang.Number"), state)) { + return false; } - - StringBuilder fixedExpression = new StringBuilder(); - - if (tree.getKind() == Tree.Kind.NOT_EQUAL_TO) { - fixedExpression.append("!"); + Symbol sym = ASTHelpers.getSymbol(tree); + if (isFinal(sym) && sym.isStatic()) { + // Using a static final object as a sentinel is OK + return false; } - fixedExpression.append( - "Objects.equals(" + leftOperand + ", " + rightOperand + ")"); - - Fix fix = SuggestedFix.builder() - .replace(tree, fixedExpression.toString()) - .addImport("java.util.Objects") - .build(); - return describeMatch(tree, fix); + return true; } public static boolean isFinal(Symbol s) { diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/OptionalEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/OptionalEquality.java index 2d1c26e77a8..0c7f00fcf98 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/OptionalEquality.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/OptionalEquality.java @@ -19,21 +19,13 @@ import static com.google.errorprone.BugPattern.Category.GUAVA; import static com.google.errorprone.BugPattern.MaturityLevel.MATURE; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; -import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NONNULL; -import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULL; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; -import com.google.errorprone.dataflow.nullnesspropagation.Nullness; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.matchers.Description; import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.BinaryTree; import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.Tree.Kind; -import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Type; /** @author cushon@google.com (Liam Miller-Cushon) */ @@ -47,63 +39,19 @@ severity = ERROR, maturity = MATURE ) -public class OptionalEquality extends BugChecker implements BinaryTreeMatcher { +public class OptionalEquality extends AbstractReferenceEquality { - @Override - public final Description matchBinary(BinaryTree tree, VisitorState state) { - switch (tree.getKind()) { - case EQUAL_TO: - case NOT_EQUAL_TO: - break; - default: - return Description.NO_MATCH; - } - if (!isOptionalType(ASTHelpers.getType(tree.getLeftOperand()), state)) { - return Description.NO_MATCH; - } - if (!isOptionalType(ASTHelpers.getType(tree.getRightOperand()), state)) { - return Description.NO_MATCH; - } - - Description.Builder builder = buildDescription(tree); - addFixes(builder, tree, state); - return builder.build(); - } - - void addFixes(Description.Builder builder, BinaryTree tree, VisitorState state) { - String prefix = tree.getKind() == Kind.NOT_EQUAL_TO ? "!" : ""; - String lhs = state.getSourceForNode(tree.getLeftOperand()); - String rhs = state.getSourceForNode(tree.getRightOperand()); - Nullness nullness = getNullness(tree.getLeftOperand(), state); - - // If the lhs is possibly-null, provide both options. + private static final ImmutableSet OPTIONAL_CLASSES = + ImmutableSet.of(com.google.common.base.Optional.class.getName(), "java.util.Optional"); - // Never swap the order (e.g. rhs.equals(lhs) when lhs is nullable and rhs is non-null), since - // that has observable side-effects. - - if (nullness != NONNULL) { - builder.addFix( - SuggestedFix.builder() - .replace(tree, String.format("%sObjects.equals(%s, %s)", prefix, lhs, rhs)) - .addImport("java.util.Objects") - .build()); - } - if (nullness != NULL) { - builder.addFix( - SuggestedFix.replace(tree, String.format("%s%s.equals(%s)", prefix, lhs, rhs))); + @Override + protected boolean matchArgument(ExpressionTree tree, VisitorState state) { + Type type = ASTHelpers.getType(tree); + for (String className : OPTIONAL_CLASSES) { + if (ASTHelpers.isSameType(type, state.getTypeFromString(className), state)) { + return true; + } } - } - - private Nullness getNullness(ExpressionTree expr, VisitorState state) { - TreePath pathToExpr = new TreePath(state.getPath(), expr); - return state.getNullnessAnalysis().getNullness(pathToExpr, state.context); - } - - private static boolean isOptionalType(Type type, VisitorState state) { - Type guavaOptional = state.getTypeFromString(com.google.common.base.Optional.class.getName()); - Type utilOptional = state.getTypeFromString("java.util.Optional"); - // TODO(cushon): write a test for the util.Optional case once we can depend on 8. - return ASTHelpers.isSameType(type, guavaOptional, state) - || ASTHelpers.isSameType(type, utilOptional, state); + return false; } } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ProtoStringFieldReferenceEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/ProtoStringFieldReferenceEquality.java index 7aad5ba2d1b..2e8708658b2 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ProtoStringFieldReferenceEquality.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ProtoStringFieldReferenceEquality.java @@ -25,49 +25,30 @@ import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; -import com.sun.source.tree.BinaryTree; import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.Tree.Kind; -import com.sun.tools.javac.tree.JCTree; -@BugPattern(category = ONE_OFF, maturity = EXPERIMENTAL, - name = "ProtoStringFieldReferenceEquality", severity = ERROR, - summary = "Comparing protobuf fields of type String using reference equality", - explanation = "Comparing strings with == is almost always an error, but it is an error 100% " - + "of the time when one of the strings is a protobuf field. Additionally, protobuf " - + "fields cannot be null, so Object.equals(Object) is always more correct.") -public class ProtoStringFieldReferenceEquality extends BugChecker implements BinaryTreeMatcher { +@BugPattern( + category = ONE_OFF, + maturity = EXPERIMENTAL, + name = "ProtoStringFieldReferenceEquality", + severity = ERROR, + summary = "Comparing protobuf fields of type String using reference equality", + explanation = + "Comparing strings with == is almost always an error, but it is an error 100% " + + "of the time when one of the strings is a protobuf field. Additionally, protobuf " + + "fields cannot be null, so Object.equals(Object) is always more correct." +) +public class ProtoStringFieldReferenceEquality extends AbstractReferenceEquality { private static final String PROTO_SUPER_CLASS = "com.google.protobuf.GeneratedMessage"; - private static final Matcher PROTO_STRING_METHOD = allOf( - instanceMethod().onDescendantOf(PROTO_SUPER_CLASS), - isSameType("java.lang.String")); + private static final Matcher PROTO_STRING_METHOD = + allOf(instanceMethod().onDescendantOf(PROTO_SUPER_CLASS), isSameType("java.lang.String")); @Override - public Description matchBinary(BinaryTree tree, VisitorState state) { - if (tree.getKind() != Kind.EQUAL_TO && tree.getKind() != Kind.NOT_EQUAL_TO) { - return Description.NO_MATCH; - } - String leftOperand = state.getSourceForNode((JCTree) tree.getLeftOperand()).toString(); - String rightOperand = state.getSourceForNode((JCTree) tree.getRightOperand()).toString(); - if ((PROTO_STRING_METHOD.matches(tree.getLeftOperand(), state) - && tree.getRightOperand().getKind() != Kind.NULL_LITERAL) - || (PROTO_STRING_METHOD.matches(tree.getRightOperand(), state) - && tree.getLeftOperand().getKind() != Kind.NULL_LITERAL)) { - String result = leftOperand + ".equals(" + rightOperand + ")"; - if (tree.getKind() == Kind.NOT_EQUAL_TO) { - result = "!" + result; - } - return describeMatch(tree, SuggestedFix.replace(tree, result)); - } else { - return Description.NO_MATCH; - } + protected boolean matchArgument(ExpressionTree tree, VisitorState state) { + return PROTO_STRING_METHOD.matches(tree, state); } - } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java index 5945a86f654..53081eedaa5 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java @@ -19,149 +19,30 @@ import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.MaturityLevel.MATURE; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; -import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NONNULL; -import static com.google.errorprone.matchers.Matchers.allOf; -import static com.google.errorprone.matchers.Matchers.anyOf; -import static com.google.errorprone.matchers.Matchers.kindIs; -import static com.google.errorprone.util.ASTHelpers.constValue; -import static com.sun.source.tree.Tree.Kind.EQUAL_TO; -import static com.sun.source.tree.Tree.Kind.NOT_EQUAL_TO; -import com.google.common.base.Joiner; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.matchers.Description; -import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.BinaryTree; import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.Tree; -import com.sun.source.util.TreePath; -import com.sun.tools.javac.code.Type; -import com.sun.tools.javac.tree.JCTree; /** * @author ptoomey@google.com (Patrick Toomey) */ -@BugPattern(name = "StringEquality", - summary = "String comparison using reference equality instead of value equality", - explanation = "Strings are compared for reference equality/inequality using == or !=" - + "instead of for value equality using .equals()", - category = JDK, severity = WARNING, maturity = MATURE) -public class StringEquality extends BugChecker implements BinaryTreeMatcher { +@BugPattern( + name = "StringEquality", + summary = "String comparison using reference equality instead of value equality", + explanation = + "Strings are compared for reference equality/inequality using == or !=" + + "instead of for value equality using .equals()", + category = JDK, + severity = WARNING, + maturity = MATURE +) +public class StringEquality extends AbstractReferenceEquality { - /** - * A {@link Matcher} that matches whether the operands in a {@link BinaryTree} are - * strictly String operands. For Example, if either operand is {@code null} the matcher - * will return {@code false} - */ - private static final Matcher STRING_OPERANDS = new Matcher() { - @Override - public boolean matches(BinaryTree tree, VisitorState state) { - Type stringType = state.getSymtab().stringType; - ExpressionTree leftOperand = tree.getLeftOperand(); - Type leftType = ((JCTree.JCExpression) leftOperand).type; - // The left operand is not a String (ex. null) so no match - if (!state.getTypes().isSameType(leftType, stringType)) { - return false; - } - ExpressionTree rightOperand = tree.getRightOperand(); - Type rightType = ((JCTree.JCExpression) rightOperand).type; - // We know that both operands are String objects - return state.getTypes().isSameType(rightType, stringType); - } - }; - - public static final Matcher MATCHER = allOf( - anyOf(kindIs(EQUAL_TO), kindIs(NOT_EQUAL_TO)), - STRING_OPERANDS); - - /* Match string that are compared with == and != */ @Override - public Description matchBinary(BinaryTree tree, final VisitorState state) { - if (!MATCHER.matches(tree, state)) { - return Description.NO_MATCH; - } - - SuggestedFix.Builder fix = SuggestedFix.builder(); - - // Consider one of the tree's operands. If it is "", and the other is non-null, - // then call isEmpty on the other. - StringBuilder fixExpr = considerOneOf(tree.getLeftOperand(), tree.getRightOperand(), - new HandleChoice() { - @Override - public StringBuilder apply(ExpressionTree it, ExpressionTree other) { - return "".equals(constValue((JCTree) it)) && isNonNull(other, state) - ? methodCall(other, "isEmpty") : null; - } - }); - - if (fixExpr == null) { - // Consider one of the tree's operands. If it is non-null, - // then call equals on it, passing the other operand as argument. - fixExpr = considerOneOf(tree.getLeftOperand(), tree.getRightOperand(), - new HandleChoice() { - @Override - public StringBuilder apply(ExpressionTree it, ExpressionTree other) { - return isNonNull(it, state) ? methodCall(it, "equals", other) : null; - } - }); - - if (fixExpr == null) { - fixExpr = methodCall( - null, "Objects.equals", tree.getLeftOperand(), tree.getRightOperand()); - fix.addImport("java.util.Objects"); - } - } - - if (tree.getKind() == Tree.Kind.NOT_EQUAL_TO) { - fixExpr.insert(0, "!"); - } - - fix.replace(tree, fixExpr.toString()); - return describeMatch(tree, fix.build()); - } - - private interface HandleChoice { - R apply(T it, T other); - } - - private static R considerOneOf(final T a, final T b, final HandleChoice f) { - R r = f.apply(a, b); - return r == null ? f.apply(b, a) : r; - } - - private boolean isNonNull(ExpressionTree expr, VisitorState state) { - TreePath pathToExpr = new TreePath(state.getPath(), expr); - return state.getNullnessAnalysis().getNullness(pathToExpr, state.context) == NONNULL; - } - - /** - * Create a method call {@code methodName} with parameters {@code params}. - * If {@code receiver} is null, the call is static or to {@code this}, - * otherwise the call is to {@code receiver}. - */ - private static StringBuilder methodCall(ExpressionTree receiver, String methodName, - ExpressionTree... params) { - final StringBuilder fixedExpression = new StringBuilder(); - if (receiver != null) { - boolean isBinop = receiver instanceof BinaryTree; - if (isBinop) { - fixedExpression.append("("); - } - fixedExpression.append(receiver); - if (isBinop) { - fixedExpression.append(")"); - } - fixedExpression.append("."); - } - fixedExpression.append(methodName); - fixedExpression.append("("); - fixedExpression.append(Joiner.on(", ").join(params)); - fixedExpression.append(")"); - - return fixedExpression; + protected boolean matchArgument(ExpressionTree tree, VisitorState state) { + return ASTHelpers.isSameType(ASTHelpers.getType(tree), state.getSymtab().stringType, state); } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityPositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityPositiveCases.java index 7bd9f9fffb2..288a4c618d3 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityPositiveCases.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityPositiveCases.java @@ -24,7 +24,7 @@ public class StringEqualityPositiveCases { public boolean testEquality(String x, String y) { boolean retVal; - // BUG: Diagnostic contains: Objects.equals(x, y) + // BUG: Diagnostic contains: Objects.equals(x, y) retVal = (x == y); // BUG: Diagnostic contains: !Objects.equals(x, y) retVal = (x != y); @@ -32,16 +32,16 @@ public boolean testEquality(String x, String y) { retVal = (x + y == y + x); // BUG: Diagnostic contains: !(x + y).equals(y + x) retVal = (x + y != y + x); - // BUG: Diagnostic contains: (x + "str").equals(y + "str") + // BUG: Diagnostic contains: (x + "str").equals(y + "str") retVal = (x + "str" == y + "str"); - // BUG: Diagnostic contains: !(x + "str").equals(y + "str") + // BUG: Diagnostic contains: !(x + "str").equals(y + "str") retVal = (x + "str" != y + "str"); // BUG: Diagnostic contains: "str".equals(x) retVal = ("str" == x); // BUG: Diagnostic contains: "str".equals(x) - retVal = (x == "str") ; + retVal = (x == "str"); // BUG: Diagnostic contains: "str2".equals("str") - retVal = ("str2" == "str"); + retVal = ("str2" == "str"); final String constValue = "str"; // BUG: Diagnostic contains: constValue.equals(x) retVal = (x == constValue); @@ -54,32 +54,9 @@ public boolean testEquality(String x, String y) { retVal = (constValue + constValue2 == x); // BUG: Diagnostic contains: (constValue + constValue2).equals(x) retVal = (x == constValue + constValue2); - // BUG: Diagnostic contains: constValue.isEmpty() - retVal = (constValue == ""); - // BUG: Diagnostic contains: constValue.isEmpty() - retVal = ("" == constValue); - // BUG: Diagnostic contains: !constValue.isEmpty() - retVal = (constValue != ""); - // BUG: Diagnostic contains: !constValue.isEmpty() - retVal = ("" != constValue); // BUG: Diagnostic contains: "".equals(x) retVal = ("" == x); - String constEmpty = ""; - // BUG: Diagnostic contains: constEmpty.equals(y) - retVal = (y == constEmpty); - - x.toString(); - // BUG: Diagnostic contains: x.equals(y) - retVal = (x == y); - // BUG: Diagnostic contains: x.equals(y) - retVal = (y == x); - String constValue3 = "non-null"; - // BUG: Diagnostic contains: constValue3.equals(y) - retVal = (constValue3 == y); - // BUG: Diagnostic contains: constValue3.equals(y) - retVal = (y == constValue3); return retVal; } - }