Repository: VKCOM/noverify
Pull request #1193 (2022-08-29 18:22:07)

linter: added new `getTypeMisUse` checker

YuriyNasretdinov: I think there's one thing missing in this entire PR: why are you adding this change? Why is gettype wrong and e.g. is_string right to use?

Patch:
diff --git a/docs/checkers_doc.md b/docs/checkers_doc.md
index 3ad1f97..fea7b9d 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 |
 | ------------ | ------------------------- | -------------------------- | ------------------ |
-| 102           | 85                        | 17                         | 12                 |
+| 103          | 85                        | 18                         | 13                 |
 
 ## Table of contents
  - Enabled by default
@@ -100,6 +100,7 @@
    - [`complexity` checker](#complexity-checker)
    - [`deprecatedUntagged` checker](#deprecateduntagged-checker)
    - [`errorSilence` checker](#errorsilence-checker)
+   - [`getTypeMisUse` checker (autofixable)](#gettypemisuse-checker)
    - [`langDeprecated` checker](#langdeprecated-checker)
    - [`missingPhpdoc` checker](#missingphpdoc-checker)
    - [`packaging` checker](#packaging-checker)
@@ -1993,6 +1994,26 @@ f();
 <p><br></p>
 
 
+### `getTypeMisUse` checker
+
+> Auto fix available
+
+#### Description
+
+Report call gettype function.
+
+#### Non-compliant code:
+```php
+if (gettype($a) == "string") { ... }
+```
+
+#### Compliant code:
+```php
+if (is_string($a)) { ... }
+```
+<p><br></p>
+
+
 ### `langDeprecated` checker
 
 #### Description
diff --git a/src/ir/phpcore/phpcore.go b/src/ir/phpcore/phpcore.go
index 0fae434..da6e47f 100644
--- a/src/ir/phpcore/phpcore.go
+++ b/src/ir/phpcore/phpcore.go
@@ -50,3 +50,13 @@ var funcAliases = map[string]*ir.Name{
 	`socket_get_status`:       {Value: `stream_get_meta_data`},
 	`socket_set_timeout`:      {Value: `stream_set_timeout`},
 }
+
+var TypeToIsFunction = map[string]string{
+	"boolean":  "is_bool",
+	"integer":  "is_int",
+	"double":   "is_float",
+	"string":   "is_string",
+	"array":    "is_array",
+	"object":   "is_object",
+	"resource": "is_resource",
+}
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index c95cd4b..bd7bc36 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -8,6 +8,7 @@ import (
 	"github.com/VKCOM/noverify/src/constfold"
 	"github.com/VKCOM/noverify/src/ir"
 	"github.com/VKCOM/noverify/src/ir/irutil"
+	"github.com/VKCOM/noverify/src/ir/phpcore"
 	"github.com/VKCOM/noverify/src/linter/autogen"
 	"github.com/VKCOM/noverify/src/meta"
 	"github.com/VKCOM/noverify/src/quickfix"
@@ -114,15 +115,19 @@ func (b *blockLinter) enterNode(n ir.Node) {
 	case *ir.EqualExpr:
 		b.checkBinaryVoidType(n.Left, n.Right)
 		b.checkBinaryDupArgsNoFloat(n, n.Left, n.Right)
+		b.checkGettype(n)
 	case *ir.NotEqualExpr:
 		b.checkBinaryVoidType(n.Left, n.Right)
 		b.checkBinaryDupArgsNoFloat(n, n.Left, n.Right)
+		b.checkGettype(n)
 	case *ir.IdenticalExpr:
 		b.checkBinaryVoidType(n.Left, n.Right)
 		b.checkBinaryDupArgsNoFloat(n, n.Left, n.Right)
+		b.checkGettype(n)
 	case *ir.NotIdenticalExpr:
 		b.checkBinaryVoidType(n.Left, n.Right)
 		b.checkBinaryDupArgsNoFloat(n, n.Left, n.Right)
+		b.checkGettype(n)
 	case *ir.SmallerExpr:
 		b.checkBinaryVoidType(n.Left, n.Right)
 		b.checkBinaryDupArgs(n, n.Left, n.Right)
@@ -1039,6 +1044,69 @@ func (b *blockLinter) checkFunctionCall(e *ir.FunctionCallExpr) {
 	}
 }
 
+func (b *blockLinter) checkGettype(node ir.Node) {
+	var left ir.Node
+	var right ir.Node
+	var isNegative = false
+
+	switch node := node.(type) {
+	case *ir.EqualExpr:
+		left = node.Left
+		right = node.Right
+	case *ir.NotEqualExpr:
+		left = node.Left
+		right = node.Right
+		isNegative = true
+	case *ir.IdenticalExpr:
+		left = node.Left
+		right = node.Right
+	case *ir.NotIdenticalExpr:
+		left = node.Left
+		right = node.Right
+		isNegative = true
+	default:
+		return
+	}
+
+	call, ok := left.(*ir.FunctionCallExpr)
+	if !ok {
+		return
+	}
+
+	if len(call.Args) != 1 {
+		return
+	}
+
+	name, ok := call.Function.(*ir.Name)
+	if !ok {
+		return
+	}
+
+	if name.Value != "gettype" {
+		return
+	}
+
+	firstArg, ok := call.Args[0].(*ir.Argument)
+	if !ok {
+		return
+	}
+
+	nodeText := b.walker.r.nodeText(firstArg)
+
+	str, ok := right.(*ir.String)
+	if !ok {
+		return
+	}
+
+	isFunctionName, ok := phpcore.TypeToIsFunction[str.Value]
+	if !ok {
+		return
+	}
+
+	b.report(node, LevelWarning, "getTypeMisUse", "use %s instead of '%s'", isFunctionName, b.walker.r.nodeText(node))
+	b.walker.r.addQuickFix("getTypeMisUse", b.quickfix.GetType(node, isFunctionName, nodeText, isNegative))
+}
+
 func (b *blockLinter) checkRandomIntCall(e *ir.FunctionCallExpr) {
 	if len(e.Args) < 2 {
 		return
diff --git a/src/linter/quickfix.go b/src/linter/quickfix.go
index 1f937e6..ef83aa8 100644
--- a/src/linter/quickfix.go
+++ b/src/linter/quickfix.go
@@ -46,3 +46,19 @@ func (g *QuickFixGenerator) NullForNotNullableProperty(prop *ir.PropertyStmt) qu
 		Replacement: string(withoutAssign),
 	}
 }
+
+func (g *QuickFixGenerator) GetType(node ir.Node, isFunctionName, nodeText string, isNegative bool) quickfix.TextEdit {
+	pos := ir.GetPosition(node)
+
+	if isNegative {
+		isFunctionName = "!" + isFunctionName
+	}
+
+	isFunctionName = isFunctionName + "(" + nodeText + ")"
+
+	return quickfix.TextEdit{
+		StartPos:    pos.StartPos,
+		EndPos:      pos.EndPos,
+		Replacement: isFunctionName,
+	}
+}
diff --git a/src/linter/report.go b/src/linter/report.go
index 0e28be3..d1914da 100644
--- a/src/linter/report.go
+++ b/src/linter/report.go
@@ -1130,6 +1130,15 @@ class Main {
   }
 }`,
 		},
+
+		{
+			Name:     "getTypeMisUse",
+			Default:  false,
+			Quickfix: true,
+			Comment:  `Report call gettype function.`,
+			Before:   `if (gettype($a) == "string") { ... }`,
+			After:    `if (is_string($a)) { ... }`,
+		},
 	}
 
 	for _, info := range allChecks {


Test patch:
diff --git a/src/tests/checkers/get_type_miss_use_test.go b/src/tests/checkers/get_type_miss_use_test.go
new file mode 100644
index 0000000..eadfa26
--- /dev/null
+++ b/src/tests/checkers/get_type_miss_use_test.go
@@ -0,0 +1,49 @@
+package checkers
+
+import (
+	"testing"
+
+	"github.com/VKCOM/noverify/src/linttest"
+)
+
+func TestGettypeFunction(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.LoadStubs = []string{
+		`stubs/phpstorm-stubs/standard/standard_5.php`,
+	}
+	test.AddFile(`<?php
+function getTypeMisUse(mixed $var) {
+  if (gettype($var) === "string") {
+  }
+
+  if (gettype($var) == "double") {
+  }
+
+  if (gettype($var) !== "array") {
+  }
+
+  if (gettype($var) != "boolean") {
+  }
+
+  if (gettype($var) === "object" && true) {
+  }
+
+  if (gettype(getTypeMisUse($var)) === "integer") {
+  }
+
+  if (gettype(getTypeMisUse($var)) != "resource") {
+  }
+}
+`)
+	test.Expect = []string{
+		`use is_string instead of 'gettype($var) === "string"'`,
+		`use is_float instead of 'gettype($var) == "double"'`,
+		`use is_array instead of 'gettype($var) !== "array"'`,
+		`use is_bool instead of 'gettype($var) != "boolean"'`,
+		`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"'`,
+	}
+
+	test.RunAndMatch()
+}
diff --git a/src/tests/golden/testdata/quickfix/getTypeMisUse.php b/src/tests/golden/testdata/quickfix/getTypeMisUse.php
new file mode 100644
index 0000000..885fea1
--- /dev/null
+++ b/src/tests/golden/testdata/quickfix/getTypeMisUse.php
@@ -0,0 +1,24 @@
+<?php
+
+function getTypeMisUse(mixed $var) {
+  if (gettype($var) === "string") {
+  }
+
+  if (gettype($var) == "double") {
+  }
+
+  if (gettype($var) !== "array") {
+  }
+
+  if (gettype($var) != "boolean") {
+  }
+
+  if (gettype($var) === "object" && true) {
+  }
+
+  if (gettype(getTypeMisUse($var)) === "integer") {
+  }
+
+  if (gettype(getTypeMisUse($var)) != "resource") {
+  }
+}
diff --git a/src/tests/golden/testdata/quickfix/getTypeMisUse.php.fix.expected b/src/tests/golden/testdata/quickfix/getTypeMisUse.php.fix.expected
new file mode 100644
index 0000000..e100ed6
--- /dev/null
+++ b/src/tests/golden/testdata/quickfix/getTypeMisUse.php.fix.expected
@@ -0,0 +1,24 @@
+<?php
+
+function getTypeMisUse(mixed $var) {
+  if (is_string($var)) {
+  }
+
+  if (is_float($var)) {
+  }
+
+  if (!is_array($var)) {
+  }
+
+  if (!is_bool($var)) {
+  }
+
+  if (is_object($var) && true) {
+  }
+
+  if (is_int(getTypeMisUse($var))) {
+  }
+
+  if (!is_resource(getTypeMisUse($var))) {
+  }
+}