Bug 539878 - Bad stale error markers and stale error content
Summary: Bad stale error markers and stale error content
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.9   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2018-10-06 11:27 EDT by Ed Willink CLA
Modified: 2022-10-19 01:52 EDT (History)
2 users (show)

See Also:


Attachments
repro (2.23 KB, application/x-zip-compressed)
2018-10-06 11:27 EDT, Ed Willink CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Willink CLA 2018-10-06 11:27:19 EDT
Created attachment 276151 [details]
repro

The attached plugin project has a trivial main class Test.java that uses com.ibm.icu.lang.UCharacter.

Open Test.java in the JDT editor.
Delete the "U" in "UCharacter.isWhitespace".
Delete the now redundant "import com.ibm.icu.lang.UCharacter;"
Do not save.

Open the MANIFEST.MF and remove "Import-Package: com.ibm.icu.lang;version="62.1.0""
Save the MANIFEST.MF.

Observe the Test.java editor.

a) the editor shows a stale "The import com.ibm cannot be resolved"

b) the red squiggle under "class T" is misleading

This appears to be a simple repro of a longstanding incremental update bug.
Comment 1 Stephan Herrmann CLA 2018-10-06 12:15:57 EDT
A assume you are aware of the two levels of sources & compilation:

- All stuff on disc is compiled, class files are generated, and any errors are displayed in the Problems (or Markers) view.

- All unsaved stuff in editors is "reconciled" against unsaved stuff in other editors (if available) and persistent files for everything else.

Now, when MANIFEST.MF is saved to no longer import that package, the file Test.java still references that import, which can no longer be resolved.

So far all seems to work as designed.

Remaining question: how should that error be reported?
- Problems view: yes, since this should reflect the persisted state
- Editor: that's where the gray error icon comes into play to indicate an error that will disappear upon save.

Are you with me?

Hint: for cross file editing without save of each individual save, the "Save All" action is really helpful to avoid inconsistent state.
Comment 2 Ed Willink CLA 2018-10-06 12:35:49 EDT
(In reply to Stephan Herrmann from comment #1)
> Now, when MANIFEST.MF is saved to no longer import that package, the file
> Test.java still references that import, which can no longer be resolved.

You may think so, but I don't. I see a perfectly good (dirty) edit in front of me that has no problems with respect to all other files in the workspace.

IMHO the red / yellow markers that I see in the editor should correspond to the compilation of all files on disk with the exception that all dirty files displace their saved counterparts. class files generated for dirty files should not be saved to disk, just used from memory by the prevailing 'global' compilation.

If you really want grey markers to indicate errors that have gone away, fine. But no new grey markers for errors that did not exist at editor open and do not exist now. If thee red error for "The import com.ibm cannot be resolved" was grey it would be slightly confusing. The red squiggle under "class T" is a consequence of trying to display a non-existent error; there is no sensible location.

(Unfortunately Save All is broken if you have used an Untitled text file as a scratch pad for console / debugger copy and pastes. Anyway I save when I am happy with my work, not while I am playing.)
Comment 3 Stephan Herrmann CLA 2018-10-06 14:14:01 EDT
(In reply to Ed Willink from comment #2)
> (In reply to Stephan Herrmann from comment #1)
> > Now, when MANIFEST.MF is saved to no longer import that package, the file
> > Test.java still references that import, which can no longer be resolved.
> 
> You may think so, but I don't. 

Whether or not a *file* has an error is not a matter of belief. The state on your disk is erroneous at that point and the builder must report that fact.

Can you also check the Problems view when it happens? To the best of my understanding, it must show the error. I assume reporting was designed such that all errors in the Problems view must also be shown in the editor.

More technically, when MANIFEST.MF is saved this triggers a project build (of saved files), but is does not trigger reconciling all dirty buffers. So the build detecting the error is simply the last action happening.

> Anyway I save when I am happy with my work, not while I am playing.

In that case the recommendation would be: avoid saving MANIFEST.MF in that situation.


> Unfortunately Save All is broken if you have used an Untitled text file

That sounds like s.t. that should be improved, IMHO.
Comment 4 Ed Willink CLA 2018-10-06 14:53:18 EDT
Yes the state on disk is erroneous and a rebuild should put updated errors in the Problems View and perhaps provide 'bad' class files.

But the dirty file(s) form a coherent work-in-progress state that is error free and should therefore not show any red errors or yellow warnings. Whether grey markers refresh is perhaps optional; I can see benefits both ways.

The repro is a simple example of what I have often observed. It doesn't have to be a MANIFEST.MF that triggers the confusing messages. The problem is that error free work-in-progress files are shown as having errors and those errors have ridiculous locality.
Comment 5 Stephan Herrmann CLA 2018-10-06 15:47:24 EDT
Let me describe what I consider bad about the current behavior:
- the error should be gray but is red
- editing (and thus triggering the reconciler) does not improve the situation
- error locations indeed use positions from the file, which don't make sense
  in the dirty buffer
- the second error actually fails to show as its position is outside the buffer's range

To see a correpsonding gray error, do the opposite: 
- restore the Import-Package in MANIFEST.MF
- change usage back to UCharacter, without inserting import in Java
- Save All and see consistent error re missing import
- insert import in Java without saving
=> On disc error still exists
   - is shown red in Problems view
   - is shown gray in the editor

I believe, both situations should be signaled similarly.

S.o. should check, why the reconciler fails to update/remove the error markers from the builder.
Comment 6 Ed Willink CLA 2018-10-07 00:53:45 EDT
Good. We agree on the problem.
Comment 7 Eclipse Genie CLA 2020-09-29 11:47:48 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 8 Eclipse Genie CLA 2022-10-19 01:52:04 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.