D16598 had an error where code contained this construction:

if (x) {
  // ...
}

The actual intent was:

if ($x) {
  // ...
}

I think we could detect this in lint since I think it's never correct in this codebase -- basically, if a logical expression contains any component which is a single constant, it is written incorrectly.

In the general case, PHP allows you to do this:

if (something()) {
  define('X', 1);
} else {
  define('X', 0);
}

if (X) {
 // ...
}

...so this construction isn't universally incorrect, but is always incorrect in libphutil/Phabricator codebases.

Technically, there are several variants of this. We already detect these (as "Tautological Expressions"):

$a = true || f();
if (true || f()) {}

We do not detect these:

$a = CONST || f(); // Constant in logical expression.
if (CONST) {} // Constant in control condition.
if (true) {} // Boolean literal in control condition.
$b = false || g(); // Meaningless (but not tautological) subexpression.
$c = true && h(); // Meaningless (but not tautological) subexpression.

Writing this rule probably looks something like:

• Roughly follow ArcanistTautologicalExpressionXHPASTLinterRule to create a new rule (it only needs to examine logical expressions).
• If either side of an expression is a constant (n_SYMBOL_NAME), raise a message.
• Have ArcanistTautologicalExpressionXHPASTLinterRule examine n_IF for n_CONTROL_CONDITION containing a boolean literal.
• Have the new rule examine the same for a constant.
• Have ArcanistTautologicalExpressionXHPASTLinterRule raise a message if any expression contains a meaningless subexpression.
• Add the constant detecting rule to the libphutil lint ruleset, but not to the default ruleset (this message is not unambiguously correct in PHP code in the general case, only code where constants are guaranteed to be constant).