Bug 548647 - Turn off cast checking in instanceof when neededI20190726-1800I20190725-1800

- change InstanceofExpression.resolveType() to register the
  instanceof type in the left hand expression if it is a
  CastExpression
- modify CastExpression to disable unnecessary type checking if
  an instanceof type has been registered and it is provably
  distinct from the expression type
- add new tests to CastTest

Change-Id: Ifb7730706b189915ef8afd859722ea831dccca76
Also-by: Stephan Herrmann <stephan.herrmann@berlin.de>

4 files changed, 95 insertions, 2 deletions

diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java
index 5deac455aa..0eb6a2810f 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java
@@ -3346,6 +3346,81 @@ public void test543727_notequals() {
 		},
 		"SUCCESS");
 }
+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=548647 JDT reports unnecessary cast, using the Quickfix to remove it creates syntax error
+public void test548647() {
+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
+		return;
+	Map customOptions = getCompilerOptions();
+	customOptions.put(CompilerOptions.OPTION_ReportUnnecessaryTypeCheck, CompilerOptions.ERROR);
+	this.runConformTest(
+		new String[] {
+			"X.java",
+			"interface MUIElement {}\n" + 
+			"interface MUIElementContainer<T extends MUIElement> extends MUIElement{}\n" + 
+			"interface MWindowElement extends MUIElement {}\n" + 
+			"interface MWindow extends MUIElementContainer<MWindowElement> {}\n" + 
+			"public class X {\n" +
+			"	MUIElementContainer<MUIElement> field;\n" +
+			"	MUIElementContainer<MUIElement> getField() {\n" +
+			"		return field;\n" +
+			"	}\n" +
+			"	void test(MUIElementContainer<MUIElement> me) {\n" +
+			"		MUIElementContainer<MUIElement> localVar = me;\n" +
+			"		if ((Object) localVar instanceof MWindow) return;\n" +
+			"		if(((Object) me) instanceof MWindow) return;\n" +
+			"		if ((MUIElement)field instanceof MWindow) return;\n" +
+			"		if ((MUIElement)getField() instanceof MWindow) return;\n" +
+			"		MWindow mw = (MWindow)((MUIElement)me);\n" + 
+			"	}\n" + 
+			"}\n"
+		},
+		customOptions);
+}
+public void test548647a() {
+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
+		return;
+	Runner runner = new Runner();
+	runner.customOptions = getCompilerOptions();
+	runner.customOptions.put(CompilerOptions.OPTION_ReportUnnecessaryTypeCheck, CompilerOptions.WARNING);
+	runner.testFiles =
+		new String[] {
+			"Bug.java",
+			"public class Bug {\n" +
+			"	Integer k;\n" +
+			"	private Number getK() { return k; }\n" +
+			"	public void fn(Number n) {\n" + 
+			"		Number j = n;\n" +
+			"		if ((Number) n instanceof Long) return;\n" +
+			"		if ((Number) k instanceof Integer) return;\n" +
+			"		if ((Number) j instanceof Integer) return;\n" +
+			"		if ((Number) getK() instanceof Integer) return;\n" +
+			"	}\n" + 
+			"}"
+		};
+	runner.expectedCompilerLog =
+			"----------\n" +
+			"1. WARNING in Bug.java (at line 6)\n" +
+			"	if ((Number) n instanceof Long) return;\n" +
+			"	    ^^^^^^^^^^\n" +
+			"Unnecessary cast from Number to Number\n" +
+			"----------\n" +
+			"2. WARNING in Bug.java (at line 7)\n" +
+			"	if ((Number) k instanceof Integer) return;\n" +
+			"	    ^^^^^^^^^^\n" +
+			"Unnecessary cast from Integer to Number\n" +
+			"----------\n" +
+			"3. WARNING in Bug.java (at line 8)\n" +
+			"	if ((Number) j instanceof Integer) return;\n" +
+			"	    ^^^^^^^^^^\n" +
+			"Unnecessary cast from Number to Number\n" +
+			"----------\n" +
+			"4. WARNING in Bug.java (at line 9)\n" +
+			"	if ((Number) getK() instanceof Integer) return;\n" +
+			"	    ^^^^^^^^^^^^^^^\n" +
+			"Unnecessary cast from Number to Number\n" +
+			"----------\n";
+	runner.runWarningTest();
+}
 
 public static Class testClass() {
 	return CastTest.class;
diff --git a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
index 26ecdd31b2..d0ea0342fe 100644
--- a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
+++ b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
@@ -4081,6 +4081,12 @@ public final class CompletionEngine
 			} else if(parent instanceof InstanceOfExpression) {
 				InstanceOfExpression e = (InstanceOfExpression) parent;
 				TypeBinding binding = e.expression.resolvedType;
+				if (binding == null) {
+					if (scope instanceof BlockScope)
+						binding = e.expression.resolveType((BlockScope) scope);
+					else if (scope instanceof ClassScope)
+						binding = e.expression.resolveType((ClassScope) scope);
+				}
 				if(binding != null){
 					addExpectedType(binding, scope);
 					this.expectedTypesFilter = SUBTYPE | SUPERTYPE;
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java
index c03d019af6..bf216e5ff3 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java
@@ -64,6 +64,7 @@ public class CastExpression extends Expression {
 	public Expression expression;
 	public TypeReference type;
 	public TypeBinding expectedType; // when assignment conversion to a given expected type: String s = (String) t;
+	public TypeBinding instanceofType; // set by InstanceofExpression to ensure we don't flag a necessary cast unnecessary
 
 //expression.implicitConversion holds the cast for baseType casting
 public CastExpression(Expression expression, TypeReference type) {
@@ -610,6 +611,10 @@ public TypeBinding resolveType(BlockScope scope) {
 			boolean nullAnnotationMismatch = scope.compilerOptions().isAnnotationBasedNullAnalysisEnabled
 					&& NullAnnotationMatching.analyse(castType, expressionType, -1).isAnyMismatch();
 
+			if (this.instanceofType != null && expressionType.isParameterizedType()
+					&& expressionType.isProvablyDistinct(this.instanceofType)) {
+				this.bits |= ASTNode.DisableUnnecessaryCastCheck;
+			}
 			boolean isLegal = checkCastTypesCompatibility(scope, castType, expressionType, this.expression);
 			if (isLegal) {
 				this.expression.computeConversion(scope, castType, expressionType);
@@ -691,6 +696,10 @@ public void tagAsUnnecessaryCast(Scope scope, TypeBinding castType) {
 	this.bits |= ASTNode.UnnecessaryCast;
 }
 
+public void setInstanceofType(TypeBinding instanceofTypeBinding) {
+	this.instanceofType = instanceofTypeBinding;
+}
+
 @Override
 public void traverse(ASTVisitor visitor, BlockScope blockScope) {
 	if (visitor.visit(this, blockScope)) {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
index 2c5503f5e8..96e26646d2 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2015 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -97,8 +97,11 @@ public StringBuffer printExpressionNoParenthesis(int indent, StringBuffer output
 @Override
 public TypeBinding resolveType(BlockScope scope) {
 	this.constant = Constant.NotAConstant;
-	TypeBinding expressionType = this.expression.resolveType(scope);
 	TypeBinding checkedType = this.type.resolveType(scope, true /* check bounds*/);
+	if (this.expression instanceof CastExpression) {
+		((CastExpression) this.expression).setInstanceofType(checkedType); // for cast expression we need to know instanceof type to not tag unnecessary when needed
+	}
+	TypeBinding expressionType = this.expression.resolveType(scope);
 	if (expressionType != null && checkedType != null && this.type.hasNullTypeAnnotation(AnnotationPosition.ANY)) {
 		// don't complain if the entire operation is redundant anyway
 		if (!expressionType.isCompatibleWith(checkedType) || NullAnnotationMatching.analyse(checkedType, expressionType, -1).isAnyMismatch())