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 4f0f76ae578..7cafdeb341a 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/StringUtils.java @@ -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; - } } 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 af660485f32..c631e3998a1 100644 --- a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java +++ b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java @@ -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; @@ -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()) { @@ -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 @@ -1717,19 +1713,17 @@ public void visitMapEntryExpression(MapEntryExpression expression) { @Override public void visitMapExpression(MapExpression map) { - 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())); - })); + 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()))); } @Override @@ -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 @@ -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() { 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 2b7d4c82756..bfbadb76655 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,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' + """ + ) ); } } 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 aca502d3571..13239c0fc76 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,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; @@ -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)") @@ -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() { 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 4c009de95e3..5cf82ae85be 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,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; @@ -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( @@ -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( @@ -113,6 +103,7 @@ 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 d921377b661..b42f9579fd4 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,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( @@ -56,15 +33,4 @@ 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 12d638870d4..46851b2f65a 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,6 +216,18 @@ void literalValueAndTypeAgree() { ); } + @Test + void emptyListLiteral() { + rewriteRun( + groovy( + """ + def a = [] + def b = [ ] + """ + ) + ); + } + @Test void multilineStringWithApostrophes() { rewriteRun( @@ -242,6 +254,17 @@ 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 67ee80177cd..65bb4a17492 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,13 +55,6 @@ 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 42a73d160a0..a0714126901 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,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; @@ -30,11 +31,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' @@ -45,6 +46,7 @@ void gradle() { ); } + @ExpectedToFail("Parentheses with method invocation is not yet supported") @Test @Issue("https://github.com/openrewrite/rewrite/issues/4615") void gradleWithParentheses() { @@ -347,7 +349,7 @@ class StringUtils { static boolean isEmpty(String value) { return value == null || value.isEmpty() } - + static void main(String[] args) { isEmpty("") } @@ -357,29 +359,7 @@ static void main(String[] args) { ); } - @Issue("https://github.com/openrewrite/rewrite/issues/4703") - @Test - void insideParenthesesSimple() { - rewriteRun( - groovy( - """ - ((a.invoke "b" )) - """ - ) - ); - } - - @Test - void lotOfSpacesAroundConstantWithParentheses() { - rewriteRun( - groovy( - """ - ( ( ( "x" ) ).toString() ) - """ - ) - ); - } - + @ExpectedToFail("Parentheses with method invocation is not yet supported") @Issue("https://github.com/openrewrite/rewrite/issues/4703") @Test void insideParentheses() { @@ -395,21 +375,7 @@ static def foo(Map map) { ); } - @Test - void insideParenthesesWithNewline() { - rewriteRun( - groovy( - """ - static def foo(Map map) { - (( - map.containsKey("foo")) - && ((map.get("foo")).equals("bar"))) - } - """ - ) - ); - } - + @ExpectedToFail("Parentheses with method invocation is not yet supported") @Issue("https://github.com/openrewrite/rewrite/issues/4703") @Test void insideParenthesesWithoutNewLineAndEscapedMethodName() { @@ -421,17 +387,4 @@ 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 e0ff32f3d94..ac839e34d88 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,6 +16,7 @@ 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; @@ -47,6 +48,7 @@ void parenthesized() { ); } + @ExpectedToFail("Parentheses with method invocation is not yet supported") @Test void parenthesizedAndInvokeMethodWithParentheses() { rewriteRun(