Repository: VKCOM/noverify
Pull request #1082 (2021-08-16 12:56:28)

linter: moved part of `undefined` checker to separate `constCase` checker


Patch:
diff --git a/docs/checkers_doc.md b/docs/checkers_doc.md
index 8920bcd..d9a6d75 100644
--- a/docs/checkers_doc.md
+++ b/docs/checkers_doc.md
@@ -4,7 +4,7 @@
 
 | Total checks | Checks enabled by default | Disabled checks by default | Autofixable checks |
 | ------------ | ------------------------- | -------------------------- | ------------------ |
-| 86           | 76                        | 10                         | 11                 |
+| 87           | 77                        | 10                         | 12                 |
 ## Table of contents
  - Enabled by default
    - [`accessLevel` checker](#accesslevel-checker)
@@ -23,6 +23,7 @@
    - [`caseContinue` checker](#casecontinue-checker)
    - [`catchOrder` checker](#catchorder-checker)
    - [`complexity` checker](#complexity-checker)
+   - [`constCase` checker (autofixable)](#constcase-checker)
    - [`countUse` checker (autofixable)](#countuse-checker)
    - [`deadCode` checker](#deadcode-checker)
    - [`discardExpr` checker](#discardexpr-checker)
@@ -450,6 +451,26 @@ function checkRights() {
 <p><br></p>
 
 
+### `constCase` checker
+
+> Auto fix available
+
+#### Description
+
+Report built-in constants that are not in the lower case.
+
+#### Non-compliant code:
+```php
+return TRUE;
+```
+
+#### Compliant code:
+```php
+return true;
+```
+<p><br></p>
+
+
 ### `countUse` checker
 
 > Auto fix available
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index e6a3ec3..8cb4963 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -522,18 +522,27 @@ func (b *blockLinter) checkConstFetch(e *ir.ConstFetchExpr) {
 		// If it's builtin constant, give a more precise report message.
 		switch nm := utils.NameNodeToString(e.Constant); strings.ToLower(nm) {
 		case "null", "true", "false":
-			// TODO(quasilyte): should probably issue not "undefined" warning
-			// here, but something else, like "constCase" or something.
-			// Since it *was* "undefined" before, leave it as is for now,
-			// only make error message more user-friendly.
-			lcName := strings.ToLower(nm)
-			b.report(e.Constant, LevelError, "undefined", "Use %s instead of %s", lcName, nm)
+			expected := strings.ToLower(nm)
+			b.report(e.Constant, LevelError, "constCase", "Constant '%s' should be used in lower case as '%s'", nm, expected)
+			b.addFixForBuiltInConstantCase(e.Constant, expected)
 		default:
 			b.report(e.Constant, LevelError, "undefined", "Undefined constant %s", nm)
 		}
 	}
 }
 
+func (b *blockLinter) addFixForBuiltInConstantCase(constant *ir.Name, expected string) {
+	if !b.walker.r.config.ApplyQuickFixes {
+		return
+	}
+
+	b.walker.r.addQuickFix("constCase", quickfix.TextEdit{
+		StartPos:    constant.Position.StartPos,
+		EndPos:      constant.Position.EndPos,
+		Replacement: expected,
+	})
+}
+
 func (b *blockLinter) checkTernary(e *ir.TernaryExpr) {
 	if e.IfTrue == nil {
 		return // Skip `$x ?: $y` expressions
diff --git a/src/linter/report.go b/src/linter/report.go
index aea62dc..e307a0a 100644
--- a/src/linter/report.go
+++ b/src/linter/report.go
@@ -75,6 +75,15 @@ return -9223372036854775808;`,
 			After:    `return $x;`,
 		},
 
+		{
+			Name:     "constCase",
+			Default:  true,
+			Quickfix: true,
+			Comment:  `Report built-in constants that are not in the lower case.`,
+			Before:   `return TRUE;`,
+			After:    `return true;`,
+		},
+
 		{
 			Name:     "accessLevel",
 			Default:  true,


Test patch:
diff --git a/src/tests/checkers/basic_test.go b/src/tests/checkers/basic_test.go
index ec77336..c164491 100644
--- a/src/tests/checkers/basic_test.go
+++ b/src/tests/checkers/basic_test.go
@@ -1169,9 +1169,9 @@ func TestBuiltinConstant(t *testing.T) {
 		$_ = null;
 	}`)
 	test.Expect = []string{
-		"Use null instead of NULL",
-		"Use true instead of True",
-		"Use false instead of FaLsE",
+		"Constant 'NULL' should be used in lower case as 'null'",
+		"Constant 'True' should be used in lower case as 'true'",
+		"Constant 'FaLsE' should be used in lower case as 'false'",
 	}
 	test.RunAndMatch()
 }
diff --git a/src/tests/golden/testdata/phprocksyd/golden.txt b/src/tests/golden/testdata/phprocksyd/golden.txt
index 690d680..45ad2da 100644
--- a/src/tests/golden/testdata/phprocksyd/golden.txt
+++ b/src/tests/golden/testdata/phprocksyd/golden.txt
@@ -1,7 +1,7 @@
 MAYBE   deprecated: Call to deprecated function dl (5.3) at testdata/phprocksyd/Phprocksyd.php:73
             if (!extension_loaded($ext) && !dl($ext . '.so')) {
                                             ^^
-ERROR   undefined: Use null instead of NULL at testdata/phprocksyd/Phprocksyd.php:321
+ERROR   constCase: Constant 'NULL' should be used in lower case as 'null' at testdata/phprocksyd/Phprocksyd.php:321
             $n = stream_select($read, $write, $except, NULL);
                                                        ^^^^
 WARNING phpdocLint: Malformed @param $stream_id tag (maybe type is missing?) at testdata/phprocksyd/Phprocksyd.php:364
@@ -34,6 +34,6 @@ WARNING phpdocLint: Malformed @param $req tag (maybe type is missing?) at testda
 ERROR   undefined: Call to undefined method {mixed}->run() at testdata/phprocksyd/Phprocksyd.php:479
                 $instance->run($req['params']);
                            ^^^
-ERROR   undefined: Use null instead of NULL at testdata/phprocksyd/Simple.php:158
+ERROR   constCase: Constant 'NULL' should be used in lower case as 'null' at testdata/phprocksyd/Simple.php:158
             $n = stream_select($read, $write, $except, NULL);
                                                        ^^^^
diff --git a/src/tests/golden/testdata/quickfix/constCase.php b/src/tests/golden/testdata/quickfix/constCase.php
new file mode 100644
index 0000000..7645bcc
--- /dev/null
+++ b/src/tests/golden/testdata/quickfix/constCase.php
@@ -0,0 +1,18 @@
+<?php
+
+function f($a) {
+    if ($a === NULL) {
+        return FaLse;
+    }
+    return True;
+}
+
+function f1($a) {
+    if ($a === null) {
+        return false;
+    }
+    return true;
+}
+
+$a = NULL;
+$a = null;
diff --git a/src/tests/golden/testdata/quickfix/constCase.php.fix.expected b/src/tests/golden/testdata/quickfix/constCase.php.fix.expected
new file mode 100644
index 0000000..ddb2695
--- /dev/null
+++ b/src/tests/golden/testdata/quickfix/constCase.php.fix.expected
@@ -0,0 +1,18 @@
+<?php
+
+function f($a) {
+    if ($a === null) {
+        return false;
+    }
+    return true;
+}
+
+function f1($a) {
+    if ($a === null) {
+        return false;
+    }
+    return true;
+}
+
+$a = null;
+$a = null;