Community
Participate
Working Groups
ArrayList al = new ArrayList[CTRL-SPACE] i get options to select what class it is (java.util..ArrayList) What would be nice if the i select on that then also the opening brace is inserted. Because after that i can immediantly ask the code completion to give me all the constructor options. (This can also be automatic so right after i select the java.util.ArrayList of the code completion list i get a list with the constructors)
This would not work in the case you were targeting a member type incrementally.
Can you give me an example of that? Because if i really select a ArrayList for example in the first popup list It then knows i want that one.
class X { static class Y { } } new X.Y() CodeAssist doesn't provide all member completions on a type request, so when completing on 'new', we will provide 'X', not 'X.Y' and you need to be able to go from there to 'X.Y'. Now I agree it is not a likely scenario, but it could happen. Will see if we can do better post 2.0
Anyway, that will be nice to have, at least for "normal" constructors.
+1 here. just wanted to enter that exact request when i saw this report
Reopening for 3.2 consideration. Since a constructor invocation after "new" is much more likely than a member type qualification, I believe that constructor proposals should be shown after "new". Notes: - Do you know if a type has any inner / member types? If this information is available, you could rule out the qualification scenario and propose a type proposal (as you do now) only for types that have member types. - We can still be smart and not insert the parentheses if the user applies a proposal by pressing '.', assuming he wants to qualify a member type. - One problem I see is that the number of proposals will grow - In the example of comment 0, instead of just one ArrayList proposal, there would be 3, one per constructor. This would blow up the number of proposals, especially if the completion token is short and many types match: List list= new A| - Also, it may have a performance impact to look for constructors instead of just types.
Additional note: we quickly discussed at the JDT Summit whether some basics changed in JDT Core that would make it possible to tackle bug 83392 but this isn't the case. The simpler case i.e. proposing constructors in addition to types (no computation of compatible types) would be a good start.
>Reopening for 3.2 consideration. I guess you meant 3.3.
Discussed with Dani and Martin. Agreed for a simpler solution based on new allocation proposal (i.e. type completion after new keyword).
Created attachment 58244 [details] Possible fix With this patch the completion engine propose a new kind of proposal when the completion occurs in a type ref of an allocation. This proposal contains the same information as type ref proposal. We need to check with JDT/Text if this API is usable.
Where are we at now ? Are we willing to release this for 3.3 ?
*** Bug 52834 has been marked as a duplicate of this bug. ***
I hacked a prototype around the attached patch and looked/discussed it with Martin. We think it is not good enough for 3.3 for these three reasons: 1) JDT Text currently has a limitation that it cannot fill/guess the argument for proposals after a '(', see bug 52834 for details) 2) the list gets too long with each type being duplicated (this will be hard to change/fix for JDT Core) 3) it looks strange that the user has to choose the generic constructor proposal and then the real one - this let's us look stupid Here's another possible solution that came to mind: Couldn't JDT Core suggest the constructor proposals of the type that's currently being completed i.e. new HereIsAResolvedType<code assist> would also suggest the constructors for HereIsAResolvedType (but not for all other types that start with 'HereIsAResolvedType'). The result expected from JDT Text would be the same as when completing method<code assist> i.e. the completion string would not be empty but contain the select constructor string, e.g. "HereIsAResolvedType()". This solution would not require new API and could be continued in M7.
>for proposals after a '(', see bug 52834 for details) The correct bug number is bug 25313.
I do a prototype of your last suggestion: suggest the constructor proposals of the type that's currently being completed. So that's possible to add this behavior and indeed that doesn't require new API. I need to check impact on performances
Created attachment 60971 [details] Prototype
David we can go with this prototype - it looks good! Please commit for M7.
Ping. Please commit the patch for M7. It helps a lot.
The patch isn't complete. Types qualification (or imports addition) and constructor of member types aren't correctly supported by this patch. This patch cannot be released and must be improved.
I tested performances of this patch when the requestor filter TYPE_REF proposals. Code assist can be more than twice slower in this case and this configuration is the same as the jdt/text requestor. The prototype must be rewritten to improve performances.
Created attachment 67013 [details] Improved fix
The patch of comment 21 is an upgrade of the previous prototype. Constructors are proposed as METHOD_REF and when the type is not yet imported then these proposals have a TYPE_IMPORT required proposal. Member types are proposed if they doesn't require any qualification. Construtors of generic types are proposed as raw type. With a workspace which contains eclipse 3.0 as source and a java project which reference all other projects, then completion request is twice slower than before (this test case is bigger than the test case used in comment 20) and the time of the request is around 100ms. This patch require some modification on jdt/text side. When the type is not yet imported then 'new Test.Test()' is inserted by jdt/text instead of 'new Test()' So for 3.3 i could 1) release a fix similar to the patch in comment 10 - jdt/text doesn't find it useful (comment 13) - performance could be twice slower if ALLOCATION_TYPE_REF aren't computed in the same computer as TYPE_REF or 2) release the patch of comment 21 - only a subset of the problem is fixed because only contructors with the same name as the completion prefix are proposed - perfomance are 30% slower with a small test case and 100% slower with a big test case or 3) defer this bug post 3.3 In my point of view, if we consider that the performance loss is acceptable then 2) is better than 1) because of the argument 3) in comment 13. Daniel - what do you think of this patch ?
I discussed with Phillipe and he don't think that we should release this patch for 3.3. Only a small subset of the problem is fixed by this patch and we are late in 3.3 release cycle for this kind of behavior.
Agree.
May investigate more during 3.4, time permitting.
This involves API. We should decide now whether we do this for 3.4.
Not for 3.4. To be investigated again early 3.5
Need to reassess the requirement, and define a possible solution for 3.5. By M3 the decision needs to occur. David to drive the decision making.
Another way to fix this issue is to have bug 247433 fixed. Performance would then be no problem and we could accurately report constructor proposals. Moving target to 3.5M4 assuming bug 247433 will be fixed for M3.
Created attachment 120712 [details] Proposed patch using search indexes This patch contains an algorithm that use the constructor declaration search indexes. These indexes have been improved to contain all the required data. With this improvement the indexes are around 10% bigger and the indexing performance test is 4% slower. I do not see another regression on other performance tests of search and completion. The computation of constructor proposals can be long when the workspace is big. So to avoid to surprise existing clients with this long computation these proposals are not proposed unless they are explicitly asked. To ask these proposals the new API CompletionRequestor#setAllowsLongComputationProposals(true) must be called. The proposed constructors are given through a new completion proposal kind CompletionProposal#CONSTRUCTOR_INVOCATION. This new kind of proposals give the same information as METHOD_REF but the completion string is different. eg new ArrayLi| when doing code assist at |, a proposed completion string is "java.util.ArrayList()" On my laptop when i complete constructor in small test cases then all constructors are proposed before the timeout of 500ms With a test case like all jdtcore projects as source and all its required projects as source, when i do code assist with only one character (eg 'new S|') the timeout is reached before end. With two characters (eg 'new So|') the timeout is sometimes reached (For information there is around 50 proposed constructors in this case). Compute constructor proposals is faster with binary types. The added API are: public class CompletionProposal { ... /** * Completion is a reference to a constructor. * This kind of completion might occur in a context like * <code>"new Lis"</code> and complete it to * <code>"new List();"</code>. * <p> * The following additional context information is available * for this kind of completion proposal at little extra cost: * <ul> * <li>{@link #getDeclarationSignature()} - * the type signature of the type that declares the constructor that is referenced * </li> * <li>{@link #getFlags()} - * the modifiers flags of the constructor that is referenced * </li> * <li>{@link #getName()} - * the simple name of the constructor that is referenced * </li> * <li>{@link #getSignature()} - * the method signature of the constructor that is referenced * </li> * </ul> * </p> * <p> * This kind of proposal could require a long computation, so they are computed only if * {@link CompletionRequestor#isAllowingLongComputationProposals()} return <code>true</code>. * </p> * * @see #getKind() * @see CompletionRequestor#isAllowingLongComputationProposals() * * @since 3.5 */ public static final int CONSTRUCTOR_INVOCATION = 26; /** * Completion is a reference of a constructor of an anonymous class. * This kind of completion might occur in a context like * <code>"new Lis^;"</code> and complete it to * <code>"new List() {}"</code>. * <p> * The following additional context information is available * for this kind of completion proposal at little extra cost: * <ul> * <li>{@link #getDeclarationSignature()} - * the type signature of the type being implemented or subclassed * </li> * <li>{@link #getDeclarationKey()} - * the type unique key of the type being implemented or subclassed * </li> * <li>{@link #getSignature()} - * the method signature of the constructor that is referenced * </li> * <li>{@link #getKey()} - * the method unique key of the constructor that is referenced * if the declaring type is not an interface * </li> * <li>{@link #getFlags()} - * the modifiers flags of the constructor that is referenced * </li> * </ul> * </p> * <p> * This kind of proposal could require a long computation, so they are computed only if * {@link CompletionRequestor#isAllowingLongComputationProposals()} return <code>true</code>. * </p> * * @see #getKind() * @see CompletionRequestor#isAllowingLongComputationProposals() * * @since 3.5 */ public static final int ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION = 27; ... } public abstract class CompletionRequestor { ... /** * Returns whether proposals which could require long computation must be proposed. * Proposals which could require long computation are among the following kinds * <ul> * <li><code>CONSTRUCTOR_INVOCATION</code></li> * <li><code>ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION</code></li> * </ul> * * @return <code>true</code> proposals which could require long computation must be proposed * , and <code>false</code> if it isn't of interest. * * <p> * By default, proposals which could require long computation aren't allowed. * </p> * * @see #setAllowsLongComputationProposals(boolean) * @see ICodeAssist#codeComplete(int, CompletionRequestor, org.eclipse.core.runtime.IProgressMonitor) * * @since 3.5 */ public boolean isAllowingLongComputationProposals() {...} /** * Sets whether proposals which could require long computation must be proposed. * <p> * Compute these proposals can be very long in some circumstance (eg. a workspace with lot of types). * To avoid that the code assist operation take too much time you could call code assist operation * with a progress monitor ({@link ICodeAssist#codeComplete(int, CompletionRequestor, org.eclipse.core.runtime.IProgressMonitor)} * and automatically cancel the code assist operation when a specified amount of time is reached. * * <pre> * ICodeAssist var = ...; * int offset = = ...; * CompletionRequestor requestor = ...; * var.codeComplete(offset, requestor, new IProgressMonitor() { * private long endTime; * public void beginTask(String name, int totalWork) { * fEndTime= System.currentTimeMillis() + timeout; * } * public boolean isCanceled() { * return endTime <= System.currentTimeMillis(); * } * ... * }); * </pre> * <p> * * @param allow <code>true</code> if proposals which could require long computation must be proposed, * and <code>false</code> if it isn't of interest * @see #isAllowingLongComputationProposals() * * @since 3.5 */ public void setAllowsLongComputationProposals(boolean allow) {...} ... }
Created attachment 120815 [details] Improved patch Improve the previous patch by fixing some bugs in code and regression tests. The added test CompletionTest2#testBug6930_17() has some problems and can cause the failure of testBug6930_17() or testBug6930_18().
Created attachment 121945 [details] Improved patch Improve the previous patch. The problem in CompletionTest2#testBug6930_17() is fixed. Daniel - What do you think about the proposed API and the proposed behavior ?
- CompletionProposal.findParameterNames(IProgressMonitor) should be changed to also work for constructor proposals. We need/call this in our code. Currently I get an NPE when using: String s= new String| and selecting the one with the String as argument - I would not add CompletionRequestor.setAllowsLongComputationProposals(boolean) but instead implicitly enable the long running kinds if the new codeSelect API with a progress monitor is used. If you decide to keep the new API then I would document on this API which kinds depend on this flag.
Created attachment 122300 [details] Improved patch Improve the previous patch. This patch add a fix for the problem of CompletionProposal.findParameterNames(IProgressMonitor). Parameter names were not computed for binary constructors computed from expected types. Added CompletionTest2#testBug6930_29()
The fix works but on a second look at the constructor proposals I see that the parameter names are neither resolved from attached source nor from attached Javadoc. Our clients are used to see the real parameter name in code assist and they would not understand why we show the parameter name for e.g. String.toLowerCase(Locale locale) but not for String(String original). This would make us look bad and is a no-go from the UI side.
More findings: - the new proposals currently don't support required (import) proposals like methods do. We'd like to treat the new proposals like method proposals and hence we should either support required TYPE_REF proposals or not propose them when the type is not yet known. Note that JDT Core also treats constructors like methods i.e. there's not IConstructor but IMethod.isConstructor() - on the same page we'd like to get the type name when calling getName() on the anonymous constructor proposal - the anonymous constructor proposal's Javadoc should mention in which cases it is proposed (e.g. for new T| it is only proposed if T is abstract or an interface)
Created attachment 122551 [details] Improved patch Improve the previous patch with suggestions from Daniel 1) I removed the API setAllowsLongComputationProposals(boolean) and isAllowingLongComputationProposals(). The long running proposals are enabled when code assist is called with a not null progress monitor. 2) getName() of ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION returns a result. The information were already present in the declaring type signature but use getName() will be simpler. 3) I added in the javadoc of ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION that this proposal is proposed only for interfaces or abstract classes and i added in the javadoc of CONSTRUCTOR_INVOCATION that this proposal is proposed only for not abstract classes. 4) In the previous patch all useful information were stored in a simple proposal CONSTRUCTOR_INVOCATION. These proposals look like a METHOD_REF with some information about the declaring type stored int the completion string. It seems simpler for clients to avoid to store these information in the completion string and add a required proposal to describe the declaring type instead. In this patch when new Lis| is completed the proposal is a CONSTRUCTOR_INVOCATION with "()" as completion string with a required proposal TYPE_REF with "java.util.List" as completion string. If java.util.List is already imported then the completion string of the required proposal is "List". 5) Arguments names of binary methods which are in a classfile with no debug info were not proposed because these argument names are slightly costly to extract from attached source. And when there is a lot of proposed constructors it can become very costly to compute. rt.jar is in this case, so this limitation is very visible. This patch allow to extract arguments names from attached source and it slow down code assist but the behavior looks acceptable. This is especially true when only one character is completed (eg. new S|). On windows only argument names of proposals displayed in the list of proposals are asked but on some other platforms the argument names will be asked for all proposals of the list (displayed or not), so the problem will be worse Arguments names computation can open some java model elements, so i added a protection to not open too much elements. When a binary method come from an archive then extract arguments names from the attached source doesn't open the corresponding java model element but when the binary method doesn't come from an archive then the corresponding java model element is opened. To avoid to put too much useless elements in the java model cache and remove useful elements, argument names computation does not try to open more than 100 elements. When this limit is reached then arg0, arg1 are returned.
Created attachment 122817 [details] Improved patch Improve the previous patch. Fix several problems in constructor modifiers stored in search indexes and some other problems. Add test ComptestTest2#testBug6930_30 -> testBug6930_33()
Released for 3.5M5. Added JavaSearchBugsTests#testBug6930_AllConstructorDeclarations01() -> testBug6930_AllConstructorDeclarations05() CompletionTest2#testBug6930_01() -> testBug6930_33() I released the last patch.
Encountered 1 problem with completing a raw type constructor, see bug 262607. And the related bug 260717 has problems. Verified for 3.5M5 using I20090126-1300
*** Bug 90577 has been marked as a duplicate of this bug. ***
*** Bug 74294 has been marked as a duplicate of this bug. ***
