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

Rename Rewrite to Checker/Rule? #293

Closed
olafurpg opened this issue Aug 17, 2017 · 6 comments
Closed

Rename Rewrite to Checker/Rule? #293

olafurpg opened this issue Aug 17, 2017 · 6 comments

Comments

@olafurpg
Copy link
Contributor

Following discussion in #291 (comment).

Now that rewrites can emit linter warnings, the name "Rewrite" may be confusing for linters that don't rewrite. Would it make sense to rename scalafix.Rewrite to scalafix.rule?

@olafurpg olafurpg added the core label Aug 17, 2017
@olafurpg olafurpg mentioned this issue Aug 17, 2017
7 tasks
@olafurpg olafurpg changed the title Rename Rewrite to Rule? Rename Rewrite to Checker? Sep 4, 2017
@olafurpg
Copy link
Contributor Author

olafurpg commented Sep 4, 2017

After having looked around names clang-tidy, fb infer and pylint use, I think a more appropriate name for a scalafix rewrite would be a "Checker".

@gabro
Copy link
Collaborator

gabro commented Sep 4, 2017

Just to throw another one in the mix, eslint uses Rule and rules can be "fixable". But I'm totally ok with Checker

@olafurpg olafurpg changed the title Rename Rewrite to Checker? Rename Rewrite to Checker/Rule? Sep 4, 2017
@olafurpg
Copy link
Contributor Author

olafurpg commented Sep 4, 2017

Interesting. I think Rule is equally fine then.

Question is, what should Rewrite.rewrite become then?

Rewrite.rewrite
Checker.check
Rule.rule ?
Rule.check ?
Rule.apply ?

@xeno-by
Copy link
Contributor

xeno-by commented Sep 4, 2017

I vote for Checker.check.

@gabro
Copy link
Collaborator

gabro commented Sep 5, 2017

Ok, let the bike shedding begin! 🚲

Checker is ok, but I would expect a checker to only check, not to actually do anything.

Rule is instead more "prescriptive": here's the rule, and you can either:

  • check it
  • apply a fix

Rule.check
Rule.fix

(this follows eslint's convention)

@olafurpg
Copy link
Contributor Author

olafurpg commented Sep 6, 2017

Rule it is! We discussed this at the Scala Center meeting yesterday and we were unanimously in favor of Rule.fix/Rule.check over Checker. PR coming soon!

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

No branches or pull requests

3 participants