Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: print minimal amount of round brackets in sniper mode #3823

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a438c09
Add naive implementation that considers only precedence
slarse Mar 4, 2021
606c90a
Preserve AST structure
slarse Mar 4, 2021
f9038e7
Add test to verify that unary operator parentheses are kept
slarse Mar 4, 2021
5680169
Refactor duplicated test logic into parameterized test
slarse Mar 4, 2021
a9bdf76
Fix parenthesis optimization associativity rule when parent has highe…
slarse Mar 4, 2021
6dc3bed
Add another expression
slarse Mar 4, 2021
e2115a3
Add parameterized test for testing statements
slarse Mar 4, 2021
89ff049
Add support for optimizing parentheses of unary operators
slarse Mar 4, 2021
f1aa4d4
Positiong overloads next to each other
slarse Mar 4, 2021
6f91d65
Add test expressions for bitwise operators
slarse Mar 4, 2021
848402a
Add explicit contracts to test cases
slarse Mar 4, 2021
43f7d95
Refactor test cases
slarse Mar 4, 2021
bf84230
Fix style issues
slarse Mar 4, 2021
9d00a64
Refactor
slarse Mar 4, 2021
41db039
Remove unused imports
slarse Mar 4, 2021
0ff1a45
Clarify intent by removing negation
slarse Mar 4, 2021
f831167
Adjust note on optimizeParentheses field
slarse Mar 4, 2021
03de60d
Merge branch 'master' into issue/3809-print-minimal-brackets-for-uop-…
slarse Mar 8, 2021
ea40045
Refactor parenthesis calculation into separate class
slarse Mar 8, 2021
636624e
Activate parenthesis optimization for sniper printer
slarse Mar 8, 2021
ad087f8
Document parenthesis optimization
slarse Mar 8, 2021
2e2c406
Reformat ParenOptimizer with tabs
slarse Mar 8, 2021
df6bcf1
Add private constructor to ParenOptimizer
slarse Mar 8, 2021
e584ae5
Revert redundant whitespace changes
slarse Mar 8, 2021
b664773
Remove redundant blank line
slarse Mar 8, 2021
2e83211
Add license header
slarse Mar 8, 2021
e5c82d5
Fix broken test
slarse Mar 8, 2021
db058e1
Revise terminology: parenthesis -> round bracket, optimize -> minimize
slarse Mar 9, 2021
73338f7
Fix indentation
slarse Mar 9, 2021
a6a43e9
Remove funky header formatting inserted by IntelliJ
slarse Mar 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Revise terminology: parenthesis -> round bracket, optimize -> minimize
  • Loading branch information
slarse committed Mar 9, 2021
commit db058e1431d14e5c259be2a47d1bd5721587dec0
39 changes: 20 additions & 19 deletions src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,10 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter {
protected boolean ignoreImplicit = true;

/**
* EXPERIMENTAL: If true, the printer will attempt to print a minimal set of parentheses that
* preserve the syntactical structure of the AST.
* 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 optimizeParentheses = false;
private boolean minimizeRoundBrackets = false;

public boolean inlineElseIf = true;

Expand Down Expand Up @@ -397,10 +397,11 @@ private boolean shouldSetBracket(CtExpression<?> e) {
return true;
}
try {
if (isOptimizeParentheses()) {
ParenOptimizer.MustParenthesize mustSetParens = ParenOptimizer.mustParenthesize(e);
if (mustSetParens != ParenOptimizer.MustParenthesize.UNKNOWN) {
return mustSetParens == ParenOptimizer.MustParenthesize.YES;
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)) {
Expand Down Expand Up @@ -2144,27 +2145,27 @@ public void visitCtYieldStatement(CtYieldStatement statement) {
}

/**
* @return true if the printer is optimizing parentheses around expressions
* @return true if the printer is minimizing the amount of round brackets in expressions
*/
protected boolean isOptimizeParentheses() {
return optimizeParentheses;
protected boolean isMinimizeRoundBrackets() {
return minimizeRoundBrackets;
}

/**
* When set to true, this activates parenthesis optimization for expressions. This means that
* the printer will attempt to only write those strictly necessary for preserving syntactical
* structure (and by extension, semantics).
* 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 <code>1 + 2 + 3 + 4</code> is written as
* <code>((1 + 2) + 3) + 4</code> without parenthesis optimization, but entirely without
* parentheses when optimization is enabled. However, an expression <code>1 + 2 + (3 + 4)</code>
* <code>((1 + 2) + 3) + 4</code> without round bracket minimization, but entirely without
* parentheses when minimization is enabled. However, an expression <code>1 + 2 + (3 + 4)</code>
* is still written as <code>1 + 2 + (3 + 4)</code> to preserve syntactical structure, even though
* the parentheses are semantically redundant.
* the brackets are semantically redundant.
*
* @param optimizeParentheses set whether or not to optimize parentheses around expressions
* @param minimizeRoundBrackets set whether or not to minimize round brackets in expressions
*/
protected void setOptimizeParentheses(boolean optimizeParentheses) {
this.optimizeParentheses = optimizeParentheses;
protected void setMinimizeRoundBrackets(boolean minimizeRoundBrackets) {
this.minimizeRoundBrackets = minimizeRoundBrackets;
}

}
115 changes: 0 additions & 115 deletions src/main/java/spoon/reflect/visitor/ParenOptimizer.java

This file was deleted.

114 changes: 114 additions & 0 deletions src/main/java/spoon/reflect/visitor/RoundBracketAnalyzer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* SPDX-License-Identifier: (MIT OR CECILL-C)
* <p>
* Copyright (C) 2006-2019 INRIA and contributors
* <p>
* 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 <code>e</code> with a parent <code>p</code>, we must parenthesize
* <code>e</code> if any of the following are true.
*
* <ul>
* <li>The parent p is a unary operator</li>
* <li>The parent p is a binary operator, and <code>precedence(p) > precedence(e></code></li>
* <li>The parent p is a binary operator, <code>precedence(p) == precedence(e)</code>,
* e appears as the X-hand-side operand of p, and e's operator is Y-associative, where
* <code>X != Y</code></li>
* </ul>
*
* 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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public SniperJavaPrettyPrinter(Environment env) {
this.setIgnoreImplicit(false);

// don't print redundant parentheses
this.setOptimizeParentheses(true);
this.setMinimizeRoundBrackets(true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ private static Launcher createLauncherWithOptimizeParenthesesPrinter() {
Launcher launcher = new Launcher();
launcher.getEnvironment().setPrettyPrinterCreator(() -> {
DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(launcher.getEnvironment());
printer.setOptimizeParentheses(true);
printer.setMinimizeRoundBrackets(true);
return printer;
});
return launcher;
Expand Down