Refactor luacheck.fs

Split out implementations of functions using lfs and using shell.
This way both implementations can be tested in busted in one go
(and busted requires lfs).

Rename fs.mtime() and fs.current_dir() to fs.get_mtime() and
fs.get_current_dir().

install.lua

@@ -120,7 +120,9 @@ for _, filename in ipairs {
       "argparse.lua",
       "whitespace.lua",
       "detect_globals.lua",
-      "standards.lua"} do
+      "standards.lua",
+      "lua_fs.lua",
+      "lfs_fs.lua"} do
    copy("src" .. dirsep .. "luacheck" .. dirsep .. filename, luacheck_lib_dir)
 end
 

luacheck-scm-1.rockspec

@@ -19,31 +19,33 @@ build = {
    type = "builtin",
    modules = {
       luacheck = "src/luacheck/init.lua",
-      ["luacheck.main"] = "src/luacheck/main.lua",
-      ["luacheck.config"] = "src/luacheck/config.lua",
-      ["luacheck.linearize"] = "src/luacheck/linearize.lua",
       ["luacheck.analyze"] = "src/luacheck/analyze.lua",
-      ["luacheck.reachability"] = "src/luacheck/reachability.lua",
-      ["luacheck.core_utils"] = "src/luacheck/core_utils.lua",
-      ["luacheck.check"] = "src/luacheck/check.lua",
-      ["luacheck.parser"] = "src/luacheck/parser.lua",
-      ["luacheck.lexer"] = "src/luacheck/lexer.lua",
-      ["luacheck.filter"] = "src/luacheck/filter.lua",
-      ["luacheck.options"] = "src/luacheck/options.lua",
-      ["luacheck.inline_options"] = "src/luacheck/inline_options.lua",
+      ["luacheck.argparse"] = "src/luacheck/argparse.lua",
       ["luacheck.builtin_standards"] = "src/luacheck/builtin_standards.lua",
-      ["luacheck.expand_rockspec"] = "src/luacheck/expand_rockspec.lua",
-      ["luacheck.multithreading"] = "src/luacheck/multithreading.lua",
       ["luacheck.cache"] = "src/luacheck/cache.lua",
+      ["luacheck.check"] = "src/luacheck/check.lua",
+      ["luacheck.config"] = "src/luacheck/config.lua",
+      ["luacheck.core_utils"] = "src/luacheck/core_utils.lua",
+      ["luacheck.detect_globals"] = "src/luacheck/detect_globals.lua",
+      ["luacheck.expand_rockspec"] = "src/luacheck/expand_rockspec.lua",
+      ["luacheck.filter"] = "src/luacheck/filter.lua",
       ["luacheck.format"] = "src/luacheck/format.lua",
-      ["luacheck.version"] = "src/luacheck/version.lua",
       ["luacheck.fs"] = "src/luacheck/fs.lua",
       ["luacheck.globbing"] = "src/luacheck/globbing.lua",
+      ["luacheck.inline_options"] = "src/luacheck/inline_options.lua",
+      ["luacheck.lexer"] = "src/luacheck/lexer.lua",
+      ["luacheck.lfs_fs"] = "src/luacheck/lfs_fs.lua",
+      ["luacheck.linearize"] = "src/luacheck/linearize.lua",
+      ["luacheck.lua_fs"] = "src/luacheck/lua_fs.lua",
+      ["luacheck.main"] = "src/luacheck/main.lua",
+      ["luacheck.multithreading"] = "src/luacheck/multithreading.lua",
+      ["luacheck.options"] = "src/luacheck/options.lua",
+      ["luacheck.parser"] = "src/luacheck/parser.lua",
+      ["luacheck.reachability"] = "src/luacheck/reachability.lua",
+      ["luacheck.standards"] = "src/luacheck/standards.lua",
       ["luacheck.utils"] = "src/luacheck/utils.lua",
-      ["luacheck.argparse"] = "src/luacheck/argparse.lua",
-      ["luacheck.whitespace"] = "src/luacheck/whitespace.lua",
-      ["luacheck.detect_globals"] = "src/luacheck/detect_globals.lua",
-      ["luacheck.standards"] = "src/luacheck/standards.lua"
+      ["luacheck.version"] = "src/luacheck/version.lua",
+      ["luacheck.whitespace"] = "src/luacheck/whitespace.lua"
    },
    install = {
       bin = {

spec/config_spec.lua

@@ -1,24 +1,22 @@
+local lfs = require "lfs"
+
 local config = require "luacheck.config"
 local fs = require "luacheck.fs"
 local P = fs.normalize
-local cur_dir = fs.has_lfs and fs.lfs.currentdir()
+local cur_dir = lfs.currentdir()
 
 local function nest(dir, func)
-   if not fs.has_lfs then
-      pending("uses lfs")
-   end
-
    local backed = false
 
    local function back()
       if not backed then
-         fs.lfs.chdir(cur_dir)
+         lfs.chdir(cur_dir)
          backed = true
       end
    end
 
    finally(back)
-   fs.lfs.chdir(dir)
+   lfs.chdir(dir)
    func()
    back()
 end

spec/fs_spec.lua

@@ -40,19 +40,19 @@ describe("fs", function()
       end)
    end)
 
-   describe("mtime", function()
+   describe("get_mtime", function()
       it("returns modification time as a number", function()
-         assert.number(fs.mtime("spec/folder/foo"))
+         assert.number(fs.get_mtime("spec/folder/foo"))
       end)
 
       it("returns nil for non-existent files", function()
-         assert.is_nil(fs.mtime("spec/folder/non-existent"))
+         assert.is_nil(fs.get_mtime("spec/folder/non-existent"))
       end)
    end)
 
-   describe("current_dir", function()
+   describe("get_current_dir", function()
       it("returns absolute path to current directory", function()
-         local current_dir = fs.current_dir()
+         local current_dir = fs.get_current_dir()
          assert.string(current_dir)
          assert.not_equal("", (fs.split_base(current_dir)))
          assert.is_true(fs.is_file(current_dir .. "spec/folder/foo"))
@@ -61,18 +61,18 @@ describe("fs", function()
 
    describe("find_file", function()
       it("finds file in a directory", function()
-         local path = fs.current_dir() .. P"spec/folder"
+         local path = fs.get_current_dir() .. P"spec/folder"
          assert.equal(path, fs.find_file(path, "foo"))
       end)
 
       it("finds file in a parent directory", function()
-         local path = fs.current_dir() .. P"spec/folder"
+         local path = fs.get_current_dir() .. P"spec/folder"
          assert.equal(path, fs.find_file(fs.join(path, "folder1"), "foo"))
       end)
 
       it("returns nil if can't find file", function()
          assert.is_nil(
-            fs.find_file(fs.current_dir(), "this file shouldn't exist or it will make luacheck testsuite break"))
+            fs.find_file(fs.get_current_dir(), "this file shouldn't exist or it will make luacheck testsuite break"))
       end)
    end)
 end)

src/luacheck/config.lua

@@ -170,7 +170,7 @@ function config.load_config(path)
    local is_default_path = not path
    path = path or config.default_path
 
-   local current_dir = fs.current_dir()
+   local current_dir = fs.get_current_dir()
    local abs_conf_dir, rel_conf_dir = fs.find_file(current_dir, path)
 
    if not abs_conf_dir then

src/luacheck/fs.lua

@@ -2,7 +2,15 @@ local fs = {}
 
 local utils = require "luacheck.utils"
 
-fs.has_lfs, fs.lfs = pcall(require, "lfs")
+fs.has_lfs = pcall(require, "lfs")
+
+local base_fs
+
+if fs.has_lfs then
+   base_fs = require "luacheck.lfs_fs"
+else
+   base_fs = require "luacheck.lua_fs"
+end
 
 local function ensure_dir_sep(path)
    if path:sub(-1) ~= utils.dir_sep then
@@ -12,17 +20,15 @@ local function ensure_dir_sep(path)
    return path
 end
 
-if utils.is_windows then
-   function fs.split_base(path)
+function fs.split_base(path)
+   if utils.is_windows then
       if path:match("^%a:\\") then
          return path:sub(1, 3), path:sub(4)
       else
-         -- Disregard UNC stuff for now.
+         -- Disregard UNC paths and relative paths with drive letter.
          return "", path
       end
-   end
-else
-   function fs.split_base(path)
+   else
       if path:match("^/") then
          if path:match("^//") then
             return "//", path:sub(3)
@@ -85,6 +91,14 @@ function fs.is_subpath(path, subpath)
    return rest1 == rest2 or rest2:sub(#rest1 + 1, #rest1 + 1) == utils.dir_sep
 end
 
+function fs.is_dir(path)
+   return base_fs.get_mode(path) == "directory"
+end
+
+function fs.is_file(path)
+   return base_fs.get_mode(path) == "file"
+end
+
 -- Searches for file starting from path, going up until the file
 -- is found or root directory is reached.
 -- Path must be absolute.
@@ -110,69 +124,14 @@ function fs.find_file(path, file)
    end
 end
 
-local get_mode
-
-if fs.has_lfs then
-   function get_mode(path)
-      return fs.lfs.attributes(path, "mode")
-   end
-else
-   local mode_cmd
-
-   if utils.is_windows then
-      mode_cmd = [[if exist "%s\*" (echo directory) else (if exist "%s" echo "file")]]
-   else
-      mode_cmd = [[if [ -d '%s' ]; then echo directory; elif [ -f '%s' ]; then echo file; fi]]
-   end
-
-   function get_mode(path)
-      local cmd = mode_cmd:format(path, path)
-      local fh = io.popen(cmd)
-      local mode = fh:read("*a")
-      fh:close()
-      return mode:match("^(%S*)")
-   end
-end
-
--- Returns whether path points to a directory.
-function fs.is_dir(path)
-   return get_mode(path) == "directory"
-end
-
--- Returns whether path points to a file.
-function fs.is_file(path)
-   return get_mode(path) == "file"
-end
-
-if not fs.has_lfs then
-   function fs.extract_files(_, _)
-      return {}
-   end
-
-   function fs.mtime(_)
-      return 0
-   end
-
-   local pwd_command = utils.is_windows and "cd" or "pwd"
-
-   function fs.current_dir()
-      local fh = io.popen(pwd_command)
-      local current_dir = fh:read("*a")
-      fh:close()
-      -- Remove extra newline at the end.
-      return ensure_dir_sep(current_dir:sub(1, -2))
-   end
-
-   return fs
-end
-
 -- Returns list of all files in directory matching pattern.
 -- Returns nil, error message on error.
 function fs.extract_files(dir_path, pattern)
+   assert(fs.has_lfs)
    local res = {}
 
    local function scan(dir)
-      local ok, iter, state, var = pcall(fs.lfs.dir, dir)
+      local ok, iter, state, var = pcall(base_fs.dir_iter, dir)
 
       if not ok then
          local err = utils.unprefix(iter, "cannot open " .. dir .. ": ")
@@ -207,13 +166,14 @@ function fs.extract_files(dir_path, pattern)
 end
 
 -- Returns modification time for a file.
-function fs.mtime(path)
-   return fs.lfs.attributes(path, "modification")
+function fs.get_mtime(path)
+   assert(fs.has_lfs)
+   return base_fs.get_mtime(path)
 end
 
--- Returns absolute path to current working directory.
-function fs.current_dir()
-   return ensure_dir_sep(assert(fs.lfs.currentdir()))
+-- Returns absolute path to current working directory, with trailing directory separator.
+function fs.get_current_dir()
+   return ensure_dir_sep(base_fs.get_current_dir())
 end
 
 return fs

src/luacheck/globbing.lua

@@ -5,7 +5,7 @@ local utils = require "luacheck.utils"
 -- Hidden files are not treated specially. Special characters can't be escaped.
 local globbing = {}
 
-local cur_dir = fs.current_dir()
+local cur_dir = fs.get_current_dir()
 
 local function is_regular_path(glob)
    return not glob:find("[*?%[]")

src/luacheck/lfs_fs.lua

@@ -0,0 +1,21 @@
+local lfs = require "lfs"
+
+local lfs_fs = {}
+
+function lfs_fs.get_mode(path)
+   return lfs.attributes(path, "mode")
+end
+
+function lfs_fs.get_current_dir()
+   return assert(lfs.currentdir())
+end
+
+function lfs_fs.get_mtime(path)
+   return lfs.attributes(path, "modification")
+end
+
+function lfs_fs.dir_iter(path)
+   return lfs.dir(path)
+end
+
+return lfs_fs

src/luacheck/lua_fs.lua

@@ -0,0 +1,29 @@
+local utils = require "luacheck.utils"
+
+local lua_fs = {}
+
+local mode_cmd_template
+
+if utils.is_windows then
+   mode_cmd_template = [[if exist "%s\*" (echo directory) else (if exist "%s" echo "file")]]
+else
+   mode_cmd_template = [[if [ -d '%s' ]; then echo directory; elif [ -f '%s' ]; then echo file; fi]]
+end
+
+function lua_fs.get_mode(path)
+   local fh = assert(io.popen(mode_cmd_template:format(path, path)))
+   local mode = fh:read("*a"):match("^(%S*)")
+   fh:close()
+   return mode
+end
+
+local pwd_cmd = utils.is_windows and "cd" or "pwd"
+
+function lua_fs.get_current_dir()
+   local fh = assert(io.popen(pwd_cmd))
+   local current_dir = fh:read("*a"):gsub("\n$", "")
+   fh:close()
+   return current_dir
+end
+
+return lua_fs

src/luacheck/main.lua

@@ -355,7 +355,7 @@ patterns.]])
       for i, file in ipairs(files) do
          if not bad_files[i] and file ~= io.stdin then
             table.insert(cache_files, file)
-            local mtime = fs.mtime(file)
+            local mtime = fs.get_mtime(file)
             table.insert(cache_mtimes, mtime)
             sparse_mtimes[i] = mtime
          end

src/luacheck/version.lua

@@ -13,7 +13,8 @@ else
 end
 
 if fs.has_lfs then
-   version.lfs = fs.lfs._VERSION
+   local lfs = require "lfs"
+   version.lfs = lfs._VERSION
 else
    version.lfs = "Not found"
 end