From 9da3c5c3135a8f63bb781e8e664e665e974fe251 Mon Sep 17 00:00:00 2001 From: Valeriy Selitskiy Date: Mon, 6 Nov 2023 18:44:39 +0100 Subject: [PATCH 1/5] add: test for `sqlpath.Glob` --- internal/sql/sqlpath/read_test.go | 193 ++++++++++++++++++ internal/sql/sqlpath/testdata/.hidden.sql | 0 .../sql/sqlpath/testdata/.hiddendir/file1.sql | 0 internal/sql/sqlpath/testdata/extra.txt | 0 internal/sql/sqlpath/testdata/file1.sql | 0 .../sql/sqlpath/testdata/file1.symlink.sql | 1 + internal/sql/sqlpath/testdata/file2.sql | 0 internal/sql/sqlpath/testdata/file3.down.sql | 0 internal/sql/sqlpath/testdata/file4.SQL | 0 internal/sql/sqlpath/testdata/symlink | 1 + 10 files changed, 195 insertions(+) create mode 100644 internal/sql/sqlpath/read_test.go create mode 100644 internal/sql/sqlpath/testdata/.hidden.sql create mode 100644 internal/sql/sqlpath/testdata/.hiddendir/file1.sql create mode 100644 internal/sql/sqlpath/testdata/extra.txt create mode 100644 internal/sql/sqlpath/testdata/file1.sql create mode 120000 internal/sql/sqlpath/testdata/file1.symlink.sql create mode 100644 internal/sql/sqlpath/testdata/file2.sql create mode 100644 internal/sql/sqlpath/testdata/file3.down.sql create mode 100644 internal/sql/sqlpath/testdata/file4.SQL create mode 120000 internal/sql/sqlpath/testdata/symlink diff --git a/internal/sql/sqlpath/read_test.go b/internal/sql/sqlpath/read_test.go new file mode 100644 index 0000000000..84733585ca --- /dev/null +++ b/internal/sql/sqlpath/read_test.go @@ -0,0 +1,193 @@ +package sqlpath + +import ( + "fmt" + "github.com/google/go-cmp/cmp" + "testing" +) + +// Returns a list of SQL files from given paths. +func TestReturnsListOfSQLFiles(t *testing.T) { + // Arrange + paths := []string{"testdata/file1.sql", "testdata/file2.sql"} + + // Act + result, err := Glob(paths) + + // Assert + expected := []string{"testdata/file1.sql", "testdata/file2.sql"} + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v, %v", expected, result, cmp.Diff(expected, result)) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestReturnsNilListWhenNoSQLFilesFound(t *testing.T) { + // Arrange + paths := []string{"testdata/extra.txt"} + + // Act + result, err := Glob(paths) + + // Assert + var expected []string + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v, %v", expected, result, cmp.Diff(expected, result)) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestIgnoresHiddenFilesWhenSearchingForSQLFiles(t *testing.T) { + // Arrange + paths := []string{"testdata/.hidden.sql"} + + // Act + result, err := Glob(paths) + + // Assert + var expected []string + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestIgnoresNonSQLFilesWhenSearchingForSQLFiles(t *testing.T) { + // Arrange + paths := []string{"testdata/extra.txt"} + + // Act + result, err := Glob(paths) + + // Assert + var expected []string + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestExcludesSQLFilesEndingWithDownSQLWhenSearchingForSQLFiles(t *testing.T) { + // Arrange + paths := []string{"testdata/file1.sql", "testdata/file3.down.sql"} + + // Act + result, err := Glob(paths) + + // Assert + expected := []string{"testdata/file1.sql"} + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestReturnsErrorWhenPathDoesNotExist(t *testing.T) { + // Arrange + paths := []string{"non_existent_path"} + + // Act + result, err := Glob(paths) + + // Assert + var expected []string + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err == nil { + t.Errorf("Expected an error, but got nil") + } else { + expectedError := fmt.Errorf("path non_existent_path does not exist") + if !cmp.Equal(err.Error(), expectedError.Error()) { + t.Errorf("Expected error %v, but got %v", expectedError, err) + } + } +} + +func TestReturnsErrorWhenDirectoryCannotBeRead(t *testing.T) { + // Arrange + paths := []string{"testdata/unreadable"} + + // Act + result, err := Glob(paths) + + // Assert + var expected []string + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err == nil { + t.Errorf("Expected an error, but got nil") + } else { + expectedError := fmt.Errorf("open testdata/unreadable: permission denied") + if !cmp.Equal(err.Error(), expectedError.Error()) { + t.Errorf("Expected error %v, but got %v", expectedError, err) + } + } +} + +func TestDoesNotIncludesSQLFilesWithUppercaseExtension(t *testing.T) { + // Arrange + paths := []string{"testdata/file4.SQL"} + + // Act + result, err := Glob(paths) + + // Assert + var expected []string + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestIncludesSQLFilesWithLeadingDotsInDirectoryName(t *testing.T) { + // Arrange + paths := []string{"./testdata/.hiddendir/file1.sql"} + + // Act + result, err := Glob(paths) + + // Assert + expected := []string{"./testdata/.hiddendir/file1.sql"} + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} + +func TestPathIsSymlink(t *testing.T) { + // Arrange + paths := []string{"testdata/symlink", "testdata/file1.symlink.sql"} + + // Act + result, err := Glob(paths) + + // Assert + expected := []string{ + "testdata/symlink/file1.sql", + "testdata/symlink/file1.symlink.sql", + "testdata/symlink/file2.sql", + "testdata/file1.symlink.sql", + } + if !cmp.Equal(result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + if err != nil { + t.Errorf("Expected no error, but got %v", err) + } +} diff --git a/internal/sql/sqlpath/testdata/.hidden.sql b/internal/sql/sqlpath/testdata/.hidden.sql new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/.hiddendir/file1.sql b/internal/sql/sqlpath/testdata/.hiddendir/file1.sql new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/extra.txt b/internal/sql/sqlpath/testdata/extra.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/file1.sql b/internal/sql/sqlpath/testdata/file1.sql new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/file1.symlink.sql b/internal/sql/sqlpath/testdata/file1.symlink.sql new file mode 120000 index 0000000000..7c823b3226 --- /dev/null +++ b/internal/sql/sqlpath/testdata/file1.symlink.sql @@ -0,0 +1 @@ +./file1.sql \ No newline at end of file diff --git a/internal/sql/sqlpath/testdata/file2.sql b/internal/sql/sqlpath/testdata/file2.sql new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/file3.down.sql b/internal/sql/sqlpath/testdata/file3.down.sql new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/file4.SQL b/internal/sql/sqlpath/testdata/file4.SQL new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/sql/sqlpath/testdata/symlink b/internal/sql/sqlpath/testdata/symlink new file mode 120000 index 0000000000..945c9b46d6 --- /dev/null +++ b/internal/sql/sqlpath/testdata/symlink @@ -0,0 +1 @@ +. \ No newline at end of file From a69b0d4b1aabb2b01dbbbc364cb3a0ba5e9fc61a Mon Sep 17 00:00:00 2001 From: Valeriy Selitskiy Date: Tue, 7 Nov 2023 14:56:11 +0100 Subject: [PATCH 2/5] fix: improve tests, minor change one expected error message --- internal/sql/sqlpath/read_test.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/internal/sql/sqlpath/read_test.go b/internal/sql/sqlpath/read_test.go index 84733585ca..33ea8c570f 100644 --- a/internal/sql/sqlpath/read_test.go +++ b/internal/sql/sqlpath/read_test.go @@ -107,7 +107,8 @@ func TestReturnsErrorWhenPathDoesNotExist(t *testing.T) { if err == nil { t.Errorf("Expected an error, but got nil") } else { - expectedError := fmt.Errorf("path non_existent_path does not exist") + expectedError := fmt.Errorf(`failed to stat path "non_existent_path": ` + + `lstat non_existent_path: no such file or directory`) if !cmp.Equal(err.Error(), expectedError.Error()) { t.Errorf("Expected error %v, but got %v", expectedError, err) } @@ -153,24 +154,39 @@ func TestDoesNotIncludesSQLFilesWithUppercaseExtension(t *testing.T) { } } -func TestIncludesSQLFilesWithLeadingDotsInDirectoryName(t *testing.T) { +func TestNotIncludesHiddenFilesAnyPath(t *testing.T) { // Arrange - paths := []string{"./testdata/.hiddendir/file1.sql"} + paths := []string{ + "./testdata/.hiddendir/file1.sql", // pass + "./testdata/.hidden.sql", // skip + } // Act result, err := Glob(paths) // Assert - expected := []string{"./testdata/.hiddendir/file1.sql"} - if !cmp.Equal(result, expected) { - t.Errorf("Expected %v, but got %v", expected, result) + expectedAny := [][]string{ + {"./testdata/.hiddendir/file1.sql"}, + {"testdata/.hiddendir/file1.sql"}, + } + + match := false + for _, expected := range expectedAny { + if cmp.Equal(result, expected) { + match = true + break + } } + if !match { + t.Errorf("Expected any of %v, but got %v", expectedAny, result) + } + if err != nil { t.Errorf("Expected no error, but got %v", err) } } -func TestPathIsSymlink(t *testing.T) { +func TestFollowSymlinks(t *testing.T) { // Arrange paths := []string{"testdata/symlink", "testdata/file1.symlink.sql"} From 66edc84a50de25191c7215fef7688a26d075cedf Mon Sep 17 00:00:00 2001 From: Valeriy Selitskiy Date: Tue, 7 Nov 2023 15:14:12 +0100 Subject: [PATCH 3/5] add: glob expanding maintaining backward compartibility --- internal/sql/sqlpath/read.go | 37 ++++++++++++++++++- .../sql/sqlpath/testdata/subdir/file2.sql | 0 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 internal/sql/sqlpath/testdata/subdir/file2.sql diff --git a/internal/sql/sqlpath/read.go b/internal/sql/sqlpath/read.go index 24072001c9..c8038cd0b2 100644 --- a/internal/sql/sqlpath/read.go +++ b/internal/sql/sqlpath/read.go @@ -12,6 +12,10 @@ import ( // Return a list of SQL files in the listed paths. Only includes files ending // in .sql. Omits hidden files, directories, and migrations. func Glob(paths []string) ([]string, error) { + paths, err := expandGlobs(paths) + if err != nil { + return nil, err + } var files []string for _, path := range paths { f, err := os.Stat(path) @@ -30,7 +34,7 @@ func Glob(paths []string) ([]string, error) { files = append(files, path) } } - var sqlFiles []string + var sqlFiles []string //nolint:prealloc // can be empty for _, file := range files { if !strings.HasSuffix(file, ".sql") { continue @@ -45,3 +49,34 @@ func Glob(paths []string) ([]string, error) { } return sqlFiles, nil } + +func expandGlobs(paths []string) ([]string, error) { + expandedPatterns := make([]string, 0, len(paths)) + for _, pattern := range paths { + expansion, err := filepath.Glob(pattern) + if err != nil { + return nil, fmt.Errorf("failed to expand pattern %q: %w", pattern, err) + } + if len(expansion) == 0 { + fi, err := os.Lstat(pattern) + if err != nil { + return nil, fmt.Errorf("failed to stat path %q: %w", pattern, err) + } + if fi == nil { + return nil, fmt.Errorf("failed to stat path %q: %w", pattern, os.ErrNotExist) + } + var isFilepath bool + for _, mask := range []os.FileMode{os.ModeDir, os.ModeSymlink, os.FileMode(0x400)} { + if fi.Mode()&mask == 0 { + isFilepath = true + break + } + } + if !isFilepath { + continue + } + } + expandedPatterns = append(expandedPatterns, expansion...) + } + return expandedPatterns, nil +} diff --git a/internal/sql/sqlpath/testdata/subdir/file2.sql b/internal/sql/sqlpath/testdata/subdir/file2.sql new file mode 100644 index 0000000000..e69de29bb2 From 6286e7096d52cc02d6363dd63b5152c093e25c95 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 7 Nov 2023 13:00:15 -0800 Subject: [PATCH 4/5] feat(sqlpath): Support filepath.Glob patterns --- internal/sql/sqlpath/read.go | 62 ++++++++++++------------------- internal/sql/sqlpath/read_test.go | 9 ++--- 2 files changed, 27 insertions(+), 44 deletions(-) diff --git a/internal/sql/sqlpath/read.go b/internal/sql/sqlpath/read.go index c8038cd0b2..0dd06796bf 100644 --- a/internal/sql/sqlpath/read.go +++ b/internal/sql/sqlpath/read.go @@ -9,18 +9,33 @@ import ( "github.com/sqlc-dev/sqlc/internal/migrations" ) -// Return a list of SQL files in the listed paths. Only includes files ending -// in .sql. Omits hidden files, directories, and migrations. -func Glob(paths []string) ([]string, error) { - paths, err := expandGlobs(paths) - if err != nil { - return nil, err +// Return a list of SQL files in the listed paths. +// +// Only includes files ending in .sql. Omits hidden files, directories, and +// down migrations. + +// If a path contains *, ?, [, or ], treat the path as a pattern and expand it +// filepath.Glob. +func Glob(patterns []string) ([]string, error) { + var files, paths []string + for _, pattern := range patterns { + if strings.ContainsAny(pattern, "*?[]") { + matches, err := filepath.Glob(pattern) + if err != nil { + return nil, err + } + // if len(matches) == 0 { + // slog.Warn("zero files matched", "pattern", pattern) + // } + paths = append(paths, matches...) + } else { + paths = append(paths, pattern) + } } - var files []string for _, path := range paths { f, err := os.Stat(path) if err != nil { - return nil, fmt.Errorf("path %s does not exist", path) + return nil, fmt.Errorf("path error: %w", err) } if f.IsDir() { listing, err := os.ReadDir(path) @@ -49,34 +64,3 @@ func Glob(paths []string) ([]string, error) { } return sqlFiles, nil } - -func expandGlobs(paths []string) ([]string, error) { - expandedPatterns := make([]string, 0, len(paths)) - for _, pattern := range paths { - expansion, err := filepath.Glob(pattern) - if err != nil { - return nil, fmt.Errorf("failed to expand pattern %q: %w", pattern, err) - } - if len(expansion) == 0 { - fi, err := os.Lstat(pattern) - if err != nil { - return nil, fmt.Errorf("failed to stat path %q: %w", pattern, err) - } - if fi == nil { - return nil, fmt.Errorf("failed to stat path %q: %w", pattern, os.ErrNotExist) - } - var isFilepath bool - for _, mask := range []os.FileMode{os.ModeDir, os.ModeSymlink, os.FileMode(0x400)} { - if fi.Mode()&mask == 0 { - isFilepath = true - break - } - } - if !isFilepath { - continue - } - } - expandedPatterns = append(expandedPatterns, expansion...) - } - return expandedPatterns, nil -} diff --git a/internal/sql/sqlpath/read_test.go b/internal/sql/sqlpath/read_test.go index 33ea8c570f..774561057e 100644 --- a/internal/sql/sqlpath/read_test.go +++ b/internal/sql/sqlpath/read_test.go @@ -2,8 +2,9 @@ package sqlpath import ( "fmt" - "github.com/google/go-cmp/cmp" "testing" + + "github.com/google/go-cmp/cmp" ) // Returns a list of SQL files from given paths. @@ -30,7 +31,6 @@ func TestReturnsNilListWhenNoSQLFilesFound(t *testing.T) { // Act result, err := Glob(paths) - // Assert var expected []string if !cmp.Equal(result, expected) { @@ -107,8 +107,7 @@ func TestReturnsErrorWhenPathDoesNotExist(t *testing.T) { if err == nil { t.Errorf("Expected an error, but got nil") } else { - expectedError := fmt.Errorf(`failed to stat path "non_existent_path": ` + - `lstat non_existent_path: no such file or directory`) + expectedError := fmt.Errorf("path error: stat non_existent_path: no such file or directory") if !cmp.Equal(err.Error(), expectedError.Error()) { t.Errorf("Expected error %v, but got %v", expectedError, err) } @@ -130,7 +129,7 @@ func TestReturnsErrorWhenDirectoryCannotBeRead(t *testing.T) { if err == nil { t.Errorf("Expected an error, but got nil") } else { - expectedError := fmt.Errorf("open testdata/unreadable: permission denied") + expectedError := fmt.Errorf("path error: stat testdata/unreadable: no such file or directory") if !cmp.Equal(err.Error(), expectedError.Error()) { t.Errorf("Expected error %v, but got %v", expectedError, err) } From 54cd5341a2b940ba399293aaf834ccb51835a848 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Tue, 7 Nov 2023 13:01:38 -0800 Subject: [PATCH 5/5] fix: Remove nolint comment --- internal/sql/sqlpath/read.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/sql/sqlpath/read.go b/internal/sql/sqlpath/read.go index 0dd06796bf..02c8b2855c 100644 --- a/internal/sql/sqlpath/read.go +++ b/internal/sql/sqlpath/read.go @@ -49,7 +49,7 @@ func Glob(patterns []string) ([]string, error) { files = append(files, path) } } - var sqlFiles []string //nolint:prealloc // can be empty + var sqlFiles []string for _, file := range files { if !strings.HasSuffix(file, ".sql") { continue