Figure out and document whether module-level @NullMarked helps users who don't declare a module

[cpovirk]

@xenoterracide asks: Suppose that I put @NullMarked in my library's module-info. Then:

If the consumers of the module aren't using JPMS is it still supposed to be respected by tools?

@msridhar is optimistic that tools will still recognize it.

I am more pessimistic. I worry on two levels:

• First, the obvious way to implement the search for @NullMarked is (in javac terms) to keep calling getEnclosingElement() until there are no enclosing elements left. I would assume that pkg.getEnclosingElement() will not return the module for a package unless the package actually comes from the module path. (I have not verified this.) If so, then tools would need to go out of their way to look for the module-info for the package. (And if not all the tools you use do this (e.g., Kotlin, IntelliJ, a build-time Java checker), then you'll get inconsistent behavior.)
• I fear that "the module-info for the package" is technically not a well-defined question unless Java "loaded the module for real." In practice, you could probably look up the jar/directory that contains a given package (or a specific class from that package, since multiple jars/directories might contain classes from the same package!). Then you could look in that jar for module-info.class (possibly under META-INF/versions/9 or similar) and read the file with ASM. But I wouldn't be shocked if that weren't technically enough. Might the jar come from an in-memory filesystem that you might not have access to? Even if it comes from the filesystem, might it be in some kind of special bundle that you need to understand? And even if it comes from a plain old jar, what if the system is doing some wild bytecode rewriting during class loading, which should have gotten applied to the module-info but never did because Java didn't load the module-info? Does anything change if we're talking about the annotation-processor module path? I suspect that this is all not a big practical concern, but it's at least going to be a reason that tool authors are not enthusiastic about putting in any special handling for the non-JPMS case: It feels like another case in which people may want to avoid "swimming upstream."

Once we know more about this, we should write something for tool authors and something for users (possibly even in the Javadoc). It's very possible that the advice for users will end up being: "You don't actually want to rely on module-level @NullMarked for libraries that you publicly release; it's best reserved for closed systems, like if your company has fully bought in to modules."

[SentryMan]

• I would assume that pkg.getEnclosingElement() will not return the module for a package unless the package actually comes from the module path.

at least on Java 11+ annotation processors, pkg.getEnclosingElement() will return null only if the package has no module. Doesn't matter if running on module-path or not in my experience.

[cpovirk]

Hmm, thanks. Do you have any code that you can share to show that behavior? I finally gave it a shot, myself, and I see "unnamed module" when the class comes from the classpath.

Code:

==> ./plugin/p/P.java <==
package p;

import com.sun.source.util.JavacTask;
import com.sun.source.util.Plugin;
import com.sun.source.util.TaskEvent;
import com.sun.source.util.TaskListener;
import javax.lang.model.element.Element;

public class P implements Plugin {
  @Override
  public String getName() {
    return "P";
  }

  @Override
  public void init(JavacTask javacTask, String... strings) {
    javacTask.addTaskListener(
        new TaskListener() {
          @Override
          public void finished(TaskEvent event) {
            if (event.getKind() != TaskEvent.Kind.ENTER) {
              return;
            }
            for (Element e =
                    javacTask
                        .getElements()
                        .getTypeElement("com.google.errorprone.annotations.Keep");
                e != null;
                e = e.getEnclosingElement()) {
              System.err.println(e);
            }
            if (true) throw new RuntimeException();
          }
        });
  }
}

==> ./plugin/module-info.java <==
module p {
  requires transitive jdk.compiler;
  provides com.sun.source.util.Plugin with p.P;
}

==> ./user/u/User.java <==
package u;

class User {}

==> ./user/module-info.java <==
module u {
  requires com.google.errorprone.annotations;
}

Setup:

$ javac $(find plugin -name '*.java')

Module path:

$ javac --processor-module-path plugin -Xplugin:P --module-path $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/2.26.1/error_prone_annotations-2.26.1.jar $(find user -name '*.java')
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
com.google.errorprone.annotations
An exception has occurred in the compiler (11.0.20.1). Please file a bug: go/javac-bug

Classpath:

$ javac --processor-module-path plugin -Xplugin:P -cp $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/2.26.1/error_prone_annotations-2.26.1.jar user/u/User.java 
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
unnamed module
An exception has occurred in the compiler (11.0.20.1). Please file a bug: go/javac-bug

[agentgt]

@cpovirk

I'm wagering why @SentryMan and I see the module is because our stuff (we both write APT plugins) are running as an annotation processor and not a compiler plugin.

Whatever the case build systems barely support --processor-module-path. It was just added IIRC to Maven and Gradle like in the last year or so. That is most are not running errorprone that way or other annotation processors.

So that might be the issue as well.

I don't have time today to make an experimental APT project but @SentryMan might be able to. If not I will later this week.

[agentgt]

@cpovirk Can you try putting the plugin on the classpath and or regular --processor-path and then put the code on the module-path or classpath?

Basically we have a nice 2x2 grid of combinations that need to be tried: --processor-module-path vs --processor-path and for the code being compiled: -cp and --module-path. We are already half way there based on your experiments.

I think but need to check but I believe if you use --processor-module-path with Maven it will force everything on the --module-path (or maybe that was eclipse).

[cpovirk]

Somehow this makes me feel very ChatGPT: Certainly! Here are the results for putting the plugin on the classpath or regular --processor-path, using a regular annotation processor instead of a compiler plugin.

(tl;dr I'm still seeing "unnamed module.")

==> plugin/p/AP.java <==
package p;

import java.util.Collections;
import java.util.Set;
import javax.annotation.processing.Completion;
import javax.annotation.processing.ProcessingEnvironment;
import javax.annotation.processing.Processor;
import javax.annotation.processing.RoundEnvironment;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.tools.Diagnostic.Kind;

public class AP implements Processor {
  private ProcessingEnvironment processingEnvironment;

  @Override
  public void init(ProcessingEnvironment environment) {
    processingEnvironment = environment;
  }

  @Override
  public Set<String> getSupportedAnnotationTypes() {
    return Collections.singleton("com.google.errorprone.annotations.Keep");
  }

  @Override
  public Set<String> getSupportedOptions() {
    return Collections.emptySet();
  }

  @Override
  public SourceVersion getSupportedSourceVersion() {
    return SourceVersion.latest();
  }

  @Override
  public Iterable<Completion> getCompletions(
      Element element, AnnotationMirror annotation, ExecutableElement member, String userText) {
    return Collections.emptyList();
  }

  @Override
  public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment env) {
    for (Element e =
            processingEnvironment
                .getElementUtils()
                .getTypeElement("com.google.errorprone.annotations.Keep");
        e != null;
        e = e.getEnclosingElement()) {
      System.err.println(e);
    }
    processingEnvironment.getMessager().printMessage(Kind.ERROR, "failing");
    return false;
  }
}

==> user/u/User.java <==
package u;

@com.google.errorprone.annotations.Keep
class User {}

Module path:

$ javac -processorpath plugin -processor p.AP --module-path $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/2.26.1/error_prone_annotations-2.26.1.jar $(find user -name '*.java')
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
com.google.errorprone.annotations
error: failing
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
com.google.errorprone.annotations
error: failing
2 errors

Classpath:

$ javac -processorpath plugin -processor p.AP -cp $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/2.26.1/error_prone_annotations-2.26.1.jar user/u/User.java 
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
unnamed module
error: failing
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
unnamed module
error: failing
2 errors

Classpath with the annotation processor also on the classpath. (I think my brain had blocked out the knowledge that that was possible :))

