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 support for refaster #2521

Merged
merged 2 commits into from
Mar 16, 2023
Merged

Remove support for refaster #2521

merged 2 commits into from
Mar 16, 2023

Conversation

carterkozak
Copy link
Contributor

Refaster rules, while simpler to write, are substantially more difficult to maintain compared to standard error-prone rules. We no longer write them ourselves, and have recommended against them for the last few years, as it's difficult to tell where they will match, and more difficult to tweak them to avoid edge cases.

We expect that removal of refaster will improve build times, especially for excavators. Failures which are caught and ignored compiling refaster rules are a common red herring which point devs in the wrong direction while debugging other issues.

==COMMIT_MSG==
Remove support for refaster
==COMMIT_MSG==

@changelog-app
Copy link

changelog-app bot commented Mar 15, 2023

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Remove support for refaster

Check the box to generate changelog(s)

  • Generate changelog entry

Refaster rules, while simpler to write, are substantially more
difficult to maintain compared to standard error-prone rules.
We no longer write them ourselves, and have recommended against
them for the last few years, as it's difficult to tell where they
will match, and more difficult to tweak them to avoid edge cases.

We expect that removal of refaster will improve build times,
especially for excavators. Failures which are caught and ignored
compiling refaster rules are a common red herring which point
devs in the wrong direction while debugging other issues.
Copy link
Contributor

@schlosna schlosna left a comment

Choose a reason for hiding this comment

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

Overall I'm supportive.

Do we have any plans to convert some/all of these refaster templates into error-prone checker(s)/fixers?

@carterkozak
Copy link
Contributor Author

Do we have any plans to convert some/all of these refaster templates into error-prone checker(s)/fixers?

It's not something I'm currently tracking, but it would be a great project to ramp somebody up on writing error-prone rules.

cc @mpritham ;-)

@bulldozer-bot bulldozer-bot bot merged commit 6a089d3 into develop Mar 16, 2023
@bulldozer-bot bulldozer-bot bot deleted the ckozak/delete_refaster branch March 16, 2023 13:37
This was referenced Mar 16, 2023
This was referenced Mar 16, 2023
@ash211
Copy link
Contributor

ash211 commented Sep 15, 2023

Clearly this has already happened, but I'm not sure that it's realistic to rewrite refaster rules into error prone rules at scale. They end up being an order of magnitude larger in errorprone. Take for example #2530 which is ~450 lines in error prone but is 36 lines in refaster.

We've got a bunch of rules in https://github.com/palantir/assertj-automation/tree/develop/assertj-refaster-rules/src/main/java/com/palantir/assertj/refaster that I value, but now I'm not sure they're worth the effort to port to errorprone.

@carterkozak
Copy link
Contributor Author

Take for example #2530 which is ~450 lines in error prone but is 36 lines in refaster.

I'd consider that a feature, not a bug. The biggest problem with refaster is precision: It's tremendously difficult to gain confidence that it will impact precisely what you want without overreaching or under-reaching. Furthermore refaster rules can't be applied in a way that prevents regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants