linter: new rule for dangerous bool/int condition

src/linter/block_linter.go

@@ -175,8 +175,10 @@ func (b *blockLinter) enterNode(n ir.Node) {
 		b.walker.r.checker.CheckKeywordCase(n, "for")
 	case *ir.WhileStmt:
 		b.walker.r.checker.CheckKeywordCase(n, "while")
+		b.checkDangerousBoolCond(n.Cond)
 	case *ir.DoStmt:
 		b.walker.r.checker.CheckKeywordCase(n, "do")
+		b.checkDangerousBoolCond(n.Cond)
 
 	case *ir.ContinueStmt:
 		b.checkContinueStmt(n)
@@ -757,10 +759,44 @@ func (b *blockLinter) checkIfStmt(s *ir.IfStmt) {
 			b.report(s, LevelWarning, "dupBranchBody", "Duplicated if/else actions")
 		}
 	}
+	b.checkDangerousBoolCond(s.Cond)
 
 	b.checkIfStmtDupCond(s)
 }
 
+func (b *blockLinter) checkDangerousBoolCond(s ir.Node) {
+	cond, ok := s.(*ir.BooleanOrExpr)
+	if !ok {
+		checkNodeDangerousBoolCond(s, b)
+		return
+	}
+
+	checkIfStatementConditionBool(cond.Left, cond.Right, b)
+}
+func checkIfStatementConditionBool(left ir.Node, right ir.Node, b *blockLinter) {
+	checkNodeDangerousBoolCond(left, b)
+	checkNodeDangerousBoolCond(right, b)
+}
+
+func checkNodeDangerousBoolCond(node ir.Node, b *blockLinter) {
+	switch n := node.(type) {
+	case *ir.ConstFetchExpr:
+		if strings.EqualFold(n.Constant.Value, "true") || strings.EqualFold(n.Constant.Value, "false") {
+			b.report(node, LevelWarning, "dangerousCondition", "Potential dangerous bool value: you have constant bool value in condition")
+		}
+	case *ir.Lnumber:
+		if n.Value == "0" || n.Value == "1" {
+			b.report(node, LevelWarning, "dangerousCondition", "Potential dangerous value: you have constant int value that interpreted as bool")
+		}
+	case *ir.BooleanOrExpr:
+		checkNodeDangerousBoolCond(n.Left, b)
+		checkNodeDangerousBoolCond(n.Right, b)
+	case *ir.BooleanAndExpr:
+		checkNodeDangerousBoolCond(n.Left, b)
+		checkNodeDangerousBoolCond(n.Right, b)
+	}
+}
+
 func (b *blockLinter) checkIfStmtDupCond(s *ir.IfStmt) {
 	conditions := irutil.NewNodeSet()
 	conditions.Add(s.Cond)

src/linter/report.go

@@ -1012,6 +1012,16 @@ function main(): void {
 			After:    `(string)$x`,
 		},
 
+		{
+			Name:     "dangerousCondition",
+			Default:  true,
+			Quickfix: false,
+			Comment:  "Report a dangerous condition",
+			Before:   "if(true){}",
+			After: `$a = getCond(); // get bool value from some func
+				if($a){}`,
+		},
+
 		{
 			Name:     "reverseAssign",
 			Default:  true,

src/tests/checkers/basic_test.go

@@ -2183,6 +2183,7 @@ function f() {
 	test.Expect = []string{
 		`since PHP 7.4, using array_key_exists() with an object has been deprecated, use isset() or property_exists() instead`,
 		`since PHP 7.4, using array_key_exists() with an object has been deprecated, use isset() or property_exists() instead`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
 	}
 	test.RunAndMatch()
 }
@@ -2364,6 +2365,7 @@ function f() {
 `)
 	test.Expect = []string{
 		`Cannot find referenced variable $e`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
 	}
 	test.RunAndMatch()
 }

src/tests/checkers/dangerouse_condition_test.go

@@ -0,0 +1,100 @@
+package checkers
+
+import (
+	"testing"
+
+	"github.com/VKCOM/noverify/src/linttest"
+)
+
+func TestDangerousCondition1(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+if(true){
+}
+
+if(1){
+}
+
+`)
+	test.Expect = []string{
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+	}
+	test.RunAndMatch()
+}
+
+func TestDangerousCondition2(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+
+$a = true;
+if(true||$a){
+echo "test";
+}
+
+if(1||$a||1||true||false||0){
+}
+`)
+	test.Expect = []string{
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous value: you have constant int value that interpreted`,
+	}
+	test.RunAndMatch()
+}
+
+func TestDangerousCondition3(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+
+$a = true;
+if($a && false && true && 1 && 0){
+}
+
+`)
+	test.Expect = []string{
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+	}
+	test.RunAndMatch()
+}
+
+func TestDangerousCondition4(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+
+$a = true;
+if($a || false && 1 || true){
+}
+
+`)
+	test.Expect = []string{
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+	}
+	test.RunAndMatch()
+}
+
+func TestDangerousCondition5(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+
+while(true){
+}
+
+do{
+
+}while(1);
+`)
+	test.Expect = []string{
+		`Potential dangerous bool value: you have constant bool value in condition`,
+		`Potential dangerous value: you have constant int value that interpreted as bool`,
+	}
+	test.RunAndMatch()
+}

src/tests/checkers/get_type_miss_use_test.go

@@ -43,6 +43,7 @@ function getTypeMisUse(mixed $var) {
 		`use is_object instead of 'gettype($var) === "object"'`,
 		`use is_int instead of 'gettype(getTypeMisUse($var)) === "integer"'`,
 		`use is_resource instead of 'gettype(getTypeMisUse($var)) != "resource"'`,
+		`Potential dangerous bool value: you have constant bool value in condition`,
 	}
 
 	test.RunAndMatch()

src/tests/golden/golden_test.go

@@ -112,7 +112,7 @@ func TestGolden(t *testing.T) {
 
 		{
 			Name:    "phprocksyd",
-			Disable: []string{"missingPhpdoc"},
+			Disable: []string{"missingPhpdoc", "dangerousCondition"},
 			Deps: []string{
 				`stubs/phpstorm-stubs/standard/basic.php`,
 				`stubs/phpstorm-stubs/pcntl/pcntl.php`,

src/tests/regression/issue78_test.go

@@ -194,6 +194,7 @@ function trailing_exit_while($xs) {
 		"Unreachable code",
 		"Unreachable code",
 		"Unreachable code",
+		"Potential dangerous value: you have constant int value that interpreted as bool",
 	}
 
 	test.RunAndMatch()
@@ -375,6 +376,7 @@ function trailing_exit_while($xs) {
 		"Unreachable code",
 		"Unreachable code",
 		"Unreachable code",
+		"Potential dangerous value: you have constant int value that interpreted as bool",
 	}
 
 	test.RunAndMatch()
@@ -505,7 +507,8 @@ function trailing_exit_for($xs) {
 }
 
 function trailing_exit_while($xs) {
-  while (1) {
+$b = 1;
+  while ($b) {
     if ($xs[0] < 1000) {
       return "ok";
     }