diff --git a/baselines/packages/mimir/api/packlist.txt b/baselines/packages/mimir/api/packlist.txt index aa0694829..c42f1b999 100644 --- a/baselines/packages/mimir/api/packlist.txt +++ b/baselines/packages/mimir/api/packlist.txt @@ -133,6 +133,9 @@ src/rules/prefer-number-methods.js.map src/rules/prefer-object-spread.d.ts src/rules/prefer-object-spread.js src/rules/prefer-object-spread.js.map +src/rules/prefer-spread-arguments.d.ts +src/rules/prefer-spread-arguments.js +src/rules/prefer-spread-arguments.js.map src/rules/return-never-call.d.ts src/rules/return-never-call.js src/rules/return-never-call.js.map diff --git a/baselines/packages/mimir/api/src/rules/prefer-spread-arguments.d.ts b/baselines/packages/mimir/api/src/rules/prefer-spread-arguments.d.ts new file mode 100644 index 000000000..8f85b0dd9 --- /dev/null +++ b/baselines/packages/mimir/api/src/rules/prefer-spread-arguments.d.ts @@ -0,0 +1,4 @@ +import { TypedRule } from '@fimbul/ymir'; +export declare class Rule extends TypedRule { + apply(): void; +} diff --git a/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.fix b/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.fix index ba6a3f66b..98460abb6 100644 --- a/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.fix +++ b/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.fix @@ -1,8 +1,14 @@ interface Empty {} class Foo { - dontSpreadTypeParam(param: T, param2: T | undefined) { + spreadTypeParam(param: T, param2: T | undefined, param3: U) { + ({...param}); + ({...param2}); + ({...param3}); + } + dontSpreadPrimitiveTypeParameter(param: T, param2: U, param3: V) { Object.assign({}, param); Object.assign({}, param2); + Object.assign({}, param3); } dontSpreadNonObject(param: number | boolean) { return Object.assign({}, param); @@ -27,7 +33,13 @@ class Foo { spreadAny(param: any) { return {...param}; } - dontSpreadThis() { + spreadThis() { + return {...this}; + } + spreadThisParameter(this: object) { + return {...this}; + } + dontSpreadPrimitiveThis(this: number) { return Object.assign({}, this); } spreadUnknown(param: unknown) { diff --git a/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.lint b/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.lint index a2ebf9a9b..faafc71c4 100644 --- a/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.lint +++ b/baselines/packages/mimir/test/prefer-object-spread/default/test.ts.lint @@ -1,8 +1,17 @@ interface Empty {} class Foo { - dontSpreadTypeParam(param: T, param2: T | undefined) { + spreadTypeParam(param: T, param2: T | undefined, param3: U) { Object.assign({}, param); + ~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-object-spread: Prefer object spread over 'Object.assign'.] Object.assign({}, param2); + ~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-object-spread: Prefer object spread over 'Object.assign'.] + Object.assign({}, param3); + ~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-object-spread: Prefer object spread over 'Object.assign'.] + } + dontSpreadPrimitiveTypeParameter(param: T, param2: U, param3: V) { + Object.assign({}, param); + Object.assign({}, param2); + Object.assign({}, param3); } dontSpreadNonObject(param: number | boolean) { return Object.assign({}, param); @@ -31,7 +40,15 @@ class Foo { return Object.assign({}, param); ~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-object-spread: Prefer object spread over 'Object.assign'.] } - dontSpreadThis() { + spreadThis() { + return Object.assign({}, this); + ~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-object-spread: Prefer object spread over 'Object.assign'.] + } + spreadThisParameter(this: object) { + return Object.assign({}, this); + ~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-object-spread: Prefer object spread over 'Object.assign'.] + } + dontSpreadPrimitiveThis(this: number) { return Object.assign({}, this); } spreadUnknown(param: unknown) { diff --git a/baselines/packages/mimir/test/prefer-spread-arguments/default/test.ts.fix b/baselines/packages/mimir/test/prefer-spread-arguments/default/test.ts.fix new file mode 100644 index 000000000..e3ffa3627 --- /dev/null +++ b/baselines/packages/mimir/test/prefer-spread-arguments/default/test.ts.fix @@ -0,0 +1,79 @@ +export {}; + +declare const args: any[]; + +alert(...args); +alert(...args); +alert.apply(alert, args); +alert.apply(null, ...[args] as const); +alert.apply(...[null, args] as const); + +declare const fn: ((...args: any[]) => void) | undefined; +fn!(...args); +fn?.(...args); +(fn as (...args: any[]) => void)(...args); + +declare const obj: {apply(a: any, b: any): any}; +obj.apply(null, args); + +const nested = {obj}; +nested.obj.apply(nested, args); + +console.log.apply(null, args); +console.log(...args); +console.log.apply(console.log, args); +console.log.apply(obj, args); +console['log'](...args); +(console['log'])!(...args); +window.console.log(...args); +window.console.log(...args); +window.console.log(...args); +window['console'].log(...args); +window['console'].log(...args); + +declare let objArr: {fn: (...args: any[]) => void}[]; +declare let i: number; +objArr[0].fn(...args); +objArr[0].fn(...args); +objArr[0].fn.apply(objArr[1], args); +objArr[i].fn(...args); +objArr[objArr.length].fn(...args); +objArr[objArr.length].fn(...args); +objArr[objArr.length - 1].fn.apply(objArr[objArr.length - 1], args); + +objArr[Symbol.iterator](...args); + +class C { + #fn!: (...args: any[]) => void; + #obj: {fn: (...args: any[]) => void}; + [key: string]: {fn: (...args: any[]) => void}; + constructor() { + this.#fn(...args); + this.#fn.apply(new C(), args); + this.#fn.apply(null, args); + this.#obj.fn(...args); + this.#obj.fn.apply(this, args); + + this.#obj.fn.apply(this['#obj'], args); + this.#obj.fn.apply(this['obj'], args); + this['#obj'].fn.apply(this.#obj, args); + this['#obj'].fn(...args); + } +} + +class Base { + fn(...args: any[]) {}; +} + +class Derived extends Base { + fn() { + super.fn(...args); + } +} + +fn(...args); +fn(...args); + +fn(...args); + +fn.apply // diff --git a/baselines/packages/mimir/test/prefer-spread-arguments/default/test.ts.lint b/baselines/packages/mimir/test/prefer-spread-arguments/default/test.ts.lint new file mode 100644 index 000000000..3c9258341 --- /dev/null +++ b/baselines/packages/mimir/test/prefer-spread-arguments/default/test.ts.lint @@ -0,0 +1,111 @@ +export {}; + +declare const args: any[]; + +alert.apply(null, args); +~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +alert.apply(undefined, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +alert.apply(alert, args); +alert.apply(null, ...[args] as const); +alert.apply(...[null, args] as const); + +declare const fn: ((...args: any[]) => void) | undefined; +fn!.apply(null, args); +~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +fn?.apply(null, args); +~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +(fn as (...args: any[]) => void).apply(null, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + +declare const obj: {apply(a: any, b: any): any}; +obj.apply(null, args); + +const nested = {obj}; +nested.obj.apply(nested, args); + +console.log.apply(null, args); +console.log.apply(console, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +console.log.apply(console.log, args); +console.log.apply(obj, args); +console['log'].apply(console, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +(console['log'])!.apply(console, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +window.console.log.apply(window.console, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +window.console.log.apply(window.console!, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +window.console.log.apply(window['console'], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +window['console'].log.apply(window.console, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +window['console'].log.apply(window['console'], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + +declare let objArr: {fn: (...args: any[]) => void}[]; +declare let i: number; +objArr[0].fn.apply(objArr[0], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +objArr[0].fn.apply(objArr['0'], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +objArr[0].fn.apply(objArr[1], args); +objArr[i].fn.apply(objArr[i], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +objArr[objArr.length].fn.apply(objArr[objArr.length], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +objArr[objArr.length].fn.apply(objArr[objArr['length']], args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +objArr[objArr.length - 1].fn.apply(objArr[objArr.length - 1], args); + +objArr[Symbol.iterator].apply(objArr, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + +class C { + #fn!: (...args: any[]) => void; + #obj: {fn: (...args: any[]) => void}; + [key: string]: {fn: (...args: any[]) => void}; + constructor() { + this.#fn.apply(this, args); + ~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + this.#fn.apply(new C(), args); + this.#fn.apply(null, args); + this.#obj.fn.apply(this.#obj, args); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + this.#obj.fn.apply(this, args); + + this.#obj.fn.apply(this['#obj'], args); + this.#obj.fn.apply(this['obj'], args); + this['#obj'].fn.apply(this.#obj, args); + this['#obj'].fn.apply(this['#obj'], args); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + } +} + +class Base { + fn(...args: any[]) {}; +} + +class Derived extends Base { + fn() { + super.fn.apply(this, args); + ~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + } +} + +fn.apply /* .apply() */(null /* .apply() */, args); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] +fn. // +~~~~~~ + apply +~~~~~~~ +(null, args); +~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + +fn.apply // +~~~~~~~~~~~ + (null, args); +~~~~~~~~~~~~~~~~ [error prefer-spread-arguments: Prefer spread arguments over 'Function.prototype.apply'.] + +fn.apply // diff --git a/packages/mimir/README.md b/packages/mimir/README.md index 5ef651128..47e882e89 100644 --- a/packages/mimir/README.md +++ b/packages/mimir/README.md @@ -67,6 +67,7 @@ Rule | Description | Difference to TSLint rule / Why you should use it [`prefer-namespace-keyword`](docs/prefer-namespace-keyword.md) | :wrench: Prefer `namespace foo {}` over `module foo {}` to avoid confusion with ECMAScript modules. | Same as TSLint's `no-internal-module`. [`prefer-number-methods`](docs/prefer-number-methods.md) | :mag: :wrench: Prefer ES2015's `Number.isNaN` and `Number.isFinite` over the global `isNaN` and `isFinite`. | No similar rule in TSLint. [`prefer-object-spread`](docs/prefer-object-spread.md) | :mag: :wrench: Prefer object spread over `Object.assign` for copying properties to a new object. | Performance, better handling of parens in fixer and avoids false positives that would cause a compile error when fixed. +[`prefer-spread-arguments`](docs/prefer-spread-arguments.md) | :mag: :wrench: Prefer spread arguments over `Function.prototype.apply` to call variadic functions. | No similar rule in TSLint. [`return-never-call`](docs/return-never-call.md) | :mag: Enforces `return`ing or `throw`ing the result of a function of method call that returns `never`. | TSLint has no similar rule. [`syntaxcheck`](docs/syntaxcheck.md) | :mag: :x: Reports syntax errors as lint errors.| Used to be part of the deprecated `tslint --type-check` [`trailing-newline`](docs/trailing-newline.md) | :wrench: Enforces a line break at the end of each file. | Nothing fancy here :( diff --git a/packages/mimir/docs/delete-only-optional-property.md b/packages/mimir/docs/delete-only-optional-property.md index 8b34fe5bb..14d6203ce 100644 --- a/packages/mimir/docs/delete-only-optional-property.md +++ b/packages/mimir/docs/delete-only-optional-property.md @@ -1,6 +1,6 @@ # delete-only-optional-property -:mag: requires type information +:mag: requires type information and `strictNullChecks` compiler option Disallows `delete` of required properties. diff --git a/packages/mimir/docs/prefer-spread-arguments.md b/packages/mimir/docs/prefer-spread-arguments.md new file mode 100644 index 000000000..7a81ef5e0 --- /dev/null +++ b/packages/mimir/docs/prefer-spread-arguments.md @@ -0,0 +1,50 @@ +# prefer-spread-arguments + +:mag: requires type information and `strictBindCallApply` compiler option +:wrench: fixable + +Prefer spread arguments over `Function.prototype.apply` to call variadic functions. + +## Rationale + +ECMAScript 2015 added a way to syntactically express variadic arguments in function calls. Before you had to use `Function.prototype.apply`. +Spread arguments provide additional safety as its behavior cannot be overridden, which is possible for `Fucntion.prototype.apply`. In addition there is no chance to accidentally call the function with the wrong `thisArg` (value of `this`). + +## Examples + +:thumbsdown: Examples of incorrect code + +```ts +declare const args: string[]; + +alert.apply(null, args); +alert.apply(undefined, args); +console.log.apply(console, args); +``` + +:thumbsup: Examples of correct code + +```ts +declare const args: string[]; + +// fixed examples from above +alert(...args); +alert(...args); +console.log(...args); + +// intentionally providing a different thisArg +declare const fn: (...args: any[]) => void; +fn.apply(global, args); + +// calling a method named 'apply' +const obj = { apply(a: any, b: any) {}} +obj.apply(null, args); +``` + +## Further Reading + +* MDN: [Spread syntax / Spread in function calls](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_function_calls) + +## Related Rules + +* [`no-useless-spread`](no-useless-spread.md) diff --git a/packages/mimir/recommended.yaml b/packages/mimir/recommended.yaml index 90664329d..1f10f09a6 100644 --- a/packages/mimir/recommended.yaml +++ b/packages/mimir/recommended.yaml @@ -40,6 +40,7 @@ rules: prefer-namespace-keyword: error prefer-number-methods: error prefer-object-spread: error + prefer-spread-arguments: error return-never-call: error trailing-newline: error try-catch-return-await: error diff --git a/packages/mimir/src/rules/no-useless-initializer.ts b/packages/mimir/src/rules/no-useless-initializer.ts index a1b97e3ee..13f8204cc 100644 --- a/packages/mimir/src/rules/no-useless-initializer.ts +++ b/packages/mimir/src/rules/no-useless-initializer.ts @@ -1,7 +1,6 @@ import { AbstractRule, Replacement, excludeDeclarationFiles } from '@fimbul/ymir'; import * as ts from 'typescript'; import { - isIdentifier, getChildOfKind, isFunctionWithBody, isUnionTypeNode, @@ -17,6 +16,7 @@ import { LateBoundPropertyNames, getLateBoundPropertyNamesOfPropertyName, } from 'tsutils'; +import { isUndefined } from '../utils'; @excludeDeclarationFiles export class Rule extends AbstractRule { @@ -220,7 +220,3 @@ function typeMaybeUndefined(checker: ts.TypeChecker, type: ts.Type): boolean { return type.types.some((t) => typeMaybeUndefined(checker, t)); return (type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Any | ts.TypeFlags.Unknown)) !== 0; } - -function isUndefined(node: ts.Expression) { - return isIdentifier(node) && node.originalKeywordKind === ts.SyntaxKind.UndefinedKeyword; -} diff --git a/packages/mimir/src/rules/no-useless-predicate.ts b/packages/mimir/src/rules/no-useless-predicate.ts index 9dd4e69eb..8046e0662 100644 --- a/packages/mimir/src/rules/no-useless-predicate.ts +++ b/packages/mimir/src/rules/no-useless-predicate.ts @@ -7,7 +7,6 @@ import { isFalsyType, isTypeOfExpression, isTextualLiteral, - isIdentifier, isEmptyObjectType, isIntersectionType, isStrictCompilerOptionEnabled, @@ -20,7 +19,7 @@ import { getPropertyOfType, intersectionTypeParts, } from 'tsutils'; -import { formatPseudoBigInt } from '../utils'; +import { formatPseudoBigInt, isUndefined } from '../utils'; interface TypePredicate { nullable: boolean; @@ -331,10 +330,6 @@ export class Rule extends TypedRule { } } -function isUndefined(node: ts.Expression): node is ts.Identifier { - return isIdentifier(node) && node.originalKeywordKind === ts.SyntaxKind.UndefinedKeyword; -} - function falsy(type: ts.Type): false | undefined { return isFalsyType(type) ? false : undefined; } diff --git a/packages/mimir/src/rules/prefer-for-of.ts b/packages/mimir/src/rules/prefer-for-of.ts index 7767fe7c8..4c654e4d3 100644 --- a/packages/mimir/src/rules/prefer-for-of.ts +++ b/packages/mimir/src/rules/prefer-for-of.ts @@ -24,7 +24,6 @@ import { isUnionType, getIteratorYieldResultFromIteratorResult, } from 'tsutils'; -import * as path from 'path'; import { typesAreEqual } from '../utils'; @excludeDeclarationFiles @@ -91,9 +90,7 @@ export class Rule extends TypedRule { } private isDeclaredInDefaultLib(node: ts.Node): boolean { - // we assume it's the global array type if it comes from any lib.xxx.d.ts file - return path.normalize(path.dirname(node.getSourceFile().fileName)) - === path.dirname(ts.getDefaultLibFilePath(this.context.compilerOptions)); + return node.getSourceFile().hasNoDefaultLib; } private isIterable(type: ts.Type, node: ts.Expression): boolean { diff --git a/packages/mimir/src/rules/prefer-object-spread.ts b/packages/mimir/src/rules/prefer-object-spread.ts index c21d0ec75..eac7d6d4b 100644 --- a/packages/mimir/src/rules/prefer-object-spread.ts +++ b/packages/mimir/src/rules/prefer-object-spread.ts @@ -1,49 +1,32 @@ import { TypedRule, Replacement, excludeDeclarationFiles } from '@fimbul/ymir'; import * as ts from 'typescript'; import { - WrappedAst, - getWrappedNodeAtPosition, isIdentifier, - isPropertyAccessExpression, - isCallExpression, isObjectLiteralExpression, unionTypeParts, isFalsyType, } from 'tsutils'; -import { objectLiteralNeedsParens } from '../utils'; +import { findMethodCalls, objectLiteralNeedsParens } from '../utils'; @excludeDeclarationFiles export class Rule extends TypedRule { public apply() { - const re = /(?:[.\n]|\*\/)\s*assign\b/g; - let wrappedAst: WrappedAst | undefined; - for (let match = re.exec(this.sourceFile.text); match !== null; match = re.exec(this.sourceFile.text)) { - const {node} = getWrappedNodeAtPosition(wrappedAst || (wrappedAst = this.context.getWrappedAst()), re.lastIndex - 1)!; - if (node.kind !== ts.SyntaxKind.Identifier || node.end !== re.lastIndex) + for (const node of findMethodCalls(this.context, 'assign')) { + if ( + node.arguments.length === 0 || node.arguments[0].kind !== ts.SyntaxKind.ObjectLiteralExpression || + !isIdentifier(node.expression.expression) || node.expression.expression.escapedText !== 'Object' + ) continue; - const parent = node.parent!; - if (!isPropertyAccessExpression(parent) || parent.name !== node || - !isIdentifier(parent.expression) || parent.expression.text !== 'Object') - continue; - const grandParent = parent.parent!; - if (!isCallExpression(grandParent) || grandParent.expression !== parent || - grandParent.arguments.length === 0 || !isObjectLiteralExpression(grandParent.arguments[0])) - continue; - if (grandParent.arguments.length === 1) { - this.addFindingAtNode( - grandParent, - "No need for 'Object.assign', use the object directly.", - createFix(grandParent, this.sourceFile), - ); - } else if (grandParent.arguments.every(this.isSpreadableObject, this)) { - this.addFindingAtNode(grandParent, "Prefer object spread over 'Object.assign'.", createFix(grandParent, this.sourceFile)); + if (node.arguments.length === 1) { + this.addFindingAtNode(node, "No need for 'Object.assign', use the object directly.", createFix(node, this.sourceFile)); + } else if (node.arguments.every(this.isSpreadableObject, this)) { + this.addFindingAtNode(node, "Prefer object spread over 'Object.assign'.", createFix(node, this.sourceFile)); } } } private isSpreadableObject(node: ts.Expression): boolean { switch (node.kind) { - case ts.SyntaxKind.ThisKeyword: case ts.SyntaxKind.SpreadElement: return false; case ts.SyntaxKind.ObjectLiteralExpression: @@ -53,14 +36,22 @@ export class Rule extends TypedRule { if (type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) return true; let seenObject = false; - for (const t of unionTypeParts(type)) { + const isSpreadable = (t: ts.Type): boolean => { if (t.flags & (ts.TypeFlags.Object | ts.TypeFlags.NonPrimitive)) { seenObject = true; - } else if (!isFalsyType(t)) { - return false; + return true; } - } - return seenObject; + if (t.flags & ts.TypeFlags.TypeParameter) { + const constraint = this.checker.getBaseConstraintOfType(t); + if (!constraint) { + seenObject = true; + return true; + } + return unionTypeParts(constraint).every(isSpreadable); + } + return isFalsyType(t); + }; + return unionTypeParts(type).every(isSpreadable) && seenObject; } } diff --git a/packages/mimir/src/rules/prefer-spread-arguments.ts b/packages/mimir/src/rules/prefer-spread-arguments.ts new file mode 100644 index 000000000..ee1ec41e9 --- /dev/null +++ b/packages/mimir/src/rules/prefer-spread-arguments.ts @@ -0,0 +1,105 @@ +import { TypedRule, excludeDeclarationFiles, requiresCompilerOption, Replacement } from '@fimbul/ymir'; +import * as ts from 'typescript'; +import { + isAssertionExpression, + isElementAccessExpression, + isIdentifier, + isInterfaceDeclaration, + isMethodSignature, + isNonNullExpression, + isParenthesizedExpression, + isPropertyAccessExpression, + isSourceFile, + isTextualLiteral, +} from 'tsutils'; +import { findMethodCalls, isUndefined } from '../utils'; + +@excludeDeclarationFiles +@requiresCompilerOption('strictBindCallApply') +export class Rule extends TypedRule { + public apply() { + for (const node of findMethodCalls(this.context, 'apply')) + if ( + node.arguments.length === 2 && + node.arguments[1].kind !== ts.SyntaxKind.SpreadElement && + receiverMatches(node.expression.expression, node.arguments[0]) && + this.isFunctionPrototypeApply(node.expression.name) + ) + this.addFindingAtNode( + node, + `Prefer spread arguments over 'Function.prototype.apply'.`, + // replace '.apply(thisArg, ' with '(...' + Replacement.replace( + // preserve optional chain: 'foo?.apply(null, args)' -> 'foo?.(...args)' + node.expression.questionDotToken ? node.expression.name.pos : node.expression.expression.end, + node.arguments[1].getStart(this.sourceFile), + '(...', + ), + ); + } + + private isFunctionPrototypeApply(methodName: ts.Identifier): boolean { + const symbol = this.checker.getSymbolAtLocation(methodName); + return symbol !== undefined && + symbol.valueDeclaration !== undefined && + isMethodSignature(symbol.valueDeclaration) && + isInterfaceDeclaration(symbol.valueDeclaration.parent!) && + symbol.valueDeclaration.parent.name.text === 'CallableFunction' && + isSourceFile(symbol.valueDeclaration.parent!.parent!) && + symbol.valueDeclaration.parent!.parent!.hasNoDefaultLib; + } +} + +function receiverMatches(callee: ts.Expression, arg0: ts.Expression): boolean { + callee = unwrapParenthesesAndAssertions(callee); // foo!.apply(null, args) + if (isIdentifier(callee)) // foo.apply(null, args) + return arg0.kind === ts.SyntaxKind.NullKeyword || isUndefined(arg0); + return (isPropertyAccessExpression(callee) || isElementAccessExpression(callee)) && isMatchingReference(callee.expression, arg0); +} + +function isMatchingReference(a: ts.Expression, b: ts.Expression): boolean { + a = unwrapParenthesesAndAssertions(a); + b = unwrapParenthesesAndAssertions(b); + if (isIdentifier(a)) // foo.bar.apply(foo, args) + return isIdentifier(b) && a.escapedText === b.escapedText; + if (a.kind === ts.SyntaxKind.ThisKeyword || a.kind === ts.SyntaxKind.SuperKeyword) + return b.kind === ts.SyntaxKind.ThisKeyword; // this.foo.apply(this, args) and super.foo.apply(this, args) + if (isPropertyAccessExpression(a)) + return isPropertyAccessExpression(b) // foo.bar.baz(foo.bar, args) + ? a.name.escapedText === b.name.escapedText && isMatchingReference(a.expression, b.expression) + : isElementAccessExpression(b) && // foo.bar.baz(foo['bar'], args) + a.name.kind === ts.SyntaxKind.Identifier && + isTextualLiteral(b.argumentExpression) && + a.name.text === b.argumentExpression.text && + isMatchingReference(a.expression, b.expression); + if (isElementAccessExpression(a)) + return isPropertyAccessExpression(b) // foo['bar'].baz.apply(foo.bar, args) + ? b.name.kind === ts.SyntaxKind.Identifier && + isTextualLiteral(a.argumentExpression) && + b.name.text === a.argumentExpression.text && + isMatchingReference(a.expression, b.expression) + : isElementAccessExpression(b) && // foo['bar'].baz.apply(foo['bar'], args) + ( + isTextualOrNumericLiteral(a.argumentExpression) // foo['bar'].baz.apply(foo['bar'], args) + ? isTextualOrNumericLiteral(b.argumentExpression) && a.argumentExpression.text === b.argumentExpression.text + : isMatchingReference(a.argumentExpression, b.argumentExpression) // arr[i].fn.apply(arr[i], args) + ) && + isMatchingReference(a.expression, b.expression); + return false; +} + +function unwrapParenthesesAndAssertions(node: ts.Expression) { + while (isParenthesizedExpression(node) || isAssertionExpression(node) || isNonNullExpression(node)) + node = node.expression; + return node; +} + +function isTextualOrNumericLiteral(node: ts.Expression): node is ts.StringLiteralLike | ts.NumericLiteral { + switch (node.kind) { + case ts.SyntaxKind.StringLiteral: + case ts.SyntaxKind.NoSubstitutionTemplateLiteral: + case ts.SyntaxKind.NumericLiteral: + return true; + } + return false; +} diff --git a/packages/mimir/src/utils.ts b/packages/mimir/src/utils.ts index 73b8846a9..304f87cc1 100644 --- a/packages/mimir/src/utils.ts +++ b/packages/mimir/src/utils.ts @@ -9,6 +9,9 @@ import { getPropertyOfType, getLateBoundPropertyNames, PropertyName, + isIdentifier, + isCallExpression, + isPropertyAccessExpression, } from 'tsutils'; import { RuleContext } from '@fimbul/ymir'; @@ -34,6 +37,26 @@ export function* tryStatements(context: RuleContext) { } } +/** + * Finds all CallExpressions where callee is a PropertyAccessExpression with name `methodName`. + * Does not find ElementAccessExpression or optional CallExpression. + */ +export function* findMethodCalls(context: RuleContext, methodName: string) { + const {text} = context.sourceFile; + const re = new RegExp(`(?:[.\\n]|\\*\\/)\\s*${methodName}\\s*[(\\/]`, 'g'); + let wrappedAst: WrappedAst | undefined; + for (let match = re.exec(text); match !== null; match = re.exec(text)) { + const {node} = getWrappedNodeAtPosition(wrappedAst || (wrappedAst = context.getWrappedAst()), re.lastIndex - 1)!; + if ( + isCallExpression(node) && + match.index < node.expression.end && // prevent duplicates caused by matching comment text + isPropertyAccessExpression(node.expression) && + node.expression.name.text === methodName + ) + yield node; + } +} + export function isAsyncFunction(node: ts.Node): node is ts.FunctionLikeDeclaration & {body: ts.Block} { switch (node.kind) { case ts.SyntaxKind.FunctionDeclaration: @@ -211,3 +234,7 @@ export function hasDirectivePrologue(node: ts.Node): node is ts.BlockLike { export function formatPseudoBigInt(v: ts.PseudoBigInt) { return `${v.negative ? '-' : ''}${v.base10Value}n`; } + +export function isUndefined(node: ts.Expression): node is ts.Identifier { + return isIdentifier(node) && node.originalKeywordKind === ts.SyntaxKind.UndefinedKeyword; +} diff --git a/packages/mimir/test/prefer-object-spread/test.ts b/packages/mimir/test/prefer-object-spread/test.ts index 874af15ba..22f3eb36f 100644 --- a/packages/mimir/test/prefer-object-spread/test.ts +++ b/packages/mimir/test/prefer-object-spread/test.ts @@ -1,8 +1,14 @@ interface Empty {} class Foo { - dontSpreadTypeParam(param: T, param2: T | undefined) { + spreadTypeParam(param: T, param2: T | undefined, param3: U) { Object.assign({}, param); Object.assign({}, param2); + Object.assign({}, param3); + } + dontSpreadPrimitiveTypeParameter(param: T, param2: U, param3: V) { + Object.assign({}, param); + Object.assign({}, param2); + Object.assign({}, param3); } dontSpreadNonObject(param: number | boolean) { return Object.assign({}, param); @@ -27,7 +33,13 @@ class Foo { spreadAny(param: any) { return Object.assign({}, param); } - dontSpreadThis() { + spreadThis() { + return Object.assign({}, this); + } + spreadThisParameter(this: object) { + return Object.assign({}, this); + } + dontSpreadPrimitiveThis(this: number) { return Object.assign({}, this); } spreadUnknown(param: unknown) { diff --git a/packages/mimir/test/prefer-spread-arguments/.wotanrc.yaml b/packages/mimir/test/prefer-spread-arguments/.wotanrc.yaml new file mode 100644 index 000000000..db206d4bb --- /dev/null +++ b/packages/mimir/test/prefer-spread-arguments/.wotanrc.yaml @@ -0,0 +1,2 @@ +rules: + prefer-spread-arguments: error diff --git a/packages/mimir/test/prefer-spread-arguments/default.test.json b/packages/mimir/test/prefer-spread-arguments/default.test.json new file mode 100644 index 000000000..269914ebe --- /dev/null +++ b/packages/mimir/test/prefer-spread-arguments/default.test.json @@ -0,0 +1,3 @@ +{ + "project": "tsconfig.json" +} diff --git a/packages/mimir/test/prefer-spread-arguments/test.ts b/packages/mimir/test/prefer-spread-arguments/test.ts new file mode 100644 index 000000000..a54ef2400 --- /dev/null +++ b/packages/mimir/test/prefer-spread-arguments/test.ts @@ -0,0 +1,82 @@ +export {}; + +declare const args: any[]; + +alert.apply(null, args); +alert.apply(undefined, args); +alert.apply(alert, args); +alert.apply(null, ...[args] as const); +alert.apply(...[null, args] as const); + +declare const fn: ((...args: any[]) => void) | undefined; +fn!.apply(null, args); +fn?.apply(null, args); +(fn as (...args: any[]) => void).apply(null, args); + +declare const obj: {apply(a: any, b: any): any}; +obj.apply(null, args); + +const nested = {obj}; +nested.obj.apply(nested, args); + +console.log.apply(null, args); +console.log.apply(console, args); +console.log.apply(console.log, args); +console.log.apply(obj, args); +console['log'].apply(console, args); +(console['log'])!.apply(console, args); +window.console.log.apply(window.console, args); +window.console.log.apply(window.console!, args); +window.console.log.apply(window['console'], args); +window['console'].log.apply(window.console, args); +window['console'].log.apply(window['console'], args); + +declare let objArr: {fn: (...args: any[]) => void}[]; +declare let i: number; +objArr[0].fn.apply(objArr[0], args); +objArr[0].fn.apply(objArr['0'], args); +objArr[0].fn.apply(objArr[1], args); +objArr[i].fn.apply(objArr[i], args); +objArr[objArr.length].fn.apply(objArr[objArr.length], args); +objArr[objArr.length].fn.apply(objArr[objArr['length']], args); +objArr[objArr.length - 1].fn.apply(objArr[objArr.length - 1], args); + +objArr[Symbol.iterator].apply(objArr, args); + +class C { + #fn!: (...args: any[]) => void; + #obj: {fn: (...args: any[]) => void}; + [key: string]: {fn: (...args: any[]) => void}; + constructor() { + this.#fn.apply(this, args); + this.#fn.apply(new C(), args); + this.#fn.apply(null, args); + this.#obj.fn.apply(this.#obj, args); + this.#obj.fn.apply(this, args); + + this.#obj.fn.apply(this['#obj'], args); + this.#obj.fn.apply(this['obj'], args); + this['#obj'].fn.apply(this.#obj, args); + this['#obj'].fn.apply(this['#obj'], args); + } +} + +class Base { + fn(...args: any[]) {}; +} + +class Derived extends Base { + fn() { + super.fn.apply(this, args); + } +} + +fn.apply /* .apply() */(null /* .apply() */, args); +fn. // + apply +(null, args); + +fn.apply // + (null, args); + +fn.apply // diff --git a/packages/mimir/test/prefer-spread-arguments/tsconfig.json b/packages/mimir/test/prefer-spread-arguments/tsconfig.json new file mode 100644 index 000000000..9a67816a0 --- /dev/null +++ b/packages/mimir/test/prefer-spread-arguments/tsconfig.json @@ -0,0 +1,7 @@ +{ + "compilerOptions": { + "strictBindCallApply": true, + "lib": ["dom"], + "target": "ESNEXT", + } +}