Skip to content
This repository was 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.

@lexidor
Support multiple forms of inline suppression
aa3c87c 
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.
@lexidor
Demonstrate one aliassed suppression commen
8de5826
@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

This was referenced Nov 5, 2021
Closed
Merged
@lexidor
Copy link
Contributor Author

lexidor commented Nov 25, 2021

Major merge conflict with refactored code base.

@lexidor lexidor closed this Nov 25, 2021
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support multiple forms of inline suppression
4 participants
@lexidor @jjergus @fredemmott @facebook-github-bot