Implement file parsing error handling

This PR implements proper file parsing error handling. Previously
`file_options` would call `pm_string_mapped_init` which would print an
error from `perror`. However this wouldn't raise a proper Ruby error so
it was just a string output. I've done the following:

- Raise an error from `rb_syserr_fail` with the filepath in
`file_options`.
- No longer return `Qnil` if `file_options` returns false (because now
it will raise)
- Update `file_options` to return `static void` instead of `static
bool`.
- Update `file_options` and `profile_file` to check the type so when
passing `nil` we see a `TypeError`.
- Delete `perror` from `pm_string_mapped_init`
- Update `FFI` backend to raise appropriate errors when calling
`pm_string_mapped_init`.
- Add tests for `dump_file`, `lex_file`, `parse_file`,
`parse_file_comments`, `parse_lex_file`, and `parse_file_success?`
when a file doesn't exist and for `nil`.
- Updates the `bin/parse` script to no longer raise it's own
`ArgumentError` now that we raise a proper error.

Fixes: #2207

bin/parse

@@ -13,8 +13,6 @@ if ARGV[0] == "-e"
   result = Prism.parse(ARGV[1])
 else
   result = Prism.parse_file(ARGV[0] || "test.rb")
-
-  raise ArgumentError, "There was nothing to parse. You must pass source code with `-e` or a Ruby file." if result.nil?
 end
 
 result.mark_newlines! if ENV["MARK_NEWLINES"]

ext/prism/extension.c

@@ -1,5 +1,9 @@
 #include "prism/extension.h"
 
+#ifdef _WIN32
+#include <ruby/win32.h>
+#endif
+
 // NOTE: this file should contain only bindings. All non-trivial logic should be
 // in libprism so it can be shared its the various callers.
 
@@ -212,20 +216,29 @@ string_options(int argc, VALUE *argv, pm_string_t *input, pm_options_t *options)
 /**
  * Read options for methods that look like (filepath, **options).
  */
-static bool
+static void
 file_options(int argc, VALUE *argv, pm_string_t *input, pm_options_t *options) {
     VALUE filepath;
     VALUE keywords;
     rb_scan_args(argc, argv, "1:", &filepath, &keywords);
 
+    Check_Type(filepath, T_STRING);
+
     extract_options(options, filepath, keywords);
 
-    if (!pm_string_mapped_init(input, (const char *) pm_string_source(&options->filepath))) {
+    const char * string_source = (const char *) pm_string_source(&options->filepath);
+
+    if (!pm_string_mapped_init(input, string_source)) {
         pm_options_free(options);
-        return false;
-    }
 
-    return true;
+#ifdef _WIN32
+        int e = rb_w32_map_errno(GetLastError());
+#else
+        int e = errno;
+#endif
+
+        rb_syserr_fail(e, string_source);
+    }
 }
 
 /******************************************************************************/
