From 9cbd483b170dc391aeace606897500ee379c2cbd Mon Sep 17 00:00:00 2001 From: Damien Menanteau Date: Thu, 27 Jul 2023 18:47:06 +0200 Subject: [PATCH] [#258] Add UnreachableDirectoryError on FileTreeFilter.findAllMatchingFiles() - to allow retrieval of all the unreachable directories when running tcr check --- src/language/file_tree_filter.go | 95 +++++++++++++++++++-------- src/language/file_tree_filter_test.go | 36 +++++++++- src/language/language.go | 23 ++----- 3 files changed, 110 insertions(+), 44 deletions(-) diff --git a/src/language/file_tree_filter.go b/src/language/file_tree_filter.go index 369c9539..7472502a 100644 --- a/src/language/file_tree_filter.go +++ b/src/language/file_tree_filter.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2021 Murex +Copyright (c) 2023 Murex Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -23,7 +23,6 @@ SOFTWARE. package language import ( - "errors" "github.com/murex/tcr/utils" "github.com/spf13/afero" "os" @@ -33,6 +32,37 @@ import ( "strings" ) +// UnreachableDirectoryError is used to indicate that one or more directories cannot be accessed. +type UnreachableDirectoryError struct { + dirs []string +} + +// Error returns the error description +func (e *UnreachableDirectoryError) Error() string { + var sb strings.Builder + for _, dir := range e.DirList() { + _, _ = sb.WriteString("cannot access directory: ") + _, _ = sb.WriteString(dir) + _, _ = sb.WriteRune('\n') + } + return sb.String() +} + +// DirList returns the list of missing directories +func (e *UnreachableDirectoryError) DirList() []string { + return e.dirs +} + +// add adds directories to the list of unreachable directories +func (e *UnreachableDirectoryError) add(dir ...string) { + e.dirs = append(e.dirs, dir...) +} + +// IsEmpty indicates if the list of unreachable directories is empty +func (e *UnreachableDirectoryError) IsEmpty() bool { + return len(e.dirs) == 0 +} + type ( // FileTreeFilter provides filtering mechanisms allowing to determine if a file or directory // is related to a language @@ -50,16 +80,16 @@ func toSlashedPath(input string) string { return path.Join(strings.Split(input, "\\")...) } -func (treeFilter FileTreeFilter) isInFileTree(aPath string, baseDir string) bool { +func (ftf FileTreeFilter) isInFileTree(aPath string, baseDir string) bool { absPath, _ := filepath.Abs(aPath) // If no directory is configured, any path that is under baseDir path is ok - if treeFilter.Directories == nil || len(treeFilter.Directories) == 0 { + if ftf.Directories == nil || len(ftf.Directories) == 0 { if utils.IsSubPathOf(absPath, baseDir) { return true } } - for _, dir := range treeFilter.Directories { + for _, dir := range ftf.Directories { filterAbsPath, _ := filepath.Abs(filepath.Join(baseDir, dir)) if utils.IsSubPathOf(absPath, filterAbsPath) { return true @@ -68,16 +98,16 @@ func (treeFilter FileTreeFilter) isInFileTree(aPath string, baseDir string) bool return false } -func (treeFilter FileTreeFilter) matches(p string, baseDir string) bool { +func (ftf FileTreeFilter) matches(p string, baseDir string) bool { if p == "" { return false } - if treeFilter.isInFileTree(p, baseDir) { + if ftf.isInFileTree(p, baseDir) { // If no pattern is set, any file matches as long as it's in the file tree - if treeFilter.FilePatterns == nil || len(treeFilter.FilePatterns) == 0 { + if ftf.FilePatterns == nil || len(ftf.FilePatterns) == 0 { return true } - for _, filter := range treeFilter.FilePatterns { + for _, filter := range ftf.FilePatterns { re := regexp.MustCompile(filter) if re.MatchString(p) { return true @@ -87,26 +117,37 @@ func (treeFilter FileTreeFilter) matches(p string, baseDir string) bool { return false } -func (treeFilter FileTreeFilter) findAllMatchingFiles(baseDir string) (files []string, err error) { - for _, dir := range treeFilter.Directories { - err := afero.Walk(appFS, filepath.Join(baseDir, dir), - func(path string, fi os.FileInfo, err error) error { - if err != nil { - return errors.New("something wrong with " + path + err.Error()) - } - if fi.IsDir() { - return nil - } - // If the filename matches the file pattern, we add it to the list of files - if treeFilter.matches(path, baseDir) { - files = append(files, path) - return err - } - return nil - }) +func (ftf FileTreeFilter) findAllMatchingFiles(baseDir string) (files []string, err error) { + dirErr := UnreachableDirectoryError{} + + for _, dir := range ftf.Directories { + matchingFiles, err := ftf.findMatchingFilesInDir(baseDir, dir) if err != nil { - return nil, err + dirErr.add(filepath.Join(baseDir, dir)) + continue } + files = append(files, matchingFiles...) + } + + if !dirErr.IsEmpty() { + return files, &dirErr } return files, nil } + +func (ftf FileTreeFilter) findMatchingFilesInDir(baseDir string, dir string) (files []string, err error) { + return files, afero.Walk(appFS, filepath.Join(baseDir, dir), + func(path string, fi os.FileInfo, err error) error { + if err != nil { + return err + } + if fi.IsDir() { + return nil + } + if ftf.matches(path, baseDir) { + files = append(files, path) + return err + } + return nil + }) +} diff --git a/src/language/file_tree_filter_test.go b/src/language/file_tree_filter_test.go index 2444b25d..700ceb21 100644 --- a/src/language/file_tree_filter_test.go +++ b/src/language/file_tree_filter_test.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2021 Murex +Copyright (c) 2023 Murex Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -121,4 +121,38 @@ func Test_find_all_matching_files_with_wrong_base_dir(t *testing.T) { filter := AFileTreeFilter(WithDirectory("src"), WithPattern(".*\\.ext")) _, err := filter.findAllMatchingFiles("wrong-dir") assert.Error(t, err) + assert.Equal(t, &UnreachableDirectoryError{dirs: []string{filepath.Join("wrong-dir", "src")}}, err) +} + +func Test_unreachable_directory_error(t *testing.T) { + tests := []struct { + desc string + dirs []string + expected string + }{ + { + "no directory", + nil, + ""}, + { + "one directory", + []string{"dir1"}, + "cannot access directory: dir1\n", + }, + { + "multiple directories", + []string{"dir1", "dir2"}, + "cannot access directory: dir1\n" + + "cannot access directory: dir2\n", + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + e := UnreachableDirectoryError{} + e.add(test.dirs...) + assert.Equal(t, test.dirs, e.DirList()) + assert.Equal(t, test.expected, e.Error()) + }) + } } diff --git a/src/language/language.go b/src/language/language.go index 4db58cf9..0d01ba3b 100644 --- a/src/language/language.go +++ b/src/language/language.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2021 Murex +Copyright (c) 2023 Murex Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -141,17 +141,16 @@ func (lang *Language) DirsToWatch(baseDir string) (dirs []string) { // First we concatenate the 2 lists concat := append(lang.GetSrcFileFilter().Directories, lang.GetTestFileFilter().Directories...) - // Then we use a map to remove duplicates - unique := make(map[string]int) + // Then we remove duplicates + unique := make(map[string]bool) for _, dir := range concat { - unique[dir] = 1 + unique[dir] = true } // Finally, we prefix each item with baseDir for dir := range unique { dirs = append(dirs, filepath.Join(baseDir, toLocalPath(dir))) } - // report.PostInfo(dirs) return dirs } @@ -214,27 +213,19 @@ func (lang *Language) worksWithToolchain(toolchainName string) bool { // If there is an overlap between source and test files patterns, test files // are excluded from the returned list func (lang *Language) AllSrcFiles() (result []string, err error) { - var srcFiles, testFiles []string - + testFiles, _ := lang.allMatchingTestFiles() testFilesMap := make(map[string]bool) - testFiles, err = lang.allMatchingTestFiles() - if err != nil { - return nil, err - } for _, path := range testFiles { testFilesMap[path] = true } - srcFiles, err = lang.allMatchingSrcFiles() - if err != nil { - return nil, err - } + srcFiles, errSrc := lang.allMatchingSrcFiles() for _, path := range srcFiles { if !testFilesMap[path] { result = append(result, path) } } - return result, nil + return result, errSrc } // AllTestFiles returns the list of test files for this language.