Bugzilla – Full Text Bug Listing
| Summary: | [content assist] Annotation attributes are not completed for abstract method parameter annotations | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Gayan Perera <gayanper> |
| Component: | Core | Assignee: | Gayan Perera <gayanper> |
| Status: | CLOSED FIXED | QA Contact: | Till Brychcy <register.eclipse> |
| Severity: | major | ||
| Priority: | P3 | CC: | jarthana, manoj.palat, register.eclipse |
| Version: | 4.7 | ||
| Target Milestone: | 4.8 M4 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://git.eclipse.org/r/111844 https://git.eclipse.org/r/111979 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=d9bc7897f13a47cf70b0edfdef55bb3d99dfc9d8 |
||
| Whiteboard: | |||
No result is returned by ICodeAssist#codeComplete in this case when invoked from JavaCompletionProposalComputer#internalComputeCompletionProposals. Moving to JDT Core. Raised to major since this cause lack of functionality. Any update on this issue? I would like to provide a patch if someone can shed some light on where i should look at. (In reply to Gayan Perera from comment #3) > Any update on this issue? I would like to provide a patch if someone can > shed some light on where i should look at. First thing you need to find is whether or not the parsed AST is okay. Plant a breakpoint in CompletionEngine#complete() on code parsedUnit.resolve() and see what the unit looks like. Typically, the unit will contain a node whose type starts with CompletionOn*. If there's a problem with recovery, it is likely you will see something broken here, for e.g, the scope or enclosing element for the completion node is incorrect. If everything is alright, let it resolve and see where things go wrong in resolving the Completion node. Typically, most CompletionOn* nodes have a method starting with resolve*(), which is another good place to watch. Thanks for the offer, BTW. I know the annotation completion code a bit because of my work on bug 425035, so I can have a look at this. While debugging the path i found out that the reason is that parsedUnit.resolve(); doesn't throw a CompletionNodeFound signal. The reason in that at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolveStatements() the #statements field is null. But if this was the method implementation then the the #statments field is not null and resolving the first statement will signal. Probably the completionnode is moved from the parameters to the body during recovery but the body is not resolved in this case, because the method is abstract. Maybe you can find the if() that causes this to be skipped. Found the place, Seems like the curpit is at org.eclipse.jdt.internal.codeassist.impl.AssistParser.parseBlockStatements(MethodDeclaration, CompilationUnitDeclaration) if (md.isAbstract()) return; this cause the method not to be parsed which cause the parseBlockStatements method call in CompletionEngine#complete method not to update the method body. This cause the org.eclipse.jdt.internal.compiler.parser.RecoveredMethod.methodBody not to get updated which cause the problem i mentioned in my previous comment. Removing the if (md.isAbstract()) cause the completion to work, but not sure why that check is there is the first place. Can anyone help me out ? This may be just an optimization. First thing you can try is to remove the if() and run the tests. As this is only in AssistParser, it probably won't break anything. Easiest way would be to create a gerrit. I think this should be documented in https://wiki.eclipse.org/JDT_Core_Committer_FAQ New Gerrit change created: https://git.eclipse.org/r/111844 (In reply to Eclipse Genie from comment #10) > New Gerrit change created: https://git.eclipse.org/r/111844 Thanks! The tests ran successfully. What remains to be done: 1) Please make gerrit for the master. All changes first go into the master, and if no problems appear in a milestone build, they may be backported to the maintenance branch. 2) Whenever possible, a regression test should be added. Easiest way to write one is to look for a similar one. In this case, the test should probably go into CompletionTests_1_5. Note you always have to run the whole class as Junit Plugin Test. But there is one useful feature: look for the commented line that sets the static variable TEST_NAMES and set it to the name of the test you're developing, then only this test will only be executed. when i tried to push to gerrit using the master branch it keeps on rejecting. any reason ? (In reply to Gayan Perera from comment #12) > when i tried to push to gerrit using the master branch it keeps on > rejecting. any reason ? It usually tells you the reason in the dialog that pops up. Have you added the signed-off-by? Have you added a commit id? If you forgot any of these, go to the git staging view, click on the "Amend..."-button, Click on the relevant button(s) next to it to add was missing, commit again. New Gerrit change created: https://git.eclipse.org/r/111979 (In reply to Eclipse Genie from comment #14) > New Gerrit change created: https://git.eclipse.org/r/111979 Excellent! One minor thing: Can you please update the patch so the "Author" and "Signed-Off" fields contain your name, i.e. instead of gayanper <gayanper@gmail.com> it should be Gayan Perera <gayanper@gmail.com> Gerrit change https://git.eclipse.org/r/111979 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=d9bc7897f13a47cf70b0edfdef55bb3d99dfc9d8 (In reply to Eclipse Genie from comment #16) > Gerrit change https://git.eclipse.org/r/111979 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=d9bc7897f13a47cf70b0edfdef55bb3d99dfc9d8 Released for 4.8M4 Thanks, Gayan! Glad that i could make a contribution to improve eclipse :). (In reply to Gayan Perera from comment #18) > Glad that i could make a contribution to improve eclipse :). +1. Thanks Gayan for the contribution. To make it a complete commit, can you create a test as well? Almost every fix generally is accompanied with a reproducible testcase which clearly fails before the fix and passes after the fix. See CompletionTests18 for various examples. Let us know if you require more info.. (In reply to Manoj Palat from comment #19) > (In reply to Gayan Perera from comment #18) > > Glad that i could make a contribution to improve eclipse :). > > +1. Thanks Gayan for the contribution. > > To make it a complete commit, can you create a test as well? Almost every > fix generally is accompanied with a reproducible testcase which clearly > fails before the fix and passes after the fix. > See CompletionTests18 for various examples. Let us know if you require more > info.. Manoj, I guess you looked at the wrong gerrit, he already did that! (In reply to Till Brychcy from comment #20) > (In reply to Manoj Palat from comment #19) > > (In reply to Gayan Perera from comment #18) > > > Glad that i could make a contribution to improve eclipse :). > > > > +1. Thanks Gayan for the contribution. > > Manoj, I guess you looked at the wrong gerrit, he already did that! Thanks Till. My bad - yes, I see the tests in the commit - was looking at the wrong gerrit as you mentioned. Verified for Eclipse Photon 4.8 M4 with Build id: I20171205-0800 This is now fixed in Eclipse Photon 4.8, therefore closing the issue. |
Annotation attributes are not suggested when content assist is triggered on - method parameter annotation on interface methods - method parameter annotation on abstract methods in classes But attributes are suggested for: - default method on interfaces - none abstract methods on classes. Ex: @RestController public interface Controller { public List findAll(@RequestParameter(...) String type); }