inspection: types nullable (#1235)

docs/checkers_doc.md

@@ -4,7 +4,7 @@
 
 | Total checks | Checks enabled by default | Disabled checks by default | Autofixable checks |
 | ------------ | ------------------------- | -------------------------- | ------------------ |
-| 103          | 85                        | 18                         | 13                 |
+| 104           | 86                        | 18                         | 14                 |
 
 ## Table of contents
  - Enabled by default
@@ -59,6 +59,7 @@
    - [`nestedTernary` checker](#nestedternary-checker)
    - [`newAbstract` checker](#newabstract-checker)
    - [`nonPublicInterfaceMember` checker](#nonpublicinterfacemember-checker)
+   - [`notExplicitNullableParam` checker (autofixable)](#notexplicitnullableparam-checker)
    - [`offBy1` checker (autofixable)](#offby1-checker)
    - [`oldStyleConstructor` checker](#oldstyleconstructor-checker)
    - [`paramClobber` checker](#paramclobber-checker)
@@ -1176,6 +1177,26 @@ interface Iface {
 <p><br></p>
 
 
+### `notExplicitNullableParam` checker
+
+> Auto fix available
+
+#### Description
+
+Report not nullable param can be null.
+
+#### Non-compliant code:
+```php
+function f(string $str = null);
+```
+
+#### Compliant code:
+```php
+function f(?string $str = null);
+```
+<p><br></p>
+
+
 ### `offBy1` checker
 
 > Auto fix available

src/linter/block.go

@@ -551,8 +551,44 @@ func (b *blockWalker) handleAndCheckGlobalStmt(s *ir.GlobalStmt) {
 	}
 }
 
+func (b *blockWalker) CheckParamNullability(params []ir.Node) {
+	for _, param := range params {
+		if p, ok := param.(*ir.Parameter); ok {
+			var paramType ir.Node
+			paramType, paramOk := p.VariableType.(*ir.Name)
+			if !paramOk {
+				paramIdentifier, paramIdentifierOk := p.VariableType.(*ir.Identifier)
+				if !paramIdentifierOk {
+					continue
+				}
+				paramType = paramIdentifier
+			}
+
+			paramName, ok := paramType.(*ir.Name)
+			if ok {
+				if paramName.Value == "mixed" {
+					continue
+				}
+			}
+
+			defValue, defValueOk := p.DefaultValue.(*ir.ConstFetchExpr)
+			if !defValueOk {
+				continue
+			}
+
+			if defValue.Constant.Value != "null" {
+				continue
+			}
+
+			b.linter.report(paramType, LevelWarning, "notExplicitNullableParam", "parameter with null default value should be explicitly nullable")
+			b.r.addQuickFix("notExplicitNullableParam", b.linter.quickfix.notExplicitNullableParam(paramType))
+		}
+	}
+}
+
 func (b *blockWalker) handleFunction(fun *ir.FunctionStmt) bool {
 	if b.ignoreFunctionBodies {
+		b.CheckParamNullability(fun.Params)
 		return false
 	}
 
@@ -1034,6 +1070,7 @@ func (b *blockWalker) handleCallArgs(args []ir.Node, fn meta.FuncInfo) {
 				ArgTypes: funcArgTypes,
 			}
 
+			b.CheckParamNullability(a.Params)
 			b.enterClosure(a, isInstance, typ, closureSolver)
 		default:
 			a.Walk(b)

src/linter/block_linter.go

@@ -33,9 +33,18 @@ func (b *blockLinter) enterNode(n ir.Node) {
 	case *ir.ClassStmt:
 		b.checkClass(n)
 
+	case *ir.TraitStmt:
+		b.checkTrait(n)
+
 	case *ir.FunctionCallExpr:
 		b.checkFunctionCall(n)
 
+	case *ir.ArrowFunctionExpr:
+		b.walker.CheckParamNullability(n.Params)
+
+	case *ir.ClosureExpr:
+		b.walker.CheckParamNullability(n.Params)
+
 	case *ir.MethodCallExpr:
 		b.checkMethodCall(n)
 
@@ -207,15 +216,25 @@ func (b *blockLinter) checkUnaryPlus(n *ir.UnaryPlusExpr) {
 	b.report(n, LevelWarning, "strangeCast", "Unary plus with non-constant expression, possible type cast, use an explicit cast to int or float instead of using the unary plus")
 }
 
+func (b *blockLinter) checkTrait(n *ir.TraitStmt) {
+	for _, stmt := range n.Stmts {
+		method, ok := stmt.(*ir.ClassMethodStmt)
+		if ok {
+			b.walker.CheckParamNullability(method.Params)
+		}
+	}
+}
+
 func (b *blockLinter) checkClass(class *ir.ClassStmt) {
 	const classMethod = 0
 	const classOtherMember = 1
 
 	var members = make([]int, 0, len(class.Stmts))
 	for _, stmt := range class.Stmts {
-		switch stmt.(type) {
+		switch stmt := stmt.(type) {
 		case *ir.ClassMethodStmt:
 			members = append(members, classMethod)
+			b.walker.CheckParamNullability(stmt.Params)
 		default:
 			members = append(members, classOtherMember)
 		}
@@ -1223,7 +1242,6 @@ func (b *blockLinter) checkStripTags(e *ir.FunctionCallExpr) {
 
 func (b *blockLinter) checkMethodCall(e *ir.MethodCallExpr) {
 	parseState := b.classParseState()
-
 	call := resolveMethodCall(b.walker.ctx.sc, parseState, b.walker.ctx.customTypes, e, b.walker.r.strictMixed)
 	if !call.canAnalyze {
 		return
@@ -1433,6 +1451,7 @@ func (b *blockLinter) checkInterfaceStmt(iface *ir.InterfaceStmt) {
 					b.report(x, LevelWarning, "nonPublicInterfaceMember", "'%s' can't be %s", methodName, modifier.Value)
 				}
 			}
+			b.walker.CheckParamNullability(x.Params)
 		case *ir.ClassConstListStmt:
 			for _, modifier := range x.Modifiers {
 				if strings.EqualFold(modifier.Value, "private") || strings.EqualFold(modifier.Value, "protected") {

src/linter/quickfix.go

@@ -7,6 +7,7 @@ import (
 	"github.com/VKCOM/noverify/src/ir"
 	"github.com/VKCOM/noverify/src/quickfix"
 	"github.com/VKCOM/noverify/src/workspace"
+	"github.com/VKCOM/php-parser/pkg/position"
 )
 
 type QuickFixGenerator struct {
@@ -47,6 +48,32 @@ func (g *QuickFixGenerator) NullForNotNullableProperty(prop *ir.PropertyStmt) qu
 	}
 }
 
+func (g *QuickFixGenerator) notExplicitNullableParam(param ir.Node) quickfix.TextEdit {
+	var pos *position.Position
+	var value string
+
+	switch v := param.(type) {
+	case *ir.Name:
+		pos = &position.Position{
+			StartPos: v.Position.StartPos,
+			EndPos:   v.Position.EndPos,
+		}
+		value = v.Value
+	case *ir.Identifier:
+		pos = &position.Position{
+			StartPos: v.Position.StartPos,
+			EndPos:   v.Position.EndPos,
+		}
+		value = v.Value
+	}
+
+	return quickfix.TextEdit{
+		StartPos:    pos.StartPos,
+		EndPos:      pos.EndPos,
+		Replacement: "?" + value,
+	}
+}
+
 func (g *QuickFixGenerator) GetType(node ir.Node, isFunctionName, nodeText string, isNegative bool) quickfix.TextEdit {
 	pos := ir.GetPosition(node)
 

src/linter/report.go

@@ -25,6 +25,15 @@ func addBuiltinCheckers(reg *CheckersRegistry) {
 			After:    `$s = strip_tags($s, '<br>')`,
 		},
 
+		{
+			Name:     "notExplicitNullableParam",
+			Default:  true,
+			Quickfix: true,
+			Comment:  "Report not nullable param with explicit null default value.",
+			Before:   `function f(string $str = null);`,
+			After:    `function f(?string $str = null);`,
+		},
+
 		{
 			Name:     "emptyStmt",
 			Default:  true,

src/linter/root_checker.go

@@ -604,7 +604,6 @@ func (r *rootChecker) checkFuncParam(p *ir.Parameter) {
 		}
 		return true
 	})
-
 	r.CheckTypeHintFunctionParam(p)
 }