fix #302: fix concurrent astcache access

Author: jirfag

pkg/lint/astcache/astcache.go

@@ -5,7 +5,6 @@ import (
 	"go/parser"
 	"go/token"
 	"path/filepath"
-	"strings"
 	"time"
 
 	"golang.org/x/tools/go/packages"
@@ -33,37 +32,30 @@ func NewCache(log logutils.Log) *Cache {
 	}
 }
 
-func (c Cache) Get(filename string) *File {
-	return c.m[filepath.Clean(filename)]
-}
-
-func (c Cache) keys() []string {
+func (c Cache) ParsedFilenames() []string {
 	var keys []string
 	for k := range c.m {
 		keys = append(keys, k)
 	}
 	return keys
 }
 
-func (c Cache) GetOrParse(filename string, fset *token.FileSet) *File {
-	if !filepath.IsAbs(filename) {
-		absFilename, err := filepath.Abs(filename)
-		if err != nil {
-			c.log.Warnf("Can't abs-ify filename %s: %s", filename, err)
-		} else {
-			filename = absFilename
-		}
+func (c Cache) normalizeFilename(filename string) string {
+	if filepath.IsAbs(filename) {
+		return filepath.Clean(filename)
 	}
 
-	f := c.m[filename]
-	if f != nil {
-		return f
+	absFilename, err := filepath.Abs(filename)
+	if err != nil {
+		c.log.Warnf("Can't abs-ify filename %s: %s", filename, err)
+		return filename
 	}
 
-	c.log.Infof("Parse AST for file %s on demand, existing files are %s",
-		filename, strings.Join(c.keys(), ","))
-	c.parseFile(filename, fset)
-	return c.m[filename]
+	return absFilename
+}
+
+func (c Cache) Get(filename string) *File {
+	return c.m[c.normalizeFilename(filename)]
 }
 
 func (c Cache) GetAllValidFiles() []*File {
@@ -81,6 +73,18 @@ func (c *Cache) prepareValidFiles() {
 	c.s = files
 }
 
+func LoadFromFilenames(log logutils.Log, filenames ...string) *Cache {
+	c := NewCache(log)
+
+	fset := token.NewFileSet()
+	for _, filename := range filenames {
+		c.parseFile(filename, fset)
+	}
+
+	c.prepareValidFiles()
+	return c
+}
+
 func LoadFromPackages(pkgs []*packages.Package, log logutils.Log) (*Cache, error) {
 	c := NewCache(log)
 
@@ -127,6 +131,8 @@ func (c *Cache) parseFile(filePath string, fset *token.FileSet) {
 		fset = token.NewFileSet()
 	}
 
+	filePath = c.normalizeFilename(filePath)
+
 	// comments needed by e.g. golint
 	f, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments)
 	c.m[filePath] = &File{

pkg/result/processors/autogenerated_exclude.go

@@ -92,9 +92,9 @@ func (p *AutogeneratedExclude) getOrCreateFileSummary(i *result.Issue) (*ageFile
 		return nil, fmt.Errorf("no file path for issue")
 	}
 
-	f := p.astCache.GetOrParse(i.FilePath(), nil)
-	if f.Err != nil {
-		return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), f.Err)
+	f := p.astCache.Get(i.FilePath())
+	if f == nil || f.Err != nil {
+		return nil, fmt.Errorf("can't parse file %s: %v", i.FilePath(), f)
 	}
 
 	autogenDebugf("file %q: astcache file is %+v", i.FilePath(), *f)

pkg/result/processors/nolint.go

@@ -87,9 +87,9 @@ func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) {
 		return nil, fmt.Errorf("no file path for issue")
 	}
 
-	file := p.astCache.GetOrParse(i.FilePath(), nil)
-	if file.Err != nil {
-		return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), file.Err)
+	file := p.astCache.Get(i.FilePath())
+	if file == nil || file.Err != nil {
+		return nil, fmt.Errorf("can't parse file %s: %v, astcache is %v", i.FilePath(), file, p.astCache.ParsedFilenames())
 	}
 
 	fd.ignoredRanges = p.buildIgnoredRangesForFile(file.F, file.Fset, i.FilePath())

pkg/result/processors/nolint_test.go

@@ -31,7 +31,12 @@ func newNolint2FileIssue(line int) result.Issue {
 }
 
 func newTestNolintProcessor(log logutils.Log) *Nolint {
-	return NewNolint(astcache.NewCache(log), log)
+	cache := astcache.LoadFromFilenames(log,
+		filepath.Join("testdata", "nolint.go"),
+		filepath.Join("testdata", "nolint2.go"),
+		filepath.Join("testdata", "nolint_bad_names.go"),
+	)
+	return NewNolint(cache, log)
 }
 
 func getOkLogger(ctrl *gomock.Controller) *logutils.MockLog {