Skip to content

Commit

Permalink
[Fix] no-unused-prop-types, no-unused-state: fix false positives …
Browse files Browse the repository at this point in the history 
…when using TS type assertions

Fixes #2517

 - add `unwrapTSAsExpression` to `lib/util/ast`
 - change `as any` assertions in tests to `as unknown`
 - add failing test cases
  • Loading branch information
@ljharb
Konrad Madej authored and ljharb committed Dec 28, 2019
1 parent 99cea84 commit d1dc277
Show file tree
Hide file tree
Showing 5 changed files with 370 additions and 54 deletions.
77 changes: 47 additions & 30 deletions lib/rules/no-unused-state.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

const Components = require('../util/Components');
const docsUrl = require('../util/docsUrl');
const ast = require('../util/ast');

// Descend through all wrapping TypeCastExpressions and return the expression
// that was cast.
Expand Down Expand Up @@ -41,7 +42,7 @@ function getName(node) {
}

function isThisExpression(node) {
return uncast(node).type === 'ThisExpression';
return ast.unwrapTSAsExpression(uncast(node)).type === 'ThisExpression';
}

function getInitialClassInfo() {
Expand All @@ -62,10 +63,12 @@ function getInitialClassInfo() {
}

function isSetStateCall(node) {
const unwrappedCalleeNode = ast.unwrapTSAsExpression(node.callee);

return (
node.callee.type === 'MemberExpression' &&
isThisExpression(node.callee.object) &&
getName(node.callee.property) === 'setState'
unwrappedCalleeNode.type === 'MemberExpression' &&
isThisExpression(unwrappedCalleeNode.object) &&
getName(unwrappedCalleeNode.property) === 'setState'
);
}

Expand Down Expand Up @@ -178,16 +181,18 @@ module.exports = {
// Used to record used state fields and new aliases for both
// AssignmentExpressions and VariableDeclarators.
function handleAssignment(left, right) {
const unwrappedRight = ast.unwrapTSAsExpression(right);

switch (left.type) {
case 'Identifier':
if (isStateReference(right) && classInfo.aliases) {
if (isStateReference(unwrappedRight) && classInfo.aliases) {
classInfo.aliases.add(left.name);
}
break;
case 'ObjectPattern':
if (isStateReference(right)) {
if (isStateReference(unwrappedRight)) {
handleStateDestructuring(left);
} else if (isThisExpression(right) && classInfo.aliases) {
} else if (isThisExpression(unwrappedRight) && classInfo.aliases) {
for (const prop of left.properties) {
if (prop.type === 'Property' && getName(prop.key) === 'state') {
const name = getName(prop.value);
Expand Down Expand Up @@ -254,24 +259,30 @@ module.exports = {
if (!classInfo) {
return;
}

const unwrappedNode = ast.unwrapTSAsExpression(node);
const unwrappedArgumentNode = ast.unwrapTSAsExpression(unwrappedNode.arguments[0]);

// If we're looking at a `this.setState({})` invocation, record all the
// properties as state fields.
if (
isSetStateCall(node) &&
node.arguments.length > 0 &&
node.arguments[0].type === 'ObjectExpression'
isSetStateCall(unwrappedNode) &&
unwrappedNode.arguments.length > 0 &&
unwrappedArgumentNode.type === 'ObjectExpression'
) {
addStateFields(node.arguments[0]);
addStateFields(unwrappedArgumentNode);
} else if (
isSetStateCall(node) &&
node.arguments.length > 0 &&
node.arguments[0].type === 'ArrowFunctionExpression'
isSetStateCall(unwrappedNode) &&
unwrappedNode.arguments.length > 0 &&
unwrappedArgumentNode.type === 'ArrowFunctionExpression'
) {
if (node.arguments[0].body.type === 'ObjectExpression') {
addStateFields(node.arguments[0].body);
const unwrappedBodyNode = ast.unwrapTSAsExpression(unwrappedArgumentNode.body);

if (unwrappedBodyNode.type === 'ObjectExpression') {
addStateFields(unwrappedBodyNode);
}
if (node.arguments[0].params.length > 0 && classInfo.aliases) {
const firstParam = node.arguments[0].params[0];
if (unwrappedArgumentNode.params.length > 0 && classInfo.aliases) {
const firstParam = unwrappedArgumentNode.params[0];
if (firstParam.type === 'ObjectPattern') {
handleStateDestructuring(firstParam);
} else {
Expand All @@ -287,19 +298,21 @@ module.exports = {
}
// If we see state being assigned as a class property using an object
// expression, record all the fields of that object as state fields.
const unwrappedValueNode = ast.unwrapTSAsExpression(node.value);

if (
getName(node.key) === 'state' &&
!node.static &&
node.value &&
node.value.type === 'ObjectExpression'
unwrappedValueNode &&
unwrappedValueNode.type === 'ObjectExpression'
) {
addStateFields(node.value);
addStateFields(unwrappedValueNode);
}

if (
!node.static &&
node.value &&
node.value.type === 'ArrowFunctionExpression'
unwrappedValueNode &&
unwrappedValueNode.type === 'ArrowFunctionExpression'
) {
// Create a new set for this.state aliases local to this method.
classInfo.aliases = new Set();
Expand Down Expand Up @@ -364,12 +377,16 @@ module.exports = {
if (!classInfo) {
return;
}

const unwrappedLeft = ast.unwrapTSAsExpression(node.left);
const unwrappedRight = ast.unwrapTSAsExpression(node.right);

// Check for assignments like `this.state = {}`
if (
node.left.type === 'MemberExpression' &&
isThisExpression(node.left.object) &&
getName(node.left.property) === 'state' &&
node.right.type === 'ObjectExpression'
unwrappedLeft.type === 'MemberExpression' &&
isThisExpression(unwrappedLeft.object) &&
getName(unwrappedLeft.property) === 'state' &&
unwrappedRight.type === 'ObjectExpression'
) {
// Find the nearest function expression containing this assignment.
let fn = node;
Expand All @@ -383,11 +400,11 @@ module.exports = {
fn.parent.type === 'MethodDefinition' &&
fn.parent.kind === 'constructor'
) {
addStateFields(node.right);
addStateFields(unwrappedRight);
}
} else {
// Check for assignments like `alias = this.state` and record the alias.
handleAssignment(node.left, node.right);
handleAssignment(unwrappedLeft, unwrappedRight);
}
},

Expand All @@ -402,7 +419,7 @@ module.exports = {
if (!classInfo) {
return;
}
if (isStateReference(node.object)) {
if (isStateReference(ast.unwrapTSAsExpression(node.object))) {
// If we see this.state[foo] access, give up.
if (node.computed && node.property.type !== 'Literal') {
classInfo = null;
Expand Down
14 changes: 13 additions & 1 deletion lib/util/ast.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ function isAssignmentLHS(node) {
);
}

/**
* Extracts the expression node that is wrapped inside a TS type assertion
*
* @param {ASTNode} node - potential TS node
* @returns {ASTNode} - unwrapped expression node
*/
function unwrapTSAsExpression(node) {
if (node && node.type === 'TSAsExpression') return node.expression;
return node;
}

module.exports = {
findReturnStatement,
getFirstNodeInLine,
Expand All @@ -196,5 +207,6 @@ module.exports = {
isClass,
isFunction,
isFunctionLikeExpression,
isNodeFirstInLine
isNodeFirstInLine,
unwrapTSAsExpression
};
54 changes: 33 additions & 21 deletions lib/util/usedPropTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,12 @@ function isInLifeCycleMethod(node, checkAsyncSafeLifeCycles) {
* @return {boolean}
*/
function isSetStateUpdater(node) {
return node.parent.type === 'CallExpression' &&
node.parent.callee.property &&
node.parent.callee.property.name === 'setState' &&
const unwrappedParentCalleeNode = node.parent.type === 'CallExpression' &&
ast.unwrapTSAsExpression(node.parent.callee);

return unwrappedParentCalleeNode &&
unwrappedParentCalleeNode.property &&
unwrappedParentCalleeNode.property.name === 'setState' &&
// Make sure we are in the updater not the callback
node.parent.arguments[0] === node;
}
Expand All @@ -158,11 +161,14 @@ function isPropArgumentInSetStateUpdater(context, name) {
}
let scope = context.getScope();
while (scope) {
if (
const unwrappedParentCalleeNode =
scope.block && scope.block.parent &&
scope.block.parent.type === 'CallExpression' &&
scope.block.parent.callee.property &&
scope.block.parent.callee.property.name === 'setState' &&
ast.unwrapTSAsExpression(scope.block.parent.callee);
if (
unwrappedParentCalleeNode &&
unwrappedParentCalleeNode.property &&
unwrappedParentCalleeNode.property.name === 'setState' &&
// Make sure we are in the updater not the callback
scope.block.parent.arguments[0].start === scope.block.start &&
scope.block.parent.arguments[0].params &&
Expand All @@ -187,7 +193,7 @@ function isInClassComponent(utils) {
function isThisDotProps(node) {
return !!node &&
node.type === 'MemberExpression' &&
node.object.type === 'ThisExpression' &&
ast.unwrapTSAsExpression(node.object).type === 'ThisExpression' &&
node.property.name === 'props';
}

Expand Down Expand Up @@ -242,26 +248,28 @@ function getPropertyName(node) {
* @returns {boolean}
*/
function isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafeLifeCycles) {
const unwrappedObjectNode = ast.unwrapTSAsExpression(node.object);

if (isInClassComponent(utils)) {
// this.props.*
if (isThisDotProps(node.object)) {
if (isThisDotProps(unwrappedObjectNode)) {
return true;
}
// props.* or prevProps.* or nextProps.*
if (
isCommonVariableNameForProps(node.object.name) &&
isCommonVariableNameForProps(unwrappedObjectNode.name) &&
(inLifeCycleMethod(context, checkAsyncSafeLifeCycles) || utils.inConstructor())
) {
return true;
}
// this.setState((_, props) => props.*))
if (isPropArgumentInSetStateUpdater(context, node.object.name)) {
if (isPropArgumentInSetStateUpdater(context, unwrappedObjectNode.name)) {
return true;
}
return false;
}
// props.* in function component
return node.object.name === 'props' && !ast.isAssignmentLHS(node);
return unwrappedObjectNode.name === 'props' && !ast.isAssignmentLHS(node);
}

module.exports = function usedPropTypesInstructions(context, components, utils) {
Expand Down Expand Up @@ -442,13 +450,15 @@ module.exports = function usedPropTypesInstructions(context, components, utils)

return {
VariableDeclarator(node) {
const unwrappedInitNode = ast.unwrapTSAsExpression(node.init);

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

// Only handles destructuring
if (node.id.type !== 'ObjectPattern' || !node.init) {
if (node.id.type !== 'ObjectPattern' || !unwrappedInitNode) {
return;
}

Expand All @@ -457,35 +467,36 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
property.key &&
(property.key.name === 'props' || property.key.value === 'props')
));
if (node.init.type === 'ThisExpression' && propsProperty && propsProperty.value.type === 'ObjectPattern') {

if (unwrappedInitNode.type === 'ThisExpression' && propsProperty && propsProperty.value.type === 'ObjectPattern') {
markPropTypesAsUsed(propsProperty.value);
return;
}

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

// let {firstname} = props
if (
isCommonVariableNameForProps(node.init.name) &&
isCommonVariableNameForProps(unwrappedInitNode.name) &&
(utils.getParentStatelessComponent() || isInLifeCycleMethod(node, checkAsyncSafeLifeCycles))
) {
markPropTypesAsUsed(node.id);
return;
}

// let {firstname} = this.props
if (isThisDotProps(node.init) && isInClassComponent(utils)) {
if (isThisDotProps(unwrappedInitNode) && 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.id, propVariables.get(node.init.name));
if (propVariables.get(unwrappedInitNode.name)) {
markPropTypesAsUsed(node.id, propVariables.get(unwrappedInitNode.name));
}
},

Expand Down Expand Up @@ -514,8 +525,9 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
return;
}

if (propVariables.get(node.object.name)) {
markPropTypesAsUsed(node, propVariables.get(node.object.name));
const propVariable = propVariables.get(ast.unwrapTSAsExpression(node.object).name);
if (propVariable) {
markPropTypesAsUsed(node, propVariable);
}
},

Expand Down
Loading

0 comments on commit d1dc277

Please sign in to comment.