Bug 126734 - [correction] MarkerResolution not provided if multiple markers are on same line
Summary: [correction] MarkerResolution not provided if multiple markers are on same line
Status: RESOLVED INVALID
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.1.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.2 M6   Edit
Assignee: Dani Megert CLA
QA Contact:
URL:
Whiteboard:
Keywords: investigate
Depends on:
Blocks:
 
Reported: 2006-02-07 10:35 EST by Lars Ködderitzsch CLA
Modified: 2006-03-26 16:56 EST (History)
0 users

See Also:


Attachments
Current eclipse-cs head as complete eclipse project (1.80 MB, application/octet-stream)
2006-02-07 16:51 EST, Lars Ködderitzsch CLA
no flags Details
Example project containing the markers (2.08 KB, application/octet-stream)
2006-02-07 16:59 EST, Lars Ködderitzsch CLA
no flags Details
Debugger snapshot (46.29 KB, image/png)
2006-03-26 16:56 EST, Dani Megert CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Ködderitzsch CLA 2006-02-07 10:35:45 EST
I am using marker resolutions to provide quickfixes for the markers my plugin (http://eclipse-cs.sourceforge.net) produces.

If more than one marker is on a given line and for one of the markers no marker resolution is provided by my custom MarkerResolutionGenerator then no quickfix at all is proposed.

If only a single marker is on a line the quickfix proposal works as expected.

A bit of debugging showed that the getResolutions() method on my MarkerResolutionsGenerator for the other markers is not even called, althoug the call to hasResolution() (my MarkerResolutionGenerator implements IMarkerResolutionGenerator2) returned true for these markers.
Comment 1 Dani Megert CLA 2006-02-07 10:55:58 EST
Could you provide a simple test case? Maybe a stripped down version of your code that shows this?
Comment 2 Lars Ködderitzsch CLA 2006-02-07 16:51:12 EST
Created attachment 34299 [details]
Current eclipse-cs head as complete eclipse project

This is the head stream of the current eclipse-cs development.
The zip file contains the whole eclipse plugin project.
It should be importable without problems. Debugging can (as usual) be made via runtime workbench.
Comment 3 Lars Ködderitzsch CLA 2006-02-07 16:59:08 EST
Created attachment 34302 [details]
Example project containing the markers

This example project should be imported within the runtime workbench where the eclipse-cs plugin is debugged (see previous attachment).
The project produces the checkstyle markers (project needs to be built once) to show the problem.

Line 11 of Example.java contains two markers - for one marker (Empty Statement) a MarkerResolution exists but is not provided when quickfix proposals are requested.
Line 14 also contains a similar Empty Statement marker - but this time no other marker is present in the line. The quickfix proposal (Remove Semicolon) is correctly provided and the quickfix can be invoked.
Comment 4 Lars Ködderitzsch CLA 2006-02-07 17:02:59 EST
Sorry about the huge attachment. I could not provide a simple test case because the problem manifest itself only within the plugin environment.

The class providing the MarkerResolutions is the CheckstyleMarkerResolutionGenerator class which implements IMarkerResolutionGenerator2.
The resolution generator is bound via the appropriate markerResolution extension point.

Please don't hesitate if you need additional info.
Comment 5 Lars Ködderitzsch CLA 2006-02-08 15:49:43 EST
I did some more debugging and think I tracked the problem a bit more down.

The problem seems to be that the call to JavaCorrectionAssistant#collectQuickFixableAnnotations gives an empty collection for 'resultingAnnotations' when calculating for the line 11 from the sample file.

The invocationLocation when clicked on the left hand ruler on line 11 is '193'.
Offset 193 seemingly is the end offset for line 11, so the IRegion for this line spans from offset 190 to 193.

The first marker on line 11 (Tab character warning) starts at offset 190 (length 3) and falls into the region taken into account to look for fixable markers. The CheckstyleMarkerResolutionGenerator correctly returns no resolution for this marker so this annotation is skipped.

So far so good...
Now the second marker on line 11 gets processed. It starts at offset 193 with a length of 0.
The call of "isInside(pos.offset, rangeStart, rangeEnd)" - in this case (isInside(193, 190, 193)) - for this marker returns false.
So this marker (which would have a quickfix) is skipped too, resulting in no fixable marker for line 11.

The code of isInside looks the following (Eclipse 3.1.1):
private static boolean isInside(int offset, int start, int end) {
  return offset == start || (offset > start && offset < end); // make sure to handle 0-length ranges
}

Judging from the code the second annotation on line 11 (offset 193) is clearly not inside the line range (190-193).

This imposes two possible problem causes:
1. the implementation of isInside is wrong, because the upper threshold is not correctly taken into account

2. My code that produces the marker offsets is wrong because it places the marker at the end of the line.

I think that pretty much narrows it down, although I can't decide what is the true cause.
Judging from that the lower threshold (offset == start) is taken into account my uneducated guess would be that the same must be done for the upper threshold.

Phew, I hope that was all still understandable and helps a bit...
Comment 6 Lars Ködderitzsch CLA 2006-02-08 16:16:49 EST
More testing revealed that the problem has in fact nothing to do with how many markers there are on the line - the problem described in my previous message can also be reproduced with only one marker per line, when the markers starting offset==line end offset.

So I don't know if the described problem classifies a bug.
Comment 7 Dani Megert CLA 2006-02-09 03:14:39 EST
Note that the line length includes the line delimiters i.e. if the marker exceeds the last character then we ignore it. Is that the case with your markers?
Comment 8 Lars Ködderitzsch CLA 2006-02-09 03:37:22 EST
No, the marker is exaclty at the line end offset (line end offset  = 193, marker start offset = 193 with length = 0). So the marker annotation does not exeed the line.

The annotation for this marker is not visible in the editor test area (because of lenght 0) but the marker is correctly shown in the left hand side ruler as being "on the line".
So the marker (while admittingly has an odd position) clearly belongs to the line, I would suspect that the marker would need to be considered when looking for fixable markers on the line.

Now that I found the cause for the problem it is clearly fixable on my side with a workaround (when marker start offset fall onto the end off a line use calculated offset -1).
Comment 9 Dani Megert CLA 2006-02-09 03:42:40 EST
If the marker is shown on the left then quick fix should offer a resolution.
Comment 10 Lars Ködderitzsch CLA 2006-02-09 03:55:48 EST
I agree.

A proposed fix would be to change the implementation of JavaCorrectionAssistant#isInside() to something like that

private static boolean isInside(int offset, int start, int end) {
  return offset == start || (offset > start && offset <= end); // make sure to
handle 0-length ranges
}

(Note the <=)
Comment 11 Lars Ködderitzsch CLA 2006-02-10 07:45:25 EST
Sorry if I am annoying, but is there an agreement that this bug needs to get fixed in the eclipse codebase? I didn't quite understand it from Daniel Megerts last post.
Comment 12 Dani Megert CLA 2006-02-10 07:49:58 EST
Yes, otherwise I would have closed the bug.
Comment 13 Lars Ködderitzsch CLA 2006-02-10 07:55:35 EST
OK, thanks :-)
Comment 14 Dani Megert CLA 2006-03-26 16:56:32 EST
Created attachment 36962 [details]
Debugger snapshot

I verified that the current code is OK and installed the attached examples to verify it: as you can see in the picture the offset of the marker is at 0A which is in the middle of the line feed. This is not supported. I also looked at the code which translates the marker into annotations: the marker already points to 0A at that point.
Comment 15 Dani Megert CLA 2006-03-26 16:56:59 EST
.