Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
--- cpovirk adds: This turns out to be very important but only in pseudorandom cases. JavaStubifier sometimes fails to identify java.base/share/classes as a source root, and thus it chooses not to stubify the files under this directory. The reason that it sometimes fails to identify the directory as a source root is the invalid module-info file. But sometimes, even though the invalid module-info file is processed and rejected, JavaStubifier still identifies the directory as a source root, and thus it chooses to stubify the files under the directory. The difference seems to be the order in which the filesystem returns the contents of that directory. As it happens, my root filesystem (where my /tmp is) has been consistently returning the contents in a bad order, while my larger filesystem (where my home directory is) has been consistently returning the contents in a good order. (GitHub Actions, too, has been consistently seen the contents in a bad order. See jspecify/jspecify-reference-checker#9 for some false starts on my investigation of that. I may still end up wanting to commit part of that change -- the part that turns `applyQualifierParameterDefaults` into a no-op -- depending on what happens after this fix.) Presumably, if we see module-info before we see a file that we can "usefully parse," we skip scanning more files under the directory. Or something like that. I haven't dug into this deeply, but I assume the relevant logic is the values returned from the `FileVisitor` implementation in `ParserCollectionStrategy`, e.g., https://github.com/typetools/stubparser/blob/25e36946755b040d23c0af8f4b7bd2bc0fcf4042/javaparser-core/src/main/java/com/github/javaparser/utils/ParserCollectionStrategy.java#L84 (By "usefully parse," I'm not sure exactly what I mean. But it's something like "Can we get enough information out of it to identify the source root -- typically, a package name that matches the directory hierarchy?" However, even under the "bad order" I was seeing, I would have thought we would still have gotten that far, since I'm seeing a normal-looking .java file processed before module-info. I'd need to dig more to understand exactly what's happening.) Anyway, the result is that we fail to call `addSourceRoot` on the way out: https://github.com/typetools/stubparser/blob/25e36946755b040d23c0af8f4b7bd2bc0fcf4042/javaparser-core/src/main/java/com/github/javaparser/utils/ParserCollectionStrategy.java#L92 (Previously I had a theory that the cause was my directory names: Under /tmp, I was using directories with a period in the name. I thought that maybe JavaParser was rejecting this as an invalid package name or trying to treat it as a module name or something. I don't _think_ the names were actually relevant, but I haven't tested further.) To debug this, I found it useful to add a `Log.info` to `CollectionStrategy.getRoot` and also to change logging to use stdout/stderr. To do the latter, I modified the `SourceRoot` constructor to call `Log.setAdapter(new Log.StandardOutStandardErrorAdapter());`. (Probably I could have found a better place to do that, but it was good enough this time.) So why did any of _that_ matter? Because, if JavaStubifier doesn't run on java.base/share/classes, we end up with sources that contain method bodies, private members, etc. In principle, I think that ought to be fine. However, at least one of those extras turns out to be trouble: `AnnotationFileParser.sameType` does not expect `INTERSECTION`: https://github.com/jspecify/checker-framework/blob/5ba2cd26b470a1dacb2cd6a80829d137e80b3a7a/framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java#L1838 (In an ideal world, I will minimize this and file a bug. But I probably won't anytime soon. It's entirely possible that the problem arises only because of other questionable things that our hacky checker is doing.) Thus, I see exceptions when I run the checker -- though only on _some_ code, it seems :) I was having trouble even seeing those exceptions, though: My checker runs were using up all RAM that the container made available, so they were getting killed. The problem was that `AnnotationFileElementTypes.parseJarEntry` was wrapping the original `BugInCF` instance with an instance that added the name of the stub file it was processing. That, too, would be fine, but we parse stub files recursively, which had already caused us problems in the past (jspecify/checker-framework#11). As a result, `parseJarEntry` was on the stack multiple times, and each of those frames was making the exception message longer and longer until we couldn't allocate any more: https://github.com/jspecify/checker-framework/blob/5ba2cd26b470a1dacb2cd6a80829d137e80b3a7a/framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileElementTypes.java#L584 I had some luck avoiding this by running the checker myself locally instead of under GitHub Actions or our usual Google build farm. I'm not sure if that just made more memory available or if it let me see some streamed output that would otherwise have been lost or what. (In hindsight, I may have found it useful to just change that `throw new BugInCF` line to rethrow `e` directly. Then again, I had tried throwing an `AssertionError` wrapper instead, and that would probably have worked, too, if only the tests hadn't started _succeeding_ then because of the "good order" / "bad order" issue!) --- `AnnotationFileParser.sameType` does not expect `INTERSECTION`: ``` Exception while parsing annotated-jdk/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java: Unhandled type java.lang.Object&java.lang.Comparable<? super T> of kind INTERSECTION at org.checkerframework.framework.stub.AnnotationFileElementTypes.parseJarEntry(AnnotationFileElementTypes.java:584) at org.checkerframework.framework.stub.AnnotationFileElementTypes.parseEnclosingClass(AnnotationFileElementTypes.java:502) at org.checkerframework.framework.stub.AnnotationFileElementTypes.getDeclAnnotation(AnnotationFileElementTypes.java:374) at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotations(AnnotatedTypeFactory.java:3703) at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotation(AnnotatedTypeFactory.java:3631) at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotation(AnnotatedTypeFactory.java:3553) at com.google.jspecify.nullness.NullSpecAnnotatedTypeFactory.hasNullAwareOrEquivalent(NullSpecAnnotatedTypeFactory.java:1416) at com.google.jspecify.nullness.NullSpecAnnotatedTypeFactory.access$800(NullSpecAnnotatedTypeFactory.java:116) at com.google.jspecify.nullness.NullSpecAnnotatedTypeFactory$NullSpecQualifierDefaults.populateNewDefaults(NullSpecAnnotatedTypeFactory.java:777) at org.checkerframework.framework.util.defaults.QualifierDefaults.defaultsAt(QualifierDefaults.java:629) at org.checkerframework.framework.util.defaults.QualifierDefaults.applyDefaultsElement(QualifierDefaults.java:695) at org.checkerframework.framework.util.defaults.QualifierDefaults.annotate(QualifierDefaults.java:405) at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.addComputedTypeAnnotations(GenericAnnotatedTypeFactory.java:1986) at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.postDirectSuperTypes(GenericAnnotatedTypeFactory.java:854) at org.checkerframework.framework.type.SupertypeFinder.directSupertypes(SupertypeFinder.java:55) at org.checkerframework.framework.type.AnnotatedTypeMirror$AnnotatedDeclaredType.directSupertypes(AnnotatedTypeMirror.java:974) at org.checkerframework.framework.stub.AnnotationFileParser.annotateSupertypes(AnnotationFileParser.java:989) at org.checkerframework.framework.stub.AnnotationFileParser.processType(AnnotationFileParser.java:935) at org.checkerframework.framework.stub.AnnotationFileParser.processTypeDecl(AnnotationFileParser.java:813) at org.checkerframework.framework.stub.AnnotationFileParser.processCompilationUnit(AnnotationFileParser.java:685) at org.checkerframework.framework.stub.AnnotationFileParser.processStubUnit(AnnotationFileParser.java:661) at org.checkerframework.framework.stub.AnnotationFileParser.process(AnnotationFileParser.java:650) at org.checkerframework.framework.stub.AnnotationFileParser.parseStubFile(AnnotationFileParser.java:604) at org.checkerframework.framework.stub.AnnotationFileParser.parseJdkFileAsStub(AnnotationFileParser.java:580) at org.checkerframework.framework.stub.AnnotationFileElementTypes.parseJarEntry(AnnotationFileElementTypes.java:575) at org.checkerframework.framework.stub.AnnotationFileElementTypes.parseEnclosingClass(AnnotationFileElementTypes.java:502) at org.checkerframework.framework.stub.AnnotationFileElementTypes.getAnnotatedTypeMirror(AnnotationFileElementTypes.java:354) at org.checkerframework.framework.type.AnnotatedTypeFactory.fromElement(AnnotatedTypeFactory.java:1280) at org.checkerframework.framework.type.AnnotatedTypeFactory.fromElement(AnnotatedTypeFactory.java:2884) at org.checkerframework.common.basetype.BaseTypeVisitor.<init>(BaseTypeVisitor.java:242) at org.checkerframework.common.basetype.BaseTypeVisitor.<init>(BaseTypeVisitor.java:223) at com.google.jspecify.nullness.NullSpecVisitor.<init>(NullSpecVisitor.java:91) at com.google.jspecify.nullness.NullSpecChecker.createSourceVisitor(NullSpecChecker.java:49) at com.google.jspecify.nullness.NullSpecChecker.createSourceVisitor(NullSpecChecker.java:36) at org.checkerframework.framework.source.SourceChecker.initChecker(SourceChecker.java:878) at org.checkerframework.common.basetype.BaseTypeChecker.initChecker(BaseTypeChecker.java:112) at org.checkerframework.framework.source.SourceChecker.typeProcessingStart(SourceChecker.java:829) at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:167) at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828) at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132) at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1394) at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1351) at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:947) at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:311) at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:170) at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:57) at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:43) ``` --- "Bad order": ``` New source root at "/tmp/tmp.hMPBYGuMAr/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/unix/classes" trying /tmp/tmp.hMPBYGuMAr/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/share/classes/jdk/internal/module/ModuleReferenceImpl.java trying /tmp/tmp.hMPBYGuMAr/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/share/classes/module-info.java Parsing was not successful. There were (1) problems parsing file: [(line 341,col 20) Parse error. Found "exports", expected one of "," ";" Problem stacktrace : com.github.javaparser.GeneratedJavaParser.generateParseException(GeneratedJavaParser.java:13115) com.github.javaparser.GeneratedJavaParser.jj_consume_token(GeneratedJavaParser.java:12961) com.github.javaparser.GeneratedJavaParser.ModuleDirective(GeneratedJavaParser.java:7745) com.github.javaparser.GeneratedJavaParser.ModuleDeclaration(GeneratedJavaParser.java:7865) com.github.javaparser.GeneratedJavaParser.CompilationUnit(GeneratedJavaParser.java:377) com.github.javaparser.JavaParser.parse(JavaParser.java:124) com.github.javaparser.JavaParser.parse(JavaParser.java:307) com.github.javaparser.utils.CollectionStrategy.getRoot(CollectionStrategy.java:50) com.github.javaparser.utils.ParserCollectionStrategy$1.visitFile(ParserCollectionStrategy.java:73) com.github.javaparser.utils.ParserCollectionStrategy$1.visitFile(ParserCollectionStrategy.java:66) java.base/java.nio.file.Files.walkFileTree(Files.java:2726) java.base/java.nio.file.Files.walkFileTree(Files.java:2798) com.github.javaparser.utils.ParserCollectionStrategy.collect(ParserCollectionStrategy.java:66) org.checkerframework.framework.stub.JavaStubifier.process(JavaStubifier.java:74) org.checkerframework.framework.stub.JavaStubifier.main(JavaStubifier.java:57)] trying /tmp/tmp.hMPBYGuMAr/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java ``` --- "Good order": ``` New source root at "/usr/local/google/home/cpovirk/clients/nullness-checker-for-checker-framework-everything/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/unix/classes" trying /usr/local/google/home/cpovirk/clients/nullness-checker-for-checker-framework-everything/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/share/classes/java/security/AlgorithmParametersSpi.java trying /usr/local/google/home/cpovirk/clients/nullness-checker-for-checker-framework-everything/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/share/classes/module-info.java Parsing was not successful. There were (1) problems parsing file: [(line 341,col 20) Parse error. Found "exports", expected one of "," ";" Problem stacktrace : com.github.javaparser.GeneratedJavaParser.generateParseException(GeneratedJavaParser.java:13115) com.github.javaparser.GeneratedJavaParser.jj_consume_token(GeneratedJavaParser.java:12961) com.github.javaparser.GeneratedJavaParser.ModuleDirective(GeneratedJavaParser.java:7745) com.github.javaparser.GeneratedJavaParser.ModuleDeclaration(GeneratedJavaParser.java:7865) com.github.javaparser.GeneratedJavaParser.CompilationUnit(GeneratedJavaParser.java:377) com.github.javaparser.JavaParser.parse(JavaParser.java:124) com.github.javaparser.JavaParser.parse(JavaParser.java:307) com.github.javaparser.utils.CollectionStrategy.getRoot(CollectionStrategy.java:50) com.github.javaparser.utils.ParserCollectionStrategy$1.visitFile(ParserCollectionStrategy.java:73) com.github.javaparser.utils.ParserCollectionStrategy$1.visitFile(ParserCollectionStrategy.java:66) java.base/java.nio.file.Files.walkFileTree(Files.java:2726) java.base/java.nio.file.Files.walkFileTree(Files.java:2798) com.github.javaparser.utils.ParserCollectionStrategy.collect(ParserCollectionStrategy.java:66) org.checkerframework.framework.stub.JavaStubifier.process(JavaStubifier.java:74) org.checkerframework.framework.stub.JavaStubifier.main(JavaStubifier.java:57)] trying /usr/local/google/home/cpovirk/clients/nullness-checker-for-checker-framework-everything/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/share/classes/com/sun/java/util/jar/pack/Instruction.java New source root at "/usr/local/google/home/cpovirk/clients/nullness-checker-for-checker-framework-everything/checker-framework/framework/build/generated/resources/annotated-jdk/src/java.base/share/classes" ```
- Loading branch information
