Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter: null safety for functions, methods and properties for classes #1245

Merged
merged 13 commits into from
Mar 4, 2025
22 changes: 21 additions & 1 deletion docs/checkers_doc.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

| Total checks | Checks enabled by default | Disabled checks by default | Autofixable checks |
| ------------ | ------------------------- | -------------------------- | ------------------ |
| 106 | 88 | 18 | 15 |
| 107 | 89 | 18 | 15 |

## Table of contents
- Enabled by default
Expand Down Expand Up @@ -60,6 +60,7 @@
- [`newAbstract` checker](#newabstract-checker)
- [`nonPublicInterfaceMember` checker](#nonpublicinterfacemember-checker)
- [`notExplicitNullableParam` checker (autofixable)](#notexplicitnullableparam-checker)
- [`notNullSafety` checker](#notnullsafety-checker)
- [`offBy1` checker (autofixable)](#offby1-checker)
- [`oldStyleConstructor` checker](#oldstyleconstructor-checker)
- [`paramClobber` checker](#paramclobber-checker)
Expand Down Expand Up @@ -1199,6 +1200,25 @@ function f(?string $str = null);
<p><br></p>


### `notNullSafety` checker

#### Description

Report not nullsafety call

#### Non-compliant code:
```php
function f(A $klass);
f(null);
```

#### Compliant code:
```php
reported not safety call
```
<p><br></p>


### `offBy1` checker

> Auto fix available
Expand Down
196 changes: 196 additions & 0 deletions src/linter/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,202 @@ func (b *blockWalker) handleIssetDimFetch(e *ir.ArrayDimFetchExpr) {
}
}

func nullSafetyRealParamForCheck(fn meta.FuncInfo, paramIndex int, haveVariadic bool) meta.FuncParam {
if haveVariadic && paramIndex >= len(fn.Params)-1 {
return fn.Params[len(fn.Params)-1]
}
return fn.Params[paramIndex]
}

func formatSlashesFuncName(fn meta.FuncInfo) string {
return strings.TrimPrefix(fn.Name, "\\")
}

func (b *blockWalker) checkNullSafetyCallArgsF(args []ir.Node, fn meta.FuncInfo) {
if fn.Params == nil || fn.Name == "" {
return
}
haveVariadic := fn.IsVariadic

for i, arg := range args {
if arg == nil {
continue
}

// If there are more arguments than declared and function is not variadic, ignore extra arguments.
if i > len(fn.Params)-1 && !haveVariadic {
return
}

switch a := arg.(*ir.Argument).Expr.(type) {
case *ir.SimpleVar:
b.checkSimpleVarNullSafety(arg, fn, i, a, haveVariadic)
case *ir.ConstFetchExpr:
b.checkConstFetchNullSafety(arg, fn, i, a, haveVariadic)
case *ir.ArrayDimFetchExpr:
b.checkArrayDimFetchNullSafety(arg, fn, i, a, haveVariadic)
case *ir.ListExpr:
b.checkListExprNullSafety(arg, fn, i, a, haveVariadic)
case *ir.PropertyFetchExpr:
b.checkPropertyFetchNullSafety(a, fn, i, haveVariadic)
}
}
}

func (b *blockWalker) checkSimpleVarNullSafety(arg ir.Node, fn meta.FuncInfo, paramIndex int, variable *ir.SimpleVar, haveVariadic bool) {
varInfo, ok := b.ctx.sc.GetVar(variable)
if !ok {
return
}

param := nullSafetyRealParamForCheck(fn, paramIndex, haveVariadic)
if haveVariadic && paramIndex >= len(fn.Params)-1 {
// For variadic parameter check, if type is mixed then skip.
if types.IsTypeMixed(param.Typ) {
return
}
}
paramAllowsNull := types.IsTypeNullable(param.Typ)
varIsNullable := types.IsTypeNullable(varInfo.Type)
if varIsNullable && !paramAllowsNull {
b.report(arg, LevelWarning, "notNullSafety",
"not null safety call in function %s signature of param %s",
formatSlashesFuncName(fn), param.Name)
}
}

func (b *blockWalker) checkConstFetchNullSafety(arg ir.Node, fn meta.FuncInfo, paramIndex int, constExpr *ir.ConstFetchExpr, haveVariadic bool) {
constVal := constExpr.Constant.Value
isNull := constVal == "null"

param := nullSafetyRealParamForCheck(fn, paramIndex, haveVariadic)
if haveVariadic && paramIndex >= len(fn.Params)-1 {
if types.IsTypeMixed(param.Typ) {
return
}
}
paramAllowsNull := types.IsTypeNullable(param.Typ)
if isNull && !paramAllowsNull {
b.report(arg, LevelWarning, "notNullSafety",
"null passed to non-nullable parameter %s in function %s",
param.Name, formatSlashesFuncName(fn))
}
}

func (b *blockWalker) checkArrayDimFetchNullSafety(arg ir.Node, fn meta.FuncInfo, paramIndex int, arrayExpr *ir.ArrayDimFetchExpr, haveVariadic bool) {
baseVar, ok := arrayExpr.Variable.(*ir.SimpleVar)
if !ok {
return
}

varInfo, found := b.ctx.sc.GetVar(baseVar)
if !found {
return
}

param := nullSafetyRealParamForCheck(fn, paramIndex, haveVariadic)
if haveVariadic && paramIndex >= len(fn.Params)-1 {
if types.IsTypeMixed(param.Typ) {
return
}
}
paramAllowsNull := types.IsTypeNullable(param.Typ)
if types.IsTypeNullable(varInfo.Type) && !paramAllowsNull {
b.report(arg, LevelWarning, "notNullSafety",
"potential null array access in parameter %s of function %s",
param.Name, formatSlashesFuncName(fn))
}
}

func (b *blockWalker) checkListExprNullSafety(arg ir.Node, fn meta.FuncInfo, paramIndex int, listExpr *ir.ListExpr, haveVariadic bool) {
for _, item := range listExpr.Items {
if item == nil {
continue
}

if item.Key != nil {
b.checkNullSafetyCallArgsF([]ir.Node{item.Key}, fn)
}

if item.Val != nil {
param := nullSafetyRealParamForCheck(fn, paramIndex, haveVariadic)
if simpleVar, ok := item.Val.(*ir.SimpleVar); ok {
varInfo, found := b.ctx.sc.GetVar(simpleVar)
if found && types.IsTypeNullable(varInfo.Type) && !types.IsTypeNullable(param.Typ) {
b.report(arg, LevelWarning, "notNullSafety",
"potential null value in list assignment for param %s in function %s",
param.Name, formatSlashesFuncName(fn))
}
}
b.checkNullSafetyCallArgsF([]ir.Node{item.Val}, fn)
}
}
}

func (b *blockWalker) getPropertyComputedType(expr *ir.PropertyFetchExpr) (meta.ClassInfo, types.Map) {
baseCall, ok := expr.Variable.(*ir.SimpleVar)
if !ok {
return meta.ClassInfo{}, types.Map{}
}

varInfo, ok := b.ctx.sc.GetVar(baseCall)
if !ok {
return meta.ClassInfo{}, types.Map{}
}

classInfo, ok := b.r.ctx.st.Info.GetClass(varInfo.Type.String())
if !ok {
return meta.ClassInfo{}, types.Map{}
}

property, ok := expr.Property.(*ir.Identifier)
if !ok {
return meta.ClassInfo{}, types.Map{}
}

propertyInfoFromClass := classInfo.Properties[property.Value]
return classInfo, propertyInfoFromClass.Typ
}

func (b *blockWalker) checkPropertyFetchNullSafety(expr *ir.PropertyFetchExpr, fn meta.FuncInfo, paramIndex int, haveVariadic bool) {
// Recursively check the left part of the chain if it is also a property fetch.
if nested, ok := expr.Variable.(*ir.PropertyFetchExpr); ok {
b.checkPropertyFetchNullSafety(nested, fn, paramIndex, haveVariadic)
}

classInfo, propType := b.getPropertyComputedType(expr)
if classInfo.Name == "" || propType.Empty() {
return
}

prp, ok := expr.Property.(*ir.Identifier)
if !ok {
return
}

isPrpNullable := types.IsTypeNullable(propType)
param := nullSafetyRealParamForCheck(fn, paramIndex, haveVariadic)
if haveVariadic && paramIndex >= len(fn.Params)-1 {
if types.IsTypeMixed(param.Typ) {
return
}
paramAllowsNull := types.IsTypeNullable(param.Typ)
if isPrpNullable && !paramAllowsNull {
b.report(expr, LevelWarning, "notNullSafety",
"potential null dereference when accessing property '%s'", prp.Value)
}
return
}

paramAllowsNull := types.IsTypeNullable(param.Typ)
if isPrpNullable && !paramAllowsNull {
b.report(expr, LevelWarning, "notNullSafety",
"potential null dereference when accessing property '%s'", prp.Value)
}
}
func (b *blockWalker) handleCallArgs(args []ir.Node, fn meta.FuncInfo) {
b.checkNullSafetyCallArgsF(args, fn)

for i, arg := range args {
if i >= len(fn.Params) {
arg.Walk(b)
Expand Down Expand Up @@ -1397,6 +1592,7 @@ func (b *blockWalker) enterClosure(fun *ir.ClosureExpr, haveThis bool, thisType
Flags: 0,
ExitFlags: exitFlags,
DeprecationInfo: doc.Deprecation,
IsVariadic: params.isVariadic,
})

return false
Expand Down
73 changes: 37 additions & 36 deletions src/linter/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,43 @@ import (
//
// Version log:
//
// 27 - added Static field to meta.FuncInfo
// 28 - array type parsed as mixed[]
// 29 - updated type inference for ClassConstFetch
// 30 - resolve ClassConstFetch to a wrapped type string
// 31 - fixed plus operator type inference for arrays
// 32 - replaced Static:bool with Flags:uint8 in meta.FuncInfo
// 33 - support parsing of array<k,v> and list<type>
// 34 - support parsing of ?ClassName as "ClassName|null"
// 35 - added Flags:uint8 to meta.ClassInfo
// 36 - added FuncAbstract bit to FuncFlags
// added FuncFinal bit to FuncFlags
// added ClassFinal bit to ClassFlags
// FuncInfo now stores original function name
// ClassInfo now stores original class name
// 37 - added ClassShape bit to ClassFlags
// changed meta.scopeVar bool fields representation
// 38 - replaced TypesMap.immutable:bool with flags:uint8.
// added mapPrecise flag to mark precise type maps.
// 39 - added new field Value in ConstantInfo
// 40 - changed string const value storage (no quotes)
// 41 - const-folding affected const definition values
// 42 - bool-typed consts are now stored in meta info
// 43 - define'd const values stored in cache
// 44 - rename ConstantInfo => ConstInfo
// 45 - added Mixins field to meta.ClassInfo
// 46 - changed the way of inferring the return type of functions and methods
// 47 - forced cache version invalidation due to the #921
// 48 - renamed meta.TypesMap to types.Map; this affects gob encoding
// 49 - for shape, names are now generated using the keys that make up this shape
// 50 - added Flags field for meta.PropertyInfo
// 51 - added anonymous classes
// 52 - renamed all PhpDoc and Phpdoc with PHPDoc
// 53 - added DeprecationInfo for functions and methods and support for some attributes
// 54 - forced cache version invalidation due to the #1165
// 55 - updated go version 1.16 -> 1.21
const cacheVersion = 55
// 27 - added Static field to meta.FuncInfo
// 28 - array type parsed as mixed[]
// 29 - updated type inference for ClassConstFetch
// 30 - resolve ClassConstFetch to a wrapped type string
// 31 - fixed plus operator type inference for arrays
// 32 - replaced Static:bool with Flags:uint8 in meta.FuncInfo
// 33 - support parsing of array<k,v> and list<type>
// 34 - support parsing of ?ClassName as "ClassName|null"
// 35 - added Flags:uint8 to meta.ClassInfo
// 36 - added FuncAbstract bit to FuncFlags
// added FuncFinal bit to FuncFlags
// added ClassFinal bit to ClassFlags
// FuncInfo now stores original function name
// ClassInfo now stores original class name
// 37 - added ClassShape bit to ClassFlags
// changed meta.scopeVar bool fields representation
// 38 - replaced TypesMap.immutable:bool with flags:uint8.
// added mapPrecise flag to mark precise type maps.
// 39 - added new field Value in ConstantInfo
// 40 - changed string const value storage (no quotes)
// 41 - const-folding affected const definition values
// 42 - bool-typed consts are now stored in meta info
// 43 - define'd const values stored in cache
// 44 - rename ConstantInfo => ConstInfo
// 45 - added Mixins field to meta.ClassInfo
// 46 - changed the way of inferring the return type of functions and methods
// 47 - forced cache version invalidation due to the #921
// 48 - renamed meta.TypesMap to types.Map; this affects gob encoding
// 49 - for shape, names are now generated using the keys that make up this shape
// 50 - added Flags field for meta.PropertyInfo
// 51 - added anonymous classes
// 52 - renamed all PhpDoc and Phpdoc with PHPDoc
// 53 - added DeprecationInfo for functions and methods and support for some attributes
// 54 - forced cache version invalidation due to the #1165
// 55 - updated go version 1.16 -> 1.21
// 56 - added isVariadic to meta.FuncInfo
const cacheVersion = 56

var (
errWrongVersion = errors.New("Wrong cache version")
Expand Down
4 changes: 2 additions & 2 deletions src/linter/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ main();
//
// If cache encoding changes, there is a very high chance that
// encoded data lengh will change as well.
wantLen := 5952
wantLen := 5967
haveLen := buf.Len()
if haveLen != wantLen {
t.Errorf("cache len mismatch:\nhave: %d\nwant: %d", haveLen, wantLen)
Expand All @@ -158,7 +158,7 @@ main();
// 2. Check cache "strings" hash.
//
// It catches new fields in cached types, field renames and encoding of additional named attributes.
wantStrings := "690e77c94ecdd7878de0bf6f6881d786cf1fafa4588f7905f54d700646c4952aad359008ae2dcddb1c7f29163ecee62355d525672090ac30257bc414f690006f"
wantStrings := "876ec90cb8a340db375f0f833cdb625a2a5a8560e008e111baefbad4766cc964a705c0e4f0dee76bd30a61de436986f580069c98b88b2caaed3a63a53beaae24"
haveStrings := collectCacheStrings(buf.String())
if haveStrings != wantStrings {
t.Errorf("cache strings mismatch:\nhave: %q\nwant: %q", haveStrings, wantStrings)
Expand Down
10 changes: 10 additions & 0 deletions src/linter/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ func addBuiltinCheckers(reg *CheckersRegistry) {
After: `$s = strip_tags($s, '<br>')`,
},

{
Name: "notNullSafety",
Default: true,
Quickfix: false,
Comment: "Report not nullsafety call",
Before: `function f(A $klass);
f(null);`,
After: `reported not safety call`,
},

{
Name: "notExplicitNullableParam",
Default: true,
Expand Down
Loading