From f44e75fc728d01e22413ffec8767603d83a64de6 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 31 Jan 2019 17:23:12 -0800 Subject: [PATCH 01/13] Added tslint unified-signatures rule --- .../docs/rules/unified-signatures.md | 32 + .../lib/rules/unified-signatures.js | 575 ++++++++++++++++++ packages/eslint-plugin/lib/util.js | 25 + .../tests/lib/rules/unified-signatures.js | 317 ++++++++++ 4 files changed, 949 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/unified-signatures.md create mode 100644 packages/eslint-plugin/lib/rules/unified-signatures.js create mode 100644 packages/eslint-plugin/tests/lib/rules/unified-signatures.js diff --git a/packages/eslint-plugin/docs/rules/unified-signatures.md b/packages/eslint-plugin/docs/rules/unified-signatures.md new file mode 100644 index 00000000000..63c77e3a30f --- /dev/null +++ b/packages/eslint-plugin/docs/rules/unified-signatures.md @@ -0,0 +1,32 @@ +# Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. (unified-signatures) + +Please describe the origin of the rule here. + + +## Rule Details + +This rule aims to... + +Examples of **incorrect** code for this rule: + +```ts +function f(x: number): void; +function f(x: string): void; +``` +```ts +f(): void; +f(...x: number[]): void; +``` + +Examples of **correct** code for this rule: + +```ts +function f(x: number | stting): void; +``` +```ts +function f(x?: ...number[]): void; +``` + +## Related to + + - TSLint: ['unified-signatures`](https://palantir.github.io/tslint/rules/unified-signatures/) \ No newline at end of file diff --git a/packages/eslint-plugin/lib/rules/unified-signatures.js b/packages/eslint-plugin/lib/rules/unified-signatures.js new file mode 100644 index 00000000000..3e2af4fc99c --- /dev/null +++ b/packages/eslint-plugin/lib/rules/unified-signatures.js @@ -0,0 +1,575 @@ +// @ts-check +/** + * @fileoverview Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. + * @author Armando Aguirre + */ +"use strict"; +const ts = require("typescript"); +const tsutils = require("tsutils"); +const util = require("../util"); +const { Syntax } = require("@typescript-eslint/parser") + +/** @typedef {import("estree").Node} Node */ +/** @typedef {import("eslint").Rule.RuleContext} Context */ + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +/** @type {import("eslint").Rule.RuleModule} */ +module.exports = { + meta: { + docs: { + description: "Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter.", + category: "TypeScript-specific", + recommended: false, + extraDescription: [util.tslintRule('unified-signatures')], + url: util.metaDocsUrl(" unified-signature") + }, + fixable: "code", + type: "suggestion" + }, + + create: function (context) { + + const sourceCode = context.getSourceCode(); + + /** + * @typedef Failure + * @type {object} + * @property {Unify} unify + * @property {boolean} only2 + */ + /** + * @typedef Unify + * @type {{kind: 'single-parameter-difference', p0: any, p1: any} | {kind: 'extra-parameter', extraParameter: any, otherSignature: any}} + */ + /** + * Given a node, if it could potentially be an overload, return its signature and key. + * All signatures which are overloads should have equal keys. + * @template T + * @callback GetOverload + * @param {T} node + * @returns {{signature: any, key: string} | undefined} + */ + /** + * Returns true if typeName is the name of an *outer* type parameter. + * In: `interface I { m(x: U): T }`, only `T` is an outer type parameter. + * @callback IsTypeParameter + * @param {string} typeName + * @returns {boolean} + */ + /** + * @template T + * @template Out + * @callback ForEachPairCallback + * @param {T} a + * @param {T} b + * @returns {Out | undefined} + */ + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + /** + * @param {number} [otherLine] + */ + function FAILURE_STRING_OMITTING_SINGLE_PARAMETER(otherLine) { + return `${FAILURE_STRING_START(otherLine)} with an optional parameter.`; + } + + /** + * @param {number} [otherLine] + */ + function FAILURE_STRING_OMITTING_REST_PARAMETER(otherLine) { + return `${FAILURE_STRING_START(otherLine)} with a rest parameter.`; + } + + /** + * @param {number | undefined} otherLine + * @param {string} type1 + * @param {string} type2 + */ + function FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE(otherLine, type1, type2) { + return `${FAILURE_STRING_START(otherLine)} taking \`${type1} | ${type2}\`.`; + } + + /** + * @param {number} [otherLine] + * @returns {string} + */ + function FAILURE_STRING_START(otherLine) { + // For only 2 overloads we don't need to specify which is the other one. + const overloads = + otherLine === undefined + ? "These overloads" + : `This overload and the one on line ${otherLine}`; + return `${overloads} can be combined into one signature`; + } + + /** + * @param {Array} statements + * @returns {void} + */ + function checkStatements(statements) { + addFailures( + checkOverloads(statements, undefined, statement => { + if (statement.type === Syntax.TSDeclareFunction) { + const { body, id } = statement; + return body === undefined && id !== undefined + ? { signature: statement, key: id.name } + : undefined; + } else { + return undefined; + } + }) + ); + } + + /** + * @param {Array} members + * @param {Array} [typeParameters] + * @returns {void} + */ + function checkMembers(members, typeParameters) { + addFailures( + checkOverloads(members, typeParameters, member => { + switch (member.type) { + case Syntax.TSCallSignatureDeclaration: + case Syntax.TSConstructSignatureDeclaration: + case Syntax.TSMethodSignature: + break; + case Syntax.TSAbstractMethodDefinition: + case Syntax.MethodDefinition: + if (member.value.body !== null) { + return undefined; + } + break; + default: + return undefined; + + } + + const key = getOverloadKey(member); + return key === undefined ? undefined : { signature: member, key }; + }) + ); + } + + /** + * @param {Failure[]} failures + */ + function addFailures(failures) { + for (const failure of failures) { + const { unify, only2 } = failure; + switch (unify.kind) { + case "single-parameter-difference": { + const { p0, p1 } = unify; + const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line + context.report({ + loc: p1.loc, + message: FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( + lineOfOtherOverload, + sourceCode.getText(p0.typeAnnotation.typeAnnotation), + sourceCode.getText(p1.typeAnnotation.typeAnnotation) + ), + }); + break; + } + case "extra-parameter": { + const { extraParameter, otherSignature } = unify; + const lineOfOtherOverload = only2 ? undefined : otherSignature.loc.start.line; + + context.report({ + loc: extraParameter.loc, + message: extraParameter.type === Syntax.RestElement + ? FAILURE_STRING_OMITTING_REST_PARAMETER(lineOfOtherOverload) + : FAILURE_STRING_OMITTING_SINGLE_PARAMETER(lineOfOtherOverload) + }); + } + } + } + } + + /** + * @template T + * @param {Array} signatures + * @param {Array | undefined} typeParameters + * @param {GetOverload} getOverload + * @return {Failure[]} + */ + function checkOverloads(signatures, typeParameters, getOverload) { + /** @type {Failure[]} */ + const result = []; + const isTypeParameter = getIsTypeParameter(typeParameters); + for (const overloads of collectOverloads(signatures, getOverload)) { + if (overloads.length === 2) { + + // Classes returns parameters on its value property + const signature0 = overloads[0].value || overloads[0]; + const signature1 = overloads[1].value || overloads[1]; + + const unify = compareSignatures(signature0, signature1, isTypeParameter); + if (unify !== undefined) { + result.push({ unify, only2: true }); + } + } else { + forEachPair(overloads, (a, b) => { + const unify = compareSignatures(a, b, isTypeParameter); + if (unify !== undefined) { + result.push({ unify, only2: false }); + } + }); + } + } + return result; + } + + /** + * @param {any} a + * @param {any} b + * @param {IsTypeParameter} isTypeParameter + * @returns {Unify | undefined} + */ + function compareSignatures(a, b, isTypeParameter) { + if (!signaturesCanBeUnified(a, b, isTypeParameter)) { + return undefined; + } + + return a.params.length === b.params.length + ? signaturesDifferBySingleParameter(a.params, b.params) + : signaturesDifferByOptionalOrRestParameter(a, b); + } + + /** + * @param {any} a + * @param {any} b + * @param {IsTypeParameter} isTypeParameter + * @returns {boolean} + */ + function signaturesCanBeUnified(a, b, isTypeParameter) { + // Must return the same type. + + const aTypeParams = a.typeParameters !== undefined ? a.typeParameters.params : undefined; + const bTypeParams = b.typeParameters !== undefined ? b.typeParameters.params : undefined; + + return ( + typesAreEqual(a.returnType, b.returnType) && + // Must take the same type parameters. + // If one uses a type parameter (from outside) and the other doesn't, they shouldn't be joined. + util.arraysAreEqual(aTypeParams, bTypeParams, typeParametersAreEqual) && + signatureUsesTypeParameter(a, isTypeParameter) === signatureUsesTypeParameter(b, isTypeParameter) + ); + } + + /** + * Detect `a(x: number, y: number, z: number)` and `a(x: number, y: string, z: number)`. + * @param {Array} types1 + * @param {Array} types2 + * @returns {Unify | undefined} + */ + function signaturesDifferBySingleParameter(types1, types2) { + const index = getIndexOfFirstDifference(types1, types2, parametersAreEqual); + if (index === undefined) { + return undefined; + } + + // If remaining arrays are equal, the signatures differ by just one parameter type + if (!util.arraysAreEqual(types1.slice(index + 1), types2.slice(index + 1), parametersAreEqual)) { + return undefined; + } + + const a = types1[index]; + const b = types2[index]; + // Can unify `a?: string` and `b?: number`. Can't unify `...args: string[]` and `...args: number[]`. + // See https://github.com/Microsoft/TypeScript/issues/5077 + return parametersHaveEqualSigils(a, b) && a.type !== Syntax.RestElement + ? { kind: "single-parameter-difference", p0: a, p1: b } + : undefined; + } + + /** + * Detect `a(): void` and `a(x: number): void`. + * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. + * @param {any} a + * @param {any} b + * @returns {Unify | undefined} + */ + function signaturesDifferByOptionalOrRestParameter(a, b) { + + const sig1 = a.params; + const sig2 = b.params; + + const minLength = Math.min(sig1.length, sig2.length); + const longer = sig1.length < sig2.length ? sig2 : sig1; + const shorter = sig1.length < sig2.length ? sig1 : sig2; + const shorterSig = sig1.length < sig2.length ? a : b; + + // If one is has 2+ parameters more than the other, they must all be optional/rest. + // Differ by optional parameters: f() and f(x), f() and f(x, ?y, ...z) + // Not allowed: f() and f(x, y) + for (let i = minLength + 1; i < longer.length; i++) { + if (!parameterMayBeMissing(longer[i])) { + return undefined; + } + } + + for (let i = 0; i < minLength; i++) { + if (!typesAreEqual(sig1[i].typeAnnotation, sig2[i].typeAnnotation)) { + return undefined; + } + } + + if (minLength > 0 && shorter[minLength - 1].type === Syntax.RestElement) { + return undefined; + } + + return { + extraParameter: longer[longer.length - 1], + kind: "extra-parameter", + otherSignature: shorterSig, + }; + } + + /** + * Given type parameters, returns a function to test whether a type is one of those parameters. + * @param {any} [typeParameters] + * @returns {IsTypeParameter} + */ + function getIsTypeParameter(typeParameters) { + if (typeParameters === undefined) { + return () => false; + } + + /** @type {Set} */ + const set = new Set([]); + for (const t of typeParameters.params) { + set.add(t.name.name); + } + return (typeName) => set.has(typeName); + } + + /** + * True if any of the outer type parameters are used in a signature. + * @param {any} sig + * @param {IsTypeParameter} isTypeParameter + * @returns {boolean} + */ + function signatureUsesTypeParameter(sig, isTypeParameter) { + return sig.params.some( + p => typeContainsTypeParameter(p.typeAnnotation) === true, + ); + + /** + * @param {any} type + * @returns {boolean | undefined} + */ + function typeContainsTypeParameter(type) { + if (!type) { + return false; + } + + if (type.type === Syntax.TSTypeReference) { + const typeName = type.typeName; + if (typeName.type === Syntax.Identifier && isTypeParameter(typeName.name)) { + return true; + } + } + + return typeContainsTypeParameter(type.typeAnnotation || type.elementType); + } + } + + /** + * Given all signatures, collects an array of arrays of signatures which are all overloads. + * Does not rely on overloads being adjacent. This is similar to code in adjacentOverloadSignaturesRule.ts, but not the same. + * @template T + * @param {Array} nodes + * @param {GetOverload} getOverload + * @returns {any[][]} + */ + function collectOverloads(nodes, getOverload) { + /** @type {Map} */ + const map = new Map(); + for (const sig of nodes) { + const overload = getOverload(sig); + if (overload === undefined) { + continue; + } + + const { signature, key } = overload; + const overloads = map.get(key); + if (overloads !== undefined) { + overloads.push(signature); + } else { + map.set(key, [signature]); + } + } + return Array.from(map.values()); + } + + /** + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function parametersAreEqual(a, b) { + return parametersHaveEqualSigils(a, b) && typesAreEqual(a.typeAnnotation, b.typeAnnotation); + } + + /** + * True for optional/rest parameters. + * @param {any} p + * @returns {boolean} + */ + function parameterMayBeMissing(p) { + return p.type === Syntax.RestElement || p.optional; + } + + /** + * False if one is optional and the other isn't, or one is a rest parameter and the other isn't. + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function parametersHaveEqualSigils(a, b) { + return ( + (a.type === Syntax.RestElement) === (b.type === Syntax.RestElement) && + (a.optional !== undefined) === (b.optional !== undefined) + ); + } + + /** + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function typeParametersAreEqual(a, b) { + return a.name.name === b.name.name && constraintsAreEqual(a.constraint, b.constraint); + } + + /** + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function typesAreEqual(a, b) { + // TODO: Could traverse AST so that formatting differences don't affect this. + return a === b || (a !== undefined && b !== undefined && a.typeAnnotation.type === b.typeAnnotation.type); + } + + /** + * + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function constraintsAreEqual(a, b) { + return a === b || (a !== undefined && b !== undefined && a.type === b.type); + } + + /** + * Returns the first index where `a` and `b` differ. + * @template T + * @param {Array} a + * @param {Array} b + * @param {util.Equal} equal + * @returns {number | undefined} + */ + function getIndexOfFirstDifference(a, b, equal) { + for (let i = 0; i < a.length && i < b.length; i++) { + if (!equal(a[i], b[i])) { + return i; + } + } + return undefined; + } + + /** + * Calls `action` for every pair of values in `values`. + * @template T + * @template Out + * @param {Array} values + * @param {ForEachPairCallback} action + * @returns {Out | undefined} + */ + function forEachPair(values, action) { + for (let i = 0; i < values.length; i++) { + for (let j = i + 1; j < values.length; j++) { + const result = action(values[i], values[j]); + if (result !== undefined) { + return result; + } + } + } + return undefined; + } + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + Program(node) { + checkStatements(node.body) + }, + TSModuleBlock(node) { + checkStatements(node.body); + }, + TSInterfaceDeclaration(node) { + checkMembers(node.body.body, node.typeParameters); + }, + ClassDeclaration(node) { + checkMembers(node.body.body, node.typeParameters); + }, + TSTypeLiteral(node) { + checkMembers(node.members); + } + }; + } +}; + +/** + * @param {any} node + * @returns {string | undefined} + */ +function getOverloadKey(node) { + const info = getOverloadInfo(node); + if (info === undefined) { + return undefined; + } + + return (node.computed ? "0" : "1") + (node.static ? "0" : "1") + info; +} + +/** + * @param {any} node + * @returns {string | { name: string; computed?: boolean } | undefined} + */ +function getOverloadInfo(node) { + switch (node.type) { + case Syntax.TSConstructSignatureDeclaration: + return "constructor"; + case Syntax.TSCallSignatureDeclaration: + return "()"; + default: { + const { key } = node; + if (key === undefined) { + return undefined; + } + + switch (key.type) { + case Syntax.Identifier: + return key.name; + case Syntax.Property: + const { value } = key; + return tsutils.isLiteralExpression(value) + ? value.name + : { name: value.getText(), computed: true }; + default: + return tsutils.isLiteralExpression(key) ? key.name : undefined; + } + } + } +} diff --git a/packages/eslint-plugin/lib/util.js b/packages/eslint-plugin/lib/util.js index e12e15f8041..a1893326dbf 100644 --- a/packages/eslint-plugin/lib/util.js +++ b/packages/eslint-plugin/lib/util.js @@ -127,3 +127,28 @@ exports.getParserServices = context => { } return context.parserServices; }; + +/** + * @template T + * @callback Equal + * @param {T} a + * @param {T} b + * @returns {boolean} + */ + +/** + * @template T + * @param {Array | undefined} a + * @param {Array | undefined} b + * @param {Equal} eq + * @returns {boolean} + */ +exports.arraysAreEqual = (a, b, eq) => { + return ( + a === b || + (a !== undefined && + b !== undefined && + a.length === b.length && + a.every((x, idx) => eq(x, b[idx]))) + ); +} \ No newline at end of file diff --git a/packages/eslint-plugin/tests/lib/rules/unified-signatures.js b/packages/eslint-plugin/tests/lib/rules/unified-signatures.js new file mode 100644 index 00000000000..c41011f959e --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/unified-signatures.js @@ -0,0 +1,317 @@ +/** + * @fileoverview Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. + * @author Armando Aguirre + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../../../lib/rules/unified-signatures"), + + RuleTester = require("eslint").RuleTester; + + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var ruleTester = new RuleTester({ parser: '@typescript-eslint/parser' }); + +ruleTester.run("unified-signatures", rule, { + valid: [ + ` +function g(): void; +function g(a: number, b: number): void; +function g(a?: number, b?: number): void {} + `, + ` +function rest(...xs: number[]): void; +function rest(xs: number[], y: string): void; +function rest(...args: any[]) {} +`, + ` +class C { + constructor(); + constructor(a: number, b: number); + constructor(a?: number, b?: number) {} + + a(): void; + a(a: number, b: number): void; + a(a?: number, b?: number): void {} +} +`, + // No error for arity difference greater than 1. + ` +interface I { + a2(): void; + a2(x: number, y: number): void; +} +`, + // No error for different return types. + ` +interface I { + a4(): void; + a4(x: number): number; +} +`, + // No error if one takes a type parameter and the other doesn't. + ` +interface I { + a5(x: T): T; + a5(x: number): number; +} +`, + // No error if one is a rest parameter and other isn't. + ` +interface I { + b2(x: string): void; + b2(...x: number[]): void; +} +`, + // No error if both are rest parameters. (https://github.com/Microsoft/TypeScript/issues/5077) + ` +interface I { + b3(...x: number[]): void; + b3(...x: string[]): void; +} +`, + // No error if one is optional and the other isn't. + ` +interface I { + c3(x: number): void; + c3(x?: string): void; +} +`, + // No error if they differ by 2 or more parameters. + ` +interface I { + d2(x: string, y: number): void; + d2(x: number, y: string): void; +} +`, + // No conflict between static/non-static members. + ` +declare class D { + static a(); + a(x: number); +} +`, + // Allow separate overloads if one is generic and the other isn't. + ` +interface Generic { + x(): void; + x(x: T[]): void; +} +`], + invalid: [{ + code: ` +function f(x: number): void; +function f(x: string): void; +function f(x: any): any { + return x; +} +`, + errors: [{ + message: "These overloads can be combined into one signature taking `number | string`.", + line: 3, + column: 12, + endLine: 3, + endColumn: 21 + }] + }, { + code: ` +function opt(xs?: number[]): void; +function opt(xs: number[], y: string): void; +function opt(...args: any[]) {} +`, + errors: [{ + message: "These overloads can be combined into one signature with an optional parameter.", + line: 3, + column: 28, + endLine: 3, + endColumn: 37 + }] + }, { + // For 3 or more overloads, mentions the line. + code: ` +interface I { + a0(): void; + a0(x: string): string; + a0(x: number): void; +} +`, + errors: [{ + message: "This overload and the one on line 3 can be combined into one signature with an optional parameter.", + line: 5, + column: 8, + endLine: 5, + endColumn: 17 + }] + }, { + // Error for extra parameter. + code: ` +interface I { + a1(): void; + a1(x: number): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature with an optional parameter.", + line: 4, + column: 8, + endLine: 4, + endColumn: 17 + }] + }, { + // Error for arity difference greater than 1 if the additional parameters are all optional/rest. + code: ` +interface I { + a3(): void; + a3(x: number, y?: number, ...z: number[]): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature with a rest parameter.", + line: 4, + column: 31, + endLine: 4, + endColumn: 45 + }] + }, { + // Error if only one defines a rest parameter. + code: ` +interface I { + b(): void; + b(...x: number[]): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature with a rest parameter.", + line: 4, + column: 7, + endLine: 4, + endColumn: 21 + }] + }, { + // Error if only one defines an optional parameter. + code: ` +interface I { + c(): void; + c(x?: number): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature with an optional parameter.", + line: 4, + column: 7, + endLine: 4, + endColumn: 17 + }] + }, { + // Error if both are optional. + code: ` +interface I { + c2(x?: number): void; + c2(x?: string): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature taking `number | string`.", + line: 4, + column: 8, + endLine: 4, + endColumn: 18 + }] + }, { + // Error for different types (could be a union) + code: ` +interface I { + d(x: string): void; + d(x: number): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature taking `string | number`.", + line: 4, + column: 7, + endLine: 4, + endColumn: 16 + }] + }, { + // Works for type literal and call signature too. + code: ` +type T = { + (): void; + (x: number): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature with an optional parameter.", + line: 4, + column: 6, + endLine: 4, + endColumn: 15 + }] + }, { + // Works for constructor. + code: ` +declare class C { + constructor(); + constructor(x: number); +} +`, + errors: [{ + message: "These overloads can be combined into one signature with an optional parameter.", + line: 4, + column: 17, + endLine: 4, + endColumn: 26 + }] + }, { + // Works with unions. + code: ` +interface I { + f(x: number); + f(x: string | boolean); +} +`, + errors: [{ + message: "These overloads can be combined into one signature taking `number | string | boolean`.", + line: 4, + column: 7, + endLine: 4, + endColumn: 26 + }] + }, { + // Works with tuples. + code: ` +interface I { + f(x: number); + f(x: [string, boolean]); +} +`, + errors: [{ + message: "These overloads can be combined into one signature taking `number | [string, boolean]`.", + line: 4, + column: 7, + endLine: 4, + endColumn: 27 + }] + }, { + code: ` +interface Generic { + y(x: T[]): void; + y(x: T): void; +} +`, + errors: [{ + message: "These overloads can be combined into one signature taking `T[] | T`.", + line: 4, + column: 7, + endLine: 4, + endColumn: 11 + }] + }] +}); From ce97e17ca06b8ccf02460529ef2b90d6c4d1b9ae Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 31 Jan 2019 17:48:49 -0800 Subject: [PATCH 02/13] Added unified-signatures documentation --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 28 +++++++++---------- .../docs/rules/unified-signatures.md | 5 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f0c11446353..8cf093f4e56 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -129,5 +129,6 @@ Install [`eslint-config-prettier`](https://github.com/prettier/eslint-config-pre | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules. (`no-internal-module` from TSLint) | :heavy_check_mark: | :wrench: | | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string. (`restrict-plus-operands` from TSLint) | | | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations (`typedef-whitespace` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one. (`unified-signatures` from TSLint) | | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 378a3140ffb..7e0186d065c 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,10 +1,10 @@ # Roadmap -✅ (28) = done -🌟 (79) = in ESLint core -🔌 (33) = in another plugin -🌓 (16) = implementations differ or ESLint version is missing functionality -🛑 (70) = unimplemented +✅ (29) = done +🌟 (79) = in ESLint core +🔌 (33) = in another plugin +🌓 (16) = implementations differ or ESLint version is missing functionality +🛑 (69) = unimplemented ## TSLint rules @@ -33,7 +33,7 @@ | [`promise-function-async`] | 🛑 | N/A ([relevant plugin][plugin:promise]) | | [`typedef`] | 🛑 | N/A | | [`typedef-whitespace`] | ✅ | [`@typescript-eslint/type-annotation-spacing`] | -| [`unified-signatures`] | 🛑 | N/A | +| [`unified-signatures`] | ✅ | [`@typescript-eslint/unified-signatures`] | ### Functionality @@ -96,7 +96,7 @@ | [`use-default-type-parameter`] | 🛑 | N/A | | [`use-isnan`] | 🌟 | [`use-isnan`][use-isnan] | -[1] The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`) +[1] The ESLint rule also supports silencing with an extra set of parens (`if ((foo = bar)) {}`) [2] Missing private class member support. [`@typescript-eslint/no-unused-vars`] adds support for some TS-specific features. ### Maintainability @@ -120,7 +120,7 @@ | [`prefer-readonly`] | 🛑 | N/A | | [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] | -[1] Only warns when importing deprecated symbols +[1] Only warns when importing deprecated symbols [2] Missing support for blank-line-delimited sections ### Style @@ -179,7 +179,7 @@ | [`variable-name`] | 🌟 | [2] | | [`whitespace`] | 🔌 | Use [Prettier] | -[1] Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]` +[1] Recommended config: `["error", { blankLine: "always", prev: "*", next: "return" }]` [2] [`camelcase`][camelcase], [`no-underscore-dangle`][no-underscore-dangle], [`id-blacklist`][id-blacklist], and/or [`id-match`][id-match] ## tslint-microsoft-contrib rules @@ -245,10 +245,10 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- | `use-named-parameter` | 🛑 | N/A | | `use-simple-attributes` | 🛑 | N/A | -[1] Enforces blank lines both at the beginning and end of a block -[2] Recommended config: `["error", "ForInStatement"]` -[3] Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]` -[4] Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]` +[1] Enforces blank lines both at the beginning and end of a block +[2] Recommended config: `["error", "ForInStatement"]` +[3] Recommended config: `["error", "declaration", { "allowArrowFunctions": true }]` +[4] Recommended config: `["error", { "terms": ["BUG", "HACK", "FIXME", "LATER", "LATER2", "TODO"], "location": "anywhere" }]` [5] Does not check class fields. [insecure-random]: https://github.com/desktop/desktop/blob/master/eslint-rules/insecure-random.js @@ -310,7 +310,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- | `react-a11y-titles` | 🛑 | N/A | | `react-anchor-blank-noopener` | 🛑 | N/A | -[1] TSLint rule is more strict +[1] TSLint rule is more strict [2] ESLint rule only reports for click handlers [prettier]: https://prettier.io diff --git a/packages/eslint-plugin/docs/rules/unified-signatures.md b/packages/eslint-plugin/docs/rules/unified-signatures.md index 63c77e3a30f..3780ed36ba9 100644 --- a/packages/eslint-plugin/docs/rules/unified-signatures.md +++ b/packages/eslint-plugin/docs/rules/unified-signatures.md @@ -1,11 +1,10 @@ # Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. (unified-signatures) -Please describe the origin of the rule here. - +Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. ## Rule Details -This rule aims to... +This rule aims to keep the source code as maintanable as posible by reducing the amount of overloads. Examples of **incorrect** code for this rule: From a4d6425fb6b1121fc56eab35d20116b025c7824d Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Fri, 1 Feb 2019 16:05:17 -0800 Subject: [PATCH 03/13] Fixes to unified-signatures rule --- packages/eslint-plugin/docs/rules/unified-signatures.md | 2 +- packages/eslint-plugin/lib/rules/unified-signatures.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unified-signatures.md b/packages/eslint-plugin/docs/rules/unified-signatures.md index 3780ed36ba9..b35dfd56b9b 100644 --- a/packages/eslint-plugin/docs/rules/unified-signatures.md +++ b/packages/eslint-plugin/docs/rules/unified-signatures.md @@ -20,7 +20,7 @@ f(...x: number[]): void; Examples of **correct** code for this rule: ```ts -function f(x: number | stting): void; +function f(x: number | string): void; ``` ```ts function f(x?: ...number[]): void; diff --git a/packages/eslint-plugin/lib/rules/unified-signatures.js b/packages/eslint-plugin/lib/rules/unified-signatures.js index 3e2af4fc99c..eb5cdc825ac 100644 --- a/packages/eslint-plugin/lib/rules/unified-signatures.js +++ b/packages/eslint-plugin/lib/rules/unified-signatures.js @@ -26,7 +26,6 @@ module.exports = { extraDescription: [util.tslintRule('unified-signatures')], url: util.metaDocsUrl(" unified-signature") }, - fixable: "code", type: "suggestion" }, From 97c33d9c4d586147cf52aee32b4f6b321185af73 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Fri, 1 Feb 2019 16:21:26 -0800 Subject: [PATCH 04/13] Fixed formatting on unified-signatures rule --- .../docs/rules/unified-signatures.md | 4 +- .../lib/rules/unified-signatures.js | 1048 +++++++++-------- packages/eslint-plugin/lib/util.js | 2 +- .../tests/lib/rules/unified-signatures.js | 390 +++--- 4 files changed, 772 insertions(+), 672 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unified-signatures.md b/packages/eslint-plugin/docs/rules/unified-signatures.md index b35dfd56b9b..929c82ae159 100644 --- a/packages/eslint-plugin/docs/rules/unified-signatures.md +++ b/packages/eslint-plugin/docs/rules/unified-signatures.md @@ -12,6 +12,7 @@ Examples of **incorrect** code for this rule: function f(x: number): void; function f(x: string): void; ``` + ```ts f(): void; f(...x: number[]): void; @@ -22,10 +23,11 @@ Examples of **correct** code for this rule: ```ts function f(x: number | string): void; ``` + ```ts function f(x?: ...number[]): void; ``` ## Related to - - TSLint: ['unified-signatures`](https://palantir.github.io/tslint/rules/unified-signatures/) \ No newline at end of file +- TSLint: ['unified-signatures`](https://palantir.github.io/tslint/rules/unified-signatures/) diff --git a/packages/eslint-plugin/lib/rules/unified-signatures.js b/packages/eslint-plugin/lib/rules/unified-signatures.js index eb5cdc825ac..d2f772558e4 100644 --- a/packages/eslint-plugin/lib/rules/unified-signatures.js +++ b/packages/eslint-plugin/lib/rules/unified-signatures.js @@ -1,13 +1,12 @@ -// @ts-check /** * @fileoverview Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. * @author Armando Aguirre */ -"use strict"; -const ts = require("typescript"); -const tsutils = require("tsutils"); -const util = require("../util"); -const { Syntax } = require("@typescript-eslint/parser") +'use strict'; +const ts = require('typescript'); +const tsutils = require('tsutils'); +const util = require('../util'); +const { Syntax } = require('@typescript-eslint/parser'); /** @typedef {import("estree").Node} Node */ /** @typedef {import("eslint").Rule.RuleContext} Context */ @@ -18,515 +17,558 @@ const { Syntax } = require("@typescript-eslint/parser") /** @type {import("eslint").Rule.RuleModule} */ module.exports = { - meta: { - docs: { - description: "Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter.", - category: "TypeScript-specific", - recommended: false, - extraDescription: [util.tslintRule('unified-signatures')], - url: util.metaDocsUrl(" unified-signature") - }, - type: "suggestion" + meta: { + docs: { + description: + 'Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter.', + category: 'TypeScript-specific', + recommended: false, + extraDescription: [util.tslintRule('unified-signatures')], + url: util.metaDocsUrl(' unified-signature') }, + type: 'suggestion' + }, + + create: function(context) { + const sourceCode = context.getSourceCode(); + + /** + * @typedef Failure + * @type {object} + * @property {Unify} unify + * @property {boolean} only2 + */ + /** + * @typedef Unify + * @type {{kind: 'single-parameter-difference', p0: any, p1: any} | {kind: 'extra-parameter', extraParameter: any, otherSignature: any}} + */ + /** + * Given a node, if it could potentially be an overload, return its signature and key. + * All signatures which are overloads should have equal keys. + * @template T + * @callback GetOverload + * @param {T} node + * @returns {{signature: any, key: string} | undefined} + */ + /** + * Returns true if typeName is the name of an *outer* type parameter. + * In: `interface I { m(x: U): T }`, only `T` is an outer type parameter. + * @callback IsTypeParameter + * @param {string} typeName + * @returns {boolean} + */ + /** + * @template T + * @template Out + * @callback ForEachPairCallback + * @param {T} a + * @param {T} b + * @returns {Out | undefined} + */ + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + /** + * @param {number} [otherLine] + */ + function FAILURE_STRING_OMITTING_SINGLE_PARAMETER(otherLine) { + return `${FAILURE_STRING_START(otherLine)} with an optional parameter.`; + } - create: function (context) { - - const sourceCode = context.getSourceCode(); - - /** - * @typedef Failure - * @type {object} - * @property {Unify} unify - * @property {boolean} only2 - */ - /** - * @typedef Unify - * @type {{kind: 'single-parameter-difference', p0: any, p1: any} | {kind: 'extra-parameter', extraParameter: any, otherSignature: any}} - */ - /** - * Given a node, if it could potentially be an overload, return its signature and key. - * All signatures which are overloads should have equal keys. - * @template T - * @callback GetOverload - * @param {T} node - * @returns {{signature: any, key: string} | undefined} - */ - /** - * Returns true if typeName is the name of an *outer* type parameter. - * In: `interface I { m(x: U): T }`, only `T` is an outer type parameter. - * @callback IsTypeParameter - * @param {string} typeName - * @returns {boolean} - */ - /** - * @template T - * @template Out - * @callback ForEachPairCallback - * @param {T} a - * @param {T} b - * @returns {Out | undefined} - */ - - //---------------------------------------------------------------------- - // Helpers - //---------------------------------------------------------------------- - - /** - * @param {number} [otherLine] - */ - function FAILURE_STRING_OMITTING_SINGLE_PARAMETER(otherLine) { - return `${FAILURE_STRING_START(otherLine)} with an optional parameter.`; - } - - /** - * @param {number} [otherLine] - */ - function FAILURE_STRING_OMITTING_REST_PARAMETER(otherLine) { - return `${FAILURE_STRING_START(otherLine)} with a rest parameter.`; - } + /** + * @param {number} [otherLine] + */ + function FAILURE_STRING_OMITTING_REST_PARAMETER(otherLine) { + return `${FAILURE_STRING_START(otherLine)} with a rest parameter.`; + } - /** - * @param {number | undefined} otherLine - * @param {string} type1 - * @param {string} type2 - */ - function FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE(otherLine, type1, type2) { - return `${FAILURE_STRING_START(otherLine)} taking \`${type1} | ${type2}\`.`; - } + /** + * @param {number | undefined} otherLine + * @param {string} type1 + * @param {string} type2 + */ + function FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( + otherLine, + type1, + type2 + ) { + return `${FAILURE_STRING_START( + otherLine + )} taking \`${type1} | ${type2}\`.`; + } - /** - * @param {number} [otherLine] - * @returns {string} - */ - function FAILURE_STRING_START(otherLine) { - // For only 2 overloads we don't need to specify which is the other one. - const overloads = - otherLine === undefined - ? "These overloads" - : `This overload and the one on line ${otherLine}`; - return `${overloads} can be combined into one signature`; - } + /** + * @param {number} [otherLine] + * @returns {string} + */ + function FAILURE_STRING_START(otherLine) { + // For only 2 overloads we don't need to specify which is the other one. + const overloads = + otherLine === undefined + ? 'These overloads' + : `This overload and the one on line ${otherLine}`; + return `${overloads} can be combined into one signature`; + } - /** - * @param {Array} statements - * @returns {void} - */ - function checkStatements(statements) { - addFailures( - checkOverloads(statements, undefined, statement => { - if (statement.type === Syntax.TSDeclareFunction) { - const { body, id } = statement; - return body === undefined && id !== undefined - ? { signature: statement, key: id.name } - : undefined; - } else { - return undefined; - } - }) - ); - } + /** + * @param {Array} statements + * @returns {void} + */ + function checkStatements(statements) { + addFailures( + checkOverloads(statements, undefined, statement => { + if (statement.type === Syntax.TSDeclareFunction) { + const { body, id } = statement; + return body === undefined && id !== undefined + ? { signature: statement, key: id.name } + : undefined; + } else { + return undefined; + } + }) + ); + } - /** - * @param {Array} members - * @param {Array} [typeParameters] - * @returns {void} - */ - function checkMembers(members, typeParameters) { - addFailures( - checkOverloads(members, typeParameters, member => { - switch (member.type) { - case Syntax.TSCallSignatureDeclaration: - case Syntax.TSConstructSignatureDeclaration: - case Syntax.TSMethodSignature: - break; - case Syntax.TSAbstractMethodDefinition: - case Syntax.MethodDefinition: - if (member.value.body !== null) { - return undefined; - } - break; - default: - return undefined; - - } - - const key = getOverloadKey(member); - return key === undefined ? undefined : { signature: member, key }; - }) - ); - } + /** + * @param {Array} members + * @param {Array} [typeParameters] + * @returns {void} + */ + function checkMembers(members, typeParameters) { + addFailures( + checkOverloads(members, typeParameters, member => { + switch (member.type) { + case Syntax.TSCallSignatureDeclaration: + case Syntax.TSConstructSignatureDeclaration: + case Syntax.TSMethodSignature: + break; + case Syntax.TSAbstractMethodDefinition: + case Syntax.MethodDefinition: + if (member.value.body !== null) { + return undefined; + } + break; + default: + return undefined; + } + + const key = getOverloadKey(member); + return key === undefined ? undefined : { signature: member, key }; + }) + ); + } - /** - * @param {Failure[]} failures - */ - function addFailures(failures) { - for (const failure of failures) { - const { unify, only2 } = failure; - switch (unify.kind) { - case "single-parameter-difference": { - const { p0, p1 } = unify; - const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line - context.report({ - loc: p1.loc, - message: FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( - lineOfOtherOverload, - sourceCode.getText(p0.typeAnnotation.typeAnnotation), - sourceCode.getText(p1.typeAnnotation.typeAnnotation) - ), - }); - break; - } - case "extra-parameter": { - const { extraParameter, otherSignature } = unify; - const lineOfOtherOverload = only2 ? undefined : otherSignature.loc.start.line; - - context.report({ - loc: extraParameter.loc, - message: extraParameter.type === Syntax.RestElement - ? FAILURE_STRING_OMITTING_REST_PARAMETER(lineOfOtherOverload) - : FAILURE_STRING_OMITTING_SINGLE_PARAMETER(lineOfOtherOverload) - }); - } - } - } + /** + * @param {Failure[]} failures + */ + function addFailures(failures) { + for (const failure of failures) { + const { unify, only2 } = failure; + switch (unify.kind) { + case 'single-parameter-difference': { + const { p0, p1 } = unify; + const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line; + context.report({ + loc: p1.loc, + message: FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( + lineOfOtherOverload, + sourceCode.getText(p0.typeAnnotation.typeAnnotation), + sourceCode.getText(p1.typeAnnotation.typeAnnotation) + ) + }); + break; + } + case 'extra-parameter': { + const { extraParameter, otherSignature } = unify; + const lineOfOtherOverload = only2 + ? undefined + : otherSignature.loc.start.line; + + context.report({ + loc: extraParameter.loc, + message: + extraParameter.type === Syntax.RestElement + ? FAILURE_STRING_OMITTING_REST_PARAMETER(lineOfOtherOverload) + : FAILURE_STRING_OMITTING_SINGLE_PARAMETER( + lineOfOtherOverload + ) + }); + } } + } + } - /** - * @template T - * @param {Array} signatures - * @param {Array | undefined} typeParameters - * @param {GetOverload} getOverload - * @return {Failure[]} - */ - function checkOverloads(signatures, typeParameters, getOverload) { - /** @type {Failure[]} */ - const result = []; - const isTypeParameter = getIsTypeParameter(typeParameters); - for (const overloads of collectOverloads(signatures, getOverload)) { - if (overloads.length === 2) { - - // Classes returns parameters on its value property - const signature0 = overloads[0].value || overloads[0]; - const signature1 = overloads[1].value || overloads[1]; - - const unify = compareSignatures(signature0, signature1, isTypeParameter); - if (unify !== undefined) { - result.push({ unify, only2: true }); - } - } else { - forEachPair(overloads, (a, b) => { - const unify = compareSignatures(a, b, isTypeParameter); - if (unify !== undefined) { - result.push({ unify, only2: false }); - } - }); - } + /** + * @template T + * @param {Array} signatures + * @param {Array | undefined} typeParameters + * @param {GetOverload} getOverload + * @return {Failure[]} + */ + function checkOverloads(signatures, typeParameters, getOverload) { + /** @type {Failure[]} */ + const result = []; + const isTypeParameter = getIsTypeParameter(typeParameters); + for (const overloads of collectOverloads(signatures, getOverload)) { + if (overloads.length === 2) { + // Classes returns parameters on its value property + const signature0 = overloads[0].value || overloads[0]; + const signature1 = overloads[1].value || overloads[1]; + + const unify = compareSignatures( + signature0, + signature1, + isTypeParameter + ); + if (unify !== undefined) { + result.push({ unify, only2: true }); + } + } else { + forEachPair(overloads, (a, b) => { + const unify = compareSignatures(a, b, isTypeParameter); + if (unify !== undefined) { + result.push({ unify, only2: false }); } - return result; + }); } + } + return result; + } - /** - * @param {any} a - * @param {any} b - * @param {IsTypeParameter} isTypeParameter - * @returns {Unify | undefined} - */ - function compareSignatures(a, b, isTypeParameter) { - if (!signaturesCanBeUnified(a, b, isTypeParameter)) { - return undefined; - } + /** + * @param {any} a + * @param {any} b + * @param {IsTypeParameter} isTypeParameter + * @returns {Unify | undefined} + */ + function compareSignatures(a, b, isTypeParameter) { + if (!signaturesCanBeUnified(a, b, isTypeParameter)) { + return undefined; + } - return a.params.length === b.params.length - ? signaturesDifferBySingleParameter(a.params, b.params) - : signaturesDifferByOptionalOrRestParameter(a, b); - } + return a.params.length === b.params.length + ? signaturesDifferBySingleParameter(a.params, b.params) + : signaturesDifferByOptionalOrRestParameter(a, b); + } - /** - * @param {any} a - * @param {any} b - * @param {IsTypeParameter} isTypeParameter - * @returns {boolean} - */ - function signaturesCanBeUnified(a, b, isTypeParameter) { - // Must return the same type. - - const aTypeParams = a.typeParameters !== undefined ? a.typeParameters.params : undefined; - const bTypeParams = b.typeParameters !== undefined ? b.typeParameters.params : undefined; - - return ( - typesAreEqual(a.returnType, b.returnType) && - // Must take the same type parameters. - // If one uses a type parameter (from outside) and the other doesn't, they shouldn't be joined. - util.arraysAreEqual(aTypeParams, bTypeParams, typeParametersAreEqual) && - signatureUsesTypeParameter(a, isTypeParameter) === signatureUsesTypeParameter(b, isTypeParameter) - ); - } + /** + * @param {any} a + * @param {any} b + * @param {IsTypeParameter} isTypeParameter + * @returns {boolean} + */ + function signaturesCanBeUnified(a, b, isTypeParameter) { + // Must return the same type. + + const aTypeParams = + a.typeParameters !== undefined ? a.typeParameters.params : undefined; + const bTypeParams = + b.typeParameters !== undefined ? b.typeParameters.params : undefined; + + return ( + typesAreEqual(a.returnType, b.returnType) && + // Must take the same type parameters. + // If one uses a type parameter (from outside) and the other doesn't, they shouldn't be joined. + util.arraysAreEqual(aTypeParams, bTypeParams, typeParametersAreEqual) && + signatureUsesTypeParameter(a, isTypeParameter) === + signatureUsesTypeParameter(b, isTypeParameter) + ); + } - /** - * Detect `a(x: number, y: number, z: number)` and `a(x: number, y: string, z: number)`. - * @param {Array} types1 - * @param {Array} types2 - * @returns {Unify | undefined} - */ - function signaturesDifferBySingleParameter(types1, types2) { - const index = getIndexOfFirstDifference(types1, types2, parametersAreEqual); - if (index === undefined) { - return undefined; - } + /** + * Detect `a(x: number, y: number, z: number)` and `a(x: number, y: string, z: number)`. + * @param {Array} types1 + * @param {Array} types2 + * @returns {Unify | undefined} + */ + function signaturesDifferBySingleParameter(types1, types2) { + const index = getIndexOfFirstDifference( + types1, + types2, + parametersAreEqual + ); + if (index === undefined) { + return undefined; + } + + // If remaining arrays are equal, the signatures differ by just one parameter type + if ( + !util.arraysAreEqual( + types1.slice(index + 1), + types2.slice(index + 1), + parametersAreEqual + ) + ) { + return undefined; + } + + const a = types1[index]; + const b = types2[index]; + // Can unify `a?: string` and `b?: number`. Can't unify `...args: string[]` and `...args: number[]`. + // See https://github.com/Microsoft/TypeScript/issues/5077 + return parametersHaveEqualSigils(a, b) && a.type !== Syntax.RestElement + ? { kind: 'single-parameter-difference', p0: a, p1: b } + : undefined; + } - // If remaining arrays are equal, the signatures differ by just one parameter type - if (!util.arraysAreEqual(types1.slice(index + 1), types2.slice(index + 1), parametersAreEqual)) { - return undefined; - } + /** + * Detect `a(): void` and `a(x: number): void`. + * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. + * @param {any} a + * @param {any} b + * @returns {Unify | undefined} + */ + function signaturesDifferByOptionalOrRestParameter(a, b) { + const sig1 = a.params; + const sig2 = b.params; + + const minLength = Math.min(sig1.length, sig2.length); + const longer = sig1.length < sig2.length ? sig2 : sig1; + const shorter = sig1.length < sig2.length ? sig1 : sig2; + const shorterSig = sig1.length < sig2.length ? a : b; + + // If one is has 2+ parameters more than the other, they must all be optional/rest. + // Differ by optional parameters: f() and f(x), f() and f(x, ?y, ...z) + // Not allowed: f() and f(x, y) + for (let i = minLength + 1; i < longer.length; i++) { + if (!parameterMayBeMissing(longer[i])) { + return undefined; + } + } - const a = types1[index]; - const b = types2[index]; - // Can unify `a?: string` and `b?: number`. Can't unify `...args: string[]` and `...args: number[]`. - // See https://github.com/Microsoft/TypeScript/issues/5077 - return parametersHaveEqualSigils(a, b) && a.type !== Syntax.RestElement - ? { kind: "single-parameter-difference", p0: a, p1: b } - : undefined; + for (let i = 0; i < minLength; i++) { + if (!typesAreEqual(sig1[i].typeAnnotation, sig2[i].typeAnnotation)) { + return undefined; } + } - /** - * Detect `a(): void` and `a(x: number): void`. - * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. - * @param {any} a - * @param {any} b - * @returns {Unify | undefined} - */ - function signaturesDifferByOptionalOrRestParameter(a, b) { - - const sig1 = a.params; - const sig2 = b.params; - - const minLength = Math.min(sig1.length, sig2.length); - const longer = sig1.length < sig2.length ? sig2 : sig1; - const shorter = sig1.length < sig2.length ? sig1 : sig2; - const shorterSig = sig1.length < sig2.length ? a : b; - - // If one is has 2+ parameters more than the other, they must all be optional/rest. - // Differ by optional parameters: f() and f(x), f() and f(x, ?y, ...z) - // Not allowed: f() and f(x, y) - for (let i = minLength + 1; i < longer.length; i++) { - if (!parameterMayBeMissing(longer[i])) { - return undefined; - } - } + if (minLength > 0 && shorter[minLength - 1].type === Syntax.RestElement) { + return undefined; + } - for (let i = 0; i < minLength; i++) { - if (!typesAreEqual(sig1[i].typeAnnotation, sig2[i].typeAnnotation)) { - return undefined; - } - } + return { + extraParameter: longer[longer.length - 1], + kind: 'extra-parameter', + otherSignature: shorterSig + }; + } - if (minLength > 0 && shorter[minLength - 1].type === Syntax.RestElement) { - return undefined; - } + /** + * Given type parameters, returns a function to test whether a type is one of those parameters. + * @param {any} [typeParameters] + * @returns {IsTypeParameter} + */ + function getIsTypeParameter(typeParameters) { + if (typeParameters === undefined) { + return () => false; + } + + /** @type {Set} */ + const set = new Set([]); + for (const t of typeParameters.params) { + set.add(t.name.name); + } + return typeName => set.has(typeName); + } - return { - extraParameter: longer[longer.length - 1], - kind: "extra-parameter", - otherSignature: shorterSig, - }; + /** + * True if any of the outer type parameters are used in a signature. + * @param {any} sig + * @param {IsTypeParameter} isTypeParameter + * @returns {boolean} + */ + function signatureUsesTypeParameter(sig, isTypeParameter) { + return sig.params.some( + p => typeContainsTypeParameter(p.typeAnnotation) === true + ); + + /** + * @param {any} type + * @returns {boolean | undefined} + */ + function typeContainsTypeParameter(type) { + if (!type) { + return false; } - /** - * Given type parameters, returns a function to test whether a type is one of those parameters. - * @param {any} [typeParameters] - * @returns {IsTypeParameter} - */ - function getIsTypeParameter(typeParameters) { - if (typeParameters === undefined) { - return () => false; - } - - /** @type {Set} */ - const set = new Set([]); - for (const t of typeParameters.params) { - set.add(t.name.name); - } - return (typeName) => set.has(typeName); + if (type.type === Syntax.TSTypeReference) { + const typeName = type.typeName; + if ( + typeName.type === Syntax.Identifier && + isTypeParameter(typeName.name) + ) { + return true; + } } - /** - * True if any of the outer type parameters are used in a signature. - * @param {any} sig - * @param {IsTypeParameter} isTypeParameter - * @returns {boolean} - */ - function signatureUsesTypeParameter(sig, isTypeParameter) { - return sig.params.some( - p => typeContainsTypeParameter(p.typeAnnotation) === true, - ); - - /** - * @param {any} type - * @returns {boolean | undefined} - */ - function typeContainsTypeParameter(type) { - if (!type) { - return false; - } - - if (type.type === Syntax.TSTypeReference) { - const typeName = type.typeName; - if (typeName.type === Syntax.Identifier && isTypeParameter(typeName.name)) { - return true; - } - } - - return typeContainsTypeParameter(type.typeAnnotation || type.elementType); - } - } + return typeContainsTypeParameter( + type.typeAnnotation || type.elementType + ); + } + } - /** - * Given all signatures, collects an array of arrays of signatures which are all overloads. - * Does not rely on overloads being adjacent. This is similar to code in adjacentOverloadSignaturesRule.ts, but not the same. - * @template T - * @param {Array} nodes - * @param {GetOverload} getOverload - * @returns {any[][]} - */ - function collectOverloads(nodes, getOverload) { - /** @type {Map} */ - const map = new Map(); - for (const sig of nodes) { - const overload = getOverload(sig); - if (overload === undefined) { - continue; - } - - const { signature, key } = overload; - const overloads = map.get(key); - if (overloads !== undefined) { - overloads.push(signature); - } else { - map.set(key, [signature]); - } - } - return Array.from(map.values()); + /** + * Given all signatures, collects an array of arrays of signatures which are all overloads. + * Does not rely on overloads being adjacent. This is similar to code in adjacentOverloadSignaturesRule.ts, but not the same. + * @template T + * @param {Array} nodes + * @param {GetOverload} getOverload + * @returns {any[][]} + */ + function collectOverloads(nodes, getOverload) { + /** @type {Map} */ + const map = new Map(); + for (const sig of nodes) { + const overload = getOverload(sig); + if (overload === undefined) { + continue; } - /** - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function parametersAreEqual(a, b) { - return parametersHaveEqualSigils(a, b) && typesAreEqual(a.typeAnnotation, b.typeAnnotation); + const { signature, key } = overload; + const overloads = map.get(key); + if (overloads !== undefined) { + overloads.push(signature); + } else { + map.set(key, [signature]); } + } + return Array.from(map.values()); + } - /** - * True for optional/rest parameters. - * @param {any} p - * @returns {boolean} - */ - function parameterMayBeMissing(p) { - return p.type === Syntax.RestElement || p.optional; - } + /** + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function parametersAreEqual(a, b) { + return ( + parametersHaveEqualSigils(a, b) && + typesAreEqual(a.typeAnnotation, b.typeAnnotation) + ); + } - /** - * False if one is optional and the other isn't, or one is a rest parameter and the other isn't. - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function parametersHaveEqualSigils(a, b) { - return ( - (a.type === Syntax.RestElement) === (b.type === Syntax.RestElement) && - (a.optional !== undefined) === (b.optional !== undefined) - ); - } + /** + * True for optional/rest parameters. + * @param {any} p + * @returns {boolean} + */ + function parameterMayBeMissing(p) { + return p.type === Syntax.RestElement || p.optional; + } - /** - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function typeParametersAreEqual(a, b) { - return a.name.name === b.name.name && constraintsAreEqual(a.constraint, b.constraint); - } + /** + * False if one is optional and the other isn't, or one is a rest parameter and the other isn't. + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function parametersHaveEqualSigils(a, b) { + return ( + (a.type === Syntax.RestElement) === (b.type === Syntax.RestElement) && + (a.optional !== undefined) === (b.optional !== undefined) + ); + } - /** - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function typesAreEqual(a, b) { - // TODO: Could traverse AST so that formatting differences don't affect this. - return a === b || (a !== undefined && b !== undefined && a.typeAnnotation.type === b.typeAnnotation.type); - } + /** + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function typeParametersAreEqual(a, b) { + return ( + a.name.name === b.name.name && + constraintsAreEqual(a.constraint, b.constraint) + ); + } - /** - * - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function constraintsAreEqual(a, b) { - return a === b || (a !== undefined && b !== undefined && a.type === b.type); - } + /** + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function typesAreEqual(a, b) { + // TODO: Could traverse AST so that formatting differences don't affect this. + return ( + a === b || + (a !== undefined && + b !== undefined && + a.typeAnnotation.type === b.typeAnnotation.type) + ); + } - /** - * Returns the first index where `a` and `b` differ. - * @template T - * @param {Array} a - * @param {Array} b - * @param {util.Equal} equal - * @returns {number | undefined} - */ - function getIndexOfFirstDifference(a, b, equal) { - for (let i = 0; i < a.length && i < b.length; i++) { - if (!equal(a[i], b[i])) { - return i; - } - } - return undefined; - } + /** + * + * @param {any} a + * @param {any} b + * @returns {boolean} + */ + function constraintsAreEqual(a, b) { + return ( + a === b || (a !== undefined && b !== undefined && a.type === b.type) + ); + } - /** - * Calls `action` for every pair of values in `values`. - * @template T - * @template Out - * @param {Array} values - * @param {ForEachPairCallback} action - * @returns {Out | undefined} - */ - function forEachPair(values, action) { - for (let i = 0; i < values.length; i++) { - for (let j = i + 1; j < values.length; j++) { - const result = action(values[i], values[j]); - if (result !== undefined) { - return result; - } - } - } - return undefined; + /** + * Returns the first index where `a` and `b` differ. + * @template T + * @param {Array} a + * @param {Array} b + * @param {util.Equal} equal + * @returns {number | undefined} + */ + function getIndexOfFirstDifference(a, b, equal) { + for (let i = 0; i < a.length && i < b.length; i++) { + if (!equal(a[i], b[i])) { + return i; } + } + return undefined; + } - //---------------------------------------------------------------------- - // Public - //---------------------------------------------------------------------- - - return { - Program(node) { - checkStatements(node.body) - }, - TSModuleBlock(node) { - checkStatements(node.body); - }, - TSInterfaceDeclaration(node) { - checkMembers(node.body.body, node.typeParameters); - }, - ClassDeclaration(node) { - checkMembers(node.body.body, node.typeParameters); - }, - TSTypeLiteral(node) { - checkMembers(node.members); - } - }; + /** + * Calls `action` for every pair of values in `values`. + * @template T + * @template Out + * @param {Array} values + * @param {ForEachPairCallback} action + * @returns {Out | undefined} + */ + function forEachPair(values, action) { + for (let i = 0; i < values.length; i++) { + for (let j = i + 1; j < values.length; j++) { + const result = action(values[i], values[j]); + if (result !== undefined) { + return result; + } + } + } + return undefined; } + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + Program(node) { + checkStatements(node.body); + }, + TSModuleBlock(node) { + checkStatements(node.body); + }, + TSInterfaceDeclaration(node) { + checkMembers(node.body.body, node.typeParameters); + }, + ClassDeclaration(node) { + checkMembers(node.body.body, node.typeParameters); + }, + TSTypeLiteral(node) { + checkMembers(node.members); + } + }; + } }; /** @@ -534,12 +576,12 @@ module.exports = { * @returns {string | undefined} */ function getOverloadKey(node) { - const info = getOverloadInfo(node); - if (info === undefined) { - return undefined; - } + const info = getOverloadInfo(node); + if (info === undefined) { + return undefined; + } - return (node.computed ? "0" : "1") + (node.static ? "0" : "1") + info; + return (node.computed ? '0' : '1') + (node.static ? '0' : '1') + info; } /** @@ -547,28 +589,28 @@ function getOverloadKey(node) { * @returns {string | { name: string; computed?: boolean } | undefined} */ function getOverloadInfo(node) { - switch (node.type) { - case Syntax.TSConstructSignatureDeclaration: - return "constructor"; - case Syntax.TSCallSignatureDeclaration: - return "()"; - default: { - const { key } = node; - if (key === undefined) { - return undefined; - } - - switch (key.type) { - case Syntax.Identifier: - return key.name; - case Syntax.Property: - const { value } = key; - return tsutils.isLiteralExpression(value) - ? value.name - : { name: value.getText(), computed: true }; - default: - return tsutils.isLiteralExpression(key) ? key.name : undefined; - } - } + switch (node.type) { + case Syntax.TSConstructSignatureDeclaration: + return 'constructor'; + case Syntax.TSCallSignatureDeclaration: + return '()'; + default: { + const { key } = node; + if (key === undefined) { + return undefined; + } + + switch (key.type) { + case Syntax.Identifier: + return key.name; + case Syntax.Property: + const { value } = key; + return tsutils.isLiteralExpression(value) + ? value.name + : { name: value.getText(), computed: true }; + default: + return tsutils.isLiteralExpression(key) ? key.name : undefined; + } } + } } diff --git a/packages/eslint-plugin/lib/util.js b/packages/eslint-plugin/lib/util.js index 858683c1c99..ad1632563e5 100644 --- a/packages/eslint-plugin/lib/util.js +++ b/packages/eslint-plugin/lib/util.js @@ -151,4 +151,4 @@ exports.arraysAreEqual = (a, b, eq) => { a.length === b.length && a.every((x, idx) => eq(x, b[idx]))) ); -} \ No newline at end of file +}; diff --git a/packages/eslint-plugin/tests/lib/rules/unified-signatures.js b/packages/eslint-plugin/tests/lib/rules/unified-signatures.js index c41011f959e..f0bf69c065d 100644 --- a/packages/eslint-plugin/tests/lib/rules/unified-signatures.js +++ b/packages/eslint-plugin/tests/lib/rules/unified-signatures.js @@ -2,16 +2,14 @@ * @fileoverview Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. * @author Armando Aguirre */ -"use strict"; +'use strict'; //------------------------------------------------------------------------------ // Requirements //------------------------------------------------------------------------------ -var rule = require("../../../lib/rules/unified-signatures"), - - RuleTester = require("eslint").RuleTester; - +var rule = require('../../../lib/rules/unified-signatures'), + RuleTester = require('eslint').RuleTester; //------------------------------------------------------------------------------ // Tests @@ -19,19 +17,19 @@ var rule = require("../../../lib/rules/unified-signatures"), var ruleTester = new RuleTester({ parser: '@typescript-eslint/parser' }); -ruleTester.run("unified-signatures", rule, { - valid: [ - ` +ruleTester.run('unified-signatures', rule, { + valid: [ + ` function g(): void; function g(a: number, b: number): void; function g(a?: number, b?: number): void {} `, - ` + ` function rest(...xs: number[]): void; function rest(xs: number[], y: string): void; function rest(...args: any[]) {} `, - ` + ` class C { constructor(); constructor(a: number, b: number); @@ -42,276 +40,334 @@ class C { a(a?: number, b?: number): void {} } `, - // No error for arity difference greater than 1. - ` + // No error for arity difference greater than 1. + ` interface I { a2(): void; a2(x: number, y: number): void; } `, - // No error for different return types. - ` + // No error for different return types. + ` interface I { a4(): void; a4(x: number): number; } `, - // No error if one takes a type parameter and the other doesn't. - ` + // No error if one takes a type parameter and the other doesn't. + ` interface I { a5(x: T): T; a5(x: number): number; } `, - // No error if one is a rest parameter and other isn't. - ` + // No error if one is a rest parameter and other isn't. + ` interface I { b2(x: string): void; b2(...x: number[]): void; } `, - // No error if both are rest parameters. (https://github.com/Microsoft/TypeScript/issues/5077) - ` + // No error if both are rest parameters. (https://github.com/Microsoft/TypeScript/issues/5077) + ` interface I { b3(...x: number[]): void; b3(...x: string[]): void; } `, - // No error if one is optional and the other isn't. - ` + // No error if one is optional and the other isn't. + ` interface I { c3(x: number): void; c3(x?: string): void; } `, - // No error if they differ by 2 or more parameters. - ` + // No error if they differ by 2 or more parameters. + ` interface I { d2(x: string, y: number): void; d2(x: number, y: string): void; } `, - // No conflict between static/non-static members. - ` + // No conflict between static/non-static members. + ` declare class D { static a(); a(x: number); } `, - // Allow separate overloads if one is generic and the other isn't. - ` + // Allow separate overloads if one is generic and the other isn't. + ` interface Generic { x(): void; x(x: T[]): void; } -`], - invalid: [{ - code: ` +` + ], + invalid: [ + { + code: ` function f(x: number): void; function f(x: string): void; function f(x: any): any { return x; } `, - errors: [{ - message: "These overloads can be combined into one signature taking `number | string`.", - line: 3, - column: 12, - endLine: 3, - endColumn: 21 - }] - }, { - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature taking `number | string`.', + line: 3, + column: 12, + endLine: 3, + endColumn: 21 + } + ] + }, + { + code: ` function opt(xs?: number[]): void; function opt(xs: number[], y: string): void; function opt(...args: any[]) {} `, - errors: [{ - message: "These overloads can be combined into one signature with an optional parameter.", - line: 3, - column: 28, - endLine: 3, - endColumn: 37 - }] - }, { - // For 3 or more overloads, mentions the line. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with an optional parameter.', + line: 3, + column: 28, + endLine: 3, + endColumn: 37 + } + ] + }, + { + // For 3 or more overloads, mentions the line. + code: ` interface I { a0(): void; a0(x: string): string; a0(x: number): void; } `, - errors: [{ - message: "This overload and the one on line 3 can be combined into one signature with an optional parameter.", - line: 5, - column: 8, - endLine: 5, - endColumn: 17 - }] - }, { - // Error for extra parameter. - code: ` + errors: [ + { + message: + 'This overload and the one on line 3 can be combined into one signature with an optional parameter.', + line: 5, + column: 8, + endLine: 5, + endColumn: 17 + } + ] + }, + { + // Error for extra parameter. + code: ` interface I { a1(): void; a1(x: number): void; } `, - errors: [{ - message: "These overloads can be combined into one signature with an optional parameter.", - line: 4, - column: 8, - endLine: 4, - endColumn: 17 - }] - }, { - // Error for arity difference greater than 1 if the additional parameters are all optional/rest. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with an optional parameter.', + line: 4, + column: 8, + endLine: 4, + endColumn: 17 + } + ] + }, + { + // Error for arity difference greater than 1 if the additional parameters are all optional/rest. + code: ` interface I { a3(): void; a3(x: number, y?: number, ...z: number[]): void; } `, - errors: [{ - message: "These overloads can be combined into one signature with a rest parameter.", - line: 4, - column: 31, - endLine: 4, - endColumn: 45 - }] - }, { - // Error if only one defines a rest parameter. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with a rest parameter.', + line: 4, + column: 31, + endLine: 4, + endColumn: 45 + } + ] + }, + { + // Error if only one defines a rest parameter. + code: ` interface I { b(): void; b(...x: number[]): void; } `, - errors: [{ - message: "These overloads can be combined into one signature with a rest parameter.", - line: 4, - column: 7, - endLine: 4, - endColumn: 21 - }] - }, { - // Error if only one defines an optional parameter. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with a rest parameter.', + line: 4, + column: 7, + endLine: 4, + endColumn: 21 + } + ] + }, + { + // Error if only one defines an optional parameter. + code: ` interface I { c(): void; c(x?: number): void; } `, - errors: [{ - message: "These overloads can be combined into one signature with an optional parameter.", - line: 4, - column: 7, - endLine: 4, - endColumn: 17 - }] - }, { - // Error if both are optional. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with an optional parameter.', + line: 4, + column: 7, + endLine: 4, + endColumn: 17 + } + ] + }, + { + // Error if both are optional. + code: ` interface I { c2(x?: number): void; c2(x?: string): void; } `, - errors: [{ - message: "These overloads can be combined into one signature taking `number | string`.", - line: 4, - column: 8, - endLine: 4, - endColumn: 18 - }] - }, { - // Error for different types (could be a union) - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature taking `number | string`.', + line: 4, + column: 8, + endLine: 4, + endColumn: 18 + } + ] + }, + { + // Error for different types (could be a union) + code: ` interface I { d(x: string): void; d(x: number): void; } `, - errors: [{ - message: "These overloads can be combined into one signature taking `string | number`.", - line: 4, - column: 7, - endLine: 4, - endColumn: 16 - }] - }, { - // Works for type literal and call signature too. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature taking `string | number`.', + line: 4, + column: 7, + endLine: 4, + endColumn: 16 + } + ] + }, + { + // Works for type literal and call signature too. + code: ` type T = { (): void; (x: number): void; } `, - errors: [{ - message: "These overloads can be combined into one signature with an optional parameter.", - line: 4, - column: 6, - endLine: 4, - endColumn: 15 - }] - }, { - // Works for constructor. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with an optional parameter.', + line: 4, + column: 6, + endLine: 4, + endColumn: 15 + } + ] + }, + { + // Works for constructor. + code: ` declare class C { constructor(); constructor(x: number); } `, - errors: [{ - message: "These overloads can be combined into one signature with an optional parameter.", - line: 4, - column: 17, - endLine: 4, - endColumn: 26 - }] - }, { - // Works with unions. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature with an optional parameter.', + line: 4, + column: 17, + endLine: 4, + endColumn: 26 + } + ] + }, + { + // Works with unions. + code: ` interface I { f(x: number); f(x: string | boolean); } `, - errors: [{ - message: "These overloads can be combined into one signature taking `number | string | boolean`.", - line: 4, - column: 7, - endLine: 4, - endColumn: 26 - }] - }, { - // Works with tuples. - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature taking `number | string | boolean`.', + line: 4, + column: 7, + endLine: 4, + endColumn: 26 + } + ] + }, + { + // Works with tuples. + code: ` interface I { f(x: number); f(x: [string, boolean]); } `, - errors: [{ - message: "These overloads can be combined into one signature taking `number | [string, boolean]`.", - line: 4, - column: 7, - endLine: 4, - endColumn: 27 - }] - }, { - code: ` + errors: [ + { + message: + 'These overloads can be combined into one signature taking `number | [string, boolean]`.', + line: 4, + column: 7, + endLine: 4, + endColumn: 27 + } + ] + }, + { + code: ` interface Generic { y(x: T[]): void; y(x: T): void; } `, - errors: [{ - message: "These overloads can be combined into one signature taking `T[] | T`.", - line: 4, - column: 7, - endLine: 4, - endColumn: 11 - }] - }] + errors: [ + { + message: + 'These overloads can be combined into one signature taking `T[] | T`.', + line: 4, + column: 7, + endLine: 4, + endColumn: 11 + } + ] + } + ] }); From 1f4c7fe9cb9edd56d35d7ba3705099fd53a0bb57 Mon Sep 17 00:00:00 2001 From: Armano Date: Sun, 10 Feb 2019 15:34:14 +0100 Subject: [PATCH 05/13] docs(eslint-plugin): fix merge conflict --- packages/eslint-plugin/ROADMAP.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index a810b3a19f0..1d2328a3ead 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -120,7 +120,6 @@ | [`prefer-readonly`] | 🛑 | N/A | | [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] | -[1] Only warns when importing deprecated symbols [1] Only warns when importing deprecated symbols
[2] Missing support for blank-line-delimited sections From aa24eda9cccbf7fcb346bfeff8634133c3a46bc4 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Mon, 11 Mar 2019 15:11:44 -0700 Subject: [PATCH 06/13] feat(unified-signature): migrated to ts, used messageId --- .../lib/rules/unified-signatures.js | 616 ------------------ .../src/rules/unified-signatures.ts | 494 ++++++++++++++ packages/eslint-plugin/src/util/misc.ts | 17 + .../unified-signatures.test.ts} | 216 +++--- packages/eslint-plugin/typings/ts-eslint.d.ts | 4 + 5 files changed, 631 insertions(+), 716 deletions(-) delete mode 100644 packages/eslint-plugin/lib/rules/unified-signatures.js create mode 100644 packages/eslint-plugin/src/rules/unified-signatures.ts rename packages/eslint-plugin/tests/{lib/rules/unified-signatures.js => rules/unified-signatures.test.ts} (59%) diff --git a/packages/eslint-plugin/lib/rules/unified-signatures.js b/packages/eslint-plugin/lib/rules/unified-signatures.js deleted file mode 100644 index d2f772558e4..00000000000 --- a/packages/eslint-plugin/lib/rules/unified-signatures.js +++ /dev/null @@ -1,616 +0,0 @@ -/** - * @fileoverview Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. - * @author Armando Aguirre - */ -'use strict'; -const ts = require('typescript'); -const tsutils = require('tsutils'); -const util = require('../util'); -const { Syntax } = require('@typescript-eslint/parser'); - -/** @typedef {import("estree").Node} Node */ -/** @typedef {import("eslint").Rule.RuleContext} Context */ - -//------------------------------------------------------------------------------ -// Rule Definition -//------------------------------------------------------------------------------ - -/** @type {import("eslint").Rule.RuleModule} */ -module.exports = { - meta: { - docs: { - description: - 'Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter.', - category: 'TypeScript-specific', - recommended: false, - extraDescription: [util.tslintRule('unified-signatures')], - url: util.metaDocsUrl(' unified-signature') - }, - type: 'suggestion' - }, - - create: function(context) { - const sourceCode = context.getSourceCode(); - - /** - * @typedef Failure - * @type {object} - * @property {Unify} unify - * @property {boolean} only2 - */ - /** - * @typedef Unify - * @type {{kind: 'single-parameter-difference', p0: any, p1: any} | {kind: 'extra-parameter', extraParameter: any, otherSignature: any}} - */ - /** - * Given a node, if it could potentially be an overload, return its signature and key. - * All signatures which are overloads should have equal keys. - * @template T - * @callback GetOverload - * @param {T} node - * @returns {{signature: any, key: string} | undefined} - */ - /** - * Returns true if typeName is the name of an *outer* type parameter. - * In: `interface I { m(x: U): T }`, only `T` is an outer type parameter. - * @callback IsTypeParameter - * @param {string} typeName - * @returns {boolean} - */ - /** - * @template T - * @template Out - * @callback ForEachPairCallback - * @param {T} a - * @param {T} b - * @returns {Out | undefined} - */ - - //---------------------------------------------------------------------- - // Helpers - //---------------------------------------------------------------------- - - /** - * @param {number} [otherLine] - */ - function FAILURE_STRING_OMITTING_SINGLE_PARAMETER(otherLine) { - return `${FAILURE_STRING_START(otherLine)} with an optional parameter.`; - } - - /** - * @param {number} [otherLine] - */ - function FAILURE_STRING_OMITTING_REST_PARAMETER(otherLine) { - return `${FAILURE_STRING_START(otherLine)} with a rest parameter.`; - } - - /** - * @param {number | undefined} otherLine - * @param {string} type1 - * @param {string} type2 - */ - function FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( - otherLine, - type1, - type2 - ) { - return `${FAILURE_STRING_START( - otherLine - )} taking \`${type1} | ${type2}\`.`; - } - - /** - * @param {number} [otherLine] - * @returns {string} - */ - function FAILURE_STRING_START(otherLine) { - // For only 2 overloads we don't need to specify which is the other one. - const overloads = - otherLine === undefined - ? 'These overloads' - : `This overload and the one on line ${otherLine}`; - return `${overloads} can be combined into one signature`; - } - - /** - * @param {Array} statements - * @returns {void} - */ - function checkStatements(statements) { - addFailures( - checkOverloads(statements, undefined, statement => { - if (statement.type === Syntax.TSDeclareFunction) { - const { body, id } = statement; - return body === undefined && id !== undefined - ? { signature: statement, key: id.name } - : undefined; - } else { - return undefined; - } - }) - ); - } - - /** - * @param {Array} members - * @param {Array} [typeParameters] - * @returns {void} - */ - function checkMembers(members, typeParameters) { - addFailures( - checkOverloads(members, typeParameters, member => { - switch (member.type) { - case Syntax.TSCallSignatureDeclaration: - case Syntax.TSConstructSignatureDeclaration: - case Syntax.TSMethodSignature: - break; - case Syntax.TSAbstractMethodDefinition: - case Syntax.MethodDefinition: - if (member.value.body !== null) { - return undefined; - } - break; - default: - return undefined; - } - - const key = getOverloadKey(member); - return key === undefined ? undefined : { signature: member, key }; - }) - ); - } - - /** - * @param {Failure[]} failures - */ - function addFailures(failures) { - for (const failure of failures) { - const { unify, only2 } = failure; - switch (unify.kind) { - case 'single-parameter-difference': { - const { p0, p1 } = unify; - const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line; - context.report({ - loc: p1.loc, - message: FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( - lineOfOtherOverload, - sourceCode.getText(p0.typeAnnotation.typeAnnotation), - sourceCode.getText(p1.typeAnnotation.typeAnnotation) - ) - }); - break; - } - case 'extra-parameter': { - const { extraParameter, otherSignature } = unify; - const lineOfOtherOverload = only2 - ? undefined - : otherSignature.loc.start.line; - - context.report({ - loc: extraParameter.loc, - message: - extraParameter.type === Syntax.RestElement - ? FAILURE_STRING_OMITTING_REST_PARAMETER(lineOfOtherOverload) - : FAILURE_STRING_OMITTING_SINGLE_PARAMETER( - lineOfOtherOverload - ) - }); - } - } - } - } - - /** - * @template T - * @param {Array} signatures - * @param {Array | undefined} typeParameters - * @param {GetOverload} getOverload - * @return {Failure[]} - */ - function checkOverloads(signatures, typeParameters, getOverload) { - /** @type {Failure[]} */ - const result = []; - const isTypeParameter = getIsTypeParameter(typeParameters); - for (const overloads of collectOverloads(signatures, getOverload)) { - if (overloads.length === 2) { - // Classes returns parameters on its value property - const signature0 = overloads[0].value || overloads[0]; - const signature1 = overloads[1].value || overloads[1]; - - const unify = compareSignatures( - signature0, - signature1, - isTypeParameter - ); - if (unify !== undefined) { - result.push({ unify, only2: true }); - } - } else { - forEachPair(overloads, (a, b) => { - const unify = compareSignatures(a, b, isTypeParameter); - if (unify !== undefined) { - result.push({ unify, only2: false }); - } - }); - } - } - return result; - } - - /** - * @param {any} a - * @param {any} b - * @param {IsTypeParameter} isTypeParameter - * @returns {Unify | undefined} - */ - function compareSignatures(a, b, isTypeParameter) { - if (!signaturesCanBeUnified(a, b, isTypeParameter)) { - return undefined; - } - - return a.params.length === b.params.length - ? signaturesDifferBySingleParameter(a.params, b.params) - : signaturesDifferByOptionalOrRestParameter(a, b); - } - - /** - * @param {any} a - * @param {any} b - * @param {IsTypeParameter} isTypeParameter - * @returns {boolean} - */ - function signaturesCanBeUnified(a, b, isTypeParameter) { - // Must return the same type. - - const aTypeParams = - a.typeParameters !== undefined ? a.typeParameters.params : undefined; - const bTypeParams = - b.typeParameters !== undefined ? b.typeParameters.params : undefined; - - return ( - typesAreEqual(a.returnType, b.returnType) && - // Must take the same type parameters. - // If one uses a type parameter (from outside) and the other doesn't, they shouldn't be joined. - util.arraysAreEqual(aTypeParams, bTypeParams, typeParametersAreEqual) && - signatureUsesTypeParameter(a, isTypeParameter) === - signatureUsesTypeParameter(b, isTypeParameter) - ); - } - - /** - * Detect `a(x: number, y: number, z: number)` and `a(x: number, y: string, z: number)`. - * @param {Array} types1 - * @param {Array} types2 - * @returns {Unify | undefined} - */ - function signaturesDifferBySingleParameter(types1, types2) { - const index = getIndexOfFirstDifference( - types1, - types2, - parametersAreEqual - ); - if (index === undefined) { - return undefined; - } - - // If remaining arrays are equal, the signatures differ by just one parameter type - if ( - !util.arraysAreEqual( - types1.slice(index + 1), - types2.slice(index + 1), - parametersAreEqual - ) - ) { - return undefined; - } - - const a = types1[index]; - const b = types2[index]; - // Can unify `a?: string` and `b?: number`. Can't unify `...args: string[]` and `...args: number[]`. - // See https://github.com/Microsoft/TypeScript/issues/5077 - return parametersHaveEqualSigils(a, b) && a.type !== Syntax.RestElement - ? { kind: 'single-parameter-difference', p0: a, p1: b } - : undefined; - } - - /** - * Detect `a(): void` and `a(x: number): void`. - * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. - * @param {any} a - * @param {any} b - * @returns {Unify | undefined} - */ - function signaturesDifferByOptionalOrRestParameter(a, b) { - const sig1 = a.params; - const sig2 = b.params; - - const minLength = Math.min(sig1.length, sig2.length); - const longer = sig1.length < sig2.length ? sig2 : sig1; - const shorter = sig1.length < sig2.length ? sig1 : sig2; - const shorterSig = sig1.length < sig2.length ? a : b; - - // If one is has 2+ parameters more than the other, they must all be optional/rest. - // Differ by optional parameters: f() and f(x), f() and f(x, ?y, ...z) - // Not allowed: f() and f(x, y) - for (let i = minLength + 1; i < longer.length; i++) { - if (!parameterMayBeMissing(longer[i])) { - return undefined; - } - } - - for (let i = 0; i < minLength; i++) { - if (!typesAreEqual(sig1[i].typeAnnotation, sig2[i].typeAnnotation)) { - return undefined; - } - } - - if (minLength > 0 && shorter[minLength - 1].type === Syntax.RestElement) { - return undefined; - } - - return { - extraParameter: longer[longer.length - 1], - kind: 'extra-parameter', - otherSignature: shorterSig - }; - } - - /** - * Given type parameters, returns a function to test whether a type is one of those parameters. - * @param {any} [typeParameters] - * @returns {IsTypeParameter} - */ - function getIsTypeParameter(typeParameters) { - if (typeParameters === undefined) { - return () => false; - } - - /** @type {Set} */ - const set = new Set([]); - for (const t of typeParameters.params) { - set.add(t.name.name); - } - return typeName => set.has(typeName); - } - - /** - * True if any of the outer type parameters are used in a signature. - * @param {any} sig - * @param {IsTypeParameter} isTypeParameter - * @returns {boolean} - */ - function signatureUsesTypeParameter(sig, isTypeParameter) { - return sig.params.some( - p => typeContainsTypeParameter(p.typeAnnotation) === true - ); - - /** - * @param {any} type - * @returns {boolean | undefined} - */ - function typeContainsTypeParameter(type) { - if (!type) { - return false; - } - - if (type.type === Syntax.TSTypeReference) { - const typeName = type.typeName; - if ( - typeName.type === Syntax.Identifier && - isTypeParameter(typeName.name) - ) { - return true; - } - } - - return typeContainsTypeParameter( - type.typeAnnotation || type.elementType - ); - } - } - - /** - * Given all signatures, collects an array of arrays of signatures which are all overloads. - * Does not rely on overloads being adjacent. This is similar to code in adjacentOverloadSignaturesRule.ts, but not the same. - * @template T - * @param {Array} nodes - * @param {GetOverload} getOverload - * @returns {any[][]} - */ - function collectOverloads(nodes, getOverload) { - /** @type {Map} */ - const map = new Map(); - for (const sig of nodes) { - const overload = getOverload(sig); - if (overload === undefined) { - continue; - } - - const { signature, key } = overload; - const overloads = map.get(key); - if (overloads !== undefined) { - overloads.push(signature); - } else { - map.set(key, [signature]); - } - } - return Array.from(map.values()); - } - - /** - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function parametersAreEqual(a, b) { - return ( - parametersHaveEqualSigils(a, b) && - typesAreEqual(a.typeAnnotation, b.typeAnnotation) - ); - } - - /** - * True for optional/rest parameters. - * @param {any} p - * @returns {boolean} - */ - function parameterMayBeMissing(p) { - return p.type === Syntax.RestElement || p.optional; - } - - /** - * False if one is optional and the other isn't, or one is a rest parameter and the other isn't. - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function parametersHaveEqualSigils(a, b) { - return ( - (a.type === Syntax.RestElement) === (b.type === Syntax.RestElement) && - (a.optional !== undefined) === (b.optional !== undefined) - ); - } - - /** - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function typeParametersAreEqual(a, b) { - return ( - a.name.name === b.name.name && - constraintsAreEqual(a.constraint, b.constraint) - ); - } - - /** - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function typesAreEqual(a, b) { - // TODO: Could traverse AST so that formatting differences don't affect this. - return ( - a === b || - (a !== undefined && - b !== undefined && - a.typeAnnotation.type === b.typeAnnotation.type) - ); - } - - /** - * - * @param {any} a - * @param {any} b - * @returns {boolean} - */ - function constraintsAreEqual(a, b) { - return ( - a === b || (a !== undefined && b !== undefined && a.type === b.type) - ); - } - - /** - * Returns the first index where `a` and `b` differ. - * @template T - * @param {Array} a - * @param {Array} b - * @param {util.Equal} equal - * @returns {number | undefined} - */ - function getIndexOfFirstDifference(a, b, equal) { - for (let i = 0; i < a.length && i < b.length; i++) { - if (!equal(a[i], b[i])) { - return i; - } - } - return undefined; - } - - /** - * Calls `action` for every pair of values in `values`. - * @template T - * @template Out - * @param {Array} values - * @param {ForEachPairCallback} action - * @returns {Out | undefined} - */ - function forEachPair(values, action) { - for (let i = 0; i < values.length; i++) { - for (let j = i + 1; j < values.length; j++) { - const result = action(values[i], values[j]); - if (result !== undefined) { - return result; - } - } - } - return undefined; - } - - //---------------------------------------------------------------------- - // Public - //---------------------------------------------------------------------- - - return { - Program(node) { - checkStatements(node.body); - }, - TSModuleBlock(node) { - checkStatements(node.body); - }, - TSInterfaceDeclaration(node) { - checkMembers(node.body.body, node.typeParameters); - }, - ClassDeclaration(node) { - checkMembers(node.body.body, node.typeParameters); - }, - TSTypeLiteral(node) { - checkMembers(node.members); - } - }; - } -}; - -/** - * @param {any} node - * @returns {string | undefined} - */ -function getOverloadKey(node) { - const info = getOverloadInfo(node); - if (info === undefined) { - return undefined; - } - - return (node.computed ? '0' : '1') + (node.static ? '0' : '1') + info; -} - -/** - * @param {any} node - * @returns {string | { name: string; computed?: boolean } | undefined} - */ -function getOverloadInfo(node) { - switch (node.type) { - case Syntax.TSConstructSignatureDeclaration: - return 'constructor'; - case Syntax.TSCallSignatureDeclaration: - return '()'; - default: { - const { key } = node; - if (key === undefined) { - return undefined; - } - - switch (key.type) { - case Syntax.Identifier: - return key.name; - case Syntax.Property: - const { value } = key; - return tsutils.isLiteralExpression(value) - ? value.name - : { name: value.getText(), computed: true }; - default: - return tsutils.isLiteralExpression(key) ? key.name : undefined; - } - } - } -} diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts new file mode 100644 index 00000000000..6faf2a38673 --- /dev/null +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -0,0 +1,494 @@ +import * as tsutils from 'tsutils'; +import * as util from '../util'; +import { Syntax } from '@typescript-eslint/parser'; + +interface Failure { + unify: Unify; + only2: boolean; +} +type Unify = + | { + kind: 'single-parameter-difference'; + p0: any; + p1: any; + } + | { + kind: 'extra-parameter'; + extraParameter: any; + otherSignature: any; + }; + +/** + * Returns true if typeName is the name of an *outer* type parameter. + * In: `interface I { m(x: U): T }`, only `T` is an outer type parameter. + */ +type IsTypeParameter = (typeName: string) => boolean; + +export default util.createRule({ + name: 'unified-signatures', + meta: { + docs: { + description: + 'Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter.', + category: 'Variables', + recommended: false, + tslintName: 'unified-signatures', + }, + type: 'suggestion', + messages: { + omittingRestParameter: '{{failureStringStart}} with a rest parameter.', + omittingSingleParameter: + '{{failureStringStart}} with an optional parameter.', + singleParameterDifference: + '{{failureStringStart}} taking `{{type1}} | {{type2}}`.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const sourceCode = context.getSourceCode(); + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + function failureStringStart(otherLine?: number): string { + // For only 2 overloads we don't need to specify which is the other one. + const overloads = + otherLine === undefined + ? 'These overloads' + : `This overload and the one on line ${otherLine}`; + return `${overloads} can be combined into one signature`; + } + + function addFailures(failures: Failure[]): void { + for (const failure of failures) { + const { unify, only2 } = failure; + switch (unify.kind) { + case 'single-parameter-difference': { + const { p0, p1 } = unify; + const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line; + context.report({ + loc: p1.loc, + messageId: 'singleParameterDifference', + data: { + failureStringStart: failureStringStart(lineOfOtherOverload), + type1: sourceCode.getText(p0.typeAnnotation.typeAnnotation), + type2: sourceCode.getText(p1.typeAnnotation.typeAnnotation), + }, + node: p1, + }); + break; + } + case 'extra-parameter': { + const { extraParameter, otherSignature } = unify; + const lineOfOtherOverload = only2 + ? undefined + : otherSignature.loc.start.line; + + context.report({ + loc: extraParameter.loc, + messageId: + extraParameter.type === Syntax.RestElement + ? 'omittingRestParameter' + : 'omittingSingleParameter', + data: { + failureStringStart: failureStringStart(lineOfOtherOverload), + }, + node: extraParameter, + }); + } + } + } + } + + function checkOverloads( + signatures: ReadonlyArray, + typeParameters: ReadonlyArray | undefined, + ): Failure[] { + const result: Failure[] = []; + const isTypeParameter = getIsTypeParameter(typeParameters); + for (const overloads of signatures) { + if (overloads.length === 2) { + // Classes returns parameters on its value property + const signature0 = overloads[0].value || overloads[0]; + const signature1 = overloads[1].value || overloads[1]; + + const unify = compareSignatures( + signature0, + signature1, + isTypeParameter, + ); + if (unify !== undefined) { + result.push({ unify, only2: true }); + } + } else { + forEachPair(overloads, (a, b) => { + const unify = compareSignatures(a, b, isTypeParameter); + if (unify !== undefined) { + result.push({ unify, only2: false }); + } + }); + } + } + return result; + } + + function compareSignatures( + a: any, + b: any, + isTypeParameter: IsTypeParameter, + ): Unify | undefined { + if (!signaturesCanBeUnified(a, b, isTypeParameter)) { + return undefined; + } + + return a.params.length === b.params.length + ? signaturesDifferBySingleParameter(a.params, b.params) + : signaturesDifferByOptionalOrRestParameter(a, b); + } + + function signaturesCanBeUnified( + a: any, + b: any, + isTypeParameter: IsTypeParameter, + ): boolean { + // Must return the same type. + + const aTypeParams = + a.typeParameters !== undefined ? a.typeParameters.params : undefined; + const bTypeParams = + b.typeParameters !== undefined ? b.typeParameters.params : undefined; + + return ( + typesAreEqual(a.returnType, b.returnType) && + // Must take the same type parameters. + // If one uses a type parameter (from outside) and the other doesn't, they shouldn't be joined. + util.arraysAreEqual(aTypeParams, bTypeParams, typeParametersAreEqual) && + signatureUsesTypeParameter(a, isTypeParameter) === + signatureUsesTypeParameter(b, isTypeParameter) + ); + } + + /** Detect `a(x: number, y: number, z: number)` and `a(x: number, y: string, z: number)`. */ + function signaturesDifferBySingleParameter( + types1: ReadonlyArray, + types2: ReadonlyArray, + ): Unify | undefined { + const index = getIndexOfFirstDifference( + types1, + types2, + parametersAreEqual, + ); + if (index === undefined) { + return undefined; + } + + // If remaining arrays are equal, the signatures differ by just one parameter type + if ( + !util.arraysAreEqual( + types1.slice(index + 1), + types2.slice(index + 1), + parametersAreEqual, + ) + ) { + return undefined; + } + + const a = types1[index]; + const b = types2[index]; + // Can unify `a?: string` and `b?: number`. Can't unify `...args: string[]` and `...args: number[]`. + // See https://github.com/Microsoft/TypeScript/issues/5077 + return parametersHaveEqualSigils(a, b) && a.type !== Syntax.RestElement + ? { kind: 'single-parameter-difference', p0: a, p1: b } + : undefined; + } + + /** + * Detect `a(): void` and `a(x: number): void`. + * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. + */ + function signaturesDifferByOptionalOrRestParameter( + a: any, + b: any, + ): Unify | undefined { + const sig1 = a.params; + const sig2 = b.params; + + const minLength = Math.min(sig1.length, sig2.length); + const longer = sig1.length < sig2.length ? sig2 : sig1; + const shorter = sig1.length < sig2.length ? sig1 : sig2; + const shorterSig = sig1.length < sig2.length ? a : b; + + // If one is has 2+ parameters more than the other, they must all be optional/rest. + // Differ by optional parameters: f() and f(x), f() and f(x, ?y, ...z) + // Not allowed: f() and f(x, y) + for (let i = minLength + 1; i < longer.length; i++) { + if (!parameterMayBeMissing(longer[i])) { + return undefined; + } + } + + for (let i = 0; i < minLength; i++) { + if (!typesAreEqual(sig1[i].typeAnnotation, sig2[i].typeAnnotation)) { + return undefined; + } + } + + if (minLength > 0 && shorter[minLength - 1].type === Syntax.RestElement) { + return undefined; + } + + return { + extraParameter: longer[longer.length - 1], + kind: 'extra-parameter', + otherSignature: shorterSig, + }; + } + + /** Given type parameters, returns a function to test whether a type is one of those parameters. */ + function getIsTypeParameter(typeParameters: any): IsTypeParameter { + if (typeParameters === undefined) { + return () => false; + } + + const set = new Set(); + for (const t of typeParameters.params) { + set.add(t.name.name); + } + return typeName => set.has(typeName); + } + + /** True if any of the outer type parameters are used in a signature. */ + function signatureUsesTypeParameter( + sig: any, + isTypeParameter: IsTypeParameter, + ): boolean { + return sig.params.some( + (p: any) => typeContainsTypeParameter(p.typeAnnotation) === true, + ); + + function typeContainsTypeParameter(type: any): boolean | undefined { + if (!type) { + return false; + } + + if (type.type === Syntax.TSTypeReference) { + const typeName = type.typeName; + if ( + typeName.type === Syntax.Identifier && + isTypeParameter(typeName.name) + ) { + return true; + } + } + + return typeContainsTypeParameter( + type.typeAnnotation || type.elementType, + ); + } + } + + function parametersAreEqual(a: any, b: any): boolean { + return ( + parametersHaveEqualSigils(a, b) && + typesAreEqual(a.typeAnnotation, b.typeAnnotation) + ); + } + + /** True for optional/rest parameters. */ + function parameterMayBeMissing(p: any): boolean { + return p.type === Syntax.RestElement || p.optional; + } + + /** False if one is optional and the other isn't, or one is a rest parameter and the other isn't. */ + function parametersHaveEqualSigils(a: any, b: any): boolean { + return ( + (a.type === Syntax.RestElement) === (b.type === Syntax.RestElement) && + (a.optional !== undefined) === (b.optional !== undefined) + ); + } + + function typeParametersAreEqual(a: any, b: any): boolean { + return ( + a.name.name === b.name.name && + constraintsAreEqual(a.constraint, b.constraint) + ); + } + + function typesAreEqual(a: any, b: any): boolean { + return ( + a === b || + (a !== undefined && + b !== undefined && + a.typeAnnotation.type === b.typeAnnotation.type) + ); + } + + function constraintsAreEqual(a: any, b: any): boolean { + return ( + a === b || (a !== undefined && b !== undefined && a.type === b.type) + ); + } + + /* Returns the first index where `a` and `b` differ. */ + function getIndexOfFirstDifference( + a: ReadonlyArray, + b: ReadonlyArray, + equal: util.Equal, + ): number | undefined { + for (let i = 0; i < a.length && i < b.length; i++) { + if (!equal(a[i], b[i])) { + return i; + } + } + return undefined; + } + + /** Calls `action` for every pair of values in `values`. */ + function forEachPair( + values: ReadonlyArray, + action: (a: T, b: T) => Out | undefined, + ): Out | undefined { + for (let i = 0; i < values.length; i++) { + for (let j = i + 1; j < values.length; j++) { + const result = action(values[i], values[j]); + if (result !== undefined) { + return result; + } + } + } + return undefined; + } + + interface Scope { + overloads: Map; + parent: any; + typeParameters: ReadonlyArray | undefined; + } + + const scopes: Scope[] = []; + let currentScope: Scope | undefined = { + overloads: new Map(), + parent: undefined, + typeParameters: undefined, + }; + + function createScope(parent: any, typeParameters: any = undefined) { + currentScope && scopes.push(currentScope); + currentScope = { + overloads: new Map(), + parent, + typeParameters, + }; + } + + function checkScope() { + const failures = currentScope + ? checkOverloads( + Array.from(currentScope.overloads.values()), + currentScope.typeParameters, + ) + : []; + addFailures(failures); + currentScope = scopes.pop(); + } + + function addOverload(signature: any, key?: string) { + key = key || getOverloadKey(signature); + if (currentScope && signature.parent === currentScope.parent && key) { + const overloads = currentScope.overloads.get(key); + if (overloads !== undefined) { + overloads.push(signature); + } else { + currentScope.overloads.set(key, [signature]); + } + } + } + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + Program(node) { + createScope(node); + }, + TSModuleBlock(node) { + createScope(node); + }, + TSInterfaceDeclaration(node) { + createScope(node.body, node.typeParameters); + }, + ClassDeclaration(node) { + createScope(node.body, node.typeParameters); + }, + TSTypeLiteral(node) { + createScope(node); + }, + // collect overloads + TSDeclareFunction(node) { + if (node.id && !node.body) { + addOverload(node, node.id.name); + } + }, + TSCallSignatureDeclaration: addOverload, + TSConstructSignatureDeclaration: addOverload, + TSMethodSignature: addOverload, + TSAbstractMethodDefinition(node) { + if (!node.value.body) { + addOverload(node); + } + }, + MethodDefinition(node) { + if (!node.value.body) { + addOverload(node); + } + }, + // validate scopes + 'Program:exit': checkScope, + 'TSModuleBlock:exit': checkScope, + 'TSInterfaceDeclaration:exit': checkScope, + 'ClassDeclaration:exit': checkScope, + 'TSTypeLiteral:exit': checkScope, + }; + }, +}); + +function getOverloadKey(node: any): string | undefined { + const info = getOverloadInfo(node); + if (info === undefined) { + return undefined; + } + + return (node.computed ? '0' : '1') + (node.static ? '0' : '1') + info; +} + +function getOverloadInfo( + node: any, +): string | { name: string; computed?: boolean } | undefined { + switch (node.type) { + case Syntax.TSConstructSignatureDeclaration: + return 'constructor'; + case Syntax.TSCallSignatureDeclaration: + return '()'; + default: { + const { key } = node; + if (key === undefined) { + return undefined; + } + + switch (key.type) { + case Syntax.Identifier: + return key.name; + case Syntax.Property: + const { value } = key; + return tsutils.isLiteralExpression(value) + ? value.text + : { name: value.getText(), computed: true }; + default: + return tsutils.isLiteralExpression(key) ? value.text : undefined; + } + } + } +} diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index dbaaebc3f7a..2e0baddf643 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -63,3 +63,20 @@ export function getNameFromPropertyName( } return `${propertyName.value}`; } + +/** Return true if both parameters are equal. */ +export type Equal = (a: T, b: T) => boolean; + +export function arraysAreEqual( + a: T[] | undefined, + b: T[] | undefined, + eq: (a: T, b: T) => boolean, +): boolean { + return ( + a === b || + (a !== undefined && + b !== undefined && + a.length === b.length && + a.every((x, idx) => eq(x, b[idx]))) + ); +} diff --git a/packages/eslint-plugin/tests/lib/rules/unified-signatures.js b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts similarity index 59% rename from packages/eslint-plugin/tests/lib/rules/unified-signatures.js rename to packages/eslint-plugin/tests/rules/unified-signatures.test.ts index f0bf69c065d..e6b00cfbac2 100644 --- a/packages/eslint-plugin/tests/lib/rules/unified-signatures.js +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -1,15 +1,5 @@ -/** - * @fileoverview Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter. - * @author Armando Aguirre - */ -'use strict'; - -//------------------------------------------------------------------------------ -// Requirements -//------------------------------------------------------------------------------ - -var rule = require('../../../lib/rules/unified-signatures'), - RuleTester = require('eslint').RuleTester; +import rule from '../../src/rules/unified-signatures'; +import { RuleTester } from '../RuleTester'; //------------------------------------------------------------------------------ // Tests @@ -102,7 +92,7 @@ interface Generic { x(): void; x(x: T[]): void; } -` +`, ], invalid: [ { @@ -115,14 +105,17 @@ function f(x: any): any { `, errors: [ { - message: - 'These overloads can be combined into one signature taking `number | string`.', + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, line: 3, column: 12, - endLine: 3, - endColumn: 21 - } - ] + }, + ], }, { code: ` @@ -132,14 +125,15 @@ function opt(...args: any[]) {} `, errors: [ { - message: - 'These overloads can be combined into one signature with an optional parameter.', + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 3, column: 28, - endLine: 3, - endColumn: 37 - } - ] + }, + ], }, { // For 3 or more overloads, mentions the line. @@ -152,14 +146,15 @@ interface I { `, errors: [ { - message: - 'This overload and the one on line 3 can be combined into one signature with an optional parameter.', + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'This overload and the one on line 3 can be combined into one signature', + }, line: 5, column: 8, - endLine: 5, - endColumn: 17 - } - ] + }, + ], }, { // Error for extra parameter. @@ -171,14 +166,15 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature with an optional parameter.', + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 4, column: 8, - endLine: 4, - endColumn: 17 - } - ] + }, + ], }, { // Error for arity difference greater than 1 if the additional parameters are all optional/rest. @@ -190,14 +186,15 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature with a rest parameter.', + messageId: 'omittingRestParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 4, column: 31, - endLine: 4, - endColumn: 45 - } - ] + }, + ], }, { // Error if only one defines a rest parameter. @@ -209,14 +206,15 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature with a rest parameter.', + messageId: 'omittingRestParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 4, column: 7, - endLine: 4, - endColumn: 21 - } - ] + }, + ], }, { // Error if only one defines an optional parameter. @@ -228,14 +226,15 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature with an optional parameter.', + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 4, column: 7, - endLine: 4, - endColumn: 17 - } - ] + }, + ], }, { // Error if both are optional. @@ -247,33 +246,39 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature taking `number | string`.', + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, line: 4, column: 8, - endLine: 4, - endColumn: 18 - } - ] + }, + ], }, { // Error for different types (could be a union) code: ` interface I { - d(x: string): void; d(x: number): void; + d(x: string): void; } `, errors: [ { - message: - 'These overloads can be combined into one signature taking `string | number`.', + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, line: 4, column: 7, - endLine: 4, - endColumn: 16 - } - ] + }, + ], }, { // Works for type literal and call signature too. @@ -285,14 +290,15 @@ type T = { `, errors: [ { - message: - 'These overloads can be combined into one signature with an optional parameter.', + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 4, column: 6, - endLine: 4, - endColumn: 15 - } - ] + }, + ], }, { // Works for constructor. @@ -304,14 +310,15 @@ declare class C { `, errors: [ { - message: - 'These overloads can be combined into one signature with an optional parameter.', + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, line: 4, column: 17, - endLine: 4, - endColumn: 26 - } - ] + }, + ], }, { // Works with unions. @@ -323,14 +330,17 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature taking `number | string | boolean`.', + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string | boolean', + }, line: 4, column: 7, - endLine: 4, - endColumn: 26 - } - ] + }, + ], }, { // Works with tuples. @@ -342,14 +352,17 @@ interface I { `, errors: [ { - message: - 'These overloads can be combined into one signature taking `number | [string, boolean]`.', + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: '[string, boolean]', + }, line: 4, column: 7, - endLine: 4, - endColumn: 27 - } - ] + }, + ], }, { code: ` @@ -360,14 +373,17 @@ interface Generic { `, errors: [ { - message: - 'These overloads can be combined into one signature taking `T[] | T`.', + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'T[]', + type2: 'T', + }, line: 4, column: 7, - endLine: 4, - endColumn: 11 - } - ] - } - ] + }, + ], + }, + ], }); diff --git a/packages/eslint-plugin/typings/ts-eslint.d.ts b/packages/eslint-plugin/typings/ts-eslint.d.ts index 998c6958342..3bf34e2173a 100644 --- a/packages/eslint-plugin/typings/ts-eslint.d.ts +++ b/packages/eslint-plugin/typings/ts-eslint.d.ts @@ -473,6 +473,9 @@ declare module 'ts-eslint' { Token?: RuleFunction; TryStatement?: RuleFunction; TSAbstractKeyword?: RuleFunction; + TSAbstractMethodDefinition?: RuleFunction< + TSESTree.TSAbstractMethodDefinition + >; TSAnyKeyword?: RuleFunction; TSArrayType?: RuleFunction; TSAsExpression?: RuleFunction; @@ -481,6 +484,7 @@ declare module 'ts-eslint' { TSBooleanKeyword?: RuleFunction; TSConditionalType?: RuleFunction; TSDeclareKeyword?: RuleFunction; + TSDeclareFunction?: RuleFunction; TSEnumDeclaration?: RuleFunction; TSEnumMember?: RuleFunction; TSExportAssignment?: RuleFunction; From 312a182d29eefce75cc472e2537259aa5d32c4d9 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Mon, 11 Mar 2019 15:50:15 -0700 Subject: [PATCH 07/13] fix(eslint-plugin): fixed case block lint issue --- packages/eslint-plugin/src/rules/unified-signatures.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index 6faf2a38673..63c4d8f7167 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -478,11 +478,12 @@ function getOverloadInfo( return undefined; } + const { value } = key; + switch (key.type) { case Syntax.Identifier: return key.name; case Syntax.Property: - const { value } = key; return tsutils.isLiteralExpression(value) ? value.text : { name: value.getText(), computed: true }; From 2fd63d17fd406e206c8d782a0c3cdad49f32fc3b Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Wed, 13 Mar 2019 18:24:06 -0700 Subject: [PATCH 08/13] fix(unified-signatures): improved types --- .../src/rules/unified-signatures.ts | 239 +++++++++++++----- .../tests/rules/unified-signatures.test.ts | 7 + packages/eslint-plugin/typings/ts-eslint.d.ts | 6 + 3 files changed, 182 insertions(+), 70 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index 63c4d8f7167..5aa430b2095 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -1,21 +1,22 @@ import * as tsutils from 'tsutils'; import * as util from '../util'; -import { Syntax } from '@typescript-eslint/parser'; +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; interface Failure { unify: Unify; only2: boolean; } + type Unify = | { kind: 'single-parameter-difference'; - p0: any; - p1: any; + p0: TSESTree.Parameter; + p1: TSESTree.Parameter; } | { kind: 'extra-parameter'; - extraParameter: any; - otherSignature: any; + extraParameter: TSESTree.Parameter; + otherSignature: SignatureDefinition; }; /** @@ -24,6 +25,27 @@ type Unify = */ type IsTypeParameter = (typeName: string) => boolean; +type ScopeNode = + | TSESTree.Program + | TSESTree.TSModuleBlock + | TSESTree.TSInterfaceBody + | TSESTree.ClassBody + | TSESTree.TSTypeLiteral; + +type OverloadNode = MethodDefinition | SignatureDefinition; + +type SignatureDefinition = + | TSESTree.FunctionExpression + | TSESTree.TSCallSignatureDeclaration + | TSESTree.TSConstructSignatureDeclaration + | TSESTree.TSDeclareFunction + | TSESTree.TSEmptyBodyFunctionExpression + | TSESTree.TSMethodSignature; + +type MethodDefinition = + | TSESTree.MethodDefinition + | TSESTree.TSAbstractMethodDefinition; + export default util.createRule({ name: 'unified-signatures', meta: { @@ -68,13 +90,25 @@ export default util.createRule({ case 'single-parameter-difference': { const { p0, p1 } = unify; const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line; + + const typeAnnotation0 = !isTSParameterProperty(p0) + ? p0.typeAnnotation + : undefined; + const typeAnnotation1 = !isTSParameterProperty(p1) + ? p1.typeAnnotation + : undefined; + context.report({ loc: p1.loc, messageId: 'singleParameterDifference', data: { failureStringStart: failureStringStart(lineOfOtherOverload), - type1: sourceCode.getText(p0.typeAnnotation.typeAnnotation), - type2: sourceCode.getText(p1.typeAnnotation.typeAnnotation), + type1: sourceCode.getText( + typeAnnotation0 && typeAnnotation0.typeAnnotation, + ), + type2: sourceCode.getText( + typeAnnotation1 && typeAnnotation1.typeAnnotation, + ), }, node: p1, }); @@ -89,7 +123,7 @@ export default util.createRule({ context.report({ loc: extraParameter.loc, messageId: - extraParameter.type === Syntax.RestElement + extraParameter.type === AST_NODE_TYPES.RestElement ? 'omittingRestParameter' : 'omittingSingleParameter', data: { @@ -103,16 +137,17 @@ export default util.createRule({ } function checkOverloads( - signatures: ReadonlyArray, - typeParameters: ReadonlyArray | undefined, + signatures: ReadonlyArray, + typeParameters?: TSESTree.TSTypeParameterDeclaration, ): Failure[] { const result: Failure[] = []; const isTypeParameter = getIsTypeParameter(typeParameters); for (const overloads of signatures) { if (overloads.length === 2) { - // Classes returns parameters on its value property - const signature0 = overloads[0].value || overloads[0]; - const signature1 = overloads[1].value || overloads[1]; + const signature0 = + (overloads[0] as MethodDefinition).value || overloads[0]; + const signature1 = + (overloads[1] as MethodDefinition).value || overloads[1]; const unify = compareSignatures( signature0, @@ -124,7 +159,14 @@ export default util.createRule({ } } else { forEachPair(overloads, (a, b) => { - const unify = compareSignatures(a, b, isTypeParameter); + const signature0 = (a as MethodDefinition).value || a; + const signature1 = (b as MethodDefinition).value || b; + + const unify = compareSignatures( + signature0, + signature1, + isTypeParameter, + ); if (unify !== undefined) { result.push({ unify, only2: false }); } @@ -135,8 +177,8 @@ export default util.createRule({ } function compareSignatures( - a: any, - b: any, + a: SignatureDefinition, + b: SignatureDefinition, isTypeParameter: IsTypeParameter, ): Unify | undefined { if (!signaturesCanBeUnified(a, b, isTypeParameter)) { @@ -149,8 +191,8 @@ export default util.createRule({ } function signaturesCanBeUnified( - a: any, - b: any, + a: SignatureDefinition, + b: SignatureDefinition, isTypeParameter: IsTypeParameter, ): boolean { // Must return the same type. @@ -172,8 +214,8 @@ export default util.createRule({ /** Detect `a(x: number, y: number, z: number)` and `a(x: number, y: string, z: number)`. */ function signaturesDifferBySingleParameter( - types1: ReadonlyArray, - types2: ReadonlyArray, + types1: ReadonlyArray, + types2: ReadonlyArray, ): Unify | undefined { const index = getIndexOfFirstDifference( types1, @@ -199,7 +241,8 @@ export default util.createRule({ const b = types2[index]; // Can unify `a?: string` and `b?: number`. Can't unify `...args: string[]` and `...args: number[]`. // See https://github.com/Microsoft/TypeScript/issues/5077 - return parametersHaveEqualSigils(a, b) && a.type !== Syntax.RestElement + return parametersHaveEqualSigils(a, b) && + a.type !== AST_NODE_TYPES.RestElement ? { kind: 'single-parameter-difference', p0: a, p1: b } : undefined; } @@ -209,8 +252,8 @@ export default util.createRule({ * Returns the parameter declaration (`x: number` in this example) that should be optional/rest, and overload it's a part of. */ function signaturesDifferByOptionalOrRestParameter( - a: any, - b: any, + a: SignatureDefinition, + b: SignatureDefinition, ): Unify | undefined { const sig1 = a.params; const sig2 = b.params; @@ -230,12 +273,24 @@ export default util.createRule({ } for (let i = 0; i < minLength; i++) { - if (!typesAreEqual(sig1[i].typeAnnotation, sig2[i].typeAnnotation)) { + const sig1i = sig1[i]; + const sig2i = sig2[i]; + const typeAnnotation1 = !isTSParameterProperty(sig1i) + ? sig1i.typeAnnotation + : undefined; + const typeAnnotation2 = !isTSParameterProperty(sig2i) + ? sig2i.typeAnnotation + : undefined; + + if (!typesAreEqual(typeAnnotation1, typeAnnotation2)) { return undefined; } } - if (minLength > 0 && shorter[minLength - 1].type === Syntax.RestElement) { + if ( + minLength > 0 && + shorter[minLength - 1].type === AST_NODE_TYPES.RestElement + ) { return undefined; } @@ -247,7 +302,9 @@ export default util.createRule({ } /** Given type parameters, returns a function to test whether a type is one of those parameters. */ - function getIsTypeParameter(typeParameters: any): IsTypeParameter { + function getIsTypeParameter( + typeParameters?: TSESTree.TSTypeParameterDeclaration, + ): IsTypeParameter { if (typeParameters === undefined) { return () => false; } @@ -261,62 +318,102 @@ export default util.createRule({ /** True if any of the outer type parameters are used in a signature. */ function signatureUsesTypeParameter( - sig: any, + sig: SignatureDefinition, isTypeParameter: IsTypeParameter, ): boolean { return sig.params.some( - (p: any) => typeContainsTypeParameter(p.typeAnnotation) === true, + (p: TSESTree.Parameter) => + !isTSParameterProperty(p) && + typeContainsTypeParameter(p.typeAnnotation), ); - function typeContainsTypeParameter(type: any): boolean | undefined { + function typeContainsTypeParameter( + type?: TSESTree.TSTypeAnnotation | TSESTree.TypeNode, + ): boolean { if (!type) { return false; } - if (type.type === Syntax.TSTypeReference) { + if (type.type === AST_NODE_TYPES.TSTypeReference) { const typeName = type.typeName; - if ( - typeName.type === Syntax.Identifier && - isTypeParameter(typeName.name) - ) { + if (isIdentifier(typeName) && isTypeParameter(typeName.name)) { return true; } } return typeContainsTypeParameter( - type.typeAnnotation || type.elementType, + (type as TSESTree.TSTypeAnnotation).typeAnnotation || + (type as TSESTree.TSArrayType).elementType, ); } } - function parametersAreEqual(a: any, b: any): boolean { + function isTSParameterProperty( + node: TSESTree.Node, + ): node is TSESTree.TSParameterProperty { + return ( + (node as TSESTree.TSParameterProperty).type === + AST_NODE_TYPES.TSParameterProperty + ); + } + + function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { + return node.type === AST_NODE_TYPES.Identifier; + } + + function parametersAreEqual( + a: TSESTree.Parameter, + b: TSESTree.Parameter, + ): boolean { + const typeAnnotationA = !isTSParameterProperty(a) + ? a.typeAnnotation + : undefined; + const typeAnnotationB = !isTSParameterProperty(b) + ? b.typeAnnotation + : undefined; + return ( parametersHaveEqualSigils(a, b) && - typesAreEqual(a.typeAnnotation, b.typeAnnotation) + typesAreEqual(typeAnnotationA, typeAnnotationB) ); } /** True for optional/rest parameters. */ - function parameterMayBeMissing(p: any): boolean { - return p.type === Syntax.RestElement || p.optional; + function parameterMayBeMissing(p: TSESTree.Parameter): boolean | undefined { + const optional = !isTSParameterProperty(p) ? p.optional : undefined; + + return p.type === AST_NODE_TYPES.RestElement || optional; } /** False if one is optional and the other isn't, or one is a rest parameter and the other isn't. */ - function parametersHaveEqualSigils(a: any, b: any): boolean { + function parametersHaveEqualSigils( + a: TSESTree.Parameter, + b: TSESTree.Parameter, + ): boolean { + const optionalA = !isTSParameterProperty(a) ? a.optional : undefined; + const optionalB = !isTSParameterProperty(b) ? b.optional : undefined; + return ( - (a.type === Syntax.RestElement) === (b.type === Syntax.RestElement) && - (a.optional !== undefined) === (b.optional !== undefined) + (a.type === AST_NODE_TYPES.RestElement) === + (b.type === AST_NODE_TYPES.RestElement) && + (optionalA !== undefined) === (optionalB !== undefined) ); } - function typeParametersAreEqual(a: any, b: any): boolean { + function typeParametersAreEqual( + a: TSESTree.TSTypeParameter, + b: TSESTree.TSTypeParameter, + ): boolean { return ( a.name.name === b.name.name && constraintsAreEqual(a.constraint, b.constraint) ); } - function typesAreEqual(a: any, b: any): boolean { + function typesAreEqual( + a: TSESTree.TSTypeAnnotation | undefined, + b: TSESTree.TSTypeAnnotation | undefined, + ): boolean { return ( a === b || (a !== undefined && @@ -325,7 +422,10 @@ export default util.createRule({ ); } - function constraintsAreEqual(a: any, b: any): boolean { + function constraintsAreEqual( + a: TSESTree.TypeNode | undefined, + b: TSESTree.TypeNode | undefined, + ): boolean { return ( a === b || (a !== undefined && b !== undefined && a.type === b.type) ); @@ -362,19 +462,20 @@ export default util.createRule({ } interface Scope { - overloads: Map; - parent: any; - typeParameters: ReadonlyArray | undefined; + overloads: Map; + parent?: ScopeNode; + typeParameters?: TSESTree.TSTypeParameterDeclaration; } const scopes: Scope[] = []; let currentScope: Scope | undefined = { overloads: new Map(), - parent: undefined, - typeParameters: undefined, }; - function createScope(parent: any, typeParameters: any = undefined) { + function createScope( + parent: ScopeNode, + typeParameters?: TSESTree.TSTypeParameterDeclaration, + ) { currentScope && scopes.push(currentScope); currentScope = { overloads: new Map(), @@ -394,7 +495,7 @@ export default util.createRule({ currentScope = scopes.pop(); } - function addOverload(signature: any, key?: string) { + function addOverload(signature: OverloadNode, key?: string) { key = key || getOverloadKey(signature); if (currentScope && signature.parent === currentScope.parent && key) { const overloads = currentScope.overloads.get(key); @@ -411,21 +512,15 @@ export default util.createRule({ //---------------------------------------------------------------------- return { - Program(node) { - createScope(node); - }, - TSModuleBlock(node) { - createScope(node); - }, + Program: createScope, + TSModuleBlock: createScope, TSInterfaceDeclaration(node) { createScope(node.body, node.typeParameters); }, ClassDeclaration(node) { createScope(node.body, node.typeParameters); }, - TSTypeLiteral(node) { - createScope(node); - }, + TSTypeLiteral: createScope, // collect overloads TSDeclareFunction(node) { if (node.id && !node.body) { @@ -455,35 +550,39 @@ export default util.createRule({ }, }); -function getOverloadKey(node: any): string | undefined { +function getOverloadKey(node: OverloadNode): string | undefined { const info = getOverloadInfo(node); if (info === undefined) { return undefined; } - return (node.computed ? '0' : '1') + (node.static ? '0' : '1') + info; + return ( + ((node as MethodDefinition).computed ? '0' : '1') + + ((node as MethodDefinition).static ? '0' : '1') + + info + ); } function getOverloadInfo( - node: any, + node: OverloadNode, ): string | { name: string; computed?: boolean } | undefined { switch (node.type) { - case Syntax.TSConstructSignatureDeclaration: + case AST_NODE_TYPES.TSConstructSignatureDeclaration: return 'constructor'; - case Syntax.TSCallSignatureDeclaration: + case AST_NODE_TYPES.TSCallSignatureDeclaration: return '()'; default: { - const { key } = node; + const { key } = node as any; // TODO: FIgure out the type if (key === undefined) { return undefined; } - const { value } = key; + const { value } = key as any; // TODO: FIgure out the type switch (key.type) { - case Syntax.Identifier: + case AST_NODE_TYPES.Identifier: return key.name; - case Syntax.Property: + case AST_NODE_TYPES.Property: return tsutils.isLiteralExpression(value) ? value.text : { name: value.getText(), computed: true }; diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index e6b00cfbac2..ccc4802d79a 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -92,6 +92,13 @@ interface Generic { x(): void; x(x: T[]): void; } +`, + // Allow signatures if the type is not equal. + ` +interface I { + f(x1:number): void; + f(x1:boolean, x2?: number): void; +} `, ], invalid: [ diff --git a/packages/eslint-plugin/typings/ts-eslint.d.ts b/packages/eslint-plugin/typings/ts-eslint.d.ts index b2439aace2e..33b667dab46 100644 --- a/packages/eslint-plugin/typings/ts-eslint.d.ts +++ b/packages/eslint-plugin/typings/ts-eslint.d.ts @@ -482,7 +482,13 @@ declare module 'ts-eslint' { TSAsyncKeyword?: RuleFunction; TSBigIntKeyword?: RuleFunction; TSBooleanKeyword?: RuleFunction; + TSCallSignatureDeclaration?: RuleFunction< + TSESTree.TSCallSignatureDeclaration + >; TSConditionalType?: RuleFunction; + TSConstructSignatureDeclaration?: RuleFunction< + TSESTree.TSConstructSignatureDeclaration + >; TSDeclareKeyword?: RuleFunction; TSDeclareFunction?: RuleFunction; TSEnumDeclaration?: RuleFunction; From f2a0d9d24f02e91982047406ef6c24156bc2debd Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 14 Mar 2019 00:57:27 -0700 Subject: [PATCH 09/13] fix(unified-signatures): improved tests --- .../src/rules/unified-signatures.ts | 46 +++------- .../tests/rules/unified-signatures.test.ts | 89 +++++++++++++++++++ 2 files changed, 101 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index 5aa430b2095..1e1d07f047d 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -1,4 +1,3 @@ -import * as tsutils from 'tsutils'; import * as util from '../util'; import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; @@ -357,10 +356,6 @@ export default util.createRule({ ); } - function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { - return node.type === AST_NODE_TYPES.Identifier; - } - function parametersAreEqual( a: TSESTree.Parameter, b: TSESTree.Parameter, @@ -446,19 +441,15 @@ export default util.createRule({ } /** Calls `action` for every pair of values in `values`. */ - function forEachPair( + function forEachPair( values: ReadonlyArray, - action: (a: T, b: T) => Out | undefined, - ): Out | undefined { + action: (a: T, b: T) => void, + ): void { for (let i = 0; i < values.length; i++) { for (let j = i + 1; j < values.length; j++) { - const result = action(values[i], values[j]); - if (result !== undefined) { - return result; - } + action(values[i], values[j]); } } - return undefined; } interface Scope { @@ -563,32 +554,19 @@ function getOverloadKey(node: OverloadNode): string | undefined { ); } -function getOverloadInfo( - node: OverloadNode, -): string | { name: string; computed?: boolean } | undefined { +function getOverloadInfo(node: OverloadNode): string { switch (node.type) { case AST_NODE_TYPES.TSConstructSignatureDeclaration: return 'constructor'; case AST_NODE_TYPES.TSCallSignatureDeclaration: return '()'; - default: { - const { key } = node as any; // TODO: FIgure out the type - if (key === undefined) { - return undefined; - } + default: + const { key } = node as MethodDefinition; - const { value } = key as any; // TODO: FIgure out the type - - switch (key.type) { - case AST_NODE_TYPES.Identifier: - return key.name; - case AST_NODE_TYPES.Property: - return tsutils.isLiteralExpression(value) - ? value.text - : { name: value.getText(), computed: true }; - default: - return tsutils.isLiteralExpression(key) ? value.text : undefined; - } - } + return isIdentifier(key) ? key.name : (key as TSESTree.Literal).raw; } } + +function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { + return node.type === AST_NODE_TYPES.Identifier; +} diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index ccc4802d79a..6d2045b62b2 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -100,6 +100,11 @@ interface I { f(x1:boolean, x2?: number): void; } `, + // AllowType parameters that are not equal + ` +function f(x: T[]): void; +function f(x: T): void; + `, ], invalid: [ { @@ -392,5 +397,89 @@ interface Generic { }, ], }, + { + // Check type parameters when equal + code: ` +function f(x: T[]): void; +function f(x: T): void; +`, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'T[]', + type2: 'T', + }, + line: 3, + column: 15, + }, + ], + }, + { + // Verifies type parameters and constraints + code: ` +function f(x: T[]): void; +function f(x: T): void; +`, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'T[]', + type2: 'T', + }, + line: 3, + column: 30, + }, + ], + }, + { + // Works with abstract + code: ` +abstract class Foo { + public abstract f(x: number): void; + public abstract f(x: string): void; +} +`, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, + line: 4, + column: 23, + }, + ], + }, + { + // Works with literals + code: ` +interface Foo { + "f"(x: string): void; + "f"(x: number): void; +} +`, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'string', + type2: 'number', + }, + line: 4, + column: 9, + }, + ], + }, ], }); From 13a26e54615610ab4ec7d2cbd9e22813f37a533f Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 14 Mar 2019 01:02:26 -0700 Subject: [PATCH 10/13] fix(unified-signatures): fixed link in roadmap --- packages/eslint-plugin/ROADMAP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 842fc55a539..11ccb8ac22b 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -589,6 +589,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/no-unnecessary-type-assertion`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md [`@typescript-eslint/no-var-requires`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-var-requires.md [`@typescript-eslint/type-annotation-spacing`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/type-annotation-spacing.md +[`@typescript-eslint/unified-signatures`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unified-signatures.md [`@typescript-eslint/no-misused-new`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-misused-new.md [`@typescript-eslint/no-object-literal-type-assertion`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-object-literal-type-assertion.md [`@typescript-eslint/no-this-alias`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-this-alias.md From c73f27831301f9da87cb2850298880e2f47651b9 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 14 Mar 2019 01:09:29 -0700 Subject: [PATCH 11/13] fix(unified-signatures): fixed lint break --- packages/eslint-plugin/src/rules/unified-signatures.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index 1e1d07f047d..494fd5472d1 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -560,10 +560,11 @@ function getOverloadInfo(node: OverloadNode): string { return 'constructor'; case AST_NODE_TYPES.TSCallSignatureDeclaration: return '()'; - default: + default: { const { key } = node as MethodDefinition; return isIdentifier(key) ? key.name : (key as TSESTree.Literal).raw; + } } } From 95f88f8429f03bd2ff9119a3eb90010089104191 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 14 Mar 2019 01:28:47 -0700 Subject: [PATCH 12/13] fix(unified-signatures): more tests improvements --- .../src/rules/unified-signatures.ts | 3 --- .../tests/rules/unified-signatures.test.ts | 22 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index 494fd5472d1..fe3cd99763a 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -543,9 +543,6 @@ export default util.createRule({ function getOverloadKey(node: OverloadNode): string | undefined { const info = getOverloadInfo(node); - if (info === undefined) { - return undefined; - } return ( ((node as MethodDefinition).computed ? '0' : '1') + diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index 6d2045b62b2..754d1eb67b7 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -466,6 +466,28 @@ interface Foo { "f"(x: string): void; "f"(x: number): void; } +`, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'string', + type2: 'number', + }, + line: 4, + column: 9, + }, + ], + }, + { + // Works with new constructor + code: ` +interface Foo { + new(x: string): Foo; + new(x: number): Foo; +} `, errors: [ { From e755f3bd0dfc24f4580b5951df580064bd80b633 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 14 Mar 2019 14:33:46 -0700 Subject: [PATCH 13/13] fix(unified-signatures): even more tests improvements --- .../src/rules/unified-signatures.ts | 72 ++++++++------- .../tests/rules/unified-signatures.test.ts | 88 +++++++++++++++++++ 2 files changed, 127 insertions(+), 33 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index fe3cd99763a..1b54bd4217f 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -90,12 +90,12 @@ export default util.createRule({ const { p0, p1 } = unify; const lineOfOtherOverload = only2 ? undefined : p0.loc.start.line; - const typeAnnotation0 = !isTSParameterProperty(p0) - ? p0.typeAnnotation - : undefined; - const typeAnnotation1 = !isTSParameterProperty(p1) - ? p1.typeAnnotation - : undefined; + const typeAnnotation0 = isTSParameterProperty(p0) + ? p0.parameter.typeAnnotation + : p0.typeAnnotation; + const typeAnnotation1 = isTSParameterProperty(p1) + ? p1.parameter.typeAnnotation + : p1.typeAnnotation; context.report({ loc: p1.loc, @@ -274,12 +274,12 @@ export default util.createRule({ for (let i = 0; i < minLength; i++) { const sig1i = sig1[i]; const sig2i = sig2[i]; - const typeAnnotation1 = !isTSParameterProperty(sig1i) - ? sig1i.typeAnnotation - : undefined; - const typeAnnotation2 = !isTSParameterProperty(sig2i) - ? sig2i.typeAnnotation - : undefined; + const typeAnnotation1 = isTSParameterProperty(sig1i) + ? sig1i.parameter.typeAnnotation + : sig1i.typeAnnotation; + const typeAnnotation2 = isTSParameterProperty(sig2i) + ? sig2i.parameter.typeAnnotation + : sig2i.typeAnnotation; if (!typesAreEqual(typeAnnotation1, typeAnnotation2)) { return undefined; @@ -320,10 +320,12 @@ export default util.createRule({ sig: SignatureDefinition, isTypeParameter: IsTypeParameter, ): boolean { - return sig.params.some( - (p: TSESTree.Parameter) => - !isTSParameterProperty(p) && - typeContainsTypeParameter(p.typeAnnotation), + return sig.params.some((p: TSESTree.Parameter) => + typeContainsTypeParameter( + isTSParameterProperty(p) + ? p.parameter.typeAnnotation + : p.typeAnnotation, + ), ); function typeContainsTypeParameter( @@ -360,12 +362,12 @@ export default util.createRule({ a: TSESTree.Parameter, b: TSESTree.Parameter, ): boolean { - const typeAnnotationA = !isTSParameterProperty(a) - ? a.typeAnnotation - : undefined; - const typeAnnotationB = !isTSParameterProperty(b) - ? b.typeAnnotation - : undefined; + const typeAnnotationA = isTSParameterProperty(a) + ? a.parameter.typeAnnotation + : a.typeAnnotation; + const typeAnnotationB = isTSParameterProperty(b) + ? b.parameter.typeAnnotation + : b.typeAnnotation; return ( parametersHaveEqualSigils(a, b) && @@ -375,7 +377,9 @@ export default util.createRule({ /** True for optional/rest parameters. */ function parameterMayBeMissing(p: TSESTree.Parameter): boolean | undefined { - const optional = !isTSParameterProperty(p) ? p.optional : undefined; + const optional = isTSParameterProperty(p) + ? p.parameter.optional + : p.optional; return p.type === AST_NODE_TYPES.RestElement || optional; } @@ -385,8 +389,12 @@ export default util.createRule({ a: TSESTree.Parameter, b: TSESTree.Parameter, ): boolean { - const optionalA = !isTSParameterProperty(a) ? a.optional : undefined; - const optionalB = !isTSParameterProperty(b) ? b.optional : undefined; + const optionalA = isTSParameterProperty(a) + ? a.parameter.optional + : a.optional; + const optionalB = isTSParameterProperty(b) + ? b.parameter.optional + : b.optional; return ( (a.type === AST_NODE_TYPES.RestElement) === @@ -459,7 +467,7 @@ export default util.createRule({ } const scopes: Scope[] = []; - let currentScope: Scope | undefined = { + let currentScope: Scope = { overloads: new Map(), }; @@ -476,14 +484,12 @@ export default util.createRule({ } function checkScope() { - const failures = currentScope - ? checkOverloads( - Array.from(currentScope.overloads.values()), - currentScope.typeParameters, - ) - : []; + const failures = checkOverloads( + Array.from(currentScope.overloads.values()), + currentScope.typeParameters, + ); addFailures(failures); - currentScope = scopes.pop(); + currentScope = scopes.pop()!; } function addOverload(signature: OverloadNode, key?: string) { diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index 754d1eb67b7..94b6520de81 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -503,5 +503,93 @@ interface Foo { }, ], }, + { + // Works with new computed properties + code: ` +enum Enum { + Func = "function", +} + +interface IFoo { + [Enum.Func](x: string): void; + [Enum.Func](x: number): void; +} +`, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'string', + type2: 'number', + }, + line: 8, + column: 17, + }, + ], + }, + { + // Works with parameter properties. Note that this is invalid TypeScript syntax. + code: ` +class Foo { + constructor(readonly x: number); + constructor(readonly x: string); +} + `, + errors: [ + { + messageId: 'singleParameterDifference', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + type1: 'number', + type2: 'string', + }, + line: 4, + column: 17, + }, + ], + }, + { + // Works with parameter properties. Note that this is invalid TypeScript syntax. + code: ` +class Foo { + constructor(readonly x: number); + constructor(readonly x: number, readonly y: string); +} +`, + errors: [ + { + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, + line: 4, + column: 37, + }, + ], + }, + { + // Works with parameter properties. Note that this is invalid TypeScript syntax. + code: ` +class Foo { + constructor(readonly x: number); + constructor(readonly x: number, readonly y?: string, readonly z?: string); +} +`, + errors: [ + { + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, + line: 4, + column: 58, + }, + ], + }, ], });