From a780c237aa82dfd79155916a6f5ea5cbeddf6e69 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 4 Aug 2023 13:38:04 +0200 Subject: [PATCH 1/3] Eliminate false positive for new(js.object) --- analysis/passes/jsobjectptr/jsobjectptr.go | 2 +- .../passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/analysis/passes/jsobjectptr/jsobjectptr.go b/analysis/passes/jsobjectptr/jsobjectptr.go index 0541f4a..6c9a560 100644 --- a/analysis/passes/jsobjectptr/jsobjectptr.go +++ b/analysis/passes/jsobjectptr/jsobjectptr.go @@ -35,7 +35,7 @@ func run(pass *analysis.Pass) (interface{}, error) { parent := stack[len(stack)-2] switch pt := parent.(type) { case *ast.StarExpr: - // Fall through + return true case *ast.CallExpr: fun, ok := pt.Fun.(*ast.Ident) if !ok { diff --git a/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go b/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go index 6adc183..67a7132 100644 --- a/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go +++ b/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go @@ -22,6 +22,7 @@ var ( _ struct{ x js.Object } // want "js.Object must always be a pointer" _ = struct{ x js.Object }{} // want "js.Object must always be a pointer" _ = new(js.Object) + _ *js.Object ) func _(js.Object) {} // want "js.Object must always be a pointer" From cd7ebadcf9865958e0187802b19ecd0b55aadfbd Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 4 Aug 2023 13:42:35 +0200 Subject: [PATCH 2/3] Add AncestorN helper --- analysis/passes/jsobjectptr/jsobjectptr.go | 5 ++++- internal/jsobject.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/analysis/passes/jsobjectptr/jsobjectptr.go b/analysis/passes/jsobjectptr/jsobjectptr.go index 6c9a560..828f356 100644 --- a/analysis/passes/jsobjectptr/jsobjectptr.go +++ b/analysis/passes/jsobjectptr/jsobjectptr.go @@ -32,7 +32,10 @@ func run(pass *analysis.Pass) (interface{}, error) { if !internal.Is_jsObject(pass, node) { return true } - parent := stack[len(stack)-2] + parent, ok := internal.AncestorN(stack, 1) + if !ok { + return true + } switch pt := parent.(type) { case *ast.StarExpr: return true diff --git a/internal/jsobject.go b/internal/jsobject.go index 8a6b663..947df0d 100644 --- a/internal/jsobject.go +++ b/internal/jsobject.go @@ -43,3 +43,13 @@ func Is_jsObject(pass *analysis.Pass, node ast.Node) bool { } return pkg.Path() == "github.com/gopherjs/gopherjs/js" } + +// AncestorN returns the nth ancestor from stack, and true, or nil and false if +// it does not exist. +func AncestorN(stack []ast.Node, n int) (ast.Node, bool) { + l := len(stack) + if n > l { + return nil, false + } + return stack[l-(n+1)], true +} From 2ae6f49bc678c2fefe13a54751290bbf075f3fe8 Mon Sep 17 00:00:00 2001 From: Jonathan Hall Date: Fri, 4 Aug 2023 13:49:22 +0200 Subject: [PATCH 3/3] Add a TODO for a known failure case --- .../jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go b/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go index 67a7132..ff59c2a 100644 --- a/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go +++ b/analysis/passes/jsobjectptr/testdata/src/jsobjectptr/jsobjectptr.go @@ -34,5 +34,9 @@ func _() js.Object { // want "js.Object must always be a pointer" func _() { var x any _, _ = x.(js.Object) // want "js.Object must always be a pointer" - _ = x +} + +func _() { + var x *js.Object + _ = *x // TODO: This should fail }