From 733c3136524464b8ac8c6b17681a25433e382ee1 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Thu, 15 Feb 2024 22:23:16 -0500 Subject: [PATCH 1/2] Fix one if cleanup to watch for PatternInstanceof nodes - fixes #1200 - add new PatternInstanceof logic to OneIfRatherThanDuplicateBlocksThatFallThroughFixCore - add new test to CleanUpTest16 --- ...DuplicateBlocksThatFallThroughFixCore.java | 44 ++++++++++++- .../jdt/ui/tests/quickfix/CleanUpTest16.java | 62 +++++++++++++++++++ 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java index 4394cf6a632..6e4fb0de3a4 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/OneIfRatherThanDuplicateBlocksThatFallThroughFixCore.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2021 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 @@ -14,7 +14,9 @@ package org.eclipse.jdt.internal.corext.fix; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.core.runtime.CoreException; @@ -29,6 +31,10 @@ import org.eclipse.jdt.core.dom.Expression; import org.eclipse.jdt.core.dom.IfStatement; import org.eclipse.jdt.core.dom.InfixExpression; +import org.eclipse.jdt.core.dom.Pattern; +import org.eclipse.jdt.core.dom.PatternInstanceofExpression; +import org.eclipse.jdt.core.dom.SingleVariableDeclaration; +import org.eclipse.jdt.core.dom.TypePattern; import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; import org.eclipse.jdt.core.dom.rewrite.TargetSourceRangeComputer; @@ -72,10 +78,14 @@ public boolean visit(final Block node) { public boolean visit(final IfStatement visited) { if (result && ASTNodes.fallsThrough(visited.getThenStatement())) { List duplicateIfBlocks= new ArrayList<>(ASTNodes.EXCESSIVE_OPERAND_NUMBER - 1); + Set patternNames= new HashSet<>(); + PatternNameVisitor visitor= new PatternNameVisitor(); + visited.accept(visitor); + patternNames= visitor.getPatternNames(); AtomicInteger operandCount= new AtomicInteger(ASTNodes.getNbOperands(visited.getExpression())); duplicateIfBlocks.add(visited); - while (addOneMoreIf(duplicateIfBlocks, operandCount)) { + while (addOneMoreIf(duplicateIfBlocks, patternNames, operandCount)) { // OK continue } @@ -89,7 +99,27 @@ public boolean visit(final IfStatement visited) { return true; } - private boolean addOneMoreIf(final List duplicateIfBlocks, final AtomicInteger operandCount) { + private class PatternNameVisitor extends ASTVisitor { + private Set patternNames= new HashSet<>(); + + @Override + public boolean visit(PatternInstanceofExpression node) { + Pattern p= node.getPattern(); + if (p instanceof TypePattern typePattern) { + List patternVariables= typePattern.patternVariables(); + for (SingleVariableDeclaration patternVariable : patternVariables) { + patternNames.add(patternVariable.getName().getFullyQualifiedName()); + } + } + return true; + } + + public Set getPatternNames() { + return patternNames; + } + } + + private boolean addOneMoreIf(final List duplicateIfBlocks, final Set patternNames, final AtomicInteger operandCount) { IfStatement lastBlock= duplicateIfBlocks.get(duplicateIfBlocks.size() - 1); if (lastBlock.getElseStatement() == null) { @@ -99,6 +129,14 @@ private boolean addOneMoreIf(final List duplicateIfBlocks, final At && nextSibling.getElseStatement() == null && operandCount.get() + ASTNodes.getNbOperands(nextSibling.getExpression()) < ASTNodes.EXCESSIVE_OPERAND_NUMBER && ASTNodes.match(lastBlock.getThenStatement(), nextSibling.getThenStatement())) { + PatternNameVisitor visitor= new PatternNameVisitor(); + nextSibling.getExpression().accept(visitor); + Set siblingPatternNames= visitor.getPatternNames(); + for (String siblingPatternName : siblingPatternNames) { + if (!patternNames.add(siblingPatternName)) { + return false; + } + } operandCount.addAndGet(ASTNodes.getNbOperands(nextSibling.getExpression())); duplicateIfBlocks.add(nextSibling); return true; diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest16.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest16.java index e4368f378af..c0156c06555 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest16.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest16.java @@ -332,6 +332,68 @@ public void testPatternMatchingForInstanceof2() throws Exception { // https://gi new HashSet<>(Arrays.asList(MultiFixMessages.PatternMatchingForInstanceofCleanup_description))); } + @Test + public void testOneIfWithPatternInstanceof() throws Exception { // https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1200 + IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null); + String sample= "" // + + "package test1;\n" // + + "\n" // + + "public class E {\n" // + + "\n" // + + " protected String getString(Number number) {\n" // + + "\n" // + + " if (number instanceof Long n) {\n" // + + " return n.toString();\n" // + + " }\n" // + + " if (number instanceof Float n) {\n" // + + " return n.toString();\n" // + + " }\n" // + + " if (number instanceof Double n) {\n" // + + " return n.toString();\n" // + + " }\n" // + + " if (number instanceof Float n && n.isInfinite()) {\n" // + + " return \"Inf\"; //$NON-NLS-1$\n" // + + " }\n" // + + " if (number instanceof Double m && m.isInfinite()) {\n" // + + " return \"Inf\"; //$NON-NLS-1$\n" // + + " }\n" // + + "\n" // + + " return null;\n" // + + " }\n" // + + "\n" // + + "}\n"; // + ICompilationUnit cu= pack.createCompilationUnit("E.java", sample, false, null); + + enable(CleanUpConstants.ONE_IF_RATHER_THAN_DUPLICATE_BLOCKS_THAT_FALL_THROUGH); + + String expected= "" // + + "package test1;\n" // + + "\n" // + + "public class E {\n" // + + "\n" // + + " protected String getString(Number number) {\n" // + + "\n" // + + " if (number instanceof Long n) {\n" // + + " return n.toString();\n" // + + " }\n" // + + " if (number instanceof Float n) {\n" // + + " return n.toString();\n" // + + " }\n" // + + " if (number instanceof Double n) {\n" // + + " return n.toString();\n" // + + " }\n" // + + " if ((number instanceof Float n && n.isInfinite()) || (number instanceof Double m && m.isInfinite())) {\n" // + + " return \"Inf\"; //$NON-NLS-1$\n" // + + " }\n" // + + "\n" // + + " return null;\n" // + + " }\n" // + + "\n" // + + "}\n"; // + assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected }, + new HashSet<>(Arrays.asList(MultiFixMessages.OneIfRatherThanDuplicateBlocksThatFallThroughCleanUp_description))); + } + @Test public void testDoNotMatchPatternForInstanceof() throws Exception { IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null); From 3587fd2521c3e87ed6e6b5effade254fc5978e06 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Fri, 8 Mar 2024 23:24:06 -0500 Subject: [PATCH 2/2] Remove warning --- org.eclipse.jdt.core.manipulation/.settings/.api_filters | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/org.eclipse.jdt.core.manipulation/.settings/.api_filters b/org.eclipse.jdt.core.manipulation/.settings/.api_filters index a42165f7e4b..3198459f671 100644 --- a/org.eclipse.jdt.core.manipulation/.settings/.api_filters +++ b/org.eclipse.jdt.core.manipulation/.settings/.api_filters @@ -59,6 +59,15 @@ + + + + + + + + +