typetools / checker-framework

Pluggable type-checking for Java
http://checkerframework.org/
Other
990 stars 347 forks source link

ElementAnnotationUtil.getLocationTypeADT: invalid location [WILDCARD] for type: String; #2173

Closed wmdietl closed 4 years ago

wmdietl commented 5 years ago

Bug report copied from message by William Shackleford https://groups.google.com/d/msg/checker-framework-dev/jrllYpfH1GQ/nTmnDJ1vAQAJ

I have a maven project that I have integrated checker-framework into that has started failing with the message:

[ERROR] ElementAnnotationUtil.getLocationTypeADT: invalid location [WILDCARD] for type: String; The Checker Framework crashed.  Please report the crash.

(The same message is repeated 10 times.)

Unfortunately I don't see where in my code the issue is and the project is too large to paste/attach to a bug report.

If someone has time they may be able to reproduce the problem by checking out and building the project with:

git clone https://github.com/usnistgov/crcl.git
cd crcl
# switch to dev2 branch
git checkout dev2
cd tools/java/crcl4java/crcl4java-ui/
mvn clean compile -Pwith_checkers

For me mvn -version produces:

Apache Maven 3.5.2
Maven home: /usr/share/maven
Java version: 1.8.0_181, vendor: Oracle Corporation
Java home: /home/shackle/jdk1.8.0_181/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.15.0-34-generic", arch: "amd64", family: "unix"

In case I make changes in the future and you need to reproduce the problem the current latest commit is 64cfb6dd871a106bb49360afdebd3e5e5de95d3c

bmarwell commented 5 years ago

Expieriencing the same on one of my private repos. The last class to be printed by the javac executable using verbose compilation is an immutable generated class, if that helps.

neilccbrown commented 5 years ago

I think I have the same bug, stack trace below. This is a central 1000-line class in my project so I can't easily cut it down. Some notes:

