From 464450fd3a4be7683b7e9dea3317e11b4239df2b Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 12:59:04 +0100 Subject: [PATCH 01/11] Detect variables captured by closures tool --- .../detect_captured_variables.go | 75 +++++++++++++++++++ tools/code-analysis/examples/captured.go | 13 ++++ .../examples/captured_as_argument.go | 13 ++++ tools/code-analysis/examples/not_captured.go | 12 +++ .../examples/pointer_captured.go | 14 ++++ tools/code-analysis/go.mod | 3 + 6 files changed, 130 insertions(+) create mode 100644 tools/code-analysis/detect_captured_variables.go create mode 100644 tools/code-analysis/examples/captured.go create mode 100644 tools/code-analysis/examples/captured_as_argument.go create mode 100644 tools/code-analysis/examples/not_captured.go create mode 100644 tools/code-analysis/examples/pointer_captured.go create mode 100644 tools/code-analysis/go.mod diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go new file mode 100644 index 000000000..f165be6e9 --- /dev/null +++ b/tools/code-analysis/detect_captured_variables.go @@ -0,0 +1,75 @@ +package main + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" +) + +func main() { + // Get the file path from the FILENAME environment variable + filePath := os.Getenv("FILENAME") + if filePath == "" { + fmt.Println("FILENAME environment variable is not set.") + os.Exit(1) + } + + // Parse the source code + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + // Create a visitor function + ast.Inspect(node, func(n ast.Node) bool { + // Check if node is a function literal (closure) + funcLit, ok := n.(*ast.FuncLit) + if !ok { + return true + } + + // Track variables declared inside the closure + declaredVars := make(map[string]bool) + + ast.Inspect(funcLit, func(nn ast.Node) bool { + switch innerNode := nn.(type) { + case *ast.AssignStmt: + for _, expr := range innerNode.Lhs { + if ident, ok := expr.(*ast.Ident); ok { + declaredVars[ident.Name] = true + } + } + case *ast.ValueSpec: + for _, ident := range innerNode.Names { + declaredVars[ident.Name] = true + } + } + return true + }) + + // Check for captured variables + ast.Inspect(funcLit, func(nn ast.Node) bool { + ident, ok := nn.(*ast.Ident) + if !ok { + return true + } + + if ident.Obj == nil || ident.Obj.Kind != ast.Var { + return true + } + + if _, isDeclared := declaredVars[ident.Name]; !isDeclared { + pos := fset.Position(ident.Pos()) + fmt.Printf("Variable '%s' captured by closure at %s:%d\n", ident.Name, pos.Filename, pos.Line) + } + + return true + }) + + return true + }) +} diff --git a/tools/code-analysis/examples/captured.go b/tools/code-analysis/examples/captured.go new file mode 100644 index 000000000..f54114571 --- /dev/null +++ b/tools/code-analysis/examples/captured.go @@ -0,0 +1,13 @@ +package main + +import "fmt" + +func captured() { + x := 42 + + fn := func() { + fmt.Println("Captured variable:", x) + } + + fn() +} diff --git a/tools/code-analysis/examples/captured_as_argument.go b/tools/code-analysis/examples/captured_as_argument.go new file mode 100644 index 000000000..fd6f15a57 --- /dev/null +++ b/tools/code-analysis/examples/captured_as_argument.go @@ -0,0 +1,13 @@ +package main + +import "fmt" + +func captured_as_argument() { + x := 42 + + fn := func(y int) { + fmt.Println("Variable passed as argument:", y) + } + + fn(x) +} diff --git a/tools/code-analysis/examples/not_captured.go b/tools/code-analysis/examples/not_captured.go new file mode 100644 index 000000000..0b8a3862a --- /dev/null +++ b/tools/code-analysis/examples/not_captured.go @@ -0,0 +1,12 @@ +package main + +import "fmt" + +func not_captured() { + fn := func() { + y := 42 + fmt.Println("Local variable:", y) + } + + fn() +} diff --git a/tools/code-analysis/examples/pointer_captured.go b/tools/code-analysis/examples/pointer_captured.go new file mode 100644 index 000000000..388085638 --- /dev/null +++ b/tools/code-analysis/examples/pointer_captured.go @@ -0,0 +1,14 @@ +package main + +import "fmt" + +func pointer_captured() { + x := 42 + ptr := &x + + fn := func() { + fmt.Println("Captured pointer:", *ptr) + } + + fn() +} diff --git a/tools/code-analysis/go.mod b/tools/code-analysis/go.mod new file mode 100644 index 000000000..db1358483 --- /dev/null +++ b/tools/code-analysis/go.mod @@ -0,0 +1,3 @@ +module github.com/iotaledger/iota-core/tools/code-analysis + +go 1.21.3 From 2d300da8bfd7661856fff4053507d58d6a23de63 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 13:09:15 +0100 Subject: [PATCH 02/11] More complex examples --- .../detect_captured_variables.go | 9 +++ tools/code-analysis/examples/complex.go | 55 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tools/code-analysis/examples/complex.go diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go index f165be6e9..8773c8909 100644 --- a/tools/code-analysis/detect_captured_variables.go +++ b/tools/code-analysis/detect_captured_variables.go @@ -62,6 +62,15 @@ func main() { return true } + // Check if the variable is a parameter of the function literal + for _, param := range funcLit.Type.Params.List { + for _, name := range param.Names { + if name.Name == ident.Name { + return true // Skip parameters, they are not captured variables + } + } + } + if _, isDeclared := declaredVars[ident.Name]; !isDeclared { pos := fset.Position(ident.Pos()) fmt.Printf("Variable '%s' captured by closure at %s:%d\n", ident.Name, pos.Filename, pos.Line) diff --git a/tools/code-analysis/examples/complex.go b/tools/code-analysis/examples/complex.go new file mode 100644 index 000000000..f0d4f04dd --- /dev/null +++ b/tools/code-analysis/examples/complex.go @@ -0,0 +1,55 @@ +package main + +import "fmt" + +func main() { + // Captured variables + captured1 := 1 + captured2 := 2 + + // Non-captured variables + nonCaptured1 := 10 + nonCaptured2 := 20 + + // Nested function with captured variables + func() { + fmt.Println("Captured1:", captured1) + func() { + fmt.Println("Captured2:", captured2) + }() + }() + + // Nested function with argument, not captured + func(x int, y int) { + fmt.Println("Non-captured1:", x) + fmt.Println("Non-captured2:", y) + }(nonCaptured1, nonCaptured2) + + // Non-captured variables with shadowing + func() { + nonCaptured1 := 30 + nonCaptured2 := 40 + fmt.Println("Shadowed non-captured1:", nonCaptured1) + fmt.Println("Shadowed non-captured2:", nonCaptured2) + }() + + // Non-captured variable in a loop + for i := 0; i < 3; i++ { + go func(n int) { + fmt.Println("Captured in loop:", n) + }(i) + } + + // Captured variable in a loop + for i := 0; i < 3; i++ { + go func() { + fmt.Println("Captured in loop:", i) + }() + } + + // Captured by pointer + func() { + ptr := &captured1 + fmt.Println("Captured by pointer:", *ptr) + }() +} From a7b44153c5f58e33637833464f305e61816381bf Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 13:12:48 +0100 Subject: [PATCH 03/11] Better print formatting --- tools/code-analysis/detect_captured_variables.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go index 8773c8909..2f9ae26b6 100644 --- a/tools/code-analysis/detect_captured_variables.go +++ b/tools/code-analysis/detect_captured_variables.go @@ -73,7 +73,7 @@ func main() { if _, isDeclared := declaredVars[ident.Name]; !isDeclared { pos := fset.Position(ident.Pos()) - fmt.Printf("Variable '%s' captured by closure at %s:%d\n", ident.Name, pos.Filename, pos.Line) + fmt.Printf("%s:%d: variable '%s' captured by closure\n", pos.Filename, pos.Line, ident.Name) } return true From a3ebe5d7e8cccd3fe6792b273d73a0c8d054bff0 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 13:17:03 +0100 Subject: [PATCH 04/11] Example with struct fields --- tools/code-analysis/examples/complex.go | 2 +- .../code-analysis/examples/complex_struct.go | 77 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 tools/code-analysis/examples/complex_struct.go diff --git a/tools/code-analysis/examples/complex.go b/tools/code-analysis/examples/complex.go index f0d4f04dd..80188ca22 100644 --- a/tools/code-analysis/examples/complex.go +++ b/tools/code-analysis/examples/complex.go @@ -2,7 +2,7 @@ package main import "fmt" -func main() { +func complex() { // Captured variables captured1 := 1 captured2 := 2 diff --git a/tools/code-analysis/examples/complex_struct.go b/tools/code-analysis/examples/complex_struct.go new file mode 100644 index 000000000..f8e83533f --- /dev/null +++ b/tools/code-analysis/examples/complex_struct.go @@ -0,0 +1,77 @@ +package main + +import ( + "fmt" +) + +type Animal interface { + Speak() string +} + +type Dog struct { + Name string +} + +func (d Dog) Speak() string { + return "Woof!" +} + +type ClosureHolder struct { + fn func() string +} + +func complex_struct() { + // Captured variables + captured := "I'm captured!" + alsoCaptured := 42 + + // Non-captured variables + notCaptured := "I'm free!" + alsoNotCaptured := 84 + + // Struct with a captured field + dog := Dog{Name: "Rex"} + + // Nested function with captured variable + func() { + fmt.Println(captured) + }() + + // Interface holding struct with a non-captured field + var a Animal = Dog{Name: notCaptured} + + // Function capturing a field of a struct + func() { + fmt.Println("Captured field: ", dog.Name) + }() + + // Nested closures, second one capturing a variable from the first + func() { + nestedCapture := "I'm also captured!" + func() { + fmt.Println(nestedCapture) + fmt.Println(alsoCaptured) + }() + }() + + // Struct capturing a local variable through a closure + holder := ClosureHolder{ + fn: func() string { + return captured + }, + } + + // Using the captured variable through the struct's function field + fmt.Println("Through struct: ", holder.fn()) + + // Interface method call + func(a Animal) { + fmt.Println(a.Speak()) + }(a) + + // Non-captured variable passed as a function argument + func(s string, n int) { + fmt.Println("Non-captured string:", s) + fmt.Println("Non-captured integer:", n) + }(notCaptured, alsoNotCaptured) +} From 9ac7440da30f866cfb3b1ce16169e5c1d415ea5a Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 13:36:29 +0100 Subject: [PATCH 05/11] Do not report captured method pointer receiver --- .../detect_captured_variables.go | 93 ++++++++++--------- .../examples/captured_pointer_receiver.go | 60 ++++++++++++ 2 files changed, 109 insertions(+), 44 deletions(-) create mode 100644 tools/code-analysis/examples/captured_pointer_receiver.go diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go index 2f9ae26b6..0e3bd065e 100644 --- a/tools/code-analysis/detect_captured_variables.go +++ b/tools/code-analysis/detect_captured_variables.go @@ -9,14 +9,12 @@ import ( ) func main() { - // Get the file path from the FILENAME environment variable filePath := os.Getenv("FILENAME") if filePath == "" { fmt.Println("FILENAME environment variable is not set.") os.Exit(1) } - // Parse the source code fset := token.NewFileSet() node, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) if err != nil { @@ -24,61 +22,68 @@ func main() { os.Exit(1) } - // Create a visitor function - ast.Inspect(node, func(n ast.Node) bool { - // Check if node is a function literal (closure) - funcLit, ok := n.(*ast.FuncLit) - if !ok { - return true - } + var currentMethodReceiver string - // Track variables declared inside the closure - declaredVars := make(map[string]bool) + ast.Inspect(node, func(n ast.Node) bool { + switch t := n.(type) { - ast.Inspect(funcLit, func(nn ast.Node) bool { - switch innerNode := nn.(type) { - case *ast.AssignStmt: - for _, expr := range innerNode.Lhs { - if ident, ok := expr.(*ast.Ident); ok { - declaredVars[ident.Name] = true + case *ast.FuncDecl: + if t.Recv != nil { // This is a method + for _, field := range t.Recv.List { + if len(field.Names) > 0 { + currentMethodReceiver = field.Names[0].Name } } - case *ast.ValueSpec: - for _, ident := range innerNode.Names { - declaredVars[ident.Name] = true - } + } else { + currentMethodReceiver = "" } - return true - }) - // Check for captured variables - ast.Inspect(funcLit, func(nn ast.Node) bool { - ident, ok := nn.(*ast.Ident) - if !ok { - return true - } + case *ast.FuncLit: + declaredVars := make(map[string]bool) - if ident.Obj == nil || ident.Obj.Kind != ast.Var { + ast.Inspect(t, func(nn ast.Node) bool { + switch innerNode := nn.(type) { + case *ast.AssignStmt: + for _, expr := range innerNode.Lhs { + if ident, ok := expr.(*ast.Ident); ok { + declaredVars[ident.Name] = true + } + } + case *ast.ValueSpec: + for _, ident := range innerNode.Names { + declaredVars[ident.Name] = true + } + } return true - } + }) - // Check if the variable is a parameter of the function literal - for _, param := range funcLit.Type.Params.List { - for _, name := range param.Names { - if name.Name == ident.Name { - return true // Skip parameters, they are not captured variables - } + ast.Inspect(t, func(nn ast.Node) bool { + ident, ok := nn.(*ast.Ident) + if !ok { + return true } - } - if _, isDeclared := declaredVars[ident.Name]; !isDeclared { - pos := fset.Position(ident.Pos()) - fmt.Printf("%s:%d: variable '%s' captured by closure\n", pos.Filename, pos.Line, ident.Name) - } + if ident.Obj == nil || ident.Obj.Kind != ast.Var { + return true + } - return true - }) + for _, param := range t.Type.Params.List { + for _, name := range param.Names { + if name.Name == ident.Name { + return true + } + } + } + if _, isDeclared := declaredVars[ident.Name]; !isDeclared { + if ident.Name != currentMethodReceiver { + pos := fset.Position(ident.Pos()) + fmt.Printf("%s:%d: variable '%s' captured by closure\n", pos.Filename, pos.Line, ident.Name) + } + } + return true + }) + } return true }) } diff --git a/tools/code-analysis/examples/captured_pointer_receiver.go b/tools/code-analysis/examples/captured_pointer_receiver.go new file mode 100644 index 000000000..fb86bda0f --- /dev/null +++ b/tools/code-analysis/examples/captured_pointer_receiver.go @@ -0,0 +1,60 @@ +package main + +import ( + "fmt" +) + +type MyStruct struct { + field string +} + +func (m *MyStruct) MethodWithClosure() { + capturedInMethod := "captured in method" + + // Closure capturing a variable declared inside the method + func() { + fmt.Println(capturedInMethod) + }() + + // Closure not capturing any variables + func() { + fmt.Println("Not capturing anything.") + }() + + // Closure capturing the method's receiver (should not be reported) + func() { + fmt.Println(m.field) + }() +} + +func main() { + // Variable declarations + captured := "I am captured!" + alsoCaptured := "Me too!" + notCaptured := "I am free!" + + // Closure capturing a variable + func() { + fmt.Println(captured) + }() + + // Closure not capturing any variable + func() { + fmt.Println("Nothing captured here.") + }() + + // Closure capturing multiple variables + func() { + fmt.Println(captured) + fmt.Println(alsoCaptured) + }() + + // A method that has a closure which captures a variable + m := &MyStruct{field: "I'm a field!"} + m.MethodWithClosure() + + // Closure with variables passed as arguments (should not be reported) + func(nc string) { + fmt.Println(nc) + }(notCaptured) +} From c992e9b24cdd42d7bedcb7c205b24fb2f2a9fdab Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 14:10:31 +0100 Subject: [PATCH 06/11] Only report modified-after-capture --- .../detect_captured_variables.go | 14 +++- .../examples/modified_after_capture.go | 83 +++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 tools/code-analysis/examples/modified_after_capture.go diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go index 0e3bd065e..4d2d5b71e 100644 --- a/tools/code-analysis/detect_captured_variables.go +++ b/tools/code-analysis/detect_captured_variables.go @@ -23,6 +23,7 @@ func main() { } var currentMethodReceiver string + capturedVars := make(map[string]token.Pos) ast.Inspect(node, func(n ast.Node) bool { switch t := n.(type) { @@ -38,6 +39,16 @@ func main() { currentMethodReceiver = "" } + case *ast.AssignStmt: + for _, expr := range t.Lhs { + if ident, ok := expr.(*ast.Ident); ok { + if pos, captured := capturedVars[ident.Name]; captured { + pos := fset.Position(pos) + fmt.Printf("%s:%d: variable '%s' captured and modified outside closure\n", pos.Filename, pos.Line, ident.Name) + } + } + } + case *ast.FuncLit: declaredVars := make(map[string]bool) @@ -77,8 +88,7 @@ func main() { if _, isDeclared := declaredVars[ident.Name]; !isDeclared { if ident.Name != currentMethodReceiver { - pos := fset.Position(ident.Pos()) - fmt.Printf("%s:%d: variable '%s' captured by closure\n", pos.Filename, pos.Line, ident.Name) + capturedVars[ident.Name] = ident.Pos() } } return true diff --git a/tools/code-analysis/examples/modified_after_capture.go b/tools/code-analysis/examples/modified_after_capture.go new file mode 100644 index 000000000..f4e5515c0 --- /dev/null +++ b/tools/code-analysis/examples/modified_after_capture.go @@ -0,0 +1,83 @@ +package main + +import ( + "fmt" +) + +func modified_after_capture() { + // Scenario 1: Captured and Modified + capturedAndModified := "captured" + func() { + fmt.Println("Scenario 1:", capturedAndModified) + }() + capturedAndModified = "modified" + + // Scenario 2: Captured but Not Modified + capturedNotModified := "captured" + func() { + fmt.Println("Scenario 2:", capturedNotModified) + }() + + // Scenario 3: Multiple variables + var1 := "var1" + var2 := "var2" + func() { + fmt.Println("Scenario 3:", var1, var2) + }() + var1 = "modified_var1" + var2 = "modified_var2" + + // Scenario 4: Not captured + func() { + fmt.Println("Scenario 4: Not capturing anything.") + }() +} + +func nested_modified_after_capture() { + // Outermost scope variables + outerCapturedAndModified := "outer_captured" + + // Scenario 1: Nested Closures + func() { + // Middle scope variables + middleCapturedAndModified := "middle_captured" + + func() { + // Innermost scope + fmt.Println("Nested Scenario 1:", outerCapturedAndModified, middleCapturedAndModified) + }() + middleCapturedAndModified = "middle_modified" + }() + outerCapturedAndModified = "outer_modified" + + // Scenario 2: Functions returning functions + outerFuncVar := "outer_func_var" + + fn := func() func() { + innerFuncVar := "inner_func_var" + return func() { + fmt.Println("Scenario 2:", outerFuncVar, innerFuncVar) + } + }() + + fn() + outerFuncVar = "outer_func_var_modified" + + // Scenario 3: Combined nested and functions returning functions + combinedVar := "combined_var" + + fn2 := func() func() { + return func() { + fmt.Println("Combined Scenario:", combinedVar) + } + }() + + func() func() { + return func() { + fmt.Println("Combined Scenario:", combinedVar) + } + }() + + fn2() + combinedVar = "combined_var_modified" +} From 98a36e7a1c82f4c0b22af60122f8eb6aa5cf179f Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 14:14:51 +0100 Subject: [PATCH 07/11] Provide the two versions of the analyzer --- .../detect_captured_variables.go | 14 +-- ...etect_modified_after_captured_variables.go | 99 +++++++++++++++++++ 2 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 tools/code-analysis/detect_modified_after_captured_variables.go diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go index 4d2d5b71e..0e3bd065e 100644 --- a/tools/code-analysis/detect_captured_variables.go +++ b/tools/code-analysis/detect_captured_variables.go @@ -23,7 +23,6 @@ func main() { } var currentMethodReceiver string - capturedVars := make(map[string]token.Pos) ast.Inspect(node, func(n ast.Node) bool { switch t := n.(type) { @@ -39,16 +38,6 @@ func main() { currentMethodReceiver = "" } - case *ast.AssignStmt: - for _, expr := range t.Lhs { - if ident, ok := expr.(*ast.Ident); ok { - if pos, captured := capturedVars[ident.Name]; captured { - pos := fset.Position(pos) - fmt.Printf("%s:%d: variable '%s' captured and modified outside closure\n", pos.Filename, pos.Line, ident.Name) - } - } - } - case *ast.FuncLit: declaredVars := make(map[string]bool) @@ -88,7 +77,8 @@ func main() { if _, isDeclared := declaredVars[ident.Name]; !isDeclared { if ident.Name != currentMethodReceiver { - capturedVars[ident.Name] = ident.Pos() + pos := fset.Position(ident.Pos()) + fmt.Printf("%s:%d: variable '%s' captured by closure\n", pos.Filename, pos.Line, ident.Name) } } return true diff --git a/tools/code-analysis/detect_modified_after_captured_variables.go b/tools/code-analysis/detect_modified_after_captured_variables.go new file mode 100644 index 000000000..4d2d5b71e --- /dev/null +++ b/tools/code-analysis/detect_modified_after_captured_variables.go @@ -0,0 +1,99 @@ +package main + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" +) + +func main() { + filePath := os.Getenv("FILENAME") + if filePath == "" { + fmt.Println("FILENAME environment variable is not set.") + os.Exit(1) + } + + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + var currentMethodReceiver string + capturedVars := make(map[string]token.Pos) + + ast.Inspect(node, func(n ast.Node) bool { + switch t := n.(type) { + + case *ast.FuncDecl: + if t.Recv != nil { // This is a method + for _, field := range t.Recv.List { + if len(field.Names) > 0 { + currentMethodReceiver = field.Names[0].Name + } + } + } else { + currentMethodReceiver = "" + } + + case *ast.AssignStmt: + for _, expr := range t.Lhs { + if ident, ok := expr.(*ast.Ident); ok { + if pos, captured := capturedVars[ident.Name]; captured { + pos := fset.Position(pos) + fmt.Printf("%s:%d: variable '%s' captured and modified outside closure\n", pos.Filename, pos.Line, ident.Name) + } + } + } + + case *ast.FuncLit: + declaredVars := make(map[string]bool) + + ast.Inspect(t, func(nn ast.Node) bool { + switch innerNode := nn.(type) { + case *ast.AssignStmt: + for _, expr := range innerNode.Lhs { + if ident, ok := expr.(*ast.Ident); ok { + declaredVars[ident.Name] = true + } + } + case *ast.ValueSpec: + for _, ident := range innerNode.Names { + declaredVars[ident.Name] = true + } + } + return true + }) + + ast.Inspect(t, func(nn ast.Node) bool { + ident, ok := nn.(*ast.Ident) + if !ok { + return true + } + + if ident.Obj == nil || ident.Obj.Kind != ast.Var { + return true + } + + for _, param := range t.Type.Params.List { + for _, name := range param.Names { + if name.Name == ident.Name { + return true + } + } + } + + if _, isDeclared := declaredVars[ident.Name]; !isDeclared { + if ident.Name != currentMethodReceiver { + capturedVars[ident.Name] = ident.Pos() + } + } + return true + }) + } + return true + }) +} From efa53025d9d6086ec33d0ead1f750f19b7a2a4e0 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 14:36:21 +0100 Subject: [PATCH 08/11] Improvements --- .../detect_modified_after_captured_variables.go | 10 ++++++++-- .../examples/redeclared_in_different_scope.go | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 tools/code-analysis/examples/redeclared_in_different_scope.go diff --git a/tools/code-analysis/detect_modified_after_captured_variables.go b/tools/code-analysis/detect_modified_after_captured_variables.go index 4d2d5b71e..6be3003b0 100644 --- a/tools/code-analysis/detect_modified_after_captured_variables.go +++ b/tools/code-analysis/detect_modified_after_captured_variables.go @@ -43,8 +43,10 @@ func main() { for _, expr := range t.Lhs { if ident, ok := expr.(*ast.Ident); ok { if pos, captured := capturedVars[ident.Name]; captured { - pos := fset.Position(pos) - fmt.Printf("%s:%d: variable '%s' captured and modified outside closure\n", pos.Filename, pos.Line, ident.Name) + capturedPos := fset.Position(pos) + modifiedPos := fset.Position(ident.Pos()) + fmt.Printf("%s:%d: variable '%s' captured by closure and modified at %s:%d\n", + capturedPos.Filename, capturedPos.Line, ident.Name, modifiedPos.Filename, modifiedPos.Line) } } } @@ -74,6 +76,10 @@ func main() { return true } + if ident.Name == "_" { // skip underscore variables + return true + } + if ident.Obj == nil || ident.Obj.Kind != ast.Var { return true } diff --git a/tools/code-analysis/examples/redeclared_in_different_scope.go b/tools/code-analysis/examples/redeclared_in_different_scope.go new file mode 100644 index 000000000..cd78294ad --- /dev/null +++ b/tools/code-analysis/examples/redeclared_in_different_scope.go @@ -0,0 +1,14 @@ +package main + +import "fmt" + +func redaclered() { + x := 42 + + // Outer closure + func() { + fmt.Println(x) + }() + + x = 2 +} From a8f35c6b6ae11183247be667993ee0265b15a350 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:10:07 +0100 Subject: [PATCH 09/11] Ignore _ variable --- tools/code-analysis/detect_captured_variables.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go index 0e3bd065e..86798ac7f 100644 --- a/tools/code-analysis/detect_captured_variables.go +++ b/tools/code-analysis/detect_captured_variables.go @@ -63,6 +63,10 @@ func main() { return true } + if ident.Name == "_" { // skip underscore variables + return true + } + if ident.Obj == nil || ident.Obj.Kind != ast.Var { return true } From 53498145a6b2aaa62d8dd1aeb8c7438c1913dbaa Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:10:14 +0100 Subject: [PATCH 10/11] Fix context capture --- components/dashboard/component.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/dashboard/component.go b/components/dashboard/component.go index 62326f082..c442a4102 100644 --- a/components/dashboard/component.go +++ b/components/dashboard/component.go @@ -68,7 +68,7 @@ func run() error { Component.LogInfo("Starting Dashboard ... done") stopped := make(chan struct{}) - go func() { + go func(ctx context.Context) { server.Server.BaseContext = func(_ net.Listener) context.Context { // set BaseContext to be the same as the plugin, so that requests being processed don't hang the shutdown procedure return ctx @@ -81,7 +81,7 @@ func run() error { } close(stopped) } - }() + }(ctx) // stop if we are shutting down or the server could not be started select { From 3234a12ea492a5feabc37af38ad65470a3fe8459 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Mon, 13 Nov 2023 12:47:36 +0100 Subject: [PATCH 11/11] Moved tools to a separate repo --- .../detect_captured_variables.go | 93 ---------------- ...etect_modified_after_captured_variables.go | 105 ------------------ tools/code-analysis/examples/captured.go | 13 --- .../examples/captured_as_argument.go | 13 --- .../examples/captured_pointer_receiver.go | 60 ---------- tools/code-analysis/examples/complex.go | 55 --------- .../code-analysis/examples/complex_struct.go | 77 ------------- .../examples/modified_after_capture.go | 83 -------------- tools/code-analysis/examples/not_captured.go | 12 -- .../examples/pointer_captured.go | 14 --- .../examples/redeclared_in_different_scope.go | 14 --- tools/code-analysis/go.mod | 3 - 12 files changed, 542 deletions(-) delete mode 100644 tools/code-analysis/detect_captured_variables.go delete mode 100644 tools/code-analysis/detect_modified_after_captured_variables.go delete mode 100644 tools/code-analysis/examples/captured.go delete mode 100644 tools/code-analysis/examples/captured_as_argument.go delete mode 100644 tools/code-analysis/examples/captured_pointer_receiver.go delete mode 100644 tools/code-analysis/examples/complex.go delete mode 100644 tools/code-analysis/examples/complex_struct.go delete mode 100644 tools/code-analysis/examples/modified_after_capture.go delete mode 100644 tools/code-analysis/examples/not_captured.go delete mode 100644 tools/code-analysis/examples/pointer_captured.go delete mode 100644 tools/code-analysis/examples/redeclared_in_different_scope.go delete mode 100644 tools/code-analysis/go.mod diff --git a/tools/code-analysis/detect_captured_variables.go b/tools/code-analysis/detect_captured_variables.go deleted file mode 100644 index 86798ac7f..000000000 --- a/tools/code-analysis/detect_captured_variables.go +++ /dev/null @@ -1,93 +0,0 @@ -package main - -import ( - "fmt" - "go/ast" - "go/parser" - "go/token" - "os" -) - -func main() { - filePath := os.Getenv("FILENAME") - if filePath == "" { - fmt.Println("FILENAME environment variable is not set.") - os.Exit(1) - } - - fset := token.NewFileSet() - node, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) - if err != nil { - fmt.Println(err) - os.Exit(1) - } - - var currentMethodReceiver string - - ast.Inspect(node, func(n ast.Node) bool { - switch t := n.(type) { - - case *ast.FuncDecl: - if t.Recv != nil { // This is a method - for _, field := range t.Recv.List { - if len(field.Names) > 0 { - currentMethodReceiver = field.Names[0].Name - } - } - } else { - currentMethodReceiver = "" - } - - case *ast.FuncLit: - declaredVars := make(map[string]bool) - - ast.Inspect(t, func(nn ast.Node) bool { - switch innerNode := nn.(type) { - case *ast.AssignStmt: - for _, expr := range innerNode.Lhs { - if ident, ok := expr.(*ast.Ident); ok { - declaredVars[ident.Name] = true - } - } - case *ast.ValueSpec: - for _, ident := range innerNode.Names { - declaredVars[ident.Name] = true - } - } - return true - }) - - ast.Inspect(t, func(nn ast.Node) bool { - ident, ok := nn.(*ast.Ident) - if !ok { - return true - } - - if ident.Name == "_" { // skip underscore variables - return true - } - - if ident.Obj == nil || ident.Obj.Kind != ast.Var { - return true - } - - for _, param := range t.Type.Params.List { - for _, name := range param.Names { - if name.Name == ident.Name { - return true - } - } - } - - if _, isDeclared := declaredVars[ident.Name]; !isDeclared { - if ident.Name != currentMethodReceiver { - pos := fset.Position(ident.Pos()) - fmt.Printf("%s:%d: variable '%s' captured by closure\n", pos.Filename, pos.Line, ident.Name) - } - } - return true - }) - } - return true - }) -} diff --git a/tools/code-analysis/detect_modified_after_captured_variables.go b/tools/code-analysis/detect_modified_after_captured_variables.go deleted file mode 100644 index 6be3003b0..000000000 --- a/tools/code-analysis/detect_modified_after_captured_variables.go +++ /dev/null @@ -1,105 +0,0 @@ -package main - -import ( - "fmt" - "go/ast" - "go/parser" - "go/token" - "os" -) - -func main() { - filePath := os.Getenv("FILENAME") - if filePath == "" { - fmt.Println("FILENAME environment variable is not set.") - os.Exit(1) - } - - fset := token.NewFileSet() - node, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) - if err != nil { - fmt.Println(err) - os.Exit(1) - } - - var currentMethodReceiver string - capturedVars := make(map[string]token.Pos) - - ast.Inspect(node, func(n ast.Node) bool { - switch t := n.(type) { - - case *ast.FuncDecl: - if t.Recv != nil { // This is a method - for _, field := range t.Recv.List { - if len(field.Names) > 0 { - currentMethodReceiver = field.Names[0].Name - } - } - } else { - currentMethodReceiver = "" - } - - case *ast.AssignStmt: - for _, expr := range t.Lhs { - if ident, ok := expr.(*ast.Ident); ok { - if pos, captured := capturedVars[ident.Name]; captured { - capturedPos := fset.Position(pos) - modifiedPos := fset.Position(ident.Pos()) - fmt.Printf("%s:%d: variable '%s' captured by closure and modified at %s:%d\n", - capturedPos.Filename, capturedPos.Line, ident.Name, modifiedPos.Filename, modifiedPos.Line) - } - } - } - - case *ast.FuncLit: - declaredVars := make(map[string]bool) - - ast.Inspect(t, func(nn ast.Node) bool { - switch innerNode := nn.(type) { - case *ast.AssignStmt: - for _, expr := range innerNode.Lhs { - if ident, ok := expr.(*ast.Ident); ok { - declaredVars[ident.Name] = true - } - } - case *ast.ValueSpec: - for _, ident := range innerNode.Names { - declaredVars[ident.Name] = true - } - } - return true - }) - - ast.Inspect(t, func(nn ast.Node) bool { - ident, ok := nn.(*ast.Ident) - if !ok { - return true - } - - if ident.Name == "_" { // skip underscore variables - return true - } - - if ident.Obj == nil || ident.Obj.Kind != ast.Var { - return true - } - - for _, param := range t.Type.Params.List { - for _, name := range param.Names { - if name.Name == ident.Name { - return true - } - } - } - - if _, isDeclared := declaredVars[ident.Name]; !isDeclared { - if ident.Name != currentMethodReceiver { - capturedVars[ident.Name] = ident.Pos() - } - } - return true - }) - } - return true - }) -} diff --git a/tools/code-analysis/examples/captured.go b/tools/code-analysis/examples/captured.go deleted file mode 100644 index f54114571..000000000 --- a/tools/code-analysis/examples/captured.go +++ /dev/null @@ -1,13 +0,0 @@ -package main - -import "fmt" - -func captured() { - x := 42 - - fn := func() { - fmt.Println("Captured variable:", x) - } - - fn() -} diff --git a/tools/code-analysis/examples/captured_as_argument.go b/tools/code-analysis/examples/captured_as_argument.go deleted file mode 100644 index fd6f15a57..000000000 --- a/tools/code-analysis/examples/captured_as_argument.go +++ /dev/null @@ -1,13 +0,0 @@ -package main - -import "fmt" - -func captured_as_argument() { - x := 42 - - fn := func(y int) { - fmt.Println("Variable passed as argument:", y) - } - - fn(x) -} diff --git a/tools/code-analysis/examples/captured_pointer_receiver.go b/tools/code-analysis/examples/captured_pointer_receiver.go deleted file mode 100644 index fb86bda0f..000000000 --- a/tools/code-analysis/examples/captured_pointer_receiver.go +++ /dev/null @@ -1,60 +0,0 @@ -package main - -import ( - "fmt" -) - -type MyStruct struct { - field string -} - -func (m *MyStruct) MethodWithClosure() { - capturedInMethod := "captured in method" - - // Closure capturing a variable declared inside the method - func() { - fmt.Println(capturedInMethod) - }() - - // Closure not capturing any variables - func() { - fmt.Println("Not capturing anything.") - }() - - // Closure capturing the method's receiver (should not be reported) - func() { - fmt.Println(m.field) - }() -} - -func main() { - // Variable declarations - captured := "I am captured!" - alsoCaptured := "Me too!" - notCaptured := "I am free!" - - // Closure capturing a variable - func() { - fmt.Println(captured) - }() - - // Closure not capturing any variable - func() { - fmt.Println("Nothing captured here.") - }() - - // Closure capturing multiple variables - func() { - fmt.Println(captured) - fmt.Println(alsoCaptured) - }() - - // A method that has a closure which captures a variable - m := &MyStruct{field: "I'm a field!"} - m.MethodWithClosure() - - // Closure with variables passed as arguments (should not be reported) - func(nc string) { - fmt.Println(nc) - }(notCaptured) -} diff --git a/tools/code-analysis/examples/complex.go b/tools/code-analysis/examples/complex.go deleted file mode 100644 index 80188ca22..000000000 --- a/tools/code-analysis/examples/complex.go +++ /dev/null @@ -1,55 +0,0 @@ -package main - -import "fmt" - -func complex() { - // Captured variables - captured1 := 1 - captured2 := 2 - - // Non-captured variables - nonCaptured1 := 10 - nonCaptured2 := 20 - - // Nested function with captured variables - func() { - fmt.Println("Captured1:", captured1) - func() { - fmt.Println("Captured2:", captured2) - }() - }() - - // Nested function with argument, not captured - func(x int, y int) { - fmt.Println("Non-captured1:", x) - fmt.Println("Non-captured2:", y) - }(nonCaptured1, nonCaptured2) - - // Non-captured variables with shadowing - func() { - nonCaptured1 := 30 - nonCaptured2 := 40 - fmt.Println("Shadowed non-captured1:", nonCaptured1) - fmt.Println("Shadowed non-captured2:", nonCaptured2) - }() - - // Non-captured variable in a loop - for i := 0; i < 3; i++ { - go func(n int) { - fmt.Println("Captured in loop:", n) - }(i) - } - - // Captured variable in a loop - for i := 0; i < 3; i++ { - go func() { - fmt.Println("Captured in loop:", i) - }() - } - - // Captured by pointer - func() { - ptr := &captured1 - fmt.Println("Captured by pointer:", *ptr) - }() -} diff --git a/tools/code-analysis/examples/complex_struct.go b/tools/code-analysis/examples/complex_struct.go deleted file mode 100644 index f8e83533f..000000000 --- a/tools/code-analysis/examples/complex_struct.go +++ /dev/null @@ -1,77 +0,0 @@ -package main - -import ( - "fmt" -) - -type Animal interface { - Speak() string -} - -type Dog struct { - Name string -} - -func (d Dog) Speak() string { - return "Woof!" -} - -type ClosureHolder struct { - fn func() string -} - -func complex_struct() { - // Captured variables - captured := "I'm captured!" - alsoCaptured := 42 - - // Non-captured variables - notCaptured := "I'm free!" - alsoNotCaptured := 84 - - // Struct with a captured field - dog := Dog{Name: "Rex"} - - // Nested function with captured variable - func() { - fmt.Println(captured) - }() - - // Interface holding struct with a non-captured field - var a Animal = Dog{Name: notCaptured} - - // Function capturing a field of a struct - func() { - fmt.Println("Captured field: ", dog.Name) - }() - - // Nested closures, second one capturing a variable from the first - func() { - nestedCapture := "I'm also captured!" - func() { - fmt.Println(nestedCapture) - fmt.Println(alsoCaptured) - }() - }() - - // Struct capturing a local variable through a closure - holder := ClosureHolder{ - fn: func() string { - return captured - }, - } - - // Using the captured variable through the struct's function field - fmt.Println("Through struct: ", holder.fn()) - - // Interface method call - func(a Animal) { - fmt.Println(a.Speak()) - }(a) - - // Non-captured variable passed as a function argument - func(s string, n int) { - fmt.Println("Non-captured string:", s) - fmt.Println("Non-captured integer:", n) - }(notCaptured, alsoNotCaptured) -} diff --git a/tools/code-analysis/examples/modified_after_capture.go b/tools/code-analysis/examples/modified_after_capture.go deleted file mode 100644 index f4e5515c0..000000000 --- a/tools/code-analysis/examples/modified_after_capture.go +++ /dev/null @@ -1,83 +0,0 @@ -package main - -import ( - "fmt" -) - -func modified_after_capture() { - // Scenario 1: Captured and Modified - capturedAndModified := "captured" - func() { - fmt.Println("Scenario 1:", capturedAndModified) - }() - capturedAndModified = "modified" - - // Scenario 2: Captured but Not Modified - capturedNotModified := "captured" - func() { - fmt.Println("Scenario 2:", capturedNotModified) - }() - - // Scenario 3: Multiple variables - var1 := "var1" - var2 := "var2" - func() { - fmt.Println("Scenario 3:", var1, var2) - }() - var1 = "modified_var1" - var2 = "modified_var2" - - // Scenario 4: Not captured - func() { - fmt.Println("Scenario 4: Not capturing anything.") - }() -} - -func nested_modified_after_capture() { - // Outermost scope variables - outerCapturedAndModified := "outer_captured" - - // Scenario 1: Nested Closures - func() { - // Middle scope variables - middleCapturedAndModified := "middle_captured" - - func() { - // Innermost scope - fmt.Println("Nested Scenario 1:", outerCapturedAndModified, middleCapturedAndModified) - }() - middleCapturedAndModified = "middle_modified" - }() - outerCapturedAndModified = "outer_modified" - - // Scenario 2: Functions returning functions - outerFuncVar := "outer_func_var" - - fn := func() func() { - innerFuncVar := "inner_func_var" - return func() { - fmt.Println("Scenario 2:", outerFuncVar, innerFuncVar) - } - }() - - fn() - outerFuncVar = "outer_func_var_modified" - - // Scenario 3: Combined nested and functions returning functions - combinedVar := "combined_var" - - fn2 := func() func() { - return func() { - fmt.Println("Combined Scenario:", combinedVar) - } - }() - - func() func() { - return func() { - fmt.Println("Combined Scenario:", combinedVar) - } - }() - - fn2() - combinedVar = "combined_var_modified" -} diff --git a/tools/code-analysis/examples/not_captured.go b/tools/code-analysis/examples/not_captured.go deleted file mode 100644 index 0b8a3862a..000000000 --- a/tools/code-analysis/examples/not_captured.go +++ /dev/null @@ -1,12 +0,0 @@ -package main - -import "fmt" - -func not_captured() { - fn := func() { - y := 42 - fmt.Println("Local variable:", y) - } - - fn() -} diff --git a/tools/code-analysis/examples/pointer_captured.go b/tools/code-analysis/examples/pointer_captured.go deleted file mode 100644 index 388085638..000000000 --- a/tools/code-analysis/examples/pointer_captured.go +++ /dev/null @@ -1,14 +0,0 @@ -package main - -import "fmt" - -func pointer_captured() { - x := 42 - ptr := &x - - fn := func() { - fmt.Println("Captured pointer:", *ptr) - } - - fn() -} diff --git a/tools/code-analysis/examples/redeclared_in_different_scope.go b/tools/code-analysis/examples/redeclared_in_different_scope.go deleted file mode 100644 index cd78294ad..000000000 --- a/tools/code-analysis/examples/redeclared_in_different_scope.go +++ /dev/null @@ -1,14 +0,0 @@ -package main - -import "fmt" - -func redaclered() { - x := 42 - - // Outer closure - func() { - fmt.Println(x) - }() - - x = 2 -} diff --git a/tools/code-analysis/go.mod b/tools/code-analysis/go.mod deleted file mode 100644 index db1358483..000000000 --- a/tools/code-analysis/go.mod +++ /dev/null @@ -1,3 +0,0 @@ -module github.com/iotaledger/iota-core/tools/code-analysis - -go 1.21.3