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

[Fix]: false positives for no-unused-... when using TS type assertions #2536

Merged
merged 1 commit into from Jan 15, 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
77 changes: 47 additions & 30 deletions lib/rules/no-unused-state.js
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
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
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