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

Remove redundant space for DeleteMethodArgument when argumentIndex=0 #4677

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

ckcd
Copy link
Contributor

@ckcd ckcd commented Nov 15, 2024

What's changed?

Below case will failed because it will change to "public class A {{ B.foo( 1, 2); }}" which contains a redundant space before the first argument.

    @Test
    void deleteFirstArgument() {
        rewriteRun(
          spec -> spec.recipe(new DeleteMethodArgument("B foo(int, int, int)", 0)),
          java(b),
          java(
            "public class A {{ B.foo(0, 1, 2); }}",
            "public class A {{ B.foo(1, 2); }}"
          )
        );
    }

correct answer: "public class A {{ B.foo(1, 2); }}", no space before the first 1 element.
actual answer: "public class A {{ B.foo( 1, 2); }}" has a redundant space before the first 1 element.

This PR aims to fix the issue.

What's your motivation?

Fix #4676

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindPluginsTest.java
    • lines 72-72

@ckcd ckcd force-pushed the ck2411_delete_argument branch from 57057fd to b138330 Compare November 15, 2024 07:38
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • rewrite-gradle/src/test/java/org/openrewrite/gradle/search/FindPluginsTest.java
    • lines 72-72

@timtebeek timtebeek added bug Something isn't working enhancement New feature or request labels Nov 15, 2024
Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks @ckcd ! Indeed looks better when there's no leading space after removing the first element. I've pushed up a very small change to assume the prefix used previously, with an additional test class that shows when that would help here. With that we're good to merge here.

@timtebeek timtebeek merged commit 5ef0ef1 into openrewrite:main Nov 15, 2024
2 checks passed
MBoegers pushed a commit to MBoegers/rewrite that referenced this pull request Dec 18, 2024
…penrewrite#4677)

* Remove redundant space for DeleteMethodArgument when argumentIndex=0

Signed-off-by: kuchang <[email protected]>

* Update FindPluginsTest.java

* Adopt prefix of removed element

---------

Signed-off-by: kuchang <[email protected]>
Co-authored-by: Tim te Beek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Redundant space for DeleteMethodArgument when argumentIndex=0
2 participants