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
1 change: 1 addition & 0 deletions .README/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ When `true`, only checks files with a [`@flow` annotation](http://flowtype.org/d
{"gitdown": "include", "file": "./rules/define-flow-type.md"}
{"gitdown": "include", "file": "./rules/delimiter-dangle.md"}
{"gitdown": "include", "file": "./rules/enforce-line-break.md"}
{"gitdown": "include", "file": "./rules/enforce-suppression-code.md"}
{"gitdown": "include", "file": "./rules/generic-spacing.md"}
{"gitdown": "include", "file": "./rules/newline-after-flow-annotation.md"}
{"gitdown": "include", "file": "./rules/no-dupe-keys.md"}
Expand Down
5 changes: 5 additions & 0 deletions .README/rules/enforce-suppression-code.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### `enforce-suppression-code`

This rule enforces a suppression code on flow suppression comments such as `$FlowFixMe` and `$FlowExpectedError`.

<!-- assertions enforceSuppressionCode -->
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import booleanStyle from './rules/booleanStyle';
import defineFlowType from './rules/defineFlowType';
import delimiterDangle from './rules/delimiterDangle';
import enforceLineBreak from './rules/enforceLineBreak';
import enforceSuppressionCode from './rules/enforceSuppressionCode';
import genericSpacing from './rules/genericSpacing';
import newlineAfterFlowAnnotation from './rules/newlineAfterFlowAnnotation';
import noDupeKeys from './rules/noDupeKeys';
Expand Down Expand Up @@ -55,6 +56,7 @@ const rules = {
'define-flow-type': defineFlowType,
'delimiter-dangle': delimiterDangle,
'enforce-line-break': enforceLineBreak,
'enforce-suppression-code': enforceSuppressionCode,
'generic-spacing': genericSpacing,
'newline-after-flow-annotation': newlineAfterFlowAnnotation,
'no-dupe-keys': noDupeKeys,
Expand Down
54 changes: 54 additions & 0 deletions src/rules/enforceSuppressionCode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const schema = [
{
type: 'string',
},
];

const message = (suppression = '') => {
return `${suppression} is missing a suppression code`;
};

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.


let failedType;
suppressionTypes.forEach((cur) => {
if (value.startsWith(cur) &&
!value.startsWith(`${cur}[`) &&
!value.endsWith(']')) {
failedType = cur;
}
});

return failedType;
};

const handleComment = function (comment) {
const value = comment.value.trim().split(' ').filter((arg) => {
return arg.length > 0;
})[0];
const failedType = isMissingSuppressionCode(value);

if (failedType) {
context.report(comment, message(failedType));
}
};

return {
Program () {
context
.getSourceCode()
.getAllComments()
.filter((comment) => {
return comment.type === 'Block' || comment.type === 'Line';
})
.forEach(handleComment);
},
};
};

export default {
create,
schema,
};
31 changes: 31 additions & 0 deletions tests/rules/assertions/enforceSuppressionCode.js
Original file line number Diff line number Diff line change
@@ -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

invalid: [
{
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

},
],
},
{
code: '// $FlowExpectedError I am doing something evil here\nconst text = \'HELLO\';',
errors: [
{
message: '$FlowExpectedError is missing a suppression code',
},
],
},
],
valid: [
{
code: 'const text = \'HELLO\';',
},
{
code: '// $FlowFixMe[incompatible-call] TODO 48\nconst text = \'HELLO\';',
},
{
code: '// $FlowExpectedError[incompatible-call] TODO 48\nconst text = \'HELLO\';',
},
],
};
1 change: 1 addition & 0 deletions tests/rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const reportingRules = [
'define-flow-type',
'delimiter-dangle',
'enforce-line-break',
'enforce-suppression-code',
'generic-spacing',
'newline-after-flow-annotation',
'no-dupe-keys',
Expand Down