Summary: | Static import resolution does not record all static elements being imported | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Ayushman Jain <amj87.iitr> | ||||||||
Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> | ||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||
Severity: | normal | ||||||||||
Priority: | P3 | CC: | jarthana, srikanth_sankaran | ||||||||
Version: | 3.8 | Flags: | srikanth_sankaran:
review+
|
||||||||
Target Milestone: | 3.8 M4 | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Whiteboard: | |||||||||||
Attachments: |
|
Description
Ayushman Jain
2011-10-19 01:18:12 EDT
I have a fairly cooked fix for this, but targetting 3.8M4 anyway, since we're towards the end of M3. Created attachment 205486 [details]
proposed fix v1.0
This patch tries to assuage the current static imports problem, by recording all possible static elements being imported by a static import.
It changes the current way of finding a single import as follows:
1) Find a single import in the usual way.
2) if we're looking at static import statement, there may be more static elements.
- If a field was found in (1), a method and a type may still be left out.
- If a method was found in (1), a type may still be left out.
- If a type was found in (1), nothing more needs to be found.
3) For each of the above, the proper process to find duplicate/conflicting imports needs to be followed. As soon as an import is complained against, we bail out and move on to the next import.
4) Created two new fields - tempImports to replace the resolvedImports local variable (so that changing this across the new methods reflects reflects everywhere), and importPtr to replace the index local variable.
5) Created a new method recordImportBinding(..) to grow the imports array on demand and add a found import binding (Takes care of one to many mapping for each import).
6) Extracted the code in faultInImports() which checked for duplicates into a new method checkAndRecordImportBinding(..)
Created attachment 206454 [details]
proposed fix v1.0 + regression tests
Added tests to the above patch.
Note that test85 and versions test the last 2 sentences in JLS 7.5.3, while test086 checks the obscuring rules in the same section.
(In reply to comment #3) > Created attachment 206454 [details] [diff] > proposed fix v1.0 + regression tests Note to self: All tests pass. Ayush, patch looks good. Sorry for the delay. Here are a couple of comments: - I was surprised to see that checkMoreStaticBindings was not recursing on itself but was duplicating the top half into bottom. - I think the code will be cleaner with checkMoreStaticBindings being a void function. I didn't see the current return value being used in a meaningful fashion. (control flow is the same irrespective of whether -1 is returned or 0) - Did I miss something ? Created attachment 206918 [details]
proposed fix v1.1+ regression tests
This patch takes care of above review comments.
Released in HEAD via commit 4889f3babd91783f30bca6f07ba58254cecf87d1 Verified for 3.8M4 with build I20111202-0800. |