Introduce `RefasterMethodParameterOrder` check #775

Stephan202 · 2023-09-02T16:00:03Z

❗ This PR is on top of #962 ❗

Another step towards canonical Refaster rule definitions.

Suggested commit message:

Introduce `RefasterMethodParameterOrder` check (#775)

While there, simplify some `EqualityRules` Refaster rules, as a suggested
parameter order change highlighted that they could be collapsed.

github-actions · 2023-09-02T16:02:34Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

github-actions · 2023-09-02T16:27:28Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

github-actions · 2023-09-02T16:51:45Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

github-actions · 2023-09-02T16:52:05Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

sonarqubecloud · 2023-09-02T16:52:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

Stephan202 · 2023-10-08T13:23:42Z

Rebased and added a commit to resolve a new violation.

github-actions · 2023-10-08T13:25:44Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Stephan202 · 2023-10-23T15:51:18Z

Rebased and resolved conflict.

github-actions · 2023-10-23T15:53:43Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

sonarqubecloud · 2023-10-23T15:53:58Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

mohamedsamehsalah

Clean 🧹

github-actions · 2023-12-18T14:25:12Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie

Took some time to wrap my head around the algorithm, but it looks really nice 🚀 !

One question 👀.

rickie · 2023-12-31T14:40:54Z

...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrderTest.java

+            "    }",
+            "  }",
+            "}")
+        .doTest();


It feels a bit odd to have an identification test with only things that should not be matched. I get that the replacement check is way more interesting here but I'd suggest we still add at least one 🤔?

Part of the reason that I put this comment here is that we could also choose to use expectNoDiagnostics here instead (?) of doTest to make the intention of the test clearer.

Fair point. We should add a positive case, as that verifies on which tree node the violation is reported.

rickie · 2024-01-09T08:16:49Z

Hmm, interesting (currently in a different issue):

Error:  Failures: 
Error:    RefasterMethodParameterOrderTest.replacement:119 compilation failed unexpectedly: [/17892554101468984197/ARecipes.java:1: error: ';' expected
package unnamed package;
               ^]

Stephan202 · 2024-01-10T07:35:01Z

I can reproduce that locally, also when upgrading to the currently-latest template-writing (version 1.4.1).

Also, when I run the tests in IDEA (in an attempt to debug the problem 🙃), I get the same issue as mentioned in #925 (comment); it seems that issue breaks ~all tests in IDEA, not just StringRulesRecipesTest 😬.

This'll require a closer look (can't promise when; busy agenda). CC @timtebeek as an FYI.

Stephan202 · 2024-01-10T18:48:41Z

I traced the IDEA issue to IDEA-342187, for which I filed #958. This should unblock debugging the other issue.

Stephan202 · 2024-01-10T19:05:55Z

W.r.t. the other issue: I suspect it's due to this code, which doesn't properly handle the unnamed package.

I rebased and pushed a commit to fix the build, but would like to reference an issue or PR before merging. I may have time for a PR later today/this week; need to leave the train first ;)

github-actions · 2024-01-10T19:08:12Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Stephan202 · 2024-01-10T21:31:09Z

Alright, I filed openrewrite/rewrite-templating#64 and referenced it in the code.

github-actions · 2024-01-10T21:32:52Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Stephan202

Added one more commit.

Stephan202 · 2024-01-10T21:39:24Z

...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterMethodParameterOrderTest.java

+            "    }",
+            "  }",
+            "}")
+        .doTest();


Fair point. We should add a positive case, as that verifies on which tree node the violation is reported.

github-actions · 2024-01-10T21:45:27Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Stephan202 · 2024-01-11T06:36:53Z

I rebased the PR on top of #962 and reverted the package workaround. (So we should merge the upgrade PR first.)

github-actions · 2024-01-11T06:41:22Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Stephan202 · 2024-01-13T15:06:40Z

Rebased; the PR now targets master. Can be merged once built.

sonarqubecloud · 2024-01-13T15:08:01Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-01-13T15:08:13Z

