Repository: VKCOM/noverify
Pull request #1045 (2021-07-08 10:50:07)

linter: added handling of the case when a static method was called through a variable


Patch:
diff --git a/src/linter/block.go b/src/linter/block.go
index e96d65d..413f010 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -1002,7 +1002,7 @@ func (b *blockWalker) handleMethodCall(e *ir.MethodCallExpr) bool {
 }
 
 func (b *blockWalker) handleStaticCall(e *ir.StaticCallExpr) bool {
-	call := resolveStaticMethodCall(b.r.ctx.st, e)
+	call := resolveStaticMethodCall(b.ctx.sc, b.r.ctx.st, e)
 	if !b.callsParentConstructor {
 		b.callsParentConstructor = call.isCallsParentConstructor
 	}
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index 73ce731..8cffc7c 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -1063,7 +1063,7 @@ func (b *blockLinter) checkMethodCall(e *ir.MethodCallExpr) {
 }
 
 func (b *blockLinter) checkStaticCall(e *ir.StaticCallExpr) {
-	call := resolveStaticMethodCall(b.classParseState(), e)
+	call := resolveStaticMethodCall(b.walker.ctx.sc, b.classParseState(), e)
 	if !call.canAnalyze {
 		return
 	}
diff --git a/src/linter/utils.go b/src/linter/utils.go
index a38b363..0740a2d 100644
--- a/src/linter/utils.go
+++ b/src/linter/utils.go
@@ -294,7 +294,7 @@ type staticMethodCallInfo struct {
 	canAnalyze               bool
 }
 
-func resolveStaticMethodCall(st *meta.ClassParseState, e *ir.StaticCallExpr) staticMethodCallInfo {
+func resolveStaticMethodCall(scope *meta.Scope, st *meta.ClassParseState, e *ir.StaticCallExpr) staticMethodCallInfo {
 	if !st.Info.IsIndexingComplete() {
 		return staticMethodCallInfo{canAnalyze: false}
 	}
@@ -308,15 +308,61 @@ func resolveStaticMethodCall(st *meta.ClassParseState, e *ir.StaticCallExpr) sta
 		return staticMethodCallInfo{canAnalyze: false}
 	}
 
-	classNameNode, ok := e.Class.(*ir.Name)
-	parentCall := ok && classNameNode.Value == "parent"
+	var ok bool
+	var className string
+	var parentCall bool
 	var callsParentConstructor bool
-	if parentCall && methodName == "__construct" {
-		callsParentConstructor = true
-	}
 
-	className, ok := solver.GetClassName(st, e.Class)
-	if !ok {
+	switch n := e.Class.(type) {
+	case *ir.Name:
+		parentCall = n.Value == "parent"
+		if parentCall && methodName == "__construct" {
+			callsParentConstructor = true
+		}
+
+		className, ok = solver.GetClassName(st, e.Class)
+		if !ok {
+			return staticMethodCallInfo{canAnalyze: false}
+		}
+	case *ir.Identifier:
+		className, ok = solver.GetClassName(st, e.Class)
+		if !ok {
+			return staticMethodCallInfo{canAnalyze: false}
+		}
+	case *ir.SimpleVar:
+		tp, ok := scope.GetVarNameType(n.Name)
+		if !ok {
+			return staticMethodCallInfo{canAnalyze: false}
+		}
+
+		// We need to resolve the types here, as the function
+		// may return a class or a string with the class name.
+		if !tp.IsResolved() {
+			resolvedTypes := solver.ResolveTypes(st.Info, st.CurrentClass, tp, solver.ResolverMap{})
+			tp = types.NewMapFromMap(resolvedTypes)
+		}
+
+		var isClass bool
+		var isString bool
+		var isMixed bool
+		tp.Iterate(func(typ string) {
+			isString = typ == "string"
+			isMixed = typ == "mixed"
+			if !isString && !isMixed {
+				_, isClass = st.Info.GetClass(typ)
+			}
+		})
+
+		if !isClass && !isString && !isMixed {
+			return staticMethodCallInfo{canAnalyze: false}
+		}
+
+		if !isClass || tp.Len() != 1 {
+			return staticMethodCallInfo{canAnalyze: false}
+		}
+
+		className = tp.String()
+	default:
 		return staticMethodCallInfo{canAnalyze: false}
 	}
 


Test patch:
diff --git a/src/tests/checkers/oop_test.go b/src/tests/checkers/oop_test.go
index 68e5c74..779b59c 100644
--- a/src/tests/checkers/oop_test.go
+++ b/src/tests/checkers/oop_test.go
@@ -1648,3 +1648,54 @@ class Foo5 {
 }
 `)
 }
+
+func TestCallStaticWithVariable(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+class Foo {
+  /** */
+  public static function some_method() {}
+}
+function ret_int() {
+  return 12;
+}
+function ret_string() {
+  return "Foo";
+}
+function ret_object() {
+  return new Foo();
+}
+function f($arg) {
+  $foo = new Foo();
+  $foo::some_method(); // Ok
+  $foo::non_existing_method(); // Error
+
+  $a = 10;
+  $a::some_method(); // invalid class name
+
+  $foo2 = new Foo();
+  $foo3 = new Foo();
+  $foo4 = $arg;
+
+  $foo5 = ret_string();
+  $foo6 = ret_object();
+  $foo7 = ret_int();
+
+  if ($a > 100) {
+    $foo2 = "Foo";
+    $foo3 = 10;
+  }
+
+  $foo2::some_method(); // Skip, via \Foo|string type (both is correct for class name)
+  $foo3::some_method(); // Error, int type is invalid class name
+  $foo4::some_method(); // Skip, via mixed type
+  $foo5::some_method(); // Ok ret_string returns the string
+  $foo6::some_method(); // Ok ret_object returns the Foo object
+  $foo7::some_method(); // Error, ret_int returns the int
+}
+`)
+	test.Expect = []string{
+		`Call to undefined method \Foo::non_existing_method()`,
+	}
+	test.RunAndMatch()
+}