From fb8ee5f6671ce63ccb12ba98400532877d96f5a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Wed, 10 Mar 2021 10:00:31 +0100 Subject: [PATCH] feat: print minimal amount of round brackets in sniper mode (#3823) --- spoon-pom/pom.xml | 8 +- .../visitor/DefaultJavaPrettyPrinter.java | 38 ++++++ .../spoon/reflect/visitor/OperatorHelper.java | 118 ++++++++++++++++++ .../reflect/visitor/RoundBracketAnalyzer.java | 114 +++++++++++++++++ .../sniper/SniperJavaPrettyPrinter.java | 3 + .../visitor/DefaultJavaPrettyPrinterTest.java | 63 ++++++++++ .../test/prettyprinter/TestSniperPrinter.java | 19 +++ 7 files changed, 362 insertions(+), 1 deletion(-) create mode 100644 src/main/java/spoon/reflect/visitor/RoundBracketAnalyzer.java create mode 100644 src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java diff --git a/spoon-pom/pom.xml b/spoon-pom/pom.xml index 137251adffd..7ffd9665f41 100644 --- a/spoon-pom/pom.xml +++ b/spoon-pom/pom.xml @@ -54,7 +54,13 @@ 5.7.1 test - + + org.junit.jupiter + junit-jupiter-params + 5.7.1 + test + + diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 6c94c29c199..58ae446795a 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -217,6 +217,12 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter { */ protected boolean ignoreImplicit = true; + /** + * EXPERIMENTAL: If true, the printer will attempt to print a minimal set of round brackets in + * expressions while preserving the syntactical structure of the AST. + */ + private boolean minimizeRoundBrackets = false; + public boolean inlineElseIf = true; /** @@ -391,6 +397,13 @@ private boolean shouldSetBracket(CtExpression e) { return true; } try { + if (isMinimizeRoundBrackets()) { + RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets = + RoundBracketAnalyzer.requiresRoundBrackets(e); + if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) { + return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; + } + } if ((e.getParent() instanceof CtBinaryOperator) || (e.getParent() instanceof CtUnaryOperator)) { return (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || e instanceof CtBinaryOperator; } @@ -2130,4 +2143,29 @@ public void visitCtYieldStatement(CtYieldStatement statement) { scan(statement.getExpression()); exitCtStatement(statement); } + + /** + * @return true if the printer is minimizing the amount of round brackets in expressions + */ + protected boolean isMinimizeRoundBrackets() { + return minimizeRoundBrackets; + } + + /** + * When set to true, this activates round bracket minimization for expressions. This means that + * the printer will attempt to only write round brackets strictly necessary for preserving + * syntactical structure (and by extension, semantics). + * + * As an example, the expression 1 + 2 + 3 + 4 is written as + * ((1 + 2) + 3) + 4 without round bracket minimization, but entirely without + * parentheses when minimization is enabled. However, an expression 1 + 2 + (3 + 4) + * is still written as 1 + 2 + (3 + 4) to preserve syntactical structure, even though + * the brackets are semantically redundant. + * + * @param minimizeRoundBrackets set whether or not to minimize round brackets in expressions + */ + protected void setMinimizeRoundBrackets(boolean minimizeRoundBrackets) { + this.minimizeRoundBrackets = minimizeRoundBrackets; + } + } diff --git a/src/main/java/spoon/reflect/visitor/OperatorHelper.java b/src/main/java/spoon/reflect/visitor/OperatorHelper.java index 3d5cdde6156..83dccc3c1cb 100644 --- a/src/main/java/spoon/reflect/visitor/OperatorHelper.java +++ b/src/main/java/spoon/reflect/visitor/OperatorHelper.java @@ -16,6 +16,10 @@ */ class OperatorHelper { + public enum OperatorAssociativity { + LEFT, RIGHT, NONE + } + private OperatorHelper() { } @@ -101,4 +105,118 @@ public static String getOperatorText(BinaryOperatorKind o) { throw new SpoonException("Unsupported operator " + o.name()); } } + + /** + * Get the precedence of a binary operator as defined by + * https://introcs.cs.princeton.edu/java/11precedence/ + * + * @param o A binary operator kind. + * @return The precedence of the given operator. + */ + public static int getOperatorPrecedence(BinaryOperatorKind o) { + switch (o) { + case OR: // || + return 3; + case AND: // && + return 4; + case BITOR: // | + return 5; + case BITXOR: // ^ + return 6; + case BITAND: // & + return 7; + case EQ: // == + case NE: // != + return 8; + case LT: // < + case GT: // > + case LE: // <= + case GE: // >= + case INSTANCEOF: + return 9; + case SL: // << + case SR: // >> + case USR: // >>> + return 10; + case PLUS: // + + case MINUS: // - + return 11; + case MUL: // * + case DIV: // / + case MOD: // % + return 12; + default: + throw new SpoonException("Unsupported operator " + o.name()); + } + } + + /** + * Get the precedence of a unary operator as defined by + * https://introcs.cs.princeton.edu/java/11precedence/ + * + * @param o A unary operator kind. + * @return The precedence of the given operator. + */ + public static int getOperatorPrecedence(UnaryOperatorKind o) { + switch (o) { + case POS: + case NEG: + case NOT: + case COMPL: + case PREINC: + case PREDEC: + return 14; + case POSTINC: + case POSTDEC: + return 15; + default: + throw new SpoonException("Unsupported operator " + o.name()); + } + } + + /** + * Get the associativity of a binary operator as defined by + * https://introcs.cs.princeton.edu/java/11precedence/ + * + * All binary operators are left-associative in Java, except for the relational operators that + * have no associativity (i.e. you can't chain them). + * + * There's an exception: the ternary operator ?: is right-associative, but that's not an + * operator kind in Spoon so we don't deal with it. + * + * @param o A binary operator kind. + * @return The associativity of the operator. + */ + public static OperatorAssociativity getOperatorAssociativity(BinaryOperatorKind o) { + switch (o) { + case LT: // < + case GT: // > + case LE: // <= + case GE: // >= + case INSTANCEOF: + return OperatorAssociativity.NONE; + default: + return OperatorAssociativity.LEFT; + } + } + + /** + * Get the associativity of a unary operator, as defined by + * https://introcs.cs.princeton.edu/java/11precedence/ + * + * All unary operators are right-associative, except for post increment and decrement, which + * are not associative. + * + * @param o A unary operator kind. + * @return The associativity of the operator. + */ + public static OperatorAssociativity getOperatorAssociativity(UnaryOperatorKind o) { + switch (o) { + case POSTINC: + case POSTDEC: + return OperatorAssociativity.NONE; + default: + return OperatorAssociativity.RIGHT; + } + } } diff --git a/src/main/java/spoon/reflect/visitor/RoundBracketAnalyzer.java b/src/main/java/spoon/reflect/visitor/RoundBracketAnalyzer.java new file mode 100644 index 00000000000..270ac436c73 --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/RoundBracketAnalyzer.java @@ -0,0 +1,114 @@ +/** + * SPDX-License-Identifier: (MIT OR CECILL-C) + * + * Copyright (C) 2006-2019 INRIA and contributors + * + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. + */ +package spoon.reflect.visitor; + +import spoon.reflect.code.CtBinaryOperator; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtUnaryOperator; +import spoon.reflect.declaration.CtElement; + +/** + * Class for determining whether or not an expression requires round brackets in order to preserve + * AST structure (and consequently semantics). + */ +class RoundBracketAnalyzer { + + enum EncloseInRoundBrackets { + YES, NO, UNKNOWN; + } + + private RoundBracketAnalyzer() { + } + + /** + * @param expr A unary or binary expr. + * @return true if the expr should be enclosed in round brackets. + */ + static EncloseInRoundBrackets requiresRoundBrackets(CtExpression expr) { + return isNestedOperator(expr) + ? nestedOperatorRequiresRoundBrackets(expr) + : EncloseInRoundBrackets.UNKNOWN; + } + + /** + * Assuming that operator is a nested operator (i.e. both operator and its parent are + * {@link CtUnaryOperator} or {@link CtBinaryOperator}), determine whether or not it must be + * enclosed in round brackets. + * + * Given an element e with a parent p, we must parenthesize + * e if any of the following are true. + * + * + * + * Note that the final rule is necessary to preserve syntactical structure, but it is not + * required for preserving semantics. + * + * @param nestedOperator A nested operator. + * @return Whether or not to enclose the nested operator in round brackets. + */ + private static EncloseInRoundBrackets nestedOperatorRequiresRoundBrackets(CtExpression nestedOperator) { + if (nestedOperator.getParent() instanceof CtUnaryOperator) { + return EncloseInRoundBrackets.YES; + } + + OperatorHelper.OperatorAssociativity associativity = getOperatorAssociativity(nestedOperator); + OperatorHelper.OperatorAssociativity positionInParent = getPositionInParent(nestedOperator); + + int parentPrecedence = getOperatorPrecedence(nestedOperator.getParent()); + int precedence = getOperatorPrecedence(nestedOperator); + return precedence < parentPrecedence + || (precedence == parentPrecedence && associativity != positionInParent) + ? EncloseInRoundBrackets.YES + : EncloseInRoundBrackets.NO; + } + + private static boolean isNestedOperator(CtElement e) { + return e.isParentInitialized() && isOperator(e) && isOperator(e.getParent()); + } + + private static boolean isOperator(CtElement e) { + return e instanceof CtBinaryOperator || e instanceof CtUnaryOperator; + } + + private static int getOperatorPrecedence(CtElement e) { + if (e instanceof CtBinaryOperator) { + return OperatorHelper.getOperatorPrecedence(((CtBinaryOperator) e).getKind()); + } else if (e instanceof CtUnaryOperator) { + return OperatorHelper.getOperatorPrecedence(((CtUnaryOperator) e).getKind()); + } else { + return 0; + } + } + + private static OperatorHelper.OperatorAssociativity getOperatorAssociativity(CtElement e) { + if (e instanceof CtBinaryOperator) { + return OperatorHelper.getOperatorAssociativity(((CtBinaryOperator) e).getKind()); + } else if (e instanceof CtUnaryOperator) { + return OperatorHelper.getOperatorAssociativity(((CtUnaryOperator) e).getKind()); + } else { + return OperatorHelper.OperatorAssociativity.NONE; + } + } + + private static OperatorHelper.OperatorAssociativity getPositionInParent(CtElement e) { + CtElement parent = e.getParent(); + if (parent instanceof CtBinaryOperator) { + return ((CtBinaryOperator) parent).getLeftHandOperand() == e + ? OperatorHelper.OperatorAssociativity.LEFT + : OperatorHelper.OperatorAssociativity.RIGHT; + } else { + return OperatorHelper.OperatorAssociativity.NONE; + } + } +} diff --git a/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java b/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java index dc7dd080ba1..8e2436de826 100644 --- a/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java +++ b/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java @@ -82,6 +82,9 @@ public SniperJavaPrettyPrinter(Environment env) { // newly added elements are not fully qualified this.setIgnoreImplicit(false); + + // don't print redundant parentheses + this.setMinimizeRoundBrackets(true); } /** diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java new file mode 100644 index 00000000000..82d8d5b8e04 --- /dev/null +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -0,0 +1,63 @@ +package spoon.reflect.visitor; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import spoon.Launcher; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtStatement; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +public class DefaultJavaPrettyPrinterTest { + + @ParameterizedTest + @ValueSource(strings = { + "1 + 2 + 3", + "1 + (2 + 3)", + "1 + 2 + -3", + "1 + 2 + -(2 + 3)", + "\"Sum: \" + (1 + 2)", + "\"Sum: \" + 1 + 2", + "-(1 + 2 + 3)", + "true || true && false", + "(true || false) && false", + "1 | 2 | 3", + "1 | (2 | 3)", + "1 | 2 & 3", + "(1 | 2) & 3", + "1 | 2 ^ 3", + "(1 | 2) ^ 3" + }) + public void testParenOptimizationCorrectlyPrintsParenthesesForExpressions(String rawExpression) { + // contract: When input expressions are minimally parenthesized, pretty-printed output + // should match the input + CtExpression expr = createLauncherWithOptimizeParenthesesPrinter() + .getFactory().createCodeSnippetExpression(rawExpression).compile(); + assertThat(expr.toString(), equalTo(rawExpression)); + } + + @ParameterizedTest + @ValueSource(strings = { + "int sum = 1 + 2 + 3", + "java.lang.String s = \"Sum: \" + (1 + 2)", + "java.lang.String s = \"Sum: \" + 1 + 2" + }) + public void testParenOptimizationCorrectlyPrintsParenthesesForStatements(String rawStatement) { + // contract: When input expressions as part of statements are minimally parenthesized, + // pretty-printed output should match the input + CtStatement statement = createLauncherWithOptimizeParenthesesPrinter() + .getFactory().createCodeSnippetStatement(rawStatement).compile(); + assertThat(statement.toString(), equalTo(rawStatement)); + } + + private static Launcher createLauncherWithOptimizeParenthesesPrinter() { + Launcher launcher = new Launcher(); + launcher.getEnvironment().setPrettyPrinterCreator(() -> { + DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(launcher.getEnvironment()); + printer.setMinimizeRoundBrackets(true); + return printer; + }); + return launcher; + } +} diff --git a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java index fe33ca9e1c5..023168dbc4b 100644 --- a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java +++ b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java @@ -608,6 +608,25 @@ public void testDefaultsToSingleTabIndentationWhenThereAreNoTypeMembers() { }); } + @Test + public void testOptimizesParenthesesForAddedNestedOperators() { + // contract: The sniper printer should optimize parentheses for newly inserted elements + + // without parentheses optimization, the expression will be printed as `(1 + 2) + (-(2 + 3))` + String declaration = "int a = 1 + 2 + -(2 + 3)"; + Launcher launcher = new Launcher(); + CtStatement nestedOps = launcher.getFactory().createCodeSnippetStatement(declaration).compile(); + + Consumer> addNestedOperator = type -> { + CtMethod method = type.getMethodsByName("main").get(0); + method.getBody().addStatement(nestedOps); + }; + BiConsumer, String> assertCorrectlyPrinted = + (type, result) -> assertThat(result, containsString(declaration)); + + testSniper("methodimport.ClassWithStaticMethod", addNestedOperator, assertCorrectlyPrinted); + } + @Test public void testPrintTypeWithMethodImportAboveMethodDefinition() { // contract: The type references of a method import (e.g. its return type) has source