Skip to content

Commit

Permalink
[Fix] no-unused-prop-types: false positive with callback
Browse files Browse the repository at this point in the history
  • Loading branch information
golopot authored and ljharb committed Aug 6, 2019
1 parent 398b7d7 commit cfa6135
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 51 deletions.
60 changes: 26 additions & 34 deletions lib/util/Components.js
Expand Up @@ -584,43 +584,35 @@ function componentRule(rule, context) {
let scope = context.getScope();
while (scope) {
const node = scope.block;
const isFunction = /Function/.test(node.type); // Functions
const isArrowFunction = astUtil.isArrowFunction(node);
const enclosingScope = isArrowFunction ? utils.getArrowFunctionScope(scope) : scope;
const enclosingScopeParent = enclosingScope && enclosingScope.block.parent;
const isClass = enclosingScope && astUtil.isClass(enclosingScope.block);
const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods
const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.)
// Attribute Expressions inside JSX Elements (<button onClick={() => props.handleClick()}></button>)
const isJSXExpressionContainer = node.parent && node.parent.type === 'JSXExpressionContainer';
const pragmaComponentWrapper = this.getPragmaComponentWrapper(node);
if (isFunction && pragmaComponentWrapper) {
return pragmaComponentWrapper;
}
// Stop moving up if we reach a class or an argument (like a callback)
if (isClass || isArgument) {
return null;
}
// Return the node if it is a function that is not a class method and is not inside a JSX Element
if (isFunction && !isMethod && !isJSXExpressionContainer && utils.isReturningJSXOrNull(node)) {
return node;

if (node.type === 'FunctionDeclaration') {
if (utils.isReturningJSXOrNull(node)) {
return node;
}
}
scope = scope.upper;
}
return null;
},

/**
* Get an enclosing scope used to find `this` value by an arrow function
* @param {Scope} scope Current scope
* @returns {Scope} An enclosing scope used by an arrow function
*/
getArrowFunctionScope(scope) {
scope = scope.upper;
while (scope) {
if (astUtil.isFunction(scope.block) || astUtil.isClass(scope.block)) {
return scope;
if (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') {
switch (node.parent.type) {
case 'VariableDeclarator':
case 'AssignmentExpression':
case 'Property':
case 'ReturnStatement':
case 'ExportDefaultDeclaration': {
if (utils.isReturningJSXOrNull(node)) {
return node;
}
break;
}
default:
}

// Case like `React.memo(() => <></>)`
const pragmaComponentWrapper = this.getPragmaComponentWrapper(node);
if (pragmaComponentWrapper) {
return pragmaComponentWrapper;
}
}

scope = scope.upper;
}
return null;
Expand Down
10 changes: 0 additions & 10 deletions lib/util/ast.js
Expand Up @@ -132,15 +132,6 @@ function isFunction(node) {
return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration';
}

/**
* Checks if the node is an arrow function.
* @param {ASTNode} node The node to check
* @return {Boolean} true if it's an arrow function
*/
function isArrowFunction(node) {
return node.type === 'ArrowFunctionExpression';
}

/**
* Checks if the node is a class.
* @param {ASTNode} node The node to check
Expand Down Expand Up @@ -196,7 +187,6 @@ module.exports = {
getPropertyNameNode,
getComponentProperties,
getKeyValue,
isArrowFunction,
isAssignmentLHS,
isClass,
isFunction,
Expand Down
23 changes: 23 additions & 0 deletions tests/lib/rules/destructuring-assignment.js
Expand Up @@ -256,6 +256,29 @@ ruleTester.run('destructuring-assignment', rule, {
errors: [
{message: 'Must use destructuring props assignment'}
]
}, {
code: `
module.exports = {
Foo(props) {
return <p>{props.a}</p>;
}
}
`,
errors: [{message: 'Must use destructuring props assignment'}]
}, {
code: `
export default function Foo(props) {
return <p>{props.a}</p>;
}
`,
errors: [{message: 'Must use destructuring props assignment'}]
}, {
code: `
function hof() {
return (props) => <p>{props.a}</p>;
}
`,
errors: [{message: 'Must use destructuring props assignment'}]
}, {
code: `const Foo = class extends React.PureComponent {
render() {
Expand Down
12 changes: 12 additions & 0 deletions tests/lib/rules/display-name.js
Expand Up @@ -661,6 +661,18 @@ ruleTester.run('display-name', rule, {
errors: [{
message: 'Component definition is missing display name'
}]
}, {
code: `
function Hof() {
return function () {
return <div />
}
}
`,
parser: parsers.BABEL_ESLINT,
errors: [{
message: 'Component definition is missing display name'
}]
}, {
code: `
import React, { createElement } from "react";
Expand Down
7 changes: 0 additions & 7 deletions tests/lib/rules/no-this-in-sfc.js
Expand Up @@ -217,13 +217,6 @@ ruleTester.run('no-this-in-sfc', rule, {
return <div onClick={onClick}>{this.props.foo}</div>;
}`,
errors: [{message: ERROR_MESSAGE}, {message: ERROR_MESSAGE}]
}, {
code: `
() => {
this.something();
return null;
}`,
errors: [{message: ERROR_MESSAGE}]
}, {
code: `
class Foo {
Expand Down
16 changes: 16 additions & 0 deletions tests/lib/rules/no-unused-prop-types.js
Expand Up @@ -1856,6 +1856,22 @@ ruleTester.run('no-unused-prop-types', rule, {
' previousPage: PropTypes.func.isRequired,',
'};'
].join('\n')
}, {
// issue 2350
code: `
function Foo(props) {
useEffect(() => {
const { a } = props;
document.title = a;
});
return <p/>;
}
Foo.propTypes = {
a: PropTypes.string,
}
`
}, {
code: [
'class Hello extends Component {',
Expand Down

0 comments on commit cfa6135

Please sign in to comment.