From 9493943e89141e9770afedf38a7c6d3bccb4ddfc Mon Sep 17 00:00:00 2001 From: guidsdo Date: Mon, 26 Aug 2019 18:08:11 +0200 Subject: [PATCH 1/3] Fix bug where the strict ts flag wasn't recognised correctly This commit fixes #4840 --- src/rules/noUnnecessaryTypeAssertionRule.ts | 6 +- .../strict/test.ts.fix | 104 +++++++++++++++ .../strict/test.ts.lint | 124 ++++++++++++++++++ .../strict/tsconfig.json | 6 + .../strict/tslint.json | 5 + 5 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 test/rules/no-unnecessary-type-assertion/strict/test.ts.fix create mode 100644 test/rules/no-unnecessary-type-assertion/strict/test.ts.lint create mode 100644 test/rules/no-unnecessary-type-assertion/strict/tsconfig.json create mode 100644 test/rules/no-unnecessary-type-assertion/strict/tslint.json diff --git a/src/rules/noUnnecessaryTypeAssertionRule.ts b/src/rules/noUnnecessaryTypeAssertionRule.ts index b6eabf610d0..2fbff48e02e 100644 --- a/src/rules/noUnnecessaryTypeAssertionRule.ts +++ b/src/rules/noUnnecessaryTypeAssertionRule.ts @@ -44,13 +44,17 @@ export class Rule extends Lint.Rules.TypedRule { "This assertion is unnecessary since it does not change the type of the expression."; public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const strictChecksEnabled = !!program.getCompilerOptions().strict; + const strictNullChecksEnabled = program.getCompilerOptions().strictNullChecks === true; + const strictNullChecksNotDisabled = program.getCompilerOptions().strictNullChecks !== false; + return this.applyWithWalker( new Walker( sourceFile, this.ruleName, this.ruleArguments, program.getTypeChecker(), - !!program.getCompilerOptions().strictNullChecks, + strictNullChecksEnabled || (strictChecksEnabled && strictNullChecksNotDisabled), ), ); } diff --git a/test/rules/no-unnecessary-type-assertion/strict/test.ts.fix b/test/rules/no-unnecessary-type-assertion/strict/test.ts.fix new file mode 100644 index 00000000000..17726c9bc0c --- /dev/null +++ b/test/rules/no-unnecessary-type-assertion/strict/test.ts.fix @@ -0,0 +1,104 @@ +const nonNullStringLiteral: 'test'; +const nonNullString: string; +const nullableString: string|undefined; +let anyType: any; +type AnyDuringMigration = any; +let tuple: [number, number] = [1, 2]; + +// non-null +let a = nonNullStringLiteral; +let b = nonNullString; +let c = nullableString!; +tuple; + +// as +let d = nonNullStringLiteral as string; +let e = nonNullString; +let f = nullableString as string; + +// type assertion +let g = nonNullStringLiteral; +let h = nonNullString; +let i = nullableString; + +// complex inner expression +let j = (nonNullString + nonNullStringLiteral); +let k = (nonNullString + nonNullStringLiteral); +let l = (nonNullString + nonNullStringLiteral); +let m = nonNullString.trim(); +let n = nonNullString.trim(); +let o = nonNullString.trim(); +let p = nonNullString.trim(); + +// custom types +interface Iface1 { + prop: string; +} +interface Iface2 { + prop: string; +} + +const value1: Iface1 = {prop: 'test'}; +const value2: Iface2 = {prop: 'test'}; + +let q = value1; +let r = value1; +let s = value2; +let t = value2 as Iface1; +let aa = anyType as AnyDuringMigration; + +interface TypeA { + kind: 'a'; +} +interface TypeB { + kind: 'b'; +} + +function isB(x: TypeA|TypeB): x is TypeB { + return true; +} + +function func(aOrB: TypeA|TypeB) { + let u = aOrB as TypeA; + let v = aOrB; + + if (aOrB.kind === 'a') { + let w = aOrB; + } else { + let x = aOrB; + } + + if (isB(aOrB)) { + let y = aOrB; + } else { + let z = aOrB; + } +} + +// Expecting no warning for these assertions as they are not unnecessary. + +type Bar = 'bar'; +const data = { + x: 'foo' as 'foo', + y: 'bar' as Bar, +} + +[1, 2, 3, 4, 5].map(x => [x, 'A' + x] as [number, string]); +let x: Array<[number, string]> = [1, 2, 3, 4, 5].map(x => [x, 'A' + x] as [number, string]); + +interface NotATuple { + 0: number, + 0.5: number, + 2: number, +} + +declare const notATuple: NotATuple; +notATuple; + +function foo() { + let xx: 1 | 2 = 1; + const f = () => xx = 2; + f(); + xx as 1 | 2 === 2; // xx is inferred as 1, assertion is necessary to avoid compile error +} + diff --git a/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint b/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint new file mode 100644 index 00000000000..0bac9d59232 --- /dev/null +++ b/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint @@ -0,0 +1,124 @@ +const nonNullStringLiteral: 'test'; +const nonNullString: string; +const nullableString: string|undefined; +let anyType: any; +type AnyDuringMigration = any; +let tuple: [number, number] = [1, 2]; + +// non-null +let a = nonNullStringLiteral!; + ~~~~~~~~~~~~~~~~~~~~~ [0] +let b = nonNullString!; + ~~~~~~~~~~~~~~ [0] +let c = nullableString!; +tuple!; +~~~~~~ [0] + +// as +let d = nonNullStringLiteral as string; +let e = nonNullString as string; + ~~~~~~~~~~~~~~~~~~~~~~~ [0] +let f = nullableString as string; + +// type assertion +let g = nonNullStringLiteral; +let h = nonNullString; + ~~~~~~~~~~~~~~~~~~~~~ [0] +let i = nullableString; + +// complex inner expression +let j = (nonNullString + nonNullStringLiteral)!; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +let k = (nonNullString + nonNullStringLiteral) as string; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +let l = (nonNullString + nonNullStringLiteral); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +let m = nonNullString.trim()!; + ~~~~~~~~~~~~~~~~~~~~~ [0] +let n = nonNullString.trim() as string; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +let o = nonNullString.trim(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +let p = nonNullString!.trim(); + ~~~~~~~~~~~~~~ [0] + +// custom types +interface Iface1 { + prop: string; +} +interface Iface2 { + prop: string; +} + +const value1: Iface1 = {prop: 'test'}; +const value2: Iface2 = {prop: 'test'}; + +let q = value1; + ~~~~~~~~~~~~~~ [0] +let r = value1; +let s = value2 as Iface2; + ~~~~~~~~~~~~~~~~ [0] +let t = value2 as Iface1; +let aa = anyType as AnyDuringMigration; + +interface TypeA { + kind: 'a'; +} +interface TypeB { + kind: 'b'; +} + +function isB(x: TypeA|TypeB): x is TypeB { + return true; +} + +function func(aOrB: TypeA|TypeB) { + let u = aOrB as TypeA; + let v = aOrB; + + if (aOrB.kind === 'a') { + let w = aOrB as TypeA; + ~~~~~~~~~~~~~ [0] + } else { + let x = aOrB; + ~~~~~~~~~~~ [0] + } + + if (isB(aOrB)) { + let y = aOrB as TypeB; + ~~~~~~~~~~~~~ [0] + } else { + let z = aOrB; + ~~~~~~~~~~~ [0] + } +} + +// Expecting no warning for these assertions as they are not unnecessary. + +type Bar = 'bar'; +const data = { + x: 'foo' as 'foo', + y: 'bar' as Bar, +} + +[1, 2, 3, 4, 5].map(x => [x, 'A' + x] as [number, string]); +let x: Array<[number, string]> = [1, 2, 3, 4, 5].map(x => [x, 'A' + x] as [number, string]); + +interface NotATuple { + 0: number, + 0.5: number, + 2: number, +} + +declare const notATuple: NotATuple; +notATuple; +~~~~~~~~~~~~~~~~~~~~ [0] + +function foo() { + let xx: 1 | 2 = 1; + const f = () => xx = 2; + f(); + xx as 1 | 2 === 2; // xx is inferred as 1, assertion is necessary to avoid compile error +} + +[0]: This assertion is unnecessary since it does not change the type of the expression. diff --git a/test/rules/no-unnecessary-type-assertion/strict/tsconfig.json b/test/rules/no-unnecessary-type-assertion/strict/tsconfig.json new file mode 100644 index 00000000000..71e2b87a156 --- /dev/null +++ b/test/rules/no-unnecessary-type-assertion/strict/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "target": "es2015", + "strict": true + } +} diff --git a/test/rules/no-unnecessary-type-assertion/strict/tslint.json b/test/rules/no-unnecessary-type-assertion/strict/tslint.json new file mode 100644 index 00000000000..6c6fdcf5f38 --- /dev/null +++ b/test/rules/no-unnecessary-type-assertion/strict/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-unnecessary-type-assertion": [true, "AnyDuringMigration"] + } +} From 2c90cb78532214988732772e2e0521c316929412 Mon Sep 17 00:00:00 2001 From: guidsdo Date: Mon, 26 Aug 2019 18:19:51 +0200 Subject: [PATCH 2/3] Make sure to only test 'strict' flag option for tsc 2.4+ --- test/rules/no-unnecessary-type-assertion/strict/test.ts.lint | 1 + 1 file changed, 1 insertion(+) diff --git a/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint b/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint index 0bac9d59232..42ae2598262 100644 --- a/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint +++ b/test/rules/no-unnecessary-type-assertion/strict/test.ts.lint @@ -1,3 +1,4 @@ +[typescript]: >= 2.4.0 const nonNullStringLiteral: 'test'; const nonNullString: string; const nullableString: string|undefined; From 15087ccd87302b396ab07de979ca13f3a065cb90 Mon Sep 17 00:00:00 2001 From: guidsdo Date: Sat, 31 Aug 2019 15:57:01 +0200 Subject: [PATCH 3/3] Review: put getCompilerOptions into var --- src/rules/noUnnecessaryTypeAssertionRule.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/rules/noUnnecessaryTypeAssertionRule.ts b/src/rules/noUnnecessaryTypeAssertionRule.ts index 2fbff48e02e..4754aeb0a91 100644 --- a/src/rules/noUnnecessaryTypeAssertionRule.ts +++ b/src/rules/noUnnecessaryTypeAssertionRule.ts @@ -44,9 +44,10 @@ export class Rule extends Lint.Rules.TypedRule { "This assertion is unnecessary since it does not change the type of the expression."; public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { - const strictChecksEnabled = !!program.getCompilerOptions().strict; - const strictNullChecksEnabled = program.getCompilerOptions().strictNullChecks === true; - const strictNullChecksNotDisabled = program.getCompilerOptions().strictNullChecks !== false; + const compilerOptions = program.getCompilerOptions(); + const strictChecksEnabled = !!compilerOptions.strict; + const strictNullChecksEnabled = compilerOptions.strictNullChecks === true; + const strictNullChecksNotDisabled = compilerOptions.strictNullChecks !== false; return this.applyWithWalker( new Walker(