$ javac -processor p.AP -cp plugin:$HOME/.m2/repository/com/google/errorprone/error_prone_annotations/2.26.1/error_prone_annotations-2.26.1.jar user/u/User.java 
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
unnamed module
error: failing
com.google.errorprone.annotations.Keep
com.google.errorprone.annotations
unnamed module
error: failing
2 errors

[agentgt]

FWIW @cpovirk you seem more effective than Chat GPT or at least the free version 😄

Thanks for doing that! I will have to figure out someway to make it up as I do have projects that have modules with annotation processors running on classpath that definitely access module-info so I will have to look how they work! Its actually one of the core configuration mechanisms of JStachio as it does a similar crawl up the enclosing stuff as JSpecify.

[agentgt]

It just occurred to me that I don't actually check if it is a named module. I just check to see if an annotation is on the module and it does seem to find that.

I know that works because our Spring Example app has an important annotation: https://github.com/jstachio/jstachio/blob/ab86b11599c62589fe4daa21bad90d41ce57f1f6/opt/jstachio-spring-example/src/main/java/module-info.java#L29

And the annotation processor is on the classpath: https://github.com/jstachio/jstachio/blob/ab86b11599c62589fe4daa21bad90d41ce57f1f6/opt/jstachio-spring-example/pom.xml#L18

