Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 366645 - [quick fix] Sometimes (when Quick Fix Process returns multiple ProblemLocations) proposals can get duplicated in Quick Fix Dialog
Summary: [quick fix] Sometimes (when Quick Fix Process returns multiple ProblemLocatio...
Status: RESOLVED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 minor (vote)
Target Milestone: ---   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-13 18:00 EST by Kivanc Muslu CLA
Modified: 2012-07-18 07:11 EDT (History)
3 users (show)

See Also:


Attachments
Test Case for reproducing the bug (170 bytes, application/octet-stream)
2011-12-13 18:02 EST, Kivanc Muslu CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kivanc Muslu CLA 2011-12-13 18:00:55 EST
Build Identifier: I20110613-1736

I have attached a simple test case that reproduces the bug deterministically. For the first compilation error, Quick Fix Processor (or Eclipse) returns 2 ProblemLocations (I don't know why). Moreover, I guess the proposals generated for these ProblemLocations have one proposal in common ("Make type 'Concrete Type' abstract"). However, this is not detected by either JDT or UI so that proposal is duplicated in the Quick Fix Dialog. 

Some possible reasons/solutions I can think of: 
- One of the ProblemLocations is generated incorrectly (I always thought that problem locations were corresponding compilation errors, and at that line, there is only one compilation error).
- While generating the proposals inside the loop QuickFixProcessor.getCorrections(...), resultingCollections is an array that collects all proposals generated. This can be replaced by a set or you might want to check whether a proposal is already added or not.
    - Note that this is partially done by looking at handledProblems set (Integer), however it does not seem to work. Maybe, there is something wrong with the problem id?

Reproducible: Always

Steps to Reproduce:
1. Download the sample file I attached (the file name should be AbstractType.java in default package in Eclipse. 
2. For the first compilation error (i.e., ConcreteType should be underlined red), invoke quick fix by either clicking on the icon or by hitting shortcut (don't hover on the error, hovering works correctly)
3. You should be able to see the duplicated proposal (Make type 'ConcreteType' abstract)
Comment 1 Kivanc Muslu CLA 2011-12-13 18:02:39 EST
Created attachment 208358 [details]
Test Case for reproducing the bug
Comment 2 Dani Megert CLA 2011-12-14 02:41:59 EST
Deepak, please investigate.
Comment 3 Deepak Azad CLA 2011-12-16 13:58:54 EST
Not exactly a bug..

--------------------------------------------------
public abstract class AbstractType 
{
	public abstract void doWork();
}

class ConcreteType extends AbstractType      // 2 errors
{
	public abstract void concreteMethod() {}
}
--------------------------------------------------

There are 2 compile errors on the line, which can be seen in the problems view or the hover in the vertical ruler. 
- The type ConcreteType must implement the inherited abstract method AbstractType.doWork()
- The type ConcreteType must be an abstract class to define abstract methods
=> The problem locations are correct.

When Ctrl+1 is pressed we return proposals for all the problems at the invocation location. It just happens that in this case both problems have a common quick fix which results in duplicates. Not sure how we can try to eliminate duplicates.

The problem hover works correctly because there we return proposals for only one problem, hence nothing is duplicated.
Comment 4 Deepak Azad CLA 2011-12-16 14:15:35 EST
(In reply to comment #3)
> Not sure how we can try to eliminate duplicates.
Maybe we can just check in QuickFixProcessor.getCorrections(...) if two quick fixes have the same name/label and eliminate any duplicates.
Comment 5 Kivanc Muslu CLA 2011-12-16 14:18:36 EST
Deepak, this was exactly what I thought after reading your explanation. You might just add all proposal.getDisplayString(...) to a HashSet<String> in the getCorrections(...) method and just check if there exists a proposal with the exact string description already (by the way there might be a more structural way over the equals or .getClass() maybe, however I couldn't find it yet).

(In reply to comment #4)
> Maybe we can just check in QuickFixProcessor.getCorrections(...) if two quick
> fixes have the same name/label and eliminate any duplicates.
Comment 6 Deepak Azad CLA 2011-12-16 14:46:54 EST
(In reply to comment #5)
> by the way there might be a more structural
> way over the equals or .getClass() maybe, however I couldn't find it yet
Unfortunately there isn't. While comparing names should be sufficient in most cases, it does not really guarantee equivalence as you have to compare the underlying 'change' to be 100% sure.

In any case, comparing names feels kind of dirty and the 'duplicates' problem is also rare. Hence, unless this is known to happen in more cases I am leaning towards doing nothing for now.
Comment 7 Deepak Azad CLA 2012-07-18 07:11:08 EDT
Closing as WONTFIX. Can be reopened if the problem occurs in more cases.