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

Added recipe for changing Groovy params #4581

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5cdedcb
WIP added ChangeGroovyMethodInvocationParameter Recipe
Oct 11, 2024
3ec001b
added license header
sgartner03 Oct 11, 2024
2dcf846
Simplified tests
sgartner03 Oct 11, 2024
e77a05a
Added support for double quotes and filtering for given method
ThomasKianek-Gepardec Oct 14, 2024
3ae4b21
Merge branch 'main' into feature/groovy-recipe-param
timtebeek Oct 14, 2024
a52557e
Merge branch 'main' into feature/groovy-recipe-param
Xaeras Oct 15, 2024
2a3d54e
incorporated actions import change
sgartner03 Oct 16, 2024
4c8171d
Incoorporated UUID actions change
sgartner03 Oct 16, 2024
b550ddf
incorporated actions ctx change
sgartner03 Oct 16, 2024
a420fb9
incorporated actions method change
sgartner03 Oct 16, 2024
a0f8611
incorporated actions test change
sgartner03 Oct 16, 2024
71fa8b9
c
sgartner03 Oct 16, 2024
fff88fb
incorporated actions test change
sgartner03 Oct 16, 2024
2adc6ed
Fixed code after broken GH Actions suggestions
sgartner03 Oct 16, 2024
a1fe037
Merge branch 'main' into feature/groovy-recipe-param
timtebeek Oct 16, 2024
c618093
Changed Recipe name and added tests
sgartner03 Oct 18, 2024
c6b9735
Merge branch 'main' into feature/groovy-recipe-param
timtebeek Oct 21, 2024
5d558f4
Update test formatting and naming to match existing tests
timtebeek Oct 21, 2024
1b6c398
Removed extractQuoting
sgartner03 Oct 21, 2024
edd9a59
Refactoring
sgartner03 Oct 21, 2024
d7b1e1c
Added null check for warning
sgartner03 Oct 21, 2024
8b5d7ec
Merge branch 'main' into feature/groovy-recipe-param
timtebeek Dec 6, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright 2024 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;

import org.openrewrite.*;
import org.openrewrite.groovy.tree.G;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.Space;
import org.openrewrite.marker.Markers;

public class ChangeStringValueOfNamedParameterInMethodInvocation extends Recipe {

@Option
public String methodName;

@Option
public String key;

@Option
public String value;

public ChangeStringValueOfNamedParameterInMethodInvocation(final String methodName, final String key, final String value) {
this.methodName = methodName;
this.key = key;
this.value = value;
}

@Override
public @NlsRewrite.DisplayName String getDisplayName() {
return "Change the value of a groovy named string parameter of a method invocation";
}

@Override
public @NlsRewrite.Description String getDescription() {
return "Changes the value of a given parameter in a given groovy method invocation, supporting Strings and GStrings.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new GroovyIsoVisitor<ExecutionContext>() {
@Override
public J visitMapEntry(G.MapEntry mapEntry, ExecutionContext ctx) {
mapEntry = (G.MapEntry) super.visitMapEntry(mapEntry, ctx);

if (!isInTargetMethod()) {
return mapEntry;
}

char quote = extractQuoting(mapEntry);
if (quote != '\'' && extractQuoting(mapEntry) != '"') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See below as well for a potential alternative looking at the type of the mapEntry instead, but calling the same method twice with the same argument is a bit suspect.

Suggested change
char quote = extractQuoting(mapEntry);
if (quote != '\'' && extractQuoting(mapEntry) != '"') {
char quote = extractQuoting(mapEntry);
if (quote != '\'' && quote != '"') {

return mapEntry;
}

if (!mapEntry.getKey().toString().equals(key)) {
return mapEntry;
}

if (mapEntry.getValue().toString().equals(value)) {
return mapEntry;
}

return replaceValue(mapEntry, quote);
}

private boolean isInTargetMethod() {
return getCursor().firstEnclosingOrThrow(J.MethodInvocation.class).getSimpleName().equals(methodName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the intended use case here? Because we'd much rather match using a MethodMatcher as opposed to a String comparision, as this would match any method anywhere of the same name, which is not as type safe as we prefer to be. For Gradle build script type information isn't always available to match reliably, but even then there's established patterns to be a little more sure we're not just matching any method by simple name, hence why I ask here.

}

private G. MapEntry replaceValue(final G.MapEntry mapEntry, char quote) {
return mapEntry.withValue(new J.Literal(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, value, String.format("%c%s%c", quote, value, quote), null, JavaType.Primitive.String));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we not want to retain any original formatting and comments by using withValue and withValueSource instead?

}

private char extractQuoting(final G.MapEntry mapEntry) {
return mapEntry.getValue().printTrimmed(getCursor()).charAt(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only time I've seen printTrimmed used in all of rewrite-groovy; is there no safer alternative? It seems you're looking to skip over anything that's not a J.Literal or GString; perhaps that can be checked here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right. I've removed extractQuoting

}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* Copyright 2024 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;

import org.junit.jupiter.api.Test;
import org.openrewrite.test.RewriteTest;

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

class ChangeNamedStringParameterValueOfMethodInvocationTest implements RewriteTest {

@Test
void whenParamSet_thenChangeToNewValue() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method", "param", "newValue")),
//language=groovy
groovy(
"""
method(
param: 'oldValue'
)
""", """
method(
param: 'newValue'
)
"""
)
);
}

@Test
void noChangeWhenNewValueEqualsOldValue() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method", "param", "value")),
//language=groovy
groovy(
"""
method(
param: 'value'
)
"""
)
);
}

@Test
void whenSingleParamSetWithGString_thenChangeToNewValue() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method", "param", "newValue")),
//language=groovy
groovy(
"""
method(
param: "oldValue"
)
""", """
method(
param: "newValue"
)
"""
)
);
}

@Test
void whenMethodWithMultipleParameters_thenChangeOnlySelectedToNewValue() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method", "param2", "newValue")),
//language=groovy
groovy(
"""
method(
param1: "oldValue",
param2: "oldValue",
param3: "oldValue"
)
""", """
method(
param1: "oldValue",
param2: "newValue",
param3: "oldValue"
)
"""
)
);
}

@Test
void whenTwoMethods_thenOnlyChangeOne() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method2", "param", "newValue")),
groovy(
"""
method1(
param: "oldValue"
)
method2(
param: "oldValue"
)
""", """
method1(
param: "oldValue"
)
method2(
param: "newValue"
)
"""
)
);
}

@Test
void noChangeWhenParameterWithOtherDatatype() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method", "param", "newValue")),
groovy(
"""
method(
param: 1
)
"""
)
);
}

@Test
void noChangeWhenMethodWithoutParameters() {
rewriteRun(spec -> spec.recipe(new ChangeStringValueOfNamedParameterInMethodInvocation("method2", "param", "newValue")),
groovy(
"""
method()
"""
)
);
}
}