In maven if you just have an APT as a dependency pre Java 21 aka before -proc:full it will just run it without needing -processorpath....... as in -processor is not specified... oh no we have another combination to test.

Anyway tonight I promise I will get on this to figure out what is going on at least on my end.

[cpovirk]

Ah, that's interesting. Maybe an annotation on one module can affect the entire unnamed module? (Spoiler: maybe still no? But there are other scenarios that also seem worth testing.)

To test some more, I hacked up error_prone_annotations:

diff --git a/annotations/src/main/java/com/google/errorprone/annotations/Keep.java b/annotations/src/main/java/com/google/errorprone/annotations/Keep.java
index 8288b63d9d..d1e0430faa 100644
--- a/annotations/src/main/java/com/google/errorprone/annotations/Keep.java
+++ b/annotations/src/main/java/com/google/errorprone/annotations/Keep.java
@@ -19,6 +19,7 @@ import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
 import static java.lang.annotation.ElementType.CONSTRUCTOR;
 import static java.lang.annotation.ElementType.FIELD;
 import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.MODULE;
 import static java.lang.annotation.ElementType.TYPE;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
@@ -37,6 +38,6 @@ import java.lang.annotation.Target;
  * should also be kept.
  */
 @Documented
-@Target({ANNOTATION_TYPE, CONSTRUCTOR, FIELD, METHOD, TYPE})
+@Target({ANNOTATION_TYPE, CONSTRUCTOR, FIELD, METHOD, MODULE, TYPE})
 @Retention(RUNTIME)
 public @interface Keep {}
