Bug 287136 - [quick fix][api] Make ASTRewriteCorrectionProposal and parents public API
Summary: [quick fix][api] Make ASTRewriteCorrectionProposal and parents public API
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P2 enhancement (vote)
Target Milestone: 3.8 M6   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2009-08-20 00:08 EDT by Heath Borders CLA
Modified: 2012-03-12 22:46 EDT (History)
3 users (show)

See Also:
markus.kell.r: review+


Attachments
first draft (25.57 KB, patch)
2011-08-13 03:37 EDT, Deepak Azad CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Heath Borders CLA 2009-08-20 00:08:48 EDT
I'm writing a plugin to add quickfix support for creating mock objects with mockito, and I'm discovering that it is quite difficult to supply a correct IJavaCompletionProposal.  ASTRewriteCorrectionProposal looks like a very simple API to achieve the functionality I want, but of course it is internal.  Could it be made public?
Comment 1 Markus Keller CLA 2009-08-20 06:45:57 EDT
Looks doable, tentatively targeting 3.6.

Would have to clean up Javadocs a little (add where it's missing), and make sure methods are final where possible or add @nooverride or @noreference.

The only bigger issue is LinkedProposalModel, LinkedProposalPositionGroup, etc., which documentation and look too complicated in their current form.
Comment 2 Heath Borders CLA 2009-08-20 09:52:41 EDT
Sweet.  I'd be happy to work on this.  I can give about 5 hours/week to it.

If this becomes public API, could we try to simplify the QuickFixProcessor as well?  Unless there are performance concerns, QuickFixProcessor seems like it could be broken into one extension per problem.
Comment 3 Markus Keller CLA 2009-08-20 14:15:53 EDT
> The only bigger issue is LinkedProposalModel, LinkedProposalPositionGroup,
> etc., which documentation and look too complicated in their current form.
             ^
             are lacking


> Sweet.  I'd be happy to work on this.  I can give about 5 hours/week to it.

Great, thanks. Please start with fixing Javadocs and see which members could be made less visible and where we could add @nooverride, etc.


> Unless there are performance concerns, QuickFixProcessor seems like it
> could be broken into one extension per problem.

That would have to go to a separate bug. But actually, I like it the way it currently is. We don't want multiple contributions to the same extension point, and having everything in one "gatekeeper" also simplifies debugging and keeping the overview over what's already handled and how.
Comment 4 Deepak Azad CLA 2011-08-13 03:37:08 EDT
Created attachment 201456 [details]
first draft

(In reply to comment #1)
> The only bigger issue is LinkedProposalModel, LinkedProposalPositionGroup,
> etc., which documentation and look too complicated in their current form.

One solution could be to not make linked mode API, at least not in first go. The patch takes this approach.

CUCorrectionProposal shouldn't really be doing anything with linked mode. Linked mode is better handled in LinkedCorrectionProposal. But linked mode is also required in FixCorrectionProposal and RefactoringCorrectionProposal, for the lack of a better idea I created CULinkedCorrectionProposal.

Once we apply this patch, it should be trivial to make ASTRewriteCorrectionProposal and its parents API.
Comment 5 Deepak Azad CLA 2012-01-31 09:29:43 EST
I have pushed branch - dazad/bug287136-Make-ASTRewriteCorrectionProposal-API.

- In the first few commits on the branch I refactor the code to get it ready to be made API. The last commits moves 4 types - ASTRewriteCorrectionProposal, 
ChangeCorrectionProposal, CUCorrectionProposal, ICommandAccess - to a public package. (A large number of files are touched on the last commit as the import declarations need to be updated)

(In reply to comment #4)
> One solution could be to not make linked mode API, at least not in first go.
- I stick to this approach, however I have gotten rid of CULinkedCorrectionProposal mentioned in comment 4. Everything related to linked mode is now only in LinkedCorrectionProposal, which looks cleaner to me.

- I have also added constructors to the three 'proposal' types which use a default image (the green arrow). I think this is useful as JavaPluginImages.IMG_CORRECTION_CHANGE and others are not API.

- I also tried to contribute a quick assist from outside o.e.jdt.ui, and I think  (almost) everything that is needed to do that is API with these changes.

Markus, the changes look good to you?
Comment 6 Deepak Azad CLA 2012-02-28 08:04:54 EST
Ping!
Comment 7 Markus Keller CLA 2012-03-01 15:49:47 EST
I rebased the branch onto master and started the review (looks good so far).

Deepak, you created a new package "org.eclipse.jdt.ui.text.correction.proposals" for the new APIs:

ASTRewriteCorrectionProposal.java
ChangeCorrectionProposal.java
CUCorrectionProposal.java
ICommandAccess.java

I would either put them into "org.eclipse.jdt.ui.text.java" or at least call the package "org.eclipse.jdt.ui.text.java.correction".
Comment 8 Deepak Azad CLA 2012-03-01 22:12:30 EST
(In reply to comment #7)
> I would either put them into "org.eclipse.jdt.ui.text.java" or at least call
> the package "org.eclipse.jdt.ui.text.java.correction".

Sure, we can call the package "org.eclipse.jdt.ui.text.java.correction".
Comment 10 Deepak Azad CLA 2012-03-12 22:46:22 EDT
Thanks Markus!

Did you make any significant changes? (Though I did not notice anything so far...)