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

feat: new enforceSuppressionCode rule #489

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Brianzchen
Copy link
Contributor


const create = (context) => {
const isMissingSuppressionCode = function (value) {
const suppressionTypes = ['$FlowFixMe', '$FlowExpectedError'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about adding there all the newly supported suppressions? See: https://flow.org/en/docs/config/options/#toc-suppress-comment-regex

Note: In version v0.127.0, the option to specify the suppression comment syntax was removed. $FlowFixMe, $FlowIssue, $FlowExpectedError, and $FlowIgnore became the only standard suppressions.

@@ -0,0 +1,31 @@
export default {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have here some examples of block comments as well, see: https://flow.org/en/docs/errors/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool found issue with block comments so fixed it

@Brianzchen
Copy link
Contributor Author

@mrtnzlml any more thoughts on the change?

code: '// $FlowFixMe I am doing something evil here\nconst text = \'HELLO\';',
errors: [
{
message: '$FlowFixMe is missing a suppression code',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flow is warning with the following message when the suppression code is missing:

Suppression is missing a code. Please update this suppression to use an error code: $FlowFixMe[incompatible-cast]

I understand that this would be complicated to do in Eslint, but maybe it would be nice to explain how to fix this error? For example:

$FlowFixMe is missing a suppression error code. Please update this suppression to use an error code: $FlowFixMe[…]

What do you think? Just a nit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah no worries, let me look into it later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, agree it's heaps better

@Brianzchen
Copy link
Contributor Author

@gajus bump

@Brianzchen
Copy link
Contributor Author

@gajus Can this be merged?

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

Successfully merging this pull request may close these issues.

Add rule to enforce error codes on flow suppressions
2 participants