JPMS

[xenoterracide]

Is your feature request related to a problem? Please describe.

java module (module-info) should agree with the gradle dependencies, for example compileOnlyApi should generally match requires static transitive

I suspect this is a bidirectional thing though. For example if I have exports com.myorg.foo but do not have exports com.myorg.foo.internal then even if something that would normally be detected as api should not be api.

if possible, opens... to specifications should possibly be determined, I suspect this would require looking at tests though.

I hope this is clearer than mud.

cc: @jjohannes you might be interested in this issue, or have ideas/suggestions

[Vampire]

At least the dependency consistency check would really be helpful, so that implementation => requires api => requires transitive compileOnly => requires static compileOnlyApi => requires static transitive

Not sure about runtimeOnly, but they might need to be absent in module-info.

[xenoterracide]

AFAIK runtime only dependencies should be excluded from module-info.

There's one exception to things being added to module-info that I've found though. java jdk namespaced modules may need to be listed in a module-info even if no direct abi dependency exists. This is because automatic modules can access everything and do not define which modules they require (static or otherwise). I've run into spring boot issues where I needed to add java.sql to my dependencies. I also ran into a spring boot triggered issue where it tried to load mockito even though I wasn't using it at all, just because it was on the classpath, and that triggered a mockito bug where it requires jdk.agent. So essentially any false positives on java/jdk should be ignored.

[autonomousapps]

Thanks for the issue. I'm not entirely clear on what the request is. What does this plugin do now, and what should it do differently? The issue title is also just a noun ("JPMS"), when a sentence with a verb would clarify things for me.

[Vampire]

Currently, the plugin validates dependencies declared in build logic.

In the module-info.class you also declare dependencies, usually the same ones as in the build logic.

It can easily happen that the scopes you use in both places differ.

It would be great if the plugin could also verify the dependencies in the module-info.class and advice on changes to be consistent to what is declared in the build logic. Otherwise one might just use requires for all dependencies which then is like using implementation for all dependencies.

That's what I hope for to get from this issue. Caleb described some further things, I just am not sure how helpful those would be. :-)

[Vampire]

Additionally to external dependencies, it would of course be awesome to get advice which JRE modules are needed in which scopes too.

[xenoterracide]

also this is creating false positives

Advice for :commons-jpa
Unused dependencies which should be removed:
  testImplementation(libs.jspecify)
  whiteboxImplementation(libs.jspecify)

but in module-info, I need it to at least compile the module

import org.jspecify.annotations.NullMarked;

@NullMarked module com.xenoterracide.commons.jpa.test {
  opens com.xenoterracide.commons.jpa.test
    to org.junit.platform.commons, org.hibernate.orm.core, net.bytebuddy, spring.core;
  opens com.xenoterracide.commons.jpa.test.transaction
    to org.junit.platform.commons, org.hibernate.orm.core, net.bytebuddy, spring.core;
  requires org.assertj.core;
  requires com.xenoterracide.commons.jpa;
  requires org.junit.jupiter.api;
  requires org.junit.jupiter.params;
  requires spring.beans;
  requires spring.boot.test;
  requires spring.tx;
  requires org.hibernate.orm.envers;
  requires org.apache.commons.lang3;
  requires com.xenoterracide.jpa.fixtures;
  requires spring.test;
  requires spring.boot.test.autoconfigure;
  requires spring.orm;
  requires org.jspecify;
}

what information is still needed? note: your verb is probably support. Currently JPMS isn't looked at, used, or included as part of the analysis.

[autonomousapps]

Essentially all of my work is in a Kotlin environment, where JPMS is not used, therefore I have limited experience with that. I'm happy to attempt to support it here, but I'm going to need some help to understand the requirements.

One thing that might help me is to describe, in natural language, what the spec is. You could write pseudocode Specifications (to use Spock lingo) that demonstrate various situations that can occur and what you'd expect DAGP's advice to be. Or if you're comfortable with how DAGP's test suite works, you could write actual failing specs. Let me know how I can help.

[xenoterracide]

So, I've been working on my thing and hoping to provide some solid recommendations off the bat. I've found some edge cases on other things. Most 3rd party libraries do not use a module-info instead relying on an automatic module. Either way access is .class.getModule() (AFAIK).

Any of these rules only matter if the build provides a module-info for that specific source set.

Let's start with two rules. There are many, but we'll see if we can get there.

1. If a Jar does not provide a module then it must be a transient dependency, this is actually enforced by javac, so it's not really something you have to look at. Simply put, no automatic or module-info then JPMS will not work. However, these can be transient dependencies.
2. if a dependency is not listed or used in any way in module-info then it must be runtimeOnly.

In this example by default only the modules org.junit.platform.commons, org.junit.jupiter.api and org.assertj.core are allowed to not be runtime only. By default what I've defined here should be an error because I've also requested junit-parameters to be implementation and yet it's not being used. Obviously I've got an exclusion list ;).

