Skip to content

Commit

Permalink
Exclude parenthese determination from comments and string literals (#…
Browse files Browse the repository at this point in the history 
…4964)

* Exclude parenthese determination from comments

* Add literal test

* implementation

* improvement

* improvement

* improvement

* Added gStringWithStringLiteralsWithParentheses test

* improvement

* improvement

* Update rewrite-groovy/src/main/java/org/openrewrite/groovy/internal/Delimiter.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@jevanlingen @github-actions
jevanlingen and github-actions[bot] authored Jan 30, 2025
1 parent d33ff3f commit 63c9526
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 40 deletions.
77 changes: 45 additions & 32 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 @@ -32,6 +32,7 @@
import org.openrewrite.Cursor;
import org.openrewrite.ExecutionContext;
import org.openrewrite.FileAttributes;
import org.openrewrite.groovy.internal.Delimiter;
import org.openrewrite.groovy.marker.*;
import org.openrewrite.groovy.tree.G;
import org.openrewrite.internal.EncodingDetectingInputStream;
Expand Down Expand Up @@ -62,8 +63,8 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static org.openrewrite.Tree.randomId;
import static org.openrewrite.groovy.internal.Delimiter.*;
import static org.openrewrite.internal.StringUtils.indexOfNextNonWhitespace;
import static org.openrewrite.java.tree.Space.EMPTY;
import static org.openrewrite.java.tree.Space.format;
Expand Down Expand Up @@ -1337,13 +1338,11 @@ public void visitConstantExpression(ConstantExpression expression) {
value = "@" + value;
text = "@" + text;
} else {
String delimiter = getDelimiter();
Delimiter delimiter = getDelimiter(cursor);
if (delimiter != null) {
// handle the `pattern` operator
String endDelimiter = "~/".equals(delimiter) ? "/" : delimiter;
// Get the string literal from the source, so escaping of newlines and the like works out of the box
value = sourceSubstring(cursor + delimiter.length(), endDelimiter);
text = delimiter + value + endDelimiter;
value = sourceSubstring(cursor + delimiter.open.length(), delimiter.close);
text = delimiter.open + value + delimiter.close;
}
}
} else if (expression.isNullExpression()) {
Expand Down Expand Up @@ -1597,8 +1596,8 @@ public void visitIfElse(IfStatement ifElse) {
@Override
public void visitGStringExpression(GStringExpression gstring) {
Space fmt = whitespace();
String delimiter = getDelimiter();
skip(delimiter); // Opening delim for GString
Delimiter delimiter = getDelimiter(cursor);
skip(delimiter.open);

NavigableMap<LineColumn, org.codehaus.groovy.ast.expr.Expression> sortedByPosition = new TreeMap<>();
for (ConstantExpression e : gstring.getStrings()) {
Expand Down Expand Up @@ -1629,7 +1628,7 @@ public void visitGStringExpression(GStringExpression gstring) {
}
} else if (e instanceof ConstantExpression) {
// Get the string literal from the source, so escaping of newlines and the like works out of the box
String value = sourceSubstring(cursor, ("~/".equals(delimiter) ? "/" : delimiter));
String value = sourceSubstring(cursor, delimiter.close);
// There could be a closer GString before the end of the closing delimiter, so shorten the string if needs be
int indexNextSign = source.indexOf("$", cursor);
if (indexNextSign != -1 && indexNextSign < (cursor + value.length())) {
Expand All @@ -1643,8 +1642,8 @@ public void visitGStringExpression(GStringExpression gstring) {
}
}

queue.add(new G.GString(randomId(), fmt, Markers.EMPTY, delimiter, strings, typeMapping.type(gstring.getType())));
skip(delimiter); // Closing delim for GString
queue.add(new G.GString(randomId(), fmt, Markers.EMPTY, delimiter.open, strings, typeMapping.type(gstring.getType())));
skip(delimiter.close);
}

@Override
Expand Down Expand Up @@ -2537,34 +2536,48 @@ private int determineParenthesisLevel(int childLineNumber, int parentLineNumber,
int start = sourceLineNumberOffsets[startingLineNumber - 1] + startingColumn - 1;
int end = sourceLineNumberOffsets[endingLineNumber - 1] + endingColumn - 1;

// Determine level of parentheses by going through the source
// Determine level of parentheses by going through the source, don't count parentheses in comments, string literals and regexes
int count = 0;
Delimiter delimiter = null;
for (int i = start; i < end; i++) {
if (source.charAt(i) == '(') {
count++;
} else if (source.charAt(i) == ')') {
count--;
if (delimiter == null) {
delimiter = getDelimiter(i);
if (delimiter == null) {
if (source.charAt(i) == '(') {
count++;
} else if (source.charAt(i) == ')') {
count--;
}
} else {
i += delimiter.open.length() - 1; // skip the next chars for the rest of the delimiter
}
} else if (source.startsWith(delimiter.close, i)) {
i += delimiter.close.length() - 1; // skip the next chars for the rest of the delimiter
delimiter = null;
}
}
return Math.max(count, 0);
}

private @Nullable String getDelimiter() {
String maybeDelimiter = source.substring(cursor, Math.min(cursor + 3, source.length()));
if (maybeDelimiter.startsWith("$/")) {
return "$/";
} else if (maybeDelimiter.startsWith("\"\"\"")) {
return "\"\"\"";
} else if (maybeDelimiter.startsWith("'''")) {
return "'''";
} else if (maybeDelimiter.startsWith("~/")) {
return "~/";
} else if (maybeDelimiter.startsWith("/")) {
return "/";
} else if (maybeDelimiter.startsWith("\"")) {
return "\"";
} else if (maybeDelimiter.startsWith("'")) {
return "'";
private @Nullable Delimiter getDelimiter(int cursor) {
if (source.startsWith("$/", cursor)) {
return DOLLAR_SLASHY_STRING;
} else if (source.startsWith("\"\"\"", cursor)) {
return TRIPLE_DOUBLE_QUOTE_STRING;
} else if (source.startsWith("'''", cursor)) {
return TRIPLE_SINGLE_QUOTE_STRING;
} else if (source.startsWith("~/", cursor)) {
return PATTERN_OPERATOR;
} else if (source.startsWith("//", cursor)) {
return SINGLE_LINE_COMMENT;
} else if (source.startsWith("/*", cursor)) {
return MULTILINE_COMMENT;
} else if (source.startsWith("/", cursor)) {
return SLASHY_STRING;
} else if (source.startsWith("\"", cursor)) {
return DOUBLE_QUOTE_STRING;
} else if (source.startsWith("'", cursor)) {
return SINGLE_QUOTE_STRING;
}
return null;
}
Expand Down
15 changes: 7 additions & 8 deletions rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.openrewrite.Cursor;
import org.openrewrite.PrintOutputCapture;
import org.openrewrite.Tree;
import org.openrewrite.groovy.internal.Delimiter;
import org.openrewrite.groovy.marker.*;
import org.openrewrite.groovy.tree.G;
import org.openrewrite.groovy.tree.GContainer;
Expand All @@ -34,6 +35,8 @@
import java.util.Optional;
import java.util.function.UnaryOperator;

import static org.openrewrite.groovy.internal.Delimiter.DOUBLE_QUOTE_STRING;

public class GroovyPrinter<P> extends GroovyVisitor<PrintOutputCapture<P>> {
private final GroovyJavaPrinter delegate = new GroovyJavaPrinter();

Expand Down Expand Up @@ -73,18 +76,14 @@ public J visitCompilationUnit(G.CompilationUnit cu, PrintOutputCapture<P> p) {
@Override
public J visitGString(G.GString gString, PrintOutputCapture<P> p) {
beforeSyntax(gString, GSpace.Location.GSTRING, p);
String delimiter = gString.getDelimiter();
Delimiter delimiter = Delimiter.of(gString.getDelimiter());
if (delimiter == null) {
// For backwards compatibility with ASTs before we collected this field
delimiter = "\"";
delimiter = DOUBLE_QUOTE_STRING;
}
p.append(delimiter);
p.append(delimiter.open);
visit(gString.getStrings(), p);
if ("$/".equals(delimiter)) {
p.append("/$");
} else {
p.append(delimiter);
}
p.append(delimiter.close);
afterSyntax(gString, p);
return gString;
}
Expand Down
44 changes: 44 additions & 0 deletions rewrite-groovy/src/main/java/org/openrewrite/groovy/internal/Delimiter.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.groovy.internal;

import lombok.RequiredArgsConstructor;
import org.jspecify.annotations.Nullable;

@RequiredArgsConstructor
public enum Delimiter {
SINGLE_QUOTE_STRING("'", "'"),
DOUBLE_QUOTE_STRING("\"", "\""),
TRIPLE_SINGLE_QUOTE_STRING("'''", "'''"),
TRIPLE_DOUBLE_QUOTE_STRING("\"\"\"", "\"\"\""),
SLASHY_STRING("/", "/"),
DOLLAR_SLASHY_STRING("$/", "/$"),
PATTERN_OPERATOR("~/", "/"),
SINGLE_LINE_COMMENT("//", "\n"),
MULTILINE_COMMENT("/*", "*/");

public final String open;
public final String close;

public static @Nullable Delimiter of(String open) {
for (Delimiter delim : Delimiter.values()) {
if (delim.open.equals(open)) {
return delim;
}
}
return null;
}
}
16 changes: 16 additions & 0 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/LiteralTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,17 @@ void gStringWithEscapedDelimiter() {
);
}

@Test
void gStringWithStringLiteralsWithParentheses() {
rewriteRun(
groovy(
"""
"Hello ${from(":-)").via(''':-|(''').via(":-)").to(':-(')}!"
"""
)
);
}

@Test
void mapLiteral() {
rewriteRun(
Expand Down Expand Up @@ -313,6 +324,11 @@ void stringLiteralInParentheses() {
"""
def a = (("-"))
"""
),
groovy(
"""
from(":-)").via(''':-|(''').via(":-)").to(':-(')
"""
)
);
}
Expand Down
15 changes: 15 additions & 0 deletions rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/MethodInvocationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,4 +506,19 @@ void insideFourParenthesesAndEnters() {
)
);
}

@Test
void insideFourParenthesesAndEntersWithCommentWithParentheses() {
rewriteRun(
groovy(
"""
((/* comment :-) ((
*/((
// :-((
/*)*/something(a)
))))
"""
)
);
}
}

0 comments on commit 63c9526

Please sign in to comment.