Skip to content

Commit

Permalink
Fix include search path logic (#31)
Browse files Browse the repository at this point in the history
This reverts part of the changes introduced in #24: we want to include
the file's directory in our search path, not the process's current
working directory.

In fixing this, I realized that we can simply build the list of search
directories once as part of the linting context. This further simplifies
the downstream code while also making this less error prone.
  • Loading branch information
jparise authored Aug 27, 2021
1 parent 0509bac commit cc6c9bb
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 16 deletions.
6 changes: 3 additions & 3 deletions check.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ next:
// C is a type passed to all check functions to provide context.
type C struct {
Filename string
Includes []string
Dirs []string
Program *ast.Program
Check string
Messages Messages
Expand Down Expand Up @@ -204,15 +204,15 @@ func (c *C) Errorf(node ast.Node, message string, args ...interface{}) {

// Resolve resolves a type reference.
func (c *C) Resolve(ref ast.TypeReference) ast.Node {
if n, err := Resolve(ref, c.Program, c.Includes); err == nil {
if n, err := Resolve(ref, c.Program, c.Dirs); 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.Includes); err == nil {
if n, err := ResolveType(ref, c.Program, c.Dirs); err == nil {
return n
}
return nil
Expand Down
8 changes: 1 addition & 7 deletions checks/includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,8 @@ func CheckIncludePath() *thriftcheck.Check {
return
}

// Check the current directory first to match `thrift`s behavior.
dirs := c.Includes
if cwd, err := os.Getwd(); err != nil {
dirs = append([]string{cwd}, c.Includes...)
}

found := false
for _, dir := range dirs {
for _, dir := range c.Dirs {
if _, err := os.Stat(filepath.Join(dir, i.Path)); err == nil {
found = true
break
Expand Down
3 changes: 2 additions & 1 deletion linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"io/ioutil"
"log"
"os"
"path/filepath"
"strings"

"go.uber.org/thriftrw/ast"
Expand Down Expand Up @@ -115,7 +116,7 @@ func (l *Linter) lint(program *ast.Program, filename string, parseInfo *idl.Info

ctx := &C{
Filename: filename,
Includes: l.includes,
Dirs: append([]string{filepath.Dir(filename)}, l.includes...),
Program: program,
logger: l.logger,
parseInfo: parseInfo,
Expand Down
5 changes: 0 additions & 5 deletions parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ func ParseFile(filename string, dirs []string) (*ast.Program, *idl.Info, error)
return nil, nil, fmt.Errorf("%s not found", filename)
}

// Check the current directory first to match `thrift`s behavior.
if cwd, err := os.Getwd(); err != nil {
dirs = append([]string{cwd}, dirs...)
}

for _, dir := range dirs {
if f, err := os.Open(filepath.Join(dir, filename)); err == nil {
return Parse(f)
Expand Down

0 comments on commit cc6c9bb

Please sign in to comment.