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

feat(ast-spec): remove deprecated type params #8933

Merged
Merged
Show file tree
Hide file tree
Changes from 20 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
4 changes: 4 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@ export default tseslint.config(
'react/jsx-no-target-blank': 'off',
'react/no-unescaped-entities': 'off',
'react-hooks/exhaustive-deps': 'warn', // TODO: enable it later
/* This rule
(1) causes false reports because it reads from `typeParameters` rather than `typeArguments`, and
(2) is unnecessary in TypeScript components */
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
'react/prop-types': 'off',
},
settings: {
react: {
Expand Down
4 changes: 0 additions & 4 deletions packages/ast-spec/src/base/ClassBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ export interface ClassBase extends BaseNode {
* The generic type parameters passed to the superClass.
*/
superTypeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `superTypeArguments`} instead. */
superTypeParameters: TSTypeParameterInstantiation | undefined;

/**
* The generic type parameters declared for the class.
*/
Expand Down
3 changes: 0 additions & 3 deletions packages/ast-spec/src/base/TSHeritageBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,4 @@ export interface TSHeritageBase extends BaseNode {
// TODO(#1852) - this should be restricted to MemberExpression | Identifier
expression: Expression;
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;
}
4 changes: 0 additions & 4 deletions packages/ast-spec/src/expression/CallExpression/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,5 @@ export interface CallExpression extends BaseNode {
callee: LeftHandSideExpression;
arguments: CallExpressionArgument[];
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;

optional: boolean;
}
3 changes: 0 additions & 3 deletions packages/ast-spec/src/expression/NewExpression/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,4 @@ export interface NewExpression extends BaseNode {
callee: LeftHandSideExpression;
arguments: CallExpressionArgument[];
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,4 @@ export interface TSInstantiationExpression extends BaseNode {
type: AST_NODE_TYPES.TSInstantiationExpression;
expression: Expression;
typeArguments: TSTypeParameterInstantiation;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters?: TSTypeParameterInstantiation;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ import type { TemplateLiteral } from '../TemplateLiteral/spec';
export interface TaggedTemplateExpression extends BaseNode {
type: AST_NODE_TYPES.TaggedTemplateExpression;
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;

tag: LeftHandSideExpression;
quasi: TemplateLiteral;
}
4 changes: 0 additions & 4 deletions packages/ast-spec/src/jsx/JSXOpeningElement/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import type { JSXSpreadAttribute } from '../JSXSpreadAttribute/spec';
export interface JSXOpeningElement extends BaseNode {
type: AST_NODE_TYPES.JSXOpeningElement;
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;

selfClosing: boolean;
name: JSXTagNameExpression;
attributes: (JSXAttribute | JSXSpreadAttribute)[];
Expand Down
3 changes: 0 additions & 3 deletions packages/ast-spec/src/type/TSImportType/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,4 @@ export interface TSImportType extends BaseNode {
argument: TypeNode;
qualifier: EntityName | null;
typeArguments: TSTypeParameterInstantiation | null;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | null;
}
3 changes: 0 additions & 3 deletions packages/ast-spec/src/type/TSTypeQuery/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,4 @@ export interface TSTypeQuery extends BaseNode {
type: AST_NODE_TYPES.TSTypeQuery;
exprName: EntityName | TSImportType;
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;
}
4 changes: 0 additions & 4 deletions packages/ast-spec/src/type/TSTypeReference/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,5 @@ import type { EntityName } from '../../unions/EntityName';
export interface TSTypeReference extends BaseNode {
type: AST_NODE_TYPES.TSTypeReference;
typeArguments: TSTypeParameterInstantiation | undefined;

/** @deprecated Use {@link `typeArguments`} instead. */
typeParameters: TSTypeParameterInstantiation | undefined;

typeName: EntityName;
}
206 changes: 112 additions & 94 deletions packages/eslint-plugin/src/rules/no-unsafe-argument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
isTypeAnyArrayType,
isTypeAnyType,
isUnsafeAssignment,
nullThrows,
} from '../util';

type MessageIds =
Expand Down Expand Up @@ -162,114 +163,131 @@ export default createRule<[], MessageIds>({
const services = getParserServices(context);
const checker = services.program.getTypeChecker();

return {
'CallExpression, NewExpression'(
node: TSESTree.CallExpression | TSESTree.NewExpression,
): void {
if (node.arguments.length === 0) {
return;
}
function checkUnsafeArguments(
args: TSESTree.Expression[] | TSESTree.CallExpressionArgument[],
callee: TSESTree.LeftHandSideExpression,
node:
| TSESTree.CallExpression
| TSESTree.NewExpression
| TSESTree.TaggedTemplateExpression,
): void {
if (args.length === 0) {
return;
}

// ignore any-typed calls as these are caught by no-unsafe-call
if (isTypeAnyType(services.getTypeAtLocation(node.callee))) {
return;
}
// ignore any-typed calls as these are caught by no-unsafe-call
if (isTypeAnyType(services.getTypeAtLocation(callee))) {
return;
}

const tsNode = services.esTreeNodeToTSNodeMap.get(node);
const signature = FunctionSignature.create(checker, tsNode);
if (!signature) {
return;
}
const tsNode = services.esTreeNodeToTSNodeMap.get(node);
const signature = nullThrows(
FunctionSignature.create(checker, tsNode),
'Expected to a signature resolved',
);

for (const argument of node.arguments) {
switch (argument.type) {
// spreads consume
case AST_NODE_TYPES.SpreadElement: {
const spreadArgType = services.getTypeAtLocation(
argument.argument,
);
if (node.type === AST_NODE_TYPES.TaggedTemplateExpression) {
// Consumes the first parameter (TemplateStringsArray) of the function called with TaggedTemplateExpression.
signature.getNextParameterType();
}

for (const argument of args) {
switch (argument.type) {
// spreads consume
case AST_NODE_TYPES.SpreadElement: {
const spreadArgType = services.getTypeAtLocation(argument.argument);

if (isTypeAnyType(spreadArgType)) {
// foo(...any)
context.report({
node: argument,
messageId: 'unsafeSpread',
});
} else if (isTypeAnyArrayType(spreadArgType, checker)) {
// foo(...any[])
if (isTypeAnyType(spreadArgType)) {
// foo(...any)
context.report({
node: argument,
messageId: 'unsafeSpread',
});
} else if (isTypeAnyArrayType(spreadArgType, checker)) {
// foo(...any[])

// TODO - we could break down the spread and compare the array type against each argument
context.report({
node: argument,
messageId: 'unsafeArraySpread',
});
} else if (checker.isTupleType(spreadArgType)) {
// foo(...[tuple1, tuple2])
const spreadTypeArguments =
checker.getTypeArguments(spreadArgType);
for (const tupleType of spreadTypeArguments) {
const parameterType = signature.getNextParameterType();
if (parameterType == null) {
continue;
}
const result = isUnsafeAssignment(
tupleType,
parameterType,
checker,
// we can't pass the individual tuple members in here as this will most likely be a spread variable
// not a spread array
null,
);
if (result) {
context.report({
node: argument,
messageId: 'unsafeTupleSpread',
data: {
sender: checker.typeToString(tupleType),
receiver: checker.typeToString(parameterType),
},
});
}
// TODO - we could break down the spread and compare the array type against each argument
context.report({
node: argument,
messageId: 'unsafeArraySpread',
});
} else if (checker.isTupleType(spreadArgType)) {
// foo(...[tuple1, tuple2])
const spreadTypeArguments =
checker.getTypeArguments(spreadArgType);
for (const tupleType of spreadTypeArguments) {
const parameterType = signature.getNextParameterType();
if (parameterType == null) {
continue;
}
if (spreadArgType.target.hasRestElement) {
// the last element was a rest - so all remaining defined arguments can be considered "consumed"
// all remaining arguments should be compared against the rest type (if one exists)
signature.consumeRemainingArguments();
const result = isUnsafeAssignment(
tupleType,
parameterType,
checker,
// we can't pass the individual tuple members in here as this will most likely be a spread variable
// not a spread array
null,
);
if (result) {
context.report({
node: argument,
messageId: 'unsafeTupleSpread',
data: {
sender: checker.typeToString(tupleType),
receiver: checker.typeToString(parameterType),
},
});
}
} else {
// something that's iterable
// handling this will be pretty complex - so we ignore it for now
// TODO - handle generic iterable case
}
break;
if (spreadArgType.target.hasRestElement) {
// the last element was a rest - so all remaining defined arguments can be considered "consumed"
// all remaining arguments should be compared against the rest type (if one exists)
signature.consumeRemainingArguments();
}
} else {
// something that's iterable
// handling this will be pretty complex - so we ignore it for now
// TODO - handle generic iterable case
}
break;
}

default: {
const parameterType = signature.getNextParameterType();
if (parameterType == null) {
continue;
}
default: {
const parameterType = signature.getNextParameterType();
if (parameterType == null) {
continue;
}

const argumentType = services.getTypeAtLocation(argument);
const result = isUnsafeAssignment(
argumentType,
parameterType,
checker,
argument,
);
if (result) {
context.report({
node: argument,
messageId: 'unsafeArgument',
data: {
sender: checker.typeToString(argumentType),
receiver: checker.typeToString(parameterType),
},
});
}
const argumentType = services.getTypeAtLocation(argument);
const result = isUnsafeAssignment(
argumentType,
parameterType,
checker,
argument,
);
if (result) {
context.report({
node: argument,
messageId: 'unsafeArgument',
data: {
sender: checker.typeToString(argumentType),
receiver: checker.typeToString(parameterType),
},
});
}
}
}
}
}

return {
'CallExpression, NewExpression'(
node: TSESTree.CallExpression | TSESTree.NewExpression,
): void {
checkUnsafeArguments(node.arguments, node.callee, node);
},
TaggedTemplateExpression(node: TSESTree.TaggedTemplateExpression): void {
checkUnsafeArguments(node.quasi.expressions, node.tag, node);
},
};
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,24 @@ const analyzeAndChainOperand: OperandAnalyzer = (
chain,
) => {
switch (operand.comparisonType) {
case NullishComparisonType.Boolean:
case NullishComparisonType.Boolean: {
const nextOperand = chain.at(index + 1);
if (
nextOperand?.comparisonType ===
NullishComparisonType.NotStrictEqualNull &&
operand.comparedName.type === AST_NODE_TYPES.Identifier
) {
return null;
}
return [operand];
}

case NullishComparisonType.NotEqualNullOrUndefined:
return [operand];

case NullishComparisonType.NotStrictEqualNull: {
// handle `x !== null && x !== undefined`
const nextOperand = chain[index + 1] as ValidOperand | undefined;
const nextOperand = chain.at(index + 1);
if (
nextOperand?.comparisonType ===
NullishComparisonType.NotStrictEqualUndefined &&
Expand All @@ -94,7 +105,7 @@ const analyzeAndChainOperand: OperandAnalyzer = (

case NullishComparisonType.NotStrictEqualUndefined: {
// handle `x !== undefined && x !== null`
const nextOperand = chain[index + 1] as ValidOperand | undefined;
const nextOperand = chain.at(index + 1);
if (
nextOperand?.comparisonType ===
NullishComparisonType.NotStrictEqualNull &&
Expand Down Expand Up @@ -132,7 +143,7 @@ const analyzeOrChainOperand: OperandAnalyzer = (

case NullishComparisonType.StrictEqualNull: {
// handle `x === null || x === undefined`
const nextOperand = chain[index + 1] as ValidOperand | undefined;
const nextOperand = chain.at(index + 1);
if (
nextOperand?.comparisonType ===
NullishComparisonType.StrictEqualUndefined &&
Expand All @@ -159,7 +170,7 @@ const analyzeOrChainOperand: OperandAnalyzer = (

case NullishComparisonType.StrictEqualUndefined: {
// handle `x === undefined || x === null`
const nextOperand = chain[index + 1] as ValidOperand | undefined;
const nextOperand = chain.at(index + 1);
if (
nextOperand?.comparisonType === NullishComparisonType.StrictEqualNull &&
compareNodes(operand.comparedName, nextOperand.comparedName) ===
Expand Down