module com.xenoterracide.commons.model.test {
  opens com.xenoterracide.commons.model.test to org.junit.platform.commons;
  requires org.junit.jupiter.api;
  requires org.assertj.core;
}

[bundles]
test-impl = ["junit-api", "assertj", "junit-parameters"]
test-runtime = ["junit-engine", "junit-launcher"]

   implementation(platform(libs.junit.bom))
   runtimeOnly(platform(libs.junit.bom))

   implementation.bundle(libs.bundles.test.impl)
   runtimeOnly.bundle(libs.bundles.test.runtime)

javac will not compile against any module not referenced in the module-info. There is an exception to this though. @Vampire do you happen to know if the compiler will ignore the restrictions module-info if it's in java9 instead?

PS. Kotlin and any other JVM based language could be affected by the presence of a module-info should the source set or root of the application contain one. I don't really know how that would manifest though.

[xenoterracide]

A matter of java's opinion

3. If a module is in the to then it must appear in the compiletime classpath, otherwise you get this warning. It's only a warning because it's not a compiletime dependency and you might not even have a direct dependency. This one is weird and people should really only encounter this in tests as their main dependencies should be more direct, if that makes sense. There may be a better way to do this

/home/xeno/IdeaProjects/spring-app-commons/commons-model/src/test/java/module-info.java:5: warning: [module] module not found: org.hibernate.orm.core
    to org.junit.platform.commons, org.hibernate.orm.core, net.bytebuddy, spring.core;

I personally am happy that most of these are transient compiletime claspath for me, although given how this project works now it'd probably be a good idea to suggest that these be implementation. Given that it's only a warning it might also be reasonable to ignore it as it won't be the root cause of a breakage, missing them at runtime might though.

If I remove the opens to hibernate here my runtime fails.

  opens com.xenoterracide.commons.model.test
    to org.junit.platform.commons, org.hibernate.orm.core, net.bytebuddy, spring.core;

[Vampire]

given how this project works now it'd probably be a good idea to suggest that these be implementation.

I don't agree. opens A to B means, that B is allowed to use reflection on A. So if you for example develop a library and have the generic jakarta.persistence annotations on your code, that's fine and there should not be any dependency on org.hibernate.orm.core, no matter which. If a consumer would want to use your library with any jakarta.persistence implementation, they need to make sure to use --add-opens so that their implementation of choice can use reflection on your classes. One of these choices could be org.hibernate.orm.core, but it could be others too. By having the opens A to org.hibernate.orm.core you have kind of built-in support for using your lib with org.hibernate.orm.core as jakarta.persistence implementation, without the consumer needing to use --add-opens. If you would add org.hibernate.orm.core as implementation dependency, then you would force your consumers that want to use a different implementation to exclude this wrong dependency. It could be added as compileOnly dependency to "fix" the compiler warning without employing opinion on your consumers. This would maybe be appropriate, even though you don't really have a compile dependency. But well, then it could verify that the module name you used does not have a typo.

So in this case, I think it would be most appropriate to suggest compileOnly, not implementation.

do you happen to know if the compiler will ignore the restrictions module-info if it's in java9 instead?

I don't know what you mean by "in java9 instead".

[xenoterracide]

I don't know what you mean by "in java9 instead".

you can put a module-info in src/main/java9 instead of java and then it isn't supposed to affect java 8 clients. jspecify does this IIRC.

So if you for example develop a library and have the generic jakarta.persistence annotations on your code, that's fine and there should not be any dependency on org.hibernate.orm.core, no matter which. It could be added as compileOnly dependency to "fix" the compiler warning without employing opinion on your consumers.

So, I'm open to either. I'm simply raising this flag as an edge case. Currently I only notice this behavior in test suites, since the main library has no dependency on hibernate. I think this is also compileOnly + runtimeOnly != implementation ;) right? but most people would think it would. In fact I think it's absolutely reasonable to say that in this case compileOnly + runtimeOnly is the right answer. Honestly, even the runtimeOnly is sketchy. Perhaps a better way to say it is, optionally available at runtime... no errors should be given either way on runtime.

hopefully this is enough information on this for @autonomousapps to make a decision on which way to support this particular edge case.

Are these helping?

[Vampire]

you can put a module-info in src/main/java9 instead of java and then it isn't supposed to affect java 8 clients.

You can put it anywhere you want. The important thing is how you configure your build. src/main/java9 is not something built in. If done typically / properly, there should then be an additional source set defined called java9 that is configured to compile with Java 9, with the Java 8 compilation result added with --patch-module to the compile tasks context that compiles the module info file which is then packaged as multi-release jar.

