Bugzilla – Full Text Bug Listing
| Summary: | ImportRewrite throws SIOOBE when trying to add import | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Markus Keller <markus.kell.r> |
| Component: | Core | Assignee: | John Glassmyer <eclipse> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | eclipse, jarthana, manoj.palat, shankhba |
| Version: | 4.5 | ||
| Target Milestone: | 4.5 M6 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| See Also: |
https://git.eclipse.org/r/42468 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1490ec2d61fb8e59c378e50775f4749531490d5f |
||
| Whiteboard: | |||
| Bug Depends on: | 430303 | ||
| Bug Blocks: | |||
|
Description
Markus Keller
Thanks, Markus! I'll look into it. I've worked around the SIOOBE with http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1798df591429fc6a5448f8a074f2d563846bcebb John, how hard would it be to not split up an import group if it's not properly sorted? If doable, then that would be the best solution. Depending on the final result, we should either revert some of the test changes that were done in bug 430303, or at least add a test for the SIOOBE. New Gerrit change created: https://git.eclipse.org/r/42468 The cause of the exception was not the order of imports within groups per se, but rather incorrect logic used when comparing import names. Specifically, adding an import that would correctly sort between two existing imports, where the first differing package name segments (of the new import and of an adjacent existing import) shared a common prefix, but were of different lengths, would throw an exception. I have added a minimal test case addressing this at https://git.eclipse.org/r/42468. In hindsight, I vaguely remember running into this case during development of bug 430303, but sadly I neglected to add a test case and come back and fix the cause. I'm glad you investigated, Markus! As for your other question, I do not believe that it would be possible to maintain groupings of incorrectly ordered existing imports without introducing unintended bugs of the sort that I initially reported in bug 430303. The old ImportRewriteAnalyzer attempted to do some things of this sort which seemed superficially like nice things to do but which did not add up to a logically consistent whole, and this resulted in the inconsistencies reported in 430303. Gerrit change https://git.eclipse.org/r/42468 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1490ec2d61fb8e59c378e50775f4749531490d5f Thanks, pushed the test case. I agree that there's no consistent solution for cases like bug 430303 comment 0. However, the case in ImportRewriteTest#testAddImports1() / comment 0 is a bit different. Here, it's not about adding an import that doesn't match any group, or about starting with a completely unsorted import section. Here, it's about a situation where imports are already properly grouped, and it would be clear to which group the new import belongs. In comment 0, the new import "p.A" belongs to the "others" group at the end of the imports section. The only part that's wrong in the initial import section is that the entries within a group are not properly sorted. Finding the "right" position in an unsorted group is of course not possible, so all I'd expect is that the import would be added anywhere in that group (e.g. at the end). OTOH, this is probably just an academic exercise, since this situation only arises when users never use Organize Imports again. So let's see if we get bugs from "real" users. Verified for 4.5 M6 with build I20150316-2000. |