From 4608011210b88fa8c29e09c6e07d8bfd6bd5c9a2 Mon Sep 17 00:00:00 2001 From: Bart van Helvert Date: Tue, 27 Jan 2026 14:46:49 +0100 Subject: [PATCH] [java] Make Replace multiply with shift intention round towards -inf #IDEA-384124 Fixed (cherry picked from commit bc3e3ddc66db0c7b06f8ad5d281fe594a1c092db) IJ-MR-189843 GitOrigin-RevId: 04f72906c4b22476b1c45644b097abeacd556c93 --- .../ReplaceShiftWithMultiplyIntention.java | 83 ++++++++++++------- .../RightShift.java | 4 +- .../RightShiftAssign.java | 5 ++ .../RightShiftAssign_after.java | 5 ++ .../RightShift_after.java | 4 +- ...ReplaceShiftWithMultiplyIntentionTest.java | 15 +++- 6 files changed, 80 insertions(+), 36 deletions(-) create mode 100644 java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign.java create mode 100644 java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign_after.java diff --git a/java/java-impl/src/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntention.java b/java/java-impl/src/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntention.java index e771ea0c2285..e30cb419be52 100644 --- a/java/java-impl/src/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntention.java +++ b/java/java-impl/src/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntention.java @@ -17,6 +17,7 @@ package com.siyeh.ipp.shift; import com.intellij.codeInspection.CommonQuickFixBundle; import com.intellij.openapi.project.DumbAware; +import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.JavaTokenType; import com.intellij.psi.PsiAssignmentExpression; import com.intellij.psi.PsiBinaryExpression; @@ -25,6 +26,7 @@ import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiJavaToken; import com.intellij.psi.PsiLiteralExpression; import com.intellij.psi.PsiParenthesizedExpression; +import com.intellij.psi.PsiType; import com.intellij.psi.PsiTypes; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiUtil; @@ -35,6 +37,7 @@ import com.siyeh.ig.psiutils.ParenthesesUtils; import com.siyeh.ipp.base.MCIntention; import com.siyeh.ipp.base.PsiElementPredicate; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public final class ReplaceShiftWithMultiplyIntention extends MCIntention implements DumbAware { @@ -48,65 +51,73 @@ public final class ReplaceShiftWithMultiplyIntention extends MCIntention impleme if (element instanceof PsiBinaryExpression exp) { final PsiJavaToken sign = exp.getOperationSign(); final IElementType tokenType = sign.getTokenType(); - final String operatorString = tokenType.equals(JavaTokenType.LTLT) ? "*" : "/"; + final String operatorString = tokenType.equals(JavaTokenType.LTLT) ? "*" : "Math.floorDiv"; return CommonQuickFixBundle.message("fix.replace.x.with.y", sign.getText(), operatorString); } else { final PsiAssignmentExpression exp = (PsiAssignmentExpression)element; final PsiJavaToken sign = exp.getOperationSign(); final IElementType tokenType = sign.getTokenType(); - final String assignString = JavaTokenType.LTLTEQ.equals(tokenType) ? "*=" : "/="; + final String assignString = JavaTokenType.LTLTEQ.equals(tokenType) ? "*=" : "Math.floorDiv"; return CommonQuickFixBundle.message("fix.replace.x.with.y", sign.getText(), assignString); } } @Override public @NotNull PsiElementPredicate getElementPredicate() { - return new ShiftByLiteralPredicate(); + return new ShiftByLiteralPredicate() { + @Override + public boolean satisfiedBy(PsiElement element) { + if ((element instanceof PsiAssignmentExpression aExpr && aExpr.getOperationTokenType().equals(JavaTokenType.GTGTEQ)) || + (element instanceof PsiBinaryExpression bExpr && bExpr.getOperationTokenType().equals(JavaTokenType.GTGT)) + ) { + // Can't replace with Math.floorDiv because it was introduced in JDK 8 + if (PsiUtil.getLanguageLevel(element).isLessThan(LanguageLevel.JDK_1_8)) return false; + } + return super.satisfiedBy(element); + } + }; } @Override public void invoke(@NotNull PsiElement element) { - if (element instanceof PsiBinaryExpression) { - replaceShiftWithMultiplyOrDivide(element); + if (element instanceof PsiBinaryExpression expr) { + replaceShiftWithMultiplyOrDivide(expr); } - else { - replaceShiftAssignWithMultiplyOrDivideAssign(element); + else if (element instanceof PsiAssignmentExpression expr) { + replaceShiftAssignWithMultiplyOrDivideAssign(expr); } } - private static void replaceShiftAssignWithMultiplyOrDivideAssign(PsiElement element) { - final PsiAssignmentExpression exp = (PsiAssignmentExpression)element; + private static void replaceShiftAssignWithMultiplyOrDivideAssign(PsiAssignmentExpression exp) { final PsiExpression lhs = exp.getLExpression(); - final PsiExpression rhs = PsiUtil.skipParenthesizedExprDown(exp.getRExpression()); - final IElementType tokenType = exp.getOperationTokenType(); - final String assignString = tokenType.equals(JavaTokenType.LTLTEQ) ? "*=" : "/="; - final PsiLiteralExpression literal = (PsiLiteralExpression)rhs; - assert rhs != null; - final Number value = (Number)literal.getValue(); - assert value != null; + final PsiExpression rhsExpr = PsiUtil.skipParenthesizedExprDown(exp.getRExpression()); + if (!(rhsExpr instanceof PsiLiteralExpression rhsLiteral)) return; CommentTracker commentTracker = new CommentTracker(); - final String expString = PsiTypes.longType().equals(lhs.getType()) - ? commentTracker.text(lhs) + assignString + (1L << value.intValue()) + 'L' - : commentTracker.text(lhs) + assignString + (1 << value.intValue()); + final String lhsText = commentTracker.text(lhs, ParenthesesUtils.MULTIPLICATIVE_PRECEDENCE); + final String rhsText = rhsReplacement(rhsLiteral, lhs.getType()); + String expString; + if (exp.getOperationTokenType().equals(JavaTokenType.GTGTEQ)) { + expString = lhsText + "=" + "Math.floorDiv(" + lhsText + ", " + rhsText + ")"; + } else { + expString = lhsText + "*=" + rhsText; + } PsiReplacementUtil.replaceExpression(exp, expString, commentTracker); } - private static void replaceShiftWithMultiplyOrDivide(PsiElement element) { - final PsiBinaryExpression expression = (PsiBinaryExpression)element; + private static void replaceShiftWithMultiplyOrDivide(PsiBinaryExpression expression) { final PsiExpression lhs = expression.getLOperand(); - final PsiExpression rhs = PsiUtil.skipParenthesizedExprDown(expression.getROperand()); - final IElementType tokenType = expression.getOperationTokenType(); - final String operatorString = tokenType.equals(JavaTokenType.LTLT) ? "*" : "/"; + final PsiExpression rhsExpr = PsiUtil.skipParenthesizedExprDown(expression.getROperand()); + if (!(rhsExpr instanceof PsiLiteralExpression rhsLiteral)) return; CommentTracker commentTracker = new CommentTracker(); final String lhsText = commentTracker.text(lhs, ParenthesesUtils.MULTIPLICATIVE_PRECEDENCE); - final PsiLiteralExpression literal = (PsiLiteralExpression)rhs; - assert rhs != null; - final Number value = (Number)literal.getValue(); - assert value != null; - String expString = PsiTypes.longType().equals(expression.getType()) - ? lhsText + operatorString + (1L << value.intValue()) + 'L' - : lhsText + operatorString + (1 << value.intValue()); + final String rhsText = rhsReplacement(rhsLiteral, lhs.getType()); + String expString; + if (expression.getOperationTokenType().equals(JavaTokenType.GTGT)) { + expString = "Math.floorDiv(" + lhsText + ", " + rhsText + ")"; + } else { + expString = lhsText + "*" + rhsText; + } if (expression.getParent() instanceof PsiExpression parent && !(parent instanceof PsiParenthesizedExpression) && ParenthesesUtils.getPrecedence(parent) < ParenthesesUtils.MULTIPLICATIVE_PRECEDENCE) { expString = '(' + expString + ')'; @@ -114,4 +125,14 @@ public final class ReplaceShiftWithMultiplyIntention extends MCIntention impleme PsiReplacementUtil.replaceExpression(expression, expString, commentTracker); } + + private static String rhsReplacement(@NotNull PsiLiteralExpression rhs, @Nullable PsiType type) { + final Number value = (Number)rhs.getValue(); + assert value != null; + if (PsiTypes.longType().equals(type)) { + return Long.toString(1L << value.intValue()) + 'L'; + } else { + return Integer.toString(1 << value.intValue()); + } + } } \ No newline at end of file diff --git a/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift.java b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift.java index 592f29630884..72dfc6b70c3c 100644 --- a/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift.java +++ b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift.java @@ -1,5 +1,5 @@ class Test { void test(int foo) { - int x = foo >> 12; + int x = foo >> 12; } -} \ No newline at end of file +} diff --git a/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign.java b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign.java new file mode 100644 index 000000000000..fa66efb8d29f --- /dev/null +++ b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign.java @@ -0,0 +1,5 @@ +class Test { + void test(int foo) { + foo >>= 12; + } +} diff --git a/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign_after.java b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign_after.java new file mode 100644 index 000000000000..03117e87dac6 --- /dev/null +++ b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShiftAssign_after.java @@ -0,0 +1,5 @@ +class Test { + void test(int foo) { + foo = Math.floorDiv(foo, 4096); + } +} diff --git a/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift_after.java b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift_after.java index 9e13383086d1..a4767771bd0c 100644 --- a/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift_after.java +++ b/java/java-tests/testData/ipp/com/siyeh/ipp/shift/replace_shift_with_multiply/RightShift_after.java @@ -1,5 +1,5 @@ class Test { void test(int foo) { - int x = foo / 4096; + int x = Math.floorDiv(foo, 4096); } -} \ No newline at end of file +} diff --git a/java/java-tests/testSrc/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntentionTest.java b/java/java-tests/testSrc/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntentionTest.java index c21c16301a7f..323f9f2bed88 100644 --- a/java/java-tests/testSrc/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntentionTest.java +++ b/java/java-tests/testSrc/com/siyeh/ipp/shift/ReplaceShiftWithMultiplyIntentionTest.java @@ -1,6 +1,8 @@ // Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.siyeh.ipp.shift; +import com.intellij.pom.java.LanguageLevel; +import com.intellij.testFramework.IdeaTestUtil; import com.siyeh.ipp.IPPTestCase; public class ReplaceShiftWithMultiplyIntentionTest extends IPPTestCase { @@ -11,7 +13,18 @@ public class ReplaceShiftWithMultiplyIntentionTest extends IPPTestCase { public void testLeftShiftAssign() { doTest("Replace '<<=' with '*='"); } public void testLongShiftAssign() { doTest("Replace '<<=' with '*='"); } public void testParentheses() { doTest("Replace '<<' with '*'"); } - public void testRightShift() { doTest("Replace '>>' with '/'"); } + public void testRightShift() { + doTest("Replace '>>' with 'Math.floorDiv'"); + IdeaTestUtil.withLevel(myFixture.getModule(), LanguageLevel.JDK_1_7, () -> { + assertIntentionNotAvailable("Replace '>>' with 'Math.floorDiv'"); + }); + } + public void testRightShiftAssign() { + doTest("Replace '>>=' with 'Math.floorDiv'"); + IdeaTestUtil.withLevel(myFixture.getModule(), LanguageLevel.JDK_1_7, () -> { + assertIntentionNotAvailable("Replace '>>' with 'Math.floorDiv'"); + }); + } @Override protected String getRelativePath() {