Skip to content

Conversation

jeremylong
Copy link
Collaborator

@jeremylong
Copy link
Collaborator Author

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

@dwalluck
Copy link
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
Contributor

@jeremylong,

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

@jeremylong
Copy link
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
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
Copy link
Contributor

ppkarwasz commented Mar 16, 2025

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
Contributor

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

@jeremylong
Copy link
Collaborator Author

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

@jeremylong jeremylong merged commit ab7176f into master Mar 19, 2025
4 checks passed
@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