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

[New] prop-types: handle variables defined as props #2301

Merged
merged 4 commits into from
Jun 13, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 84 additions & 3 deletions lib/util/usedPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,46 @@ const ast = require('./ast');
const LIFE_CYCLE_METHODS = ['componentWillReceiveProps', 'shouldComponentUpdate', 'componentWillUpdate', 'componentDidUpdate'];
const ASYNC_SAFE_LIFE_CYCLE_METHODS = ['getDerivedStateFromProps', 'getSnapshotBeforeUpdate', 'UNSAFE_componentWillReceiveProps', 'UNSAFE_componentWillUpdate'];

function createPropVariables() {
/** @type {Map<string, string[]>} Maps the variable to its definition. `props.a.b` is stored as `['a', 'b']` */
let propVariables = new Map();
let hasBeenWritten = false;
const stack = [{propVariables, hasBeenWritten}];
return {
pushScope() {
// popVariables is not copied until first write.
stack.push({propVariables, hasBeenWritten: false});
},
popScope() {
stack.pop();
propVariables = stack[stack.length - 1].propVariables;
hasBeenWritten = stack[stack.length - 1].hasBeenWritten;
},
/**
* Add a variable name to the current scope
* @param {string} name
* @param {string[]} allNames Example: `props.a.b` should be formatted as `['a', 'b']`
*/
set(name, allNames) {
if (!hasBeenWritten) {
// copy on write
propVariables = new Map(propVariables);
Object.assign(stack[stack.length - 1], {propVariables, hasBeenWritten: true});
stack[stack.length - 1].hasBeenWritten = true;
}
return propVariables.set(name, allNames);
},
/**
* Get the definition of a variable.
* @param {string} name
* @returns {string[]} Example: `props.a.b` is represented by `['a', 'b']`
*/
get(name) {
return propVariables.get(name);
}
};
}

/**
* Checks if the string is one of `props`, `nextProps`, or `prevProps`
* @param {string} name The AST node being checked.
Expand Down Expand Up @@ -230,6 +270,10 @@ function isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafe
module.exports = function usedPropTypesInstructions(context, components, utils) {
const checkAsyncSafeLifeCycles = versionUtil.testReactVersion(context, '16.3.0');

const propVariables = createPropVariables();
const pushScope = propVariables.pushScope;
const popScope = propVariables.popScope;

/**
* Mark a prop type as used
* @param {ASTNode} node The AST node being marked.
Expand Down Expand Up @@ -261,6 +305,14 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
node.parent.id.parent = node.parent; // patch for bug in eslint@4 in which ObjectPattern has no parent
markPropTypesAsUsed(node.parent.id, allNames);
}

// const a = props.a
if (
node.parent.type === 'VariableDeclarator' &&
node.parent.id.type === 'Identifier'
) {
propVariables.set(node.parent.id.name, allNames);
}
// Do not mark computed props as used.
type = name !== '__COMPUTED_PROP__' ? 'direct' : null;
}
Expand Down Expand Up @@ -314,6 +366,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
const propName = ast.getKeyValue(context, properties[k]);

if (propName) {
propVariables.set(propName, parentNames.concat(propName));
usedPropTypes.push({
allNames: parentNames.concat([propName]),
name: propName,
Expand Down Expand Up @@ -371,6 +424,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
* FunctionDeclaration, or FunctionExpression
*/
function handleFunctionLikeExpressions(node) {
pushScope();
handleSetStateUpdater(node);
markDestructuredFunctionArgumentsAsUsed(node);
}
Expand All @@ -392,6 +446,11 @@ module.exports = function usedPropTypesInstructions(context, components, utils)

