diff --git a/checker/jtreg/missingclassfile/ClassLiteral.goal b/checker/jtreg/missingclassfile/ClassLiteral.goal new file mode 100644 index 000000000000..c1b20165d8fe --- /dev/null +++ b/checker/jtreg/missingclassfile/ClassLiteral.goal @@ -0,0 +1,2 @@ +- compiler.warn.proc.messager: [class.not.completed] Cannot read a class file that is needed by lib.SuperTypeArg: class file for lib.Missing not found. Make sure your classpath is set correctly. +1 warning diff --git a/checker/jtreg/missingclassfile/Field.goal b/checker/jtreg/missingclassfile/Field.goal new file mode 100644 index 000000000000..c1b20165d8fe --- /dev/null +++ b/checker/jtreg/missingclassfile/Field.goal @@ -0,0 +1,2 @@ +- compiler.warn.proc.messager: [class.not.completed] Cannot read a class file that is needed by lib.SuperTypeArg: class file for lib.Missing not found. Make sure your classpath is set correctly. +1 warning diff --git a/checker/jtreg/missingclassfile/InheritedAnno.goal b/checker/jtreg/missingclassfile/InheritedAnno.goal new file mode 100644 index 000000000000..ada0dc9f5c36 --- /dev/null +++ b/checker/jtreg/missingclassfile/InheritedAnno.goal @@ -0,0 +1,6 @@ +- compiler.warn.proc.messager: [class.not.completed] Cannot read a class file that is needed by lib.SubOfQualParam: class file for lib.Missing not found. Make sure your classpath is set correctly. +InheritedAnno.java:14:33: compiler.err.proc.messager: [assignment] incompatible types in assignment. +found : @Untainted SubOfQualParam +required: @Tainted SubOfQualParam +1 error +1 warning diff --git a/checker/jtreg/missingclassfile/InheritedAnno.java b/checker/jtreg/missingclassfile/InheritedAnno.java new file mode 100644 index 000000000000..9db9b365f84c --- /dev/null +++ b/checker/jtreg/missingclassfile/InheritedAnno.java @@ -0,0 +1,16 @@ +import lib.SubOfQualParam; +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.checker.tainting.qual.Untainted; + +/** + * Pins down that the {@code @Inherited} declaration annotation on lib.QualParam is still found, + * even though reading lib.QualParam's supertype fails. See Issue8055.java. + */ +public class InheritedAnno { + + void m(@Untainted SubOfQualParam u) { + // Assigning @Untainted to @Tainted is an error only because SubOfQualParam inherits + // @HasQualifierParameter from lib.QualParam, which makes the qualifier invariant. + @Tainted SubOfQualParam t = u; + } +} diff --git a/checker/jtreg/missingclassfile/Issue8055.java b/checker/jtreg/missingclassfile/Issue8055.java index 4319a9c477d0..dbf20f5491ae 100644 --- a/checker/jtreg/missingclassfile/Issue8055.java +++ b/checker/jtreg/missingclassfile/Issue8055.java @@ -1,52 +1,74 @@ /* * @test * @summary The Checker Framework must not crash when it completes a classpath class whose - * supertype mentions a class that is not on the classpath. javac throws - * com.sun.tools.javac.code.Symbol$CompletionFailure and nothing catches it. Reduced from two - * crashes running the Tainting Checker on Apache Beam: + * supertype mentions a class that is not on the classpath. javac completes such a symbol + * lazily and never needs it, so javac reports no error and the Checker Framework must not + * either. Every compilation below is checked against a .goal file, so the test fails if the + * diagnostics change, not only if a compilation crashes. + * + * Issue 8055 was two bugs, one behind the other. First, + * AnnotatedTypeFactory#getDeclAnnotations called Elements#getAllAnnotationMirrors outside the + * try block meant to guard it -- getAllAnnotationMirrors walks the superclass chain looking + * for inherited annotations, and asks each superclass isErroneous(), which completes its type + * arguments -- so the CompletionFailure escaped. Second, javac's failed completion leaves the + * absent class's symbol with kind ERR, so every ClassType naming it reports TypeKind.ERROR; + * once the CompletionFailure was caught, AnnotatedTypeMirror#createType rejected that ERROR + * type with "input is not compilable". + * + * Reduced from two crashes running the Tainting Checker on Apache Beam: * * runners/google-cloud-dataflow-java/.../util/DefaultCoderCloudObjectTranslatorRegistrar.java * line 111, the class literal TableRowJsonCoder.class in the initializer of KNOWN_ATOMIC_CODERS. * TableRowJsonCoder extends AtomicCoder, and * com.google.api.services.bigquery.model.TableRow is not on the compile classpath of the - * runners:google-cloud-dataflow-java module. Crashes with + * runners:google-cloud-dataflow-java module. It crashed with * "error: class file for ... not found", because CFAbstractAnalysis#callTransferFunction - * rethrows the CompletionFailure as new BugInCF(node.getTree(), t). + * rethrew the CompletionFailure as new BugInCF(node.getTree(), t). * * sdks/java/extensions/sql/.../meta/provider/datastore/DataStoreV1TableProvider.java line 49, * whose getTableStatistics calls * DatastoreIO.v1().read().withProjectId(...).getNumEntities(...). DatastoreV1.Read extends * PTransform>, and com.google.datastore.v1.Entity is not on the - * compile classpath of the sdks:java:extensions:sql module. Crashes with + * compile classpath of the sdks:java:extensions:sql module. It crashed with * "error: SourceChecker.typeProcess: unexpected Throwable (CompletionFailure)", because the - * CompletionFailure is thrown outside dataflow and reaches the catch (Throwable t) in - * SourceChecker#typeProcess. The reported position is the start of the class declaration, which - * in Beam is the line holding the class's AutoService annotation. + * CompletionFailure was thrown outside dataflow and reached the catch (Throwable t) in + * SourceChecker#typeProcess. The reported position was the start of the class declaration, + * which in Beam is the line holding the class's AutoService annotation. * - * AnnotatedTypeFactory and ElementUtils#isElementFromByteCode already catch CompletionFailure - * and issue a warning instead (issues 309 and 348). These paths do not. + * AnnotatedTypeFactory and ElementUtils#getSuperClass already caught CompletionFailure and + * carried on instead (issues 309 and 348). These paths did not. * - * @ignore Re-enable once the CompletionFailure crash is fixed (issue number 8055) - * - * @compile libsrc/Box.java libsrc/Missing.java libsrc/SuperTypeArg.java libsrc/Factory.java libsrc/MemberOnly.java + * @compile libsrc/Box.java libsrc/Missing.java libsrc/SuperTypeArg.java libsrc/Factory.java libsrc/MemberOnly.java libsrc/QualParam.java libsrc/SubOfQualParam.java * @build DeleteMissingClassFile * @run main DeleteMissingClassFile - * - * Ok.java is checked first, so that the "must stay clean" case is exercised even while the - * crashing cases below still fail. jtreg stops a test at its first failing action, so until the - * crash is fixed only the first crashing shape below is reached; all five run once it is fixed. - * - * @compile -processor org.checkerframework.checker.tainting.TaintingChecker Ok.java - * @compile -processor org.checkerframework.checker.tainting.TaintingChecker ClassLiteral.java - * @compile -processor org.checkerframework.checker.tainting.TaintingChecker Field.java - * @compile -processor org.checkerframework.checker.tainting.TaintingChecker MethodCall.java - * @compile -processor org.checkerframework.checker.tainting.TaintingChecker Parameter.java + * @compile/ref=Ok.goal -XDrawDiagnostics -processor org.checkerframework.checker.tainting.TaintingChecker Ok.java + * @compile/ref=ClassLiteral.goal -XDrawDiagnostics -processor org.checkerframework.checker.tainting.TaintingChecker ClassLiteral.java + * @compile/ref=Field.goal -XDrawDiagnostics -processor org.checkerframework.checker.tainting.TaintingChecker Field.java + * @compile/ref=MethodCall.goal -XDrawDiagnostics -processor org.checkerframework.checker.tainting.TaintingChecker MethodCall.java + * @compile/ref=Parameter.goal -XDrawDiagnostics -processor org.checkerframework.checker.tainting.TaintingChecker Parameter.java + * @compile/fail/ref=InheritedAnno.goal -XDrawDiagnostics -processor org.checkerframework.checker.tainting.TaintingChecker InheritedAnno.java */ /* * Each shape is in its own file because a crash aborts the whole compilation unit, so only one * crashing shape per file is observable. Ok.java holds the variants that must keep compiling * cleanly; it pins down that the absent class has to appear in a type argument of the supertype. + * It is checked first, so that the "must stay clean" case is exercised before the shapes that used + * to crash; jtreg stops a test at its first failing action. + * + * Every @compile names a .goal file, because a bare @compile checks only the exit status: it would + * pass whether the warning is issued once, five times, or not at all. -XDrawDiagnostics is what + * makes the golden files portable; without it a diagnostic that has a source position prints the + * absolute path that jtreg passed to javac. + * + * InheritedAnno.java is the only compilation that is expected to fail, and its error is the + * assertion: lib.SubOfQualParam inherits @HasQualifierParameter from lib.QualParam, whose own + * supertype is the one that cannot be read, and that annotation is what makes the assignment an + * error. Without it the file compiles cleanly, so the error is evidence that a declaration + * annotation is still inherited across the class whose supertype is missing. + * + * Keep prose out of the tag block above: jtreg treats everything after `@run main + * DeleteMissingClassFile`, up to the next tag, as command-line arguments to its main method. * * The library sources are in libsrc/ rather than lib/ on purpose. jtreg compiles each file with * -sourcepath , so if they were in lib/ javac would find lib/Missing.java there and diff --git a/checker/jtreg/missingclassfile/MethodCall.goal b/checker/jtreg/missingclassfile/MethodCall.goal new file mode 100644 index 000000000000..c1b20165d8fe --- /dev/null +++ b/checker/jtreg/missingclassfile/MethodCall.goal @@ -0,0 +1,2 @@ +- compiler.warn.proc.messager: [class.not.completed] Cannot read a class file that is needed by lib.SuperTypeArg: class file for lib.Missing not found. Make sure your classpath is set correctly. +1 warning diff --git a/checker/jtreg/missingclassfile/Ok.goal b/checker/jtreg/missingclassfile/Ok.goal new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/checker/jtreg/missingclassfile/Parameter.goal b/checker/jtreg/missingclassfile/Parameter.goal new file mode 100644 index 000000000000..c1b20165d8fe --- /dev/null +++ b/checker/jtreg/missingclassfile/Parameter.goal @@ -0,0 +1,2 @@ +- compiler.warn.proc.messager: [class.not.completed] Cannot read a class file that is needed by lib.SuperTypeArg: class file for lib.Missing not found. Make sure your classpath is set correctly. +1 warning diff --git a/checker/jtreg/missingclassfile/libsrc/QualParam.java b/checker/jtreg/missingclassfile/libsrc/QualParam.java new file mode 100644 index 000000000000..d27f1100d664 --- /dev/null +++ b/checker/jtreg/missingclassfile/libsrc/QualParam.java @@ -0,0 +1,11 @@ +package lib; + +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.framework.qual.HasQualifierParameter; + +/** + * Carries an {@code @Inherited} declaration annotation and has a supertype whose type argument's + * class file is absent, so reading this class's annotations is what fails. See Issue8055.java. + */ +@HasQualifierParameter(Tainted.class) +public class QualParam extends Box {} diff --git a/checker/jtreg/missingclassfile/libsrc/SubOfQualParam.java b/checker/jtreg/missingclassfile/libsrc/SubOfQualParam.java new file mode 100644 index 000000000000..8da0fee84dd6 --- /dev/null +++ b/checker/jtreg/missingclassfile/libsrc/SubOfQualParam.java @@ -0,0 +1,7 @@ +package lib; + +/** + * Inherits {@link QualParam}'s declaration annotations across the class whose supertype cannot be + * read. See Issue8055.java. + */ +public class SubOfQualParam extends QualParam {} diff --git a/framework/src/main/java/org/checkerframework/framework/source/messages.properties b/framework/src/main/java/org/checkerframework/framework/source/messages.properties index 0d3fd70ac7c8..cec26846b7b5 100644 --- a/framework/src/main/java/org/checkerframework/framework/source/messages.properties +++ b/framework/src/main/java/org/checkerframework/framework/source/messages.properties @@ -1,4 +1,5 @@ annotation.not.completed=Element %s contains annotation %s which can't be found. Make sure your classpath is set correctly. +class.not.completed=Cannot read a class file that is needed by %s: %s. Make sure your classpath is set correctly. unneeded.suppression=warning suppression %s is not used by %s lsp.type.information=checker=%s; kind=%s; type=%s; range=%s ambiguous.ajava=Found 2 or more candidate ajava files for this class. Disambiguate them by supplying directories instead of specific .ajava files. Ambiguous files: %s. diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java index e49a4c899e97..2493d4efdb05 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java @@ -4151,7 +4151,19 @@ public AnnotationMirrorSet getDeclAnnotations(Element elt) { // Retrieving the annotations from the element. // This includes annotations inherited from superclasses, but not superinterfaces or // overridden methods. - List fromEle = elements.getAllAnnotationMirrors(elt); + List fromEle; + try { + fromEle = elements.getAllAnnotationMirrors(elt); + } catch (com.sun.tools.javac.code.Symbol.CompletionFailure cf) { + // The failed completion left the unreadable class's symbol erroneous, so the second walk + // stops where the first one threw and returns what every readable superclass contributed. + try { + fromEle = elements.getAllAnnotationMirrors(elt); + } catch (com.sun.tools.javac.code.Symbol.CompletionFailure cf2) { + fromEle = elt.getAnnotationMirrors(); + } + reportCompletionFailure(elt, cf); + } for (AnnotationMirror annotation : fromEle) { try { results.add(annotation); @@ -4191,6 +4203,31 @@ public AnnotationMirrorSet getDeclAnnotations(Element elt) { return results; } + /** + * Issues a warning that a class file that was needed to compute {@code elt}'s declaration + * annotations could not be read. + * + * @param elt the element whose declaration annotations are incomplete + * @param completionFailure the failure to read a class file + */ + private void reportCompletionFailure( + Element elt, com.sun.tools.javac.code.Symbol.CompletionFailure completionFailure) { + String eltName = ElementUtils.getQualifiedName(elt); + try { + checker.reportWarning(elt, "class.not.completed", eltName, completionFailure.getMessage()); + } catch (com.sun.tools.javac.code.Symbol.CompletionFailure nested) { + // Deciding whether the warning is suppressed reads the annotations of `elt` and of its + // enclosing elements, which can fail to read a class file too. + checker.message( + Diagnostic.Kind.WARNING, + // Keep this in sync with the class.not.completed message in messages.properties. + "Cannot read a class file that is needed by %s: %s. " + + "Make sure your classpath is set correctly.", + eltName, + completionFailure.getMessage()); + } + } + /** * Adds into {@code results} the inherited declaration annotations found in all elements of the * super types of {@code typeMirror}. (Both superclasses and superinterfaces.) diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java index c1733dc23c91..5eb9da4cb76b 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java @@ -117,11 +117,11 @@ public static AnnotatedTypeMirror createType( AnnotatedTypeMirror result; switch (type.getKind()) { case ARRAY -> result = new AnnotatedArrayType((ArrayType) type, atypeFactory); - case DECLARED -> + // An ERROR type is a class whose class file is not on the classpath. javac reported no + // error, because it completes such a symbol lazily and never needs it, so treat the type + // like any other declared type. See issue 8055. + case DECLARED, ERROR -> result = new AnnotatedDeclaredType((DeclaredType) type, atypeFactory, isDeclaration); - case ERROR -> - throw new BugInCF( - "AnnotatedTypeMirror.createType: input is not compilable. Found error type: " + type); case EXECUTABLE -> result = new AnnotatedExecutableType((ExecutableType) type, atypeFactory); case VOID, PACKAGE, NONE -> result = new AnnotatedNoType((NoType) type, atypeFactory); case NULL -> result = new AnnotatedNullType((NullType) type, atypeFactory); @@ -1073,9 +1073,13 @@ public List getTypeArguments() { } } else if (isDeclaration()) { for (TypeMirror javaTypeArg : t.getTypeArguments()) { - AnnotatedTypeVariable tv = - (AnnotatedTypeVariable) createType(javaTypeArg, atypeFactory, true); - typeArgs.add(tv); + AnnotatedTypeMirror typeArg = createType(javaTypeArg, atypeFactory, true); + if (!(typeArg instanceof AnnotatedTypeVariable)) { + throw new BugInCF( + "Type argument %s of declaration %s has kind %s, not TYPEVAR.", + javaTypeArg, t, javaTypeArg.getKind()); + } + typeArgs.add(typeArg); } } else { for (TypeMirror javaTypeArg : t.getTypeArguments()) {