Community
Participate
Working Groups
The new options introduced in bug 186342 are currently available only inside the IDE, not for the batch compiler. We should add new command line options to allow the same also from ecj. See bugs 364815 for the corresponding preferences in the UI which should serve as a template for the new command line options.
As always, adding a reminder to update the batch compiler options doc when this is done. :)
I have a partial fix. Stephan, do you need help with this?
Created attachment 209789 [details] proposed fix + tests This adds the new command line option as: nullAnnot(<FQN of non null annot>|<FQN of nullable annot>|<FQN of non null by default annot>) I've added a test but don't know why it doesn't work. Stephan, can you look at it? Also, doc is missing in the patch, and all tests need to be run.
(In reply to comment #3) > Created attachment 209789 [details] > proposed fix + tests > > This adds the new command line option as: > nullAnnot(<FQN of non null annot>|<FQN of nullable annot>|<FQN of non null by > default annot>) > > I've added a test but don't know why it doesn't work. Stephan, can you look at > it? Also, doc is missing in the patch, and all tests need to be run. Thanks for the patch. I'm now looking at it (beyond the fact that Zork is missing from the test case :) )
Question: in the IDE we decided that providing a wrong annotation name should be detected in the UI, hence the compiler doesn't check if the configured names are resolvable (to avoid being too eager). In the batch case, what would you expect if s.o. has a typo in the annotation names, so no reference in the sources will match that annotation name. Currently, we remain silent about this. Should the batch compiler be more eager in checking these names?
(In reply to comment #5) > Should the batch compiler be more eager in checking these names? Yes, it did cross my mind. I didn't see a way of doing this validation in the command line compiler as such. A solution in this respect will be common to the usual case and command line case, and will be in the form of a compiler error. Also, we haven't yet added the validation even in the UI. Do you see an easy way of handling this? I intended to let the user be careful about the names for now. Later, we can talk more on how we can validate these at command line.
OK, no validation at this point. Next I'm going to add two more options so we'll have in total: -warn:nullAnnot enable, use default annotation names -warn:nullAnnot(fqn1|fqn2|fqn3) enable, use specified annotation names -warn:nullSpec enable warnings re null specification violations, use with +/- to en/disable, use with -err to set to error -nonNullByDefault globally use @NonNull as the default Of course inside the IDE options are more fine grained but I hope the above set is sufficient for batch. comments?
(In reply to comment #7) > -warn:nullAnnot > enable, use default annotation names > -warn:nullAnnot(fqn1|fqn2|fqn3) > enable, use specified annotation names > -warn:nullSpec > enable warnings re null specification violations, use with +/- to > en/disable, use with -err to set to error > -nonNullByDefault > globally use @NonNull as the default I'm not too much in favour for 3. The nullAnnot option itself should set all the 4 configurable options. This will be like "null" which switches on all null related options, even though they are separately configurable. Do we really see a case where the user may want to use nullAnnot to enable the null analysis and then want to set the nullSpec to disabled to curtail the diagnostics? 1,2 and 4th look good to me. :)
(In reply to comment #8) > (In reply to comment #7) > > -warn:nullAnnot > > enable, use default annotation names > > -warn:nullAnnot(fqn1|fqn2|fqn3) > > enable, use specified annotation names > > -warn:nullSpec > > enable warnings re null specification violations, use with +/- to > > en/disable, use with -err to set to error > > -nonNullByDefault > > globally use @NonNull as the default > I'm not too much in favour for 3. The nullAnnot option itself should set all > the 4 configurable options. This will be like "null" which switches on all null > related options, even though they are separately configurable. Do we really see > a case where the user may want to use nullAnnot to enable the null analysis and > then want to set the nullSpec to disabled to curtail the diagnostics? > 1,2 and 4th look good to me. :) Working on 3 I remembered that these can only be toggled between warning and error. So, using -warn:nullAnnot vs. -err:nullAnnot should suffice?
(In reply to comment #6) > I intended to let the user be careful > about the names for now. That's a very quaint way of putting it :)
X-ref: as per bug 186342 comment 115 we dropped the warning token "nullcontract", subsuming those (now irritant NullSpecViolation) under "null" for the sake of @SuppressWarnings. The patch from comment 3 re-introduces a warning token "nullAnnot" (with the above semantics), which breaks a test in NullAnnotationTests. I'm now going to remove that token again, *unless* s.o. speaks up and says that tokens for @SW and those used by the batch compiler (-warn:) must be in sync. I'm not 100% about our policy in this regard, but comparing CompilerOptions and batch/messages.properties I see about twice as many batch options as @SW tokens.
Created attachment 209806 [details] Tests & fix v1 Implementation currently under test. I'm now looking into the documentation side.
Looking for a template for documenting -warn:nullAnnot I found that for "-tasks" the parameter is not documented (in task-using_batch_compiler.htm). Is that by intention? If s.o. throws in a text snippet for explaining "tasks(<tags separated by |>)" I can quickly include that in the upcoming commit.
Remark on cosmetics: I'm aligning the order of annotation types with the UI: (nullable|nonnull|nonnullbydefault)
(In reply to comment #11) > The patch from comment 3 re-introduces a warning token "nullAnnot" (with the > above semantics), which breaks a test in NullAnnotationTests. I'm now going to > remove that token again, *unless* s.o. speaks up and says that tokens for @SW > and those used by the batch compiler (-warn:) must be in sync. I'm not 100% > about our policy in this regard, but comparing CompilerOptions and > batch/messages.properties I see about twice as many batch options as @SW > tokens. Sorry i forgot to remove that change. I had realized that it was unnecessary. So you did the right thing anyway.
Implementation has been released for 3.8 M5 via commit 16b5be2e00222a3a00629ad2d72eba0fbb1b2400. Documentation (validated html) has been released via commit 1ad20e8fed7504f8d737787b13e187f0bf5b7b43.
Cool!
(In reply to comment #16) > Implementation has been released for 3.8 M5 via commit > 16b5be2e00222a3a00629ad2d72eba0fbb1b2400. > > Documentation (validated html) has been released via commit > 1ad20e8fed7504f8d737787b13e187f0bf5b7b43. Jay, can this be included for Sunday's warm up build ? If you don't see this message in time, let me know :)
(In reply to comment #16) > Implementation has been released for 3.8 M5 via commit > 16b5be2e00222a3a00629ad2d72eba0fbb1b2400. Just out of curiosity, why did my original test not show the null problems?
Verified for 3.8M5 using build I20120122-2000