return {
VariableDeclarator(node) {
// let props = this.props
if (isThisDotProps(node.init) && isInClassComponent(utils) && node.id.type === 'Identifier') {
propVariables.set(node.id.name, []);
}

// Only handles destructuring
if (node.id.type !== 'ObjectPattern') {
return;
Expand All @@ -400,14 +459,19 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
// let {props: {firstname}} = this
const propsProperty = node.id.properties.find(property => (
property.key &&
(property.key.name === 'props' || property.key.value === 'props') &&
property.value.type === 'ObjectPattern'
(property.key.name === 'props' || property.key.value === 'props')
));
if (propsProperty && node.init.type === 'ThisExpression') {
if (node.init.type === 'ThisExpression' && propsProperty && propsProperty.value.type === 'ObjectPattern') {
markPropTypesAsUsed(propsProperty.value);
return;
}

// let {props} = this
if (node.init.type === 'ThisExpression' && propsProperty && propsProperty.value.name === 'props') {
propVariables.set('props', []);
return;
}

// let {firstname} = props
if (
isCommonVariableNameForProps(node.init.name) &&
Expand All @@ -420,6 +484,12 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
// let {firstname} = this.props
if (isThisDotProps(node.init) && isInClassComponent(utils)) {
markPropTypesAsUsed(node.id);
return;
}

// let {firstname} = thing, where thing is defined by const thing = this.props.**.*
if (propVariables.get(node.init.name)) {
markPropTypesAsUsed(node, propVariables.get(node.init.name));
}
},

Expand All @@ -429,6 +499,12 @@ module.exports = function usedPropTypesInstructions(context, components, utils)

FunctionExpression: handleFunctionLikeExpressions,

'FunctionDeclaration:exit': popScope,

'ArrowFunctionExpression:exit': popScope,

'FunctionExpression:exit': popScope,

JSXSpreadAttribute(node) {
const component = components.get(utils.getParentComponent());
components.set(component ? component.node : node, {
Expand All @@ -439,6 +515,11 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
MemberExpression(node) {
if (isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafeLifeCycles)) {
markPropTypesAsUsed(node);
return;
}

if (propVariables.get(node.object.name)) {
markPropTypesAsUsed(node, propVariables.get(node.object.name));
}
},

Expand Down
74 changes: 53 additions & 21 deletions tests/lib/rules/no-unused-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,59 @@ ruleTester.run('no-unused-prop-types', rule, {
'};'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: `
function Foo({ a }) {
return <>{ a.b }</>
}
Foo.propTypes = {
a: PropTypes.shape({
b: PropType.string,
})
}
`,
options: [{skipShapeProps: false}],
parser: parsers.BABEL_ESLINT
}, {
// Destructured assignment with Shape propTypes with skipShapeProps off issue #816
code: `
class Thing extends React.Component {
static propTypes = {
i18n: PropTypes.shape({
gettext: PropTypes.func,
}),
}

render() {
const { i18n } = this.props;
return (
<p>{i18n.gettext('Some Text')}</p>
);
}
}
`,
parser: parsers.BABEL_ESLINT,
options: [{skipShapeProps: false}]
},
{
code: `
class Thing extends React.Component {
static propTypes = {
a: PropTypes.shape({
b: PropTypes.string,
}),
}

render() {
const { a } = this.props;
return (
<p>{ a.b }</p>
);
}
}
`,
parser: parsers.BABEL_ESLINT,
options: [{skipShapeProps: false}]
}, {
code: [
'var Hello = createReactClass({',
Expand Down Expand Up @@ -4472,27 +4525,6 @@ ruleTester.run('no-unused-prop-types', rule, {
}, {
message: '\'prop2.*\' PropType is defined but prop is never used'
}]
}, {
// Destructured assignment with Shape propTypes with skipShapeProps off issue #816
code: [
'export default class NavigationButton extends React.Component {',
' static propTypes = {',
' route: PropTypes.shape({',
' getBarTintColor: PropTypes.func.isRequired,',
' }).isRequired,',
' };',

' renderTitle() {',
' const { route } = this.props;',
' return <Title tintColor={route.getBarTintColor()}>TITLE</Title>;',
' }',
'}'
].join('\n'),
parser: parsers.BABEL_ESLINT,
options: [{skipShapeProps: false}],
errors: [{
message: '\'route.getBarTintColor\' PropType is defined but prop is never used'
}]
}, {
code: [
// issue #1097
Expand Down
97 changes: 83 additions & 14 deletions tests/lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -870,17 +870,6 @@ ruleTester.run('prop-types', rule, {
'};'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
// Reassigned props are ignored
code: [
'export class Hello extends Component {',
' render() {',
' const props = this.props;',
' return <div>Hello {props.name.firstname} {props[\'name\'].lastname}</div>',
' }',
'}'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'export default function FooBar(props) {',
Expand Down Expand Up @@ -2453,6 +2442,82 @@ ruleTester.run('prop-types', rule, {
{message: "'foo' is missing in props validation"},
{message: "'foo.bar' is missing in props validation"}
]
},
{
code: `
function Foo({ a }) {
return <p>{ a.nope }</p>
}

Foo.propTypes = {
a: PropTypes.shape({
_: PropType.string,
})
}
`,
errors: [
{message: "'a.nope' is missing in props validation"}
]
},
{
code: `
function Foo(props) {
const { a } = props
return <p>{ a.nope }</p>
}

Foo.propTypes = {
a: PropTypes.shape({
_: PropType.string,
})
}
`,
errors: [
{message: "'a.nope' is missing in props validation"}
]
},
{
code: `
function Foo(props) {
const a = props.a
return <p>{ a.nope }</p>
}

Foo.propTypes = {
a: PropTypes.shape({
_: PropType.string,
})
}
`,
errors: [
{message: "'a.nope' is missing in props validation"}
]
},
{
code: `
class Foo extends Component {
render() {
const props = this.props
return <div>{props.cat}</div>
}
}
`,
errors: [
{message: "'cat' is missing in props validation"}
]
},
{
code: `
class Foo extends Component {
render() {
const {props} = this
return <div>{props.cat}</div>
}
}
`,
errors: [
{message: "'cat' is missing in props validation"}
]
}, {
code: [
'class Hello extends React.Component {',
Expand Down Expand Up @@ -3364,6 +3429,8 @@ ruleTester.run('prop-types', rule, {
].join('\n'),
errors: [{
message: '\'names\' is missing in props validation'
}, {
message: '\'names.map\' is missing in props validation'
}]
}, {
code: [
Expand Down Expand Up @@ -4521,9 +4588,11 @@ ruleTester.run('prop-types', rule, {
}
`,
parser: parsers.BABEL_ESLINT,
errors: [{
message: '\'a\' is missing in props validation'
}]
errors: [
{message: '\'a\' is missing in props validation'},
{message: '\'a.b\' is missing in props validation'},
{message: '\'a.b.c\' is missing in props validation'}
]
},
{
code: `
Expand Down