Skip to content

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.
@carterkozak carterkozak force-pushed the ckozak/delete_refaster branch from 78ec9b3 to 0470563 Compare March 15, 2023 22:33
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.

6 participants