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 @@ -165,6 +165,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/interface-id-match.md"}
{"gitdown": "include", "file": "./rules/newline-after-flow-annotation.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 interfaceIdMatch from './rules/interfaceIdMatch';
import newlineAfterFlowAnnotation from './rules/newlineAfterFlowAnnotation';
Expand Down Expand Up @@ -56,6 +57,7 @@ const rules = {
'define-flow-type': defineFlowType,
'delimiter-dangle': delimiterDangle,
'enforce-line-break': enforceLineBreak,
'enforce-suppression-code': enforceSuppressionCode,
'generic-spacing': genericSpacing,
'interface-id-match': interfaceIdMatch,
'newline-after-flow-annotation': newlineAfterFlowAnnotation,
Expand Down
57 changes: 57 additions & 0 deletions src/rules/enforceSuppressionCode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
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', '$FlowIssue', '$FlowIgnore'];

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.type === 'Block' ?
comment.value.replace(/\*/g, '') :
comment.value;
const suppression = value.trim().split(' ').filter((arg) => {
return arg.length > 0;
})[0];
const failedType = isMissingSuppressionCode(suppression);

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,
};
90 changes: 90 additions & 0 deletions tests/rules/assertions/enforceSuppressionCode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
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',
},
],
},
{
code: '// $FlowIssue I am doing something evil here\nconst text = \'HELLO\';',
errors: [
{
message: '$FlowIssue is missing a suppression code',
},
],
},
{
code: '// $FlowIgnore I am doing something evil here\nconst text = \'HELLO\';',
errors: [
{
message: '$FlowIgnore is missing a suppression code',
},
],
},
{
code: '/* $FlowIgnore I am doing something evil here */',
errors: [
{
message: '$FlowIgnore is missing a suppression code',
},
],
},
{
code: '{ /* $FlowIgnore I am doing something evil here */ }',
errors: [
{
message: '$FlowIgnore is missing a suppression code',
},
],
},
{
code: `/**
* $FlowIgnore I am doing something evil here
*/`,
errors: [
{
message: '$FlowIgnore 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\';',
},
{
code: '// $FlowIssue[incompatible-call] TODO 48\nconst text = \'HELLO\';',
},
{
code: '// $FlowIgnore[incompatible-call] TODO 48\nconst text = \'HELLO\';',
},
{
code: '/* $FlowIgnore[incompatible-call] TODO 48 */',
},
{
code: `/**
* $FlowIgnore[incompatible-call] TODO 48
*/`,
},
{
code: '/* $FlowIgnore[incompatible-call] TODO 48 */',
},
],
};
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',
'interface-id-match',
'newline-after-flow-annotation',
Expand Down