diff --git a/annotation/pom.xml b/annotation/pom.xml index fbbba7c5524..4ebcf1808f8 100644 --- a/annotation/pom.xml +++ b/annotation/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 @BugPattern annotation diff --git a/annotations/pom.xml b/annotations/pom.xml index 62ebfe88a40..c37c44b4b64 100644 --- a/annotations/pom.xml +++ b/annotations/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 error-prone annotations diff --git a/check_api/pom.xml b/check_api/pom.xml index 1a96ffd6473..64441b63bba 100644 --- a/check_api/pom.xml +++ b/check_api/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 error-prone check api diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java index 5d552288976..1707c6a4b50 100644 --- a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java +++ b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java @@ -2808,6 +2808,31 @@ private static Method getCaseTreeGetExpressionsMethod() { } } + /** + * Returns true if the given {@link CaseTree} is in the form: {@code case -> + * }. + */ + public static boolean isRuleKind(CaseTree caseTree) { + Enum kind; + try { + kind = (Enum) GET_CASE_KIND_METHOD.invoke(caseTree); + } catch (ReflectiveOperationException e) { + return false; + } + return kind.name().equals("RULE"); + } + + private static final Method GET_CASE_KIND_METHOD = getGetCaseKindMethod(); + + @Nullable + private static Method getGetCaseKindMethod() { + try { + return CaseTree.class.getMethod("getCaseKind"); + } catch (NoSuchMethodException e) { + return null; + } + } + /** * Retrieves a stream containing all case expressions, in order, for a given {@code CaseTree}. * This method acts as a facade to the {@code CaseTree.getExpressions()} API, falling back to diff --git a/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java b/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java index 14ae3dd37fc..0785489c645 100644 --- a/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java +++ b/check_api/src/main/java/com/google/errorprone/util/RuntimeVersion.java @@ -79,6 +79,11 @@ public static boolean isAtLeast21() { return FEATURE >= 21; } + /** Returns true if the current runtime is JDK 22 or newer. */ + public static boolean isAtLeast22() { + return FEATURE >= 22; + } + /** * Returns the latest {@code --release} version. * diff --git a/core/pom.xml b/core/pom.xml index db42176e114..a9628309c7b 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 error-prone library diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java b/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java index afc49c81a3b..168c5c6e458 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java @@ -50,6 +50,9 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState } ExpressionTree arg = tree.getArguments().get(0); var argSymbol = getSymbol(arg); + if (argSymbol == null) { + return NO_MATCH; + } if (!argSymbol.getSimpleName().contentEquals("UNRECOGNIZED")) { return NO_MATCH; } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java b/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java new file mode 100644 index 00000000000..090b40e0175 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpression.java @@ -0,0 +1,44 @@ +/* + * Copyright 2024 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 static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.util.ASTHelpers.isRuleKind; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.CaseTree; +import com.sun.source.tree.Tree; + +/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ +@BugPattern(summary = "Prefer -> switches for switch expressions", severity = WARNING) +public class TraditionalSwitchExpression extends BugChecker implements BugChecker.CaseTreeMatcher { + + @Override + public Description matchCase(CaseTree tree, VisitorState state) { + if (isRuleKind(tree)) { + return NO_MATCH; + } + Tree parent = state.getPath().getParentPath().getLeaf(); + if (!parent.getKind().name().equals("SWITCH_EXPRESSION")) { + return NO_MATCH; + } + return describeMatch(parent); + } +} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java index 71e12a7f237..0e8f41a500e 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java @@ -198,6 +198,7 @@ public final class UnusedVariable extends BugChecker implements CompilationUnitT this.exemptNames = ImmutableSet.builder() .add("ignored") + .add("") // `var _ = ...` is handled as an empty variable name .addAll(flags.getListOrEmpty("Unused:exemptNames")) .build(); 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 237fdeac8ac..ccd20eae719 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -379,6 +379,7 @@ import com.google.errorprone.bugpatterns.ThrowsUncheckedException; import com.google.errorprone.bugpatterns.ToStringReturnsNull; import com.google.errorprone.bugpatterns.TooManyParameters; +import com.google.errorprone.bugpatterns.TraditionalSwitchExpression; import com.google.errorprone.bugpatterns.TransientMisuse; import com.google.errorprone.bugpatterns.TreeToString; import com.google.errorprone.bugpatterns.TruthAssertExpected; @@ -1075,6 +1076,7 @@ public static ScannerSupplier warningChecks() { ThreeLetterTimeZoneID.class, TimeUnitConversionChecker.class, ToStringReturnsNull.class, + TraditionalSwitchExpression.class, TruthAssertExpected.class, TruthConstantAsserts.class, TruthGetOrDefault.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java index 860a4fdb82d..7c55a108c84 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java @@ -56,4 +56,19 @@ public void negative() { "}") .doTest(); } + + @Test + public void negativeNotEnum() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.errorprone.bugpatterns.proto.Proto3Test.TestProto3Enum;", + "import com.google.errorprone.bugpatterns.proto.Proto3Test.TestProto3Message;", + "class Test {", + " void test() {", + " TestProto3Message.newBuilder().setMyString(\"\");", + " }", + "}") + .doTest(); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpressionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpressionTest.java new file mode 100644 index 00000000000..931626e04b4 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/TraditionalSwitchExpressionTest.java @@ -0,0 +1,99 @@ +/* + * Copyright 2024 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 org.junit.Assume.assumeTrue; + +import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.util.RuntimeVersion; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class TraditionalSwitchExpressionTest { + + private final CompilationTestHelper testHelper = + CompilationTestHelper.newInstance(TraditionalSwitchExpression.class, getClass()); + + @Test + public void positive() { + assumeTrue(RuntimeVersion.isAtLeast14()); + testHelper + .addSourceLines( + "Test.java", + "class Test {", + " int f(int i) {", + " // BUG: Diagnostic contains: Prefer -> switches for switch expressions", + " return switch (i) {", + " default:", + " yield -1;", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void negativeStatement() { + assumeTrue(RuntimeVersion.isAtLeast14()); + testHelper + .addSourceLines( + "Test.java", + "class Test {", + " void f(int i) {", + " switch (i) {", + " default:", + " return;", + " }", + " }", + "}") + .doTest(); + } + + @Test + public void negativeArrowStatement() { + assumeTrue(RuntimeVersion.isAtLeast14()); + testHelper + .addSourceLines( + "Test.java", + "class Test {", + " void f(int i) {", + " switch (i) {", + " default -> System.err.println();", + " }", + " }", + "}") + .doTest(); + } + + @Test + public void negativeArrow() { + assumeTrue(RuntimeVersion.isAtLeast14()); + testHelper + .addSourceLines( + "Test.java", + "class Test {", + " int f(int i) {", + " return switch (i) {", + " default -> -1;", + " };", + " }", + "}") + .doTest(); + } +} diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java index 92107250de7..4db2f04b54f 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java @@ -1679,4 +1679,19 @@ public void parameterUsedInOverride() { .expectUnchanged() .doTest(); } + + @Test + public void underscoreVariable() { + assumeTrue(RuntimeVersion.isAtLeast22()); + refactoringHelper + .addInputLines( + "Test.java", + "class Test {", + " public static void main(String[] args) {", + " var _ = new Object();", + " }", + "}") + .expectUnchanged() + .doTest(); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java index 0c04d598bc2..cd682d46480 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java @@ -41,14 +41,7 @@ public class ThreadSafeCheckerTest { private final CompilationTestHelper compilationHelper = - CompilationTestHelper.newInstance(ThreadSafeChecker.class, getClass()) - // TODO: b/339025111 - Remove this once ThreadSafeTypeParameter actually exists. - .addSourceLines( - "ThreadSafeTypeParameter.java", - "package com.google.errorprone.annotations;", - "@java.lang.annotation.Target(java.lang.annotation.ElementType.TYPE_PARAMETER)", - "@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)", - "public @interface ThreadSafeTypeParameter {}"); + CompilationTestHelper.newInstance(ThreadSafeChecker.class, getClass()); private final BugCheckerRefactoringTestHelper refactoringHelper = BugCheckerRefactoringTestHelper.newInstance(ThreadSafeChecker.class, getClass()); diff --git a/docgen/pom.xml b/docgen/pom.xml index 8ee00587502..0158d865b2a 100644 --- a/docgen/pom.xml +++ b/docgen/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 Documentation tool for generating Error Prone bugpattern documentation diff --git a/docgen_processor/pom.xml b/docgen_processor/pom.xml index 7d97a4cb4ca..52bfad1f6ae 100644 --- a/docgen_processor/pom.xml +++ b/docgen_processor/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 JSR-269 annotation processor for @BugPattern annotation diff --git a/docs/bugpattern/URLEqualsHashCode.md b/docs/bugpattern/URLEqualsHashCode.md index 1427964c370..aac6cfb9de0 100644 --- a/docs/bugpattern/URLEqualsHashCode.md +++ b/docs/bugpattern/URLEqualsHashCode.md @@ -1,2 +1,7 @@ -Equals and HashCode method of java.net.URL make blocking network calls. Either -use java.net.URI or if that isn't possible, use Collection or List. +The `equals` and `hashCode` methods of `java.net.URL` make blocking network +calls. When you place a `URL` into a hash-based container, the container invokes +those methods. + +Prefer `java.net.URI`. Or, if you must use `URL` in a +collection, prefer to use a non-hash-based container like a `List`, and +avoid calling methods like `contains` (which calls `equals`) on it. diff --git a/pom.xml b/pom.xml index f7cad3c75bd..63fb9ae1494 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ Error Prone parent POM com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 pom Error Prone is a static analysis tool for Java that catches common programming mistakes at compile-time. @@ -45,7 +45,7 @@ 1.6.13 3.19.6 1.43.3 - 0.3.0 + 1.0.0 5.1.0 diff --git a/refaster/pom.xml b/refaster/pom.xml index 968b0c2bae4..9d1910f0cdd 100644 --- a/refaster/pom.xml +++ b/refaster/pom.xml @@ -19,7 +19,7 @@ error_prone_parent com.google.errorprone - 1.0-HEAD-SNAPSHOT + 2.29.1 4.0.0 diff --git a/test_helpers/pom.xml b/test_helpers/pom.xml index 76fbfaf30d9..fd802cc3708 100644 --- a/test_helpers/pom.xml +++ b/test_helpers/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 error-prone test helpers diff --git a/type_annotations/pom.xml b/type_annotations/pom.xml index 365123af188..ebcd322a8d9 100644 --- a/type_annotations/pom.xml +++ b/type_annotations/pom.xml @@ -21,7 +21,7 @@ com.google.errorprone error_prone_parent - 1.0-HEAD-SNAPSHOT + 2.29.1 error-prone type annotations diff --git a/type_annotations/src/main/java/com/google/errorprone/annotations/ThreadSafeTypeParameter.java b/type_annotations/src/main/java/com/google/errorprone/annotations/ThreadSafeTypeParameter.java new file mode 100644 index 00000000000..2f3dbf4c4b4 --- /dev/null +++ b/type_annotations/src/main/java/com/google/errorprone/annotations/ThreadSafeTypeParameter.java @@ -0,0 +1,67 @@ +/* + * Copyright 2018 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.annotations; + +import static java.lang.annotation.ElementType.TYPE_PARAMETER; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +/** + * When a {@link ThreadSafe} class has type parameters, annotating a parameter with {@code + * ThreadSafeTypeParameter} enforces that declarations of this class must, for that type parameter, + * use a type that is itself thread-safe. + * + *

Additionally, only type parameters that are annotated with {@code ThreadSafeTypeParameter} can + * be used as field types that are not {@link + * com.google.errorprone.annotations.concurrent.GuardedBy @GuardedBy}. + * + *

In more detail, consider this (valid) class: + * + * 

{@code
+ * @ThreadSafe class MyThreadSafeClass {
+ *
+ *   @GuardedBy("this") B b;
+ *
+ *   final C c;
+ *
+ *   MyThreadSafeClass(B b, C c) {
+ *     this.b = b;
+ *     this.c = c;
+ *   }
+ * }
+ * }
+ * + * Each of these three type parameters is valid for a different reason: type parameter {@code A} is + * ok because it is simply not used as the type of a field; type parameter {@code B} is ok because + * it is used as the type of a field that is declared to be {@code @GuardedBy}; finally, type + * parameter {@code C} is ok because it is annotated with {@code ThreadSafeTypeParameter}. + * Furthermore, the declaration {@code MyThreadSafeClass} is valid, since + * the type parameter {@code C} (i.e., {@code String}) is thread-safe, whereas a declaration {@code + * MyThreadSafeClass} would result in a compiler error. + * + *

Note: the {@code ThreadSafeTypeParameter} annotation has a secondary use case. If you annotate + * a type parameter of a method, then callers to that method are only allowed to pass in a type that + * is deemed thread-safe. For example, given the method declaration {@code static + * <@ThreadSafeTypeParameter T> void foo(T foo) {}}, a call to {@code foo} must pass a parameter + * that is deemed thread-safe. + */ +@Documented +@Target(TYPE_PARAMETER) +@Retention(RUNTIME) +public @interface ThreadSafeTypeParameter {}