search

typetools / checker-framework

Pluggable type-checking for Java
http://checkerframework.org/
Other
1k stars 350 forks source link

Is there a way to add aliases for third-party annotations? #3625

Open vlsi opened 3 years ago

vlsi commented 3 years ago

I came across custom annotations: https://github.com/janino-compiler/janino/tree/9ef0cfa9057167739211912da564054a7f3083d7/commons-compiler/src/main/java/org/codehaus/commons/nullanalysis

Just in case, janino-compiler targets Java 1.6, so it looks like they can't use type annotations yet.

Is there a way to make checker-framework understand janino-compiler nullability annotations?

I guess janino-compiler project wants to have zero-dependencies (which makes sense as the compiler is often embedded in weird class generation use-cases). Is there something like astub for annotations in checker-framework? Is there a way to write zero-dependency code yet make it compatible with checker-framework analysis?

mernst commented 3 years ago

Is there a way to make checker-framework understand janino-compiler nullability annotations?

Yes, there is a way, but it requires changing the Checker Framework source code. There are so few other aliases, and new ones are created so infrequently, that we didn't think it worthwhile to create a command-line mechanism. Pull request #3626 adds support for the Janino annotations.

vlsi commented 3 years ago

I've updated to checkerframework 3.7.0 (thanks for the prompt release!), however, the error is still there :(

calcite/core/src/main/java/org/apache/calcite/rel/metadata/JaninoRelMetadataProvider.java:434: error: [argument.type.incompatible] incompatible argument for parameter arg0 of setParentClassLoader.
    compiler.setParentClassLoader(JaninoRexCompiler.class.getClassLoader());
                                                                        ^
  found   : @Initialized @Nullable ClassLoader
  required: @Initialized @NonNull ClassLoader

Do you think it can be caused by RetentionPolicy.CLASS? (see https://github.com/janino-compiler/janino/blob/1fde5675de9f0be777e3ec2558a2534ba50112db/commons-compiler/src/main/java/org/codehaus/commons/nullanalysis/Nullable.java#L42 )

mernst commented 3 years ago

I apologize that the fix didn't work for you.

We should have written a test case. :-/ Could you provide a minimal test case that we could use for testing?

The use of RetentionPolicy.CLASS should not be a problem. (RetentionPolicy.SOURCE would have been a problem.) Maybe the problem is the fact that the annotation is a declaration annotation rather than a type annotation (its @Target meta-annotation, lacks the TYPE_USE value); but that ought to work just as the support for org.checkerframework.checker.nullness.compatqual.NonNullDecl does. So, the reason for the problem is not immediately apparent to me.

mernst commented 3 years ago

@vlsi Can you provide a minimal test case? Thanks!

vlsi commented 3 years ago

Ah, frankly speaking, I realize I do not know how to add tests. No kidding. I can add checkerframework to a third-party library, however, I don't know what is the better/preferred way to add test to checker-framework itself.

I assume you are not looking for a standalone Gradle project, but you might be looking for a testcase in the checker-framework.

Your question triggered the similar condition I have for pgjdbc. Users constantly provide public-static-void-main examples which are not easily reusable for integration or even debugging (e.g. https://github.com/pgjdbc/pgjdbc/issues/1935 ) I think I would try adding how to create a test-case section/sample for pgjdbc.

Back to your question. It might help if you provide a sample of what you are asking for.

I can't build checker-framework locally for some reason. The error is like 

$ gradlew allTests

> Task :checker:commandLineTests
make -C issue618
/Users/vladimirsitnikov/Documents/work/checker-framework/checker/bin/javac -processor regex,org.checkerframework.checker.tainting.TaintingChecker TwoCheckers.java > out.txt 2>&1 || true
diff -u expected.txt out.txt

> Task :checker:jtregJdk11Tests FAILED
Error: cannot determine version for JDK: /Library/Java/JavaVirtualMachines/liberica-jdk-11.jdk/Contents/Home

FAILURE: Build failed with an exception.

* Where:
Build file '/Users/vladimirsitnikov/Documents/work/checker-framework/checker/build.gradle' line: 227

* What went wrong:
Execution failed for task ':checker:jtregJdk11Tests'.
> Process 'command '/Users/vladimirsitnikov/Documents/work/jtreg/bin/jtreg'' finished with non-zero exit value 5
mernst commented 3 years ago

I'm sorry it was not clear how to provide a test case.

Please provide a set of commands that can be cut-and-pasted into a command shell to reproduce the problem.

I have also opened pull request #3813 to slightly streamline the bug reporting instructions. Comments on that are welcome.

mernst commented 3 years ago

Regarding the "Error: cannot determine version for JDK" message, I have not seen that before, and a web search turned up little information.

Maybe the problem is related to one of these: http://openjdk.5641.n7.nabble.com/RFR-XS-8251316-Sanity-check-the-JDK-under-test-before-running-tests-td418827.html https://github.com/AdoptOpenJDK/openjdk-tests/issues/1525

Or, maybe the Liberica JDK differs from OpenJDK in some subtle respect that confuses jtreg. Could you try with a different JDK?

For reference, jtreg issues the error in method com.sun.javatest.regtest.tool.Tool#checkJDK.

mernst commented 3 years ago

@vlsi If you can do these (independent) things, that would be very useful:

Thanks! (No huge rush, I just didn't want this to fall through the cracks.)