Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Support multiple forms of inline suppression #351

Closed
wants to merge 3 commits into from
Closed

Support multiple forms of inline suppression #351

wants to merge 3 commits into from

Conversation

lexidor
Copy link
Contributor

@lexidor lexidor commented Feb 26, 2021

Fixes #308
This PR adds the following default suppressors.
@lint-ignore x, @lint-fixme x, @lint-ignore-all x, HHAST_IGNORE[x].
You may now also specify suppression aliases in hhast-lint.json.

This commit adds the following default suppressors.
@lint-ignore x, @lint-fixme x, @lint-ignore-all x, HHAST_IGNORE[x].
You may now also specify suppression aliases in hhast-lint.json.
@jjergus
Copy link
Contributor

jjergus commented Mar 2, 2021

I have mixed feelings about this...what's the value of having so many different keywords?

The value of having a single keyword is that you can do grep HHAST_IGNORE_ERROR *.hack to find all ignored lints, there's less stuff for people to remember when writing/reading code, etc.

@lexidor
Copy link
Contributor Author

lexidor commented Mar 11, 2021

I think adding HHAST_IGNORE is a good change. HHAST_IGNORE_ERROR suggests the linter away behavior is erroneous. The @lint-* are meant for interoperability with HackAST. This allows Facebook to lint against the hsl directory. I guessed HackAST's syntaxes for global and fixme.

I don't know for sure if the alias feature I implemented matches what Fred had in mind. Should aliases only be able to alias the linter name, whilst still being required to have a HHAST_* or @lint-* prefix? I am starting to second guess supporting // This is a polling loop. It is unclear that it is suppressing a linter and looks like a useless inline comment. Wide use of this might discourage removal of useless inline comments, because they might be suppressing a linter.

@fredemmott
Copy link
Contributor

  • HHAST_IGNORE seems like a good change, and has minor change to grepability
  • for the others, perhaps it would be best to make it configurable via hhast-lint.json

@lexidor
Copy link
Contributor Author

lexidor commented Nov 25, 2021

Major merge conflict with refactored code base.

@lexidor lexidor closed this Nov 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multiple forms of inline suppression
4 participants