In such a case other rules might apply. In the normal case where the module info file is part of the main source set as I said before, it should be like implementation => requires api => requires transitive compileOnly => requires static compileOnlyApi => requires static transitive runtimeOnly => absent or requires, not sure (maybe except if the https://github.com/gradlex-org/java-module-dependencies plugin is applied)

I think this is also compileOnly + runtimeOnly != implementation ;) right? but most people would think it would. In fact I think it's absolutely reasonable to say that in this case compileOnly + runtimeOnly is the right answer.

I'd say those "most people" are correct. Adding to compileOnly and runtimeOnly should effectively be the same as adding to implementation. Have a look at https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_configurations_graph for a nice graphic. So no, if implementation is the wrong answer, then compileOnly + runtimeOnly is just as wrong.

Perhaps a better way to say it is, optionally available at runtime... no errors should be given either way on runtime.

Exactly, and that is compileOnly and thus what I suggested above to get rid of the compiler warning and to verify the module-name is correct by making sure there is no warning.

[xenoterracide]

You can put it anywhere you want.

I'm not really familiar with what they're doing or how the build is structured. I also haven’t looked at that graphic in a while.

However, what you're proposing doesn’t address the root of the warning issue. Are you suggesting we simply ignore the warnings? I’m arguing that Gradle doesn’t map cleanly to JPMS in the way you'd hope it does.

Additionally, if your primary publication includes an opens ... to directive for anything that isn't implementation-specific, it might indicate a design problem. It's not the role of this plugin to solve that for you.

When I mentioned compileOnly and runtimeOnly, I was trying to express that while compileOnly is necessary, it’s reasonable to expect runtime without triggering an error. Personally, I’m meticulous about warnings in my Java code—I enable as many as possible and keep a close eye on potential issues.

To be honest, the issue might be solvable by using a service provider, but from what I’ve seen, that only works when the provider has a full module-info file. Right now, most libraries don’t include a complete module-info. Based on a bug I saw in Spring, maybe only 60% of dependencies are even JPMS-compatible.

At the moment, I’m avoiding discussions about requires to keep things simple. My focus is on the runtime behavior and the unusual case of opens ... to .... Previously, I would have expected modules listed there to be runtimeOnly or omitted entirely.

How about we agree that opens ... to ... should require "compile", shouldn’t be part of api (unless there's something else like transitive), and shouldn’t error out if runtime dependencies are also present? In the specific case I’m dealing with, Hibernate isn’t listed as an explicit dependency—it's a transitive runtime dependency.

[Vampire]

However, what you're proposing doesn’t address the root of the warning issue.

Why not? I proposed to suggest compileOnly in that case, that should exactly fix the root of the warning, which is that the library is not part of the compile classpath while you use to ... with it.

Are you suggesting we simply ignore the warnings?

That's one possibility, but with what I suggested (compileOnly) there should not be a warning anymore.

I enable as many as possible and keep a close eye on potential issues.

👌

How about we agree that opens ... to ... should require "compile", shouldn’t be part of api [...], and shouldn’t error out if runtime dependencies are also present?

I thought we did agree on that already.

[xenoterracide]

Okay, so we're on the same page with that.

@autonomousapps are you getting this at all? Or are we completely not making any sense.

[autonomousapps]

Yes, I have seen the conversation but have been preoccupied with other things and unable to respond. I actually met with @jjohannes at the Gradle DPE Summit last week and we discussed briefly. He told me that his JPMS plugin has an integration with DAGP that solves this (from his perspective). Do you use his plugin?

[xenoterracide]

I've tried some his stuff (the testing plugin), I personally don't care for it. It has caused more problems than it's worth. In fact the other day I couldn't figure out why something thought it needed a module. Turns out I had applied that plugin to that project a while back. I then excised the plugin completely. Better to simply add a module-info when you want blackbox testing, which I do by default. If I need whitebox testing I simply add anotherJvmTestSuite without that.

I suspect he was refering to this plugin which basically requires you to completely relearn the entire dependency system. It's too invasive to gradle itself in my mind; and I don't think I'd want to try to "sell" it to any teams.

YMMV on his plugins. The nice thing about yours is it doesn't require me to completely rewrite, relearn or worry about what other plugins it might impact.

[xenoterracide]

Oh, if you're asking if it has useful code to determine this? The answer is probably.

[Vampire]

I didn't think that his plugin "solves" this. Yes, it has some integration with your plugin. But it requires to remove all dependencies and instead only have the module info files. If you just want your scopes checked, but not do that, the plugin does not help at all.

Besides that, it will probably not complain about runtimeOnly dependencies being present in the module info, as it used to require a special commented requires directive for them. And as it is still supported will probably not complain, even though there is an alternative now. Or maybe it shouldn't but complain if it's missing? Still not fully sure about that one.

Also, it only translates the advice from the plugin to requires statements, so things like adding the to ... at least as compileOnly are most probably not found as this plugin will not create an advice for it.