Repository: VKCOM/noverify
Pull request #1064 (2021-07-16 11:10:13)

linter: added `argsReverse` checker
It checks if parameters are passed in reverse order.
For functions with two parameters only.


Patch:
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index 0eab7cf..57e4c5f 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -892,6 +892,30 @@ func (b *blockLinter) checkFunctionAvailability(e *ir.FunctionCallExpr, call *fu
 
 func (b *blockLinter) checkCallArgs(fun ir.Node, args []ir.Node, fn meta.FuncInfo, callerClass string) {
 	b.checkCallArgsCount(fun, args, fn, callerClass)
+	b.checkArgsOrder(fun, args, fn)
+}
+
+func (b *blockLinter) checkArgsOrder(fun ir.Node, args []ir.Node, fn meta.FuncInfo) {
+	if len(args) != 2 || len(fn.Params) < 2 {
+		return
+	}
+
+	firstArg := args[0].(*ir.Argument)
+	secondArg := args[1].(*ir.Argument)
+
+	firstVar, ok := firstArg.Expr.(*ir.SimpleVar)
+	if !ok {
+		return
+	}
+
+	secondVar, ok := secondArg.Expr.(*ir.SimpleVar)
+	if !ok {
+		return
+	}
+
+	if firstVar.Name == fn.Params[1].Name && secondVar.Name == fn.Params[0].Name {
+		b.report(fun, LevelWarning, "argsReverse", "Perhaps the order of the arguments is messed up, $%[1]s is passed to the $%[2]s parameter, and $%[2]s is passed to the $%[1]s parameter", firstVar.Name, secondVar.Name)
+	}
 }
 
 func (b *blockLinter) checkCallArgsCount(fun ir.Node, args []ir.Node, fn meta.FuncInfo, callerClass string) {
diff --git a/src/linter/report.go b/src/linter/report.go
index 519ea5f..6ddc55e 100644
--- a/src/linter/report.go
+++ b/src/linter/report.go
@@ -835,6 +835,34 @@ class Boo extends Foo {
 
 class Boo extends Foo {
   public function f() {}
+}`,
+		},
+
+		{
+			// Checker can give many false positives, however it is
+			// useful for periodic checking when you can choose what
+			// appears to be a real error.
+			Name:     "argsReverse",
+			Default:  false,
+			Quickfix: false,
+			Comment:  `Report using variables as arguments in reverse order.`,
+			Before: `function makeHello(string $name, int $age) {
+  echo "Hello ${$name}-${$age}";
+}
+
+function main(): void {
+  $name = "John";
+  $age = 18;
+  makeHello($age, $name); // The name should come first, and then the age.
+}`,
+			After: `function makeHello(string $name, int $age) {
+  echo "Hello ${$name}-${$age}";
+}
+
+function main(): void {
+  $name = "John";
+  $age = 18;
+  makeHello($name, $age);
 }`,
 		},
 	}


Test patch:
diff --git a/src/tests/checkers/functions_test.go b/src/tests/checkers/functions_test.go
index f972fc6..99ec388 100644
--- a/src/tests/checkers/functions_test.go
+++ b/src/tests/checkers/functions_test.go
@@ -67,3 +67,48 @@ function main(callable $a) {
 	}
 	linttest.RunFilterMatch(test, "argCount")
 }
+
+func TestArgsMessedUp(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+function makeHello(string $name, int $age) {
+  echo "Hello ${$name}-${$age}";
+}
+
+function makeHello3(string $name, int $age, bool $gender) {
+  echo "Hello ${$name}-${$age}-${$gender}";
+}
+
+function makeHello1(string $name) {
+  echo "Hello ${$name}";
+}
+
+function getAge(): int {
+  return 18;
+}
+
+function main(): void {
+  $name = "John";
+  $age = 18;
+  $gender = true;
+  makeHello($age, $name); // The name should come first, and then the age.
+  makeHello($name, $age); // ok
+  makeHello("John", $age); // ok
+  makeHello("Anna", 18); // ok
+  makeHello("John", getAge()); // ok
+  makeHello("John"); // ok
+
+  makeHello3($age, $name, $gender); // ok, 3 args
+  makeHello3($age, "Anna", $gender); // ok, 3 args
+  makeHello3($name, $age, true); // ok, 3 args
+
+  makeHello1($name); // ok, < 2 args
+  makeHello1("Anna"); // ok, < 2 args
+  makeHello1("Anna", 18); // ok
+}
+`)
+	test.Expect = []string{
+		`Perhaps the order of the arguments is messed up, $age is passed to the $name parameter, and $name is passed to the $age parameter`,
+	}
+	linttest.RunFilterMatch(test, "argsReverse")
+}