Community
Participate
Working Groups
When performing code completion on an inner class's method, the names of the parameters are not included. The scenario occurs wherein the user attaches an external jar, and then attaches the source code. 1. Add the external jar to the build path 2. Start instantiating the class, foo.Foo f = new foo.Foo(); 3. Go inside Foo and then use the attach source button to add the zipped up source code. 4. f.bar. (content assist pops up) Note here that someMethod presents 'arg0' as the parameter's name although subsequently pressing F3 on someMethod will load the source up and display 'parameterName' as the parameter's name.
Created attachment 46672 [details] The jar file to attach with the compiled class file for error reproduction purposes.
Created attachment 46673 [details] Source jar file for attaching the source to the compiled class file.
Tom, please check whether this is us or JDT Core.
We use the parameter names received from core. Moving to jdt-core.
Not sure, but initial tracking seems to locate problem around findMethodParameterNames(char[],char[],char[],char[][]) method in CompletionProposal. While debugging, I saw that declaringTypeName is Foo.Bar and the NameLookup does not find type with this name as it seems to expect name with $ for member types... HTH
Still an issue in 3.6 RC4 and it's also not working for the case where the parameter names are in the class file and no source attached.
Created attachment 171895 [details] Diff for InternalCompletionProposal.java that seems to fix the problem. I debugged the issue and found that the problem is that the symbol name is concatenated in findMethodParameterNames(...) and immediately split in NameLookup.findType(name, ....). That doesn't go well for inner classes. This patch calls NameLookup.findType(typeName, packageName, ...) and bypasses the concatenation/split and seems to solve this problem. I have no idea if it creates other problems though.
(In reply to comment #7) > Created an attachment (id=171895) [details] > Diff for InternalCompletionProposal.java that seems to fix the problem. > > I debugged the issue and found that the problem is that the symbol name is > concatenated in findMethodParameterNames(...) and immediately split in > NameLookup.findType(name, ....). That doesn't go well for inner classes. > This patch calls NameLookup.findType(typeName, packageName, ...) and bypasses > the concatenation/split and seems to solve this problem. I have no idea if it > creates other problems though. Thanks for the fix Andreas. You're correct - the concatenation and then subsequent separation of the package name and the declaring type name is the culprit here. When findType(name, ...) tries to de-concatenate the package name from the declaring type name, it results in the package name as foo.Foo and declaring type name as Bar. It should rather be foo and Foo.Bar respectively. Actually, I dont even understand why we need to follow this redundant approach at all, when we have an overloaded method NameLookup.findType(String typeName, String packageName,...) which can take both the package and declaring type names as arguments. So i think its correct to bypass this concatenation step and straightaway call this instance of the findType method. Note that this only occurs in when the source is in an external jar, because in the normal case, method argument names are computed inside CompletionEngine itself. Also note that attaching source/not shouldnt have any influence on this case. Let me test the fix, and submit a final patch with testcase attached.
While investigating, I found out one peculiar behaviour in JavaElement#getElementInfo(IProgressMonitor) due to which the following case still fails with the patch: Suppose Foo.Bar in the referenced jar had a constructor - Bar(int a, int b) {}, and Foo itself had a constructor - Foo(int a) Now the following code : new foo.Foo.Bar([CTRL-SPACE] shows argument proposals as arg0 and arg1. while new foo.Foo([CTRL-SPACE] shows the correct proposal i.e. 'a'. I've opened bug 316937 for this. Once that gets fixed, the above fix will be sufficient. Note that a similar change will also have to done to InternalCompletionProposal.findConstructorParameterNames(..)
Created attachment 173389 [details] fix + tests This is the patch incorporating Andreas' fix. With this, the problem doesnt occur even without the source attached. The change in line InternalCompletionProposal.java:252 is to make sure that once parameter names are found, we dont go ahead to line 258 and attempt to find them all over again, as was the current behaviour. Jay, requesting a quick review.
(In reply to comment #10) > Created an attachment (id=173389) [details] > fix + tests > > Jay, requesting a quick review. The fix looks good, Ayush. I have a couple of comments on the tests, though. 1) I think it's better to use the relevance constant instead of the calculated value. This way when the constant values change, the existing tests will still pass. 2) I don't see how the folder "/P/src/p151500" gets used in the tests. May be I missed something?
Created attachment 173414 [details] fix + updated tests updated fix according to above review comments. Thanks Jay!
Released in HEAD for 3.7M1. Added tests: CompletionTests2#testBug151500a() CompletionTests2#testBug151500b() CompletionTests2#testBug151500c()
Ayush, you don't need to set the iplog flag since you're a committer now.
(In reply to comment #14) > Ayush, you don't need to set the iplog flag since you're a committer now. The fix was basically contributed by Andreas. I just introduced the tests and made some additions to the fix. So to give credit to Andreas, dont i need to set the iplog?
>The fix was basically contributed by Andreas. I just introduced the tests and >made some additions to the fix. So to give credit to Andreas, dont i need to >set the iplog? Yes, if that's the case you need to set the iplog flag on his patch, otherwise you will end up in the IP log used for legal review.
Comment on attachment 171895 [details] Diff for InternalCompletionProposal.java that seems to fix the problem. Setting the iplog on Andreas' patch.
Verified for 3.7 M1 using build I20100802-1800
Question -- we have an eclipse product where we have projects that have java build paths that include a Classpath Container that directs out to an external jar. The classpath container code attaches javadoc to that jar, and we have a class like this: package some.package: public class SomeException extends Exception public abstract class SomeClass public final class InnerClass extends SomeException when our end-users are typing InnerClass ic = new InnerClass([CTRL-SPACE] they just see arg0, arg1 and no javadoc. Note that the CLASS javadoc for InnerClass appears as they hover over the type string "InnerClass". If they hover over the "new InnerClass" portion, they will see: some.package.SomeClass.InnerClass.InnerClass(SomeClass arg0, String arg1, String arg2) which might be where the trouble stems from; technically, yes, the first argument is the pointer to the containing class, but shouldn't the javadoc layer know to unwind that? (I can look at the html file on disk that the javadoc setting resolves to and I can see the constructor documentation in the html) This is in 3.6.1. If this is the same issue, it would be great if this could make it to 3.6.2.
(In reply to comment #19) Please don't hijack an existing bug but open a new one instead (https://bugs.eclipse.org/bugs/enter_bug.cgi?product=JDT).
(In reply to comment #20) > (In reply to comment #19) > Please don't hijack an existing bug but open a new one instead > (https://bugs.eclipse.org/bugs/enter_bug.cgi?product=JDT). I was attempting to avoid filing a DUPLICATE as reading through this ticket it appears related, but at your demand a new ticket # is coming in.