diff --git a/annotations/src/main/java/module-info.java b/annotations/src/main/java/module-info.java
index 3dccd22910..65a0928225 100644
--- a/annotations/src/main/java/module-info.java
+++ b/annotations/src/main/java/module-info.java
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+@com.google.errorprone.annotations.Keep
 open module com.google.errorprone.annotations {
   requires java.compiler;
 

And it looks like I can see the @Keep annotation on the module com.google.errorprone.annotations (which seems analogous to "a @NullMarked annotation on the module for Guava or something someday") if and only if I'm doing "a modular build":

--processor-module-path, -Xplugin, --module-path:

$ javac --processor-module-path plugin -Xplugin:P --module-path $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/1.0-HEAD-SNAPSHOT/error_prone_annotations-1.0-HEAD-SNAPSHOT.jar $(find user -name '*.java')

com.google.errorprone.annotations.Keep: @java.lang.annotation.Documented,@java.lang.annotation.Target({java.lang.annotation.ElementType.ANNOTATION_TYPE, java.lang.annotation.ElementType.CONSTRUCTOR, java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.MODULE, java.lang.annotation.ElementType.TYPE}),@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
com.google.errorprone.annotations: 
com.google.errorprone.annotations: @com.google.errorprone.annotations.Keep

--processor-module-path, -Xplugin, -cp:

$ javac --processor-module-path plugin -Xplugin:P -cp $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/1.0-HEAD-SNAPSHOT/error_prone_annotations-1.0-HEAD-SNAPSHOT.jar user/u/User.java

com.google.errorprone.annotations.Keep: @java.lang.annotation.Documented,@java.lang.annotation.Target({java.lang.annotation.ElementType.ANNOTATION_TYPE, java.lang.annotation.ElementType.CONSTRUCTOR, java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.MODULE, java.lang.annotation.ElementType.TYPE}),@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
com.google.errorprone.annotations: 
unnamed module: 

-processorpath, -processor, --module-path:

$ javac -processorpath plugin -processor p.AP --module-path $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/1.0-HEAD-SNAPSHOT/error_prone_annotations-1.0-HEAD-SNAPSHOT.jar $(find user -name '*.java')

com.google.errorprone.annotations.Keep: @java.lang.annotation.Documented,@java.lang.annotation.Target({java.lang.annotation.ElementType.ANNOTATION_TYPE, java.lang.annotation.ElementType.CONSTRUCTOR, java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.MODULE, java.lang.annotation.ElementType.TYPE}),@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
com.google.errorprone.annotations: 
com.google.errorprone.annotations: @com.google.errorprone.annotations.Keep

-processorpath, -processor, -cp:

$ javac -processorpath plugin -processor p.AP -cp $HOME/.m2/repository/com/google/errorprone/error_prone_annotations/1.0-HEAD-SNAPSHOT/error_prone_annotations-1.0-HEAD-SNAPSHOT.jar user/u/User.java

com.google.errorprone.annotations.Keep: @java.lang.annotation.Documented,@java.lang.annotation.Target({java.lang.annotation.ElementType.ANNOTATION_TYPE, java.lang.annotation.ElementType.CONSTRUCTOR, java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.MODULE, java.lang.annotation.ElementType.TYPE}),@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
com.google.errorprone.annotations: 
unnamed module: 

-processor, -cp:

$ javac -processor p.AP -cp plugin:$HOME/.m2/repository/com/google/errorprone/error_prone_annotations/1.0-HEAD-SNAPSHOT/error_prone_annotations-1.0-HEAD-SNAPSHOT.jar user/u/User.java

com.google.errorprone.annotations.Keep: @java.lang.annotation.Documented,@java.lang.annotation.Target({java.lang.annotation.ElementType.ANNOTATION_TYPE, java.lang.annotation.ElementType.CONSTRUCTOR, java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.MODULE, java.lang.annotation.ElementType.TYPE}),@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
com.google.errorprone.annotations: 
unnamed module: 

Maybe what you're seeing with JStachio is that you can pick up annotations from the module you're compiling even if your dependencies are in the unnamed module? That much I would have hoped for, and I should make sure that any docs that we write can't be read as warnings about that case.

[agentgt]

Maybe what you're seeing with JStachio is that you can pick up annotations from the module you're compiling even if your dependencies are in the unnamed module? That much I would have hoped for, and I should make sure that any docs that we write can't be read as warnings about that case.

Yes I believe that is the case but I kind of thought you were testing that but now re-reading the title of this post I think I was confused. You are concerned non-modular jars/setup/tools cannot sniff the @NullMarked from the module and thus cannot figure out if the library is JSpecified?

Anway I finally got to a computer with some level of a dev environment and did -X maven on the Spring project.

And it appears (and I forgot that Maven does this through some magic) that it will add all the libraries that you requires in your module-info as --module-path. All other libraries that are defined as dependencies but not in module-info are put on the classpath.

So in the Spring JStachio example the APT module is actually on the classpath and not the --module-path where it still gets picked up by the compiler service loader call.

I'm remiss that I forgot that as I even documented that: https://jstach.io/doc/jstachio/current/apidocs/#maven_classpath

My apologies on forgetting that. I must admit it is hard to remember what does and doesn't work with modules.

[agentgt]

I think where @SentryMan and myself got confused is the tool in this case the annotation processor does not need to run on the module-path but the code being analyzed does.

Otherwise you need to do what Maven/Gradle does which I assume ASM based (your second bullet point at the top of this post). I wonder if the java.lang.module.ModuleReader can read modules without them being on module path? (edit I got ModuleReader mixed up with ModuleReference but either case that does not work) I assume not but

Per the doc:

A module reader is intended for cases where access to the resources in a module is required, regardless of whether the module has been loaded. A framework that scans a collection of packaged modules on the file system, for example, may use a module reader to access a specific resource in each module. A module reader is also intended to be used by ClassLoader implementations that load classes and resources from modules.

[cpovirk]

You are concerned non-modular jars/setup/tools cannot sniff the @NullMarked from the module and thus cannot figure out if the library is JSpecified?

Yep, that's it. I'll take this as another vote for being very, very clear about exactly what circumstances we're warning people about :)

I must admit it is hard to remember what does and doesn't work with modules.

My solution to that problem has been to never learn in the first place :) But that's not serving me well at the moment.... (TIL ModuleReader and ModuleReference, plus the Maven --module-path magic.)

Your point about Maven's ASM magic also shows me that my bytecode-rewriting concerns are misplaced: The time when most users care about nullness annotation is compile-time, and there's no bytecode-rewriting magic going on then: If the jar has a module-info that refers to com.foo.bar, then com.foo.bar is what the compiler will look for. This is in contrast to runtime, when it's at least theoretically possible (IIUC) for a ClassLoader to invent modules or at least classes out of thin air, perhaps in a way that is difficult for users to intercept to hand off to ASM.

On the other hand, I know less about how compile-time plugins (meaning mainly annotation processors) might look for a corresponding module-info.class for a given class/package. Maybe it can be done with JavaFileManager.list or something? But I'd rather just not go down that road.

[cpovirk]

(Oops, I guess someone had taught us this long ago, but I'd forgotten: #34 (comment).)

[agentgt]

@cpovirk I don't blame you. I was relooking at my old comments regarding this that I completely forgot as well 😆 . Damn modules be complicated.