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

Support readable aliases for HHClientLinter rules #410

Open
fredemmott opened this issue Nov 26, 2021 · 4 comments
Open

Support readable aliases for HHClientLinter rules #410

fredemmott opened this issue Nov 26, 2021 · 4 comments

Comments

@fredemmott
Copy link
Contributor

fredemmott commented Nov 26, 2021

#409

E.g. if we were removing DontAwaitInAloop linter and keeping 5583 (we should do the opposite, #408), we could make it so that suppressing DontAwaitInALoop suppresses 5583

  • avoids the BC break
  • more readable

This would be a divergence from HHAST.

This could also be more general, not just for HHClientLinter - then we could support the same suppression codes within FB with Aurora/HackAST, and externally with HHAST, when the linters do not have the same suppression code at present.

@Atry
Copy link
Contributor

Atry commented Nov 26, 2021

I think we don't want to do this manually because the aliases would be out of sync quickly when the upstream code changes. I think the best way to solve this issue is to export the metadata of OCaml lint rules to Hack automatically.

@fredemmott
Copy link
Contributor Author

I think we need a manual aliasing method regardless, as the names in ocaml do not necessarily match the historical hhast linter names - requiring renames removes the BC b enefits.

@Atry
Copy link
Contributor

Atry commented Nov 30, 2021

I agree we should allow some overridden aliases configured in the HHAST code base for historical hhast linter names, but shall we configure a default name in the OCaml code base then generate the corresponding Hack definition, so that we could have readable aliases for new OCaml linters by default?

@fredemmott
Copy link
Contributor Author

image

Though, if exported from ocaml, should probably default to the ocaml class name rather than needing explicit configuration

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

2 participants