Bug 462412 - [null] More variants for configuring the location for external annotations
Summary: [null] More variants for configuring the location for external annotations
Status: CLOSED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.5   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 485087 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-03-17 16:31 EDT by Stephan Herrmann CLA
Modified: 2020-06-06 03:00 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2015-03-17 16:31:58 EDT
Follow-up from bug 440815:

We should consider to support classpath variables. First we should have a look at handling of classpath variables for the configuration of source attachments. There it seems to say: the source attachment is variable-based iff the classpath entry is variable-based. 
For the case of external annotations I don't think we'd want this tight coupling. I'd prefer if variable-based and static paths could be freely combined. Rationale: no matter how our libraries are referenced, users may want to define a common base folder for all annotations across projects. If that folder is outside the workspace, it should be portably referenced using a variable.


On a separate notion, it is currently not possible, to define a per-project annotation location for a JRE. Interestingly, if an annotation path has been manually configured in .classpath for the JRE container (as I have in one of my non-JDT projects) the dialog correctly displays this information as a direct child of the JRE node (i.e., not specific to any contained jars). Also editing works OK. Just if no external annotation element exists as the direct child of the JRE node, it is not possible to create this entry via the project build path > library page. Have a look at how this is handled for access rules and native library locations.


Yet another option, which JDT/Core already handles fine, is specifying an annotation location relative to the project (as a relative path). Currently, the dialog interprets relative paths as file system paths (assumeably because the path is not recognized within the workspace). Aside from a shorter path, the rationale here is, that a project relative path is robust against project renaming. Fixing the logic to detect that a project relative path exists(), should be simple, actually. What I'm not sure about is: how should a path be stored after selection using the browse button? We might want to always shorten the path, if it is within the current project. WDYT?
Comment 1 Stephan Herrmann CLA 2015-03-17 16:33:35 EDT
@Frits, tentatively assigning to you. Do you accept?
Comment 2 Frits Jalvingh CLA 2015-03-17 16:36:51 EDT
(In reply to Stephan Herrmann from comment #1)
> @Frits, tentatively assigning to you. Do you accept?
Yes, I will delve into this next weekend.
Comment 3 Stephan Herrmann CLA 2015-03-17 16:39:42 EDT
(In reply to Frits Jalvingh from comment #2)
> (In reply to Stephan Herrmann from comment #1)
> > @Frits, tentatively assigning to you. Do you accept?
> Yes, I will delve into this next weekend.

Cool, thanks.
Comment 4 Stephan Herrmann CLA 2015-03-21 16:12:17 EDT
Conversely to the 2nd para of comment 0, defining an annotation path for an element of any other classpath container (e.g., "Maven Dependencies") is allowed, but has no effect. This should be handled, too.

For such containers it is even more important to be able to define an annotation location for the entire container, not its contained jars.
Comment 5 Stephan Herrmann CLA 2015-03-28 11:31:57 EDT
(In reply to Stephan Herrmann from comment #4)
> For such containers it is even more important to be able to define an
> annotation location for the entire container, not its contained jars.

From my personal p.o.v. this is the most interesting issue in this ticket, think of Maven Dependencies, Plug-in Dependencies ...
Comment 6 Stephan Herrmann CLA 2015-04-23 08:44:54 EDT
I've extracted the minimal requirement into bug 465293, rescheduling the remains of this ticket for 4.6.

During the next cycle we should also add some feedback when, e.g., configuring external annotations for a project that doesn't have null annotations enabled etc. (I quickly checked if this situation could actually be avoided, but when going back and forth between buildpath and compiler errors/warnings there's no simple way to update the buildpath page after changes on the other page - that's why I'm thinking of an information dialog instead of disabled UI elements).
Comment 7 Stephan Herrmann CLA 2015-04-23 08:47:15 EDT
(In reply to Stephan Herrmann from comment #6)
> During the next cycle we should also add some feedback when, e.g.,
> configuring external annotations for a project that doesn't have null
> annotations enabled etc. (I quickly checked if this situation could actually
> be avoided, but when going back and forth between buildpath and compiler
> errors/warnings there's no simple way to update the buildpath page after
> changes on the other page - that's why I'm thinking of an information dialog
> instead of disabled UI elements).

To stash findings from my experiments: 
   LibrariesWorkbookPage.editAttributeEntry(CPListElementAttribute)
should be a good location for providing this feedback to users.
Comment 8 Stephan Herrmann CLA 2015-04-23 11:34:24 EDT
(In reply to Stephan Herrmann from comment #7)
> (In reply to Stephan Herrmann from comment #6)
> > During the next cycle we should also add some feedback when, e.g.,
> > configuring external annotations for a project that doesn't have null
> > annotations enabled etc. (I quickly checked if this situation could actually
> > be avoided, but when going back and forth between buildpath and compiler
> > errors/warnings there's no simple way to update the buildpath page after
> > changes on the other page - that's why I'm thinking of an information dialog
> > instead of disabled UI elements).
> 
> To stash findings from my experiments: 
>    LibrariesWorkbookPage.editAttributeEntry(CPListElementAttribute)
> should be a good location for providing this feedback to users.

A simple info message is already included in the fix for bug 465293
Comment 9 Ed Willink CLA 2015-04-28 11:10:42 EDT
(In reply to Stephan Herrmann from comment #0)
> We should consider to support classpath variables.

An alternative is a JDT preference.

I enable/disable @NonNull annotations on per-project basis in the preference page.

It seems natural to specify the per-project null libraries on the same page.
Comment 10 Stephan Herrmann CLA 2015-04-28 12:04:26 EDT
(In reply to Ed Willink from comment #9)
> (In reply to Stephan Herrmann from comment #0)
> > We should consider to support classpath variables.
> 
> An alternative is a JDT preference.
> 
> I enable/disable @NonNull annotations on per-project basis in the preference
> page.
> 
> It seems natural to specify the per-project null libraries on the same page.

Technically, JDT/Core needs this information as part of the classpath.
It would be more than a matter of property/preference pages to change this.

On the long run, I believe, it should be better not to think of external annotations as associated to your project, but as associated to the library you are using. Only so can we start exchanging external annotations for commonly used libraries.

Let's see if others chime in in your request.
Comment 11 Ed Willink CLA 2015-04-29 03:03:39 EDT
(In reply to Stephan Herrmann from comment #10)
> On the long run, I believe, it should be better not to think of external
> annotations as associated to your project, but as associated to the library

That's not practical. If I load the plugins from some project as source, I would like to see the same warnings/lack-of-warnings that the developers of that project see. That means that the compilation of sources from that project must use the same non-null libraries as the developers.

These may be different to those that I use for my project, or those that another project uses.

e.g. I may define a non-null return for StringBuilder.toString() allowing me to omit related null-checks. Another project may not and so my added value will distort their compilation.

> > It seems natural to specify the per-project null libraries on the same page.
> 
> Technically, JDT/Core needs this information as part of the classpath.

OK. But perhaps the UI can still be on the preference page. Other class path entries are variously on builder pages/manifest files.
Comment 12 Stephan Herrmann CLA 2016-01-05 06:24:10 EST
We also have a situation, where the UI allows more than will be evaluated downstream:

In a plugin project, using the Java Build Path dialog I'm able to define an external annotation path per library (jar) contained in the "Plug-in Dependencies" container. Such property is never persisted and has no effect. For classpath containers other than JRE, external annotations *must* be defined at the container root, not its children.

The same holds for all kinds of provided containers, like, e.g., Maven Dependencies etc.

@Frits, do you agree?

It's probably time to take stock of the above discussion, and make a plan what should done for Neon.
Comment 13 Ed Willink CLA 2016-01-05 07:20:44 EST
(In reply to Stephan Herrmann from comment #12)
> In a plugin project, using the Java Build Path dialog I'm able to define an
> external annotation path per library (jar) contained in the "Plug-in
> Dependencies" container. Such property is never persisted and has no effect.
> For classpath containers other than JRE, external annotations *must* be
> defined at the container root, not its children.

Ah! Very helpful comment.

If I specify an External Annotations for the Plug-in Dependencies, but not the JRE, I can navigate to the JRE location such as StringBuilder.toString(), and I get offered an Annotate but it does nothing. Either it should be not offered (obtusely correct) or it should explain what I need to do, or better as with Project Setups it should fix it for me.

Separate entries for JRE and Plugin External Annotations are useful but a pain. Their content is distinctively structured. Perhaps just a single multi-entry External Annotations dialog allowing a filter to be attached to each entry:

For each entry:
- read-only/writeable checkbox
- project path
- package name regex filter

new annotations are written to the first writeable entry whose regex matches
Comment 14 Stephan Herrmann CLA 2016-01-05 08:02:23 EST
*** Bug 485087 has been marked as a duplicate of this bug. ***
Comment 15 Stephan Herrmann CLA 2016-04-11 09:46:11 EDT
Didn't find the time during 4.6, moving to 4.7.
See that some details have been resolved via their own bugs, though.
Comment 16 Stephan Herrmann CLA 2017-05-16 12:05:24 EDT
Ran out of time for 4.7. Bulk move to 4.8.
Comment 17 Ed Willink CLA 2020-06-06 02:59:26 EDT
Bug 564014 discusses derisking the ongoing use of @NonNull / @Nullable annotations for the OCL (and QVTd) projects.