Surviving mutants in this change: 2
Killed mutants in this change: 22

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder`	1	20
🧟`tech.picnic.errorprone.bugpatterns.RefasterMethodParameterOrder$1`	1	2

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Stephan202 added this to the 0.14.0 milestone Sep 2, 2023

Stephan202 requested a review from rickie September 2, 2023 16:00

Stephan202 force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from d3a7620 to fdd5e38 Compare September 2, 2023 16:49

Stephan202 changed the title ~~Introduce RefasterParameterOrder check~~ Introduce RefasterMethodParameterOrder check Sep 2, 2023

rickie modified the milestones: 0.14.0, 0.15.0 Oct 4, 2023

Stephan202 force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from fdd5e38 to ce4405d Compare October 8, 2023 13:23

Stephan202 force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from ce4405d to 2d7224b Compare October 23, 2023 15:51

mohamedsamehsalah approved these changes Dec 11, 2023

View reviewed changes

rickie force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from 2d7224b to eb51b24 Compare December 18, 2023 14:22

Stephan202 added the new feature label Dec 24, 2023

rickie approved these changes Jan 9, 2024

View reviewed changes

rickie force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from eb51b24 to ea6cc7e Compare January 9, 2024 08:14

Stephan202 mentioned this pull request Jan 10, 2024

Work around IntelliJ IDEA bug IDEA-342187 #958

Merged

Stephan202 force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from ea6cc7e to 64148fe Compare January 10, 2024 19:05

Stephan202 mentioned this pull request Jan 10, 2024

Support sources located in an unnamed package openrewrite/rewrite-templating#64

Merged

Stephan202 commented Jan 10, 2024

View reviewed changes

Stephan202 force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from d3f896b to 515a502 Compare January 11, 2024 06:36

Stephan202 changed the base branch from master to renovate/org.openrewrite-rewrite-templating-1.x January 11, 2024 06:37

mohamedsamehsalah added the do not merge label Jan 12, 2024

Picnic-Bot force-pushed the renovate/org.openrewrite-rewrite-templating-1.x branch from 8502934 to 8140f66 Compare January 13, 2024 02:01

Stephan202 force-pushed the renovate/org.openrewrite-rewrite-templating-1.x branch from 8140f66 to 1096749 Compare January 13, 2024 14:32

Base automatically changed from renovate/org.openrewrite-rewrite-templating-1.x to master January 13, 2024 14:53

Stephan202 added 8 commits January 13, 2024 16:05

Introduce RefasterParameterOrder check

d57b9b5

Fix build, simplify some rules

2050fed

Rename check

6c857d6

Post-rebase fix

474f36e

Work around rewrite-templating limitation

4e9bff6

Link PR

95b2c6e

Address PR feedback

d6690cf

Drop workaround

4984efa

Stephan202 force-pushed the sschroevers/introduce-RefasterParameterOrder-check branch from 515a502 to 4984efa Compare January 13, 2024 15:05

Stephan202 merged commit 14506ed into master Jan 13, 2024
16 checks passed

Stephan202 deleted the sschroevers/introduce-RefasterParameterOrder-check branch January 13, 2024 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `RefasterMethodParameterOrder` check #775

Introduce `RefasterMethodParameterOrder` check #775

Stephan202 commented Sep 2, 2023 •

edited

Loading

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 2, 2023

sonarqubecloud bot commented Sep 2, 2023

Stephan202 commented Oct 8, 2023

github-actions bot commented Oct 8, 2023

Stephan202 commented Oct 23, 2023

github-actions bot commented Oct 23, 2023

sonarqubecloud bot commented Oct 23, 2023

mohamedsamehsalah left a comment

github-actions bot commented Dec 18, 2023

rickie left a comment

rickie Dec 31, 2023

rickie Jan 9, 2024

Stephan202 Jan 10, 2024

rickie commented Jan 9, 2024 •

edited

Loading

Stephan202 commented Jan 10, 2024

Stephan202 commented Jan 10, 2024

Stephan202 commented Jan 10, 2024

github-actions bot commented Jan 10, 2024

Stephan202 commented Jan 10, 2024 •

edited

Loading

github-actions bot commented Jan 10, 2024

Stephan202 left a comment

Stephan202 Jan 10, 2024

github-actions bot commented Jan 10, 2024

Stephan202 commented Jan 11, 2024

github-actions bot commented Jan 11, 2024

Stephan202 commented Jan 13, 2024

sonarqubecloud bot commented Jan 13, 2024

github-actions bot commented Jan 13, 2024

Introduce RefasterMethodParameterOrder check #775

Introduce RefasterMethodParameterOrder check #775

Conversation

Stephan202 commented Sep 2, 2023 • edited Loading

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 2, 2023

github-actions bot commented Sep 2, 2023

sonarqubecloud bot commented Sep 2, 2023

Stephan202 commented Oct 8, 2023

github-actions bot commented Oct 8, 2023

Stephan202 commented Oct 23, 2023

github-actions bot commented Oct 23, 2023

sonarqubecloud bot commented Oct 23, 2023

mohamedsamehsalah left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 18, 2023

rickie left a comment

Choose a reason for hiding this comment

rickie Dec 31, 2023

Choose a reason for hiding this comment

rickie Jan 9, 2024

Choose a reason for hiding this comment

Stephan202 Jan 10, 2024

Choose a reason for hiding this comment

rickie commented Jan 9, 2024 • edited Loading

Stephan202 commented Jan 10, 2024

Stephan202 commented Jan 10, 2024

Stephan202 commented Jan 10, 2024

github-actions bot commented Jan 10, 2024

Stephan202 commented Jan 10, 2024 • edited Loading

github-actions bot commented Jan 10, 2024

Stephan202 left a comment

Choose a reason for hiding this comment

Stephan202 Jan 10, 2024

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2024

Stephan202 commented Jan 11, 2024

github-actions bot commented Jan 11, 2024

Stephan202 commented Jan 13, 2024

sonarqubecloud bot commented Jan 13, 2024

Quality Gate passed

github-actions bot commented Jan 13, 2024

Introduce `RefasterMethodParameterOrder` check #775

Introduce `RefasterMethodParameterOrder` check #775

Stephan202 commented Sep 2, 2023 •

edited

Loading

rickie commented Jan 9, 2024 •

edited

Loading

Stephan202 commented Jan 10, 2024 •

edited

Loading