New Linter rule: MisleadingCondition by tapple · Pull Request #2184 · luau-lang/luau

tapple · 2026-01-11T00:56:49Z

Closes Linter should check for non-booleans in condition expressions #2183
Matching Documentation PR: Document MisleadingCondition warning in lint.md site#83

The linter now complains about condition expressions that are not a supertype of any of:

boolean
nil
true
false

The linter considers a condition expression to be one of:

The condition expression of an if statement
The condition expression of an if expression
The left expression of an and operator
The left expression of an or operator
The first argument to an assert() function call

See the unit test for error messages

Example:

local function llGetAttached(): number
    return 1  -- stub
end
if llGetAttached() then
    print("attached")
else
    print("not attached")
end

luau-analyze MisleadingCondition.luau 
./MisleadingCondition.luau(4,1): MisleadingCondition: (num) is always true; did you mean (num ~= 0)?

jLn0n · 2026-01-11T11:56:28Z

this should be documented on a rfc and approved first before creating a new linter rule i think

WolfGangS · 2026-01-11T15:09:37Z

this should be documented on a rfc and approved first before creating a new linter rule i think

Contribute suggests rfcs only for language changes, this wouldn't do that, and an issue has been created, though maybe some more time should have passed between issue and pr.

On the rule in general, I think it's extremely useful for devs unfamiliar with lua. Nearly every other language has at least 0 as false, and many have a concept of falsey for things like empty strings or tables.

As lua and by extension luau doesn't have a broad falsey concept, this lint would catch a very common footgun for newer users.

JohnnyMorganz · 2026-01-13T06:31:07Z

Can't comment on all the cases, but I like the idea. I think this is also valuable for a case where you have a function foo that returns a boolean, and you accidentally write if foo then instead of if foo() then, which will always be true. This has caused prod incidents before

aatxe · 2026-01-13T22:30:12Z

this should be documented on a rfc and approved first before creating a new linter rule i think

The linter is not part of the language itself, and frankly will probably be sunset in favor of the linter we're writing in lute since that allows programmers to author their own lints in luau instead of writing C++ to lint luau. Even if it continues to exist as well, it's not something we've treated as requiring an RFC.

tapple · 2026-01-18T00:09:51Z

+    {
+        const char * msg;
+        bool negated;
+        if (!checkCondition(node->condition, &msg, &negated))


TODO: also check the first argument to assert()

Or maybe don't. An assert evaluating false is usually understood to be a bug. And the typechecker should often be able to catch the same bugs (at leasts the asserts that assert not nil). So nearly every time this linter rule flagged an assert, it would be a false warning.

also, asserts are pretty specialized. this linter rule is meant to catch common, stupid errors. And I don't think the linter even could distinguish between "asserts that meant to check a boolean but forgot a comparison operator" and "asserts that are checking for non-nil and the type-checker agrees". Maybe checking for intersection types, but either way, it's different enough that it should be a different linter rule

An assert evaluating false is usually understood to be a bug.

I don't think this is true at all. We use things like assert(false) or assert("this message to read about an unreachable assumption" && false) all the time to mark branches that we expect to be unreachable.

I added assert() to the list of expressions the linter checks: 29817e7

tapple · 2026-07-10T21:38:46Z

    }
 };

+class LintMisleadingCondition : AstVisitor


6 month status report:

I'm still finding this linter rule useful enough that I keep bothering to:

rebase it on new luau versions

keep building luau-lsp and it's vscode plugin locally so I can use it

Maintain this branch as luau moves forward

@aatxe mentioned above #2184 (comment) that this rule should be migrated to the lute linter. I'd be happy to do that at some point. However, it's lower priority to me than teaching lute to use definitions files. Until then, lute is useless to me.

lute check now knows about definitions files, so porting this to lute is now higher priority for me

aatxe

The overall implementation here feels messy and full of bugs and unexplained inconsistencies. At a high level, a lint for conditions that are both unintended and likely unexpectedly wrong is a good thing, but the realization of that here isn't up to our quality bar.

