From c378684655dbce4788e965130a2625a91d1bd525 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 16:30:25 +0200 Subject: [PATCH 1/9] add some test cases --- .../default/test.ts.lint | 59 ++++++++++++ .../src/rules/no-useless-object-property.ts | 95 +++++++++++++++++++ .../no-useless-object-property/.wotanrc.yaml | 2 + .../default.test.json | 1 + .../test/no-useless-object-property/test.ts | 52 ++++++++++ .../no-useless-object-property/tsconfig.json | 6 ++ 6 files changed, 215 insertions(+) create mode 100644 baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint create mode 100644 packages/mimir/src/rules/no-useless-object-property.ts create mode 100644 packages/mimir/test/no-useless-object-property/.wotanrc.yaml create mode 100644 packages/mimir/test/no-useless-object-property/default.test.json create mode 100644 packages/mimir/test/no-useless-object-property/test.ts create mode 100644 packages/mimir/test/no-useless-object-property/tsconfig.json diff --git a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint new file mode 100644 index 000000000..653dc2f43 --- /dev/null +++ b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint @@ -0,0 +1,59 @@ +export {}; + +declare function get(): T; + +const foo = 'foo'; + +({ + x: 1, + ~~~~ [error no-useless-object-property: Property is overridden later.] + ...{x: 2, y: 2}, + ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + y: 1, + ...{x: 3}, +}); + +({ + foo, + ~~~ [error no-useless-object-property: Property is overridden later.] + ...{foo}, +}); + +({ + [foo]: 1, + ~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[foo]: 2}, +}); + +({ + [Symbol.iterator]: 1, + ~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[Symbol.iterator]: 2}, +}); + +({ + [get()]: 1, + ...{[get()]: 2}, +}); + +({ + [get<'foo'>()]: 1, + ...{[get<'foo'>()]: 2}, + ~~~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[foo]: 3}, +}); + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + baz: 1, + ...get<{foo?: string, bar: number, baz: any}>(), +}); + +({ + foo: 1, + bar: 1, + baz: 1, + ...get<{foo: string, bar: number} | {bar: number, baz: boolean}>(), +}); diff --git a/packages/mimir/src/rules/no-useless-object-property.ts b/packages/mimir/src/rules/no-useless-object-property.ts new file mode 100644 index 000000000..950ecc731 --- /dev/null +++ b/packages/mimir/src/rules/no-useless-object-property.ts @@ -0,0 +1,95 @@ +import { TypedRule, excludeDeclarationFiles } from '@fimbul/ymir'; +import * as ts from 'typescript'; +import { isReassignmentTarget, getPropertyName, isObjectType, unionTypeParts } from 'tsutils'; + +@excludeDeclarationFiles +export class Rule extends TypedRule { + public apply() { + const checkedObjects = new Set(); + for (const node of this.context.getFlatAst()) { + if ( + node.kind === ts.SyntaxKind.SpreadAssignment && + !checkedObjects.has(node.parent!.pos) && + !isReassignmentTarget(node.parent) + ) { + checkedObjects.add(node.parent!.pos); + this.checkObject(node.parent); + } + } + } + + private checkObject({properties}: ts.ObjectLiteralExpression) { + const propertiesSeen = new Set(); + for (let i = properties.length - 1; i >= 0; --i) { + const info = this.getPropertyInfo(properties[i]); + if (info.known && info.names.every((name) => propertiesSeen.has(name))) + this.addFailureAtNode(properties[i], 'Property is overridden later.'); + for (const name of info.assignedNames) + propertiesSeen.add(name); + } + } + + private getPropertyInfo(property: ts.ObjectLiteralElementLike): PropertyInfo { + switch (property.kind) { + case ts.SyntaxKind.SpreadAssignment: + return this.getPropertyInfoFromSpread(property.expression); + case ts.SyntaxKind.ShorthandPropertyAssignment: + return { + known: true, + names: [property.name.text], + assignedNames: [property.name.text], + }; + default: { + const name = getPropertyName(property.name) || + getNameOfSymbol(this.checker.getSymbolAtLocation(property.name)); + if (name === undefined) + return { + known: false, + names: [], + assignedNames: [], + }; + return { + known: true, + names: [name], + assignedNames: [name], + }; + } + } + } + + private getPropertyInfoFromSpread(node: ts.Expression): PropertyInfo { + const type = this.checker.getTypeAtLocation(node); + if (!isObjectType(type)) + return { + known: false, + names: [], + assignedNames: [], + }; + const result: PropertyInfo = { + known: (type.flags & ts.TypeFlags.Any) !== 0 || + type.getStringIndexType() === undefined && type.getNumberIndexType() === undefined, + names: [], + assignedNames: [], + }; + for (const prop of type.getProperties()) { + if (!unionTypeParts(this.checker.getTypeOfSymbolAtLocation(prop, node)).some(isOptionalType)) + result.assignedNames.push(prop.name); + result.names.push(prop.name); + } + return result; + } +} + +function isOptionalType(type: ts.Type) { + return (type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Any)) !== 0; +} + +function getNameOfSymbol(symbol: ts.Symbol | undefined) { + return symbol && symbol.name; +} + +interface PropertyInfo { + known: boolean; + names: string[]; + assignedNames: string[]; +} diff --git a/packages/mimir/test/no-useless-object-property/.wotanrc.yaml b/packages/mimir/test/no-useless-object-property/.wotanrc.yaml new file mode 100644 index 000000000..4792501a3 --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/.wotanrc.yaml @@ -0,0 +1,2 @@ +rules: + no-useless-object-property: error diff --git a/packages/mimir/test/no-useless-object-property/default.test.json b/packages/mimir/test/no-useless-object-property/default.test.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/default.test.json @@ -0,0 +1 @@ +{} diff --git a/packages/mimir/test/no-useless-object-property/test.ts b/packages/mimir/test/no-useless-object-property/test.ts new file mode 100644 index 000000000..86d074d21 --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/test.ts @@ -0,0 +1,52 @@ +export {}; + +declare function get(): T; + +const foo = 'foo'; + +({ + x: 1, + ...{x: 2, y: 2}, + y: 1, + ...{x: 3}, +}); + +({ + foo, + ...{foo}, +}); + +({ + [foo]: 1, + ...{[foo]: 2}, +}); + +({ + [Symbol.iterator]: 1, + ...{[Symbol.iterator]: 2}, +}); + +({ + [get()]: 1, + ...{[get()]: 2}, +}); + +({ + [get<'foo'>()]: 1, + ...{[get<'foo'>()]: 2}, + ...{[foo]: 3}, +}); + +({ + foo: 1, + bar: 1, + baz: 1, + ...get<{foo?: string, bar: number, baz: any}>(), +}); + +({ + foo: 1, + bar: 1, + baz: 1, + ...get<{foo: string, bar: number} | {bar: number, baz: boolean}>(), +}); diff --git a/packages/mimir/test/no-useless-object-property/tsconfig.json b/packages/mimir/test/no-useless-object-property/tsconfig.json new file mode 100644 index 000000000..3338b50e9 --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "strict": true, + "target": "esnext" + } +} From 613a69b427691992753a3c265bd6d60d759210d2 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 20:14:15 +0200 Subject: [PATCH 2/9] make it work with unions and well known symbols --- .../default/test.ts.lint | 5 ++- .../src/rules/no-useless-object-property.ts | 40 ++++++++++++------- .../test/no-useless-object-property/test.ts | 4 +- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint index 653dc2f43..ae5bb2c71 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint @@ -26,6 +26,7 @@ const foo = 'foo'; }); ({ + '__@iterator': 1, [Symbol.iterator]: 1, ~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] ...{[Symbol.iterator]: 2}, @@ -54,6 +55,8 @@ const foo = 'foo'; ({ foo: 1, bar: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] baz: 1, - ...get<{foo: string, bar: number} | {bar: number, baz: boolean}>(), + bas: 1, + ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), }); diff --git a/packages/mimir/src/rules/no-useless-object-property.ts b/packages/mimir/src/rules/no-useless-object-property.ts index 950ecc731..4e24530c0 100644 --- a/packages/mimir/src/rules/no-useless-object-property.ts +++ b/packages/mimir/src/rules/no-useless-object-property.ts @@ -1,6 +1,6 @@ import { TypedRule, excludeDeclarationFiles } from '@fimbul/ymir'; import * as ts from 'typescript'; -import { isReassignmentTarget, getPropertyName, isObjectType, unionTypeParts } from 'tsutils'; +import { isReassignmentTarget, isObjectType, unionTypeParts } from 'tsutils'; @excludeDeclarationFiles export class Rule extends TypedRule { @@ -19,7 +19,7 @@ export class Rule extends TypedRule { } private checkObject({properties}: ts.ObjectLiteralExpression) { - const propertiesSeen = new Set(); + const propertiesSeen = new Set(); for (let i = properties.length - 1; i >= 0; --i) { const info = this.getPropertyInfo(properties[i]); if (info.known && info.names.every((name) => propertiesSeen.has(name))) @@ -36,12 +36,11 @@ export class Rule extends TypedRule { case ts.SyntaxKind.ShorthandPropertyAssignment: return { known: true, - names: [property.name.text], - assignedNames: [property.name.text], + names: [property.name.escapedText], + assignedNames: [property.name.escapedText], }; default: { - const name = getPropertyName(property.name) || - getNameOfSymbol(this.checker.getSymbolAtLocation(property.name)); + const name = this.getPropertyName(property.name); if (name === undefined) return { known: false, @@ -57,8 +56,17 @@ export class Rule extends TypedRule { } } + private getPropertyName(node: ts.PropertyName): ts.__String | undefined { + const symbol = this.checker.getSymbolAtLocation(node); + return symbol && symbol.escapedName; + } + private getPropertyInfoFromSpread(node: ts.Expression): PropertyInfo { const type = this.checker.getTypeAtLocation(node); + return unionTypeParts(type).map((t) => this.getPropertyInfoFromType(t, node)).reduce(combinePropertyInfo); + } + + private getPropertyInfoFromType(type: ts.Type, node: ts.Expression): PropertyInfo { if (!isObjectType(type)) return { known: false, @@ -73,23 +81,27 @@ export class Rule extends TypedRule { }; for (const prop of type.getProperties()) { if (!unionTypeParts(this.checker.getTypeOfSymbolAtLocation(prop, node)).some(isOptionalType)) - result.assignedNames.push(prop.name); - result.names.push(prop.name); + result.assignedNames.push(prop.escapedName); + result.names.push(prop.escapedName); } return result; } } -function isOptionalType(type: ts.Type) { - return (type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Any)) !== 0; +function combinePropertyInfo(a: PropertyInfo, b: PropertyInfo): PropertyInfo { + return { + known: a.known && b.known, + names: [...a.names, ...b.names], + assignedNames: a.assignedNames.filter((name) => b.assignedNames.includes(name)), + }; } -function getNameOfSymbol(symbol: ts.Symbol | undefined) { - return symbol && symbol.name; +function isOptionalType(type: ts.Type) { + return (type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Any)) !== 0; } interface PropertyInfo { known: boolean; - names: string[]; - assignedNames: string[]; + names: ts.__String[]; + assignedNames: ts.__String[]; } diff --git a/packages/mimir/test/no-useless-object-property/test.ts b/packages/mimir/test/no-useless-object-property/test.ts index 86d074d21..3b26033dd 100644 --- a/packages/mimir/test/no-useless-object-property/test.ts +++ b/packages/mimir/test/no-useless-object-property/test.ts @@ -22,6 +22,7 @@ const foo = 'foo'; }); ({ + '__@iterator': 1, [Symbol.iterator]: 1, ...{[Symbol.iterator]: 2}, }); @@ -48,5 +49,6 @@ const foo = 'foo'; foo: 1, bar: 1, baz: 1, - ...get<{foo: string, bar: number} | {bar: number, baz: boolean}>(), + bas: 1, + ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), }); From f2ed567055f0e284275584d06a64dc032b6061e5 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 20:29:30 +0200 Subject: [PATCH 3/9] add baselines for older typescript versions --- .../pre260/test.ts.lint | 56 +++++++++++++++++ .../ts260/test.ts.lint | 60 +++++++++++++++++++ .../default.test.json | 4 +- .../pre260.test.json | 3 + .../ts260.test.json | 3 + 5 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint create mode 100644 baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint create mode 100644 packages/mimir/test/no-useless-object-property/pre260.test.json create mode 100644 packages/mimir/test/no-useless-object-property/ts260.test.json diff --git a/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint new file mode 100644 index 000000000..c8dafd55e --- /dev/null +++ b/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint @@ -0,0 +1,56 @@ +export {}; + +declare function get(): T; + +const foo = 'foo'; + +({ + x: 1, + ...{x: 2, y: 2}, + ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + y: 1, + ...{x: 3}, +}); + +({ + foo, + ~~~ [error no-useless-object-property: Property is overridden later.] + ...{foo}, +}); + +({ + [foo]: 1, + ...{[foo]: 2}, +}); + +({ + '__@iterator': 1, + [Symbol.iterator]: 1, + ...{[Symbol.iterator]: 2}, +}); + +({ + [get()]: 1, + ...{[get()]: 2}, +}); + +({ + [get<'foo'>()]: 1, + ...{[get<'foo'>()]: 2}, + ...{[foo]: 3}, +}); + +({ + foo: 1, + bar: 1, + baz: 1, + ...get<{foo?: string, bar: number, baz: any}>(), +}); + +({ + foo: 1, + bar: 1, + baz: 1, + bas: 1, + ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), +}); diff --git a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint new file mode 100644 index 000000000..b952a5958 --- /dev/null +++ b/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint @@ -0,0 +1,60 @@ +export {}; + +declare function get(): T; + +const foo = 'foo'; + +({ + x: 1, + ~~~~ [error no-useless-object-property: Property is overridden later.] + ...{x: 2, y: 2}, + ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + y: 1, + ...{x: 3}, +}); + +({ + foo, + ~~~ [error no-useless-object-property: Property is overridden later.] + ...{foo}, +}); + +({ + [foo]: 1, + ...{[foo]: 2}, +}); + +({ + '__@iterator': 1, + [Symbol.iterator]: 1, + ...{[Symbol.iterator]: 2}, +}); + +({ + [get()]: 1, + ...{[get()]: 2}, +}); + +({ + [get<'foo'>()]: 1, + ...{[get<'foo'>()]: 2}, + ~~~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[foo]: 3}, +}); + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + baz: 1, + ...get<{foo?: string, bar: number, baz: any}>(), +}); + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + baz: 1, + bas: 1, + ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), +}); diff --git a/packages/mimir/test/no-useless-object-property/default.test.json b/packages/mimir/test/no-useless-object-property/default.test.json index 0967ef424..92b4d9e1c 100644 --- a/packages/mimir/test/no-useless-object-property/default.test.json +++ b/packages/mimir/test/no-useless-object-property/default.test.json @@ -1 +1,3 @@ -{} +{ + "typescriptVersion": ">= 2.7.0" +} diff --git a/packages/mimir/test/no-useless-object-property/pre260.test.json b/packages/mimir/test/no-useless-object-property/pre260.test.json new file mode 100644 index 000000000..44754ce9a --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/pre260.test.json @@ -0,0 +1,3 @@ +{ + "typescriptVersion": "<2.6.0" +} diff --git a/packages/mimir/test/no-useless-object-property/ts260.test.json b/packages/mimir/test/no-useless-object-property/ts260.test.json new file mode 100644 index 000000000..3ab0610ff --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/ts260.test.json @@ -0,0 +1,3 @@ +{ + "typescriptVersion": "~2.6.0" +} From 88a26a84ac42337293ea747cdbeaa6df9c759ef6 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 20:31:58 +0200 Subject: [PATCH 4/9] Add docs --- packages/mimir/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mimir/README.md b/packages/mimir/README.md index f7d659eb2..b5f7da1cb 100644 --- a/packages/mimir/README.md +++ b/packages/mimir/README.md @@ -41,6 +41,7 @@ Rule | Description | Difference to TSLint rule / Why you should use it `no-useless-assertion` | Detects type assertions that don't change the type or are not necessary in the first place. *requires type information* | TSLint's `no-unnecessary-type-assertion` does not detect assertions needed to silence the compiler warning `Variable ... is used before being assigned.` The Wotan builtin rule also checks whether the assertion is necessary at all or the receiver accepts the original type. `no-useless-initializer` | Detects unnecessary initialization with `undefined` and destructuring defaults (*requires type information*). | TSLint's rule `no-unnecessary-initializer` doesn't fix all parameter initializers and gives false positives for destructuring. `no-useless-jump-label` | Detects `continue label;` and `break label;` where the label is not necessary. | There's no similar TSLint rule. +`no-useless-object-property` | Detects properties in object literals that are overridden by a spreaded object. | TSLint has no such rule. `no-useless-predicate` | Detects redundant conditions that are either always true or always false. *requires type information* | Combination of TSLint's `strict-type-predicates`, `typeof-compare` and parts of `strict-boolean-expressions`. `no-useless-spread` | Detects redundant array and object spread which can safely be removed. | There's no similar TSLint rule. `prefer-const` | Prefer `const` for variables that are never reassigned. Use option `{destructuring: "any"}` if you want to see failures for each identifier of a destructuring, even if not all of them can be constants. The default is `{destructuring: "all"}`. | TSLint's `prefer-const` rule gives some false positives for merged declarations and variables used in before being declared which results in a compiler error after fixing. From 198ae2321451b462869cfa282319b1256567e34d Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 20:57:29 +0200 Subject: [PATCH 5/9] some cleanup and fixup --- .../default/test.ts.lint | 4 +- .../loose/test.ts.lint | 64 +++++++++++++++ .../pre260/test.ts.lint | 5 +- .../ts260/test.ts.lint | 4 +- packages/mimir/recommended.yaml | 1 + .../src/rules/no-useless-object-property.ts | 81 +++++++++---------- .../loose.test.json | 4 + .../test/no-useless-object-property/test.ts | 3 +- .../tsconfig-loose.json | 6 ++ 9 files changed, 122 insertions(+), 50 deletions(-) create mode 100644 baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint create mode 100644 packages/mimir/test/no-useless-object-property/loose.test.json create mode 100644 packages/mimir/test/no-useless-object-property/tsconfig-loose.json diff --git a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint index ae5bb2c71..9cbed06de 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint @@ -49,7 +49,8 @@ const foo = 'foo'; bar: 1, ~~~~~~ [error no-useless-object-property: Property is overridden later.] baz: 1, - ...get<{foo?: string, bar: number, baz: any}>(), + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ @@ -59,4 +60,5 @@ const foo = 'foo'; baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), + ...Boolean() && {foo}, }); diff --git a/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint new file mode 100644 index 000000000..9cbed06de --- /dev/null +++ b/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint @@ -0,0 +1,64 @@ +export {}; + +declare function get(): T; + +const foo = 'foo'; + +({ + x: 1, + ~~~~ [error no-useless-object-property: Property is overridden later.] + ...{x: 2, y: 2}, + ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + y: 1, + ...{x: 3}, +}); + +({ + foo, + ~~~ [error no-useless-object-property: Property is overridden later.] + ...{foo}, +}); + +({ + [foo]: 1, + ~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[foo]: 2}, +}); + +({ + '__@iterator': 1, + [Symbol.iterator]: 1, + ~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[Symbol.iterator]: 2}, +}); + +({ + [get()]: 1, + ...{[get()]: 2}, +}); + +({ + [get<'foo'>()]: 1, + ...{[get<'foo'>()]: 2}, + ~~~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...{[foo]: 3}, +}); + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + baz: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), +}); + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + baz: 1, + bas: 1, + ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), + ...Boolean() && {foo}, +}); diff --git a/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint index c8dafd55e..2691534a3 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint @@ -7,14 +7,12 @@ const foo = 'foo'; ({ x: 1, ...{x: 2, y: 2}, - ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] y: 1, ...{x: 3}, }); ({ foo, - ~~~ [error no-useless-object-property: Property is overridden later.] ...{foo}, }); @@ -44,7 +42,7 @@ const foo = 'foo'; foo: 1, bar: 1, baz: 1, - ...get<{foo?: string, bar: number, baz: any}>(), + ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ @@ -53,4 +51,5 @@ const foo = 'foo'; baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), + ...Boolean() && {foo}, }); diff --git a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint index b952a5958..3c1668ec7 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint @@ -47,7 +47,8 @@ const foo = 'foo'; bar: 1, ~~~~~~ [error no-useless-object-property: Property is overridden later.] baz: 1, - ...get<{foo?: string, bar: number, baz: any}>(), + ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ @@ -57,4 +58,5 @@ const foo = 'foo'; baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), + ...Boolean() && {foo}, }); diff --git a/packages/mimir/recommended.yaml b/packages/mimir/recommended.yaml index 2d990e8a1..74ec4a7c2 100644 --- a/packages/mimir/recommended.yaml +++ b/packages/mimir/recommended.yaml @@ -21,6 +21,7 @@ rules: no-useless-assertion: error no-useless-initializer: error no-useless-jump-label: error + no-useless-object-property: error no-useless-predicate: error no-useless-spread: error prefer-const: error diff --git a/packages/mimir/src/rules/no-useless-object-property.ts b/packages/mimir/src/rules/no-useless-object-property.ts index 4e24530c0..ca7b56683 100644 --- a/packages/mimir/src/rules/no-useless-object-property.ts +++ b/packages/mimir/src/rules/no-useless-object-property.ts @@ -2,8 +2,24 @@ import { TypedRule, excludeDeclarationFiles } from '@fimbul/ymir'; import * as ts from 'typescript'; import { isReassignmentTarget, isObjectType, unionTypeParts } from 'tsutils'; +interface PropertyInfo { + known: boolean; + names: ts.__String[]; + assignedNames: ts.__String[]; +} + +const emptyPropertyInfo: PropertyInfo = { + known: false, + names: [], + assignedNames: [], +}; + @excludeDeclarationFiles export class Rule extends TypedRule { + public static supports() { + return !ts.version.startsWith('2.4.'); + } + public apply() { const checkedObjects = new Set(); for (const node of this.context.getFlatAst()) { @@ -40,52 +56,39 @@ export class Rule extends TypedRule { assignedNames: [property.name.escapedText], }; default: { - const name = this.getPropertyName(property.name); - if (name === undefined) - return { - known: false, - names: [], - assignedNames: [], - }; + const symbol = this.checker.getSymbolAtLocation(property.name); + if (symbol === undefined) + return emptyPropertyInfo; return { known: true, - names: [name], - assignedNames: [name], + names: [symbol.escapedName], + assignedNames: [symbol.escapedName], }; } } } - private getPropertyName(node: ts.PropertyName): ts.__String | undefined { - const symbol = this.checker.getSymbolAtLocation(node); - return symbol && symbol.escapedName; - } - private getPropertyInfoFromSpread(node: ts.Expression): PropertyInfo { const type = this.checker.getTypeAtLocation(node); - return unionTypeParts(type).map((t) => this.getPropertyInfoFromType(t, node)).reduce(combinePropertyInfo); + return unionTypeParts(type).map(getPropertyInfoFromType).reduce(combinePropertyInfo); } - private getPropertyInfoFromType(type: ts.Type, node: ts.Expression): PropertyInfo { - if (!isObjectType(type)) - return { - known: false, - names: [], - assignedNames: [], - }; - const result: PropertyInfo = { - known: (type.flags & ts.TypeFlags.Any) !== 0 || - type.getStringIndexType() === undefined && type.getNumberIndexType() === undefined, - names: [], - assignedNames: [], - }; - for (const prop of type.getProperties()) { - if (!unionTypeParts(this.checker.getTypeOfSymbolAtLocation(prop, node)).some(isOptionalType)) - result.assignedNames.push(prop.escapedName); - result.names.push(prop.escapedName); - } - return result; +} +function getPropertyInfoFromType(type: ts.Type): PropertyInfo { + if (!isObjectType(type)) + return emptyPropertyInfo; + const result: PropertyInfo = { + known: (type.flags & ts.TypeFlags.Any) !== 0 || + type.getStringIndexType() === undefined && type.getNumberIndexType() === undefined, + names: [], + assignedNames: [], + }; + for (const prop of type.getProperties()) { + if ((prop.flags & ts.SymbolFlags.Optional) === 0) + result.assignedNames.push(prop.escapedName); + result.names.push(prop.escapedName); } + return result; } function combinePropertyInfo(a: PropertyInfo, b: PropertyInfo): PropertyInfo { @@ -95,13 +98,3 @@ function combinePropertyInfo(a: PropertyInfo, b: PropertyInfo): PropertyInfo { assignedNames: a.assignedNames.filter((name) => b.assignedNames.includes(name)), }; } - -function isOptionalType(type: ts.Type) { - return (type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Any)) !== 0; -} - -interface PropertyInfo { - known: boolean; - names: ts.__String[]; - assignedNames: ts.__String[]; -} diff --git a/packages/mimir/test/no-useless-object-property/loose.test.json b/packages/mimir/test/no-useless-object-property/loose.test.json new file mode 100644 index 000000000..92781ebd9 --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/loose.test.json @@ -0,0 +1,4 @@ +{ + "typescriptVersion": ">= 2.7.0", + "project": "tsconfig-loose.json" +} diff --git a/packages/mimir/test/no-useless-object-property/test.ts b/packages/mimir/test/no-useless-object-property/test.ts index 3b26033dd..2691534a3 100644 --- a/packages/mimir/test/no-useless-object-property/test.ts +++ b/packages/mimir/test/no-useless-object-property/test.ts @@ -42,7 +42,7 @@ const foo = 'foo'; foo: 1, bar: 1, baz: 1, - ...get<{foo?: string, bar: number, baz: any}>(), + ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ @@ -51,4 +51,5 @@ const foo = 'foo'; baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), + ...Boolean() && {foo}, }); diff --git a/packages/mimir/test/no-useless-object-property/tsconfig-loose.json b/packages/mimir/test/no-useless-object-property/tsconfig-loose.json new file mode 100644 index 000000000..04608d4ca --- /dev/null +++ b/packages/mimir/test/no-useless-object-property/tsconfig-loose.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "strict": false, + "target": "esnext" + } +} From 91df02a4f8a48d299e7d9dff3eae5b5107d69057 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 21:33:09 +0200 Subject: [PATCH 6/9] require strictNullChecks --- .../test/no-useless-object-property/loose/test.ts.lint | 9 --------- packages/mimir/src/rules/no-useless-object-property.ts | 7 ++++--- .../test/no-useless-object-property/loose.test.json | 1 - 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint index 9cbed06de..2691534a3 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint @@ -6,29 +6,24 @@ const foo = 'foo'; ({ x: 1, - ~~~~ [error no-useless-object-property: Property is overridden later.] ...{x: 2, y: 2}, - ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] y: 1, ...{x: 3}, }); ({ foo, - ~~~ [error no-useless-object-property: Property is overridden later.] ...{foo}, }); ({ [foo]: 1, - ~~~~~~~~ [error no-useless-object-property: Property is overridden later.] ...{[foo]: 2}, }); ({ '__@iterator': 1, [Symbol.iterator]: 1, - ~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] ...{[Symbol.iterator]: 2}, }); @@ -40,23 +35,19 @@ const foo = 'foo'; ({ [get<'foo'>()]: 1, ...{[get<'foo'>()]: 2}, - ~~~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] ...{[foo]: 3}, }); ({ foo: 1, bar: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] baz: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ foo: 1, bar: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), diff --git a/packages/mimir/src/rules/no-useless-object-property.ts b/packages/mimir/src/rules/no-useless-object-property.ts index ca7b56683..18e7bd44a 100644 --- a/packages/mimir/src/rules/no-useless-object-property.ts +++ b/packages/mimir/src/rules/no-useless-object-property.ts @@ -1,6 +1,7 @@ -import { TypedRule, excludeDeclarationFiles } from '@fimbul/ymir'; +import { TypedRule, excludeDeclarationFiles, RuleSupportsContext } from '@fimbul/ymir'; import * as ts from 'typescript'; import { isReassignmentTarget, isObjectType, unionTypeParts } from 'tsutils'; +import { isStrictNullChecksEnabled } from '../utils'; interface PropertyInfo { known: boolean; @@ -16,8 +17,8 @@ const emptyPropertyInfo: PropertyInfo = { @excludeDeclarationFiles export class Rule extends TypedRule { - public static supports() { - return !ts.version.startsWith('2.4.'); + public static supports(_sourceFile: ts.SourceFile, context: RuleSupportsContext) { + return !ts.version.startsWith('2.4.') && isStrictNullChecksEnabled(context.program!.getCompilerOptions()); } public apply() { diff --git a/packages/mimir/test/no-useless-object-property/loose.test.json b/packages/mimir/test/no-useless-object-property/loose.test.json index 92781ebd9..44860f0aa 100644 --- a/packages/mimir/test/no-useless-object-property/loose.test.json +++ b/packages/mimir/test/no-useless-object-property/loose.test.json @@ -1,4 +1,3 @@ { - "typescriptVersion": ">= 2.7.0", "project": "tsconfig-loose.json" } From 65af5a54c4d0f519e61ad73e8d2617be82dbf716 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 12 Apr 2018 21:43:45 +0200 Subject: [PATCH 7/9] add test for destructuring --- .../test/no-useless-object-property/default/test.ts.lint | 5 +++++ .../mimir/test/no-useless-object-property/loose/test.ts.lint | 5 +++++ .../test/no-useless-object-property/pre260/test.ts.lint | 5 +++++ .../mimir/test/no-useless-object-property/ts260/test.ts.lint | 5 +++++ packages/mimir/test/no-useless-object-property/test.ts | 5 +++++ 5 files changed, 25 insertions(+) diff --git a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint index 9cbed06de..87454e10c 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint @@ -62,3 +62,8 @@ const foo = 'foo'; ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), ...Boolean() && {foo}, }); + +{ + let a, b; + ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); +} diff --git a/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint index 2691534a3..44944bb71 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint @@ -53,3 +53,8 @@ const foo = 'foo'; ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), ...Boolean() && {foo}, }); + +{ + let a, b; + ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); +} diff --git a/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint index 2691534a3..44944bb71 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint @@ -53,3 +53,8 @@ const foo = 'foo'; ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), ...Boolean() && {foo}, }); + +{ + let a, b; + ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); +} diff --git a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint b/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint index 3c1668ec7..f1465aab3 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint +++ b/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint @@ -60,3 +60,8 @@ const foo = 'foo'; ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), ...Boolean() && {foo}, }); + +{ + let a, b; + ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); +} diff --git a/packages/mimir/test/no-useless-object-property/test.ts b/packages/mimir/test/no-useless-object-property/test.ts index 2691534a3..44944bb71 100644 --- a/packages/mimir/test/no-useless-object-property/test.ts +++ b/packages/mimir/test/no-useless-object-property/test.ts @@ -53,3 +53,8 @@ const foo = 'foo'; ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), ...Boolean() && {foo}, }); + +{ + let a, b; + ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); +} From 6821e403fce6819d210040022a792886fce628b9 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 26 Apr 2018 22:14:25 +0200 Subject: [PATCH 8/9] rename all the things --- .../default/test.ts.lint | 18 +++++++++--------- .../loose/test.ts.lint | 0 .../pre260/test.ts.lint | 0 .../ts260/test.ts.lint | 14 +++++++------- packages/mimir/README.md | 2 +- packages/mimir/recommended.yaml | 2 +- ...erty.ts => no-duplicate-spread-property.ts} | 11 ++++++++++- .../no-duplicate-spread-property/.wotanrc.yaml | 2 ++ .../default.test.json | 0 .../loose.test.json | 0 .../pre260.test.json | 0 .../test.ts | 0 .../ts260.test.json | 0 .../tsconfig-loose.json | 0 .../tsconfig.json | 0 .../no-useless-object-property/.wotanrc.yaml | 2 -- 16 files changed, 30 insertions(+), 21 deletions(-) rename baselines/packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/default/test.ts.lint (55%) rename baselines/packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/loose/test.ts.lint (100%) rename baselines/packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/pre260/test.ts.lint (100%) rename baselines/packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/ts260/test.ts.lint (58%) rename packages/mimir/src/rules/{no-useless-object-property.ts => no-duplicate-spread-property.ts} (88%) create mode 100644 packages/mimir/test/no-duplicate-spread-property/.wotanrc.yaml rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/default.test.json (100%) rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/loose.test.json (100%) rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/pre260.test.json (100%) rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/test.ts (100%) rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/ts260.test.json (100%) rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/tsconfig-loose.json (100%) rename packages/mimir/test/{no-useless-object-property => no-duplicate-spread-property}/tsconfig.json (100%) delete mode 100644 packages/mimir/test/no-useless-object-property/.wotanrc.yaml diff --git a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint similarity index 55% rename from baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint rename to baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint index 87454e10c..d8512b09e 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/default/test.ts.lint +++ b/baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint @@ -6,29 +6,29 @@ const foo = 'foo'; ({ x: 1, - ~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{x: 2, y: 2}, - ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] y: 1, ...{x: 3}, }); ({ foo, - ~~~ [error no-useless-object-property: Property is overridden later.] + ~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{foo}, }); ({ [foo]: 1, - ~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{[foo]: 2}, }); ({ '__@iterator': 1, [Symbol.iterator]: 1, - ~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{[Symbol.iterator]: 2}, }); @@ -40,23 +40,23 @@ const foo = 'foo'; ({ [get<'foo'>()]: 1, ...{[get<'foo'>()]: 2}, - ~~~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{[foo]: 3}, }); ({ foo: 1, bar: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] baz: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ foo: 1, bar: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), diff --git a/baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/loose/test.ts.lint similarity index 100% rename from baselines/packages/mimir/test/no-useless-object-property/loose/test.ts.lint rename to baselines/packages/mimir/test/no-duplicate-spread-property/loose/test.ts.lint diff --git a/baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/pre260/test.ts.lint similarity index 100% rename from baselines/packages/mimir/test/no-useless-object-property/pre260/test.ts.lint rename to baselines/packages/mimir/test/no-duplicate-spread-property/pre260/test.ts.lint diff --git a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint similarity index 58% rename from baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint rename to baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint index f1465aab3..df99cc57e 100644 --- a/baselines/packages/mimir/test/no-useless-object-property/ts260/test.ts.lint +++ b/baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint @@ -6,16 +6,16 @@ const foo = 'foo'; ({ x: 1, - ~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{x: 2, y: 2}, - ~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] y: 1, ...{x: 3}, }); ({ foo, - ~~~ [error no-useless-object-property: Property is overridden later.] + ~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{foo}, }); @@ -38,23 +38,23 @@ const foo = 'foo'; ({ [get<'foo'>()]: 1, ...{[get<'foo'>()]: 2}, - ~~~~~~~~~~~~~~~~~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...{[foo]: 3}, }); ({ foo: 1, bar: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] baz: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] ...get<{foo?: string, bar: number, baz: boolean | undefined}>(), }); ({ foo: 1, bar: 1, - ~~~~~~ [error no-useless-object-property: Property is overridden later.] + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] baz: 1, bas: 1, ...get<{foo: string, bar: number, bas: number} | {bar: number, baz: boolean, bas?: number}>(), diff --git a/packages/mimir/README.md b/packages/mimir/README.md index 645c146b1..a283ce35c 100644 --- a/packages/mimir/README.md +++ b/packages/mimir/README.md @@ -27,6 +27,7 @@ Rule | Description | Difference to TSLint rule / Why you should use it `no-case-declaration` | Disallow `let`, `class` and `enum` in case blocks. These are visible within the whole switch statement body but not defined in other case clauses. The compiler currently doesn't warn about such uses. You should use a block to restrict the scope of the declarations. | TSLint has no similar rule, ESLint has `no-case-declarations` which forbids function declarations as well. `no-debugger` | Ban `debugger;` statements from your production code. | Performance! `no-duplicate-case` | Detects `switch` statements where multiple `case` clauses check for the same value. *uses type information if available* | This implementation tries to infer the value instead of just comparing the source code. +`no-duplicate-spread-property` | Detects properties in object literals that are overridden by a spreaded object. | TSLint has no such rule. `no-fallthrough` | Prevents unintentional fallthough in `switch` statements from one case to another. If the fallthrough is intended, add a comment that matches `/^\s*falls? ?through\b/i`. | Allows more comment variants such as `fallthrough` or `fall through`. `no-inferred-empty-object` | Warns if a type parameter is inferred as `{}` because the compiler cannot find any inference site. *requires type information* | Really checks every type parameter of function, method and constructor calls. Correctly handles type parameters from JSDoc comments. Recognises type parameter defaults on all merged declarations. `no-invalid-assertion` | Disallows asserting a literal type to a different literal type of the same widened type, e.g. `'foo' as 'bar'`. *requires type information* | TSLint has no similar rule. @@ -42,7 +43,6 @@ Rule | Description | Difference to TSLint rule / Why you should use it `no-useless-assertion` | Detects type assertions that don't change the type or are not necessary in the first place. *requires type information* | TSLint's `no-unnecessary-type-assertion` does not detect assertions needed to silence the compiler warning `Variable ... is used before being assigned.` The Wotan builtin rule also checks whether the assertion is necessary at all or the receiver accepts the original type. `no-useless-initializer` | Detects unnecessary initialization with `undefined` and destructuring defaults (*requires type information*). | TSLint's rule `no-unnecessary-initializer` doesn't fix all parameter initializers and gives false positives for destructuring. `no-useless-jump-label` | Detects `continue label;` and `break label;` where the label is not necessary. | There's no similar TSLint rule. -`no-useless-object-property` | Detects properties in object literals that are overridden by a spreaded object. | TSLint has no such rule. `no-useless-predicate` | Detects redundant conditions that are either always true or always false. *requires type information* | Combination of TSLint's `strict-type-predicates`, `typeof-compare` and parts of `strict-boolean-expressions`. `no-useless-spread` | Detects redundant array and object spread which can safely be removed. | There's no similar TSLint rule. `prefer-const` | Prefer `const` for variables that are never reassigned. Use option `{destructuring: "any"}` if you want to see failures for each identifier of a destructuring, even if not all of them can be constants. The default is `{destructuring: "all"}`. | TSLint's `prefer-const` rule gives some false positives for merged declarations and variables used in before being declared which results in a compiler error after fixing. diff --git a/packages/mimir/recommended.yaml b/packages/mimir/recommended.yaml index c082aa11d..c3e0b3702 100644 --- a/packages/mimir/recommended.yaml +++ b/packages/mimir/recommended.yaml @@ -7,6 +7,7 @@ rules: no-case-declaration: error no-debugger: error no-duplicate-case: error + no-duplicate-spread-property: error no-fallthrough: error no-inferred-empty-object: error no-invalid-assertion: error @@ -22,7 +23,6 @@ rules: no-useless-assertion: error no-useless-initializer: error no-useless-jump-label: error - no-useless-object-property: error no-useless-predicate: error no-useless-spread: error prefer-const: error diff --git a/packages/mimir/src/rules/no-useless-object-property.ts b/packages/mimir/src/rules/no-duplicate-spread-property.ts similarity index 88% rename from packages/mimir/src/rules/no-useless-object-property.ts rename to packages/mimir/src/rules/no-duplicate-spread-property.ts index 18e7bd44a..de4533080 100644 --- a/packages/mimir/src/rules/no-useless-object-property.ts +++ b/packages/mimir/src/rules/no-duplicate-spread-property.ts @@ -1,6 +1,6 @@ import { TypedRule, excludeDeclarationFiles, RuleSupportsContext } from '@fimbul/ymir'; import * as ts from 'typescript'; -import { isReassignmentTarget, isObjectType, unionTypeParts } from 'tsutils'; +import { isReassignmentTarget, isObjectType, unionTypeParts, isClassLikeDeclaration } from 'tsutils'; import { isStrictNullChecksEnabled } from '../utils'; interface PropertyInfo { @@ -85,12 +85,21 @@ function getPropertyInfoFromType(type: ts.Type): PropertyInfo { assignedNames: [], }; for (const prop of type.getProperties()) { + if (isClassMethod(prop)) + continue; if ((prop.flags & ts.SymbolFlags.Optional) === 0) result.assignedNames.push(prop.escapedName); result.names.push(prop.escapedName); } return result; } +function isClassMethod(prop: ts.Symbol): boolean | undefined { + if (prop.flags & ts.SymbolFlags.Method && prop.declarations !== undefined) + for (const declaration of prop.declarations) + if (isClassLikeDeclaration(declaration.parent!)) + return true; + return false; +} function combinePropertyInfo(a: PropertyInfo, b: PropertyInfo): PropertyInfo { return { diff --git a/packages/mimir/test/no-duplicate-spread-property/.wotanrc.yaml b/packages/mimir/test/no-duplicate-spread-property/.wotanrc.yaml new file mode 100644 index 000000000..c76b77fcf --- /dev/null +++ b/packages/mimir/test/no-duplicate-spread-property/.wotanrc.yaml @@ -0,0 +1,2 @@ +rules: + no-duplicate-spread-property: error diff --git a/packages/mimir/test/no-useless-object-property/default.test.json b/packages/mimir/test/no-duplicate-spread-property/default.test.json similarity index 100% rename from packages/mimir/test/no-useless-object-property/default.test.json rename to packages/mimir/test/no-duplicate-spread-property/default.test.json diff --git a/packages/mimir/test/no-useless-object-property/loose.test.json b/packages/mimir/test/no-duplicate-spread-property/loose.test.json similarity index 100% rename from packages/mimir/test/no-useless-object-property/loose.test.json rename to packages/mimir/test/no-duplicate-spread-property/loose.test.json diff --git a/packages/mimir/test/no-useless-object-property/pre260.test.json b/packages/mimir/test/no-duplicate-spread-property/pre260.test.json similarity index 100% rename from packages/mimir/test/no-useless-object-property/pre260.test.json rename to packages/mimir/test/no-duplicate-spread-property/pre260.test.json diff --git a/packages/mimir/test/no-useless-object-property/test.ts b/packages/mimir/test/no-duplicate-spread-property/test.ts similarity index 100% rename from packages/mimir/test/no-useless-object-property/test.ts rename to packages/mimir/test/no-duplicate-spread-property/test.ts diff --git a/packages/mimir/test/no-useless-object-property/ts260.test.json b/packages/mimir/test/no-duplicate-spread-property/ts260.test.json similarity index 100% rename from packages/mimir/test/no-useless-object-property/ts260.test.json rename to packages/mimir/test/no-duplicate-spread-property/ts260.test.json diff --git a/packages/mimir/test/no-useless-object-property/tsconfig-loose.json b/packages/mimir/test/no-duplicate-spread-property/tsconfig-loose.json similarity index 100% rename from packages/mimir/test/no-useless-object-property/tsconfig-loose.json rename to packages/mimir/test/no-duplicate-spread-property/tsconfig-loose.json diff --git a/packages/mimir/test/no-useless-object-property/tsconfig.json b/packages/mimir/test/no-duplicate-spread-property/tsconfig.json similarity index 100% rename from packages/mimir/test/no-useless-object-property/tsconfig.json rename to packages/mimir/test/no-duplicate-spread-property/tsconfig.json diff --git a/packages/mimir/test/no-useless-object-property/.wotanrc.yaml b/packages/mimir/test/no-useless-object-property/.wotanrc.yaml deleted file mode 100644 index 4792501a3..000000000 --- a/packages/mimir/test/no-useless-object-property/.wotanrc.yaml +++ /dev/null @@ -1,2 +0,0 @@ -rules: - no-useless-object-property: error From c5a9108c0da0535c7da65d994353e5429b12117d Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 26 Apr 2018 22:24:40 +0200 Subject: [PATCH 9/9] add tests with class methods --- .../default/test.ts.lint | 50 +++++++++++++++++++ .../loose/test.ts.lint | 41 +++++++++++++++ .../pre260/test.ts.lint | 41 +++++++++++++++ .../ts260/test.ts.lint | 50 +++++++++++++++++++ .../test/no-duplicate-spread-property/test.ts | 41 +++++++++++++++ 5 files changed, 223 insertions(+) diff --git a/baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint index d8512b09e..25ad11002 100644 --- a/baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint +++ b/baselines/packages/mimir/test/no-duplicate-spread-property/default/test.ts.lint @@ -2,6 +2,12 @@ export {}; declare function get(): T; +declare class WithMethods { + foo(): void; + bar: () => void; + baz: string; +} + const foo = 'foo'; ({ @@ -67,3 +73,47 @@ const foo = 'foo'; let a, b; ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); } + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + baz: 1, + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + ~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + baz: get<() => void>(), + ~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + ...get(), +}); + +({ + foo() {}, + ~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + bar: () => {}, + ~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + baz: get<() => void>(), + ~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + ...get<{foo(): void, bar: () => void, baz: number}>(), +}); + +({ + ...get(), + ~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); + +({ + ...get<{foo: number, bar: number, baz: number}>(), + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); diff --git a/baselines/packages/mimir/test/no-duplicate-spread-property/loose/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/loose/test.ts.lint index 44944bb71..d719ed80a 100644 --- a/baselines/packages/mimir/test/no-duplicate-spread-property/loose/test.ts.lint +++ b/baselines/packages/mimir/test/no-duplicate-spread-property/loose/test.ts.lint @@ -2,6 +2,12 @@ export {}; declare function get(): T; +declare class WithMethods { + foo(): void; + bar: () => void; + baz: string; +} + const foo = 'foo'; ({ @@ -58,3 +64,38 @@ const foo = 'foo'; let a, b; ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); } + +({ + foo: 1, + bar: 1, + baz: 1, + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + baz: get<() => void>(), + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + baz: get<() => void>(), + ...get<{foo(): void, bar: () => void, baz: number}>(), +}); + +({ + ...get(), + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); + +({ + ...get<{foo: number, bar: number, baz: number}>(), + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); diff --git a/baselines/packages/mimir/test/no-duplicate-spread-property/pre260/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/pre260/test.ts.lint index 44944bb71..d719ed80a 100644 --- a/baselines/packages/mimir/test/no-duplicate-spread-property/pre260/test.ts.lint +++ b/baselines/packages/mimir/test/no-duplicate-spread-property/pre260/test.ts.lint @@ -2,6 +2,12 @@ export {}; declare function get(): T; +declare class WithMethods { + foo(): void; + bar: () => void; + baz: string; +} + const foo = 'foo'; ({ @@ -58,3 +64,38 @@ const foo = 'foo'; let a, b; ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); } + +({ + foo: 1, + bar: 1, + baz: 1, + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + baz: get<() => void>(), + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + baz: get<() => void>(), + ...get<{foo(): void, bar: () => void, baz: number}>(), +}); + +({ + ...get(), + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); + +({ + ...get<{foo: number, bar: number, baz: number}>(), + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); diff --git a/baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint b/baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint index df99cc57e..d95094a7c 100644 --- a/baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint +++ b/baselines/packages/mimir/test/no-duplicate-spread-property/ts260/test.ts.lint @@ -2,6 +2,12 @@ export {}; declare function get(): T; +declare class WithMethods { + foo(): void; + bar: () => void; + baz: string; +} + const foo = 'foo'; ({ @@ -65,3 +71,47 @@ const foo = 'foo'; let a, b; ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); } + +({ + foo: 1, + bar: 1, + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + baz: 1, + ~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + ~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + baz: get<() => void>(), + ~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + ...get(), +}); + +({ + foo() {}, + ~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + bar: () => {}, + ~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + baz: get<() => void>(), + ~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + ...get<{foo(): void, bar: () => void, baz: number}>(), +}); + +({ + ...get(), + ~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); + +({ + ...get<{foo: number, bar: number, baz: number}>(), + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [error no-duplicate-spread-property: Property is overridden later.] + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); diff --git a/packages/mimir/test/no-duplicate-spread-property/test.ts b/packages/mimir/test/no-duplicate-spread-property/test.ts index 44944bb71..d719ed80a 100644 --- a/packages/mimir/test/no-duplicate-spread-property/test.ts +++ b/packages/mimir/test/no-duplicate-spread-property/test.ts @@ -2,6 +2,12 @@ export {}; declare function get(): T; +declare class WithMethods { + foo(): void; + bar: () => void; + baz: string; +} + const foo = 'foo'; ({ @@ -58,3 +64,38 @@ const foo = 'foo'; let a, b; ({[foo]: a, foo: b, ...{}} = get<{foo: string}>()); } + +({ + foo: 1, + bar: 1, + baz: 1, + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + baz: get<() => void>(), + ...get(), +}); + +({ + foo() {}, + bar: () => {}, + baz: get<() => void>(), + ...get<{foo(): void, bar: () => void, baz: number}>(), +}); + +({ + ...get(), + foo() {}, + bar: () => {}, + baz: get<() => void>(), +}); + +({ + ...get<{foo: number, bar: number, baz: number}>(), + foo() {}, + bar: () => {}, + baz: get<() => void>(), +});