diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/CacheLoaderNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/CacheLoaderNull.java deleted file mode 100644 index c4b6d50d116..00000000000 --- a/core/src/main/java/com/google/errorprone/bugpatterns/CacheLoaderNull.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2019 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 com.google.errorprone.BugPattern; -import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; -import com.google.errorprone.matchers.Description; -import com.google.errorprone.suppliers.Supplier; -import com.google.errorprone.suppliers.Suppliers; -import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.ClassTree; -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.LambdaExpressionTree; -import com.sun.source.tree.MethodTree; -import com.sun.source.tree.ReturnTree; -import com.sun.source.tree.Tree; -import com.sun.source.util.TreeScanner; -import com.sun.tools.javac.code.Symbol.MethodSymbol; -import com.sun.tools.javac.code.Type; - -/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ -@BugPattern(summary = "The result of CacheLoader#load must be non-null.", severity = WARNING) -public class CacheLoaderNull extends BugChecker implements MethodTreeMatcher { - - private static final Supplier CACHE_LOADER_TYPE = - Suppliers.typeFromString("com.google.common.cache.CacheLoader"); - - @Override - public Description matchMethod(MethodTree tree, VisitorState state) { - if (!tree.getName().contentEquals("load")) { - return NO_MATCH; - } - MethodSymbol sym = ASTHelpers.getSymbol(tree); - if (!ASTHelpers.isSubtype(sym.owner.asType(), CACHE_LOADER_TYPE.get(state), state)) { - return NO_MATCH; - } - new TreeScanner() { - @Override - public Void visitLambdaExpression(LambdaExpressionTree tree, Void unused) { - return null; - } - - @Override - public Void visitClass(ClassTree tree, Void unused) { - return null; - } - - @Override - public Void visitReturn(ReturnTree tree, Void unused) { - ExpressionTree expression = tree.getExpression(); - if (expression != null && expression.getKind() == Tree.Kind.NULL_LITERAL) { - state.reportMatch(describeMatch(tree)); - } - return super.visitReturn(tree, null); - } - }.scan(tree.getBody(), null); - return NO_MATCH; - } -} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java b/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java index b42113e8a30..072add5671a 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java @@ -241,7 +241,7 @@ private void handleSwitch(ExpressionTree expression, List ca .filter(unused -> getFixer(withoutParens, subState).isPresent()) .ifPresent( e -> - // NOTE: This fix is possibly too big: you can write `case null, default ->`. + // NOTE: user ->`. state.reportMatch(describeMatch(caseTree, SuggestedFix.delete(caseTree)))); } } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterNaming.java b/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterNaming.java index 5a147dc7940..6a155f8c83c 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterNaming.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterNaming.java @@ -225,7 +225,7 @@ private static String suggestedSingleLetter(String id, Tree tree) { // T -> T2 // T2 -> T3 // T -> T4 (if T2 and T3 already exist) - // TODO(user) : combine this method with TypeParameterShadowing.replacementTypeVarName + // TODO(siyuanl) : combine this method with TypeParameterShadowing.replacementTypeVarName private static String firstLetterReplacementName(String name, List superTypeVars) { String firstLetterOfBase = Character.toString(name.charAt(0)); int typeVarNum = 2; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/CacheLoaderNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/CacheLoaderNull.java new file mode 100644 index 00000000000..6525e498b0d --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/CacheLoaderNull.java @@ -0,0 +1,40 @@ +/* + * Copyright 2019 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.nullness; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.fixes.SuggestedFix.emptyFix; + +import com.google.common.cache.CacheLoader; +import com.google.errorprone.BugPattern; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; +import com.sun.source.tree.ExpressionTree; + +/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ +@BugPattern(summary = "The result of CacheLoader#load must be non-null.", severity = WARNING) +public final class CacheLoaderNull extends AbstractAsyncTypeReturnsNull { + public CacheLoaderNull() { + super(CacheLoader.class); + } + + @Override + protected SuggestedFix provideFix(ExpressionTree tree) { + // The default suggestion is immediateFuture(null), which doesn't make sense for CacheLoader. + return emptyFix(); + } +} 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 ed2e535d5cd..8521577aa32 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -65,7 +65,6 @@ import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugPatternNaming; import com.google.errorprone.bugpatterns.ByteBufferBackingArray; -import com.google.errorprone.bugpatterns.CacheLoaderNull; import com.google.errorprone.bugpatterns.CannotMockFinalClass; import com.google.errorprone.bugpatterns.CannotMockMethod; import com.google.errorprone.bugpatterns.CanonicalDuration; @@ -579,6 +578,7 @@ import com.google.errorprone.bugpatterns.nullness.AddNullMarkedToPackageInfo; import com.google.errorprone.bugpatterns.nullness.AsyncCallableReturnsNull; import com.google.errorprone.bugpatterns.nullness.AsyncFunctionReturnsNull; +import com.google.errorprone.bugpatterns.nullness.CacheLoaderNull; import com.google.errorprone.bugpatterns.nullness.DereferenceWithNullBranch; import com.google.errorprone.bugpatterns.nullness.EqualsBrokenForNull; import com.google.errorprone.bugpatterns.nullness.EqualsMissingNullable; diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/CacheLoaderNullTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/CacheLoaderNullTest.java similarity index 90% rename from core/src/test/java/com/google/errorprone/bugpatterns/CacheLoaderNullTest.java rename to core/src/test/java/com/google/errorprone/bugpatterns/nullness/CacheLoaderNullTest.java index f7d07c75885..6e97d958967 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/CacheLoaderNullTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/CacheLoaderNullTest.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.google.errorprone.bugpatterns; +package com.google.errorprone.bugpatterns.nullness; import com.google.errorprone.CompilationTestHelper; import org.junit.Test; @@ -45,6 +45,13 @@ public String load(String key) { return null; } }; + new CacheLoader() { + @Override + public String load(String key) { + // BUG: Diagnostic contains: + return key.equals("") ? null : key; + } + }; abstract class MyCacheLoader extends CacheLoader {} new MyCacheLoader() { @Override @@ -92,7 +99,7 @@ public String get() { return null; } } - ; + return ""; } };