Attachment 152716 Details for Bug 194358 – Proposed fix

[patch] Proposed fix

patch_235253_3.txt (text/plain), 15.31 KB, created by Olivier Thomann

on 2009-11-20 10:22:13 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Olivier Thomann

Created: 2009-11-20 10:22:13 EST

Size: 15.31 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>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.17
>diff -u -r1.17 ImportRewriteAnalyzer.java
>--- dom/org/eclipse/jdt/internal/core/dom/rewrite/ImportRewriteAnalyzer.java	16 Nov 2009 15:07:33 -0000	1.17
>+++ dom/org/eclipse/jdt/internal/core/dom/rewrite/ImportRewriteAnalyzer.java	19 Nov 2009 03:01:47 -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;
>@@ -168,9 +170,41 @@
> 		}
> 	}
> 
>-	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(Signature.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) {
>+			return Signature.getQualifier(name);
>+		}
>+
>+		char[] searchedName = name.toCharArray();
>+		int index = name.length();
>+		/* Stop at the last fragment */
>+		do {
>+			String potentialPackageName = new String(searchedName, 0, index);
>+			IJavaElement fragment = null;
>+			try {
>+				fragment = ((JavaProject) this.compilationUnit.getJavaProject()).findPackageFragment(potentialPackageName);
>+			} catch (JavaModelException e) {
>+				return name;
>+			}
>+			if (fragment != null) {
>+				return potentialPackageName;
>+			}
>+			index = CharOperation.lastIndexOf(Signature.C_DOT, searchedName, 0, index - 1);
>+		} while (index >= 0); 
>+		return name;
> 	}
> 
> 	private static String getFullName(ImportDeclaration decl) {
>@@ -390,24 +424,17 @@
> 			return true;
> 		}
> 		String packageName= cu.getParent().getElementName();
>-		if (qualifier.equals(packageName)) {
>-			return true;
>-		}
>-		String mainTypeName= JavaCore.removeJavaLikeExtension(cu.getElementName());
>-		if (packageName.length() == 0) {
>-			return qualifier.equals(mainTypeName);
>-		}
>-		return qualifier.equals(packageName +'.' + mainTypeName);
>+		return qualifier.equals(packageName);
> 	}
> 
> 	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 +480,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,12 +551,12 @@
> 			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();
>+					pack.filterImplicitImports();
> 				}
>+				int nImports= pack.getNumberOfImports();
>+
> 				if (nImports == 0) {
> 					continue;
> 				}
>@@ -930,12 +959,17 @@
> 			return false;
> 		}
> 
>-		public void removeAllNew(Set onDemandConflicts) {
>+		public void filterImplicitImports() {
> 			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);
>+				String elementName = curr.getElementName();
>+				int lastIndexOf = elementName.lastIndexOf('.');
>+				boolean internalClassImport = lastIndexOf > getName().length();
>+				if (curr.isNew()) {
>+					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.14
>diff -u -r1.14 ImportRewriteTest.java
>--- src/org/eclipse/jdt/core/tests/rewrite/describing/ImportRewriteTest.java	16 Nov 2009 15:07:36 -0000	1.14
>+++ src/org/eclipse/jdt/core/tests/rewrite/describing/ImportRewriteTest.java	19 Nov 2009 03:01:48 -0000
>@@ -61,7 +61,7 @@
> 
> 	protected void setUp() throws Exception {
> 		super.setUp();
>-
>+	
> 		IJavaProject proj= createJavaProject("P", new String[] {"src"}, new String[] {"JCL_LIB"}, "bin");
> 		proj.setOption(DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR, JavaCore.SPACE);
> 		proj.setOption(DefaultCodeFormatterConstants.FORMATTER_TAB_SIZE, "4");
>@@ -70,11 +70,14 @@
> 		proj.setOption(JavaCore.COMPILER_SOURCE, JavaCore.VERSION_1_5);
> 		proj.setOption(JavaCore.COMPILER_CODEGEN_TARGET_PLATFORM, JavaCore.VERSION_1_5);
> 		proj.setOption(DefaultCodeFormatterConstants.FORMATTER_NUMBER_OF_EMPTY_LINES_TO_PRESERVE, String.valueOf(99));
>-
>+	
> 		proj.setOption(DefaultCodeFormatterConstants.FORMATTER_BLANK_LINES_BETWEEN_IMPORT_GROUPS, String.valueOf(1));
>-
>-
>+	
> 		this.sourceFolder = getPackageFragmentRoot("P", "src");
>+		this.sourceFolder.createPackageFragment("java.util", false, null);
>+		this.sourceFolder.createPackageFragment("java.net", false, null);
>+		this.sourceFolder.createPackageFragment("java.awt", false, null);
>+		this.sourceFolder.createPackageFragment("pack", false, null);
> 
> 		waitUntilIndexesReady();
> 	}
>@@ -469,8 +472,8 @@
> 		buf= new StringBuffer();
> 		buf.append("package pack1;\n");
> 		buf.append("\n");
>-		buf.append("import p.Inner;\n");
> 		buf.append("import p.A.*;\n");
>+		buf.append("import p.Inner;\n");
> 		buf.append("\n");
> 		buf.append("public class C {\n");
> 		buf.append("}\n");
>@@ -572,6 +575,142 @@
> 		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.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.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.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);
>#P org.eclipse.jdt.ui.tests
>Index: ui/org/eclipse/jdt/ui/tests/core/AddImportTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/core/AddImportTest.java,v
>retrieving revision 1.37
>diff -u -r1.37 AddImportTest.java
>--- ui/org/eclipse/jdt/ui/tests/core/AddImportTest.java	14 Sep 2009 10:37:34 -0000	1.37
>+++ ui/org/eclipse/jdt/ui/tests/core/AddImportTest.java	19 Nov 2009 03:01:49 -0000
>@@ -93,6 +93,7 @@
> 		IPackageFragmentRoot sourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");
> 
> 		IPackageFragment pack1= sourceFolder.createPackageFragment("pack1", false, null);
>+		
> 		StringBuffer buf= new StringBuffer();
> 		buf.append("package pack1;\n");
> 		buf.append("\n");
>@@ -204,6 +205,8 @@
> 
> 	public void testRemoveImports1() throws Exception {
> 		IPackageFragmentRoot sourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");
>+		sourceFolder.createPackageFragment("java.util", false, null);
>+		sourceFolder.createPackageFragment("pack", false, null);
> 
> 		IPackageFragment pack1= sourceFolder.createPackageFragment("pack1", false, null);
> 		StringBuffer buf= new StringBuffer();
>@@ -296,8 +299,8 @@
> 		buf= new StringBuffer();
> 		buf.append("package pack1;\n");
> 		buf.append("\n");
>-		buf.append("import p.Inner;\n");
> 		buf.append("import p.A.*;\n");
>+		buf.append("import p.Inner;\n");
> 		buf.append("\n");
> 		buf.append("public class C {\n");
> 		buf.append("}\n");
>@@ -308,6 +311,10 @@
> 		IPackageFragmentRoot sourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");
> 
> 		IPackageFragment pack1= sourceFolder.createPackageFragment("pack1", false, null);
>+		sourceFolder.createPackageFragment("java.awt", false, null);
>+		sourceFolder.createPackageFragment("java.applet", false, null);
>+		sourceFolder.createPackageFragment("java.math", false, null);
>+		
> 		StringBuffer buf= new StringBuffer();
> 		buf.append("package pack1;\n");
> 		buf.append("\n");

Actions: View | Diff

Attachments on bug 194358: 122186 | 148389 | 148758 | 149099 | 149218 | 151351 | 151446 | 151922 | 152135 | 152716