Attachment 154492 Details for Bug 235783 – Proposed fix + regression tests

[patch] Proposed fix + regression tests

patch_235783.txt (text/plain), 16.67 KB, created by Olivier Thomann

on 2009-12-15 11:32:01 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Olivier Thomann

Created: 2009-12-15 11:32:01 EST

Size: 16.67 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: eval/org/eclipse/jdt/internal/eval/CodeSnippetAllocationExpression.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/eval/org/eclipse/jdt/internal/eval/CodeSnippetAllocationExpression.java,v
>retrieving revision 1.40
>diff -u -r1.40 CodeSnippetAllocationExpression.java
>--- eval/org/eclipse/jdt/internal/eval/CodeSnippetAllocationExpression.java	17 Sep 2008 11:11:09 -0000	1.40
>+++ eval/org/eclipse/jdt/internal/eval/CodeSnippetAllocationExpression.java	15 Dec 2009 16:33:58 -0000
>@@ -13,6 +13,10 @@
> import org.eclipse.jdt.internal.compiler.ast.AllocationExpression;
> import org.eclipse.jdt.internal.compiler.ast.CastExpression;
> import org.eclipse.jdt.internal.compiler.ast.Expression;
>+import org.eclipse.jdt.internal.compiler.ast.ParameterizedQualifiedTypeReference;
>+import org.eclipse.jdt.internal.compiler.ast.TypeReference;
>+import org.eclipse.jdt.internal.compiler.ast.Wildcard;
>+import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
> import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
> import org.eclipse.jdt.internal.compiler.codegen.Opcodes;
> import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
>@@ -119,6 +123,48 @@
> 	// Propagate the type checking to the arguments, and check if the constructor is defined.
> 	this.constant = Constant.NotAConstant;
> 	this.resolvedType = this.type.resolveType(scope, true /* check bounds*/); // will check for null after args are resolved
>+	checkParameterizedAllocation: {
>+		if (this.type instanceof ParameterizedQualifiedTypeReference) { // disallow new X<String>.Y<Integer>()
>+			ReferenceBinding currentType = (ReferenceBinding)this.resolvedType;
>+			if (currentType == null) return currentType;
>+			do {
>+				// isStatic() is answering true for toplevel types
>+				if ((currentType.modifiers & ClassFileConstants.AccStatic) != 0) break checkParameterizedAllocation;
>+				if (currentType.isRawType()) break checkParameterizedAllocation;
>+			} while ((currentType = currentType.enclosingType())!= null);
>+			ParameterizedQualifiedTypeReference qRef = (ParameterizedQualifiedTypeReference) this.type;
>+			for (int i = qRef.typeArguments.length - 2; i >= 0; i--) {
>+				if (qRef.typeArguments[i] != null) {
>+					scope.problemReporter().illegalQualifiedParameterizedTypeAllocation(this.type, this.resolvedType);
>+					break;
>+				}
>+			}
>+		}
>+	}
>+
>+	// resolve type arguments (for generic constructor call)
>+	if (this.typeArguments != null) {
>+		int length = this.typeArguments.length;
>+		boolean argHasError = scope.compilerOptions().sourceLevel < ClassFileConstants.JDK1_5;
>+		this.genericTypeArguments = new TypeBinding[length];
>+		for (int i = 0; i < length; i++) {
>+			TypeReference typeReference = this.typeArguments[i];
>+			if ((this.genericTypeArguments[i] = typeReference.resolveType(scope, true /* check bounds*/)) == null) {
>+				argHasError = true;
>+			}
>+			if (argHasError && typeReference instanceof Wildcard) {
>+				scope.problemReporter().illegalUsageOfWildcard(typeReference);
>+			}
>+		}
>+		if (argHasError) {
>+			if (this.arguments != null) { // still attempt to resolve arguments
>+				for (int i = 0, max = this.arguments.length; i < max; i++) {
>+					this.arguments[i].resolveType(scope);
>+				}
>+			}
>+			return null;
>+		}
>+	}
> 
> 	// buffering the arguments' types
> 	boolean argsContainCast = false;
>@@ -204,8 +250,8 @@
> 	}
> 	if (this.arguments != null) {
> 		for (int i = 0; i < this.arguments.length; i++) {
>-		    TypeBinding parameterType = this.binding.parameters[i];
>-		    TypeBinding argumentType = argumentTypes[i];
>+			TypeBinding parameterType = this.binding.parameters[i];
>+			TypeBinding argumentType = argumentTypes[i];
> 			this.arguments[i].computeConversion(scope, parameterType, argumentType);
> 			if (argumentType.needsUncheckedConversion(parameterType)) {
> 				scope.problemReporter().unsafeTypeConversion(this.arguments[i], argumentType, parameterType);
>@@ -216,7 +262,10 @@
> 		}
> 	}
> 	if (allocatedType.isRawType() && this.binding.hasSubstitutedParameters()) {
>-	    scope.problemReporter().unsafeRawInvocation(this, this.binding);
>+		scope.problemReporter().unsafeRawInvocation(this, this.binding);
>+	}
>+	if (this.typeArguments != null && this.binding.original().typeVariables == Binding.NO_TYPE_VARIABLES) {
>+		scope.problemReporter().unnecessaryTypeArgumentsForMethodInvocation(this.binding, this.genericTypeArguments, this.typeArguments);
> 	}
> 	return allocatedType;
> }
>Index: eval/org/eclipse/jdt/internal/eval/CodeSnippetParser.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/eval/org/eclipse/jdt/internal/eval/CodeSnippetParser.java,v
>retrieving revision 1.69
>diff -u -r1.69 CodeSnippetParser.java
>--- eval/org/eclipse/jdt/internal/eval/CodeSnippetParser.java	22 Sep 2009 15:37:31 -0000	1.69
>+++ eval/org/eclipse/jdt/internal/eval/CodeSnippetParser.java	15 Dec 2009 16:33:58 -0000
>@@ -84,6 +84,59 @@
> 		this.astLengthPtr--;
> 	}
> }
>+protected void consumeClassInstanceCreationExpressionWithTypeArguments() {
>+	// ClassInstanceCreationExpression ::= 'new' TypeArguments ClassType '(' ArgumentListopt ')' ClassBodyopt
>+	AllocationExpression alloc;
>+	int length;
>+	if (((length = this.astLengthStack[this.astLengthPtr--]) == 1)
>+		&& (this.astStack[this.astPtr] == null)) {
>+		//NO ClassBody
>+		this.astPtr--;
>+		alloc = new CodeSnippetAllocationExpression(this.evaluationContext);
>+		alloc.sourceEnd = this.endPosition; //the position has been stored explicitly
>+
>+		if ((length = this.expressionLengthStack[this.expressionLengthPtr--]) != 0) {
>+			this.expressionPtr -= length;
>+			System.arraycopy(
>+				this.expressionStack,
>+				this.expressionPtr + 1,
>+				alloc.arguments = new Expression[length],
>+				0,
>+				length);
>+		}
>+		alloc.type = getTypeReference(0);
>+
>+		length = this.genericsLengthStack[this.genericsLengthPtr--];
>+		this.genericsPtr -= length;
>+		System.arraycopy(this.genericsStack, this.genericsPtr + 1, alloc.typeArguments = new TypeReference[length], 0, length);
>+		this.intPtr--;
>+
>+		//the default constructor with the correct number of argument
>+		//will be created and added by the TC (see createsInternalConstructorWithBinding)
>+		alloc.sourceStart = this.intStack[this.intPtr--];
>+		pushOnExpressionStack(alloc);
>+	} else {
>+		dispatchDeclarationInto(length);
>+		TypeDeclaration anonymousTypeDeclaration = (TypeDeclaration)this.astStack[this.astPtr];
>+		anonymousTypeDeclaration.declarationSourceEnd = this.endStatementPosition;
>+		anonymousTypeDeclaration.bodyEnd = this.endStatementPosition;
>+		if (length == 0 && !containsComment(anonymousTypeDeclaration.bodyStart, anonymousTypeDeclaration.bodyEnd)) {
>+			anonymousTypeDeclaration.bits |= ASTNode.UndocumentedEmptyBlock;
>+		}
>+		this.astPtr--;
>+		this.astLengthPtr--;
>+
>+		QualifiedAllocationExpression allocationExpression = anonymousTypeDeclaration.allocation;
>+		if (allocationExpression != null) {
>+			allocationExpression.sourceEnd = this.endStatementPosition;
>+			// handle type arguments
>+			length = this.genericsLengthStack[this.genericsLengthPtr--];
>+			this.genericsPtr -= length;
>+			System.arraycopy(this.genericsStack, this.genericsPtr + 1, allocationExpression.typeArguments = new TypeReference[length], 0, length);
>+			allocationExpression.sourceStart = this.intStack[this.intPtr--];
>+		}
>+	}
>+}
> protected void consumeClassDeclaration() {
> 	super.consumeClassDeclaration();
> 	/* recovery */
>@@ -370,7 +423,35 @@
> 		super.consumeMethodInvocationName();
> 	}
> }
>+protected void consumeMethodInvocationNameWithTypeArguments() {
>+	// MethodInvocation ::= Name '.' TypeArguments 'Identifier' '(' ArgumentListopt ')'
>+
>+	// when the name is only an identifier...we have a message send to "this" (implicit)
>+	if (this.scanner.startPosition >= this.codeSnippetStart
>+			&& this.scanner.startPosition <= this.codeSnippetEnd + 1 + this.lineSeparatorLength // 14838
>+			&& isTopLevelType()) {
> 
>+	
>+		MessageSend m = newMessageSendWithTypeArguments();
>+		m.sourceEnd = this.rParenPos;
>+		m.sourceStart =
>+			(int) ((m.nameSourcePosition = this.identifierPositionStack[this.identifierPtr]) >>> 32);
>+		m.selector = this.identifierStack[this.identifierPtr--];
>+		this.identifierLengthPtr--;
>+	
>+		// handle type arguments
>+		int length = this.genericsLengthStack[this.genericsLengthPtr--];
>+		this.genericsPtr -= length;
>+		System.arraycopy(this.genericsStack, this.genericsPtr + 1, m.typeArguments = new TypeReference[length], 0, length);
>+		this.intPtr--;
>+	
>+		m.receiver = getUnspecifiedReference();
>+		m.sourceStart = m.receiver.sourceStart;
>+		pushOnExpressionStack(m);
>+	} else {
>+		super.consumeMethodInvocationNameWithTypeArguments();
>+	}
>+}
> protected void consumeMethodInvocationSuper() {
> 	// MethodInvocation ::= 'super' '.' 'Identifier' '(' ArgumentListopt ')'
> 
>@@ -383,6 +464,25 @@
> 	m.receiver = new CodeSnippetSuperReference(m.sourceStart, this.endPosition);
> 	pushOnExpressionStack(m);
> }
>+protected void consumeMethodInvocationSuperWithTypeArguments() {
>+	// MethodInvocation ::= 'super' '.' TypeArguments 'Identifier' '(' ArgumentListopt ')'
>+
>+	MessageSend m = newMessageSendWithTypeArguments();
>+	this.intPtr--; // start position of the typeArguments
>+	m.sourceEnd = this.rParenPos;
>+	m.nameSourcePosition = this.identifierPositionStack[this.identifierPtr];
>+	m.selector = this.identifierStack[this.identifierPtr--];
>+	this.identifierLengthPtr--;
>+
>+	// handle type arguments
>+	int length = this.genericsLengthStack[this.genericsLengthPtr--];
>+	this.genericsPtr -= length;
>+	System.arraycopy(this.genericsStack, this.genericsPtr + 1, m.typeArguments = new TypeReference[length], 0, length);
>+	m.sourceStart = this.intStack[this.intPtr--]; // start position of the super keyword
>+
>+	m.receiver = new CodeSnippetSuperReference(m.sourceStart, this.endPosition);
>+	pushOnExpressionStack(m);
>+}
> protected void consumePrimaryNoNewArrayThis() {
> 	// PrimaryNoNewArray ::= 'this'
> 
>@@ -679,6 +779,22 @@
> 	}
> 	return m;
> }
>+protected MessageSend newMessageSendWithTypeArguments() {
>+	// '(' ArgumentListopt ')'
>+	// the arguments are on the expression stack
>+	CodeSnippetMessageSend m = new CodeSnippetMessageSend(this.evaluationContext);
>+	int length;
>+	if ((length = this.expressionLengthStack[this.expressionLengthPtr--]) != 0) {
>+		this.expressionPtr -= length;
>+		System.arraycopy(
>+			this.expressionStack,
>+			this.expressionPtr + 1,
>+			m.arguments = new Expression[length],
>+			0,
>+			length);
>+	}
>+	return m;
>+}
> /**
>  * Records the scanner position if we're parsing a top level type.
>  */
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/eval/DebugEvaluationTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/eval/DebugEvaluationTest.java,v
>retrieving revision 1.43
>diff -u -r1.43 DebugEvaluationTest.java
>--- src/org/eclipse/jdt/core/tests/eval/DebugEvaluationTest.java	27 Aug 2009 15:26:58 -0000	1.43
>+++ src/org/eclipse/jdt/core/tests/eval/DebugEvaluationTest.java	15 Dec 2009 16:33:59 -0000
>@@ -2782,6 +2782,163 @@
> 		removeTempClass("A62");
> 	}
> }
>+public void test065() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5) return;
>+	try {
>+		String sourceA65 =
>+			"public class A65<T> {\n"
>+				+ "\tprivate int i;\n"
>+				+ "\tpublic <U>A65() {;\n"
>+				+ "\t}\n"
>+				+ "\tprivate <U>A65(int i) {;\n"
>+				+ "\t\tthis.i = i;\n"
>+				+ "\t}\n"               
>+				+ "\tpublic void bar() {\n"
>+				+ "\t}\n"
>+				+ "}";
>+		compileAndDeploy15(sourceA65, "A65");
>+
>+		String userCode = "new <Object>A65<Object>().bar();";
>+		JDIStackFrame stackFrame =
>+			new JDIStackFrame(this.jdiVM, this, userCode, "A65", "bar", -1);
>+
>+		DebugRequestor requestor = new DebugRequestor();
>+		char[] snippet = "return new <Object>A65<Object>(3).i;".toCharArray();
>+		try {
>+			this.context.evaluate(
>+				snippet,
>+				stackFrame.localVariableTypeNames(),
>+				stackFrame.localVariableNames(),
>+				stackFrame.localVariableModifiers(),
>+				stackFrame.declaringTypeName(),
>+				stackFrame.isStatic(),
>+				stackFrame.isConstructorCall(),
>+				getEnv(),
>+				getCompilerOptions(),
>+				requestor,
>+				getProblemFactory());
>+		} catch (InstallException e) {
>+			assertTrue("No targetException " + e.getMessage(), false);
>+		}
>+		assertTrue(
>+			"Should get one result but got " + (requestor.resultIndex + 1),
>+			requestor.resultIndex == 0);
>+		EvaluationResult result = requestor.results[0];
>+		assertTrue("Code snippet should not have problems", !result.hasProblems());
>+		assertTrue("Result should have a value", result.hasValue());
>+		assertEquals("Value", "3".toCharArray(), result.getValueDisplayString());
>+		assertEquals("Type", "int".toCharArray(), result.getValueTypeName());
>+	} finally {
>+		removeTempClass("A65");
>+	}
>+}
>+public void test066() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5) return;
>+	try {
>+		String sourceA66 =
>+			"public class A66 {\n"
>+				+ "\tprivate int i;\n"
>+				+ "\tpublic A66() {;\n"
>+				+ "\t}\n"
>+				+ "\tprivate <U> int foo(int i) {;\n"
>+				+ "\t\treturn i;\n"    
>+				+ "\t}\n"               
>+				+ "\tpublic void bar() {\n"
>+				+ "\t}\n"
>+				+ "}";
>+		compileAndDeploy15(sourceA66, "A66");
>+
>+		String userCode = "new A66().bar();";
>+		JDIStackFrame stackFrame =
>+			new JDIStackFrame(this.jdiVM, this, userCode, "A66", "bar", -1);
>+
>+		DebugRequestor requestor = new DebugRequestor();
>+		char[] snippet = "return this.<Object>foo(3);".toCharArray();
>+		try {
>+			this.context.evaluate(
>+				snippet,
>+				stackFrame.localVariableTypeNames(),
>+				stackFrame.localVariableNames(),
>+				stackFrame.localVariableModifiers(),
>+				stackFrame.declaringTypeName(),
>+				stackFrame.isStatic(),
>+				stackFrame.isConstructorCall(),
>+				getEnv(),
>+				getCompilerOptions(),
>+				requestor,
>+				getProblemFactory());
>+		} catch (InstallException e) {
>+			assertTrue("No targetException " + e.getMessage(), false);
>+		}
>+		assertTrue(
>+			"Should get one result but got " + (requestor.resultIndex + 1),
>+			requestor.resultIndex == 0);
>+		EvaluationResult result = requestor.results[0];
>+		assertTrue("Code snippet should not have problems", !result.hasProblems());
>+		assertTrue("Result should have a value", result.hasValue());
>+		assertEquals("Value", "3".toCharArray(), result.getValueDisplayString());
>+		assertEquals("Type", "int".toCharArray(), result.getValueTypeName());
>+	} finally {
>+		removeTempClass("A66");
>+	}
>+}
>+public void test067() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5) return;
>+	try {
>+		String sourceA67 =
>+			"import java.util.List;\n" +
>+			"public class A67<T> {\n" +
>+			"	public static String toString(List<?> list) {\n" +
>+			"		StringBuilder builder = new StringBuilder(\"{\");\n" +
>+			"		for (Object o : list) {" +
>+			"			builder.append(o);\n" +
>+			"		}\n" +
>+			"		builder.append(\"}\");\n" +
>+			"		return String.valueOf(builder);\n" +
>+			"	}\n" +
>+			"	public void bar() {\n" +
>+			"	}\n" +
>+			"}";
>+		compileAndDeploy15(sourceA67, "A67");
>+
>+		String userCode = "new A67<Object>().bar();";
>+		JDIStackFrame stackFrame =
>+			new JDIStackFrame(this.jdiVM, this, userCode, "A67", "bar", -1);
>+
>+		DebugRequestor requestor = new DebugRequestor();
>+		char[] snippet = ("java.util.ArrayList<String> list = new java.util.ArrayList<String>();\n" +
>+				"list.add(\"Test\");\n" +
>+				"list.add(\"Hello\");\n" +
>+				"list.add(\"World\");\n" +
>+				"return A67.toString(list);").toCharArray();
>+		try {
>+			this.context.evaluate(
>+				snippet,
>+				stackFrame.localVariableTypeNames(),
>+				stackFrame.localVariableNames(),
>+				stackFrame.localVariableModifiers(),
>+				stackFrame.declaringTypeName(),
>+				stackFrame.isStatic(),
>+				stackFrame.isConstructorCall(),
>+				getEnv(),
>+				getCompilerOptions(),
>+				requestor,
>+				getProblemFactory());
>+		} catch (InstallException e) {
>+			assertTrue("No targetException " + e.getMessage(), false);
>+		}
>+		assertTrue(
>+			"Should get one result but got " + (requestor.resultIndex + 1),
>+			requestor.resultIndex == 0);
>+		EvaluationResult result = requestor.results[0];
>+		assertTrue("Code snippet should not have problems", !result.hasProblems());
>+		assertTrue("Result should have a value", result.hasValue());
>+		assertEquals("Value", "{TestHelloWorld}".toCharArray(), result.getValueDisplayString());
>+		assertEquals("Type", "java.lang.String".toCharArray(), result.getValueTypeName());
>+	} finally {
>+		removeTempClass("A67");
>+	}
>+}
> /**
>  * https://bugs.eclipse.org/bugs/show_bug.cgi?id=178861
>  */

Actions: View | Diff

Attachments on bug 235783: 154492