From 008cfb02434f698b636cb850c283c0e73121afe0 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Fri, 12 Jul 2024 11:06:25 -0700 Subject: [PATCH] Remove redundant `complete()` call and `CompletionFailure` check. Those operations are already [performed during our call to `getSymbolFromString`](https://github.com/google/error-prone/blob/03d15b56478a40534b2b104e21d070d0eebc0701/check_api/src/main/java/com/google/errorprone/VisitorState.java#L432), which returns `null` in case of failure. (I've found the comment that I'm removing a little confusing, too: I think it might be making the point that we don't want to let the `CompletionFailure` propagate because that would fail Error Prone entirely when in fact the check that called `annotationsAmong` might find one of the _other_ annotations it was looking for among the annotations inherited by the class? (We would already have handled any _directly_ present annotations in `annotationsAmong` as well as in `hasAnnotation`.) If so, we're fine, as we handle `null` gracefully. (Of course, this assumes that silently ignoring an annotation that should have been inherited isn't a bad problem that should really lead to a crash. But there's only so much we can do in the presence of an incomplete classpath.) Or is it actually making the point that we want to try to look for `@Inherited` even if completing some _other_ part of the annotation has failed (if that is even possible) and hence we fall through instead of immediately returning `false`? If so, that doesn't currently work because we would already have bailed after the `null` return from `getSymbolFromString`. Anyway, whatever the comment is getting at, this CL is a no-op, so I'm just trying to avoid putting on blinders as I remove the surrounding code.) PiperOrigin-RevId: 651828339 --- .../main/java/com/google/errorprone/util/ASTHelpers.java | 7 ------- 1 file changed, 7 deletions(-) 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 09430a8c9a7..5d552288976 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 @@ -109,7 +109,6 @@ import com.sun.tools.javac.code.Scope; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Symbol.ClassSymbol; -import com.sun.tools.javac.code.Symbol.CompletionFailure; import com.sun.tools.javac.code.Symbol.MethodSymbol; import com.sun.tools.javac.code.Symbol.PackageSymbol; import com.sun.tools.javac.code.Symbol.TypeSymbol; @@ -926,12 +925,6 @@ private static boolean isInherited(VisitorState state, Name annotationName) { if (annotationSym == null) { return false; } - try { - annotationSym.complete(); - } catch (CompletionFailure e) { - /* @Inherited won't work if the annotation isn't on the classpath, but we can still - check if it's present directly */ - } Symbol inheritedSym = state.getSymtab().inheritedType.tsym; return annotationSym.attribute(inheritedSym) != null; });