Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 355274

Summary: [assist] make the add signatures assist smarter vis-a-vis ambiguous method bindings
Product: [Tools] Objectteams Reporter: Stephan Herrmann <stephan.herrmann>
Component: OTDTAssignee: Stephan Herrmann <stephan.herrmann>
Status: VERIFIED FIXED QA Contact:
Severity: enhancement    
Priority: P3    
Version: 2.0   
Target Milestone: 2.1 M2   
Hardware: Other   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Test & implementation
none
more improvements none

Description Stephan Herrmann CLA 2011-08-19 17:23:29 EDT
In a role bound to BatchImageBuilder I had this (ambiguous) callin binding

		compile <- replace compile;

Invoking the "add signatures to method binding" quick assist throws:

java.lang.NullPointerException
	at org.eclipse.objectteams.otdt.internal.ui.text.correction.QuickAssistProcessor$2.getRewrite(QuickAssistProcessor.java:222)
	at org.eclipse.jdt.internal.ui.text.correction.proposals.ASTRewriteCorrectionProposal.addEdits(ASTRewriteCorrectionProposal.java:94)
	at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.createTextChange(CUCorrectionProposal.java:391)
	at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.createChange(CUCorrectionProposal.java:401)
	at org.eclipse.jdt.internal.ui.text.correction.proposals.ChangeCorrectionProposal.getChange(ChangeCorrectionProposal.java:340)
	at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.getTextChange(CUCorrectionProposal.java:411)
	at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.getAdditionalProposalInfo(CUCorrectionProposal.java:153)
	at org.eclipse.jface.text.contentassist.AdditionalInfoController$3.run(AdditionalInfoController.java:106)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Comment 1 Stephan Herrmann CLA 2011-08-27 11:01:55 EDT
.
Comment 2 Stephan Herrmann CLA 2011-08-27 16:06:10 EDT
The root cause of the NPE is that we were assuming an error-free callin
binding.
Let's broaden the scope of this bug so that the quick assist will also 
handle cases of ambiguity:
Find the most suitable base method but also propose as alternatives
(linked mode) any other matching base method.
Comment 3 Stephan Herrmann CLA 2011-08-27 20:04:29 EDT
Created attachment 202274 [details]
Test & implementation

This patch implements a backup strategy if a signature-less method
binding does not uniquely resolve to an existing method:

First lookup the role method (still needs to be resolvable)
and then search matching base methods given the name from the
base method spec such that parameters are compatible with the 
role method (considering lifting/lowering as well as ignoring
of extra parameters). If more than one match is found, alternatives
are presented in linked mode.

Relevance is computed such that the assist ranks higher when it
potentially fixes an ambiguity.

Additional fixes included: 
+ make dom.TypeBindingisAssignmentCompatible(..) safe for use while 
  Dependencies are not configured.
+ improve NaiveASTFlattern wrt signatures of method specs
Comment 4 Stephan Herrmann CLA 2011-08-28 10:35:34 EDT
Created attachment 202281 [details]
more improvements

This additional patch improves the interplay between different assists:
create/remove signatures & create new method.

Reduce side-effect of an old implementation trick: for proposing new
methods using existing quick-fix implementation we create a faked
method declaration with a unresolved method invocation. Adding this
faked node to the parent type declaration caused problems when the fake
node was later found by a different assist! 
Fixed by using one-way linkage only: faked node knows its parent but not
vice versa (new method AST.newFakedMethodDeclaration(TypeDeclaration)).

Robustness/completenesss in add-signatures assist: 0 matching base methods,
base method in super-base (avoid duplicates using a set of signatures).

Add/remove-signatures proposed from QuickFix/AssistProcessors:
- extract new RemoveMethodMappingSignaturesProposal
- uniform factory methods in MappingProposalSubProcessor
- avoid duplicate proposals by letting quick assist check for a problem
  for which quickfix will propose a solution
- add these proposals from quickfix processor, too
Comment 5 Stephan Herrmann CLA 2011-08-28 11:57:02 EDT
Released for 2.1 M2 (r1986-88)
Comment 6 Stephan Herrmann CLA 2012-09-22 10:52:54 EDT
Verified using build 2.2.1.201209182002