Error:java: ElementAnnotationUtil.getLocationTypeADT: invalid location TYPE_ARGUMENT(0) for type: Window
  Exception: java.lang.Throwable; Stack trace: org.checkerframework.framework.source.SourceChecker.errorAbort(SourceChecker.java:751)
  org.checkerframework.javacutil.ErrorReporter.errorAbort(ErrorReporter.java:41)
  org.checkerframework.framework.util.element.ElementAnnotationUtil.getLocationTypeADT(ElementAnnotationUtil.java:468)
  org.checkerframework.framework.util.element.ElementAnnotationUtil.getTypeAtLocation(ElementAnnotationUtil.java:388)
  org.checkerframework.framework.util.element.ElementAnnotationUtil.annotateViaTypeAnnoPosition(ElementAnnotationUtil.java:292)
  org.checkerframework.framework.util.element.ParamApplier.handleTargeted(ParamApplier.java:193)
  org.checkerframework.framework.util.element.TargetedElementAnnotationApplier.extractAndApply(TargetedElementAnnotationApplier.java:201)
  org.checkerframework.framework.util.element.ParamApplier.extractAndApply(ParamApplier.java:231)
  org.checkerframework.framework.util.element.ParamApplier.apply(ParamApplier.java:27)
  org.checkerframework.framework.type.ElementAnnotationApplier.apply(ElementAnnotationApplier.java:94)
  org.checkerframework.framework.util.element.ElementAnnotationUtil.applyAllElementAnnotations(ElementAnnotationUtil.java:72)
  org.checkerframework.framework.util.element.MethodApplier.extractAndApply(MethodApplier.java:121)
  org.checkerframework.framework.util.element.MethodApplier.apply(MethodApplier.java:30)
  org.checkerframework.framework.type.ElementAnnotationApplier.apply(ElementAnnotationApplier.java:82)
  org.checkerframework.framework.type.AnnotatedTypeFactory.fromElement(AnnotatedTypeFactory.java:1115)
  org.checkerframework.framework.type.TypeFromExpressionVisitor.visitMemberSelect(TypeFromExpressionVisitor.java:149)
  org.checkerframework.framework.type.TypeFromExpressionVisitor.visitMemberSelect(TypeFromExpressionVisitor.java:44)
  com.sun.tools.javac.tree.JCTree$JCFieldAccess.accept(JCTree.java:1903)
  com.sun.source.util.SimpleTreeVisitor.visit(SimpleTreeVisitor.java:53)
  org.checkerframework.framework.type.TypeFromTree.fromExpression(TypeFromTree.java:36)
  org.checkerframework.framework.type.AnnotatedTypeFactory.fromExpression(AnnotatedTypeFactory.java:1236)
  org.checkerframework.framework.type.AnnotatedTypeFactory.getAnnotatedType(AnnotatedTypeFactory.java:1009)
  org.checkerframework.framework.flow.CFAbstractTransfer.getValueFromFactory(CFAbstractTransfer.java:201)
  org.checkerframework.framework.flow.CFAbstractTransfer.visitNode(CFAbstractTransfer.java:572)
  org.checkerframework.framework.flow.CFAbstractTransfer.visitNode(CFAbstractTransfer.java:93)
  org.checkerframework.dataflow.cfg.node.AbstractNodeVisitor.visitMethodAccess(AbstractNodeVisitor.java:241)
  org.checkerframework.dataflow.cfg.node.MethodAccessNode.accept(MethodAccessNode.java:49)
  org.checkerframework.dataflow.analysis.Analysis.callTransferFunction(Analysis.java:408)
  org.checkerframework.dataflow.analysis.Analysis.performAnalysisBlock(Analysis.java:234)
  org.checkerframework.dataflow.analysis.Analysis.performAnalysis(Analysis.java:187)
  org.checkerframework.framework.flow.CFAbstractAnalysis.performAnalysis(CFAbstractAnalysis.java:94)
  org.checkerframework.framework.type.GenericAnnotatedTypeFactory.analyze(GenericAnnotatedTypeFactory.java:1212)
  org.checkerframework.framework.type.GenericAnnotatedTypeFactory.analyze(GenericAnnotatedTypeFactory.java:1171)
  org.checkerframework.framework.type.GenericAnnotatedTypeFactory.performFlowAnalysis(GenericAnnotatedTypeFactory.java:1107)
  org.checkerframework.framework.type.GenericAnnotatedTypeFactory.checkAndPerformFlowAnalysis(GenericAnnotatedTypeFactory.java:1472)
  org.checkerframework.framework.type.GenericAnnotatedTypeFactory.preProcessClassTree(GenericAnnotatedTypeFactory.java:258)
  org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:291)
  org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:158)
  com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:720)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:50)
  org.checkerframework.framework.source.SourceVisitor.visit(SourceVisitor.java:89)
  org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:1004)
  org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:500)
  org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:493)
  org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:182)
  com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:681)
  com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:111)
  com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1342)
  com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1296)
  com.sun.tools.javac.main.JavaCompiler.compile2(JavaCompiler.java:901)
  com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:860)
  com.sun.tools.javac.main.Main.compile(Main.java:523)
  com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:129)
  com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:138)
  org.jetbrains.jps.javac.JavacMain.compile(JavacMain.java:193)
  org.jetbrains.jps.incremental.java.JavaBuilder.compileJava(JavaBuilder.java:448)
  org.jetbrains.jps.incremental.java.JavaBuilder.compile(JavaBuilder.java:318)
  org.jetbrains.jps.incremental.java.JavaBuilder.doBuild(JavaBuilder.java:243)
  org.jetbrains.jps.incremental.java.JavaBuilder.build(JavaBuilder.java:201)
  org.jetbrains.jps.incremental.IncProjectBuilder.runModuleLevelBuilders(IncProjectBuilder.java:1317)
  org.jetbrains.jps.incremental.IncProjectBuilder.runBuildersForChunk(IncProjectBuilder.java:993)
  org.jetbrains.jps.incremental.IncProjectBuilder.buildTargetsChunk(IncProjectBuilder.java:1065)
  org.jetbrains.jps.incremental.IncProjectBuilder.buildChunkIfAffected(IncProjectBuilder.java:956)
  org.jetbrains.jps.incremental.IncProjectBuilder.access$500(IncProjectBuilder.java:73)
  org.jetbrains.jps.incremental.IncProjectBuilder$BuildParallelizer.lambda$queueTask$0(IncProjectBuilder.java:927)
  com.intellij.util.concurrency.BoundedTaskExecutor.doRun(BoundedTaskExecutor.java:226)
  com.intellij.util.concurrency.BoundedTaskExecutor.access$100(BoundedTaskExecutor.java:26)
  com.intellij.util.concurrency.BoundedTaskExecutor$2$1.run(BoundedTaskExecutor.java:199)
  com.intellij.util.ConcurrencyUtil.runUnderThreadName(ConcurrencyUtil.java:229)
  com.intellij.util.concurrency.BoundedTaskExecutor$2.run(BoundedTaskExecutor.java:193)
  org.jetbrains.jps.service.impl.SharedThreadPoolImpl.lambda$executeOnPooledThread$0(SharedThreadPoolImpl.java:42)
  java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
  java.util.concurrent.FutureTask.run(FutureTask.java:266)
  java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
  java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
  java.lang.Thread.run(Thread.java:748)
bmarwell commented 5 years ago

Since 2.5.7 (November Release) I get a bunch of them. Casting in a stream is often involved, but the error message (last visited line) will always only show the line of the class declaration.

i.E. it's probably related to https://github.com/typetools/checker-framework/issues/979

To see how to get there:

do

// ignore this with suppresswarnings
Collection<?> mycollection = singletonList((String)null); 

mycollection.stream()
  .filter(Objects::nonNull)
  .filter(item -> item instanceof String)
  .map(item -> (String) item) // or even (@NonNull String) item
  .collect(toSet());

