Repository: VKCOM/noverify
Pull request #619 (2020-07-30 18:28:25)

linter: collect and expose AST node path
For every visited node, push it to the node path.
Pop node when we leave the node.

If EnterNode() returns false, we need to do a pop
since LeaveNode() will not be called in that case.

Where we can use it?
- Get current node parent.
- Find whether this node is an argument of specific func call.
- Find whether this node located under if/for/etc.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>

quasilyte: I don't think I covered all branch nodes, so there probably will be 1 more update to make it more exhaustive.

Patch:
diff --git a/src/linter/block.go b/src/linter/block.go
index eea5f7f..21f8e89 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -58,6 +58,8 @@ type BlockWalker struct {
 
 	custom []BlockChecker
 
+	path NodePath
+
 	ignoreFunctionBodies bool
 	rootLevel            bool // analysing root-level code
 
@@ -224,6 +226,7 @@ func (b *BlockWalker) EnterNode(w walker.Walkable) (res bool) {
 	}
 
 	n := w.(node.Node)
+	b.path.push(n)
 
 	if b.ctx.exitFlags != 0 {
 		b.reportDeadCode(n)
@@ -488,6 +491,9 @@ func (b *BlockWalker) EnterNode(w walker.Walkable) (res bool) {
 		}
 	}
 
+	if !res {
+		b.path.pop()
+	}
 	return res
 }
 
@@ -2444,6 +2450,8 @@ func (b *BlockWalker) LeaveNode(w walker.Walkable) {
 		c.BeforeLeaveNode(w)
 	}
 
