Relaxed warning for checking always-true conditions (#1350)

Perryvw · web-flow · commit fc508a83e152 · 2022-09-18T18:30:14.000+02:00
diff --git a/src/transformation/utils/diagnostics.ts b/src/transformation/utils/diagnostics.ts
@@ -146,7 +146,7 @@ export const annotationDeprecated = createWarningDiagnosticFactory(
 );
 
 export const truthyOnlyConditionalValue = createWarningDiagnosticFactory(
-    "Numbers and strings will always evaluate to true in Lua. Explicitly check the value with ===."
+    "Only false and nil evaluate to 'false' in Lua, everything else is considered 'true'. Explicitly compare the value with ===."
 );
 
 export const notAllowedOptionalAssignment = createErrorDiagnosticFactory(
diff --git a/src/transformation/visitors/conditional.ts b/src/transformation/visitors/conditional.ts
@@ -113,6 +113,9 @@ export function transformIfStatement(statement: ts.IfStatement, context: Transfo
 }
 
 export function checkOnlyTruthyCondition(condition: ts.Expression, context: TransformationContext) {
+    if (context.options.strictNullChecks === false) return; // This check is not valid if everything could implicitly be nil
+    if (ts.isElementAccessExpression(condition)) return; // Array index could always implicitly return nil
+
     if (!canBeFalsy(context, context.checker.getTypeAtLocation(condition))) {
         context.diagnostics.push(truthyOnlyConditionalValue(condition));
     }
diff --git a/test/unit/conditionals.spec.ts b/test/unit/conditionals.spec.ts
@@ -154,6 +154,15 @@ test.each(["string", "number", "string | number"])(
     }
 );
 
+test.each(["string", "number", "string | number"])("Warning can be disabled when strict is true (%p)", type => {
+    util.testFunction`
+            if (condition) {}
+        `
+        .setTsHeader(`declare var condition: ${type};`)
+        .setOptions({ strict: true, strictNullChecks: false })
+        .expectToHaveNoDiagnostics();
+});
+
 test.each(["string", "number", "string | number"])(
     "Warning when using while statement that cannot evaluate to false undefined or null (%p)",
     type => {
@@ -187,3 +196,10 @@ test.each(["string", "number", "string | number"])(
             .expectToHaveDiagnostics([truthyOnlyConditionalValue.code]);
     }
 );
+
+test.each(["string", "number", "string | number"])("No warning when using element index in condition (%p)", type => {
+    util.testExpression`condition[0] ? 1 : 0`
+        .setTsHeader(`declare var condition: ${type}[];`)
+        .setOptions({ strict: true })
+        .expectToHaveNoDiagnostics();
+});
diff --git a/test/unit/optionalChaining.spec.ts b/test/unit/optionalChaining.spec.ts
@@ -253,7 +253,7 @@ describe("optional chaining function calls", () => {
                 const result = func?.(i++);
         `
             .setOptions({
-                alwaysStrict: strict,
+                strict,
                 target: ScriptTarget.ES5,
             })
             .expectToMatchJsResult();

Original file line number	Diff line number	Diff line change
`@@ -113,6 +113,9 @@ export function transformIfStatement(statement: ts.IfStatement, context: Transfo`
`113`	`113`	`}`
`114`	`114`
`115`	`115`	`export function checkOnlyTruthyCondition(condition: ts.Expression, context: TransformationContext) {`
	`116`	`+ if (context.options.strictNullChecks === false) return; // This check is not valid if everything could implicitly be nil`
	`117`	`+ if (ts.isElementAccessExpression(condition)) return; // Array index could always implicitly return nil`
	`118`	`+`
`116`	`119`	`if (!canBeFalsy(context, context.checker.getTypeAtLocation(condition))) {`
`117`	`120`	`context.diagnostics.push(truthyOnlyConditionalValue(condition));`
`118`	`121`	`}`
Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ describe("optional chaining function calls", () => {`
`253`	`253`	`const result = func?.(i++);`
`254`	`254`	`
`255`	`255`	`.setOptions({`
`256`		`- alwaysStrict: strict,`
	`256`	`+ strict,`
`257`	`257`	`target: ScriptTarget.ES5,`
`258`	`258`	`})`
`259`	`259`	`.expectToMatchJsResult();`