Skip to content

fix: make jspecify a compile time/optional dependency#204

Merged
jeremylong merged 2 commits into
masterfrom
scratch/jspecify-compile-time
Mar 19, 2025
Merged

fix: make jspecify a compile time/optional dependency#204
jeremylong merged 2 commits into
masterfrom
scratch/jspecify-compile-time

Conversation

@jeremylong

Copy link
Copy Markdown
Collaborator

@jeremylong

Copy link
Copy Markdown
Collaborator Author

@ppkarwasz can you please take a look at this PR? Am I correct?

@dwalluck

Copy link
Copy Markdown
Contributor

None of these examples have <scope>provided</scope>, but I think we want it (just match the existing jakarta-validation exactly).

@ppkarwasz

Copy link
Copy Markdown
Contributor

@jeremylong,

Whether to put JSpecify as compile or provided dependency is mostly a judgment call:

@jeremylong

Copy link
Copy Markdown
Collaborator Author

I think my PR title might have confused things a bit. I didn't mean to raise the concern of the <scope> - but rather the fact that many annotation libraries can be marked as <optional>true</optional>. I just installed this snapshot version locally, upgraded my local copy of DependencyCheck, and ran mvn clean verify on DC. There were no compilation or test errors.

@dwalluck

Copy link
Copy Markdown
Contributor

I am in favor of this change, but it is related to scope.

https://stackoverflow.com/questions/40393098/when-to-use-optional-dependencies-and-when-to-use-provided-scope

@ppkarwasz

ppkarwasz commented Mar 16, 2025

Copy link
Copy Markdown
Contributor

I think my PR title might have confused things a bit. I didn't mean to raise the concern of the <scope> - but rather the fact that many annotation libraries can be marked as <optional>true</optional>.

I am looking at this from a purely functional perspective: <scope>provided</scope> and <scope>compile</scope><optional>true</optional> behave exactly in the same way.

I just installed this snapshot version locally, upgraded my local copy of DependencyCheck, and ran mvn clean verify on DC. There were no compilation or test errors.

Interesting! 💯
After some simple tests it seems that a warning is issued if:

  • An annotation is missing at compile time.
  • The annotation has some values.

This is good news, because JSpecify annotations have no values (at least currently)! Go ahead and merge this.

Note: There is a bug in JRE 8 (JDK8152174) that throws an NPE if someone is trying to access a missing type annotation via reflection. So this code will throw a NullPointerException on JRE 8 (but will be OK on more recent JVMs):

PackageURL.class.getDeclaredMethod("getNamespace").getAnnotatedReturnType();

@dwalluck

Copy link
Copy Markdown
Contributor

I think you want provided or it can be pulled in as a dependency in some other ways. I think shading is one.

Comment thread pom.xml
@jeremylong

Copy link
Copy Markdown
Collaborator Author

Unfortunately - I'll need to get @stevespringett to approve this one.

@jeremylong jeremylong merged commit ab7176f into master Mar 19, 2025
@jeremylong jeremylong deleted the scratch/jspecify-compile-time branch March 19, 2025 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants