Repository: VKCOM/noverify
Pull request #1151 (2021-09-08 14:06:58)

linter: fixed handling of unused variable in arrow functions


Patch:
diff --git a/src/linter/block.go b/src/linter/block.go
index 25f517e..0dc2224 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -566,11 +566,22 @@ func (b *blockWalker) trackVarName(n ir.Node, nm string) {
 	}
 }
 
-func (b *blockWalker) untrackVarName(nm string) {
+func (b *blockWalker) untrackVarNameImpl(nm string) {
 	delete(b.unusedVars, nm)
 	delete(b.unusedParams, nm)
 }
 
+func (b *blockWalker) untrackVarName(nm string) {
+	if b.inArrowFunction {
+		b.untrackVarNameImpl(nm)
+		for _, w := range b.parentBlockWalkers {
+			w.untrackVarNameImpl(nm)
+		}
+		return
+	}
+	b.untrackVarNameImpl(nm)
+}
+
 func (b *blockWalker) addVarName(n ir.Node, nm string, typ types.Map, reason string, flags meta.VarFlags) {
 	b.ctx.sc.AddVarName(nm, typ, reason, flags)
 	b.trackVarName(n, nm)


Test patch:
diff --git a/src/tests/checkers/arrow_functions_test.go b/src/tests/checkers/arrow_functions_test.go
new file mode 100644
index 0000000..760f9ef
--- /dev/null
+++ b/src/tests/checkers/arrow_functions_test.go
@@ -0,0 +1,117 @@
+package checkers
+
+import (
+	"testing"
+
+	"github.com/VKCOM/noverify/src/linttest"
+)
+
+func TestArrowFunction(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+class Boo {
+  /** @return int */
+  public function b() { }
+}
+
+function foo() {
+  $value = 10;
+
+  // simple function
+  $_ = fn($x) => $x + 5;
+
+  // with capture
+  $_ = fn($x) => $x + $value;
+
+  // reference
+  $_ = fn&($x) => $x + $value;
+
+  // with undefined variable
+  $_ = fn($x) => $x + $undefined_variable;
+
+  if ($value == 0) {
+    $maybe_defined = 100;
+  }
+
+  // with maybe defined variable
+  $_ = fn($x) => $x + $maybe_defined;
+
+  // with unused variable
+  $_ = fn($x) => $a = $x + 5;
+  $_ = fn($x) => ($a = $x + 5) && $x;
+
+  $_ = fn($x) => ($a = $x + 5) && $a + 5;
+
+  // with PHPDoc
+  /**
+   * @param Boo $x
+   */
+  $_ = fn($x) => $x->b();
+
+  // nested
+  $_ = fn($x) => fn($y) => fn($w) => $x * $y + $w - $value;
+
+  // nested with maybe defined variable
+  $_ = fn($x) => fn($y) => fn($w) => $x * $y + $w - $maybe_defined;
+
+  // nested with unused variable
+  $_ = fn($x) => fn($y) => fn($w) => $a = $x + $y + $w;
+
+  $_ = fn($x) => fn($y) => fn($w) => ($a = $x + 5) && $a + 5;
+
+  // ok
+  $_ = fn() => ($a = 10) && $a;
+
+  // $a is undefined
+  $_ = fn() => $a = 10 && $a;
+
+  // arguments are not visible outside of arrow function
+  echo $x; // Undefined $x
+  echo $y; // Undefined $y
+  echo $w; // Undefined $w
+}
+`)
+	test.Expect = []string{
+		`Cannot find referenced variable $undefined_variable`,
+		`Possibly undefined variable $maybe_defined`,
+		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Cannot find referenced variable $a`,
+		`Cannot find referenced variable $x`,
+		`Cannot find referenced variable $y`,
+		`Cannot find referenced variable $w`,
+		`Possibly undefined variable $maybe_defined`,
+	}
+	test.RunAndMatch()
+}
+
+func TestUnusedInArrowFunction(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.LoadStubs = []string{
+		`stubs/phpstorm-stubs/standard/standard_4.php`,
+		`stubs/phpstorm-stubs/standard/standard_9.php`,
+	}
+	test.AddFile(`<?php
+function f() {
+  $a1 = [];
+  $a2 = [];
+  $a3 = [];
+  $a4 = [];
+
+  var_dump(array_filter([], fn($elem) => isset($a1[$elem]))); // ok
+  var_dump(array_filter([], fn($elem) => isset($elem))); // $a2 not used
+  var_dump(array_filter([], fn($elem) => $a3)); // ok
+  var_dump(array_filter([], fn($elem) => $elem && $a4)); // ok
+
+  $_ = fn() => ($a = 10) && $a; // ok
+  $_ = fn() => ($a = 10); // $a unused
+}
+`)
+	test.Expect = []string{
+		`Variable $a2 is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+	}
+	test.RunAndMatch()
+}
diff --git a/src/tests/checkers/basic_test.go b/src/tests/checkers/basic_test.go
index 87fada0..3594f26 100644
--- a/src/tests/checkers/basic_test.go
+++ b/src/tests/checkers/basic_test.go
@@ -1639,86 +1639,6 @@ func TestFunctionJustReturns(t *testing.T) {
 	}`)
 }
 
-func TestArrowFunction(t *testing.T) {
-	test := linttest.NewSuite(t)
-	test.AddFile(`<?php
-	class Boo {
-		/** @return int */
-		public function b() { }
-	}
-
-	function foo() {
-		$value = 10;
-
-		// simple function
-		$_ = fn($x) => $x + 5;
-
-		// with capture
-		$_ = fn($x) => $x + $value;
-
-		// reference
-		$_ = fn&($x) => $x + $value;
-
-		// with undefined variable
-		$_ = fn($x) => $x + $undefined_variable;
-
-		if ($value == 0) {
-			$maybe_defined = 100;
-		}
-
-		// with maybe defined variable
-		$_ = fn($x) => $x + $maybe_defined;
-
-		// with unused variable
-		$_ = fn($x) => $a = $x + 5;
-		$_ = fn($x) => ($a = $x + 5) && $x;
-
-		$_ = fn($x) => ($a = $x + 5) && $a + 5;
-
-		// with PHPDoc
-		/**
-		 * @param Boo $x
-		 */
-		$_ = fn($x) => $x->b();
-
-		// nested
-		$_ = fn($x) => fn($y) => fn($w) => $x * $y + $w - $value;
-
-		// nested with maybe defined variable
-		$_ = fn($x) => fn($y) => fn($w) => $x * $y + $w - $maybe_defined;
-
-		// nested with unused variable
-		$_ = fn($x) => fn($y) => fn($w) => $a = $x + $y + $w;
-
-		$_ = fn($x) => fn($y) => fn($w) => ($a = $x + 5) && $a + 5;
-
-		// ok
-		$_ = fn() => ($a = 10) && $a;
-
-		// $a is undefined
-		$_ = fn() => $a = 10 && $a;
-
-		// arguments are not visible outside of arrow function
-		echo $x; // Undefined $x
-		echo $y; // Undefined $y
-		echo $w; // Undefined $w
-	}`)
-	test.Expect = []string{
-		`Cannot find referenced variable $undefined_variable`,
-		`Possibly undefined variable $maybe_defined`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Cannot find referenced variable $a`,
-		`Cannot find referenced variable $x`,
-		`Cannot find referenced variable $y`,
-		`Cannot find referenced variable $w`,
-		`Possibly undefined variable $maybe_defined`,
-	}
-	test.RunAndMatch()
-}
-
 func TestSwitchFallthrough(t *testing.T) {
 	linttest.SimpleNegativeTest(t, `<?php
 	function withFallthrough($a) {