No chance to get it working! You'll either get a warning about NonNull vs Nullable, or (if you add the NonNull annotation) you'll get the ElementAnnotationUtil.getLocationTypeADT: invalid location.

wmdietl commented 5 years ago

@bmhm Thanks for the test case. I'm unfortunately unable to reproduce the problem. I take the following .java file:

import java.util.Collection;
import java.util.Collections;
import java.util.Objects;
import java.util.stream.Collectors;
import org.checkerframework.checker.nullness.qual.NonNull;

class Issue2173 {
  void foo() {
    Collection<?> mycollection = Collections.singletonList((String) null);

    mycollection.stream()
      .filter(Objects::nonNull)
      .filter(item -> item instanceof String)
      .map(item -> (@NonNull String) item)
      .collect(Collectors.toSet());
  }
}

When I run the current Nullness Checker I get:

Issue2173.java:14: warning: [cast.unsafe] "capture#191 of ?[ extends @Initialized @Nullable Object super @Initialized @NonNull Void]" may not be casted to the type "@Initialized @NonNull String"
      .map(item -> (@NonNull String) item)
                   ^
1 warning

What do I need to change to reproduce the invalid location error?

bmarwell commented 5 years ago

Ooh, something just came to my mind! Perhaps it's not the code but the jdk? I just realized my build server will build it just fine, but I get the error locally.

Please try the IBM j9 (please find it bundled with liberty profile on wasdev) and the successor openjdk-openj9 available from adoptopenjdk.net. J9 is quite different compared to hotspot.

That might be the reason.

bmarwell commented 5 years ago

Also regarding your example: what's wrong with the cast?

wmdietl commented 5 years ago

Regarding the JDK: I don't have any experience with j9 yet. Let me try that and see.

Regarding the warning from the cast: because of #979 we infer @Nullable as bound for item. Therefore, the cast could fail and we issue a warning in that situation. The first filter should already refine the bounds, but we currently don't #1345.

wmdietl commented 5 years ago

I tried OpenJ9 and still can't reproduce the issue.

openjdk version "1.8.0_192"
OpenJDK Runtime Environment (build 1.8.0_192-b12_openj9)
Eclipse OpenJ9 VM (build openj9-0.11.0, JRE 1.8.0 Linux amd64-64-Bit Compressed References 20181107_95 (JIT enabled, AOT enabled)
OpenJ9   - 090ff9dcd
OMR      - ea548a66
JCL      - b5a3affe73 based on jdk8u192-b12)

./checker/bin-devel/javac -processor nullness Issue2173.java 

Issue2173.java:14: warning: [cast.unsafe] "capture#599 of ?[ extends @Initialized @Nullable Object super @Initialized @NonNull Void]" may not be casted to the type "@Initialized @NonNull String"
      .map(item -> (@NonNull String) item)
                   ^
1 warning

I even built a version of our annotated jdk8.jar using J9 and still don't see the exception in the test case.

Can you try again to reproduce the issue with a small example?

bmarwell commented 5 years ago

I'll try. I had an example yesterday where I implemented a jsonb serializer (the new java ee8 JSON binding standard). Still got it on my internal svn.

Thanks for looking into it.

bmarwell commented 5 years ago

I tried my very best, but I still was not able to reproduce it. I took the maven module into a new project and the error was gone. I then tried to check all maven plugin versions, but with no luck. Sorry…

neilccbrown commented 5 years ago

I've managed to attach a debugger to the compiler when the problem occurred. I have a method like this in class ImportManager:

public void chooseAndImportFile(Window parent, TableManager tableManager, CellPosition destination, Consumer<DataSource> onLoad)

When this method is called from class View, the Window parent parameter has a @Localized annotation on it with a position of:

[METHOD_FORMAL_PARAMETER, param_index = 0, location = (TYPE_ARGUMENT(0)), pos = -1]

Except that Window doesn't have a type argument, hence the error. Not to mention that Window probably shouldn't be interacting with the @Localized checker? None of the other annotations have TYPE_ARGUMENT in them. I'll keep digging, I guess the remaining questions are: (a) how did this invalid location get there, and (b) how come a rebuild will fix it, but I can reproduce it reliably in this state until I rebuild. It feels like it may relate to pulling information out of already-compiled class files versus pulling it out of source code in the same compilation?

neilccbrown commented 5 years ago

Now that I know where the issue lies, I've been able to produce a reasonably minimal example. I have two classes which I put in an IntelliJ project, running the Nullness checker:

import javafx.stage.FileChooser.ExtensionFilter;
import javafx.stage.Window;
import org.checkerframework.checker.i18n.qual.LocalizableKey;
import org.checkerframework.checker.i18n.qual.Localized;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

interface Importer {
    public List<Pair<@Localized String, List<String>>> getSupportedFileTypes();
}
class TranslationUtility
{
    @SuppressWarnings("i18n")
    public static @Localized String getString(@LocalizableKey String key) { return key;}
}
interface Pair<A, B>
{
    A getFirst();
    B getSecond();
}

