From 5e6aa8d90ce8f0e048cefa9753d955d0d44bd0d3 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Mon, 8 Jan 2018 23:09:08 +0100 Subject: [PATCH] IG: handle parentheses in and around lambda expressions properly --- .../siyeh/ig/psiutils/ParenthesesUtils.java | 155 ++++++------------ .../igfixes/parentheses/Division.after.java | 8 + .../siyeh/igfixes/parentheses/Division.java | 8 + .../igfixes/parentheses/LambdaBody.after.java | 3 + .../siyeh/igfixes/parentheses/LambdaBody.java | 3 + .../UnnecessaryParenthesesInspection.java | 8 + .../UnnecessaryParenthesesQuickFixTest.java | 2 + 7 files changed, 78 insertions(+), 109 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java index 62cfecc7a4c9..b1c00e3dc36d 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ParenthesesUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2017 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2018 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,7 +18,6 @@ package com.siyeh.ig.psiutils; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -49,7 +48,8 @@ public class ParenthesesUtils { public static final int OR_PRECEDENCE = 14; public static final int CONDITIONAL_PRECEDENCE = 15; public static final int ASSIGNMENT_PRECEDENCE = 16; - public static final int NUM_PRECEDENCES = 17; + public static final int LAMBDA_PRECEDENCE = 17; // jls-15.2 + public static final int NUM_PRECEDENCES = 18; private static final Map s_binaryOperatorPrecedence = new HashMap<>(NUM_PRECEDENCES); @@ -187,6 +187,9 @@ public class ParenthesesUtils { if (expression instanceof PsiParenthesizedExpression) { return PARENTHESIZED_PRECEDENCE; } + if (expression instanceof PsiLambdaExpression) { + return LAMBDA_PRECEDENCE; + } return -1; } @@ -203,54 +206,65 @@ public class ParenthesesUtils { final PsiMethodCallExpression methodCall = (PsiMethodCallExpression)expression; removeParensFromMethodCallExpression(methodCall, ignoreClarifyingParentheses); } - if (expression instanceof PsiReferenceExpression) { + else if (expression instanceof PsiReferenceExpression) { final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; removeParensFromReferenceExpression(referenceExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiNewExpression) { + else if (expression instanceof PsiNewExpression) { final PsiNewExpression newExpression = (PsiNewExpression)expression; removeParensFromNewExpression(newExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiAssignmentExpression) { + else if (expression instanceof PsiAssignmentExpression) { final PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)expression; removeParensFromAssignmentExpression(assignmentExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiArrayInitializerExpression) { + else if (expression instanceof PsiArrayInitializerExpression) { final PsiArrayInitializerExpression arrayInitializerExpression = (PsiArrayInitializerExpression)expression; removeParensFromArrayInitializerExpression(arrayInitializerExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiTypeCastExpression) { + else if (expression instanceof PsiTypeCastExpression) { final PsiTypeCastExpression typeCastExpression = (PsiTypeCastExpression)expression; removeParensFromTypeCastExpression(typeCastExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiArrayAccessExpression) { + else if (expression instanceof PsiArrayAccessExpression) { final PsiArrayAccessExpression arrayAccessExpression = (PsiArrayAccessExpression)expression; removeParensFromArrayAccessExpression(arrayAccessExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiPrefixExpression) { + else if (expression instanceof PsiPrefixExpression) { final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression; removeParensFromPrefixExpression(prefixExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiPostfixExpression) { + else if (expression instanceof PsiPostfixExpression) { final PsiPostfixExpression postfixExpression = (PsiPostfixExpression)expression; removeParensFromPostfixExpression(postfixExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiPolyadicExpression) { + else if (expression instanceof PsiPolyadicExpression) { final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression; removeParensFromPolyadicExpression(polyadicExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiInstanceOfExpression) { + else if (expression instanceof PsiInstanceOfExpression) { final PsiInstanceOfExpression instanceofExpression = (PsiInstanceOfExpression)expression; removeParensFromInstanceOfExpression(instanceofExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiConditionalExpression) { + else if (expression instanceof PsiConditionalExpression) { final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)expression; removeParensFromConditionalExpression(conditionalExpression, ignoreClarifyingParentheses); } - if (expression instanceof PsiParenthesizedExpression) { + else if (expression instanceof PsiParenthesizedExpression) { final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)expression; removeParensFromParenthesizedExpression(parenthesizedExpression, ignoreClarifyingParentheses); } + else if (expression instanceof PsiLambdaExpression) { + final PsiLambdaExpression lambdaExpression = (PsiLambdaExpression)expression; + removeParensFromLambdaExpression(lambdaExpression, ignoreClarifyingParentheses); + } + } + + private static void removeParensFromLambdaExpression(PsiLambdaExpression lambdaExpression, boolean ignoreClarifyingParentheses) { + final PsiElement body = lambdaExpression.getBody(); + if (body instanceof PsiExpression) { + removeParentheses((PsiExpression)body, ignoreClarifyingParentheses); + } } private static void removeParensFromReferenceExpression(@NotNull PsiReferenceExpression referenceExpression, @@ -269,91 +283,15 @@ public class ParenthesesUtils { return; } final PsiElement parent = parenthesizedExpression.getParent(); - if (!(parent instanceof PsiExpression) || parent instanceof PsiParenthesizedExpression || - parent instanceof PsiArrayInitializerExpression || parent instanceof PsiLambdaExpression) { - final PsiExpression newExpression = (PsiExpression)replaceWithBody(parenthesizedExpression, body); + if (!(parent instanceof PsiExpression) || !areParenthesesNeeded(body, (PsiExpression)parent, ignoreClarifyingParentheses)) { + final PsiExpression newExpression = (PsiExpression)new CommentTracker().replaceAndRestoreComments(parenthesizedExpression, body); removeParentheses(newExpression, ignoreClarifyingParentheses); - return; - } - else if (parent instanceof PsiArrayAccessExpression) { - final PsiArrayAccessExpression arrayAccessExpression = (PsiArrayAccessExpression)parent; - if (parenthesizedExpression == arrayAccessExpression.getIndexExpression()) { - final PsiExpression newExpression = replaceBodyExplicitly(parenthesizedExpression, body, parent); - removeParentheses(newExpression, ignoreClarifyingParentheses); - return; - } - } - final PsiExpression parentExpression = (PsiExpression)parent; - final int parentPrecedence = getPrecedence(parentExpression); - final int childPrecedence = getPrecedence(body); - if (parentPrecedence < childPrecedence) { - final PsiElement bodyParent = body.getParent(); - final PsiParenthesizedExpression newParenthesizedExpression = (PsiParenthesizedExpression)parenthesizedExpression.replace(bodyParent); - final PsiExpression expression = newParenthesizedExpression.getExpression(); - if (expression != null) { - removeParentheses(expression, ignoreClarifyingParentheses); - } - } - else if (parentPrecedence == childPrecedence) { - if (parentExpression instanceof PsiPolyadicExpression && body instanceof PsiPolyadicExpression) { - final PsiPolyadicExpression parentPolyadicExpression = (PsiPolyadicExpression)parentExpression; - final IElementType parentOperator = parentPolyadicExpression.getOperationTokenType(); - final PsiPolyadicExpression bodyPolyadicExpression = (PsiPolyadicExpression)body; - final IElementType bodyOperator = bodyPolyadicExpression.getOperationTokenType(); - final PsiType parentType = parentPolyadicExpression.getType(); - final PsiType bodyType = body.getType(); - if (parentType != null && parentType.equals(bodyType) && parentOperator.equals(bodyOperator)) { - final PsiExpression[] parentOperands = parentPolyadicExpression.getOperands(); - if (PsiTreeUtil.isAncestor(parentOperands[0], body, true) || isCommutativeOperator(bodyOperator)) { - final PsiExpression newExpression = replaceBodyExplicitly(parenthesizedExpression, body, parent); - removeParentheses(newExpression, ignoreClarifyingParentheses); - return; - } - } - if (ignoreClarifyingParentheses || - (parentOperator == JavaTokenType.PLUS && TypeUtils.isJavaLangString(parentType) && !TypeUtils.isJavaLangString(bodyType))) { - if (parentOperator.equals(bodyOperator)) { - removeParentheses(body, ignoreClarifyingParentheses); - } - } - else { - final PsiExpression newExpression = (PsiExpression)replaceWithBody(parenthesizedExpression, body); - removeParentheses(newExpression, ignoreClarifyingParentheses); - } - } - else { - final PsiExpression newExpression = (PsiExpression)replaceWithBody(parenthesizedExpression, body); - removeParentheses(newExpression, ignoreClarifyingParentheses); - } } else { - if (ignoreClarifyingParentheses && parent instanceof PsiPolyadicExpression && - (body instanceof PsiPolyadicExpression || body instanceof PsiInstanceOfExpression)) { - removeParentheses(body, ignoreClarifyingParentheses); - } - else { - final PsiExpression newExpression = (PsiExpression)replaceWithBody(parenthesizedExpression, body); - removeParentheses(newExpression, ignoreClarifyingParentheses); - } + removeParentheses(body, ignoreClarifyingParentheses); } } - private static PsiExpression replaceBodyExplicitly(@NotNull PsiParenthesizedExpression parenthesizedExpression, - PsiExpression body, - PsiElement parent) { - // use addAfter() + delete() instead of replace() to - // workaround automatic insertion of parentheses by psi - final PsiExpression newExpression = (PsiExpression)parent.addAfter(body, parenthesizedExpression); - CommentTracker tracker = new CommentTracker(); - tracker.markUnchanged(body); - tracker.deleteAndRestoreComments(parenthesizedExpression);; - return newExpression; - } - - private static PsiElement replaceWithBody(@NotNull PsiParenthesizedExpression parenthesizedExpression, PsiExpression body) { - return new CommentTracker().replaceAndRestoreComments(parenthesizedExpression, body); - } - private static void removeParensFromConditionalExpression(@NotNull PsiConditionalExpression conditionalExpression, boolean ignoreClarifyingParentheses) { final PsiExpression condition = conditionalExpression.getCondition(); @@ -465,25 +403,11 @@ public class ParenthesesUtils { public static boolean areParenthesesNeeded(PsiParenthesizedExpression expression, boolean ignoreClarifyingParentheses) { final PsiElement parent = expression.getParent(); - if (parent instanceof PsiLambdaExpression) { - return false; - } if (!(parent instanceof PsiExpression)) { return false; } final PsiExpression child = expression.getExpression(); - if (child == null || - child instanceof PsiLambdaExpression && PsiUtil.skipParenthesizedExprUp(parent) instanceof PsiReferenceExpression) { - return true; - } - if (parent instanceof PsiArrayAccessExpression) { - final PsiArrayAccessExpression arrayAccessExpression = (PsiArrayAccessExpression)parent; - final PsiExpression indexExpression = arrayAccessExpression.getIndexExpression(); - if (expression == indexExpression) { - return false; - } - } - return areParenthesesNeeded(child, (PsiExpression)parent, ignoreClarifyingParentheses); + return child == null || areParenthesesNeeded(child, (PsiExpression)parent, ignoreClarifyingParentheses); } public static boolean areParenthesesNeeded(PsiExpression expression, PsiExpression parentExpression, @@ -491,6 +415,10 @@ public class ParenthesesUtils { if (parentExpression instanceof PsiParenthesizedExpression || parentExpression instanceof PsiArrayInitializerExpression) { return false; } + if (parentExpression instanceof PsiArrayAccessExpression) { + final PsiArrayAccessExpression arrayAccessExpression = (PsiArrayAccessExpression)parentExpression; + return PsiTreeUtil.isAncestor(arrayAccessExpression.getArrayExpression(), expression, false); + } final int parentPrecedence = getPrecedence(parentExpression); final int childPrecedence = getPrecedence(expression); if (parentPrecedence > childPrecedence) { @@ -556,6 +484,15 @@ public class ParenthesesUtils { final PsiExpression condition = conditionalExpression.getCondition(); return PsiTreeUtil.isAncestor(condition, expression, true); } + else if (expression instanceof PsiLambdaExpression) { // jls-15.16 + if (parentExpression instanceof PsiTypeCastExpression) { + return false; + } + else if (parentExpression instanceof PsiConditionalExpression) { // jls-15.25 + final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)parentExpression; + return PsiTreeUtil.isAncestor(conditionalExpression.getCondition(), expression, true); + } + } return parentPrecedence < childPrecedence; } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.after.java new file mode 100644 index 000000000000..22db5fdf6c6c --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.after.java @@ -0,0 +1,8 @@ +class Divistion { + void zz() { + int a = 10; + int b = 20; + + final int i = a * ((b + 2) / 3); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.java new file mode 100644 index 000000000000..910f0a6557b8 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/Division.java @@ -0,0 +1,8 @@ +class Divistion { + void zz() { + int a = 10; + int b = 20; + + final int i = (a * ((b + 2) / 3)); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.after.java new file mode 100644 index 000000000000..c121936f3103 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.after.java @@ -0,0 +1,3 @@ +class LambdaBody { + Comparator comparator = Comparator.comparing(s -> s.substring(2).isEmpty()); +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.java new file mode 100644 index 000000000000..4b06bf76c951 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/parentheses/LambdaBody.java @@ -0,0 +1,3 @@ +class LambdaBody { + Comparator comparator = Comparator.comparing(s -> (s.substring(2).isEmpty())); +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/unnecessary_parentheses/UnnecessaryParenthesesInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/unnecessary_parentheses/UnnecessaryParenthesesInspection.java index 555bf464523e..e0e386ac92f6 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/unnecessary_parentheses/UnnecessaryParenthesesInspection.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/unnecessary_parentheses/UnnecessaryParenthesesInspection.java @@ -120,4 +120,12 @@ public class UnnecessaryParenthesesInspection final int i = a * ((b + 2) / 3); // no warn final int j = a * ((b + 2) % 3); // no warn } + + void lambda() { + Runnable r = (()->true) ? () -> {} : () -> {}; // no warn + } + + public java.util.function.IntFunction context() { + return (a -> a)=1; + } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/parenthesis/UnnecessaryParenthesesQuickFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/parenthesis/UnnecessaryParenthesesQuickFixTest.java index d47c708527a8..4f5ac8664e16 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/parenthesis/UnnecessaryParenthesesQuickFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/parenthesis/UnnecessaryParenthesesQuickFixTest.java @@ -43,6 +43,8 @@ public class UnnecessaryParenthesesQuickFixTest extends IGQuickFixesTestCase { public void testLambdaQualifier() { assertQuickfixNotAvailable(); } public void testLambdaInTernary() { doTest(); } public void testLambdaCast() { doTest(); } + public void testLambdaBody() { doTest(); } + public void testDivision() { doTest(); } @Override protected BaseInspection getInspection() { return new UnnecessaryParenthesesInspection();