Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Ignore dot-prefixed files and subdirs when scanning packages #551

Merged
merged 5 commits into from
May 11, 2017
Merged

Ignore dot-prefixed files and subdirs when scanning packages #551

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
536add9
Don't try to parse directories named *.go
ascandella May 10, 2017
2ff0643
Update logic to ignore all '.'-prefixed files
ascandella May 11, 2017
474fb3e
Handle directories ending in .go
ascandella May 11, 2017
78ecf7b
Add tests for recursing into dot-go directories
ascandella May 11, 2017
27fbbf2
Fix header and test import
ascandella May 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Empty file added internal/gps/_testdata/src/dotgodir/.go/.gitkeep
View file
Open in desktop
Empty file.
Empty file added internal/gps/_testdata/src/dotgodir/foo.go/.gitkeep
View file
Open in desktop
Empty file.
11 changes: 9 additions & 2 deletions internal/gps/pkgtree/pkgtree.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,17 @@ func fillPackage(p *build.Package) error {
var testImports []string
var imports []string
for _, file := range gofiles {
// Skip underscore-led files, in keeping with the rest of the toolchain.
if filepath.Base(file)[0] == '_' {
// Skip underscore-led or dot-led files, in keeping with the rest of the toolchain.
bPrefix := filepath.Base(file)[0]
if bPrefix == '_' || bPrefix == '.' {
continue
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any file that starts with ., _ or called testdata should be ignored when scanning source directories. Rather than this check, extend the check on line 203 to check for `== '.'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Is this documented somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, that still leaves us open to the case where somebody names a directory "foo.go" inside their project. Perhaps we should handle that case separately, or at least give them a more helpful error message?

// Skip any directories that happened to get caught by glob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete this code, its not needed now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it still is - in the event of e.g. a directory named foo.go (as the latest commit introduces). no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still is. My comment about "NOTE: this doesn't handle directories" no longer applies, as demonstrated by the attached test case.

if stat, err := os.Stat(file); err == nil && stat.IsDir() {
continue
}

pf, err := parser.ParseFile(token.NewFileSet(), file, nil, parser.ImportsOnly|parser.ParseComments)
if err != nil {
if os.IsPermission(err) {
Expand Down
5 changes: 5 additions & 0 deletions internal/gps/pkgtree/pkgtree_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,11 @@ func TestListPackages(t *testing.T) {
},
},
},
"skip directories starting with '.'": {
fileRoot: j("dotgodir"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add e.g. a foo.go dir to accompany the .go dir, so that we're testing both the dot-led and the dir-ending-in-.go case.

importRoot: "dotgodir",
err: nil,
},
}

for name, fix := range table {
Expand Down