Warning if truthiness (#1333)

Perryvw · web-flow · commit 61d073803f6b · 2022-08-18T13:02:33.000+02:00
* Add warning diagnostic when using condition variable that will never be false in Lua

* Fix tests

* Fix ugly import
diff --git a/src/transformation/utils/diagnostics.ts b/src/transformation/utils/diagnostics.ts
@@ -147,6 +147,10 @@ export const annotationDeprecated = createWarningDiagnosticFactory(
         `See https://typescripttolua.github.io/docs/advanced/compiler-annotations#${kind.toLowerCase()} for more information.`
 );
 
+export const truthyOnlyConditionalValue = createWarningDiagnosticFactory(
+    "Numbers and strings will always evaluate to true in Lua. Explicitly check the value with ===."
+);
+
 export const notAllowedOptionalAssignment = createErrorDiagnosticFactory(
     "The left-hand side of an assignment expression may not be an optional property access."
 );
diff --git a/src/transformation/visitors/conditional.ts b/src/transformation/visitors/conditional.ts
@@ -5,6 +5,7 @@ import { transformInPrecedingStatementScope } from "../utils/preceding-statement
 import { performHoisting, ScopeType } from "../utils/scope";
 import { transformBlockOrStatement } from "./block";
 import { canBeFalsy } from "../utils/typescript";
+import { truthyOnlyConditionalValue } from "../utils/diagnostics";
 
 type EvaluatedExpression = [precedingStatemens: lua.Statement[], value: lua.Expression];
 
@@ -39,6 +40,9 @@ function transformProtectedConditionalExpression(
 }
 
 export const transformConditionalExpression: FunctionVisitor<ts.ConditionalExpression> = (expression, context) => {
+    // Check if we need to add diagnostic about Lua truthiness
+    checkOnlyTruthyCondition(expression.condition, context);
+
     const condition = transformInPrecedingStatementScope(context, () =>
         context.transformExpression(expression.condition)
     );
@@ -64,6 +68,10 @@ export const transformConditionalExpression: FunctionVisitor<ts.ConditionalExpre
 
 export function transformIfStatement(statement: ts.IfStatement, context: TransformationContext): lua.IfStatement {
     context.pushScope(ScopeType.Conditional);
+
+    // Check if we need to add diagnostic about Lua truthiness
+    checkOnlyTruthyCondition(statement.expression, context);
+
     const condition = context.transformExpression(statement.expression);
     const statements = performHoisting(context, transformBlockOrStatement(context, statement.thenStatement));
     context.popScope();
@@ -103,3 +111,9 @@ export function transformIfStatement(statement: ts.IfStatement, context: Transfo
 
     return lua.createIfStatement(condition, ifBlock);
 }
+
+export function checkOnlyTruthyCondition(condition: ts.Expression, context: TransformationContext) {
+    if (!canBeFalsy(context, context.checker.getTypeAtLocation(condition))) {
+        context.diagnostics.push(truthyOnlyConditionalValue(condition));
+    }
+}
diff --git a/src/transformation/visitors/loops/do-while.ts b/src/transformation/visitors/loops/do-while.ts
@@ -2,9 +2,13 @@ import * as ts from "typescript";
 import * as lua from "../../../LuaAST";
 import { FunctionVisitor } from "../../context";
 import { transformInPrecedingStatementScope } from "../../utils/preceding-statements";
+import { checkOnlyTruthyCondition } from "../conditional";
 import { invertCondition, transformLoopBody } from "./utils";
 
 export const transformWhileStatement: FunctionVisitor<ts.WhileStatement> = (statement, context) => {
+    // Check if we need to add diagnostic about Lua truthiness
+    checkOnlyTruthyCondition(statement.expression, context);
+
     const body = transformLoopBody(context, statement);
 
     let [conditionPrecedingStatements, condition] = transformInPrecedingStatementScope(context, () =>
@@ -37,6 +41,9 @@ export const transformWhileStatement: FunctionVisitor<ts.WhileStatement> = (stat
 };
 
 export const transformDoStatement: FunctionVisitor<ts.DoStatement> = (statement, context) => {
+    // Check if we need to add diagnostic about Lua truthiness
+    checkOnlyTruthyCondition(statement.expression, context);
+
     const body = lua.createDoStatement(transformLoopBody(context, statement));
 
     let [conditionPrecedingStatements, condition] = transformInPrecedingStatementScope(context, () =>
diff --git a/test/unit/conditionals.spec.ts b/test/unit/conditionals.spec.ts
@@ -1,5 +1,6 @@
 import * as tstl from "../../src";
 import * as util from "../util";
+import { truthyOnlyConditionalValue } from "../../src/transformation/utils/diagnostics";
 
 test.each([0, 1])("if (%p)", inp => {
     util.testFunction`
@@ -88,7 +89,9 @@ test.each([
     { condition: false, lhs: 4, rhs: 5 },
     { condition: 3, lhs: 4, rhs: 5 },
 ])("Ternary Conditional (%p)", ({ condition, lhs, rhs }) => {
-    util.testExpressionTemplate`${condition} ? ${lhs} : ${rhs}`.expectToMatchJsResult();
+    util.testExpressionTemplate`${condition} ? ${lhs} : ${rhs}`
+        .ignoreDiagnostics([truthyOnlyConditionalValue.code])
+        .expectToMatchJsResult();
 });
 
 test.each(["true", "false", "a < 4", "a == 8"])("Ternary Conditional Delayed (%p)", condition => {
@@ -138,3 +141,49 @@ test.each([false, true])("Ternary conditional with preceding statements in false
         })
         .expectToMatchJsResult();
 });
+
+test.each(["string", "number", "string | number"])(
+    "Warning when using if statement that cannot evaluate to false undefined or null (%p)",
+    type => {
+        util.testFunction`
+            if (condition) {}
+        `
+            .setTsHeader(`declare var condition: ${type};`)
+            .setOptions({ strict: true })
+            .expectToHaveDiagnostics([truthyOnlyConditionalValue.code]);
+    }
+);
+
+test.each(["string", "number", "string | number"])(
+    "Warning when using while statement that cannot evaluate to false undefined or null (%p)",
+    type => {
+        util.testFunction`
+            while (condition) {}
+        `
+            .setTsHeader(`declare var condition: ${type};`)
+            .setOptions({ strict: true })
+            .expectToHaveDiagnostics([truthyOnlyConditionalValue.code]);
+    }
+);
+
+test.each(["string", "number", "string | number"])(
+    "Warning when using do while statement that cannot evaluate to false undefined or null (%p)",
+    type => {
+        util.testFunction`
+            do {} while (condition)
+        `
+            .setTsHeader(`declare var condition: ${type};`)
+            .setOptions({ strict: true })
+            .expectToHaveDiagnostics([truthyOnlyConditionalValue.code]);
+    }
+);
+
+test.each(["string", "number", "string | number"])(
+    "Warning when using ternary that cannot evaluate to false undefined or null (%p)",
+    type => {
+        util.testExpression`condition ? 1 : 0`
+            .setTsHeader(`declare var condition: ${type};`)
+            .setOptions({ strict: true })
+            .expectToHaveDiagnostics([truthyOnlyConditionalValue.code]);
+    }
+);
diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts
@@ -302,7 +302,7 @@ describe("lua keyword as identifier doesn't interfere with lua's value", () => {
 
     test("variable (elseif)", () => {
         util.testFunction`
-            const elseif = "foobar";
+            const elseif: string | undefined = "foobar";
             if (false) {
             } else if (elseif) {
                 return elseif;
@@ -350,7 +350,7 @@ describe("lua keyword as identifier doesn't interfere with lua's value", () => {
 
     test("variable (then)", () => {
         util.testFunction`
-            const then = "foobar";
+            const then: string | undefined = "foobar";
             if (then) {
                 return then;
             }
