diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java index 19c2b57c2d4..4285eb1ec9e 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReferenceEquality.java @@ -45,8 +45,7 @@ * Abstract implementation of a BugPattern that detects the use of reference equality to compare * classes with value semantics. * - *

See e.g. {@link OptionalEquality}, {@link ProtoStringFieldReferenceEquality}, and {@link - * StringEquality}. + *

See e.g. {@link OptionalEquality}, {@link ProtoStringFieldReferenceEquality}. * * @author cushon@google.com (Liam Miller-Cushon) */ diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java deleted file mode 100644 index 1aae031a74c..00000000000 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StringEquality.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2012 The Error Prone Authors. - * - * 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.BugPattern.SeverityLevel.WARNING; - -import com.google.errorprone.BugPattern; -import com.google.errorprone.VisitorState; -import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.ExpressionTree; - -/** - * @author ptoomey@google.com (Patrick Toomey) - */ -@BugPattern( - summary = "String comparison using reference equality instead of value equality", - severity = WARNING) -public class StringEquality extends AbstractReferenceEquality { - - @Override - protected boolean matchArgument(ExpressionTree tree, VisitorState state) { - return ASTHelpers.isSameType(ASTHelpers.getType(tree), state.getSymtab().stringType, state); - } -} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index 216dcc46b11..2880bdf916b 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -316,7 +316,6 @@ import com.google.errorprone.bugpatterns.StreamToIterable; import com.google.errorprone.bugpatterns.StreamToString; import com.google.errorprone.bugpatterns.StringBuilderInitWithChar; -import com.google.errorprone.bugpatterns.StringEquality; import com.google.errorprone.bugpatterns.StringSplitter; import com.google.errorprone.bugpatterns.StronglyTypeByteString; import com.google.errorprone.bugpatterns.SubstringOfZero; @@ -1090,7 +1089,6 @@ public static ScannerSupplier errorChecks() { ScopeOrQualifierAnnotationRetention.class, StaticOrDefaultInterfaceMethod.class, StaticQualifiedUsingExpression.class, - StringEquality.class, StronglyTypeByteString.class, StronglyTypeTime.class, SuppressWarningsWithoutExplanation.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StringEqualityTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StringEqualityTest.java deleted file mode 100644 index 097af026270..00000000000 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StringEqualityTest.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2012 The Error Prone Authors. - * - * 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 com.google.errorprone.CompilationTestHelper; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * @author ptoomey@google.com (Patrick Toomey) - */ -@RunWith(JUnit4.class) -public class StringEqualityTest { - - private final CompilationTestHelper compilationHelper = - CompilationTestHelper.newInstance(StringEquality.class, getClass()); - - @Test - public void testPositiveCase() { - compilationHelper.addSourceFile("StringEqualityPositiveCases.java").doTest(); - } - - @Test - public void testNegativeCase() { - compilationHelper.addSourceFile("StringEqualityNegativeCases.java").doTest(); - } -} diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityNegativeCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityNegativeCases.java deleted file mode 100644 index e88237d894c..00000000000 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityNegativeCases.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2012 The Error Prone Authors. - * - * 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.testdata; - -/** @author ptoomey@google.com (Patrick Toomey) */ -public class StringEqualityNegativeCases { - - public boolean testEquality(String x, String y) { - boolean retVal; - - retVal = x.equals(y); - retVal = (x == null); - retVal = (x != null); - retVal = (null == x); - retVal = (null != x); - - return retVal; - } - - @SuppressWarnings("StringEquality") - public boolean testSuppressWarnings(String x, String y) { - boolean retVal; - - retVal = (x != y); - retVal = (x == y); - - return retVal; - } -} 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 deleted file mode 100644 index f1913d24085..00000000000 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/StringEqualityPositiveCases.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2012 The Error Prone Authors. - * - * 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.testdata; - -/** @author ptoomey@google.com (Patrick Toomey) */ -public class StringEqualityPositiveCases { - - public boolean testEquality(String x, String y) { - boolean retVal; - - // BUG: Diagnostic contains: Objects.equals(x, y) - retVal = (x == y); - // BUG: Diagnostic contains: !Objects.equals(x, y) - retVal = (x != y); - // BUG: Diagnostic contains: (x + y).equals(y + x) - 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") - retVal = (x + "str" == 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"); - // BUG: Diagnostic contains: "str2".equals("str") - retVal = ("str2" == "str"); - final String constValue = "str"; - // BUG: Diagnostic contains: constValue.equals(x) - retVal = (x == constValue); - // BUG: Diagnostic contains: !constValue.equals(x) - retVal = (x != constValue); - // BUG: Diagnostic contains: (x + y + constValue).equals(x + y) - retVal = (x + y + constValue == x + y); - final String constValue2 = "str2"; - // BUG: Diagnostic contains: (constValue + constValue2).equals(x) - retVal = (constValue + constValue2 == x); - // BUG: Diagnostic contains: (constValue + constValue2).equals(x) - retVal = (x == constValue + constValue2); - // BUG: Diagnostic contains: "".equals(x) - retVal = ("" == x); - - return retVal; - } -} diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java index 4a8d7cf9502..0fa66b2a940 100644 --- a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java +++ b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java @@ -48,8 +48,8 @@ import com.google.errorprone.bugpatterns.EqualsIncompatibleType; import com.google.errorprone.bugpatterns.LongLiteralLowerCaseSuffix; import com.google.errorprone.bugpatterns.PackageLocation; +import com.google.errorprone.bugpatterns.ReferenceEquality; import com.google.errorprone.bugpatterns.StaticQualifiedUsingExpression; -import com.google.errorprone.bugpatterns.StringEquality; import com.google.errorprone.bugpatterns.nullness.UnnecessaryCheckNotNull; import com.sun.source.util.JavacTask; import com.sun.tools.javac.api.JavacTool; @@ -331,10 +331,10 @@ public void applyOverridesEnableAllChecks() { @Test public void applyOverridesDisableErrors() { - // BadShiftAmount (error), ArrayEquals (unsuppressible error), StringEquality (warning) + // BadShiftAmount (error), ArrayEquals (unsuppressible error), ReferenceEquality (warning) ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses( - BadShiftAmount.class, UnsuppressibleArrayEquals.class, StringEquality.class); + BadShiftAmount.class, UnsuppressibleArrayEquals.class, ReferenceEquality.class); ErrorProneOptions epOptions = ErrorProneOptions.processArgs(ImmutableList.of("-XepAllErrorsAsWarnings")); @@ -344,31 +344,31 @@ public void applyOverridesDisableErrors() { ImmutableMap.of( "ArrayEquals", SeverityLevel.ERROR, // Unsuppressible, not demoted "BadShiftAmount", SeverityLevel.WARNING, // Demoted from error to warning - "StringEquality", SeverityLevel.WARNING)); // Already warning, unaffected + "ReferenceEquality", SeverityLevel.WARNING)); // Already warning, unaffected // Flags after AllErrorsAsWarnings flag should override it. epOptions = ErrorProneOptions.processArgs( - ImmutableList.of("-XepAllErrorsAsWarnings", "-Xep:StringEquality:ERROR")); + ImmutableList.of("-XepAllErrorsAsWarnings", "-Xep:ReferenceEquality:ERROR")); assertScanner(ss.applyOverrides(epOptions)) .hasSeverities( ImmutableMap.of( "ArrayEquals", SeverityLevel.ERROR, "BadShiftAmount", SeverityLevel.WARNING, - "StringEquality", SeverityLevel.ERROR)); + "ReferenceEquality", SeverityLevel.ERROR)); // AllErrorsAsWarnings flag should override all error-level severity flags that come before it. epOptions = ErrorProneOptions.processArgs( - ImmutableList.of("-Xep:StringEquality:ERROR", "-XepAllErrorsAsWarnings")); + ImmutableList.of("-Xep:ReferenceEquality:ERROR", "-XepAllErrorsAsWarnings")); assertScanner(ss.applyOverrides(epOptions)) .hasSeverities( ImmutableMap.of( "ArrayEquals", SeverityLevel.ERROR, "BadShiftAmount", SeverityLevel.WARNING, - "StringEquality", SeverityLevel.WARNING)); + "ReferenceEquality", SeverityLevel.WARNING)); // AllErrorsAsWarnings only overrides error-level severity flags. // That is, checks disabled before the flag are left disabled, not promoted to warnings. @@ -380,9 +380,9 @@ public void applyOverridesDisableErrors() { .hasSeverities( ImmutableMap.of( "ArrayEquals", SeverityLevel.ERROR, - "StringEquality", SeverityLevel.WARNING)); + "ReferenceEquality", SeverityLevel.WARNING)); assertScanner(ss.applyOverrides(epOptions)) - .hasEnabledChecks(UnsuppressibleArrayEquals.class, StringEquality.class); + .hasEnabledChecks(UnsuppressibleArrayEquals.class, ReferenceEquality.class); } @Test @@ -465,18 +465,20 @@ public void applyOverridesSucceedsWhenDisablingUnknownCheckAndIgnoreUnknownCheck public void applyOverridesSetsSeverity() { ScannerSupplier ss = ScannerSupplier.fromBugCheckerClasses( - BadShiftAmount.class, ChainingConstructorIgnoresParameter.class, StringEquality.class); + BadShiftAmount.class, + ChainingConstructorIgnoresParameter.class, + ReferenceEquality.class); ErrorProneOptions epOptions = ErrorProneOptions.processArgs( ImmutableList.of( - "-Xep:ChainingConstructorIgnoresParameter:WARN", "-Xep:StringEquality:ERROR")); + "-Xep:ChainingConstructorIgnoresParameter:WARN", "-Xep:ReferenceEquality:ERROR")); ScannerSupplier overriddenScannerSupplier = ss.applyOverrides(epOptions); ImmutableMap expected = ImmutableMap.of( "BadShiftAmount", SeverityLevel.ERROR, "ChainingConstructorIgnoresParameter", SeverityLevel.WARNING, - "StringEquality", SeverityLevel.ERROR); + "ReferenceEquality", SeverityLevel.ERROR); assertScanner(overriddenScannerSupplier).hasSeverities(expected); } @@ -507,18 +509,20 @@ public void allChecksAsWarningsWorks() { ScannerSupplier.fromBugCheckerClasses( BadShiftAmount.class, ChainingConstructorIgnoresParameter.class, - StringEquality.class) + ReferenceEquality.class) .filter(Predicates.alwaysFalse()); assertScanner(ss).hasEnabledChecks(); // Everything's off ErrorProneOptions epOptions = ErrorProneOptions.processArgs( - ImmutableList.of("-Xep:StringEquality:OFF", "-XepAllDisabledChecksAsWarnings")); + ImmutableList.of("-Xep:ReferenceEquality:OFF", "-XepAllDisabledChecksAsWarnings")); ScannerSupplier withOverrides = ss.applyOverrides(epOptions); assertScanner(withOverrides) .hasEnabledChecks( - BadShiftAmount.class, ChainingConstructorIgnoresParameter.class, StringEquality.class); + BadShiftAmount.class, + ChainingConstructorIgnoresParameter.class, + ReferenceEquality.class); ImmutableMap expectedSeverities = ImmutableMap.of( @@ -526,16 +530,16 @@ public void allChecksAsWarningsWorks() { SeverityLevel.WARNING, "ChainingConstructorIgnoresParameter", SeverityLevel.WARNING, - "StringEquality", + "ReferenceEquality", SeverityLevel.WARNING); assertScanner(withOverrides).hasSeverities(expectedSeverities); epOptions = ErrorProneOptions.processArgs( ImmutableList.of( - "-Xep:StringEquality:OFF", + "-Xep:ReferenceEquality:OFF", "-XepAllDisabledChecksAsWarnings", - "-Xep:StringEquality:OFF")); + "-Xep:ReferenceEquality:OFF")); withOverrides = ss.applyOverrides(epOptions); assertScanner(withOverrides) diff --git a/docs/bugpattern/StringEquality.md b/docs/bugpattern/StringEquality.md deleted file mode 100644 index d177a90a206..00000000000 --- a/docs/bugpattern/StringEquality.md +++ /dev/null @@ -1,2 +0,0 @@ -Strings are compared for reference equality/inequality using == or !=instead of -for value equality using .equals()