Download
Getting Started
Members
Projects
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
More
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
Toggle navigation
Bugzilla – Attachment 158804 Details for
Bug 235253
[organize imports] Organize imports removes needed import statement.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
Log In
[x]
|
Terms of Use
|
Copyright Agent
[patch]
Proposed fix + regression tests
patch_235253_194358.txt (text/plain), 20.28 KB, created by
Olivier Thomann
on 2010-02-10 19:53:29 EST
(
hide
)
Description:
Proposed fix + regression tests
Filename:
MIME Type:
Creator:
Olivier Thomann
Created:
2010-02-10 19:53:29 EST
Size:
20.28 KB
patch
obsolete
>### Eclipse Workspace Patch 1.0 >#P org.eclipse.jdt.core >Index: dom/org/eclipse/jdt/core/dom/rewrite/ImportRewrite.java >=================================================================== >RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/rewrite/ImportRewrite.java,v >retrieving revision 1.18 >diff -u -r1.18 ImportRewrite.java >--- dom/org/eclipse/jdt/core/dom/rewrite/ImportRewrite.java 21 Jan 2010 16:46:38 -0000 1.18 >+++ dom/org/eclipse/jdt/core/dom/rewrite/ImportRewrite.java 11 Feb 2010 00:49:20 -0000 >@@ -24,6 +24,7 @@ > import org.eclipse.jdt.core.ICompilationUnit; > import org.eclipse.jdt.core.IImportDeclaration; > import org.eclipse.jdt.core.ITypeRoot; >+import org.eclipse.jdt.core.JavaCore; > import org.eclipse.jdt.core.JavaModelException; > import org.eclipse.jdt.core.Signature; > import org.eclipse.jdt.core.compiler.CharOperation; >@@ -137,6 +138,7 @@ > private String[] createdStaticImports; > > private boolean filterImplicitImports; >+ private boolean useContextToFilterImplicitImports; > > /** > * Creates a {@link ImportRewrite} from a {@link ICompilationUnit}. If <code>restoreExistingImports</code> >@@ -221,6 +223,8 @@ > this.restoreExistingImports= false; > } > this.filterImplicitImports= true; >+ // consider that no contexts are used >+ this.useContextToFilterImplicitImports = false; > > this.defaultContext= new ImportRewriteContext() { > public int findInContext(String qualifier, String name, int kind) { >@@ -303,15 +307,41 @@ > } > > /** >- * Specifies that implicit imports (types in default package, package <code>java.lang</code> or >- * in the same package as the rewrite compilation unit should not be created except if necessary >- * to resolve an on-demand import conflict. The filter is enabled by default. >- * @param filterImplicitImports if set, implicit imports will be filtered. >+ * Specifies that implicit imports (for types in <code>java.lang</code>, types in the same package as the rewrite >+ * compilation unit, and types in the compilation unit's main type) should not be created, except if necessary to >+ * resolve an on-demand import conflict. >+ * <p> >+ * The filter is enabled by default. >+ * </p> >+ * <p> >+ * Note: {@link #setUseContextToFilterImplicitImports(boolean)} can be used to filter implicit imports >+ * when a context is used. >+ * </p> >+ * >+ * @param filterImplicitImports >+ * if <code>true</code>, implicit imports will be filtered >+ * >+ * @see #setUseContextToFilterImplicitImports(boolean) > */ > public void setFilterImplicitImports(boolean filterImplicitImports) { > this.filterImplicitImports= filterImplicitImports; > } > >+ /** >+ * Sets whether a context should be used to properly filter implicit imports. >+ * <p> >+ * By default, the option is disabled to preserve existing behavior. >+ * </p> >+ * >+ * @param useContextToFilterImplicitImports the given setting >+ * >+ * @see #setFilterImplicitImports(boolean) >+ * @since 3.6 >+ */ >+ public void setUseContextToFilterImplicitImports(boolean useContextToFilterImplicitImports) { >+ this.useContextToFilterImplicitImports = useContextToFilterImplicitImports; >+ } >+ > private static int compareImport(char prefix, String qualifier, String name, String curr) { > if (curr.charAt(0) != prefix || !curr.endsWith(name)) { > return ImportRewriteContext.RES_NAME_UNKNOWN; >@@ -360,9 +390,31 @@ > } > } > } >+ if (this.filterImplicitImports && this.useContextToFilterImplicitImports) { >+ String fPackageName= this.compilationUnit.getParent().getElementName(); >+ String mainTypeSimpleName= JavaCore.removeJavaLikeExtension(this.compilationUnit.getElementName()); >+ String fMainTypeName= concatenateName(fPackageName, mainTypeSimpleName); >+ if (kind == ImportRewriteContext.KIND_TYPE >+ && (qualifier.equals(fPackageName) >+ || fMainTypeName.equals(concatenateName(qualifier, name)))) >+ return ImportRewriteContext.RES_NAME_FOUND; >+ } > return ImportRewriteContext.RES_NAME_UNKNOWN; > } > >+ private static String concatenateName(String name1, String name2) { >+ StringBuffer buf= new StringBuffer(); >+ if (name1 != null && name1.length() > 0) { >+ buf.append(name1); >+ } >+ if (name2 != null && name2.length() > 0) { >+ if (buf.length() > 0) { >+ buf.append('.'); >+ } >+ buf.append(name2); >+ } >+ return buf.toString(); >+ } > /** > * Adds a new import to the rewriter's record and returns a {@link Type} node that can be used > * in the code as a reference to the type. The type binding can be an array binding, type variable or wildcard. >@@ -995,6 +1047,7 @@ > > ImportRewriteAnalyzer computer= new ImportRewriteAnalyzer(this.compilationUnit, usedAstRoot, this.importOrder, this.importOnDemandThreshold, this.staticImportOnDemandThreshold, this.restoreExistingImports); > computer.setFilterImplicitImports(this.filterImplicitImports); >+ computer.setUseContextToFilterImplicitImports(this.useContextToFilterImplicitImports); > > if (this.addedImports != null) { > for (int i= 0; i < this.addedImports.size(); i++) { >Index: dom/org/eclipse/jdt/internal/core/dom/rewrite/ImportRewriteAnalyzer.java >=================================================================== >RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/dom/org/eclipse/jdt/internal/core/dom/rewrite/ImportRewriteAnalyzer.java,v >retrieving revision 1.21 >diff -u -r1.21 ImportRewriteAnalyzer.java >--- dom/org/eclipse/jdt/internal/core/dom/rewrite/ImportRewriteAnalyzer.java 7 Jan 2010 20:18:49 -0000 1.21 >+++ dom/org/eclipse/jdt/internal/core/dom/rewrite/ImportRewriteAnalyzer.java 11 Feb 2010 00:49:21 -0000 >@@ -24,6 +24,7 @@ > import org.eclipse.jdt.core.JavaCore; > import org.eclipse.jdt.core.JavaModelException; > import org.eclipse.jdt.core.Signature; >+import org.eclipse.jdt.core.compiler.CharOperation; > import org.eclipse.jdt.core.dom.ASTNode; > import org.eclipse.jdt.core.dom.CompilationUnit; > import org.eclipse.jdt.core.dom.ImportDeclaration; >@@ -33,6 +34,7 @@ > import org.eclipse.jdt.core.search.IJavaSearchScope; > import org.eclipse.jdt.core.search.SearchEngine; > import org.eclipse.jdt.core.search.TypeNameRequestor; >+import org.eclipse.jdt.internal.core.JavaProject; > import org.eclipse.jface.text.IRegion; > import org.eclipse.jface.text.Region; > import org.eclipse.text.edits.DeleteEdit; >@@ -53,6 +55,7 @@ > private final int staticImportOnDemandThreshold; > > private boolean filterImplicitImports; >+ private boolean useContextToFilterImplicitImports; > private boolean findAmbiguousImports; > > private int flags= 0; >@@ -68,6 +71,7 @@ > this.staticImportOnDemandThreshold= staticThreshold; > > this.filterImplicitImports= true; >+ this.useContextToFilterImplicitImports= false; > this.findAmbiguousImports= true; //!restoreExistingImports; > > this.packageEntries= new ArrayList(20); >@@ -168,9 +172,55 @@ > } > } > >- private static String getQualifier(ImportDeclaration decl) { >- String name= decl.getName().getFullyQualifiedName(); >- return decl.isOnDemand() ? name : Signature.getQualifier(name); >+ private String getQualifier(ImportDeclaration decl) { >+ String name = decl.getName().getFullyQualifiedName(); >+ /* >+ * If it's on demand import, return the fully qualified name. (e.g. pack1.Foo.* => pack.Foo, pack1.* => pack1) >+ * This is because we need to have pack1.Foo.* and pack1.Bar under different qualifier groups. >+ */ >+ if (decl.isOnDemand()) { >+ return name; >+ } >+ return getQualifier(name, decl.isStatic()); >+ } >+ >+ private String getQualifier(String name, boolean isStatic) { >+ // For static imports, return the Type name as well as part of the qualifier >+ if (isStatic || !this.useContextToFilterImplicitImports) { >+ return Signature.getQualifier(name); >+ } >+ >+ char[] searchedName = name.toCharArray(); >+ int index = name.length(); >+ /* Stop at the last fragment */ >+ JavaProject project = (JavaProject) this.compilationUnit.getJavaProject(); >+ do { >+ String testedName = new String(searchedName, 0, index); >+ IJavaElement fragment = null; >+ try { >+ fragment = project.findPackageFragment(testedName); >+ } catch (JavaModelException e) { >+ return name; >+ } >+ if (fragment != null) { >+ return testedName; >+ } >+ try { >+ fragment = project.findType(testedName); >+ } catch (JavaModelException e) { >+ return name; >+ } >+ if (fragment != null) { >+ index = CharOperation.lastIndexOf(Signature.C_DOT, searchedName, 0, index - 1); >+ } else { >+ // we use the heuristic that a name starting with a lowercase is a package name >+ index = CharOperation.lastIndexOf(Signature.C_DOT, searchedName, 0, index - 1); >+ if (Character.isLowerCase(searchedName[index + 1])) { >+ return testedName; >+ } >+ } >+ } while (index >= 0); >+ return name; > } > > private static String getFullName(ImportDeclaration decl) { >@@ -238,18 +288,42 @@ > } > > /** >- * Sets that implicit imports (types in default package, CU- package and >- * 'java.lang') should not be created. Note that this is a heuristic filter and can >- * lead to missing imports, e.g. in cases where a type is forced to be specified >- * due to a name conflict. >- * By default, the filter is enabled. >- * @param filterImplicitImports The filterImplicitImports to set >+ * Specifies that implicit imports (for types in <code>java.lang</code>, types in the same package as the rewrite >+ * compilation unit and types in the compilation unit's main type) should not be created, except if necessary to >+ * resolve an on-demand import conflict. >+ * <p> >+ * The filter is enabled by default. >+ * </p> >+ * <p> >+ * Note: {@link #setUseContextToFilterImplicitImports(boolean)} can be used to filter implicit imports >+ * when a context is used. >+ * </p> >+ * >+ * @param filterImplicitImports >+ * if <code>true</code>, implicit imports will be filtered >+ * >+ * @see #setUseContextToFilterImplicitImports(boolean) > */ > public void setFilterImplicitImports(boolean filterImplicitImports) { > this.filterImplicitImports= filterImplicitImports; > } > > /** >+ * Sets whether a context should be used to properly filter implicit imports. >+ * <p> >+ * By default, the option is disabled. >+ * </p> >+ * >+ * @param useContextToFilterImplicitImports the given setting >+ * >+ * @see #setFilterImplicitImports(boolean) >+ * @since 3.6 >+ */ >+ public void setUseContextToFilterImplicitImports(boolean useContextToFilterImplicitImports) { >+ this.useContextToFilterImplicitImports = useContextToFilterImplicitImports; >+ } >+ >+ /** > * When set searches for imports that can not be folded into on-demand > * imports but must be specified explicitly > * @param findAmbiguousImports The new value >@@ -385,10 +459,11 @@ > return bestMatch; > } > >- private static boolean isImplicitImport(String qualifier, ICompilationUnit cu) { >+ private boolean isImplicitImport(String qualifier) { > if (JAVA_LANG.equals(qualifier)) { > return true; > } >+ ICompilationUnit cu= this.compilationUnit; > String packageName= cu.getParent().getElementName(); > if (qualifier.equals(packageName)) { > return true; >@@ -401,13 +476,13 @@ > } > > public void addImport(String fullTypeName, boolean isStatic) { >- String typeContainerName= Signature.getQualifier(fullTypeName); >+ String typeContainerName= getQualifier(fullTypeName, isStatic); > ImportDeclEntry decl= new ImportDeclEntry(fullTypeName, isStatic, null); > sortIn(typeContainerName, decl, isStatic); > } > > public boolean removeImport(String qualifiedName, boolean isStatic) { >- String containerName= Signature.getQualifier(qualifiedName); >+ String containerName= getQualifier(qualifiedName, isStatic); > > int nPackages= this.packageEntries.size(); > for (int i= 0; i < nPackages; i++) { >@@ -453,9 +528,11 @@ > PackageEntry packEntry= new PackageEntry(typeContainerName, group, isStatic); > packEntry.add(decl); > int index= this.packageEntries.indexOf(bestMatch); >- if (cmp < 0) { // insert ahead of best match >+ if (cmp < 0) { >+ // insert ahead of best match > this.packageEntries.add(index, packEntry); >- } else { // insert after best match >+ } else { >+ // insert after best match > this.packageEntries.add(index + 1, packEntry); > } > } >@@ -522,17 +599,14 @@ > int nPackageEntries= this.packageEntries.size(); > for (int i= 0; i < nPackageEntries; i++) { > PackageEntry pack= (PackageEntry) this.packageEntries.get(i); >- int nImports= pack.getNumberOfImports(); >- >- if (this.filterImplicitImports && !pack.isStatic() && isImplicitImport(pack.getName(), this.compilationUnit)) { >- pack.removeAllNew(onDemandConflicts); >- nImports= pack.getNumberOfImports(); >+ if (this.filterImplicitImports && !pack.isStatic() && isImplicitImport(pack.getName())) { >+ pack.filterImplicitImports(this.useContextToFilterImplicitImports); > } >+ int nImports= pack.getNumberOfImports(); > if (nImports == 0) { > continue; > } > >- > if (spacesBetweenGroups > 0) { > // add a space between two different groups by looking at the two adjacent imports > if (lastPackage != null && !pack.isComment() && !pack.isSameGroup(lastPackage)) { >@@ -928,12 +1002,21 @@ > return false; > } > >- public void removeAllNew(Set onDemandConflicts) { >+ public void filterImplicitImports(boolean useContextToFilterImplicitImports) { > int nInports= this.importEntries.size(); > for (int i= nInports - 1; i >= 0; i--) { > ImportDeclEntry curr= getImportAt(i); >- if (curr.isNew() /*&& (onDemandConflicts == null || onDemandConflicts.contains(curr.getSimpleName()))*/) { >- this.importEntries.remove(i); >+ if (curr.isNew()) { >+ if (!useContextToFilterImplicitImports) { >+ this.importEntries.remove(i); >+ } else { >+ String elementName = curr.getElementName(); >+ int lastIndexOf = elementName.lastIndexOf('.'); >+ boolean internalClassImport = lastIndexOf > getName().length(); >+ if (!internalClassImport) { >+ this.importEntries.remove(i); >+ } >+ } > } > } > } >#P org.eclipse.jdt.core.tests.model >Index: src/org/eclipse/jdt/core/tests/rewrite/describing/ImportRewriteTest.java >=================================================================== >RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/rewrite/describing/ImportRewriteTest.java,v >retrieving revision 1.15 >diff -u -r1.15 ImportRewriteTest.java >--- src/org/eclipse/jdt/core/tests/rewrite/describing/ImportRewriteTest.java 21 Jan 2010 16:46:36 -0000 1.15 >+++ src/org/eclipse/jdt/core/tests/rewrite/describing/ImportRewriteTest.java 11 Feb 2010 00:49:22 -0000 >@@ -572,6 +572,145 @@ > assertEqualString(cu.getSource(), buf.toString()); > } > >+ /** >+ * Test that the Inner class import comes in the right order (i.e. after the enclosing type's import) when re-organized >+ * >+ * @see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=194358" >+ */ >+ public void testBug194358() throws Exception { >+ >+ StringBuffer buf= new StringBuffer(); >+ buf.append("package pack1;\n"); >+ buf.append("\n"); >+ buf.append("import pack2.A;\n"); >+ buf.append("import pack2.A.Inner;\n"); >+ buf.append("import pack2.B;\n"); >+ buf.append("\n"); >+ buf.append("public class C {\n"); >+ buf.append("}\n"); >+ >+ IPackageFragment pack1= this.sourceFolder.createPackageFragment("pack1", false, null); >+ ICompilationUnit cu= pack1.createCompilationUnit("C.java", buf.toString(), false, null); >+ >+ // We need to actually make some state in the AST for the classes, to test that we can >+ // disambiguate between packages and inner classes (see the bug for details). >+ IPackageFragment pack2= this.sourceFolder.createPackageFragment("pack2", false, null); >+ ICompilationUnit aUnit= pack2.createCompilationUnit("A.java", "", false, null); >+ ICompilationUnit bUnit= pack2.createCompilationUnit("B.java", "", false, null); >+ bUnit.createType("class B {}", null, false, null); >+ >+ IType aType= aUnit.createType("class A {}", null, false, null); >+ aType.createType("class Inner {}", null, false, null); >+ String[] order= new String[] { "java" }; >+ >+ ImportRewrite imports= newImportsRewrite(cu, order, 99, 99, false); >+ imports.setUseContextToFilterImplicitImports(true); >+ imports.addImport("pack2.A"); >+ imports.addImport("pack2.B"); >+ imports.addImport("pack2.A.Inner"); >+ >+ apply(imports); >+ >+ buf= new StringBuffer(); >+ buf.append("package pack1;\n"); >+ buf.append("\n"); >+ buf.append("import pack2.A;\n"); >+ buf.append("import pack2.A.Inner;\n"); >+ buf.append("import pack2.B;\n"); >+ buf.append("\n"); >+ buf.append("public class C {\n"); >+ buf.append("}\n"); >+ assertEqualString(cu.getSource(), buf.toString()); >+ } >+ >+ /** >+ * Test that a valid inner class import is not removed even when the container >+ * class is implicitly available. This tests the case where the classes are in >+ * different compilation units. >+ * >+ * @see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=194358" >+ */ >+ public void testBug194358a() throws Exception { >+ StringBuffer buf= new StringBuffer(); >+ buf.append("package com.pack1;\n"); >+ buf.append("\n"); >+ buf.append("import com.pack1.A;\n"); >+ buf.append("import com.pack1.A.Inner;\n"); >+ buf.append("import com.pack2.B;\n"); >+ buf.append("\n"); >+ buf.append("public class C {\n"); >+ buf.append("}\n"); >+ >+ IPackageFragment pack1= this.sourceFolder.createPackageFragment("com.pack1", false, null); >+ ICompilationUnit cu= pack1.createCompilationUnit("C.java", buf.toString(), false, null); >+ ICompilationUnit aUnit= pack1.createCompilationUnit("A.java", "", false, null); >+ >+ IPackageFragment pack2= this.sourceFolder.createPackageFragment("com.pack2", false, null); >+ ICompilationUnit bUnit= pack2.createCompilationUnit("B.java", "", false, null); >+ bUnit.createType("class B {}", null, false, null); >+ IType aType= aUnit.createType("class A {}", null, false, null); >+ aType.createType("class Inner {}", null, false, null); >+ String[] order= new String[] { "java" }; >+ >+ ImportRewrite imports= newImportsRewrite(cu, order, 99, 99, false); >+ imports.setUseContextToFilterImplicitImports(false); >+ imports.addImport("com.pack1.A"); >+ imports.addImport("com.pack1.A.Inner"); >+ imports.addImport("com.pack2.B"); >+ >+ apply(imports); >+ >+ buf= new StringBuffer(); >+ buf.append("package com.pack1;\n"); >+ buf.append("\n"); >+ buf.append("import com.pack1.A.Inner;\n"); >+ buf.append("import com.pack2.B;\n"); >+ buf.append("\n"); >+ buf.append("public class C {\n"); >+ buf.append("}\n"); >+ assertEqualString(cu.getSource(), buf.toString()); >+ } >+ /** >+ * Test that the Inner type imports are not removed while organizing even though the >+ * containing class is implicitly available - for the case when both the classes are >+ * in the same compilation unit >+ * >+ * see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=235253" >+ */ >+ public void testBug235253() throws Exception { >+ StringBuffer buf= new StringBuffer(); >+ buf.append("package bug;\n"); >+ buf.append("\n"); >+ buf.append("class Bug {\n"); >+ buf.append("public void addFile(File file) {}\n"); >+ buf.append("\tinterface Proto{};\n"); >+ buf.append("}\n"); >+ buf.append("class Foo implements Proto{}"); >+ >+ IPackageFragment pack1= this.sourceFolder.createPackageFragment("bug", false, null); >+ ICompilationUnit cu= pack1.createCompilationUnit("Bug.java", buf.toString(), false, null); >+ String[] order= new String[] { "bug" , "java" }; >+ ImportRewrite imports= newImportsRewrite(cu, order, 99, 99, false); >+ imports.setUseContextToFilterImplicitImports(true); >+ imports.addImport("bug.Bug.Proto"); >+ imports.addImport("java.io.File"); >+ >+ apply(imports); >+ buf = new StringBuffer(); >+ buf.append("package bug;\n"); >+ buf.append("\n"); >+ buf.append("import bug.Bug.Proto;\n"); >+ buf.append("\n"); >+ buf.append("import java.io.File;\n"); >+ buf.append("\n"); >+ buf.append("class Bug {\n"); >+ buf.append("public void addFile(File file) {}\n"); >+ buf.append("\tinterface Proto{};\n"); >+ buf.append("}\n"); >+ buf.append("class Foo implements Proto{}"); >+ assertEqualString(cu.getSource(), buf.toString()); >+ } >+ > public void testAddStaticImports1() throws Exception { > > IPackageFragment pack1= this.sourceFolder.createPackageFragment("pack1", false, null);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 235253
:
151581
|
152493
|
157791
|
158758
| 158804