Added recipe for changing Groovy params
What's changed?
Added recipe for changing Groovy parameter value in method invocation.
What's your motivation?
To be used in Jenkins files (e.g. to update the JDK version).
Have you considered any alternatives or workarounds?
Preconditions for selecting a given method in Groovy did not work as expected (had to use if-statement instead).
Checklist
- [x] I've added unit tests to cover both positive and negative cases
- [x] I've read and applied the recipe conventions and best practices
- [x] I've used the IntelliJ IDEA auto-formatter on affected files
Hi @timtebeek, I've incorporated the suggested changes from the GH Action. I think this contribution matches your code styling now.
@ThomasKianek-Gepardec @Xaeras
hi @ThomasKianek-Gepardec & @sgartner03 ! Do I understand this is a recipe that can change a named parameter value in a Groovy method invocation to a different String value? I feel like we might be more explicit about those limitations in how we name the recipe, as opposed to only have that in the description.
It might also make sense to add some additional tests with methods that have more than one named parameter, only changing the right one, or no named parameters, just to ensure we handle all cases correctly.
Hi @timtebeek, yes exactly that is what this recipe does. Thank you for your, feedback, I'll incorporate it now.
Hi, I've added 3 more tests. This tests:
- Multiple params
- Empty Methods
- Parameters of other type.
As of now, this recipe only supports Strings and GStrings
@timtebeek
Hey @timtebeek ! Thank you for your extensive feedback. This is really valuable for me - so far I've only developed InHouse-Recipes for one Repo, where I didn't focus on clean code a lot.
Yes, you're right, this is intended for Jenkinsfiles on our end. We thought, it could be used in other Groovy Use-Cases too.
Initially, we thought we could decide between GString / J.String by using instanceof on mapEntry#getValue. However, we did notice that through all our tests, only J.Strings exist. Meaning, ", as well as ' quotes are represented by J.String. But I've removed extractQuoting() and replaced it by filtering through J.Literal#getType and just replacing the old value inside of J.Literal#getValueSource with the new one. This also replaced the constructor-call with just ".with*"-Operations.
However, I'm not that sure about using a MethodMatcher. I have to point out, that my groovy knowledge is close to zero. But how can types even be used in terms of stuff like a Jenkinsfile or other groovy stuff? You've mentioned other established ways to limit the scope, what would these be? And can't limiting the scope be implemented with a precondition on the user's end?
(PS I didn't mean to stress - I just forgot to ping you on the initial message)
This PR is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.