diff --git a/issue.go b/issue.go index 8f6b6ef..f0bc1f0 100644 --- a/issue.go +++ b/issue.go @@ -18,6 +18,7 @@ import ( "fmt" "go/ast" "os" + "strconv" ) // Score type used by severity and confidence values @@ -36,7 +37,7 @@ type Issue struct { What string `json:"details"` // Human readable explanation File string `json:"file"` // File name we found it in Code string `json:"code"` // Impacted code line - Line int `json:"line"` // Line number in file + Line string `json:"line"` // Line number in file } // MetaData is embedded in all GAS rules. The Severity, Confidence and What message @@ -85,7 +86,12 @@ func NewIssue(ctx *Context, node ast.Node, desc string, severity Score, confiden var code string fobj := ctx.FileSet.File(node.Pos()) name := fobj.Name() - line := fobj.Line(node.Pos()) + + start, end := fobj.Line(node.Pos()), fobj.Line(node.End()) + line := strconv.Itoa(start) + if start != end { + line = fmt.Sprintf("%d-%d", start, end) + } if file, err := os.Open(fobj.Name()); err == nil { defer file.Close() diff --git a/issue_test.go b/issue_test.go index 534c247..451ff54 100644 --- a/issue_test.go +++ b/issue_test.go @@ -1,6 +1,11 @@ package gas_test import ( + "go/ast" + + "github.com/GoASTScanner/gas" + "github.com/GoASTScanner/gas/rules" + "github.com/GoASTScanner/gas/testutils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -8,9 +13,35 @@ import ( var _ = Describe("Issue", func() { Context("when creating a new issue", func() { - It("should provide a code snippet for the specified ast.Node", func() { - Expect(1).Should(Equal(2)) - Fail("Not implemented") + It("should create a code snippet from the specified ast.Node", func() { + var target *ast.BasicLit + source := `package main + const foo = "bar" + func main(){ + println(foo) + } + ` + pkg := testutils.NewTestPackage() + defer pkg.Close() + pkg.AddFile("foo.go", source) + ctx := pkg.CreateContext("foo.go") + v := testutils.NewMockVisitor() + v.Callback = func(n ast.Node, ctx *gas.Context) bool { + if node, ok := n.(*ast.BasicLit); ok { + target = node + return false + } + return true + } + v.Context = ctx + ast.Walk(v, ctx.Root) + Expect(target).ShouldNot(BeNil()) + + issue := gas.NewIssue(ctx, target, "", gas.High, gas.High) + Expect(issue).ShouldNot(BeNil()) + Expect(issue.Code).Should(MatchRegexp(`"bar"`)) + Expect(issue.Line).Should(Equal(2)) + }) It("should return an error if specific context is not able to be obtained", func() { @@ -21,6 +52,40 @@ var _ = Describe("Issue", func() { Fail("Not implemented") }) + It("should provide accurate line and file information for multi-line statements", func() { + var target *ast.BinaryExpr + + source := `package main + import "os" + func main(){` + source += "q := `SELECT * FROM table WHERE` + \n os.Args[1] + `= ?` // nolint: gas\n" + source += `println(q)}` + + pkg := testutils.NewTestPackage() + defer pkg.Close() + pkg.AddFile("foo.go", source) + ctx := pkg.CreateContext("foo.go") + v := testutils.NewMockVisitor() + v.Callback = func(n ast.Node, ctx *gas.Context) bool { + if node, ok := n.(*ast.BinaryExpr); ok { + target = node + } + return true + } + v.Context = ctx + ast.Walk(v, ctx.Root) + Expect(target).ShouldNot(BeNil()) + + // Use SQL rule to check binary expr + cfg := gas.NewConfig() + rule, _ := rules.NewSqlStrConcat(cfg) + issue, err := rule.Match(target, ctx) + Expect(err).ShouldNot(HaveOccurred()) + Expect(issue).ShouldNot(BeNil()) + Expect(issue.File).Should(MatchRegexp("foo.go")) + Expect(issue.Line).Should(MatchRegexp("3-4")) + }) + It("should maintain the provided severity score", func() { Fail("Not implemented") }) @@ -29,9 +94,6 @@ var _ = Describe("Issue", func() { Fail("Not implemented") }) - It("should correctly record `unsafe` import as not considered a package", func() { - Fail("Not implemented") - }) }) }) diff --git a/resolve_test.go b/resolve_test.go index 7ce2f39..f17a825 100644 --- a/resolve_test.go +++ b/resolve_test.go @@ -15,6 +15,7 @@ var _ = Describe("Resolve ast node to concrete value", func() { var basicLiteral *ast.BasicLit pkg := testutils.NewTestPackage() + defer pkg.Close() pkg.AddFile("foo.go", `package main; const foo = "bar"; func main(){}`) ctx := pkg.CreateContext("foo.go") v := testutils.NewMockVisitor() @@ -34,6 +35,7 @@ var _ = Describe("Resolve ast node to concrete value", func() { It("should successfully resolve identifier", func() { var ident *ast.Ident pkg := testutils.NewTestPackage() + defer pkg.Close() pkg.AddFile("foo.go", `package main; var foo string = "bar"; func main(){}`) ctx := pkg.CreateContext("foo.go") v := testutils.NewMockVisitor() @@ -53,6 +55,7 @@ var _ = Describe("Resolve ast node to concrete value", func() { It("should successfully resolve assign statement", func() { var assign *ast.AssignStmt pkg := testutils.NewTestPackage() + defer pkg.Close() pkg.AddFile("foo.go", `package main; const x = "bar"; func main(){ y := x; println(y) }`) ctx := pkg.CreateContext("foo.go") v := testutils.NewMockVisitor() @@ -73,6 +76,7 @@ var _ = Describe("Resolve ast node to concrete value", func() { It("should successfully resolve a binary statement", func() { var target *ast.BinaryExpr pkg := testutils.NewTestPackage() + defer pkg.Close() pkg.AddFile("foo.go", `package main; const (x = "bar"; y = "baz"); func main(){ z := x + y; println(z) }`) ctx := pkg.CreateContext("foo.go") v := testutils.NewMockVisitor() diff --git a/rule_test.go b/rule_test.go index 1eb7c86..63b1b2a 100644 --- a/rule_test.go +++ b/rule_test.go @@ -46,7 +46,7 @@ var _ = Describe("Rule", func() { What: `Some explanation of the thing`, File: "main.go", Code: `#include int main(){ puts("hello world"); }`, - Line: 42, + Line: "42", }, err: nil, callback: func(n ast.Node, ctx *gas.Context) bool { return true }, diff --git a/rules/rules_test.go b/rules/rules_test.go index 0bc921b..e0fd6ca 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -32,6 +32,7 @@ var _ = Describe("gas rules", func() { for n, sample := range samples { analyzer.Reset() pkg := testutils.NewTestPackage() + defer pkg.Close() pkg.AddFile(fmt.Sprintf("sample_%d.go", n), sample.Code) pkg.Build() e := analyzer.Process(pkg.Path)