Skip to content

Commit

Permalink
Don't create pattern instanceof expression for non-conditional infix
Browse files Browse the repository at this point in the history 
- modify PatternMatchingForInstanceofFixCore to recognize when an
  instanceof expression is used in an infix expression but not with
  conditional and (&&) or conditional or (||)
- modify tests in CleanUpTest16
- fixes #1218
  • Loading branch information
@jjohnstn
jjohnstn committed Mar 6, 2024
1 parent e43c2a6 commit b3f7a0d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 41 deletions.
10 changes: 9 additions & 1 deletion ...re extension/org/eclipse/jdt/internal/corext/fix/PatternMatchingForInstanceofFixCore.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2021, 2023 Fabrice TIERCELIN and others.
* Copyright (c) 2021, 2024 Fabrice TIERCELIN and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -29,6 +29,7 @@
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.IfStatement;
import org.eclipse.jdt.core.dom.InfixExpression;
import org.eclipse.jdt.core.dom.InfixExpression.Operator;
import org.eclipse.jdt.core.dom.InstanceofExpression;
import org.eclipse.jdt.core.dom.Modifier;
import org.eclipse.jdt.core.dom.Modifier.ModifierKeyword;
Expand Down Expand Up @@ -131,6 +132,13 @@ public boolean visit(final InstanceofExpression visited) {
return true;
}

if (currentNode instanceof InfixExpression infixExp) {
if (infixExp.getOperator() != Operator.CONDITIONAL_AND &&
infixExp.getOperator() != Operator.CONDITIONAL_OR) {
return true;
}
}

IfStatement ifStatement= (IfStatement) currentNode.getParent();

if (isPositiveCaseToAnalyze) {
Expand Down
60 changes: 20 additions & 40 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest16.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2020, 2023 Red Hat Inc. and others.
* Copyright (c) 2020, 2024 Red Hat Inc. and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -160,17 +160,6 @@ public void testPatternMatchingForInstanceof() throws Exception {
+ " return 0;\n" //
+ " }\n" //
+ "\n" //
+ "\n" //
+ " public long matchPatternInAndExpression(Object object, boolean isValid) {\n" //
+ " if (object instanceof Date & isValid) {\n" //
+ " // Keep this comment\n" //
+ " Date date = (Date) object;\n" //
+ " return date.getTime();\n" //
+ " }\n" //
+ "\n" //
+ " return 0;\n" //
+ " }\n" //
+ "\n" //
+ " public long matchPatternInElse(Object object) {\n" //
+ " if (!(object instanceof Date)) {\n" //
+ " return 0;\n" //
Expand All @@ -189,15 +178,6 @@ public void testPatternMatchingForInstanceof() throws Exception {
+ " }\n" //
+ " }\n" //
+ "\n" //
+ " public long matchPatternInOrExpression(Object object, boolean isValid) {\n" //
+ " if (isValid | !(object instanceof Date)) {\n" //
+ " return 0;\n" //
+ " } else {\n" //
+ " Date date = (Date) object;\n" //
+ " return date.getTime();\n" //
+ " }\n" //
+ " }\n" //
+ "\n" //
+ " public long matchPatternInElse(Object object) {\n" //
+ " if (!(object instanceof Date)) {\n" //
+ " return 0;\n" //
Expand Down Expand Up @@ -254,16 +234,6 @@ public void testPatternMatchingForInstanceof() throws Exception {
+ " return 0;\n" //
+ " }\n" //
+ "\n" //
+ "\n" //
+ " public long matchPatternInAndExpression(Object object, boolean isValid) {\n" //
+ " if (object instanceof Date date & isValid) {\n" //
+ " // Keep this comment\n" //
+ " return date.getTime();\n" //
+ " }\n" //
+ "\n" //
+ " return 0;\n" //
+ " }\n" //
+ "\n" //
+ " public long matchPatternInElse(Object object) {\n" //
+ " if (!(object instanceof Date date)) {\n" //
+ " return 0;\n" //
Expand All @@ -280,14 +250,6 @@ public void testPatternMatchingForInstanceof() throws Exception {
+ " }\n" //
+ " }\n" //
+ "\n" //
+ " public long matchPatternInOrExpression(Object object, boolean isValid) {\n" //
+ " if (isValid | !(object instanceof Date date)) {\n" //
+ " return 0;\n" //
+ " } else {\n" //
+ " return date.getTime();\n" //
+ " }\n" //
+ " }\n" //
+ "\n" //
+ " public long matchPatternInElse(Object object) {\n" //
+ " if (!(object instanceof Date date)) {\n" //
+ " return 0;\n" //
Expand Down Expand Up @@ -478,7 +440,25 @@ public void testDoNotMatchPatternForInstanceof() throws Exception {
+ " Integer i = (Integer) bah;\n" //
+ " System.out.println(i);\n" //
+ " }\n" //
+ "}\n";
+ "\n" //
+ " public void doNotMatchBitWiseAnd(boolean useStrikethroughForCompleted, Object data) {\n" //
+ " if (data instanceof Long & useStrikethroughForCompleted) {\n" //
+ " Long task = (Long)data;\n" //
+ " if (task.intValue() == 0) {\n" //
+ " int i = 0;\n" //
+ " }\n" //
+ " }\n" //
+ " }\n" //
+ "\n" //
+ " public void doNotMatchBitWiseOr(boolean useStrikethroughForCompleted, Object data) {\n" //
+ " if (data instanceof Long | useStrikethroughForCompleted) {\n" //
+ " Long task = (Long)data;\n" //
+ " if (task.intValue() == 0) {\n" //
+ " int i = 0;\n" //
+ " }\n" //
+ " }\n" //
+ " }\n" //
+ "}\n";
ICompilationUnit cu= pack.createCompilationUnit("E.java", sample, false, null);

enable(CleanUpConstants.USE_PATTERN_MATCHING_FOR_INSTANCEOF);
Expand Down

0 comments on commit b3f7a0d

Please sign in to comment.