@@ -299,7 +312,8 @@ static VALUE
 dump_file(int argc, VALUE *argv, VALUE self) {
     pm_string_t input;
     pm_options_t options = { 0 };
-    if (!file_options(argc, argv, &input, &options)) return Qnil;
+
+    file_options(argc, argv, &input, &options);
 
     VALUE value = dump_input(&input, &options);
     pm_string_free(&input);
@@ -609,7 +623,8 @@ static VALUE
 lex_file(int argc, VALUE *argv, VALUE self) {
     pm_string_t input;
     pm_options_t options = { 0 };
-    if (!file_options(argc, argv, &input, &options)) return Qnil;
+
+    file_options(argc, argv, &input, &options);
 
     VALUE value = parse_lex_input(&input, &options, false);
     pm_string_free(&input);
@@ -710,7 +725,8 @@ static VALUE
 parse_file(int argc, VALUE *argv, VALUE self) {
     pm_string_t input;
     pm_options_t options = { 0 };
-    if (!file_options(argc, argv, &input, &options)) return Qnil;
+
+    file_options(argc, argv, &input, &options);
 
     VALUE value = parse_input(&input, &options);
     pm_string_free(&input);
@@ -770,7 +786,8 @@ static VALUE
 parse_file_comments(int argc, VALUE *argv, VALUE self) {
     pm_string_t input;
     pm_options_t options = { 0 };
-    if (!file_options(argc, argv, &input, &options)) return Qnil;
+
+    file_options(argc, argv, &input, &options);
 
     VALUE value = parse_input_comments(&input, &options);
     pm_string_free(&input);
@@ -824,7 +841,8 @@ static VALUE
 parse_lex_file(int argc, VALUE *argv, VALUE self) {
     pm_string_t input;
     pm_options_t options = { 0 };
-    if (!file_options(argc, argv, &input, &options)) return Qnil;
+
+    file_options(argc, argv, &input, &options);
 
     VALUE value = parse_lex_input(&input, &options, true);
     pm_string_free(&input);
@@ -881,7 +899,8 @@ static VALUE
 parse_file_success_p(int argc, VALUE *argv, VALUE self) {
     pm_string_t input;
     pm_options_t options = { 0 };
-    if (!file_options(argc, argv, &input, &options)) return Qnil;
+
+    file_options(argc, argv, &input, &options);
 
     VALUE result = parse_input_success_p(&input, &options);
     pm_string_free(&input);
@@ -959,7 +978,17 @@ profile_file(VALUE self, VALUE filepath) {
     pm_string_t input;
 
     const char *checked = check_string(filepath);
-    if (!pm_string_mapped_init(&input, checked)) return Qnil;
+    Check_Type(filepath, T_STRING);
+
+    if (!pm_string_mapped_init(&input, checked)) {
+#ifdef _WIN32
+        int e = rb_w32_map_errno(GetLastError());
+#else
+        int e = errno;
+#endif
+
+        rb_syserr_fail(e, checked);
+    }
 
     pm_options_t options = { 0 };
     pm_options_filepath_set(&options, checked);

lib/prism/ffi.rb

@@ -160,8 +160,13 @@ def self.with(filepath, &block)
         pointer = FFI::MemoryPointer.new(SIZEOF)
 
         begin
-          raise unless LibRubyParser.pm_string_mapped_init(pointer, filepath)
-          yield new(pointer)
+          raise TypeError unless filepath.is_a?(String)
+
+          if LibRubyParser.pm_string_mapped_init(pointer, filepath)
+            yield new(pointer)
+          else
+            raise SystemCallError.new(filepath, FFI.errno)
+          end
         ensure
           LibRubyParser.pm_string_free(pointer)
           pointer.free

src/util/pm_string.c

@@ -65,15 +65,13 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
     HANDLE file = CreateFile(filepath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
 
     if (file == INVALID_HANDLE_VALUE) {
-        perror("CreateFile failed");
         return false;
     }
 
     // Get the file size.
     DWORD file_size = GetFileSize(file, NULL);
     if (file_size == INVALID_FILE_SIZE) {
         CloseHandle(file);
-        perror("GetFileSize failed");
         return false;
     }
 
@@ -90,7 +88,6 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
     HANDLE mapping = CreateFileMapping(file, NULL, PAGE_READONLY, 0, 0, NULL);
     if (mapping == NULL) {
         CloseHandle(file);
-        perror("CreateFileMapping failed");
         return false;
     }
 
@@ -100,7 +97,6 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
     CloseHandle(file);
 
     if (source == NULL) {
-        perror("MapViewOfFile failed");
         return false;
     }
 
@@ -110,15 +106,13 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
     // Open the file for reading
     int fd = open(filepath, O_RDONLY);
     if (fd == -1) {
-        perror("open");
         return false;
     }
 
     // Stat the file to get the file size
     struct stat sb;
     if (fstat(fd, &sb) == -1) {
         close(fd);
-        perror("fstat");
         return false;
     }
 
@@ -135,7 +129,6 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
 
     source = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
     if (source == MAP_FAILED) {
-        perror("Map failed");
         return false;
     }
 

test/prism/parse_test.rb

@@ -69,11 +69,96 @@ def test_parse_lex
       assert_equal 5, tokens.length
     end
 
+    def test_dump_file
+      assert_nothing_raised do
+        Prism.dump_file(__FILE__)
+      end
+
+      error = assert_raise Errno::ENOENT do
+        Prism.dump_file("idontexist.rb")
+      end
+
+      assert_equal "No such file or directory - idontexist.rb", error.message
+
+      assert_raise TypeError do
+        Prism.dump_file(nil)
+      end
+    end
+
+    def test_lex_file
+      assert_nothing_raised do
+        Prism.lex_file(__FILE__)
+      end
+
+      error = assert_raise Errno::ENOENT do
+        Prism.lex_file("idontexist.rb")
+      end
+
+      assert_equal "No such file or directory - idontexist.rb", error.message
+
+      assert_raise TypeError do
+        Prism.lex_file(nil)
+      end
+    end
+
     def test_parse_lex_file
       node, tokens = Prism.parse_lex_file(__FILE__).value
 
       assert_kind_of ProgramNode, node
       refute_empty tokens
+
+      error = assert_raise Errno::ENOENT do
+        Prism.parse_lex_file("idontexist.rb")
+      end
+
+      assert_equal "No such file or directory - idontexist.rb", error.message
+
+      assert_raise TypeError do
+        Prism.parse_lex_file(nil)
+      end
+    end
+
+    def test_parse_file
+      node = Prism.parse_file(__FILE__).value
+      assert_kind_of ProgramNode, node
+
+      error = assert_raise Errno::ENOENT do
+        Prism.parse_file("idontexist.rb")
+      end
+
+      assert_equal "No such file or directory - idontexist.rb", error.message
+
+      assert_raise TypeError do
+        Prism.parse_file(nil)
+      end
+    end
+
+    def test_parse_file_success
+      assert_predicate Prism.parse_file_comments(__FILE__), :any?
+
+      error = assert_raise Errno::ENOENT do
+        Prism.parse_file_comments("idontexist.rb")
+      end
+
+      assert_equal "No such file or directory - idontexist.rb", error.message
+
+      assert_raise TypeError do
+        Prism.parse_file_comments(nil)
+      end
+    end
+
+    def test_parse_file_comments
+      assert_predicate Prism.parse_file_comments(__FILE__), :any?
+
+      error = assert_raise Errno::ENOENT do
+        Prism.parse_file_comments("idontexist.rb")
+      end
+
+      assert_equal "No such file or directory - idontexist.rb", error.message
+
+      assert_raise TypeError do
+        Prism.parse_file_comments(nil)
+      end
     end
 
     # To accurately compare against Ripper, we need to make sure that we're