Fix bug where the strict ts flag wasn't recognised correctly by no-unnecessary-type-assertion (#4841)

* Fix bug where the strict ts flag wasn't recognised correctly

This commit fixes #4840

* Make sure to only test 'strict' flag option for tsc 2.4+

* Review: put getCompilerOptions into var

Author: guidsdo

src/rules/noUnnecessaryTypeAssertionRule.ts

@@ -44,13 +44,18 @@ 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 compilerOptions = program.getCompilerOptions();
+        const strictChecksEnabled = !!compilerOptions.strict;
+        const strictNullChecksEnabled = compilerOptions.strictNullChecks === true;
+        const strictNullChecksNotDisabled = compilerOptions.strictNullChecks !== false;
+
         return this.applyWithWalker(
             new Walker(
                 sourceFile,
                 this.ruleName,
                 this.ruleArguments,
                 program.getTypeChecker(),
-                !!program.getCompilerOptions().strictNullChecks,
+                strictNullChecksEnabled || (strictChecksEnabled && strictNullChecksNotDisabled),
             ),
         );
     }

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 = <string>nonNullStringLiteral;
+let h = nonNullString;
+let i = <string>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 = <Iface2>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 = <TypeB>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
+}
+

test/rules/no-unnecessary-type-assertion/strict/test.ts.lint

@@ -0,0 +1,125 @@
+[typescript]: >= 2.4.0
+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 = <string>nonNullStringLiteral;
+let h = <string>nonNullString;
+        ~~~~~~~~~~~~~~~~~~~~~  [0]
+let i = <string>nullableString;
+
+// complex inner expression
+let j = (nonNullString + nonNullStringLiteral)!;
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [0]
+let k = (nonNullString + nonNullStringLiteral) as string;
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [0]
+let l = <string>(nonNullString + nonNullStringLiteral);
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [0]
+let m = nonNullString.trim()!;
+        ~~~~~~~~~~~~~~~~~~~~~  [0]
+let n = nonNullString.trim() as string;
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [0]
+let o = <string>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 = <Iface1>value1;
+        ~~~~~~~~~~~~~~  [0]
+let r = <Iface2>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 = <TypeB>aOrB;
+
+    if (aOrB.kind === 'a') {
+        let w = aOrB as TypeA;
+                ~~~~~~~~~~~~~  [0]
+    } else {
+        let x = <TypeB>aOrB;
+                ~~~~~~~~~~~  [0]
+    }
+
+    if (isB(aOrB)) {
+        let y = aOrB as TypeB;
+                ~~~~~~~~~~~~~  [0]
+    } else {
+        let z = <TypeA>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>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.

test/rules/no-unnecessary-type-assertion/strict/tsconfig.json

@@ -0,0 +1,6 @@
+{
+  "compilerOptions": {
+    "target": "es2015",
+    "strict": true
+  }
+}

test/rules/no-unnecessary-type-assertion/strict/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-unnecessary-type-assertion": [true, "AnyDuringMigration"]
+  }
+}