Simplify Crystal::System interface by adding File.stat? and lstat? (#…

…5553) By providing these methods we can make the implementation of File.empty? and File.file? platform-unspecific. This makes the interface to Crystal::System::File smaller and cleaner.
crystal-lang · Jan 13, 2018 · 48a1130 · 48a1130
1 parent 77de91f
commit 48a1130
Showing 7 changed files with 160 additions and 64 deletions.
diff --git a/spec/std/dir_spec.cr b/spec/std/dir_spec.cr
@@ -37,10 +37,17 @@ describe "Dir" do
     end
 
     it "tests empty? on nonexistent directory" do
-      expect_raises Errno do
+      expect_raises(Errno, /Error determining size of/) do
         Dir.empty?(File.join([__DIR__, "/foo/bar/"]))
       end
     end
+
+    it "tests empty? on a directory path to a file" do
+      ex = expect_raises(Errno, /Error determining size of/) do
+        Dir.empty?("#{__FILE__}/")
+      end
+      ex.errno.should eq(Errno::ENOTDIR)
+    end
   end
 
   it "tests mkdir and rmdir with a new path" do

diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr
@@ -116,10 +116,18 @@ describe "File" do
 
     it "raises an error when the file does not exist" do
       filename = "#{__DIR__}/data/non_existing_file.txt"
-      expect_raises Errno do
+      expect_raises(Errno, /Error determining size/) do
         File.empty?(filename)
       end
     end
+
+    it "raises an error when a component of the path is a file" do
+      filename = "#{__DIR__}/data/non_existing_file.txt"
+      ex = expect_raises(Errno, /Error determining size/) do
+        File.empty?("#{__FILE__}/")
+      end
+      ex.errno.should eq(Errno::ENOTDIR)
+    end
   end
 
   describe "exists?" do
@@ -130,24 +138,52 @@ describe "File" do
     it "gives false" do
       File.exists?("#{__DIR__}/data/non_existing_file.txt").should be_false
     end
+
+    it "gives false when a component of the path is a file" do
+      File.exists?("#{__FILE__}/").should be_false
+    end
   end
 
   describe "executable?" do
     it "gives false" do
       File.executable?("#{__DIR__}/data/test_file.txt").should be_false
     end
+
+    it "gives false when the file doesn't exist" do
+      File.executable?("#{__DIR__}/data/non_existing_file.txt").should be_false
+    end
+
+    it "gives false when a component of the path is a file" do
+      File.executable?("#{__FILE__}/").should be_false
+    end
   end
 
   describe "readable?" do
     it "gives true" do
       File.readable?("#{__DIR__}/data/test_file.txt").should be_true
     end
+
+    it "gives false when the file doesn't exist" do
+      File.readable?("#{__DIR__}/data/non_existing_file.txt").should be_false
+    end
+
+    it "gives false when a component of the path is a file" do
+      File.readable?("#{__FILE__}/").should be_false
+    end
   end
 
   describe "writable?" do
     it "gives true" do
       File.writable?("#{__DIR__}/data/test_file.txt").should be_true
     end
+
+    it "gives false when the file doesn't exist" do
+      File.writable?("#{__DIR__}/data/non_existing_file.txt").should be_false
+    end
+
+    it "gives false when a component of the path is a file" do
+      File.writable?("#{__FILE__}/").should be_false
+    end
   end
 
   describe "file?" do
@@ -158,6 +194,14 @@ describe "File" do
     it "gives false" do
       File.file?("#{__DIR__}/data").should be_false
     end
+
+    it "gives false when the file doesn't exist" do
+      File.file?("#{__DIR__}/data/non_existing_file.txt").should be_false
+    end
+
+    it "gives false when a component of the path is a file" do
+      File.file?("#{__FILE__}/").should be_false
+    end
   end
 
   describe "directory?" do
@@ -168,6 +212,14 @@ describe "File" do
     it "gives false" do
       File.directory?("#{__DIR__}/data/test_file.txt").should be_false
     end
+
+    it "gives false when the directory doesn't exist" do
+      File.directory?("#{__DIR__}/data/non_existing").should be_false
+    end
+
+    it "gives false when a component of the path is a file" do
+      File.directory?("#{__FILE__}/").should be_false
+    end
   end
 
   describe "link" do
@@ -204,6 +256,14 @@ describe "File" do
       File.symlink?("#{__DIR__}/data/test_file.txt").should be_false
       File.symlink?("#{__DIR__}/data/unknown_file.txt").should be_false
     end
+
+    it "gives false when the symlink doesn't exist" do
+      File.symlink?("#{__DIR__}/data/non_existing_file.txt").should be_false
+    end
+
+    it "gives false when a component of the path is a file" do
+      File.symlink?("#{__FILE__}/").should be_false
+    end
   end
 
   it "gets dirname" do
