Skip to content

Commit

Permalink
Revert parenthesis changes (openrewrite#4818)
Browse files Browse the repository at this point in the history 
* Revert "Try alternative way of determining parenthesis level for `BinaryExpression` when AST doesn't provide `_INSIDE_PARENTHESES_LEVEL` flag (openrewrite#4807)"

This reverts commit e59e48b.

* Revert "Make Groovy Parser correctly handle nested parenthesis (openrewrite#4801)"

This reverts commit 91a031a.
  • Loading branch information
@timtebeek @amishra-u
timtebeek authored and amishra-u committed Dec 28, 2024
1 parent 13cd88c commit fb008d8
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 259 deletions.
10 changes: 0 additions & 10 deletions rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,14 +720,4 @@ public static String formatUriForPropertiesFile(String uri) {
public static boolean hasLineBreak(@Nullable String s) {
return s != null && LINE_BREAK.matcher(s).find();
}

public static boolean containsWhitespace(String s) {
for (int i = 0; i < s.length(); ++i) {
if (Character.isWhitespace(s.charAt(i))) {
return true;
}
}

return false;
}
}
146 changes: 51 additions & 95 deletions rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.openrewrite.groovy.tree.G;
import org.openrewrite.internal.EncodingDetectingInputStream;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.internal.StringUtils;
import org.openrewrite.java.internal.JavaTypeCache;
import org.openrewrite.java.marker.ImplicitReturn;
import org.openrewrite.java.marker.OmitParentheses;
Expand Down Expand Up @@ -1351,20 +1350,20 @@ public void visitConstantExpression(ConstantExpression expression) {
jType = JavaType.Primitive.Short;
} else if (type == ClassHelper.STRING_TYPE) {
jType = JavaType.Primitive.String;
// String literals value returned by getValue()/getText() has already processed sequences like "\\" -> "\"
int length = sourceLengthOfString(expression);
// this is an attribute selector
boolean attributeSelector = source.startsWith("@" + value, cursor);
int length = lengthAccordingToAst(expression);
Integer insideParenthesesLevel = getInsideParenthesesLevel(expression);
if (insideParenthesesLevel != null) {
length = length - insideParenthesesLevel * 2;
if (source.startsWith("@" + value, cursor)) {
length += 1;
}
String valueAccordingToAST = source.substring(cursor, cursor + length + (attributeSelector ? 1 : 0));
int delimiterLength = getDelimiterLength();
if (StringUtils.containsWhitespace(valueAccordingToAST)) {
length = delimiterLength + expression.getValue().toString().length() + delimiterLength;
text = source.substring(cursor, cursor + length + (attributeSelector ? 1 : 0));
} else {
text = valueAccordingToAST;
text = source.substring(cursor, cursor + length);
int delimiterLength = 0;
if (text.startsWith("$/")) {
delimiterLength = 2;
} else if (text.startsWith("\"\"\"") || text.startsWith("'''")) {
delimiterLength = 3;
} else if (text.startsWith("/") || text.startsWith("\"") || text.startsWith("'")) {
delimiterLength = 1;
}
value = text.substring(delimiterLength, text.length() - delimiterLength);
} else if (expression.isNullExpression()) {
Expand Down Expand Up @@ -1691,18 +1690,15 @@ public void visitGStringExpression(GStringExpression gstring) {

@Override
public void visitListExpression(ListExpression list) {
queue.add(insideParentheses(list, fmt -> {
skip("[");
if (list.getExpressions().isEmpty()) {
return new G.ListLiteral(randomId(), fmt, Markers.EMPTY,
JContainer.build(singletonList(new JRightPadded<>(new J.Empty(randomId(), EMPTY, Markers.EMPTY), sourceBefore("]"), Markers.EMPTY))),
typeMapping.type(list.getType()));
} else {
return new G.ListLiteral(randomId(), fmt, Markers.EMPTY,
JContainer.build(visitRightPadded(list.getExpressions().toArray(new ASTNode[0]), "]")),
typeMapping.type(list.getType()));
}
}));
if (list.getExpressions().isEmpty()) {
queue.add(new G.ListLiteral(randomId(), sourceBefore("["), Markers.EMPTY,
JContainer.build(singletonList(new JRightPadded<>(new J.Empty(randomId(), EMPTY, Markers.EMPTY), sourceBefore("]"), Markers.EMPTY))),
typeMapping.type(list.getType())));
} else {
queue.add(new G.ListLiteral(randomId(), sourceBefore("["), Markers.EMPTY,
JContainer.build(visitRightPadded(list.getExpressions().toArray(new ASTNode[0]), "]")),
typeMapping.type(list.getType())));
}
}

@Override
Expand All @@ -1717,19 +1713,17 @@ public void visitMapEntryExpression(MapEntryExpression expression) {

@Override
public void visitMapExpression(MapExpression map) {
queue.add(insideParentheses(map, fmt -> {
skip("[");
JContainer<G.MapEntry> entries;
if (map.getMapEntryExpressions().isEmpty()) {
entries = JContainer.build(Collections.singletonList(JRightPadded.build(
new G.MapEntry(randomId(), whitespace(), Markers.EMPTY,
JRightPadded.build(new J.Empty(randomId(), sourceBefore(":"), Markers.EMPTY)),
new J.Empty(randomId(), sourceBefore("]"), Markers.EMPTY), null))));
} else {
entries = JContainer.build(visitRightPadded(map.getMapEntryExpressions().toArray(new ASTNode[0]), "]"));
}
return new G.MapLiteral(randomId(), fmt, Markers.EMPTY, entries, typeMapping.type(map.getType()));
}));
Space prefix = sourceBefore("[");
JContainer<G.MapEntry> entries;
if (map.getMapEntryExpressions().isEmpty()) {
entries = JContainer.build(Collections.singletonList(JRightPadded.build(
new G.MapEntry(randomId(), whitespace(), Markers.EMPTY,
JRightPadded.build(new J.Empty(randomId(), sourceBefore(":"), Markers.EMPTY)),
new J.Empty(randomId(), sourceBefore("]"), Markers.EMPTY), null))));
} else {
entries = JContainer.build(visitRightPadded(map.getMapEntryExpressions().toArray(new ASTNode[0]), "]"));
}
queue.add(new G.MapLiteral(randomId(), prefix, Markers.EMPTY, entries, typeMapping.type(map.getType())));
}

@Override
Expand Down Expand Up @@ -1907,24 +1901,23 @@ public void visitStaticMethodCallExpression(StaticMethodCallExpression call) {

@Override
public void visitAttributeExpression(AttributeExpression attr) {
queue.add(insideParentheses(attr, fmt -> {
Expression target = visit(attr.getObjectExpression());
Space beforeDot = attr.isSafe() ? sourceBefore("?.") :
sourceBefore(attr.isSpreadSafe() ? "*." : ".");
J name = visit(attr.getProperty());
if (name instanceof J.Literal) {
String nameStr = ((J.Literal) name).getValueSource();
assert nameStr != null;
name = new J.Identifier(randomId(), name.getPrefix(), Markers.EMPTY, emptyList(), nameStr, null, null);
}
if (attr.isSpreadSafe()) {
name = name.withMarkers(name.getMarkers().add(new StarDot(randomId())));
}
if (attr.isSafe()) {
name = name.withMarkers(name.getMarkers().add(new NullSafe(randomId())));
}
return new J.FieldAccess(randomId(), fmt, Markers.EMPTY, target, padLeft(beforeDot, (J.Identifier) name), null);
}));
Space fmt = whitespace();
Expression target = visit(attr.getObjectExpression());
Space beforeDot = attr.isSafe() ? sourceBefore("?.") :
sourceBefore(attr.isSpreadSafe() ? "*." : ".");
J name = visit(attr.getProperty());
if (name instanceof J.Literal) {
String nameStr = ((J.Literal) name).getValueSource();
assert nameStr != null;
name = new J.Identifier(randomId(), name.getPrefix(), Markers.EMPTY, emptyList(), nameStr, null, null);
}
if (attr.isSpreadSafe()) {
name = name.withMarkers(name.getMarkers().add(new StarDot(randomId())));
}
if (attr.isSafe()) {
name = name.withMarkers(name.getMarkers().add(new NullSafe(randomId())));
}
queue.add(new J.FieldAccess(randomId(), fmt, Markers.EMPTY, target, padLeft(beforeDot, (J.Identifier) name), null));
}

@Override
Expand Down Expand Up @@ -2529,52 +2522,15 @@ private int sourceLengthOfString(ConstantExpression expr) {
return lengthAccordingToAst;
}

private @Nullable Integer getInsideParenthesesLevel(ASTNode node) {
private static @Nullable Integer getInsideParenthesesLevel(ASTNode node) {
Object rawIpl = node.getNodeMetaData("_INSIDE_PARENTHESES_LEVEL");
if (rawIpl instanceof AtomicInteger) {
// On Java 11 and newer _INSIDE_PARENTHESES_LEVEL is an AtomicInteger
return ((AtomicInteger) rawIpl).get();
} else if (rawIpl instanceof Integer) {
} else {
// On Java 8 _INSIDE_PARENTHESES_LEVEL is a regular Integer
return (Integer) rawIpl;
} else if (node instanceof MethodCallExpression) {
MethodCallExpression expr = (MethodCallExpression) node;
return determineParenthesisLevel(expr.getObjectExpression().getLineNumber(), expr.getLineNumber(), expr.getObjectExpression().getColumnNumber(), expr.getColumnNumber());
} else if (node instanceof BinaryExpression) {
BinaryExpression expr = (BinaryExpression) node;
return determineParenthesisLevel(expr.getLeftExpression().getLineNumber(), expr.getLineNumber(), expr.getLeftExpression().getColumnNumber(), expr.getColumnNumber());

}
return null;
}

/**
* @param childLineNumber the beginning line number of the first sub node
* @param parentLineNumber the beginning line number of the parent node
* @param childColumn the column on the {@code childLineNumber} line where the sub node starts
* @param parentColumn the column on the {@code parentLineNumber} line where the parent node starts
* @return the level of parenthesis parsed from the source
*/
private int determineParenthesisLevel(int childLineNumber, int parentLineNumber, int childColumn, int parentColumn) {
int saveCursor = cursor;
whitespace();
int childBeginCursor = cursor;
if (childLineNumber > parentLineNumber) {
for (int i = 0; i < (childColumn - parentLineNumber); i++) {
childBeginCursor = source.indexOf('\n', childBeginCursor);
}
childBeginCursor += childColumn;
} else {
childBeginCursor += childColumn - parentColumn;
}
int count = 0;
for (int i = cursor; i < childBeginCursor; i++) {
if (source.charAt(i) == '(') {
count++;
}
}
cursor = saveCursor;
return count;
}

private int getDelimiterLength() {
Expand Down
25 changes: 9 additions & 16 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AttributeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,19 @@

import static org.openrewrite.groovy.Assertions.groovy;

@SuppressWarnings({"GroovyUnusedAssignment", "GrUnnecessarySemicolon"})
class AttributeTest implements RewriteTest {

@Test
void attribute() {
void usingGroovyNode() {
rewriteRun(
groovy("new User('Bob').@name")
);
}

@Test
void attributeInClosure() {
rewriteRun(
groovy("[new User('Bob')].collect { it.@name }")
);
}

@Test
void attributeWithParentheses() {
rewriteRun(
groovy("(new User('Bob').@name)")
groovy(
"""
def xml = new Node(null, "ivy")
def n = xml.dependencies.dependency.find { it.@name == 'test-module' }
n.@conf = 'runtime->default;docs->docs;sources->sources'
"""
)
);
}
}
31 changes: 1 addition & 30 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.openrewrite.groovy.tree;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.openrewrite.Issue;
import org.openrewrite.test.RewriteTest;
Expand All @@ -33,7 +34,6 @@ void insideParentheses() {

// NOT inside parentheses, but verifies the parser's
// test for "inside parentheses" condition
groovy("( 1 ) + 1"),
groovy("(1) + 1"),
// And combine the two cases
groovy("((1) + 1)")
Expand Down Expand Up @@ -216,35 +216,6 @@ void stringMultipliedInParentheses() {
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4703")
@Test
void cxds() {
rewriteRun(
groovy(
"""
def differenceInDays(int time) {
return (int) ((time)/(1000*60*60*24))
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4703")
@Test
void extraParensAroundInfixOxxxperator() {
rewriteRun(
groovy(
"""
def timestamp(int hours, int minutes, int seconds) {
30 * (hours)
(((((hours))))) * 30
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/4703")
@Test
void extraParensAroundInfixOperator() {
Expand Down
17 changes: 4 additions & 13 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.openrewrite.groovy.tree;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.ExpectedToFail;
import org.openrewrite.Issue;
import org.openrewrite.test.RewriteTest;

Expand All @@ -24,18 +25,6 @@
@SuppressWarnings({"UnnecessaryQualifiedReference", "GroovyUnusedAssignment", "GrUnnecessarySemicolon"})
class CastTest implements RewriteTest {

@Test
void cast() {
rewriteRun(
groovy(
"""
String foo = ( String ) "hallo"
String bar = "hallo" as String
"""
)
);
}

@Test
void javaStyleCast() {
rewriteRun(
Expand Down Expand Up @@ -85,13 +74,14 @@ void groovyCastAndInvokeMethod() {
rewriteRun(
groovy(
"""
( "x" as String ).toString()
( "" as String ).toString()
"""
)
);
}

@Test
@ExpectedToFail("Parentheses with method invocation is not yet supported")
void groovyCastAndInvokeMethodWithParentheses() {
rewriteRun(
groovy(
Expand All @@ -113,6 +103,7 @@ void javaCastAndInvokeMethod() {
);
}

@ExpectedToFail("Parentheses with method invocation is not yet supported")
@Test
void javaCastAndInvokeMethodWithParentheses() {
rewriteRun(
Expand Down
34 changes: 0 additions & 34 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ListTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,6 @@
@SuppressWarnings("GroovyUnusedAssignment")
class ListTest implements RewriteTest {

@Test
void emptyListLiteral() {
rewriteRun(
groovy(
"""
def a = []
def b = [ ]
"""
)
);
}

@Test
void emptyListLiteralWithParentheses() {
rewriteRun(
groovy(
"""
def y = ([])
"""
)
);
}

@Test
void listLiteral() {
rewriteRun(
Expand All @@ -56,15 +33,4 @@ void listLiteral() {
)
);
}

@Test
void listLiteralTrailingComma() {
rewriteRun(
groovy(
"""
def a = [ "foo" /* "foo" suffix */ , /* "]" prefix */ ]
"""
)
);
}
}
Loading

0 comments on commit fb008d8

Please sign in to comment.