diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java b/rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java index 7cafdeb341a..4f0f76ae578 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java @@ -720,4 +720,14 @@ 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; + } } diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java index abf35207e2e..2e8b26ae7b3 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java @@ -34,6 +34,7 @@ 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; @@ -1350,20 +1351,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 - if (source.startsWith("@" + value, cursor)) { - length += 1; + boolean attributeSelector = source.startsWith("@" + value, cursor); + int length = lengthAccordingToAst(expression); + Integer insideParenthesesLevel = getInsideParenthesesLevel(expression); + if (insideParenthesesLevel != null) { + length = length - insideParenthesesLevel * 2; } - 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; + 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; } value = text.substring(delimiterLength, text.length() - delimiterLength); } else if (expression.isNullExpression()) { @@ -1690,15 +1691,18 @@ public void visitGStringExpression(GStringExpression gstring) { @Override public void visitListExpression(ListExpression list) { - 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()))); - } + 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())); + } + })); } @Override @@ -1713,17 +1717,19 @@ public void visitMapEntryExpression(MapEntryExpression expression) { @Override public void visitMapExpression(MapExpression map) { - Space prefix = sourceBefore("["); - JContainer 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()))); + queue.add(insideParentheses(map, fmt -> { + skip("["); + JContainer 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())); + })); } @Override @@ -1901,23 +1907,24 @@ public void visitStaticMethodCallExpression(StaticMethodCallExpression call) { @Override public void visitAttributeExpression(AttributeExpression attr) { - 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)); + 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); + })); } @Override @@ -2522,15 +2529,48 @@ private int sourceLengthOfString(ConstantExpression expr) { return lengthAccordingToAst; } - private static @Nullable Integer getInsideParenthesesLevel(ASTNode node) { + private @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 { + } else if (rawIpl instanceof Integer) { // 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()); } + 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() { diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AttributeTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AttributeTest.java index bfbadb76655..2b7d4c82756 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AttributeTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AttributeTest.java @@ -20,19 +20,26 @@ import static org.openrewrite.groovy.Assertions.groovy; -@SuppressWarnings({"GroovyUnusedAssignment", "GrUnnecessarySemicolon"}) class AttributeTest implements RewriteTest { @Test - void usingGroovyNode() { + void attribute() { rewriteRun( - 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' - """ - ) + 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)") ); } } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java index 13239c0fc76..aca502d3571 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/BinaryTest.java @@ -15,7 +15,6 @@ */ 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; @@ -34,6 +33,7 @@ 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)") @@ -216,6 +216,35 @@ 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() { diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java index 5cf82ae85be..4c009de95e3 100755 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/CastTest.java @@ -16,7 +16,6 @@ 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; @@ -25,6 +24,18 @@ @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( @@ -74,14 +85,13 @@ void groovyCastAndInvokeMethod() { rewriteRun( groovy( """ - ( "" as String ).toString() + ( "x" as String ).toString() """ ) ); } @Test - @ExpectedToFail("Parentheses with method invocation is not yet supported") void groovyCastAndInvokeMethodWithParentheses() { rewriteRun( groovy( @@ -103,7 +113,6 @@ void javaCastAndInvokeMethod() { ); } - @ExpectedToFail("Parentheses with method invocation is not yet supported") @Test void javaCastAndInvokeMethodWithParentheses() { rewriteRun( diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ListTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ListTest.java index b42f9579fd4..d921377b661 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ListTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ListTest.java @@ -23,6 +23,29 @@ @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( @@ -33,4 +56,15 @@ void listLiteral() { ) ); } + + @Test + void listLiteralTrailingComma() { + rewriteRun( + groovy( + """ + def a = [ "foo" /* "foo" suffix */ , /* "]" prefix */ ] + """ + ) + ); + } } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java index 46851b2f65a..12d638870d4 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java @@ -216,18 +216,6 @@ void literalValueAndTypeAgree() { ); } - @Test - void emptyListLiteral() { - rewriteRun( - groovy( - """ - def a = [] - def b = [ ] - """ - ) - ); - } - @Test void multilineStringWithApostrophes() { rewriteRun( @@ -254,17 +242,6 @@ void mapLiteralTrailingComma() { ); } - @Test - void listLiteralTrailingComma() { - rewriteRun( - groovy( - """ - def a = [ "foo" /* "foo" suffix */ , /* "]" prefix */ ] - """ - ) - ); - } - @Test void gStringThatHasEmptyValueExpressionForUnknownReason() { rewriteRun( diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MapEntryTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MapEntryTest.java index 65bb4a17492..67ee80177cd 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MapEntryTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MapEntryTest.java @@ -55,6 +55,13 @@ void emptyMapLiteral() { ); } + @Test + void emptyMapLiteralWithParentheses() { + rewriteRun( + groovy("Map m = ([ : ])") + ); + } + @Test void mapAccess() { rewriteRun( diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java index a0714126901..42a73d160a0 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java @@ -16,7 +16,6 @@ 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; @@ -31,11 +30,11 @@ void gradle() { plugins { id 'java-library' } - + repositories { mavenCentral() } - + dependencies { implementation 'org.hibernate:hibernate-core:3.6.7.Final' api 'com.google.guava:guava:23.0' @@ -46,7 +45,6 @@ void gradle() { ); } - @ExpectedToFail("Parentheses with method invocation is not yet supported") @Test @Issue("https://github.com/openrewrite/rewrite/issues/4615") void gradleWithParentheses() { @@ -349,7 +347,7 @@ class StringUtils { static boolean isEmpty(String value) { return value == null || value.isEmpty() } - + static void main(String[] args) { isEmpty("") } @@ -359,7 +357,29 @@ static void main(String[] args) { ); } - @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Issue("https://github.com/openrewrite/rewrite/issues/4703") + @Test + void insideParenthesesSimple() { + rewriteRun( + groovy( + """ + ((a.invoke "b" )) + """ + ) + ); + } + + @Test + void lotOfSpacesAroundConstantWithParentheses() { + rewriteRun( + groovy( + """ + ( ( ( "x" ) ).toString() ) + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/4703") @Test void insideParentheses() { @@ -375,7 +395,21 @@ static def foo(Map map) { ); } - @ExpectedToFail("Parentheses with method invocation is not yet supported") + @Test + void insideParenthesesWithNewline() { + rewriteRun( + groovy( + """ + static def foo(Map map) { + (( + map.containsKey("foo")) + && ((map.get("foo")).equals("bar"))) + } + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/4703") @Test void insideParenthesesWithoutNewLineAndEscapedMethodName() { @@ -387,4 +421,17 @@ static def foo(Map someMap) {((((((someMap.get("(bar")))) ).'equals' "baz" ) ) ) ); } + + @Test + void insideFourParenthesesAndEnters() { + rewriteRun( + groovy( + """ + (((( + something(a) + )))) + """ + ) + ); + } } diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java index ac839e34d88..e0ff32f3d94 100644 --- a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/RangeTest.java @@ -16,7 +16,6 @@ package org.openrewrite.groovy.tree; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.test.RewriteTest; import static org.openrewrite.groovy.Assertions.groovy; @@ -48,7 +47,6 @@ void parenthesized() { ); } - @ExpectedToFail("Parentheses with method invocation is not yet supported") @Test void parenthesizedAndInvokeMethodWithParentheses() { rewriteRun(