aatxe · 2026-08-03T19:56:53Z

+    bool visit(AstExprCall* node) override
+    {
+        const char * msg;
+        bool negated;
+        if (const auto* global = node->func->as<AstExprGlobal>())
+            if (strcmp(global->name.value, "assert") == 0)
+                if (node->args.size >= 1)
+                    if (!checkCondition(node->args.data[0], &msg, &negated))
+                        emitWarning(*context, LintWarning::Code_MisleadingCondition, node->location, "%s", msg);
+
+        return true;
+    }
+};


As mentioned when you were only considering it, I do not think this is a reasonable addition to this lint. It is commonplace to use assert(false) for unreachable, and it will produce this lint as noise on every such instance of that.

assert(false) does not trigger a warning. It doesn't meet my definition of misleading. there's even a unit test to make sure it doesn't

Valid conditions include one of the following as a union member:

boolean

true

false

nil

If those are all missing, it is labeled misleading

Things like assert(0) or assert("response") would trigger a warning

aatxe · 2026-08-03T20:00:02Z

+        const char * msg;
+        bool negated;
+        if (const auto* global = node->func->as<AstExprGlobal>())
+            if (strcmp(global->name.value, "assert") == 0)


This matches every global function named assert, including shadowed ones, and you modified a test to mask this.

It also is again using strcmp instead of an ordinary comparison. global->name == "assert".

I'll remove the strcmp

I wonder if the typechecker can identify the global assert function. I would assume not. I assume the typechecker would just see the name assert and the type <T>(T, string?): T. Maybe I could identify it by MagicAssert

removed strcmp: b6a8e6d

aatxe · 2026-08-03T20:04:18Z

+        *negated = false;
+        if (const auto* unary = cond->as<AstExprUnary>())
+            if (unary-> op == AstExprUnary::Op::Not)
+            {
+                *negated = true;
+                cond = unary->expr;
+            }


This hard-codes exactly one not, which means weird things will happen for any chains of not, like if not not 0 then.

I'll add unit tests for this case and see what happens

aatxe · 2026-08-03T20:09:11Z