+	b.path.pop()
+
 	if b.ctx.exitFlags == 0 {
 		switch w.(type) {
 		case *stmt.Return:
diff --git a/src/linter/custom.go b/src/linter/custom.go
index 5057ece..f6d522d 100644
--- a/src/linter/custom.go
+++ b/src/linter/custom.go
@@ -165,6 +165,12 @@ type BlockContext struct {
 	w *BlockWalker
 }
 
+// NodePath returns a node path up to the current traversal position.
+// The path includes the node that is being traversed as well.
+func (ctx *BlockContext) NodePath() NodePath {
+	return ctx.w.path
+}
+
 // ExprType resolves the type of e expression node.
 func (ctx *BlockContext) ExprType(e node.Node) meta.TypesMap {
 	return ctx.w.exprType(e)
diff --git a/src/linter/nodepath.go b/src/linter/nodepath.go
new file mode 100644
index 0000000..968cad0
--- /dev/null
+++ b/src/linter/nodepath.go
@@ -0,0 +1,82 @@
+package linter
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/VKCOM/noverify/src/php/parser/node"
+	"github.com/VKCOM/noverify/src/php/parser/node/expr"
+	"github.com/VKCOM/noverify/src/php/parser/node/stmt"
+)
+
+type NodePath struct {
+	stack []node.Node
+}
+
+func newNodePath() NodePath {
+	return NodePath{stack: make([]node.Node, 0, 20)}
+}
+
+func (p NodePath) String() string {
+	parts := make([]string, len(p.stack))
+	for i, n := range p.stack {
+		parts[i] = fmt.Sprintf("%T", n)
+	}
+	return strings.Join(parts, "/")
+}
+
+func (p NodePath) Parent() node.Node {
+	return p.NthParent(1)
+}
+
+func (p NodePath) Current() node.Node {
+	return p.NthParent(0)
+}
+
+func (p NodePath) Conditional() bool {
+	return p.ConditionalUntil(p.Current())
+}
+
+func (p NodePath) NthParent(n int) node.Node {
+	index := len(p.stack) - n - 1
+	if index >= 0 {
+		return p.stack[index]
+	}
+	return nil
+}
+
+func (p NodePath) ConditionalUntil(end node.Node) bool {
+	// TODO: can we report if statement as unconditioncal?
+	// stmt.If is pushed before we handle condition,
+	// so it will occure as a path element.
+
+	for _, n := range p.stack {
+		if n == end {
+			break
+		}
+
+		switch n.(type) {
+		// Obvious branching.
+		case *stmt.If, *stmt.ElseIf, *expr.Ternary:
+			return true
+		// Else executes only if condition failed.
+		case *stmt.Else:
+			return true
+		// Loops that can be executed 0 times.
+		case *stmt.For, *stmt.Foreach, *stmt.While:
+			return true
+		case *stmt.Catch:
+			return true
+		}
+	}
+
+	return false
+}
+
+func (p *NodePath) push(n node.Node) {
+	p.stack = append(p.stack, n)
+}
+
+func (p *NodePath) pop() {
+	p.stack = p.stack[:len(p.stack)-1]
+}
diff --git a/src/linter/parser.go b/src/linter/parser.go
index 744e5cd..65957e9 100644
--- a/src/linter/parser.go
+++ b/src/linter/parser.go
@@ -210,6 +210,7 @@ func AnalyzeFileRootLevel(rootNode node.Node, d *RootWalker) {
 		nonLocalVars:         make(map[string]struct{}),
 		ignoreFunctionBodies: true,
 		rootLevel:            true,
+		path:                 newNodePath(),
 	}
 
 	for _, createFn := range d.customBlock {
diff --git a/src/linter/root.go b/src/linter/root.go
index 4aff1b7..d574751 100644
--- a/src/linter/root.go
+++ b/src/linter/root.go
@@ -647,6 +647,7 @@ func (d *RootWalker) handleFuncStmts(params []meta.FuncParam, uses, stmts []node
 		r:            d,
 		unusedVars:   make(map[string][]node.Node),
 		nonLocalVars: make(map[string]struct{}),
+		path:         newNodePath(),
 	}
 	for _, createFn := range d.customBlock {
 		b.custom = append(b.custom, createFn(&BlockContext{w: b}))


Test patch:
diff --git a/src/tests/custom/path_test.go b/src/tests/custom/path_test.go
new file mode 100644
index 0000000..e3e7802
--- /dev/null
+++ b/src/tests/custom/path_test.go
@@ -0,0 +1,161 @@
+package custom
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/VKCOM/noverify/src/linter"
+	"github.com/VKCOM/noverify/src/linttest"
+	"github.com/VKCOM/noverify/src/meta"
+	"github.com/VKCOM/noverify/src/php/parser/node"
+	"github.com/VKCOM/noverify/src/php/parser/walker"
+)
+
+func init() {
+	linter.RegisterBlockChecker(func(ctx *linter.BlockContext) linter.BlockChecker {
+		return &pathTester{ctx: ctx}
+	})
+}
+
+func TestPathIfElse(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+if ($cond1) {
+  echo $_if;
+} elseif ($cond2) {
+  echo $_elseif1;
+} else if ($cond3) {
+  echi $_elseif2;
+} else {
+  echo $_else;
+}
+`)
+	test.Expect = []string{
+		`$_if (cond=true) : *node.Root/*stmt.If/*stmt.StmtList/*stmt.Echo/*node.SimpleVar`,
+		`$_elseif1 (cond=true) : *node.Root/*stmt.If/*stmt.StmtList/*stmt.Echo/*node.SimpleVar`,
+		`$_elseif2 (cond=true) : *node.Root/*stmt.If/*stmt.StmtList/*stmt.Expression/*node.SimpleVar`,
+		`$_else (cond=true) : *node.Root/*stmt.If/*stmt.Else/*stmt.StmtList/*stmt.Echo/*node.SimpleVar`,
+	}
+	linttest.RunFilterMatch(test, "pathTest")
+}
+
+func TestPathArgument(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+f($_arg1, [$_arg2], $a + $_arg3);
+`)
+	test.Expect = []string{
+		`$_arg1 (cond=false) : *node.Root/*stmt.Expression/*expr.FunctionCall/*node.Argument/*node.SimpleVar`,
+		`$_arg2 (cond=false) : *node.Root/*stmt.Expression/*expr.FunctionCall/*node.Argument/*expr.Array/*node.SimpleVar`,
+		`$_arg3 (cond=false) : *node.Root/*stmt.Expression/*expr.FunctionCall/*node.Argument/*binary.Plus/*node.SimpleVar`,
+	}
+	linttest.RunFilterMatch(test, "pathTest")
+}
+
+func TestPathTernary(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+echo $_ternary_cond ? $_ternary_true : $_ternary_false;
+`)
+	test.Expect = []string{
+		`$_ternary_cond (cond=true) : *node.Root/*stmt.Echo/*expr.Ternary/*node.SimpleVar`,
+		`$_ternary_true (cond=true) : *node.Root/*stmt.Echo/*expr.Ternary/*node.SimpleVar`,
+		`$_ternary_false (cond=true) : *node.Root/*stmt.Echo/*expr.Ternary/*node.SimpleVar`,
+	}
+	linttest.RunFilterMatch(test, "pathTest")
+}
+
+func TestPathTopLevel(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+if ($_cond) {
+  echo $_x1;
+  switch (true) {
+  case $_case:
+    echo $_x2;
+    break;
+  }
+}
+
+do {
+  $_at_least_once;
+} while ($_do_while_cond);
+`)
+	test.Expect = []string{
+		`$_cond (cond=true) : *node.Root/*stmt.If/*node.SimpleVar`,
+		`$_x1 (cond=true) : *node.Root/*stmt.If/*stmt.StmtList/*stmt.Echo/*node.SimpleVar`,
+		`$_case (cond=true) : *node.Root/*stmt.If/*stmt.StmtList/*stmt.Switch/*node.SimpleVar`,
+		`$_x2 (cond=true) : *node.Root/*stmt.If/*stmt.StmtList/*stmt.Switch/*stmt.Echo/*node.SimpleVar`,
+
+		`$_at_least_once (cond=false) : *node.Root/*stmt.Do/*stmt.StmtList/*stmt.Expression/*node.SimpleVar`,
+		`$_do_while_cond (cond=false) : *node.Root/*stmt.Do/*node.SimpleVar`,
+	}
+	linttest.RunFilterMatch(test, "pathTest")
+}
+
+func TestPathFuncScope(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+function f() {
+  echo $_inside_func;
+  for (;;) {
+    while (false) {
+      return $_inside_loops;
+    }
+    return $_inside_loop;
+  }
+  return $_outside_loops;
+}
+`)
+	test.Expect = []string{
+		`$_inside_func (cond=false) : *stmt.Echo/*node.SimpleVar`,
+		`$_inside_loops (cond=true) : *stmt.For/*stmt.StmtList/*stmt.While/*stmt.StmtList/*stmt.Return/*node.SimpleVar`,
+		`$_inside_loop (cond=true) : *stmt.For/*stmt.StmtList/*stmt.Return/*node.SimpleVar`,
+		`$_outside_loops (cond=false) : *stmt.Return/*node.SimpleVar`,
+	}
+	linttest.RunFilterMatch(test, "pathTest")
+}
+
+func TestPathMethodScope(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+class C {
+  public function f() {
+    echo $_inside_func;
+    for (;;) {
+      while (false) {
+        return $_inside_loops;
+      }
+      return $_inside_loop;
+    }
+    return $_outside_loops;
+  }
+}
+`)
+	test.Expect = []string{
+		`$_inside_func (cond=false) : *stmt.Echo/*node.SimpleVar`,
+		`$_inside_loops (cond=true) : *stmt.For/*stmt.StmtList/*stmt.While/*stmt.StmtList/*stmt.Return/*node.SimpleVar`,
+		`$_inside_loop (cond=true) : *stmt.For/*stmt.StmtList/*stmt.Return/*node.SimpleVar`,
+		`$_outside_loops (cond=false) : *stmt.Return/*node.SimpleVar`,
+	}
+	linttest.RunFilterMatch(test, "pathTest")
+}
+
+type pathTester struct {
+	linter.BlockCheckerDefaults
+	ctx *linter.BlockContext
+}
+
+func (b *pathTester) AfterEnterNode(w walker.Walkable) {
+	if !meta.IsIndexingComplete() {
+		return
+	}
+
+	x, ok := w.(*node.SimpleVar)
+	if !ok || !strings.HasPrefix(x.Name, "_") {
+		return
+	}
+
+	path := b.ctx.NodePath()
+	b.ctx.Report(x, linter.LevelInformation, "pathTest", "$%s (cond=%v) : %s", x.Name, path.Conditional(), path.String())
+}