@@ -376,6 +436,21 @@ describe "File" do
         file.size.should eq(240)
       end
     end
+
+    it "raises an error when the file does not exist" do
+      filename = "#{__DIR__}/data/non_existing_file.txt"
+      expect_raises(Errno, /Error determining size/) do
+        File.size(filename)
+      end
+    end
+
+    it "raises an error when a component of the path is a file" do
+      filename = "#{__DIR__}/data/non_existing_file.txt"
+      ex = expect_raises(Errno, /Error determining size/) do
+        File.size("#{__FILE__}/")
+      end
+      ex.errno.should eq(Errno::ENOTDIR)
+    end
   end
 
   describe "delete" do

diff --git a/src/crystal/system/dir.cr b/src/crystal/system/dir.cr
@@ -19,9 +19,6 @@ module Crystal::System::Dir
   # Sets the current working directory of the application.
   # def self.current=(path : String)
 
-  # Returns `true` if *path* exists and is a directory.
-  # def self.exists?(path : String) : Bool
-
   # Creates a new directory at *path*. The UNIX-style directory mode *node*
   # must be applied.
   # def self.create(path : String, mode : Int32) : Nil

diff --git a/src/crystal/system/unix/dir.cr b/src/crystal/system/unix/dir.cr
@@ -48,18 +48,6 @@ module Crystal::System::Dir
     path
   end
 
-  def self.exists?(path : String) : Bool
-    if LibC.stat(path.check_no_null_byte, out stat) != 0
-      if Errno.value == Errno::ENOENT || Errno.value == Errno::ENOTDIR
-        return false
-      else
-        raise Errno.new("stat")
-      end
-    end
-
-    (stat.st_mode & LibC::S_IFMT) == LibC::S_IFDIR
-  end
-
   def self.create(path : String, mode : Int32) : Nil
     if LibC.mkdir(path.check_no_null_byte, mode) == -1
       raise Errno.new("Unable to create directory '#{path}'")

diff --git a/src/crystal/system/unix/file.cr b/src/crystal/system/unix/file.cr
@@ -70,28 +70,28 @@ module Crystal::System::File
     tmpdir.rchop(::File::SEPARATOR)
   end
 
-  def self.stat(path)
+  def self.stat?(path : String) : ::File::Stat?
     if LibC.stat(path.check_no_null_byte, out stat) != 0
-      raise Errno.new("Unable to get stat for '#{path}'")
+      if {Errno::ENOENT, Errno::ENOTDIR}.includes? Errno.value
+        return nil
+      else
+        raise Errno.new("Unable to get stat for '#{path}'")
+      end
     end
     ::File::Stat.new(stat)
   end
 
-  def self.lstat(path)
+  def self.lstat?(path : String) : ::File::Stat?
     if LibC.lstat(path.check_no_null_byte, out stat) != 0
-      raise Errno.new("Unable to get lstat for '#{path}'")
+      if {Errno::ENOENT, Errno::ENOTDIR}.includes? Errno.value
+        return nil
+      else
+        raise Errno.new("Unable to get lstat for '#{path}'")
+      end
     end
     ::File::Stat.new(stat)
   end
 
-  def self.empty?(path)
-    begin
-      stat(path).size == 0
-    rescue Errno
-      raise Errno.new("Error determining size of '#{path}'")
-    end
-  end
-
   def self.exists?(path)
     accessible?(path, LibC::F_OK)
   end
@@ -112,19 +112,8 @@ module Crystal::System::File
     LibC.access(path.check_no_null_byte, flag) == 0
   end
 
-  def self.file?(path) : Bool
-    if LibC.stat(path.check_no_null_byte, out stat) != 0
-      if Errno.value == Errno::ENOENT
-        return false
-      else
-        raise Errno.new("stat")
-      end
-    end
-    ::File::Stat.new(stat).file?
-  end
-
   def self.chown(path, uid : Int, gid : Int, follow_symlinks)
-    ret = if !follow_symlinks && symlink?(path)
+    ret = if !follow_symlinks && ::File.symlink?(path)
             LibC.lchown(path, uid, gid)
           else
             LibC.chown(path, uid, gid)
@@ -163,17 +152,6 @@ module Crystal::System::File
     ret
   end
 
-  def self.symlink?(path)
-    if LibC.lstat(path.check_no_null_byte, out stat) != 0
-      if Errno.value == Errno::ENOENT
-        return false
-      else
-        raise Errno.new("stat")
-      end
-    end
-    (stat.st_mode & LibC::S_IFMT) == LibC::S_IFLNK
-  end
-
   def self.rename(old_filename, new_filename)
     code = LibC.rename(old_filename.check_no_null_byte, new_filename.check_no_null_byte)
     if code != 0

diff --git a/src/dir.cr b/src/dir.cr
@@ -193,7 +193,11 @@ class Dir
 
   # Returns `true` if the given path exists and is a directory
   def self.exists?(path) : Bool