public class ImporterManager
{
    private static final List<Importer> registeredImporters = new ArrayList<>();

    public static void chooseAndImportFile(Window parent)
    {
        ArrayList<ExtensionFilter> filters = new ArrayList<>();
        filters.add(new ExtensionFilter(TranslationUtility.getString("importer.all.known"), registeredImporters.stream().flatMap(imp -> imp.getSupportedFileTypes().stream().flatMap((Pair<@Localized String, List<String>> p) -> p.getSecond().stream())).collect(Collectors.toList())));
    }
}

And a second:

import javafx.stage.Stage;
import javafx.stage.Window;

public class View
{
    private static void createTable()
    {
        Window window = new Stage();
        ImporterManager.chooseAndImportFile(window);
    }
}

If I Rebuild the project, everything is always fine. If I then make a trivial modification to View (e.g. add a blank line) and Build Project, I always get the error. The culprit seems to be a combination of that messy stream code in ImporterManager, coupled with building View separately to ImporterManager. I hope that helps you pinpoint the issue. I couldn't reproduce this with Maven because Maven seems to always build all classes from a module at once, and the separate compilation seems to be crucial here. I don't think IntelliJ is relevant other than that it builds only the modified classes, leaving the existing up-to-date classes out of it.

mernst commented 5 years ago

Thanks very much for the reproducible test case! This is extremely helpful.

These commands reproduce the problem from the command line for me:

export JAVA_HOME=$HOME/java/jdk1.8.0_122-b02
cd ~/tmp/issue2173/
javacheck -processor nullness -cp .:${JAVA_HOME}/jre/lib/ext/jfxrt.jar ImporterManager.java
touch View.java
javacheck -processor nullness -cp .:${JAVA_HOME}/jre/lib/ext/jfxrt.jar View.java
mernst commented 5 years ago

I have committed a test case in branch issue2173, where we can work on this issue.

wmdietl commented 5 years ago

@twistedsquare @bmhm Thanks a lot for the minimized test case and sorry for not getting to this sooner. In file ImporterManager.java method chooseAndImportFile there is a lambda parameter that has an annotated @Localized String type argument.

Java 8 stored this type annotation with method chooseAndImportFile, which then causes the crash when the Checker Framework tries to read that class and doesn't see a suitable type with type argument. Java 9 fixed this bug and no longer stores these incorrect type annotations.

I'm adding the test case here: https://github.com/eisop/checker-framework/pull/23/files The test passed locally and I hope it does on Travis. It should be possible to remove the dependencies on JavaFX and minimize the test to remove the .class files. We should do that before merging the PR.

So once #1224 is fixed and we move to a Java 9 compiler, we can close this bug.

bmarwell commented 5 years ago

@wmdietl you referenced a java 9 issue, but this issue is about java 8. Does this mean you're giving up on java 8 support? :(

wmdietl commented 5 years ago

The eisop version of the Checker Framework uses a Java 9 javac, but can still be executed with a Java 8 JVM. You can use -source 8/-target 8 to generate Java 8 bytecode. So using the Java 9 javac shouldn't exclude any existing users.

bmarwell commented 5 years ago

Hello @wmdietl

I was able to reproduce it in another repository: https://github.com/bmhm/zchunk-java/tree/346dc226208e32012c89c7b7f90386480f4a0906

I use checker 2.8.1. If you remove the skips at the end (ReflectionUtil and ChecksumUtil), both will fail with this message:

