From f289d9ef8f523ba76b433c5273a539b4e526134f Mon Sep 17 00:00:00 2001 From: ghm Date: Wed, 24 Apr 2024 03:53:18 -0700 Subject: [PATCH] `VoidUsed`: flag `Void` variables being _used_, where they can simply be replaced with a literal `null`. Findings here (unknown commit) before I fixed the bug with assignments. PiperOrigin-RevId: 627676095 --- .../errorprone/bugpatterns/VoidUsed.java | 76 +++++++++++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + .../errorprone/bugpatterns/VoidUsedTest.java | 68 +++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/VoidUsed.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/VoidUsedTest.java diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/VoidUsed.java b/core/src/main/java/com/google/errorprone/bugpatterns/VoidUsed.java new file mode 100644 index 00000000000..c3c277bed88 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/VoidUsed.java @@ -0,0 +1,76 @@ +/* + * 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.common.collect.Sets.immutableEnumSet; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.suppliers.Suppliers.JAVA_LANG_VOID_TYPE; +import static com.google.errorprone.util.ASTHelpers.getSymbol; +import static com.google.errorprone.util.ASTHelpers.isSameType; +import static javax.lang.model.element.ElementKind.FIELD; +import static javax.lang.model.element.ElementKind.LOCAL_VARIABLE; +import static javax.lang.model.element.ElementKind.PARAMETER; + +import com.google.common.collect.ImmutableSet; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.IdentifierTreeMatcher; +import com.google.errorprone.bugpatterns.BugChecker.MemberSelectTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.Tree; +import javax.lang.model.element.ElementKind; + +/** A BugPattern; see the summary. */ +@BugPattern( + summary = + "Using a Void-typed variable is potentially confusing, and can be replaced with a literal" + + " `null`.", + severity = WARNING) +public final class VoidUsed extends BugChecker + implements IdentifierTreeMatcher, MemberSelectTreeMatcher { + @Override + public Description matchIdentifier(IdentifierTree tree, VisitorState state) { + return handle(tree, state); + } + + @Override + public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) { + return handle(tree, state); + } + + private Description handle(Tree tree, VisitorState state) { + var parent = state.getPath().getParentPath().getLeaf(); + if (parent instanceof AssignmentTree && ((AssignmentTree) parent).getVariable().equals(tree)) { + return NO_MATCH; + } + var symbol = getSymbol(tree); + if (symbol == null + || !KINDS.contains(symbol.getKind()) + || !isSameType(symbol.type, JAVA_LANG_VOID_TYPE.get(state), state)) { + return NO_MATCH; + } + return describeMatch(tree, SuggestedFix.replace(tree, "null")); + } + + private static final ImmutableSet KINDS = + immutableEnumSet(PARAMETER, LOCAL_VARIABLE, FIELD); +} 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 f90f05f4a83..940ff375ac9 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -434,6 +434,7 @@ import com.google.errorprone.bugpatterns.VarTypeName; import com.google.errorprone.bugpatterns.VariableNameSameAsType; import com.google.errorprone.bugpatterns.Varifier; +import com.google.errorprone.bugpatterns.VoidUsed; import com.google.errorprone.bugpatterns.WaitNotInLoop; import com.google.errorprone.bugpatterns.WildcardImport; import com.google.errorprone.bugpatterns.WithSignatureDiscouraged; @@ -1094,6 +1095,7 @@ public static ScannerSupplier warningChecks() { UnusedVariable.class, UseBinds.class, VariableNameSameAsType.class, + VoidUsed.class, WaitNotInLoop.class, WakelockReleasedDangerously.class, WithSignatureDiscouraged.class diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/VoidUsedTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/VoidUsedTest.java new file mode 100644 index 00000000000..9a7d310dee4 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/VoidUsedTest.java @@ -0,0 +1,68 @@ +/* + * 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 com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class VoidUsedTest { + private final CompilationTestHelper helper = + CompilationTestHelper.newInstance(VoidUsed.class, getClass()); + + @Test + public void positive() { + helper + .addSourceLines( + "Test.java", // + "class Test {", + " void test(Void v) {", + " // BUG: Diagnostic contains: null", + " System.out.println(v);", + " }", + "}") + .doTest(); + } + + @Test + public void notVoid_noFinding() { + helper + .addSourceLines( + "Test.java", // + "class Test {", + " void test(Integer v) {", + " System.out.println(v);", + " }", + "}") + .doTest(); + } + + @Test + public void assignedTo_noFinding() { + helper + .addSourceLines( + "Test.java", // + "class Test {", + " void test(Void v) {", + " v = null;", + " }", + "}") + .doTest(); + } +}