Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Swap `clazz.getName()` calls to string literals to prevent potential version skew issues w/ annotations.

PiperOrigin-RevId: 698845521
  • Loading branch information
kluever authored and Error Prone Team committed Dec 13, 2024
1 parent 99cae77 commit 70df1fd
Show file tree
Hide file tree
Showing 28 changed files with 202 additions and 129 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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.util;

/** Fully qualified names of common annotations used in ErrorProne checks. */
public final class AnnotationNames {

// keep-sorted start

public static final String AFTER_TEMPLATE_ANNOTATION =
"com.google.errorprone.refaster.annotation.AfterTemplate";
public static final String BEFORE_TEMPLATE_ANNOTATION =
"com.google.errorprone.refaster.annotation.BeforeTemplate";
public static final String BUG_PATTERN_ANNOTATION = "com.google.errorprone.BugPattern";
public static final String CAN_IGNORE_RETURN_VALUE_ANNOTATION =
"com.google.errorprone.annotations.CanIgnoreReturnValue";
public static final String COMPATIBLE_WITH_ANNOTATION =
"com.google.errorprone.annotations.CompatibleWith";
public static final String DO_NOT_CALL_ANNOTATION = "com.google.errorprone.annotations.DoNotCall";
public static final String FORMAT_METHOD_ANNOTATION =
"com.google.errorprone.annotations.FormatMethod";
public static final String FORMAT_STRING_ANNOTATION =
"com.google.errorprone.annotations.FormatString";
public static final String IMMUTABLE_ANNOTATION = "com.google.errorprone.annotations.Immutable";
public static final String LAZY_INIT_ANNOTATION =
"com.google.errorprone.annotations.concurrent.LazyInit";
public static final String MUST_BE_CLOSED_ANNOTATION =
"com.google.errorprone.annotations.MustBeClosed";
public static final String REPEATED_ANNOTATION =
"com.google.errorprone.refaster.annotation.Repeated";
public static final String RESTRICTED_API_ANNOTATION =
"com.google.errorprone.annotations.RestrictedApi";
public static final String THREAD_SAFE_ANNOTATION =
"com.google.errorprone.annotations.ThreadSafe";
public static final String VAR_ANNOTATION = "com.google.errorprone.annotations.Var";

// keep-sorted end

private AnnotationNames() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
import static java.lang.annotation.ElementType.METHOD;
Expand Down Expand Up @@ -312,7 +313,8 @@ public Void visitClass(ClassTree tree, VisitorState state) {
new Matcher<ClassTree>() {
@Override
public boolean matches(ClassTree t, VisitorState state) {
return ASTHelpers.hasAnnotation(t, InheritedAnnotation.class, state);
return hasAnnotation(
t, "com.google.errorprone.util.InheritedAnnotation", state);
}
});
setAssertionsComplete();
Expand All @@ -333,9 +335,7 @@ public void annotationHelpersWrongValueCached() {
@Override
public Void visitClass(ClassTree tree, VisitorState state) {
if (tree.getSimpleName().contentEquals("D")) {
assertThat(
ASTHelpers.hasAnnotation(
tree, "TestAnnotation", state.withPath(getCurrentPath())))
assertThat(hasAnnotation(tree, "TestAnnotation", state.withPath(getCurrentPath())))
.isFalse();
setAssertionsComplete();
}
Expand Down Expand Up @@ -371,7 +371,7 @@ public Void visitClass(ClassTree tree, VisitorState state) {
new Matcher<ClassTree>() {
@Override
public boolean matches(ClassTree t, VisitorState state) {
return ASTHelpers.hasAnnotation(t, "TestAnnotation", state);
return hasAnnotation(t, "TestAnnotation", state);
}
});
setAssertionsComplete();
Expand Down Expand Up @@ -413,7 +413,7 @@ public Void visitMethod(MethodTree tree, VisitorState state) {
tree,
state,
(MethodTree t, VisitorState s) ->
ASTHelpers.hasAnnotation(t, InheritedAnnotation.class, s));
hasAnnotation(t, "com.google.errorprone.util.InheritedAnnotation", s));
setAssertionsComplete();
}
return super.visitMethod(tree, state);
Expand Down Expand Up @@ -454,8 +454,7 @@ public Void visitClass(ClassTree tree, VisitorState state) {
new Matcher<ClassTree>() {
@Override
public boolean matches(ClassTree t, VisitorState state) {
return ASTHelpers.hasAnnotation(
ASTHelpers.getSymbol(t), "test.Lib$MyAnnotation", state);
return hasAnnotation(ASTHelpers.getSymbol(t), "test.Lib$MyAnnotation", state);
}
});
setAssertionsComplete();
Expand Down Expand Up @@ -911,7 +910,7 @@ public class A {
public Void visitMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (ASTHelpers.getSymbol(tree).toString().equals("doIt()")) {
setAssertionsComplete();
assertThat(ASTHelpers.hasAnnotation(tree, Deprecated.class, state)).isFalse();
assertThat(hasAnnotation(tree, Deprecated.class.getName(), state)).isFalse();
}
return super.visitMethodInvocation(tree, state);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.AnnotationNames.FORMAT_METHOD_ANNOTATION;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
Expand All @@ -43,7 +44,7 @@

/**
* Detects occurrences of pairs of parameters being passed straight through to {@link String#format}
* from a method not annotated with {@link FormatMethod}.
* from a method not annotated with {@link com.google.errorprone.annotations.FormatMethod}.
*
* @author [email protected] (Graeme Morgan)
*/
Expand Down Expand Up @@ -90,7 +91,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
MethodTree enclosingMethod = ASTHelpers.findEnclosingNode(state.getPath(), MethodTree.class);
if (enclosingMethod == null
|| !ASTHelpers.getSymbol(enclosingMethod).isVarArgs()
|| ASTHelpers.hasAnnotation(enclosingMethod, FormatMethod.class, state)) {
|| hasAnnotation(enclosingMethod, FORMAT_METHOD_ANNOTATION, state)) {
return Description.NO_MATCH;
}
List<? extends VariableTree> enclosingParameters = enclosingMethod.getParameters();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.util.ASTHelpers.getAnnotationWithSimpleName;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
Expand All @@ -38,7 +39,7 @@ public class DeprecatedVariable extends BugChecker implements VariableTreeMatche
@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
Symbol.VarSymbol sym = ASTHelpers.getSymbol(tree);
if (!ASTHelpers.hasAnnotation(sym, Deprecated.class, state)) {
if (!hasAnnotation(sym, Deprecated.class.getName(), state)) {
return NO_MATCH;
}
switch (sym.getKind()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.AnnotationNames.CAN_IGNORE_RETURN_VALUE_ANNOTATION;

import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.matchers.Matcher;
Expand Down Expand Up @@ -104,14 +104,14 @@ public boolean matches(ExpressionTree tree, VisitorState state) {
ASTHelpers.getUpperBound(resultType, state.getTypes()), futureType, state);
}
MethodSymbol sym = (MethodSymbol) untypedSymbol;
if (hasAnnotation(sym, CanIgnoreReturnValue.class, state)) {
if (hasAnnotation(sym, CAN_IGNORE_RETURN_VALUE_ANNOTATION, state)) {
return false;
}
for (MethodSymbol superSym : ASTHelpers.findSuperMethods(sym, state.getTypes())) {
// There are interfaces annotated with @CanIgnoreReturnValue (like Guava's Function)
// whose return value really shouldn't be ignored - as a heuristic, check if the super's
// method is returning a future subtype.
if (hasAnnotation(superSym, CanIgnoreReturnValue.class, state)
if (hasAnnotation(superSym, CAN_IGNORE_RETURN_VALUE_ANNOTATION, state)
&& ASTHelpers.isSubtype(
ASTHelpers.getUpperBound(superSym.getReturnType(), state.getTypes()),
futureType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.google.errorprone.bugpatterns;

import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Streams.stream;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
Expand Down Expand Up @@ -128,7 +127,7 @@ public Description matchClass(ClassTree classTree, VisitorState state) {
.filter(member -> !EXCLUSIONS.matches(member, state))
.filter(member -> !isSuppressed(member, state))
.map(VariableTree.class::cast)
.flatMap(varTree -> stream(isReplaceable(varTree, state)))
.flatMap(varTree -> isReplaceable(varTree, state).stream())
.collect(toImmutableMap(ReplaceableVar::symbol, var -> var));
if (replaceableVars.isEmpty()) {
return Description.NO_MATCH;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static com.google.errorprone.util.ASTHelpers.hasDirectAnnotationWithSimpleName;
import static com.google.errorprone.util.ASTHelpers.isGeneratedConstructor;
import static com.google.errorprone.util.ASTHelpers.isSubtype;
import static com.google.errorprone.util.AnnotationNames.BUG_PATTERN_ANNOTATION;

import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
Expand All @@ -51,7 +52,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
}
if (!isSubtype(
symbol.owner.type, state.getTypeFromString(BugChecker.class.getCanonicalName()), state)
|| !hasAnnotation(symbol.owner, BugPattern.class, state)) {
|| !hasAnnotation(symbol.owner, BUG_PATTERN_ANNOTATION, state)) {
return NO_MATCH;
}
if (tree.getParameters().isEmpty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
if (sym.isStatic()) {
return NO_MATCH;
}
if (hasAnnotation(sym, Override.class, state)) {
if (hasAnnotation(sym, Override.class.getName(), state)) {
return NO_MATCH;
}
if (ignoreInterfaceOverrides && sym.enclClass().isInterface()) {
Expand Down Expand Up @@ -91,7 +91,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
.filter(unused -> ASTHelpers.getGeneratedBy(state).isEmpty())
// to allow deprecated methods to be removed non-atomically, we permit overrides of
// @Deprecated to skip the annotation
.filter(override -> !hasAnnotation(override, Deprecated.class, state))
.filter(override -> !hasAnnotation(override, Deprecated.class.getName(), state))
.map(
override ->
buildDescription(tree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@
import static com.google.errorprone.matchers.Matchers.methodIsConstructor;
import static com.google.errorprone.matchers.Matchers.methodReturns;
import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.AnnotationNames.MUST_BE_CLOSED_ANNOTATION;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.MustBeClosed;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
Expand All @@ -45,8 +46,9 @@
import java.util.List;

/**
* Checks if a constructor or method annotated with {@link MustBeClosed} is called within the
* resource variable initializer of a try-with-resources statement.
* Checks if a constructor or method annotated with {@link
* com.google.errorprone.annotations.MustBeClosed} is called within the resource variable
* initializer of a try-with-resources statement.
*/
@BugPattern(
altNames = "MustBeClosed",
Expand All @@ -68,7 +70,7 @@ public class MustBeClosedChecker extends AbstractMustBeClosedChecker
allOf(methodIsConstructor(), enclosingClass(isSubtypeOf(AutoCloseable.class)));

/**
* Check that the {@link MustBeClosed} annotation is only used for constructors of AutoCloseables
* Check that the {@code MustBeClosed} annotation is only used for constructors of AutoCloseables
* and methods that return an AutoCloseable.
*/
@Override
Expand Down Expand Up @@ -125,7 +127,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {

MethodTree methodTree = (MethodTree) member;
if (!ASTHelpers.getSymbol(methodTree).isConstructor()
|| ASTHelpers.hasAnnotation(methodTree, MustBeClosed.class, state)
|| hasAnnotation(methodTree, MUST_BE_CLOSED_ANNOTATION, state)
|| !invokedConstructorMustBeClosed(state, methodTree)) {
continue;
}
Expand All @@ -141,7 +143,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
SuggestedFix.Builder builder = SuggestedFix.builder();
String suggestedFixName =
SuggestedFixes.qualifyType(
state, builder, state.getTypeFromString(MustBeClosed.class.getCanonicalName()));
state, builder, state.getTypeFromString(MUST_BE_CLOSED_ANNOTATION));
SuggestedFix fix = builder.prefixWith(methodTree, "@" + suggestedFixName + " ").build();

state.reportMatch(
Expand All @@ -167,6 +169,6 @@ private static boolean invokedConstructorMustBeClosed(VisitorState state, Method
ExpressionStatementTree est = (ExpressionStatementTree) statements.get(0);
MethodInvocationTree mit = (MethodInvocationTree) est.getExpression();
MethodSymbol invokedConstructorSymbol = ASTHelpers.getSymbol(mit);
return ASTHelpers.hasAnnotation(invokedConstructorSymbol, MustBeClosed.class, state);
return hasAnnotation(invokedConstructorSymbol, MUST_BE_CLOSED_ANNOTATION, state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
import static com.google.errorprone.util.ASTHelpers.getReceiverType;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.AnnotationNames.FORMAT_METHOD_ANNOTATION;
import static com.google.errorprone.util.AnnotationNames.FORMAT_STRING_ANNOTATION;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;
import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
Expand Down Expand Up @@ -89,9 +89,10 @@ public class OrphanedFormatString extends BugChecker implements LiteralTreeMatch
&& !findMatchingMethods(
getSymbol(t).name,
ms ->
hasAnnotation(ms, FormatMethod.class, s)
hasAnnotation(ms, FORMAT_METHOD_ANNOTATION, s)
|| ms.getParameters().stream()
.anyMatch(vs -> hasAnnotation(vs, FormatString.class, s)),
.anyMatch(
vs -> hasAnnotation(vs, FORMAT_STRING_ANNOTATION, s)),
getReceiverType(t),
s.getTypes())
.isEmpty()));
Expand Down Expand Up @@ -123,7 +124,7 @@ && literalIsFormatMethodArg(tree, (MethodInvocationTree) methodInvocation, state
private static boolean literalIsFormatMethodArg(
LiteralTree tree, MethodInvocationTree methodInvocationTree, VisitorState state) {
MethodSymbol symbol = getSymbol(methodInvocationTree);
if (hasAnnotation(symbol, FormatMethod.class, state)) {
if (hasAnnotation(symbol, FORMAT_METHOD_ANNOTATION, state)) {
int indexOfParam = findIndexOfFormatStringParameter(state, symbol);
if (indexOfParam != -1) {
List<? extends ExpressionTree> args = methodInvocationTree.getArguments();
Expand All @@ -142,7 +143,7 @@ private static int findIndexOfFormatStringParameter(VisitorState state, MethodSy
List<VarSymbol> params = symbol.params();
for (int i = 0; i < params.size(); i++) {
VarSymbol varSymbol = params.get(i);
if (hasAnnotation(varSymbol, FormatString.class, state)) {
if (hasAnnotation(varSymbol, FORMAT_STRING_ANNOTATION, state)) {
return i;
}
if (indexOfFirstString == -1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static java.lang.Math.max;

import com.google.common.base.Joiner;
Expand Down Expand Up @@ -54,7 +55,8 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s

// package-info annotations are special
// TODO(cushon): fix the core suppression logic to handle this
if (ASTHelpers.hasAnnotation(tree.getPackage(), SuppressPackageLocation.class, state)) {
if (hasAnnotation(
tree.getPackage(), "com.google.errorprone.annotations.SuppressPackageLocation", state)) {
return Description.NO_MATCH;
}

Expand Down
Loading

0 comments on commit 70df1fd

Please sign in to comment.