-    Crystal::System::Dir.exists? path
+    if stat = File.stat?(path)
+      stat.directory?
+    else
+      false
+    end
   end
 
   # Returns `true` if the directory at *path* is empty, otherwise returns `false`.

diff --git a/src/file.cr b/src/file.cr
@@ -34,6 +34,36 @@ class File < IO::FileDescriptor
 
   getter path : String
 
+  # Returns a `File::Stat` object for the file given by *path* or returns `nil`
+  # if the file does not exist. Raises `Errno` in case of an error. In case of
+  # a symbolic link it is followed and information about the target is returned.
+  #
+  # ```
+  # File.write("foo", "foo")
+  # File.stat?("foo").try(&.size) # => 3
+  # File.stat?("non_existent")    # => nil
+  # ```
+  def self.stat?(path : String) : Stat?
+    Crystal::System::File.stat?(path)
+  end
+
+  # Returns a `File::Stat` object for the file given by *path* or returns `nil`
+  # if the file does not exist. Raises `Errno` in case of an error. In case of
+  # a symbolic link information about the link itself is returned.
+  #
+  # ```
+  # File.write("foo", "foo")
+  # File.lstat?("foo").try(&.size) # => 3
+  #
+  # File.symlink("foo", "bar")
+  # File.lstat?("bar").try(&.symlink?) # => true
+  #
+  # File.lstat?("non_existent") # => nil
+  # ```
+  def self.lstat?(path : String) : Stat?
+    Crystal::System::File.lstat?(path)
+  end
+
   # Returns a `File::Stat` object for the file given by *path* or raises
   # `Errno` in case of an error. In case of a symbolic link
   # it is followed and information about the target is returned.
@@ -44,20 +74,20 @@ class File < IO::FileDescriptor
   # File.stat("foo").mtime # => 2015-09-23 06:24:19 UTC
   # ```
   def self.stat(path) : Stat
-    Crystal::System::File.stat(path)
+    stat?(path) || raise Errno.new("Unable to get stat for #{path.inspect}")
   end
 
   # Returns a `File::Stat` object for the file given by *path* or raises
   # `Errno` in case of an error. In case of a symbolic link
-  # information about it is returned.
+  # information about the link itself is returned.
   #
   # ```
   # File.write("foo", "foo")
   # File.lstat("foo").size  # => 3
   # File.lstat("foo").mtime # => 2015-09-23 06:24:19 UTC
   # ```
   def self.lstat(path) : Stat
-    Crystal::System::File.lstat(path)
+    lstat?(path) || raise Errno.new("Unable to get stat for #{path.inspect}")
   end
 
   # Returns `true` if *path* exists else returns `false`
@@ -72,6 +102,20 @@ class File < IO::FileDescriptor
     Crystal::System::File.exists?(path)
   end
 
+  # Returns the size of *filename* bytes. Raises `Errno` if the file at *path*
+  # does not exist.
+  #
+  # ```
+  # File.size("foo") # raises Errno
+  # File.write("foo", "foo")
+  # File.size("foo") # => 3
+  # ```
+  def self.size(filename) : UInt64
+    stat(filename).size
+  rescue ex : Errno
+    raise Errno.new("Error determining size of #{filename.inspect}", ex.errno)
+  end
+
   # Returns `true` if the file at *path* is empty, otherwise returns `false`.
   # Raises `Errno` if the file at *path* does not exist.
   #
@@ -82,7 +126,7 @@ class File < IO::FileDescriptor
   # File.empty?("foo") # => false
   # ```
   def self.empty?(path) : Bool
-    Crystal::System::File.empty?(path)
+    size(path) == 0
   end
 
   # Returns `true` if *path* is readable by the real user id of this process else returns `false`.
@@ -125,7 +169,11 @@ class File < IO::FileDescriptor
   # File.file?("foobar") # => false
   # ```
   def self.file?(path) : Bool
-    Crystal::System::File.file?(path)
+    if stat = stat?(path)
+      stat.file?
+    else
+      false
+    end
   end
 
   # Returns `true` if the given *path* exists and is a directory.
@@ -527,7 +575,11 @@ class File < IO::FileDescriptor
 
   # Returns `true` if the *path* is a symbolic link.
   def self.symlink?(path) : Bool
-    Crystal::System::File.symlink?(path)
+    if stat = lstat?(path)
+      stat.symlink?
+    else
+      false
+    end
   end
 
   # Opens the file named by *filename*. If a file is being created, its initial
@@ -677,11 +729,6 @@ class File < IO::FileDescriptor
     end
   end
 
-  # Returns the size of *filename* bytes.
-  def self.size(filename) : UInt64
-    stat(filename.check_no_null_byte).size
-  end
-
   # Moves *old_filename* to *new_filename*.
   #
   # ```