[ERROR] ElementAnnotationUtil.getLocationTypeADT: invalid location [WILDCARD] for type: String
[ERROR]   Compilation unit: $USER/git/zchunk-java/fileformat/src/main/java/io/github/zchunk/fileformat/util/ChecksumUtil.java
[ERROR]   Last visited tree at line 41 column 1:
[ERROR]   public final class ChecksumUtil {
[ERROR]   Exception: java.lang.Throwable; Stack trace: org.checkerframework.javacutil.BugInCF.<init>(BugInCF.java:25)

You can check out that specific commit, remove the skips in the main pom.xml and run mvn clean install -DskipTests=true -Pchecker.

bmarwell commented 5 years ago

Got another one. The last one was fixed by moving the annotation between final String instead of the front.

This commit: https://github.com/zchunk/zchunk-java/commit/c7294aae70615673fbd8604dd63de0aacd9e024f Has no such issue.

This is the two commits (one push) that broke the build: https://github.com/zchunk/zchunk-java/compare/62c893aabc1252bb45b8fa3772beae4bf119bb3c...c7294aae70615673fbd8604dd63de0aacd9e024f

[INFO] -------------------------------------------------------------
[ERROR] ElementAnnotationUtil.getLocationTypeADT: invalid location [WILDCARD] for type: String
  Compilation unit: $USER/git/zchunk-java/app/src/main/java/io/github/zchunk/app/commands/Unzck.java
  Last visited tree at line 43 column 1:
  @Command(description = "Unpacks the completely downloaded zck file.",
  Exception: java.lang.Throwable; Stack trace: org.checkerframework.javacutil.BugInCF.<init>(BugInCF.java:25)
wmdietl commented 5 years ago

@bmhm Did you try whether you run into the same problem with https://github.com/eisop/checker-framework ? There are now separate Maven artifacts to make testing easy: https://github.com/typetools/checker-framework/issues/1224#issuecomment-495764269 My goal is to switch to the Java 9 compiler for the July 2019 release and that should fix this error.

bmarwell commented 4 years ago

@wmdietl you can check out the current master branch yourself whenever you like.

However, it does not work with java 8:

[ERROR] Unexpected NoSuchMethodError when invoking BaseAnnotatedTypeFactory for checker TaintingChecker
  Exception: java.lang.NoSuchMethodError: com.sun.tools.javac.code.Type.stripMetadata()Lcom/sun/tools/javac/code/Type;; Stack trace: org.checkerframework.javacutil.TypeAnnotationUtils.unannotatedType(TypeAnnotationUtils.java:465)
  org.checkerframework.framework.type.BoundsInitializer.initializeTypeArgs(BoundsInitializer.java:99)
  org.checkerframework.framework.type.AnnotatedTypeMirror$AnnotatedDeclaredType.getTypeArguments(AnnotatedTypeMirror.java:899)
mernst commented 4 years ago

Can you please try to reproduce this problem using the Java 8 compiler, version 211 or later? Version 211 fixes some bugs, whereas previous versions create incorrect .class files for certain code that uses wildcards.

bmarwell commented 4 years ago

No, does not work.

  1. Java version
$ java -version
openjdk version "1.8.0_212"
OpenJDK Runtime Environment (build 1.8.0_212-b04)
Eclipse OpenJ9 VM (build openj9-0.14.2, JRE 1.8.0 Linux amd64-64-Bit Compressed References 20190521_315 (JIT enabled, AOT enabled)
OpenJ9   - 4b1df46fe
OMR      - b56045d2
JCL      - a8c217d402 based on jdk8u212-b04)
  1. Compilation
$ ./mvnw clean install -DskipTests=true -Pchecker
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] InvocationTargetException when invoking constructor for class org.checkerframework.checker.nullness.KeyForAnnotatedTypeFactory; Underlying cause: java.lang.NoSuchMethodError: com/sun/tools/javac/code/Type.stripMetadata()Lcom/sun/tools/javac/code/Type; (loaded from file:$USER/.jabba/jdk/adopt-openj9@1.8.212-04/lib/tools.jar by java.net.FactoryURLClassLoader@62a3a9be) called from class org.checkerframework.javacutil.TypeAnnotationUtils (loaded from file:$USER/.m2/repository/io/github/eisop/checker/3.0.0-b2/checker-3.0.0-b2.jar by java.net.URLClassLoader@9c76ae4).
  Exception: java.lang.reflect.InvocationTargetException; Stack trace: sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
  sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
  sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
  java.lang.reflect.Constructor.newInstance(Constructor.java:423)
  org.checkerframework.common.basetype.BaseTypeChecker.invokeConstructorFor(BaseTypeChecker.java:274)
  org.checkerframework.common.basetype.BaseTypeVisitor.createTypeFactory(BaseTypeVisitor.java:261)

[…]

  Underlying Exception: java.lang.NoSuchMethodError: com/sun/tools/javac/code/Type.stripMetadata()Lcom/sun/tools/javac/code/Type; (loaded from file:$USER/.jabba/jdk/adopt-openj9@1.8.212-04/lib/tools.jar by java.net.FactoryURLClassLoader@62a3a9be) called from class org.checkerframework.javacutil.TypeAnnotationUtils (loaded from file:$USER/.m2/repository/io/github/eisop/checker/3.0.0-b2/checker-3.0.0-b2.jar by java.net.URLClassLoader@9c76ae4).; Stack trace: org.checkerframework.javacutil.TypeAnnotationUtils.unannotatedType(TypeAnnotationUtils.java:465)
  org.checkerframework.framework.type.BoundsInitializer.initializeTypeArgs(BoundsInitializer.java:99)
  org.checkerframework.framework.type.AnnotatedTypeMirror$AnnotatedDeclaredType.getTypeArguments(AnnotatedTypeMirror.java:899)

@mernst I think we got a new error (no such method).

Please be so kind and check out the project yourself and compile using:

jabba install adopt-openj9@1.8.212-04
jabba use adopt-openj9@1.8.212-04
./mvnw clean install -DskipTests=true -Pchecker
bmarwell commented 4 years ago

This issue is open for almost a year, is a critical blocker, and still exists in 2.9.0. :(

// Edit: wanted to mention that there is a reproducible build error in comment https://github.com/typetools/checker-framework/issues/2173#issuecomment-497922760

wmdietl commented 4 years ago

We are planning to make a 3.0 release in August, which will build on the Java 9 compiler. As said before, you can already use that version at https://github.com/eisop/checker-framework and there are Maven pre-release artifacts at https://search.maven.org/search?q=g:io.github.eisop Have you tried that version?

bmarwell commented 4 years ago

@wmdietl You always say "please try this, please try that". I am NOT the developer, you are. You ask me to test versions which you haven't even checked yourself. Heck, you didn't even adapt a single unit test case.

PRETTY PLEASE test versions yourself if there is an easy setup. I mean, come on, this issue is rated "crash" and "critical", and you haven't even looked into it. The only thing you did is sit there and wait for those having problems to do YOUR work. I'd be happy to test it the next time you say "Works for me" or "here's a junit test which I think proves that this issues has been resolved".

For the sake of progress, here's the new stack trace:

[ERROR] SourceChecker.typeProcessingStart: unexpected Throwable (NoSuchMethodError); message: com/sun/tools/javac/code/Type.stripMetadata()Lcom/sun/tools/javac/code/Type; (loaded from file:$HOME/.jabba/jdk/adopt-openj9@1.8.212-04+openjfx8/lib/tools.jar by java.net.FactoryURLClassLoader@c86f694b) called from class org.checkerframework.javacutil.TypeAnnotationUtils (loaded from file:$HOME/.m2/repository/io/github/eisop/checker/3.0.0-b2/checker-3.0.0-b2.jar by java.net.URLClassLoader@1720fdb1).
[ERROR]   Exception: java.lang.NoSuchMethodError: com/sun/tools/javac/code/Type.stripMetadata()Lcom/sun/tools/javac/code/Type; (loaded from file:$HOME/.jabba/jdk/adopt-openj9@1.8.212-04+openjfx8/lib/tools.jar by java.net.FactoryURLClassLoader@c86f694b) called from class org.checkerframework.javacutil.TypeAnnotationUtils (loaded from file:$HOME/.m2/repository/io/github/eisop/checker/3.0.0-b2/checker-3.0.0-b2.jar by java.net.URLClassLoader@1720fdb1).; Stack trace: org.checkerframework.javacutil.TypeAnnotationUtils.unannotatedType(TypeAnnotationUtils.java:465)

Just to make sure you read it, here's a quote from above:

I'd be happy to test it the next time you say "Works for me" or "here's a junit test which I think proves that this issues has been resolved".

I really don't know what else to do. I supplied tons of information, yet you do not even dare to try the new version yourself on this issue. :(

Sorry for ranting, but I do not have the feeling that you have any interest in working on this issue because you haven't even bothered to issue two commands (which are: git pull && mvn compile).

wmdietl commented 4 years ago

I know that the eisop version works, because I tested the test case for this issue there: https://github.com/eisop/checker-framework/pull/23

bmarwell commented 4 years ago

@wmdietl have you tried Java 8 compiler?

wmdietl commented 4 years ago

Starting at version 3.0 the Checker Framework builds on a Java 9 javac. So there is no way in testing on a Java 8 compiler. The error you are getting [ERROR] Exception: java.lang.NoSuchMethodError: com/sun/tools/javac/code/Type.stripMetadata()Lcom/sun/tools/javac/code/Type; (loaded from file:$HOME/.jabba/jdk/adopt-openj9@1.8.212-04+openjfx8/lib/tools.jar is because your setup is using a Java 8 compiler.

If you want to stay on a Java 8 JVM, you can simply use the error-prone javac as a dependency: https://github.com/eisop/checker-framework/blob/master/docs/examples/MavenExample-framework-all/pom.xml#L116

bmarwell commented 4 years ago

If you want to stay on a Java 8 JVM, you can simply use the error-prone javac as a dependency: https://github.com/eisop/checker-framework/blob/master/docs/examples/MavenExample-framework-all/pom.xml#L116

This really should be at the very top of the documentation, with a better example and the correct errorprone-javac version.

wmdietl commented 4 years ago

This really should be at the very top of the documentation, with a better example and the correct errorprone-javac version.

At the top of what documentation? We will highlight this in the transition notes, but otherwise why would this be at the very top?

What's wrong with that errorprone-javac version?

bmarwell commented 4 years ago

The link you provided uses <javac.version>9.nnn</javac.version>.

At the top of what documentation? https://checkerframework.org/

We will highlight this in the transition notes That's really not what I meant.

To clarifiy: For Java 8 users (there are a LOT, because enterprise support lasts until 2022 or longer), they need to stick to checker 2.x for a while.

What the documentation should mention

That said, the documentation should mention:

If you are using Java 8 (and therefore checker framework 2.9.0 (put most recent version here), please not that the JDK included javac binary has a known issue. To address this, please use the errorprone-javac (maven-artifact: com.google.errorprone:javac:1.8.0-u20) to compile your code when also using checkstyle. The complete pom.xml profile should look like this:

  <properties>
    <errorprone.javac.version>1.8.0-u20</errorprone.javac.version>
    <dependency.checker.version>2.9.0</dependency.checker.version>
  </properties>
  <!-- …  -->

  <profiles>
    <profile>
      <id>checker</id>
      <properties>
        <checker.version>${dependency.checker.version}</checker.version>
      </properties>
      <dependencies>
        <dependency>
          <groupId>org.checkerframework</groupId>
          <artifactId>checker</artifactId>
          <version>${dependency.checker.version}</version>
        </dependency>
        <dependency>
          <groupId>org.checkerframework</groupId>
          <artifactId>jdk8</artifactId>
          <version>${dependency.checker.version}</version>
        </dependency>

        <!-- https://mvnrepository.com/artifact/com.google.errorprone/javac -->
        <!-- needed for checkerframework fix -->
        <dependency>
          <groupId>com.google.errorprone</groupId>
          <artifactId>javac</artifactId>
          <version>${errorprone.javac.version}</version>
        </dependency>
      </dependencies>
      <build>
        <plugins>
          <plugin>
            <artifactId>maven-compiler-plugin</artifactId>
            <configuration>
              <source>1.8</source>
              <target>1.8</target>
              <maxmem>1024m</maxmem>
              <annotationProcessors>
                <annotationProcessor>
                  org.immutables.value.internal.$processor$.$Processor
                </annotationProcessor>
                <annotationProcessor>
                  org.checkerframework.checker.nullness.NullnessChecker
                </annotationProcessor>
                <annotationProcessor>
                  org.checkerframework.checker.tainting.TaintingChecker
                </annotationProcessor>
              </annotationProcessors>
              <compilerArgs>
                <!--suppress UnresolvedMavenProperty -->
                <arg>-Xbootclasspath/p:${org.checkerframework:jdk8:jar}</arg>
                <arg>
                  -Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${errorprone.javac.version}/javac-${errorprone.javac.version}.jar
                </arg>
                <!-- checker.stubs.dir needs to be absolute path since relative path will be
                  interpreted from each module directory -->
                <!--suppress UnresolvedMavenProperty -->
                <arg>-Astubs=${checker.stubs.dir}</arg>
                <arg>-AstubWarnIfNotFound</arg>
                <arg>-AskipDefs=.*Test$</arg>
              </compilerArgs>
            </configuration>
          </plugin>
        </plugins>
      </build>
   </profile>
  </profiles>

Really, this is not trivial.

I sincerely hope you do agree that this setup should be mentioned prominently in the documentation.

bmarwell commented 4 years ago

Sigh, this does not seem to work either.

Trying with:

  <arg>-Xbootclasspath/p:${org.checkerframework:jdk8:jar}</arg>
  <arg>
                  -Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${errorprone.javac.version}/javac-${errorprone.javac.version}.jar
                </arg>

Checker yields I'm not using their JDK8.

I think they must go in the same argument:

  <arg>-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${errorprone.javac.version}/javac-${errorprone.javac.version}.jar,${org.checkerframework:jdk8:jar}</arg>

… but I still get this message:

You do not seem to be using the distributed annotated JDK. To fix the problem, supply javac an argument like: -Xbootclasspath/p:.../checker/dist/ . Currently using: jdk8.jar

Can you please help, @wmdietl ?

bmarwell commented 4 years ago

Ok, just tried the errorprone-javac 9 to make sure it's not my fault. Also tried combining it with multiple -Xbootclasspath/p: which seems the correct way to do it.

The following example will lead to:

You do not seem to be using the distributed annotated JDK. To fix the problem, supply javac an argument like: -Xbootclasspath/p:.../checker/dist/ . Currently using: jdk8.jar

                <arg>-Xbootclasspath/p:${org.checkerframework:jdk8:jar}</arg>
                <arg>-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${errorprone.javac.version}/javac-${errorprone.javac.version}.jar</arg>

The following example will lead to checker not complaining, but the very same compilation error:

ElementAnnotationUtil.getLocationTypeADT: invalid location [WILDCARD] for type: String

                <arg>-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${errorprone.javac.version}/javac-${errorprone.javac.version}.jar</arg>
                <arg>-Xbootclasspath/p:${org.checkerframework:jdk8:jar}</arg>

That said, checker framework seems broken for Java 8. I get that it is the fault of the broken java 8 compiler, but nontheless, you are just posting ideas what may work, and sadly they don't seem to work for me (and others).

If you want to stay on a Java 8 JVM, you can simply use the error-prone javac as a dependency

Not working, unless the example you showed me is doing something in the background I am not aware of :(

Or maybe the warning "You do not seem to be using the distributed annotated JDK" is wrong with the first setup?

the1derer commented 4 years ago

@bmhm Some point to note:

For Java 8 users (there are a LOT because enterprise support lasts until 2022 or longer), they need to stick to checker 2.x for a while.

Checker Framework version 3.x will continue supporting Java 8. If you continue to use Java 8, only change you have to make when switching to version 3.x is error-prone's javac for compilation. When using Checker Framework 2.x you don't need to use Error-Prone' javac. That is why in the example we created a separate profile which is activated automatically when you compile using Java 8

Also, @wmdietl suggested you try Checker Framework 3.0.0-b2(beta version) for which <groupId>io.github.eisop</groupId> not org.checkerframework but I already tried it with your example i.e. https://github.com/zchunk/zchunk-java/ with Java 8 & I can confirm that problem still exists.

This really should be at the very top of the documentation, with a better example and the correct error-prone-javac version.

This is not mentioned in documentation because Checker Framework 3.x is still in beta and will be in transition notes as pointed out by @wmdietl. Also, there is no problem with the version of error-prone javac as the example is compiling successfully using Java 8 and above.

Not working, unless the example you showed me is doing something in the background I am not aware of :(

Or maybe the warning "You do not seem to be using the distributed annotated JDK" is wrong with the first setup?

The example referred by @wmdietl is compiling successfully using JDK 8 and above.

The warning "You do not seem to be using the distributed annotated JDK" is issued if you don't use <arg>-Xbootclasspath/p:${org.checkerframework:jdk8:jar}</arg>.

Since #2173 error is solved in Java 9 as shown in eisop/checker-framework#23 it will be solved when Checker Framework 3.x is ready for release and you use Java 9+. The problem right now is we don't have an annotated-JDK for Java 9+ which is needed for some of the checkers of Checker Framework(eg. nullness checker which you are using). I am currently working on this and hopefully, it will be ready by the end of August.

bmarwell commented 4 years ago

@the1derer thanks for the clarification.

If I understand correctly:

Thanks for working on this. Sadly I am unable to use checker for about a year now unless I put random classes on the exception list. Sometimes if I change class A, I can remove class B on the exception list but have to add another class, which previously worked.

Sorry for adding a lot of comments on this issue, but I really cannot believe that I am the only user with this issue. In fact, I saw this issue on multiple projects I contribute to.

the1derer commented 4 years ago

Although @wmdietl said he has a test case and therefore I must be doing something wrong, you confirmed that this bug does exist in both checker 2.9.0 and 3.0.0-b2 (which is what I also said).

The bug exists in 2.9.0 ( this version only supports Java 8) but using version 3.0.0-b2 with Java 9+ bug is not present.

I am using Java 8 JDK as base, therefore I do not need to fiddle with errorprone at all.

No, you don't need to fiddle with errorprone. But you do need to make changes if/when you decide to use Checker Framework 3.x

bmarwell commented 4 years ago

When you say this bug is not present, can you help me to set checker up in a java 8 project?

I am still confused when to use errorprone.

I am using JDK 8 and want to switch to checker 3.0.0-b2. I understand that in this case I need to add errorprone. I did this as seen in your example, but I still have the following dependency added: io.github.eisop:jdk8:jar as -Xbootclasspath/p:, which is missing in your example. Or is it not needed anymore?

Anyway, when I pull in errorprone for maven-compiler-plugin, add the -J-Xbootclasspath/p parameter for errorprone, this error still occurs:

error: ElementAnnotationUtil.getLocationTypeADT: invalid location [WILDCARD] for type: String

Config posted a few time above.

The bug exists in 2.9.0 ( this version only supports Java 8) but using version 3.0.0-b2 with Java 9+ bug is not present.

Which bug in which configuration?

I tried both java 8 with checker 2.9.0 and Java 8 with errorprone and checker 3.0.0 and have this bug.


But let me ask you a question. This issue was opened when the 2.5-series was active, the 2.5.5 artifact of checker-qual was just released before this issue was opened.

I wonder: do you plan to make a backport of this fix? 3.0.0 must have breaking changes according to the version. 4 minor versions and a whole year have passed.

bmarwell commented 4 years ago

@wmdietl @the1derer Thanks for your updates.

I updated zchunk-java according to your example.

Please check out: https://github.com/zchunk/zchunk-java/tree/0bcc4320632d408b4493ef6a26927479a2e92ab1 and run ./mvnw clean install -DskipTests=true -Pchecker

It will run into the same issue.

smillst commented 4 years ago

We decided that since malformed Java 8 bytecode might be on the classpath of a checker, the Checker Framework must ignore those annotations rather than crashing. This change will be in today's release, 2.10.0.

bmarwell commented 4 years ago

Thank you sooooooo much!! Great! 😃

mernst commented 4 years ago

We are happy to help. Thanks for reporting the issue and trying fixes; we appreciate it.

Please let us know of any other problems you encounter. We will help as best we can, given that we are not paid to maintain the project.

bmarwell commented 4 years ago

@mernst I want to say sorry for all the fuzz in this thread to the whole team. The discussion was very confusing, and we often didn't understand correctly what the others were writing.

We talked about different combinations of javac versions, checker versions, with or without errorprone, etc. which were really confusing at times.

Turns out everything came out well for everyone I think -- which is a great achievement for a great library/tool! :-) Thanks again!

… and yes, 2.10.0 works nicely so far!

mernst commented 4 years ago

@bmhm Figuring out build problems is my least favorite part of the project. I'm glad it worked out well in the end.