Skip to content

Commit

Permalink
Remove redundant complete() call and CompletionFailure check.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cpovirk authored and Error Prone Team committed Jul 12, 2024
1 parent 3b9ffc2 commit 008cfb0
Showing 1 changed file with 0 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
});
Expand Down

0 comments on commit 008cfb0

Please sign in to comment.