Introduce a caching FileParser (#126)

This migrates the file parsing functions to a stateful FileParser type.
This lets us implement a file-level cache so that repeated attempts to
parse the same file (via includes) return the cached result.

Author: jparise

.github/workflows/ci.yml

@@ -74,3 +74,6 @@ jobs:
         run: go build -v ./...
       - name: Test
         run: go test -v ./...
+      - name: Test (race detector)
+        if: matrix.go == '1.25'
+        run: go test -race ./...

ast.go

@@ -53,7 +53,7 @@ func Doc(node ast.Node) string {
 // The target can either be in the current program's scope or it can refer to
 // an included file using dot notation. Included files must exist in one of the
 // given search directories.
-func Resolve(name string, program *ast.Program, dirs []string) (ast.Node, error) {
+func Resolve(name string, program *ast.Program, parser *FileParser) (ast.Node, error) {
 	defs := program.Definitions
 
 	if strings.Contains(name, ".") {
@@ -73,7 +73,7 @@ func Resolve(name string, program *ast.Program, dirs []string) (ast.Node, error)
 			return nil, fmt.Errorf("missing \"include\" for type reference %q", name)
 		}
 
-		program, _, err := ParseFile(ipath, dirs)
+		program, _, err := parser.ParseFile(ipath)
 		if err != nil {
 			return nil, err
 		}
@@ -98,12 +98,12 @@ func Resolve(name string, program *ast.Program, dirs []string) (ast.Node, error)
 //   - "Enum.Value" (ast.EnumItem)
 //   - "include.Constant" (ast.Constant)
 //   - "include.Enum.Value" (ast.EnumItem)
-func ResolveConstant(ref ast.ConstantReference, program *ast.Program, dirs []string) (ast.Node, error) {
+func ResolveConstant(ref ast.ConstantReference, program *ast.Program, parser *FileParser) (ast.Node, error) {
 	parts := strings.SplitN(ref.Name, ".", 3)
 
-	n, err := Resolve(parts[0], program, dirs)
+	n, err := Resolve(parts[0], program, parser)
 	if err != nil && len(parts) > 1 {
-		n, err = Resolve(parts[0]+"."+parts[1], program, dirs)
+		n, err = Resolve(parts[0]+"."+parts[1], program, parser)
 	}
 	if err != nil {
 		return n, fmt.Errorf("%q could not be resolved", ref.Name)
@@ -125,8 +125,8 @@ func ResolveConstant(ref ast.ConstantReference, program *ast.Program, dirs []str
 // resolve the target node's own type. This is useful when the reference
 // points to an [ast.Typedef] or [ast.Constant], for example, and the caller
 // is primarily intererested in the target's ast.Type.
-func ResolveType(ref ast.TypeReference, program *ast.Program, dirs []string) (ast.Node, error) {
-	n, err := Resolve(ref.Name, program, dirs)
+func ResolveType(ref ast.TypeReference, program *ast.Program, parser *FileParser) (ast.Node, error) {
+	n, err := Resolve(ref.Name, program, parser)
 	if err != nil {
 		return nil, err
 	}

check.go

@@ -171,6 +171,7 @@ type C struct {
 	Check     string
 	Messages  Messages
 	logger    *log.Logger
+	parser    *FileParser
 	parseInfo *idl.Info
 }
 
@@ -203,23 +204,23 @@ func (c *C) Errorf(node ast.Node, message string, args ...any) {
 
 // Resolve resolves a name.
 func (c *C) Resolve(name string) ast.Node {
-	if n, err := Resolve(name, c.Program, c.Dirs); err == nil {
+	if n, err := Resolve(name, c.Program, c.parser); err == nil {
 		return n
 	}
 	return nil
 }
 
 // ResolveConstant resolves a constant reference to its target.
 func (c *C) ResolveConstant(ref ast.ConstantReference) ast.Node {
-	if n, err := ResolveConstant(ref, c.Program, c.Dirs); err == nil {
+	if n, err := ResolveConstant(ref, c.Program, c.parser); err == nil {
 		return n
 	}
 	return nil
 }
 
 // ResolveType resolves a type reference to its target type.
 func (c *C) ResolveType(ref ast.TypeReference) ast.Node {
-	if n, err := ResolveType(ref, c.Program, c.Dirs); err == nil {
+	if n, err := ResolveType(ref, c.Program, c.parser); err == nil {
 		return n
 	}
 	return nil

linter.go

@@ -30,6 +30,7 @@ import (
 // Linter is a configured Thrift linter.
 type Linter struct {
 	checks   Checks
+	parser   *FileParser
 	logger   *log.Logger
 	includes []string
 }
@@ -60,14 +61,15 @@ func NewLinter(checks Checks, options ...Option) *Linter {
 	for _, option := range options {
 		option(l)
 	}
+	l.parser = NewFileParser(l.includes)
 	l.logger.Printf("checks: %s\n", checks)
 	l.logger.Printf("includes: %s\n", strings.Join(l.includes, " "))
 	return l
 }
 
 // Lint lints a single input file.
 func (l *Linter) Lint(r io.Reader, filename string) (Messages, error) {
-	program, info, err := Parse(r)
+	program, info, err := l.parser.Parse(r, filename)
 	if err != nil {
 		var parseError *idl.ParseError
 		if errors.As(err, &parseError) {
@@ -119,6 +121,7 @@ func (l *Linter) lint(program *ast.Program, filename string, parseInfo *idl.Info
 		Dirs:      append([]string{filepath.Dir(filename)}, l.includes...),
 		Program:   program,
 		logger:    l.logger,
+		parser:    l.parser,
 		parseInfo: parseInfo,
 	}
 	activeChecks := overridableChecks{root: &l.checks}

parse.go

@@ -19,6 +19,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sync"
 
 	"go.uber.org/thriftrw/ast"
 	"go.uber.org/thriftrw/idl"
@@ -36,23 +37,70 @@ func Parse(r io.Reader) (*ast.Program, *idl.Info, error) {
 	return prog, cfg.Info, err
 }
 
-// ParseFile parses a Thrift file. The filename must appear in one of the
-// given directories.
-func ParseFile(filename string, dirs []string) (*ast.Program, *idl.Info, error) {
+type parsedFile struct {
+	prog *ast.Program
+	info *idl.Info
+}
+
+// FileParser caches parsed Thrift files to avoid re-parsing included files.
+// It is safe for concurrent use by multiple goroutines.
+type FileParser struct {
+	mu    sync.RWMutex
+	cache map[string]parsedFile
+	dirs  []string
+}
+
+// NewParser returns a new Parser with a list of directories that will be
+// searched for files.
+func NewFileParser(dirs []string) *FileParser {
+	return &FileParser{
+		cache: make(map[string]parsedFile),
+		dirs:  dirs,
+	}
+}
+
+// Parse parses Thrift document content with the given filename.
+func (p *FileParser) Parse(r io.Reader, filename string) (*ast.Program, *idl.Info, error) {
+	filename, err := filepath.Abs(filename)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	p.mu.RLock()
+	cached, ok := p.cache[filename]
+	p.mu.RUnlock()
+
+	if ok {
+		return cached.prog, cached.info, nil
+	}
+
+	prog, info, err := Parse(r)
+	if err == nil {
+		p.mu.Lock()
+		p.cache[filename] = parsedFile{prog: prog, info: info}
+		p.mu.Unlock()
+	}
+
+	return prog, info, err
+}
+
+// ParseFile parses a Thrift file from its filename.
+func (p *FileParser) ParseFile(filename string) (*ast.Program, *idl.Info, error) {
 	if filepath.IsAbs(filename) {
 		if f, err := os.Open(filename); err == nil {
 			defer f.Close()
-			return Parse(f)
+			return p.Parse(f, filename)
 		}
 		return nil, nil, fmt.Errorf("%s not found", filename)
 	}
 
+	dirs := append([]string{filepath.Dir(filename)}, p.dirs...)
 	for _, dir := range dirs {
 		if f, err := os.Open(filepath.Join(dir, filename)); err == nil {
 			defer f.Close()
-			return Parse(f)
+			return p.Parse(f, f.Name())
 		}
 	}
 
-	return nil, nil, fmt.Errorf("%s not found in %s", filename, dirs)
+	return nil, nil, fmt.Errorf("%s not found in %s", filename, p.dirs)
 }