-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specify what tools should do when the classpath is incomplete, or decide not to specify it #295
Comments
One possibility is for JSpecify not to speak to this situation: If someone tries to compile without a complete classpath, then that's a weird enough case that we're not going to get into it. Our job, after all, is to say what annotations mean, not what annotations and a classpath that's incomplete in a specific way mean. [edit: Not to mention that tools might put specific annotations on the classpath themselves, and the set of such annotations may change over time. And not to mention that the Note, though, that this is a common case: As long as there's any frequently used library that asks tools to omit any of its annotations from its deps, tools will encounter that scenario frequently. The library and its omitted annotations don't have to have anything to do with JSpecify or nullness; the point is that, once there's an annotation whose So this is going to come up. We could decide not to speak to it on principle or on the theory that each tool will have to choose the behavior that works best for its own internal error-handling implementation. But we can't claim "It won't come up" or even "It won't come up for users unless those specific users do something unwise." |
If we do choose to specify how to handle this scenario, I see at least 2 broad options, though each has (sigh) sub-options. (As usual, we're definitely not going to specify "Tools must refuse to analyze the code at all.") (Given all the complexities here, I'm inclined to keep dumping information into this thread: Even if we choose not to specify this in JSpecify, the information may be useful to some tool vendor that wishes to consciously choose a behavior—and then test it so as to avoid accidentally changing the behavior and breaking users.) Options to come in two followup posts, even though the result is going to be unreadable in any case.... |
Option A: If a
|
Option B: If a
|
In defense of Option A:
I say "may be" missing, since the tool would probably still act on The tool would do well to warn about the situation itself, as I commented in #294, but aside from that I don't think it should freak out in every individual case. |
#308 reminds me that, regardless of how we decide #308, we need to know something about the annotation that's being implied in order to know if the implication is valid: Either we need to know whether it defines required elements (#240), or we need to know whether it defines any elements at all (#308). This whole discussion probably leads yet again into https://github.com/jspecify/jspecify/wiki/two-kinds-of-analysis |
Tools should by default warn or error on unavailable annotations (I lean towards recommending error for interactive tools and warning for batch tools), and provide an option whether to suppress the error, continue with a warning, or stop. This falls under the principle that JSpecify recommends, but does not prescribe tool behavior. Alternatively, as time goes on I am more and more leaning towards the idea that semantic annotations' definitions should lie in external file(s) that are provided to static analysis tools by means other than the classpath, which would avoid this issue. |
I think we may want to focus on doing even less than "recommending" tool behavior. As Chris said earlier:
I think we might benefit from wielding this principle as a "get out of jail free" card more often. As long as it's well-defined what information is being communicated by the author's choice to use our annotations, we're fulfilling our purpose. NOTE: the JSpecify project can also add value just by being a "forum" between "all" the tool owners and some library owners, where we can discuss recommendations for cases like this. I'm happy for that to happen, possibly even under a label in this issue tracker itself (not that that's my ideal choice). The thing is: it can still do good while being a whole lot easier than our having to work the topic into our specs, tests, user docs, etc. |
When developers declare a dependency on a library of annotations (like JSpecify), they sometimes choose to declare the dependency in a way that tells tools: "When someone depends on my library, don't include that annotation library on that user's classpath, either at build time or at runtime." I think this is a bad idea in general (and so I want to discourage it: #294), but it raises additional questions for tools that implement JSpecify rules.
For example, consider:
That would appear to declare a field with type
String?
. However, if@Green
is declared such that it@Implies(NonNull.class)
, then@Green
and@Nullable
conflict, and so the type of the field isString*
[edit: at least in null-umarked code; for discussion of null-marked code, see the simpler example below] (#216).If the
.class
file for@Green
is not available on the classpath, then we don't know which of those types is correct.For that matter, even in the simpler case of one annotation on the field...
...we can't know if the field type is
String!!
orString?
unless we have the.class
file for@Green
.The text was updated successfully, but these errors were encountered: