Skip to content

Commit

Permalink
fix: eslint-plugin issue with types comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
@divdavem @quentinderoubaix
divdavem authored and quentinderoubaix committed Nov 8, 2023
1 parent 21b11f3 commit baa93b6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 31 deletions.
43 changes: 28 additions & 15 deletions eslint-plugin/src/angular-check-props.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {TSESLint} from '@typescript-eslint/utils';
import {ASTUtils, ESLintUtils, TSESTree} from '@typescript-eslint/utils';
import type {Type, TypeReference} from 'typescript';
import type {EventInfo, PropInfo} from './ast-utils';
import {
addIndentation,
Expand All @@ -12,6 +13,8 @@ import {
getNodeType,
insertNewLineBefore as insertLineBefore,
isSameDoc,
isSameType,
typeToString,
} from './ast-utils';

const getDecorator = ({decorators}: {decorators?: TSESTree.Decorator[]}, decoratorName: string) =>
Expand Down Expand Up @@ -67,7 +70,10 @@ const reportMissingInputProp = (node: TSESTree.Node, name: string, prop: PropInf
fix(fixer) {
const indentation = getIndentation(node, context);
const doc = createDocWithIndentation(prop.doc, indentation);
return fixer.insertTextBefore(node, `${doc}@Input('${validAlias(name)}') ${name}: ${prop.type};\n\n${indentation}`);
return fixer.insertTextBefore(
node,
`${doc}@Input('${validAlias(name)}') ${name}: ${typeToString(prop.type, node, context)};\n\n${indentation}`
);
},
});
};
Expand All @@ -89,7 +95,10 @@ const reportMissingOutputProp = (
*fix(fixer) {
const indentation = getIndentation(node, context);
const doc = createDocWithIndentation(prop.doc, indentation);
yield fixer.insertTextBefore(node, `${doc}@Output('${validAlias(name)}') ${name} = new EventEmitter<${prop.type}>();\n\n${indentation}`);
yield fixer.insertTextBefore(
node,
`${doc}@Output('${validAlias(name)}') ${name} = new EventEmitter<${typeToString(prop.type, node, context)}>();\n\n${indentation}`
);
const eventInApiPatch = eventsObject?.properties.get(prop.widgetProp);
const emitInFunction = eventInApiPatch ? findCallToEventEmitter(eventInApiPatch, name) : null;
if (!emitInFunction) {
Expand All @@ -105,22 +114,23 @@ const reportMissingOutputProp = (
const reportInvalidInputPropType = (
node: TSESTree.PropertyDefinition,
name: string,
foundType: string,
foundType: Type,
info: PropInfo,
context: Readonly<TSESLint.RuleContext<'invalidPropType', any>>
) => {
const expectedType = typeToString(info.type, node, context);
context.report({
node,
messageId: 'invalidPropType',
data: {
name,
type: 'input',
foundType,
expectedType: info.type,
foundType: typeToString(foundType, node, context),
expectedType,
},
fix(fixer) {
const typeAnnotation = node.typeAnnotation;
const typeText = `: ${info.type}`;
const typeText = `: ${expectedType}`;
if (typeAnnotation) {
return fixer.replaceText(typeAnnotation, typeText);
} else {
Expand All @@ -133,22 +143,22 @@ const reportInvalidInputPropType = (
const reportInvalidOutputPropType = (
node: TSESTree.PropertyDefinition,
name: string,
foundType: string,
expectedType: string,
foundType: Type,
info: PropInfo,
context: Readonly<TSESLint.RuleContext<'invalidPropType', any>>
) => {
const expectedType = `EventEmitter<${typeToString(info.type, node, context)}>`;
context.report({
node,
messageId: 'invalidPropType',
data: {
name,
type: 'output',
foundType,
foundType: typeToString(foundType, node, context),
expectedType,
},
fix(fixer) {
const newValue = `new EventEmitter<${info.type}>()`;
const newValue = `new ${expectedType}()`;
if (node.value) {
return fixer.replaceText(node.value, newValue);
} else {
Expand Down Expand Up @@ -301,7 +311,7 @@ export const angularCheckPropsRule = ESLintUtils.RuleCreator.withoutDocs({
if (inputInfo) {
widgetInfo.props.delete(name);
const nodeType = getNodeType(classMember, context);
if (nodeType !== inputInfo.type) {
if (!isSameType(nodeType, inputInfo.type, context)) {
reportInvalidInputPropType(classMember, name, nodeType, inputInfo, context);
}
const nodeComment = getNodeDocumentation(classMember.key, context);
Expand All @@ -321,10 +331,13 @@ export const angularCheckPropsRule = ESLintUtils.RuleCreator.withoutDocs({
const outputInfo = widgetInfo.events.get(name);
if (outputInfo) {
widgetInfo.events.delete(name);
const nodeType = getNodeType(classMember, context);
const expectedNodeType = `EventEmitter<${outputInfo.type}>`;
if (nodeType !== expectedNodeType) {
reportInvalidOutputPropType(classMember, name, nodeType, expectedNodeType, outputInfo, context);
const nodeType = getNodeType(classMember, context) as TypeReference;
if (
nodeType.target?.symbol?.name != 'EventEmitter' ||
nodeType.typeArguments?.length !== 1 ||
!isSameType(nodeType.typeArguments![0], outputInfo.type, context)
) {
reportInvalidOutputPropType(classMember, name, nodeType, outputInfo, context);
}
const nodeComment = getNodeDocumentation(classMember.key, context);
if (!isSameDoc(nodeComment, outputInfo.doc)) {
Expand Down
44 changes: 34 additions & 10 deletions eslint-plugin/src/ast-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ import type {TSESLint} from '@typescript-eslint/utils';
import {ASTUtils, ESLintUtils, TSESTree} from '@typescript-eslint/utils';
import type {AST as SvelteAST} from 'svelte-eslint-parser';
import type ts from 'typescript';
import {SignatureKind} from 'typescript';
import type {Type} from 'typescript';
import {SignatureKind, TypeFormatFlags} from 'typescript';

export interface EventInfo {
doc: string;
propBinding: string | undefined;
type: string;
type: Type;
widgetProp: string;
}

export interface PropInfo {
doc: string;
type: string;
type: Type;
}

export const docFromSymbol = (symbol: ts.Symbol, checker: ts.TypeChecker) => {
Expand Down Expand Up @@ -125,20 +126,18 @@ export const getInfoFromWidgetNode = (widgetNode: TSESTree.Node, context: Readon
const eventName = `${name[2].toLowerCase()}${name.slice(3)}`;
const binding = name.endsWith('Change');
const propBinding = binding ? eventName.substring(0, eventName.length - 6) : undefined;
let type = 'void';
let type = checker.getVoidType();
tsType = checker.getNonNullableType(tsType);
const eventParamSymbol = checker.getSignaturesOfType(tsType, SignatureKind.Call)[0]?.getParameters()?.[0];
if (eventParamSymbol) {
tsType = checker.getTypeOfSymbolAtLocation(eventParamSymbol, widgetTSNode);
type = checker.typeToString(tsType, widgetTSNode);
type = checker.getTypeOfSymbolAtLocation(eventParamSymbol, widgetTSNode);
}
events.set(eventName, {doc, propBinding, type, widgetProp: name});
if (propBinding) {
bindings.push(propBinding);
}
} else {
const type = checker.typeToString(tsType, widgetTSNode);
props.set(name, {doc, type});
props.set(name, {doc, type: tsType});
}
}
return {
Expand All @@ -148,12 +147,37 @@ export const getInfoFromWidgetNode = (widgetNode: TSESTree.Node, context: Readon
};
};

export const isSameType = (type1: Type, type2: Type, context: Readonly<TSESLint.RuleContext<any, any>>): boolean => {
const parserServices = ESLintUtils.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const sameTypeFlags = TypeFormatFlags.NoTruncation | TypeFormatFlags.UseFullyQualifiedType | TypeFormatFlags.InTypeAlias;
const strType1 = checker.typeToString(type1, undefined, sameTypeFlags);
const strType2 = checker.typeToString(type2, undefined, sameTypeFlags);
if (strType1 === strType2) {
return true;
}
if (strType1 === 'any' || strType2 === 'any') {
// any is assignable to any other type, and any other type is assignable to any
// but it does not mean they are the same type
return false;
}
// isTypeAssignableTo is not publicly exposed, cf https://github.com/microsoft/TypeScript/issues/9879#issuecomment-1758108764
const isTypeAssignableTo: (type1: Type, type2: Type) => boolean = (checker as any).isTypeAssignableTo.bind(checker);
return isTypeAssignableTo(type1, type2) && isTypeAssignableTo(type2, type1);
};

export const typeToString = (type: Type, node: TSESTree.Node, context: Readonly<TSESLint.RuleContext<any, any>>) => {
const parserServices = ESLintUtils.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
return checker.typeToString(type, tsNode, TypeFormatFlags.NoTruncation);
};

export const getNodeType = (node: TSESTree.Node, context: Readonly<TSESLint.RuleContext<any, any>>) => {
const parserServices = ESLintUtils.getParserServices(context);
const checker = parserServices.program.getTypeChecker();
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const nodeType = checker.getTypeAtLocation(tsNode);
return checker.typeToString(nodeType, tsNode);
return checker.getTypeAtLocation(tsNode);
};

export const getNodeDocumentation = (node: TSESTree.Node, context: Readonly<TSESLint.RuleContext<any, any>>) => {
Expand Down
16 changes: 10 additions & 6 deletions eslint-plugin/src/svelte-check-props.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {TSESLint} from '@typescript-eslint/utils';
import {ESLintUtils, TSESTree} from '@typescript-eslint/utils';
import type {AST as SvelteAST} from 'svelte-eslint-parser';
import type {Type} from 'typescript';
import type {EventInfo, PropInfo} from './ast-utils';
import {
addIndentation,
Expand All @@ -10,6 +11,8 @@ import {
getInfoFromWidgetNode,
getNodeType,
insertNewLineBefore,
isSameType,
typeToString,
} from './ast-utils';

type ExportLetNode = TSESTree.VariableDeclarator & {
Expand Down Expand Up @@ -97,28 +100,29 @@ const reportMissingProp = (
name,
},
fix(fixer) {
return insertNewLineBefore(fixer, insertPosition, `export let ${name}: ${prop.type} = undefined;`, context);
return insertNewLineBefore(fixer, insertPosition, `export let ${name}: ${typeToString(prop.type, node, context)} = undefined;`, context);
},
});
};

const reportInvalidPropType = (
node: ExportLetNode,
info: PropInfo,
foundType: string,
foundType: Type,
context: Readonly<TSESLint.RuleContext<'invalidPropType', any>>
) => {
const expectedType = typeToString(info.type, node, context);
context.report({
node,
messageId: 'invalidPropType',
data: {
name: node.id.name,
expectedType: info.type,
foundType,
expectedType,
foundType: typeToString(foundType, node, context),
},
fix(fixer) {
const typeAnnotation = node.id.typeAnnotation;
const typeText = `: ${info.type}`;
const typeText = `: ${expectedType}`;
if (typeAnnotation) {
return fixer.replaceText(typeAnnotation, typeText);
} else {
Expand Down Expand Up @@ -302,7 +306,7 @@ export const svelteCheckPropsRule = ESLintUtils.RuleCreator.withoutDocs({
if (validProp) {
const propInfo = widgetInfo.props.get(name)!;
const nodeType = getNodeType(exportNode, context);
if (propInfo.type !== nodeType) {
if (!isSameType(propInfo.type, nodeType, context)) {
reportInvalidPropType(exportNode, propInfo, nodeType, context);
}
}
Expand Down

0 comments on commit baa93b6

Please sign in to comment.