+
+        if (isNumber(*type))
+        {
+            *msg = *negated ? "(not num) is always false; did you mean (num == 0)?" : "(num) is always true; did you mean (num ~= 0)?";


I don't think any of these error messages are acceptable in their current form. We don't generally produce errors that use metavariables (like you're doing here with num, and like you're doing across all of the errors). More effective errors would refer to things the user actually wrote instead.

Ok. I could not figure out the correct incantation to get the original source code at the expression start...end. I'll look again and try to do that

oh. I also was very unsure how to both

allocate the space for a dynamic string, and especially

clean up the space some time later for a dynamic string

All the other lint rules used statically-allocated strings, which don't need cleanup, so I did too

aatxe · 2026-08-03T20:11:53Z

-if if 1 then 2 else 3 then
-elseif if 1 then 2 else 3 then
-elseif if 0 then 5 else 4 then
+a = true; b = true; c = false; d = true; e = false; f = true
+if if b then c else d then
+elseif if b then c else d then
+elseif if a then f else e then


You shouldn't be changing this test either.

I think this change would be unnecessary if number | true was treated as a valid condition, as requested in #2184 (comment)

I reverted this test, and added --!nolint: a3c91d9. It fails MisleadingCondition 6 times, correctly in my opinion:

1

2

1

2

0

5

aatxe · 2026-08-03T20:13:20Z

 _ = (true and true) and true
 _ = (true and true) or true
-_ = (true and false) and (42 and false)
+_ = (true and false) and (42 and false) -- also fails MisleadingCondition


This is an indication that misleading condition, as you have written it, does not work. We shouldn't be disabling the lint in our tests to mask bugs in the lint. Our users will actually experience the lints.

I think this change would be unnecessary if number | true was treated as a valid condition, as requested in #2184 (comment)

hmm. I think this should actually fail misleading condition. 42 is not a supertype of nil, boolean, true, or false.

and in this case is only really acting as an expression seperator, to unconditionally change the expression value. If this is idiomatic usage, maybe "the left side of and" should not be checked by MisleadingCondition

in this specific expression (42 and false) will never actually be evaluated, because the left side of the top-level and ((true and false)) is always false. The linter isn't that smart though

…everted the unit test changes where this was the only issue

tapple changed the title ~~If validator~~ Linter now warns about non-booleans used in condition expressions Jan 11, 2026

tapple changed the title ~~Linter now warns about non-booleans used in condition expressions~~ New Linter rule: MisleadingConditions Jan 11, 2026

tapple mentioned this pull request Jan 11, 2026

Document MisleadingCondition warning in lint.md luau-lang/site#83

Open

tapple commented Jan 12, 2026

View reviewed changes

Comment thread tests/Linter.test.cpp

tapple changed the title ~~New Linter rule: MisleadingConditions~~ New Linter rule: MisleadingCondition Jan 13, 2026

tapple commented Jan 18, 2026

View reviewed changes

tapple commented Feb 13, 2026

View reviewed changes

Comment thread Analysis/src/Linter.cpp

tapple mentioned this pull request May 10, 2026

Add Bitwise Operators secondlife/create#21

Merged

tapple force-pushed the if-validator branch from 393e847 to 9cf1a2b Compare June 27, 2026 20:31

tapple marked this pull request as draft June 27, 2026 20:36

tapple force-pushed the if-validator branch from 9cf1a2b to 41c359f Compare July 7, 2026 18:07

tapple marked this pull request as ready for review July 7, 2026 18:25

tapple commented Jul 10, 2026

View reviewed changes

aatxe requested changes Aug 3, 2026

View reviewed changes

tapple force-pushed the if-validator branch from 7f04dea to c37e057 Compare August 20, 2026 05:36

tapple requested a review from a team as a code owner August 20, 2026 05:36

tapple requested a review from SPY August 20, 2026 05:36

SeloTheGreat mentioned this pull request Aug 23, 2026

RFC: RedundantCast lint rule luau-lang/rfcs#126

Open

tapple added 8 commits September 26, 2026 16:41

New linter rule MisleadingConditional, duplicated from MisleadingAndOr

651bd4f

Find all conditional expressions

525fe57

utility to check conditions

4442ea8

rename conditional to condition

d78a6c0

got the skeleton of the rule written

02d52ec

wrote unit test

8289618

fixed most of the unit test

09d2e10

implemented silencing of "func() and ..."

8c968d9

tapple added 12 commits September 26, 2026 16:41

implemented bit32.band -> btest message

5ea8503

fixed other unit tests

8b7b478

Added note about unions of X | boolean

419a8c6

added a FFlag check and more notes to the weird generic test

1ab4d55

Fix AstExprUnary usage

3abf66f

Fix unit tests

0983711

Treat the first argument to assert() as a condition

be85238

fix unit tests

8f26dff

Remove unnecessary strcmp

77fceae

unions of boolean, true, false are now considered valid conditions. R…

c00bf97

…everted the unit test changes where this was the only issue

Reverted changes to unit test "DuplicateConditionsIfStatAndExpr"

2b1f5a7

Deleted generic check

6509373

tapple force-pushed the if-validator branch from c774702 to 6509373 Compare September 26, 2026 14:41

Conversation

tapple commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jLn0n commented Jan 11, 2026

Uh oh!

WolfGangS commented Jan 11, 2026

Uh oh!

Uh oh!

JohnnyMorganz commented Jan 13, 2026

Uh oh!

aatxe commented Jan 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tapple Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aatxe Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tapple Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aatxe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tapple Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tapple Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tapple Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tapple commented Jan 11, 2026 •

edited

Loading

tapple Jan 18, 2026 •

edited

Loading

aatxe Jan 20, 2026 •

edited

Loading

tapple Jul 10, 2026 •

edited

Loading

tapple Aug 4, 2026 •

edited

Loading

tapple Aug 20, 2026 •

edited

Loading

tapple Aug 20, 2026 •

edited

Loading