From 19f1e9b1e250a2d9b4137e9ad2a81982088fdcf4 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 9 Aug 2018 13:59:22 +0200 Subject: [PATCH 1/2] no-duplicate-spread-property: exclude class Getters and Setter-only properties Fixes: #371 --- .../default/test.ts.lint | 35 +++++++++++++++++++ .../loose/test.ts.lint | 33 +++++++++++++++++ .../src/rules/no-duplicate-spread-property.ts | 15 ++++---- .../test/no-duplicate-spread-property/test.ts | 33 +++++++++++++++++ 4 files changed, 110 insertions(+), 6 deletions(-) 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 b5e10d0ba..6603b1a5e 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 @@ -194,3 +194,38 @@ var v: any; bar: 2, }, }); + +({ + foo: 1, + ...new class { + get foo() { return 1; } + bar = 1; + }, +}); + +({ + foo: 1, + ~~~ [error no-duplicate-spread-property: Property 'foo' is overridden later.] + ...{ + get foo() { return 1; }, + bar: 2, + }, +}); + +({ + foo: 1, + ~~~ [error no-duplicate-spread-property: Property 'foo' is overridden later.] + ...{ + get foo() { return 1; }, + set foo(v: number) {}, + bar: 2, + }, +}); + +({ + foo: 1, + ...{ + set foo(v: number) {}, + bar: 2, + }, +}); 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 71b55ce0c..3363caaff 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 @@ -174,3 +174,36 @@ var v: any; bar: 2, }, }); + +({ + foo: 1, + ...new class { + get foo() { return 1; } + bar = 1; + }, +}); + +({ + foo: 1, + ...{ + get foo() { return 1; }, + bar: 2, + }, +}); + +({ + foo: 1, + ...{ + get foo() { return 1; }, + set foo(v: number) {}, + bar: 2, + }, +}); + +({ + foo: 1, + ...{ + set foo(v: number) {}, + bar: 2, + }, +}); diff --git a/packages/mimir/src/rules/no-duplicate-spread-property.ts b/packages/mimir/src/rules/no-duplicate-spread-property.ts index 007a15593..0edb1462f 100644 --- a/packages/mimir/src/rules/no-duplicate-spread-property.ts +++ b/packages/mimir/src/rules/no-duplicate-spread-property.ts @@ -98,7 +98,7 @@ function getPropertyInfoFromType(type: ts.Type): PropertyInfo { assignedNames: [], }; for (const prop of type.getProperties()) { - if (isClassMethod(prop)) + if (!isSpreadableProperty(prop)) continue; if ((prop.flags & ts.SymbolFlags.Optional) === 0) result.assignedNames.push(prop.escapedName); @@ -106,12 +106,15 @@ function getPropertyInfoFromType(type: ts.Type): PropertyInfo { } return result; } -function isClassMethod(prop: ts.Symbol): boolean | undefined { - if (prop.flags & ts.SymbolFlags.Method && prop.declarations !== undefined) - for (const declaration of prop.declarations) +function isSpreadableProperty(prop: ts.Symbol): boolean | undefined { + if (prop.flags & (ts.SymbolFlags.Method | ts.SymbolFlags.GetAccessor)) { + for (const declaration of prop.declarations!) if (isClassLikeDeclaration(declaration.parent!)) - return true; - return false; + return false; // class Methods and GetAccessors are not spreadable + } else if (prop.flags & ts.SymbolFlags.SetAccessor) { + return false; // SetAccessor-only properties are never spreadable + } + return true; } function combinePropertyInfo(a: PropertyInfo, b: PropertyInfo): PropertyInfo { diff --git a/packages/mimir/test/no-duplicate-spread-property/test.ts b/packages/mimir/test/no-duplicate-spread-property/test.ts index 71b55ce0c..3363caaff 100644 --- a/packages/mimir/test/no-duplicate-spread-property/test.ts +++ b/packages/mimir/test/no-duplicate-spread-property/test.ts @@ -174,3 +174,36 @@ var v: any; bar: 2, }, }); + +({ + foo: 1, + ...new class { + get foo() { return 1; } + bar = 1; + }, +}); + +({ + foo: 1, + ...{ + get foo() { return 1; }, + bar: 2, + }, +}); + +({ + foo: 1, + ...{ + get foo() { return 1; }, + set foo(v: number) {}, + bar: 2, + }, +}); + +({ + foo: 1, + ...{ + set foo(v: number) {}, + bar: 2, + }, +}); From 2a0c1a88094e62365852d8bef30c78d17ad28a4a Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Thu, 9 Aug 2018 22:22:32 +0200 Subject: [PATCH 2/2] treat setter the same as getter --- .../test/no-duplicate-spread-property/default/test.ts.lint | 2 ++ .../test/no-duplicate-spread-property/loose/test.ts.lint | 1 + packages/mimir/src/rules/no-duplicate-spread-property.ts | 7 ++----- packages/mimir/test/no-duplicate-spread-property/test.ts | 1 + 4 files changed, 6 insertions(+), 5 deletions(-) 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 6603b1a5e..2f44f773f 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 @@ -199,6 +199,7 @@ var v: any; foo: 1, ...new class { get foo() { return 1; } + set foo(v) {} bar = 1; }, }); @@ -224,6 +225,7 @@ var v: any; ({ foo: 1, + ~~~ [error no-duplicate-spread-property: Property 'foo' is overridden later.] ...{ set foo(v: number) {}, bar: 2, 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 3363caaff..c25c3b7e0 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 @@ -179,6 +179,7 @@ var v: any; foo: 1, ...new class { get foo() { return 1; } + set foo(v) {} bar = 1; }, }); diff --git a/packages/mimir/src/rules/no-duplicate-spread-property.ts b/packages/mimir/src/rules/no-duplicate-spread-property.ts index 0edb1462f..a98608821 100644 --- a/packages/mimir/src/rules/no-duplicate-spread-property.ts +++ b/packages/mimir/src/rules/no-duplicate-spread-property.ts @@ -107,13 +107,10 @@ function getPropertyInfoFromType(type: ts.Type): PropertyInfo { return result; } function isSpreadableProperty(prop: ts.Symbol): boolean | undefined { - if (prop.flags & (ts.SymbolFlags.Method | ts.SymbolFlags.GetAccessor)) { + if (prop.flags & (ts.SymbolFlags.Method | ts.SymbolFlags.Accessor)) for (const declaration of prop.declarations!) if (isClassLikeDeclaration(declaration.parent!)) - return false; // class Methods and GetAccessors are not spreadable - } else if (prop.flags & ts.SymbolFlags.SetAccessor) { - return false; // SetAccessor-only properties are never spreadable - } + return false; return true; } diff --git a/packages/mimir/test/no-duplicate-spread-property/test.ts b/packages/mimir/test/no-duplicate-spread-property/test.ts index 3363caaff..c25c3b7e0 100644 --- a/packages/mimir/test/no-duplicate-spread-property/test.ts +++ b/packages/mimir/test/no-duplicate-spread-property/test.ts @@ -179,6 +179,7 @@ var v: any; foo: 1, ...new class { get foo() { return 1; } + set foo(v) {} bar = 1; }, });