Major rework of codebase

- Get rid of 'core' and move CLI to cmd/gas directory
- Migrate (most) tests to use Ginkgo and testutils framework
- GAS now expects package to reside in $GOPATH
- GAS now can resolve dependencies for better type checking (if package
  on GOPATH)
- Simplified public API

Author: gcmurphy

analyzer.go

@@ -18,9 +18,11 @@ package gas
 import (
 	"go/ast"
 	"go/build"
+	"go/parser"
 	"go/token"
 	"go/types"
 	"log"
+	"os"
 	"path"
 	"reflect"
 	"strings"
@@ -64,10 +66,11 @@ type Analyzer struct {
 // NewAnalyzer builds a new anaylzer.
 func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer {
 	ignoreNoSec := false
-	if val, err := conf.Get("ignoreNoSec"); err == nil {
-		if override, ok := val.(bool); ok {
-			ignoreNoSec = override
-		}
+	if setting, err := conf.GetGlobal("nosec"); err == nil {
+		ignoreNoSec = setting == "true" || setting == "enabled"
+	}
+	if logger == nil {
+		logger = log.New(os.Stderr, "[gas]", log.LstdFlags)
 	}
 	return &Analyzer{
 		ignoreNosec: ignoreNoSec,
@@ -94,7 +97,7 @@ func (gas *Analyzer) Process(packagePath string) error {
 		return err
 	}
 
-	packageConfig := loader.Config{Build: &build.Default}
+	packageConfig := loader.Config{Build: &build.Default, ParserMode: parser.ParseComments}
 	packageFiles := make([]string, 0)
 	for _, filename := range basePackage.GoFiles {
 		packageFiles = append(packageFiles, path.Join(packagePath, filename))
@@ -168,3 +171,10 @@ func (gas *Analyzer) Visit(n ast.Node) ast.Visitor {
 func (gas *Analyzer) Report() ([]*Issue, *Metrics) {
 	return gas.issues, gas.stats
 }
+
+// Reset clears state such as context, issues and metrics from the configured analyzer
+func (gas *Analyzer) Reset() {
+	gas.context = &Context{}
+	gas.issues = make([]*Issue, 0, 16)
+	gas.stats = &Metrics{}
+}

analyzer_test.go

@@ -0,0 +1,113 @@
+package gas_test
+
+import (
+	"bytes"
+	"io/ioutil"
+	"log"
+	"os"
+	"strings"
+
+	"github.com/GoASTScanner/gas"
+	"github.com/GoASTScanner/gas/rules"
+
+	"github.com/GoASTScanner/gas/testutils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Analyzer", func() {
+
+	var (
+		analyzer *gas.Analyzer
+		logger   *log.Logger
+		output   *bytes.Buffer
+	)
+	BeforeEach(func() {
+		logger, output = testutils.NewLogger()
+		analyzer = gas.NewAnalyzer(nil, logger)
+	})
+
+	Context("when processing a package", func() {
+
+		It("should return an error if the package contains no Go files", func() {
+			analyzer.LoadRules(rules.Generate().Builders()...)
+			dir, err := ioutil.TempDir("", "empty")
+			defer os.RemoveAll(dir)
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(dir)
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).Should(MatchRegexp("no buildable Go source files"))
+		})
+
+		It("should return an error if the package fails to build", func() {
+			analyzer.LoadRules(rules.Generate().Builders()...)
+			pkg := testutils.NewTestPackage()
+			defer pkg.Close()
+			pkg.AddFile("wonky.go", `func main(){ println("forgot the package")}`)
+			pkg.Build()
+
+			err := analyzer.Process(pkg.Path)
+			Expect(err).Should(HaveOccurred())
+			Expect(err.Error()).Should(MatchRegexp(`expected 'package'`))
+
+		})
+
+		It("should find errors when nosec is not in use", func() {
+
+			// Rule for MD5 weak crypto usage
+			sample := testutils.SampleCodeG401[0]
+			source := sample.Code
+			analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...)
+
+			controlPackage := testutils.NewTestPackage()
+			defer controlPackage.Close()
+			controlPackage.AddFile("md5.go", source)
+			controlPackage.Build()
+			analyzer.Process(controlPackage.Path)
+			controlIssues, _ := analyzer.Report()
+			Expect(controlIssues).Should(HaveLen(sample.Errors))
+
+		})
+
+		It("should not report errors when a nosec comment is present", func() {
+			// Rule for MD5 weak crypto usage
+			sample := testutils.SampleCodeG401[0]
+			source := sample.Code
+			analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...)
+
+			nosecPackage := testutils.NewTestPackage()
+			defer nosecPackage.Close()
+			nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec", 1)
+			nosecPackage.AddFile("md5.go", nosecSource)
+			nosecPackage.Build()
+
+			analyzer.Process(nosecPackage.Path)
+			nosecIssues, _ := analyzer.Report()
+			Expect(nosecIssues).Should(BeEmpty())
+		})
+	})
+
+	It("should be possible to overwrite nosec comments, and report issues", func() {
+
+		// Rule for MD5 weak crypto usage
+		sample := testutils.SampleCodeG401[0]
+		source := sample.Code
+
+		// overwrite nosec option
+		nosecIgnoreConfig := gas.NewConfig()
+		nosecIgnoreConfig.SetGlobal("nosec", "true")
+		customAnalyzer := gas.NewAnalyzer(nosecIgnoreConfig, logger)
+		customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...)
+
+		nosecPackage := testutils.NewTestPackage()
+		defer nosecPackage.Close()
+		nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec", 1)
+		nosecPackage.AddFile("md5.go", nosecSource)
+		nosecPackage.Build()
+
+		customAnalyzer.Process(nosecPackage.Path)
+		nosecIssues, _ := customAnalyzer.Report()
+		Expect(nosecIssues).Should(HaveLen(sample.Errors))
+
+	})
+})

call_list.go

@@ -55,19 +55,19 @@ func (c CallList) Contains(selector, ident string) bool {
 
 /// ContainsCallExpr resolves the call expression name and type
 /// or package and determines if it exists within the CallList
-func (c CallList) ContainsCallExpr(n ast.Node, ctx *Context) bool {
+func (c CallList) ContainsCallExpr(n ast.Node, ctx *Context) *ast.CallExpr {
 	selector, ident, err := GetCallInfo(n, ctx)
 	if err != nil {
-		return false
+		return nil
 	}
 	// Try direct resolution
 	if c.Contains(selector, ident) {
-		return true
+		return n.(*ast.CallExpr)
 	}
 
 	// Also support explicit path
-	if path, ok := GetImportPath(selector, ctx); ok {
-		return c.Contains(path, ident)
+	if path, ok := GetImportPath(selector, ctx); ok && c.Contains(path, ident) {
+		return n.(*ast.CallExpr)
 	}
-	return false
+	return nil
 }

call_list_test.go

@@ -1,60 +1,86 @@
-package gas
+package gas_test
 
 import (
 	"go/ast"
-	"testing"
+
+	"github.com/GoASTScanner/gas"
+	"github.com/GoASTScanner/gas/testutils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
 )
 
-type callListRule struct {
-	MetaData
-	callList CallList
-	matched  int
-}
-
-func (r *callListRule) Match(n ast.Node, c *Context) (gi *Issue, err error) {
-	if r.callList.ContainsCallExpr(n, c) {
-		r.matched += 1
-	}
-	return nil, nil
-}
-
-func TestCallListContainsCallExpr(t *testing.T) {
-	config := map[string]interface{}{"ignoreNosec": false}
-	analyzer := NewAnalyzer(config, nil)
-	calls := NewCallList()
-	calls.AddAll("bytes.Buffer", "Write", "WriteTo")
-	rule := &callListRule{
-		MetaData: MetaData{
-			Severity:   Low,
-			Confidence: Low,
-			What:       "A dummy rule",
-		},
-		callList: calls,
-		matched:  0,
-	}
-	analyzer.AddRule(rule, []ast.Node{(*ast.CallExpr)(nil)})
-	source := `
-	package main
-	import (
-		"bytes"
-		"fmt"
+var _ = Describe("call list", func() {
+	var (
+		calls gas.CallList
 	)
-	func main() {
-		var b bytes.Buffer
-		b.Write([]byte("Hello "))
-		fmt.Fprintf(&b, "world!")
-	}`
-
-	analyzer.ProcessSource("dummy.go", source)
-	if rule.matched != 1 {
-		t.Errorf("Expected to match a bytes.Buffer.Write call")
-	}
-}
-
-func TestCallListContains(t *testing.T) {
-	callList := NewCallList()
-	callList.Add("fmt", "Printf")
-	if !callList.Contains("fmt", "Printf") {
-		t.Errorf("Expected call list to contain fmt.Printf")
-	}
-}
+	BeforeEach(func() {
+		calls = gas.NewCallList()
+	})
+
+	It("should not return any matches when empty", func() {
+		Expect(calls.Contains("foo", "bar")).Should(BeFalse())
+	})
+
+	It("should be possible to add a single call", func() {
+		Expect(calls).Should(HaveLen(0))
+		calls.Add("foo", "bar")
+		Expect(calls).Should(HaveLen(1))
+
+		expected := make(map[string]bool)
+		expected["bar"] = true
+		actual := map[string]bool(calls["foo"])
+		Expect(actual).Should(Equal(expected))
+	})
+
+	It("should be possible to add multiple calls at once", func() {
+		Expect(calls).Should(HaveLen(0))
+		calls.AddAll("fmt", "Sprint", "Sprintf", "Printf", "Println")
+
+		expected := map[string]bool{
+			"Sprint":  true,
+			"Sprintf": true,
+			"Printf":  true,
+			"Println": true,
+		}
+		actual := map[string]bool(calls["fmt"])
+		Expect(actual).Should(Equal(expected))
+	})
+
+	It("should not return a match if none are present", func() {
+		calls.Add("ioutil", "Copy")
+		Expect(calls.Contains("fmt", "Println")).Should(BeFalse())
+	})
+
+	It("should match a call based on selector and ident", func() {
+		calls.Add("ioutil", "Copy")
+		Expect(calls.Contains("ioutil", "Copy")).Should(BeTrue())
+	})
+
+	It("should match a call expression", func() {
+
+		// Create file to be scanned
+		pkg := testutils.NewTestPackage()
+		defer pkg.Close()
+		pkg.AddFile("md5.go", testutils.SampleCodeG401[0].Code)
+
+		ctx := pkg.CreateContext("md5.go")
+
+		// Search for md5.New()
+		calls.Add("md5", "New")
+
+		// Stub out visitor and count number of matched call expr
+		matched := 0
+		v := testutils.NewMockVisitor()
+		v.Context = ctx
+		v.Callback = func(n ast.Node, ctx *gas.Context) bool {
+			if _, ok := n.(*ast.CallExpr); ok && calls.ContainsCallExpr(n, ctx) != nil {
+				matched++
+			}
+			return true
+		}
+		ast.Walk(v, ctx.Root)
+		Expect(matched).Should(Equal(1))
+
+	})
+
+})