Refactor enumerating directory contents (#1478)

* Use m_style for OSFindFilesResult

* Refactor of FindFilesResult.

* Fixes on linux for FindFiles.

* Simplify FindFilesState, and linux support for pattern matching.

* Small fixes to linux FindFilesState

* Fix typo on linux FindFiles

* Fix typo in linux FindFiles.

* Renamed some variables, and improved comments on FindFiles.

* Improve comments on FildFiles

* Small improvements around FindFiles.

* Refactor FindFiles again.. into a visitor and function in Path.

* Fix some problems on linux.

* Fix linux typo.

* Renamed os -> find-file-util

* find-file-utl -> directory-util

Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>

7 files changed, 103 insertions, 366 deletions

diff --git a/tools/slang-test/directory-util.cpp b/tools/slang-test/directory-util.cpp
new file mode 100644
index 000000000..39d06a2a2
--- /dev/null
+++ b/tools/slang-test/directory-util.cpp
@@ -0,0 +1,29 @@
+// find-file-util.cpp
+#include "directory-util.h"
+
+#include "slang-com-helper.h"
+
+using namespace Slang;
+
+/* static */SlangResult DirectoryUtil::findDirectories(const Slang::String& directoryPath, Slang::List<Slang::String>& outPaths)
+{
+    outPaths.clear();
+    CombinePathVisitor visitor(directoryPath, Path::TypeFlag::Directory);
+    SLANG_RETURN_ON_FAIL(Path::find(directoryPath, nullptr, &visitor));
+    outPaths.swapWith(visitor.m_paths);
+    return SLANG_OK;
+}
+
+/* static */SlangResult DirectoryUtil::findFilesMatchingPattern(const Slang::String& directoryPath, const char* pattern, Slang::List<Slang::String>& outPaths)
+{
+    outPaths.clear();
+    CombinePathVisitor visitor(directoryPath, Path::TypeFlag::File);
+    SLANG_RETURN_ON_FAIL(Path::find(directoryPath, pattern, &visitor));
+    outPaths.swapWith(visitor.m_paths);
+    return SLANG_OK;
+}
+
+/* static */SlangResult DirectoryUtil::findFiles(const Slang::String& directoryPath, Slang::List<Slang::String>& outPaths)
+{
+    return findFilesMatchingPattern(directoryPath, nullptr, outPaths);
+}
diff --git a/tools/slang-test/directory-util.h b/tools/slang-test/directory-util.h
new file mode 100644
index 000000000..dbda3e616
--- /dev/null
+++ b/tools/slang-test/directory-util.h
@@ -0,0 +1,53 @@
+#ifndef SLANG_DIRECTORY_UTIL_H
+#define SLANG_DIRECTORY_UTIL_H
+
+#include "../../source/core/slang-io.h"
+
+class CombinePathVisitor : public Slang::Path::Visitor
+{
+public:
+    virtual void accept(Slang::Path::Type type, const Slang::UnownedStringSlice& filename) SLANG_OVERRIDE
+    {
+        using namespace Slang;
+        const Path::TypeFlags flags = Path::TypeFlags(1) << int(type);
+        if (flags & m_allowedFlags)
+        {
+            m_paths.add(Path::combine(m_directoryPath, filename));    
+        }
+    }
+
+        /// Ctor
+    CombinePathVisitor(const Slang::String& directoryPath, Slang::Path::TypeFlags allowedFlags):
+        m_directoryPath(directoryPath),
+        m_allowedFlags(allowedFlags)
+    {
+    }
+
+    Slang::List<Slang::String> m_paths;
+
+protected:
+
+    Slang::Path::TypeFlags m_allowedFlags;
+    Slang::String m_directoryPath;
+};
+
+/* A helper class for finding the contents of directories */
+class DirectoryUtil
+{
+public:
+        /// Enumerate subdirectories in the given `directoryPath`, storing in outPaths.
+        /// @return SLANG_OK on success or SLANG_E_NOT_FOUND if directory is not found.
+    static SlangResult findDirectories(const Slang::String& directoryPath, Slang::List<Slang::String>& outPaths);
+
+        /// Enumerate files in the given `directoryPath` that match the provided
+        /// `pattern` as a simplified regex for files to return (e.g., "*.txt")
+        /// Note that the specifics of the pattern matching are *target specific*
+        /// @return SLANG_OK on success or SLANG_E_NOT_FOUND if directory is not found.
+    static SlangResult findFilesMatchingPattern(const Slang::String& directoryPath, const char* pattern, Slang::List<Slang::String>& outPaths);
+
+        /// Enumerate files in the given `directoryPath`, storing in outPaths.
+        /// @return SLANG_OK on success or SLANG_E_NOT_FOUND if directory is not found.
+    static SlangResult findFiles(const Slang::String& directoryPath, Slang::List<Slang::String>& outPaths);
+};
+
+#endif // SLANG_DIRECTORY_UTIL_H
diff --git a/tools/slang-test/os.cpp b/tools/slang-test/os.cpp
deleted file mode 100644
index eceeef2d7..000000000
--- a/tools/slang-test/os.cpp
+++ /dev/null
@@ -1,218 +0,0 @@
-// os.cpp
-#include "os.h"
-
-#include <stdio.h>
-#include <stdlib.h>
-
-using namespace Slang;
-
-// Platform-specific code follows
-
-#ifdef _WIN32
-
-#include <Windows.h>
-
-static bool advance(OSFindFilesResult& result)
-{
-    return FindNextFileW(result.findHandle_, &result.fileData_) != 0;
-}
-
-static bool adjustToValidResult(OSFindFilesResult& result)
-{
-    for (;;)
-    {
-        if ((result.fileData_.dwFileAttributes & result.requiredMask_) != result.requiredMask_)
-            goto skip;
-
-        if ((result.fileData_.dwFileAttributes & result.disallowedMask_) != 0)
-            goto skip;
-
-        if (wcscmp(result.fileData_.cFileName, L".") == 0)
-            goto skip;
-
-        if (wcscmp(result.fileData_.cFileName, L"..") == 0)
-            goto skip;
-
-        result.filePath_ = result.directoryPath_ + String::fromWString(result.fileData_.cFileName);
-        if (result.fileData_.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
-            result.filePath_ = result.filePath_ + "/";
-
-        return true;
-
-    skip:
-        if (!advance(result))
-            return false;
-    }
-}
-
-
-bool OSFindFilesResult::findNextFile()
-{
-    if (!advance(*this)) return false;
-    return adjustToValidResult(*this);
-}
-
-OSFindFilesResult osFindFilesInDirectoryMatchingPattern(
-    Slang::String directoryPath,
-    Slang::String pattern)
-{
-    // TODO: add separator to end of directory path if needed
-
-    String searchPath = directoryPath + pattern;
-
-    OSFindFilesResult result;
-    HANDLE findHandle = FindFirstFileW(
-        searchPath.toWString(),
-        &result.fileData_);
-
-    result.directoryPath_ = directoryPath;
-    result.findHandle_ = findHandle;
-    result.requiredMask_ = 0;
-    result.disallowedMask_ = FILE_ATTRIBUTE_DIRECTORY;
-
-    if (findHandle == INVALID_HANDLE_VALUE)
-    {
-        result.findHandle_ = NULL;
-        result.error_ = kOSError_FileNotFound;
-        return result;
-    }
-
-    result.error_ = kOSError_None;
-    if (!adjustToValidResult(result))
-    {
-        result.findHandle_ = NULL;
-    }
-    return result;
-}
-
-OSFindFilesResult osFindFilesInDirectory(
-    Slang::String directoryPath)
-{
-    return osFindFilesInDirectoryMatchingPattern(directoryPath, "*");
-}
-
-OSFindFilesResult osFindChildDirectories(
-    Slang::String directoryPath)
-{
-    // TODO: add separator to end of directory path if needed
-
-    String searchPath = directoryPath + "*";
-
-    OSFindFilesResult result;
-    HANDLE findHandle = FindFirstFileW(
-        searchPath.toWString(),
-        &result.fileData_);
-
-    result.directoryPath_ = directoryPath;
-    result.findHandle_ = findHandle;
-    result.requiredMask_ = FILE_ATTRIBUTE_DIRECTORY;
-    result.disallowedMask_ = 0;
-
-    if (findHandle == INVALID_HANDLE_VALUE)
-    {
-        result.findHandle_ = NULL;
-        result.error_ = kOSError_FileNotFound;
-        return result;
-    }
-
-    result.error_ = kOSError_None;
-    if (!adjustToValidResult(result))
-    {
-        result.findHandle_ = NULL;
-    }
-    return result;
-}
-
-#else
-
-static bool advance(OSFindFilesResult& result)
-{
-    result.entry_ = readdir(result.directory_);
-    return result.entry_ != NULL;
-}
-
-static bool checkValidResult(OSFindFilesResult& result)
-{
-//    fprintf(stderr, "checkValidResullt(%s)\n", result.entry_->d_name);
-
-    if (strcmp(result.entry_->d_name, ".") == 0)
-        return false;
-
-    if (strcmp(result.entry_->d_name, "..") == 0)
-        return false;
-
-    String path = result.directoryPath_
-        + String(result.entry_->d_name);
-
-//    fprintf(stderr, "stat(%s)\n", path.getBuffer());
-    struct stat fileInfo;
-    if(stat(path.getBuffer(), &fileInfo) != 0)
-        return false;
-
-    if(S_ISDIR(fileInfo.st_mode))
-        path = path + "/";
-
-
-    result.filePath_ = path;
-    return true;    
-}
-
-static bool adjustToValidResult(OSFindFilesResult& result)
-{
-    for (;;)
-    {
-        if(checkValidResult(result))
-            return true;
-
-        if (!advance(result))
-            return false;
-    }
-}
-
-
-bool OSFindFilesResult::findNextFile()
-{
-//    fprintf(stderr, "OSFindFilesResult::findNextFile()\n");
-    if (!advance(*this)) return false;
-    return adjustToValidResult(*this);
-}
-
-OSFindFilesResult osFindFilesInDirectory(
-    Slang::String directoryPath)
-{
-    OSFindFilesResult result;
-
-//    fprintf(stderr, "osFindFilesInDirectory(%s)\n", directoryPath.getBuffer());
-
-    result.directory_ = opendir(directoryPath.getBuffer());
-    if(!result.directory_)
-    {
-        result.entry_ = NULL;
-        return result;
-    }
-
-    result.directoryPath_ = directoryPath;
-    result.findNextFile();
-    return result;
-}
-
-OSFindFilesResult osFindChildDirectories(
-    Slang::String directoryPath)
-{
-    OSFindFilesResult result;
-
-    result.directory_ = opendir(directoryPath.getBuffer());
-    if(!result.directory_)
-    {
-        result.entry_ = NULL;
-        return result;
-    }
-
-    // TODO: Set attributes to ignore everything but directories
-
-    result.directoryPath_ = directoryPath;
-    result.findNextFile();
-    return result;
-}
-
-#endif
diff --git a/tools/slang-test/os.h b/tools/slang-test/os.h
deleted file mode 100644
index 9dd20c62d..000000000
--- a/tools/slang-test/os.h
+++ /dev/null
@@ -1,135 +0,0 @@
-#ifndef SLANG_OS_H
-#define SLANG_OS_H
-
-// os.h
-
-#include "../../source/core/slang-io.h"
-
-// This file encapsulates the platform-specific operations needed by the test
-// runner that are not already provided by the core Slang libs
-
-#ifdef _WIN32
-
-// Include Windows header in a way that minimized namespace pollution.
-// TODO: We could try to avoid including this at all, but it would
-// mean trying to hide certain struct layouts, which would add
-// more dynamic allocation.
-#define WIN32_LEAN_AND_MEAN
-#define NOMINMAX
-#include <Windows.h>
-#undef WIN32_LEAN_AND_MEAN
-#undef NOMINMAX
-
-#else
-
-#include <dirent.h>
-#include <errno.h>
-//#include <poll.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <unistd.h>
-
-#endif
-
-// A simple set of error codes for possible runtime failures
-enum OSError
-{
-    kOSError_None = 0,
-    kOSError_InvalidArgument,
-    kOSError_OperationFailed,
-    kOSError_FileNotFound,
-};
-
-// A helper type used during enumeration of files in a directory.
-struct OSFindFilesResult
-{
-    Slang::String				directoryPath_;
-    Slang::String				filePath_;
-#ifdef WIN32
-    HANDLE				findHandle_;
-    WIN32_FIND_DATAW	fileData_;
-    DWORD				requiredMask_;
-    DWORD				disallowedMask_;
-    OSError				error_;
-#else
-    DIR*         directory_;
-    dirent*      entry_;
-#endif
-
-    bool findNextFile();
-
-    struct Iterator
-    {
-        OSFindFilesResult* context_;
-
-        bool operator!=(Iterator other) const { return context_ != other.context_; }
-        void operator++()
-        {
-            if (!context_->findNextFile())
-            {
-                context_ = NULL;
-            }
-        }
-        Slang::String const& operator*() const
-        {
-            return context_->filePath_;
-        }
-    };
-
-    Iterator begin()
-    {
-#ifdef WIN32
-        Iterator result = { findHandle_ ? this : NULL };
-#else
-        Iterator result = { entry_ ? this : NULL };
-#endif
-        return result;
-    }
-
-    Iterator end()
-    {
-        Iterator result = { NULL };
-        return result;
-    }
-};
-
-// Enumerate subdirectories in the given `directoryPath` and return a logical
-// collection of the results that can be iterated with a range-based
-// `for` loop:
-//
-// for( auto subdir : osFindChildDirectories(dir))
-// { ... }
-//
-// Each element in the range is a `Slang::String` representing the
-// path to a subdirecotry of the directory.
-OSFindFilesResult osFindChildDirectories(
-    Slang::String directoryPath);
-
-// Enumerate files in the given `directoryPath` that match the provided
-// `pattern` as a simplified regex for files to return (e.g., "*.txt")
-// and return a logical collection of the results
-// that can be iterated with a range-based `for` loop:
-//
-// for( auto file : osFindFilesInDirectoryMatchingPattern(dir, "*.txt"))
-// { ... }
-//
-// Each element in the range is a `Slang::String` representing the
-// path to a file in the directory.
-OSFindFilesResult osFindFilesInDirectoryMatchingPattern(
-    Slang::String directoryPath,
-    Slang::String pattern);
-
-// Enumerate files in the given `directoryPath`  and return a logical
-// collection of the results that can be iterated with a range-based
-// `for` loop:
-//
-// for( auto file : osFindFilesInDirectory(dir))
-// { ... }
-//
-// Each element in the range is a `Slang::String` representing the
-// path to a file in the directory.
-OSFindFilesResult osFindFilesInDirectory(
-    Slang::String directoryPath);
-
-#endif // SLANG_OS_H
diff --git a/tools/slang-test/slang-test-main.cpp b/tools/slang-test/slang-test-main.cpp
index 399aa52da..9ef013aad 100644
--- a/tools/slang-test/slang-test-main.cpp
+++ b/tools/slang-test/slang-test-main.cpp
@@ -13,7 +13,7 @@
 
 using namespace Slang;
 
-#include "os.h"
+#include "directory-util.h"
 #include "../../source/core/slang-render-api-util.h"
 #include "test-context.h"
 #include "test-reporter.h"
@@ -3144,17 +3144,25 @@ void runTestsInDirectory(
     TestContext*		context,
     String				directoryPath)
 {
-    for (auto file : osFindFilesInDirectory(directoryPath))
     {
-        if( shouldRunTest(context, file) )
+        List<String> files;
+        DirectoryUtil::findFiles(directoryPath, files);
+        for (auto file : files)
         {
-//            fprintf(stderr, "slang-test: found '%s'\n", file.getBuffer());
-            runTestsOnFile(context, file);
+            if( shouldRunTest(context, file) )
+            {
+    //            fprintf(stderr, "slang-test: found '%s'\n", file.getBuffer());
+                runTestsOnFile(context, file);
+            }
         }
     }
-    for (auto subdir : osFindChildDirectories(directoryPath))
     {
-        runTestsInDirectory(context, subdir);
+        List<String> subDirs;
+        DirectoryUtil::findDirectories(directoryPath, subDirs);
+        for (auto subDir : subDirs)
+        {
+            runTestsInDirectory(context, subDir);
+        }
     }
 }
 
diff --git a/tools/slang-test/slang-test.vcxproj b/tools/slang-test/slang-test.vcxproj
index d3e049222..b161aa2c9 100644
--- a/tools/slang-test/slang-test.vcxproj
+++ b/tools/slang-test/slang-test.vcxproj
@@ -162,15 +162,15 @@
     </Link>
   </ItemDefinitionGroup>
   <ItemGroup>
+    <ClInclude Include="directory-util.h" />
     <ClInclude Include="options.h" />
-    <ClInclude Include="os.h" />
     <ClInclude Include="slangc-tool.h" />
     <ClInclude Include="test-context.h" />
     <ClInclude Include="test-reporter.h" />
   </ItemGroup>
   <ItemGroup>
+    <ClCompile Include="directory-util.cpp" />
     <ClCompile Include="options.cpp" />
-    <ClCompile Include="os.cpp" />
     <ClCompile Include="slang-test-main.cpp" />
     <ClCompile Include="slangc-tool.cpp" />
     <ClCompile Include="test-context.cpp" />
diff --git a/tools/slang-test/slang-test.vcxproj.filters b/tools/slang-test/slang-test.vcxproj.filters
index 55d5a399a..bbcb30a1b 100644
--- a/tools/slang-test/slang-test.vcxproj.filters
+++ b/tools/slang-test/slang-test.vcxproj.filters
@@ -9,10 +9,10 @@
     </Filter>
   </ItemGroup>
   <ItemGroup>
-    <ClInclude Include="options.h">
+    <ClInclude Include="directory-util.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="os.h">
+    <ClInclude Include="options.h">
       <Filter>Header Files</Filter>
     </ClInclude>
     <ClInclude Include="slangc-tool.h">
@@ -26,10 +26,10 @@
     </ClInclude>
   </ItemGroup>
   <ItemGroup>
-    <ClCompile Include="options.cpp">
+    <ClCompile Include="directory-util.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="os.cpp">
+    <ClCompile Include="options.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
     <ClCompile Include="slang-test-main.cpp">