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

Add respect to @noflow annotation #451

Merged
merged 3 commits into from Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/index.js
Expand Up @@ -88,7 +88,7 @@ export default {
recommended,
},
rules: _.mapValues(rules, (rule, key) => {
if (key === 'no-types-missing-file-annotation') {
if (['no-types-missing-file-annotation', 'require-valid-file-annotation'].includes(key)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to change the way how require-valid-file-annotation works with onlyFilesWithFlowAnnotation setting, because of breaking change in checkFlowFileAnnotation.js

return rule;
}

Expand Down
2 changes: 1 addition & 1 deletion src/rules/requireValidFileAnnotation.js
Expand Up @@ -113,7 +113,7 @@ const create = (context) => {
} else {
context.report(potentialFlowFileAnnotation, 'Malformed Flow file annotation.');
}
} else if (always) {
} else if (always && !_.get(context, 'settings.flowtype.onlyFilesWithFlowAnnotation')) {
context.report({
fix: (fixer) => {
let annotation;
Expand Down
3 changes: 2 additions & 1 deletion src/utilities/checkFlowFileAnnotation.js
@@ -1,8 +1,9 @@
import _ from 'lodash';
import isFlowFile from './isFlowFile';
import isNoFlowFile from './isNoFlowFile';

export default (cb, context) => {
const checkThisFile = !_.get(context, 'settings.flowtype.onlyFilesWithFlowAnnotation') || isFlowFile(context);
const checkThisFile = (!_.get(context, 'settings.flowtype.onlyFilesWithFlowAnnotation') && !isNoFlowFile(context)) || isFlowFile(context); // eslint-disable-line no-extra-parens, max-len

if (!checkThisFile) {
return () => {};
Expand Down
1 change: 1 addition & 0 deletions src/utilities/index.js
Expand Up @@ -9,6 +9,7 @@ export {default as getParameterName} from './getParameterName';
export {default as getTokenAfterParens} from './getTokenAfterParens';
export {default as getTokenBeforeParens} from './getTokenBeforeParens';
export {default as isFlowFile} from './isFlowFile';
export {default as isNoFlowFile} from './isNoFlowFile';
export {default as isFlowFileAnnotation} from './isFlowFileAnnotation';
export {default as iterateFunctionNodes} from './iterateFunctionNodes';
export {default as quoteName} from './quoteName';
Expand Down
21 changes: 21 additions & 0 deletions src/utilities/isNoFlowFile.js
@@ -0,0 +1,21 @@
import isNoFlowFileAnnotation from './isNoFlowFileAnnotation';

/**
* Checks whether a file has an @flow or @noflow annotation.
*
* @param context
* @param [strict] - By default, the function returns true if the file starts with @flow but not if it
* starts by @noflow. When the strict flag is set to false, the function returns true if the flag has @noflow also.
*/

export default (context, strict = true) => {
const comments = context.getAllComments();

if (!comments.length) {
return false;
}

return comments.some((comment) => {
return isNoFlowFileAnnotation(comment.value, strict);
});
};
17 changes: 17 additions & 0 deletions src/utilities/isNoFlowFileAnnotation.js
@@ -0,0 +1,17 @@
import _ from 'lodash';

const FLOW_MATCHER = /^@noflow$/;

export default (comment, strict) => {
// The flow parser splits comments with the following regex to look for the @flow flag.
// See https://github.com/facebook/flow/blob/a96249b93541f2f7bfebd8d62085bf7a75de02f2/src/parsing/docblock.ml#L39
return _.some(comment.split(/[\t\n\r */\\]+/), (commentPart) => {
const match = commentPart.match(FLOW_MATCHER);

if (match === null) {
return false;
}

return !strict || match[0] === '@noflow';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is a copy of "isFlowFileAnnotation.js" with only changes in regex and on this line

});
};
14 changes: 14 additions & 0 deletions tests/rules/assertions/requireReturnType.js
Expand Up @@ -38,6 +38,14 @@ export default {
},
],
},
{
code: '/* @flow */\n(foo) => { return 1; }',
errors: [
{
message: 'Missing return type annotation.',
},
],
},
{
code: '(foo): undefined => { return; }',
errors: [
Expand Down Expand Up @@ -692,6 +700,12 @@ export default {
},
},
},
{
code: '/* @noflow */\n(foo) => { return 1; }',
options: [
'always',
],
},
{
code: '(foo) => { return undefined; }',
options: [
Expand Down