Bugzilla – Full Text Bug Listing
| Summary: | [content assist] code assist for package-info.java | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Paul Fullbright <paul.fullbright> | ||||||||
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert, Olivier_Thomann, pwebster, satyam.kandula, srikanth_sankaran, stanio, stephan.herrmann | ||||||||
| Version: | 3.6 | Flags: | satyam.kandula:
review+
|
||||||||
| Target Milestone: | 3.8 M7 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 345285 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Paul Fullbright
This needs to be provided by JDT Core. Incidentally, it looks like the lack of support extends to not being able to *add* code assist. We get no usable CompletionContext anywhere in a package-info.java file. The prefix is always null, the token start is always -1, and the token end is always -1. (In reply to comment #2) > Incidentally, it looks like the lack of support extends to not being able to > *add* code assist. We get no usable CompletionContext anywhere in a > package-info.java file. The prefix is always null, the token start is always > -1, and the token end is always -1. In that case, the Parser (CompletionParser) will have to support completion in package-info.java Should I log an additional bug against the parser? Or should this be reassigned/cloned? (In reply to comment #4) > Should I log an additional bug against the parser? Or should this be > reassigned/cloned? This bug report is sufficient for whatever changes are needed. I'll look into this as soon as I have time. You can anyways look at the CompletionParser and see what rule gets triggered when u try completing in package-info.java. Adding relevant actions to that rule should give us the completion token and we can then build the context in CompletionEngine using this token. Given that package-info.java will gain importance for hosting @NonNullByDefault declarations, I'm offering my help on this bug, if desired. If you plan to include a fix for this in 3.8 M7, please adjust the target suitably, so it becomes easier to track. Yes this is definitely important. Created attachment 213713 [details]
proposed fix v1.0 + regression tests
Thanks for taking this up for review Satyam. A few points about the patch: 1) I noticed that content assist does not work in any java file if the caret is outside of a class body. Thats why content assist didn't also work in package-info.java. Here I believe keywords should be proposed atleast. In package-info, only package and import should be proposed. Also, when package is already declared, then it should not be proposed again. 2) The completion for annotations was messed up in package-info.java because we were 'overwriting' the package annotations (i.e. those declared in package-info) onto the type's annotations. In case, of package-info, package-info is the type, so all annotations defined here are on this type, and there are no annotations coming from the package (unlike actual types in a package). So, this operation should not be done for package-info type. I also noticed two tests wrongly written where 3 classes in the same file were declared public, so fixed that as well, though that change is not related to this bug. (In reply to comment #10) I just played with the patch and here are some comments: - Please attach the jar file as a separate patch? - When I try to do a content assist on say @Deprecated, it converts into @java.lang.Deprecated even if there is an import. - Why are some tests in CompletionTests and some in CompletionTests2? - The tests are creating 1.4 projects. You should better create 1.5 projects. (In reply to comment #11) > (In reply to comment #10) > I just played with the patch and here are some comments: > - Please attach the jar file as a separate patch? Sure. > - When I try to do a content assist on say @Deprecated, it converts into > @java.lang.Deprecated even if there is an import. I think this is done by JDT/Text and may have been a side-effect of bug 215971. We should raise a separate bug for this. > - Why are some tests in CompletionTests and some in CompletionTests2? CompletionTests usually have simple tests where code complete is done on working copies whereas CompletionTests2 have more java model code. In keeping with the tradition, I separated the added tests. :) > - The tests are creating 1.4 projects. You should better create 1.5 projects. Ok. :) Created attachment 213746 [details]
jar for tests
This is to be copied inside ..\org.eclipse.jdt.core.tests.model\workspace\Completion
(In reply to comment #13) > Created attachment 213746 [details] > jar for tests > > This is to be copied inside > ..\org.eclipse.jdt.core.tests.model\workspace\Completion Looks like the jar attached is corrupted. I have picked up the one from the Eclipse installation and so, it's OK. (In reply to comment #12) > > - Why are some tests in CompletionTests and some in CompletionTests2? > CompletionTests usually have simple tests where code complete is done on > working copies whereas CompletionTests2 have more java model code. In keeping > with the tradition, I separated the added tests. :) Thanks for the info. You might want to add these tests in CompletionTests_1_5. Here are the code review comments: - Newly added call to system.arraycopy in CompilationUnitScope doesn't seem to be correct. Can you add a test which runs into this case? - Are the changes in CompletionEngine really necessary? Created attachment 213792 [details] proposed fix v1.1 + regression tests (In reply to comment #16) > Here are the code review comments: > - Newly added call to system.arraycopy in CompilationUnitScope doesn't seem to > be correct. Can you add a test which runs into this case? I couldn;t find any such case so I just removed the arrayCopy. The test in CompletionTest2 shows why the if check is needed. > - Are the changes in CompletionEngine really necessary? I've added a test to show this. (In reply to comment #17) The patch looks good. Minor nitpicks: - The variable name proposePackageKeyword doesn't convey the right usage. I think it will be better be renamed as ignorePackageKeyword. - CompilationUnitScope - The if's can be merged. Took care of above comments and released in master via commit http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=e29082e6108e43f171cfe9fae0a56914a1f2ec3e Is this going to make it into any 3.x builds? I'm looking at the latest and don't see it yet. (In reply to comment #20) > Is this going to make it into any 3.x builds? I'm looking at the latest and > don't see it yet. It should show up in the 3.8 I build when they get fixed, hopefully sometime next week ... PW I just tried eclipse-SDK-N20120320-2000-win32-x86_64. Should I be seeing some annotation completion proposals at this point? (In reply to comment #21) > It should show up in the 3.8 I build when they get fixed, hopefully sometime > next week ... > > PW (In reply to comment #22) > I just tried eclipse-SDK-N20120320-2000-win32-x86_64. > Should I be seeing some annotation completion proposals at this point? No this bug was fixed later than that. You might want to try http://download.eclipse.org/eclipse/downloads/drops/I20120419-1434/ (In reply to comment #23) > No this bug was fixed later than that. You might want to try > http://download.eclipse.org/eclipse/downloads/drops/I20120419-1434/ Grr, I'm reading "3" and seeing "4". But your link is "no longer available for download", and all the other I or N builds are from March. Any idea what's up on not having any working builds from *anytime* in the last month? (In reply to comment #24) > (In reply to comment #23) > > No this bug was fixed later than that. You might want to try > > http://download.eclipse.org/eclipse/downloads/drops/I20120419-1434/ > > Grr, I'm reading "3" and seeing "4". But your link is "no longer available for > download", and all the other I or N builds are from March. Any idea what's up > on not having any working builds from *anytime* in the last month? We're in the process of migrating our build infrastructure to build.eclipse.org, so there have been missing builds since the last month. I'm hoping there will be a good 4.2 build this Tuesday and 3.8 build this Wed. Thanks for your patience. Verified for 3.8 M7 using Build id: I20120429-2000 |