Repository: VKCOM/noverify
Pull request #819 (2020-11-23 23:05:59)

add check nullable syntax
Before that, if a `T?` entry appeared in `phpdoc`, the type was always deduced as null, which led to false positives. Now a warning will be issued for such a record.


Patch:
diff --git a/src/linter/types.go b/src/linter/types.go
index d8c03ab..95a1379 100644
--- a/src/linter/types.go
+++ b/src/linter/types.go
@@ -101,6 +101,13 @@ func (conv *phpdocTypeConverter) mapType(e phpdoc.TypeExpr) []meta.Type {
 			types = append(types, conv.mapType(a)...)
 		}
 		return types
+
+	case phpdoc.ExprOptional:
+		// Due to the fact that the optional keys for shape are processed in the mapShapeType
+		// function, while the optionality for the key is cleared, and the key itself is not
+		// processed by the mapType function, then in the mapType function the phpdoc.ExprOptional
+		// type can only be in one case, if it is an incorrect syntax of the optional type.
+		conv.warn(e.Value + ": nullable syntax is ?T, not T?")
 	}
 
 	return nil


Test patch:
diff --git a/src/phpdoc/type_parser_test.go b/src/phpdoc/type_parser_test.go
index 6d6a95b..c54faba 100644
--- a/src/phpdoc/type_parser_test.go
+++ b/src/phpdoc/type_parser_test.go
@@ -59,6 +59,10 @@ func TestParser(t *testing.T) {
 		// Nullable types.
 		{`?x`, `Nullable="?x"{Name="x"}`},
 		{`??x`, `Nullable="??x"{Nullable="?x"{Name="x"}}`},
+		// Writing a nullable type incorrectly will result in its type being inferred as Optional.
+		{`x?`, `Optional="x?"{Name="x"}`},
+		// For complex ones, the type will be completely incorrect.
+		{`x[]?`, `Optional="x[]?"{Array="x[]"{Name="x"}}`},
 
 		// Negated (Not) types.
 		{`!x`, `Not="!x"{Name="x"}`},
diff --git a/src/tests/checkers/phpdoc_test.go b/src/tests/checkers/phpdoc_test.go
index 744ff2e..6803fc4 100644
--- a/src/tests/checkers/phpdoc_test.go
+++ b/src/tests/checkers/phpdoc_test.go
@@ -306,11 +306,15 @@ class Foo {
 
   /** @var real */
   public $x;
+
+  /** @var int? */
+  public $x;
 }
 `)
 	test.Expect = []string{
 		`use int type instead of integer`,
 		`use float type instead of real`,
+		`int?: nullable syntax is ?T, not T?`,
 	}
 	test.RunAndMatch()
 }
@@ -325,6 +329,7 @@ func TestPHPDocProperty(t *testing.T) {
  * @property string
  * @property $int string
  * @property boolean[] $bools
+ * @property int? $nullable
  */
 class Foo {}
 `)
@@ -336,6 +341,7 @@ class Foo {}
 		`line 4: @property requires type and property name fields`,
 		`line 5: @property requires type and property name fields`,
 		`use bool type instead of boolean on line 7`,
+		`int?: nullable syntax is ?T, not T?`,
 	}
 	test.RunAndMatch()
 }
@@ -349,19 +355,46 @@ func TestPHPDocType(t *testing.T) {
 	 * @param real $x3
 	 * @param integer $x4
 	 * @param boolean $x5
+	 * @param int? $x6
 	 * @return []int
 	 */
-	function f($x1, $x2, $x3, $x4, $x5) {
-		$_ = [$x1, $x2, $x3, $x4, $x5];
+	function f($x1, $x2, $x3, $x4, $x5, $x6) {
+		$_ = [$x1, $x2, $x3, $x4, $x5, $x6];
 		return [1];
-	}`)
+	}
+`)
 	test.Expect = []string{
-		`[]int: array syntax is T[], not []T on line 7`,
 		`[][]string: array syntax is T[], not []T on line 2`,
 		`use float type instead of double`,
 		`use float type instead of real`,
 		`use int type instead of integer`,
 		`use bool type instead of boolean`,
+		`int?: nullable syntax is ?T, not T?`,
+		`[]int: array syntax is T[], not []T on line 8`,
+	}
+	test.RunAndMatch()
+}
+
+func TestPHPDocIncorrectSyntaxOptionalTypesType(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+	class Foo {}
+
+	/**
+	 * @param int? $x1 // error
+	 * @param shape(key:int, opt?:int) $x2 // ok, is shape
+	 * @param shape(key?:int, opt?:int) $x3 // ok, is shape
+	 * @param Foo? $x4 // error
+	 * @param string[]? $x5 // error
+	 */
+	function f1($x1, $x2, $x3, $x4, $x5) {
+		$_ = [$x1, $x2, $x3, $x4, $x5];
+	}
+`)
+	test.Expect = []string{
+		`int?: nullable syntax is ?T, not T?`,
+		`Foo?: nullable syntax is ?T, not T?`,
+		`string[]?: nullable syntax is ?T, not T?`,
 	}
 	test.RunAndMatch()
 }