Bugzilla – Full Text Bug Listing
| Summary: | [1.7][content assist] Need to know whether I can use diamond | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Dani Megert <daniel_megert> | ||||||
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P2 | CC: | deepakazad, Olivier_Thomann, raksha.vasisht, stephan.herrmann | ||||||
| Version: | 3.7 | Flags: | Olivier_Thomann:
review+
|
||||||
| Target Milestone: | 3.7.1 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 349274 | ||||||||
| Attachments: |
|
||||||||
*** Bug 350559 has been marked as a duplicate of this bug. *** (In reply to comment #0) > Note though that we will never be able to give a 100% correct > answer as it also depends which actual arguments the user passes after > inserting the constructor proposal. You mean this case? : class X<T> { X(T x) { } X<Integer> x = new X<>(1); } i.e. even though the const. uses a type variable of the declaring class, the diamond usage is legal. yet the heuristic would say, cannot be used. (In reply to comment #2) > (In reply to comment #0) > > Note though that we will never be able to give a 100% correct > > answer as it also depends which actual arguments the user passes after > > inserting the constructor proposal. > > You mean this case? : > > class X<T> { > X(T x) { > } > > X<Integer> x = new X<>(1); > } > > i.e. even though the const. uses a type variable of the declaring class, the > diamond usage is legal. yet the heuristic would say, cannot be used. Yes, for a start. As said before, we can improve the heuristic later (or even now if it is obvious/inexpensive to set the right flag value. Created attachment 199438 [details]
proposed fix v1.0
This patch declares a new API in CompletionProposal
public boolean canUseDiamond(String[] paramTypeNames, CompletionContext coreContext)
Clients can just pass the parameter types in the constructor invocation and the completion context associated with a completion proposal to use this API. The actual computation is done in InternalExtendedCompletionContext since here bindings can be constructed and compared to see if the chosen constructor proposal uses any of the class' type variables (i.e. I've used the heuristic discussed above).
An example of using this API from jdt.text.ParameterGuessingProposal is
........
public void apply(IDocument document, char trigger, int offset) {
boolean isDiamond = fProposal.canUseDiamond(this.getParameterTypes(), this.fCoreContext);
try {
........
I tried a few other ways but none of them really worked (just to document):
1) Tried setting a flag with every constructor proposal at the time the proposal is built in jdt/core. However, this failed for those types that had been found from the search engine (i.e. those not in the same CU as the one in which CTRL+SPACE was done), since there were no bindings for those types.
2) Tried to create an Allocation expression from the CompletionProposal, but we do not have enough information at the time the user choses a particular proposal to be inserted.
So the patch is the best I could do. Dani, can you please tell me if this works for you? Thanks!
> So the patch is the best I could do. Dani, can you please tell me if this works
> for you? Thanks!
It worked for the cases I tested. Of course we need to make the heuristics smarter during 3.8, e.g. for the following example the method should also return 'true':
public class X<T extends Number> {
public X(T param) {
new X
}
}
Of course passing the parameter types which JDT Text needs to compute doesn't make sense to me. You have all that information in the proposal already, see CompletionProposal.getSignature().
Created attachment 199522 [details]
proposed fix v2.0 + regression tests
This patch takes care of the above case as well i.e. wherever we don't have an LHS/return type (i.e. no expected type), we always return true (<> can always be inferred as Object).
It also checks whether the proposal is a constructor, source level is < 1.7, etc.
Olivier, please review. Thanks! Patch looks good. Released in BETA_JAVA7 branch Verified. |
BETA_JAVA7. For constructor invocation proposals I need to have a way to find out whether I can use the diamond or not, e.g. CompletionProposal.canUseDiamond(). Example where diamond cannot be used: class X<T> { X(T x) { } X<Number> x = new X<Number>(1); } For 3.7.1 we could use the following simple heuristic: if the proposed constructor uses a type variable in the signature which is also used by the declaring type, then we say the diamond cannot be used. Later we can improve that if needed. Note though that we will never be able to give a 100% correct answer as it also depends which actual arguments the user passes after inserting the constructor proposal.