Attachment 213744 Details for Bug 357547 – Proposed patch

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Proposed patch

patch.txt (text/plain), 19.60 KB, created by Satyam Kandula

on 2012-04-09 06:50:58 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Satyam Kandula

Created: 2012-04-09 06:50:58 EDT

Size: 19.60 KB

patch

obsolete

>diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/JavaSearchBugsTests2.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/JavaSearchBugsTests2.java
>index 0052f7a..d2815ba 100644
>--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/JavaSearchBugsTests2.java
>+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/JavaSearchBugsTests2.java
>@@ -37,7 +37,7 @@
> 	}
> 	
> 	static {
>-		//TESTS_NAMES = new String[] {"testBug123836"};
>+		//TESTS_NAMES = new String[] {"testBug357547f"};
> 	}
> 
> 	public static Test suite() {
>@@ -481,4 +481,250 @@
> 			deleteProject("P");
> 		}
> 	}
>+	
>+	/**
>+	 * @bug 357547: [search] Search for method references is returning methods as overriden even if the superclass's method is only package-visible
>+	 * @test Search for a non-overriden method because of package visibility should not be found
>+	 * @see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=357547"
>+	 */
>+	public void testBug357547a() throws CoreException {
>+		IJavaProject project = null;
>+		try
>+		{
>+			project = createJavaProject("P");
>+			createFolder("/P/p1");
>+			createFile("/P/p1/B.java",
>+					"package p1;\n" +
>+					"import p2.*;\n" +
>+					"public class B extends A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+					"}\n");
>+			createFolder("/P/p2");
>+			createFile("/P/p2/A.java",
>+					"package p2;\n" +
>+					"public class A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+			  		"public long m(){\n"+
>+			  		"return new A().k();\n" +
>+			  		"}\n"+
>+					"}\n");
>+			IType type = getCompilationUnit("/P/p1/B.java").getType("B");
>+			IMethod method = type.getMethod("k", new String[]{});
>+			search(method, REFERENCES, EXACT_RULE, SearchEngine.createWorkspaceScope(), this.resultCollector);
>+			assertSearchResults("Should not get any results", "", this.resultCollector);
>+		} finally {
>+			deleteProject(project);
>+		}
>+	}
>+	
>+	// search for the method name should also not return matches if not-overriden because of package-visible
>+	public void testBug357547b() throws CoreException {
>+		IJavaProject project = null;
>+		try
>+		{
>+			project = createJavaProject("P");
>+			createFolder("/P/p1");
>+			createFile("/P/p1/B.java",
>+					"package p1;\n" +
>+					"import p2.*;\n" +
>+					"public class B extends A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+					"}\n");
>+			createFolder("/P/p2");
>+			createFile("/P/p2/A.java",
>+					"package p2;\n" +
>+					"public class A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+			  		"public long m(){\n"+
>+			  		"return new A().k();\n" +
>+			  		"}\n"+
>+					"}\n");
>+			waitUntilIndexesReady();
>+			// search
>+			SearchPattern pattern = SearchPattern.createPattern("p*B.k()", METHOD, REFERENCES, EXACT_RULE);
>+			search(pattern, SearchEngine.createJavaSearchScope(new IJavaElement[] { project }), this.resultCollector);
>+			assertSearchResults("Should not get any results", "", this.resultCollector);
>+		} finally {
>+			deleteProject(project);
>+		}
>+	}
>+	
>+	// search for the method name should return the match if same package 
>+	public void testBug357547c() throws CoreException {
>+		IJavaProject project = null;
>+		try
>+		{
>+			project = createJavaProject("P");
>+			createFolder("/P/p2");
>+			createFile("/P/p2/B.java",
>+					"package p2;\n" +
>+					"public class B extends A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+					"}\n");
>+			createFile("/P/p2/A.java",
>+					"package p2;\n" +
>+					"public class A {\n" +
>+					"public long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+			  		"public long m(){\n"+
>+			  		"return new A().k();\n" +
>+			  		"}\n"+
>+					"}\n");
>+			waitUntilIndexesReady();
>+			// search
>+			SearchPattern pattern = SearchPattern.createPattern("B.k()", METHOD, REFERENCES, EXACT_RULE);
>+			search(pattern, SearchEngine.createJavaSearchScope(new IJavaElement[] { project }), this.resultCollector);
>+			assertSearchResults("Wrong results", "p2/A.java long p2.A.m() [k()] EXACT_MATCH", this.resultCollector);
>+		} finally {
>+			deleteProject(project);
>+		}
>+	}
>+	
>+	//presence of same name in the package should not effect the result
>+	public void testBug357547d() throws CoreException {
>+		IJavaProject project = null;
>+		try
>+		{
>+			project = createJavaProject("P");
>+			createFolder("/P/p1");
>+			createFile("/P/p1/B.java",
>+					"package p1;\n" +
>+					"import p2.*;\n" +
>+					"public class B extends A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+					"}\n");
>+			createFolder("/P/p2");
>+			createFile("/P/p2/A.java",
>+					"package p2;\n" +
>+					"public class A{ \n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+			  		"public long m(){\n"+
>+			  		"return new A().k();\n" +
>+			  		"}\n"+
>+					"}\n");
>+			createFile("/P/p2/B.java",
>+					"package p2;\n" +
>+					"public class B {\n" +
>+					"}\n");
>+			waitUntilIndexesReady();
>+			// search
>+			SearchPattern pattern = SearchPattern.createPattern("B.k()", METHOD, REFERENCES, EXACT_RULE);
>+			search(pattern, SearchEngine.createJavaSearchScope(new IJavaElement[] { project }), this.resultCollector);
>+			assertSearchResults("Should not get any results", "", this.resultCollector);
>+		} finally {
>+			deleteProject(project);
>+		}
>+	}
>+	
>+	// search for the method name should also not return matches if not-overriden because of package-visible
>+	// even if they are in jars
>+	public void testBug357547e() throws CoreException, IOException {
>+		IJavaProject project = null;
>+		try
>+		{
>+			project = createJavaProject("P", new String[] {""}, new String[] { "/P/lib357547.jar", "JCL15_LIB" }, "", "1.5");
>+			org.eclipse.jdt.core.tests.util.Util.createJar(new String[] {
>+					"p2/A.java",
>+					"package p2;\n" + 
>+					"public class A{}\n" }, 
>+					project.getProject().getLocation().append("libStuff.jar").toOSString(), "1.5");
>+			
>+			org.eclipse.jdt.core.tests.util.Util.createJar(
>+					new String[] {
>+						"p1/B.java",
>+						"package p1;\n"+
>+						"import p2.*;\n"+
>+						"public class B extends A {\n" +
>+						"long k(){\n" +
>+						"return 0;\n" +
>+						"}\n" + 
>+						"}\n"},
>+					null,
>+					project.getProject().getLocation().append("lib357547.jar").toOSString(),
>+					new String[] { project.getProject().getLocation().append("libStuff.jar").toOSString() },
>+					"1.5");
>+			refresh(project);
>+			createFolder("/P/p2");
>+			createFile("/P/p2/A.java",
>+					"package p2;\n" +
>+					"public class A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+			  		"public long m(){\n"+
>+			  		"return new A().k();\n" +
>+			  		"}\n"+
>+					"}\n");
>+			waitUntilIndexesReady();
>+			// search
>+			SearchPattern pattern = SearchPattern.createPattern("B.k()", METHOD, REFERENCES, EXACT_RULE);
>+			search(pattern, SearchEngine.createJavaSearchScope(new IJavaElement[] { project }, IJavaSearchScope.APPLICATION_LIBRARIES | IJavaSearchScope.SOURCES), this.resultCollector);
>+			assertSearchResults("Wrong results", "", this.resultCollector);
>+		} finally {
>+			deleteProject(project);
>+		}
>+	}
>+	// search for the method name should also not return matches if not-overriden because of package-visible
>+	// even if they are in jars
>+	public void testBug357547f() throws CoreException, IOException {
>+		IJavaProject project = null;
>+		try
>+		{
>+			project = createJavaProject("P", new String[] {""}, new String[] { "/P/lib357547.jar", "JCL15_LIB" }, "", "1.5");
>+			org.eclipse.jdt.core.tests.util.Util.createJar(new String[] {
>+					"p2/A.java",
>+					"package p2;\n" + 
>+					"public class A{}\n" }, 
>+					project.getProject().getLocation().append("libStuff.jar").toOSString(), "1.5");
>+			
>+			org.eclipse.jdt.core.tests.util.Util.createJar(
>+					new String[] {
>+						"p2/B.java",
>+						"package p2;\n" +
>+						"import p2.*;\n" +
>+						"public class B extends A {\n" +
>+						"long k(){\n" +
>+						"return 0;\n" +
>+						"}\n" + 
>+						"}\n"},
>+					null,
>+					project.getProject().getLocation().append("lib357547.jar").toOSString(),
>+					new String[] { project.getProject().getLocation().append("libStuff.jar").toOSString() },
>+					"1.5");
>+			refresh(project);
>+			createFolder("/P/p2");
>+			createFile("/P/p2/A.java",
>+					"package p2;\n" +
>+					"public class A {\n" +
>+					"long k(){\n" +
>+					"return 0;\n" +
>+			  		"}\n" +
>+			  		"public long m(){\n"+
>+			  		"return new A().k();\n" +
>+			  		"}\n"+
>+					"}\n");
>+			waitUntilIndexesReady();
>+			// search
>+			SearchPattern pattern = SearchPattern.createPattern("B.k()", METHOD, REFERENCES, EXACT_RULE);
>+			search(pattern, SearchEngine.createJavaSearchScope(new IJavaElement[] { project }, IJavaSearchScope.APPLICATION_LIBRARIES | IJavaSearchScope.SOURCES), this.resultCollector);
>+			assertSearchResults("Wrong results", "p2/A.java long p2.A.m() [k()] EXACT_MATCH", this.resultCollector);
>+		} finally {
>+			deleteProject(project);
>+		}
>+	}
> }
>diff --git a/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/MethodLocator.java b/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/MethodLocator.java
>index 9b48938..bc1fa92 100644
>--- a/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/MethodLocator.java
>+++ b/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/MethodLocator.java
>@@ -31,9 +31,17 @@
> //extra reference info
> public char[][][] allSuperDeclaringTypeNames;
> 
>+// This is set only if focus is null. In these cases
>+// it will be hard to determine if the super class is of the same package
>+// at a latter point. Hence, this array is created with all the super class 
>+// names of the same package name as of the matching class name.
>+// See https://bugs.eclipse.org/bugs/show_bug.cgi?id=357547
>+private char[][][] samePkgSuperDeclaringTypeNames;
>+
> private MatchLocator matchLocator;
> //method declarations which parameters verification fail
> private HashMap methodDeclarationsWithInvalidParam = new HashMap();
>+
> 
> public MethodLocator(MethodPattern pattern) {
> 	super(pattern);
>@@ -75,14 +83,16 @@
> 		start = System.currentTimeMillis();
> 	}
> 	try {
>-		this.allSuperDeclaringTypeNames =
>+		SuperTypeNamesCollector namesCollector = 
> 			new SuperTypeNamesCollector(
> 				this.pattern,
> 				this.pattern.declaringSimpleName,
> 				this.pattern.declaringQualification,
> 				locator,
> 				this.pattern.declaringType,
>-				locator.progressMonitor).collect();
>+				locator.progressMonitor);
>+		this.allSuperDeclaringTypeNames = namesCollector.collect();
>+		this.samePkgSuperDeclaringTypeNames = namesCollector.getSamePackageSuperTypeNames();
> 		this.matchLocator = locator;	
> 	} catch (JavaModelException e) {
> 		// inaccurate matches will be found
>@@ -109,7 +119,9 @@
>  * this message send or not.
>  */
> protected boolean isVirtualInvoke(MethodBinding method, MessageSend messageSend) {
>-	return !method.isStatic() && !method.isPrivate() && !messageSend.isSuperAccess();
>+		return !method.isStatic() && !method.isPrivate() && !messageSend.isSuperAccess()
>+				&& !(method.isDefault() && this.pattern.focus != null && 
>+				!CharOperation.equals(this.pattern.declaringQualification, method.declaringClass.qualifiedPackageName()));
> }
> public int match(ASTNode node, MatchingNodeSet nodeSet) {
> 	int declarationsLevel = IMPOSSIBLE_MATCH;
>@@ -669,9 +681,10 @@
> 			if (method.declaringClass == null || this.allSuperDeclaringTypeNames == null) {
> 				declaringLevel = INACCURATE_MATCH;
> 			} else {
>-				if (resolveLevelAsSuperInvocation(methodReceiverType, method.parameters, true)) {
>-					declaringLevel = methodLevel // since this is an ACCURATE_MATCH so return the possibly weaker match
>-						| SUPER_INVOCATION_FLAVOR; // this is an overridden method => add flavor to returned level
>+				char[][][] superTypeNames = (method.isDefault() && this.pattern.focus == null) ? this.samePkgSuperDeclaringTypeNames: this.allSuperDeclaringTypeNames;
>+				if (superTypeNames != null && resolveLevelAsSuperInvocation(methodReceiverType, method.parameters, superTypeNames, true)) {
>+						declaringLevel = methodLevel // since this is an ACCURATE_MATCH so return the possibly weaker match
>+							| SUPER_INVOCATION_FLAVOR; // this is an overridden method => add flavor to returned level
> 				}
> 			}
> 		}
>@@ -745,10 +758,10 @@
>  * Return whether the given type binding or one of its possible super interfaces
>  * matches a type in the declaring type names hierarchy.
>  */
>-private boolean resolveLevelAsSuperInvocation(ReferenceBinding type, TypeBinding[] argumentTypes, boolean methodAlreadyVerified) {
>+private boolean resolveLevelAsSuperInvocation(ReferenceBinding type, TypeBinding[] argumentTypes, char[][][] superTypeNames, boolean methodAlreadyVerified) {
> 	char[][] compoundName = type.compoundName;
>-	for (int i = 0, max = this.allSuperDeclaringTypeNames.length; i < max; i++) {
>-		if (CharOperation.equals(this.allSuperDeclaringTypeNames[i], compoundName)) {
>+	for (int i = 0, max = superTypeNames.length; i < max; i++) {
>+		if (CharOperation.equals(superTypeNames[i], compoundName)) {
> 			// need to verify if the type implements the pattern method
> 			if (methodAlreadyVerified) return true; // already verified before enter into this method (see resolveLevel(MessageSend))
> 			MethodBinding[] methods = type.getMethods(this.pattern.selector);
>@@ -780,7 +793,7 @@
> 		ReferenceBinding[] interfaces = type.superInterfaces();
> 		if (interfaces == null) return false;
> 		for (int i = 0; i < interfaces.length; i++) {
>-			if (resolveLevelAsSuperInvocation(interfaces[i], argumentTypes, false)) {
>+			if (resolveLevelAsSuperInvocation(interfaces[i], argumentTypes, superTypeNames, false)) {
> 				return true;
> 			}
> 		}
>diff --git a/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/SuperTypeNamesCollector.java b/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/SuperTypeNamesCollector.java
>index 8eae870..7fc6f1f 100644
>--- a/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/SuperTypeNamesCollector.java
>+++ b/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/SuperTypeNamesCollector.java
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2008 IBM Corporation and others.
>+ * Copyright (c) 2000, 2012 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -13,7 +13,6 @@
> import org.eclipse.core.runtime.IProgressMonitor;
> import org.eclipse.core.runtime.SubProgressMonitor;
> import org.eclipse.jdt.core.*;
>-import org.eclipse.jdt.core.ICompilationUnit;
> import org.eclipse.jdt.core.compiler.CharOperation;
> import org.eclipse.jdt.core.search.*;
> import org.eclipse.jdt.internal.compiler.ASTVisitor;
>@@ -45,19 +44,19 @@
> 		public boolean visit(TypeDeclaration typeDeclaration, BlockScope scope) {
> 			ReferenceBinding binding = typeDeclaration.binding;
> 			if (SuperTypeNamesCollector.this.matches(binding))
>-				collectSuperTypeNames(binding);
>+				collectSuperTypeNames(binding, binding.compoundName);
> 			return true;
> 		}
> 		public boolean visit(TypeDeclaration typeDeclaration, CompilationUnitScope scope) {
> 			ReferenceBinding binding = typeDeclaration.binding;
> 			if (SuperTypeNamesCollector.this.matches(binding))
>-				collectSuperTypeNames(binding);
>+				collectSuperTypeNames(binding, binding.compoundName);
> 			return true;
> 		}
> 		public boolean visit(TypeDeclaration memberTypeDeclaration, ClassScope scope) {
> 			ReferenceBinding binding = memberTypeDeclaration.binding;
> 			if (SuperTypeNamesCollector.this.matches(binding))
>-				collectSuperTypeNames(binding);
>+				collectSuperTypeNames(binding, binding.compoundName);
> 			return true;
> 		}
> 		public boolean visit(FieldDeclaration fieldDeclaration, MethodScope scope) {
>@@ -82,6 +81,9 @@
> char[][][] result;
> int resultIndex;
> 
>+char[][][] samePackageSuperTypeName; // set only if focus is null
>+int samePackageIndex;
>+
> public SuperTypeNamesCollector(
> 	SearchPattern pattern,
> 	char[] typeSimpleName,
>@@ -98,6 +100,21 @@
> 	this.progressMonitor = progressMonitor;
> }
> 
>+private boolean addIfSamePackage(char[][] compoundName, char[][] path) {
>+	if (compoundName.length != path.length) return false;
>+	int resultLength = this.samePackageSuperTypeName.length;
>+	for (int i = 0; i < resultLength; i++)
>+		if (CharOperation.equals(this.samePackageSuperTypeName[i], compoundName)) return false; // already known
>+	
>+	for (int i = 0, length = compoundName.length - 1; i < length; i ++) {
>+		if (!CharOperation.equals(compoundName[i], path[i])) return false;
>+	}
>+	if (resultLength == this.samePackageIndex)
>+		System.arraycopy(this.samePackageSuperTypeName, 0, this.samePackageSuperTypeName = new char[resultLength*2][][], 0, resultLength);
>+	this.samePackageSuperTypeName[this.samePackageIndex++] = compoundName;
>+	return true;
>+}
>+
> protected void addToResult(char[][] compoundName) {
> 	int resultLength = this.result.length;
> 	for (int i = 0; i < resultLength; i++)
>@@ -107,6 +124,7 @@
> 		System.arraycopy(this.result, 0, this.result = new char[resultLength*2][][], 0, resultLength);
> 	this.result[this.resultIndex++] = compoundName;
> }
>+
> /*
>  * Parse the given compiation unit and build its type bindings.
>  */
>@@ -141,7 +159,7 @@
> 			if (this.type.isBinary()) {
> 				BinaryTypeBinding binding = this.locator.cacheBinaryType(this.type, null);
> 				if (binding != null)
>-					collectSuperTypeNames(binding);
>+					collectSuperTypeNames(binding, null);
> 			} else {
> 				ICompilationUnit unit = this.type.getCompilationUnit();
> 				SourceType sourceType = (SourceType) this.type;
>@@ -150,7 +168,7 @@
> 				if (parsedUnit != null) {
> 					TypeDeclaration typeDecl = new ASTNodeFinder(parsedUnit).findType(this.type);
> 					if (typeDecl != null && typeDecl.binding != null)
>-						collectSuperTypeNames(typeDecl.binding);
>+						collectSuperTypeNames(typeDecl.binding, null);
> 				}
> 			}
> 		} catch (AbortCompilation e) {
>@@ -171,6 +189,7 @@
> 	Util.sort(paths); // sort by projects
> 	JavaProject previousProject = null;
> 	this.result = new char[1][][];
>+	this.samePackageSuperTypeName = new char[1][][];
> 	this.resultIndex = 0;
> 	for (int i = 0, length = paths.length; i < length; i++) {
> 		try {
>@@ -191,7 +210,7 @@
> 				IClassFile classFile = (IClassFile) openable;
> 				BinaryTypeBinding binding = this.locator.cacheBinaryType(classFile.getType(), null);
> 				if (matches(binding))
>-					collectSuperTypeNames(binding);
>+					collectSuperTypeNames(binding, binding.compoundName);
> 			}
> 		} catch (AbortCompilation e) {
> 			// ignore: continue with next element
>@@ -206,11 +225,15 @@
> /**
>  * Collects the names of all the supertypes of the given type.
>  */
>-protected void collectSuperTypeNames(ReferenceBinding binding) {
>+protected void collectSuperTypeNames(ReferenceBinding binding, char[][] path) {
> 	ReferenceBinding superclass = binding.superclass();
>+	if (path != null) {
>+		boolean samePackage = addIfSamePackage(superclass.compoundName, path);
>+		if (!samePackage) path = null;
>+	}
> 	if (superclass != null) {
> 		addToResult(superclass.compoundName);
>-		collectSuperTypeNames(superclass);
>+		collectSuperTypeNames(superclass, null);
> 	}
> 
> 	ReferenceBinding[] interfaces = binding.superInterfaces();
>@@ -218,7 +241,7 @@
> 		for (int i = 0; i < interfaces.length; i++) {
> 			ReferenceBinding interfaceBinding = interfaces[i];
> 			addToResult(interfaceBinding.compoundName);
>-			collectSuperTypeNames(interfaceBinding);
>+			collectSuperTypeNames(interfaceBinding, null);
> 		}
> 	}
> }
>@@ -254,6 +277,9 @@
> 		this.progressMonitor == null ? null : new SubProgressMonitor(this.progressMonitor, 100));
> 	return pathCollector.getPaths();
> }
>+public char[][][] getSamePackageSuperTypeNames() {
>+	return this.samePackageSuperTypeName;
>+}
> protected boolean matches(char[][] compoundName) {
> 	int length = compoundName.length;
> 	if (length == 0) return false;

Actions: View | Diff

Attachments on bug 357547: 211840 | 213744 | 214284 | 214290