Error follow-up

Split up the diagnostic levels so that error and warning levels aren't mixed. Also fix up deconstruct_keys implementation. Co-authored-by: Benoit Daloze <eregontp@gmail.com>

Author: kddnewton

bin/parse

@@ -8,7 +8,6 @@
 
 $:.unshift(File.expand_path("../lib", __dir__))
 require "prism"
-require "pp"
 
 if ARGV[0] == "-e"
  result = Prism.parse(ARGV[1])

docs/serialization.md

@@ -57,15 +57,15 @@ The comment type is one of:
 | --- | --- |
 | string | error message (ASCII-only characters) |
 | location | the location in the source this error applies to |
-| `1` | the level of the error, see pm_diagnostic_level_t for the values |
+| `1` | the level of the error: `0` for `fatal` |
 
 ## warning
 
 | # bytes | field |
 | --- | --- |
 | string | warning message (ASCII-only characters) |
 | location | the location in the source this warning applies to |
-| `1` | the level of the warning, see pm_diagnostic_level_t for the values |
+| `1` | the level of the warning: `0` for `default` and `1` for `verbose` |
 
 ## Structure
 

ext/prism/extension.c

@@ -368,19 +368,6 @@ parser_magic_comments(pm_parser_t *parser, VALUE source) {
  return magic_comments;
 }
 
-static VALUE get_diagnostic_level_symbol(uint8_t level) {
- switch (level) {
- case 0:
- return ID2SYM(rb_intern("error_default"));
- case 1:
- return ID2SYM(rb_intern("warning_verbose_not_nil"));
- case 2:
- return ID2SYM(rb_intern("warning_verbose_true"));
- default:
- rb_raise(rb_eRuntimeError, "Unknown level: %" PRIu8, level);
- }
-}
-
 /**
  * Extract out the data location from the parser into a Location instance if one
  * exists.
@@ -415,10 +402,19 @@ parser_errors(pm_parser_t *parser, rb_encoding *encoding, VALUE source) {
  LONG2FIX(error->location.end - error->location.start)
  };
 
+ VALUE level = Qnil;
+ switch (error->level) {
+ case PM_ERROR_LEVEL_FATAL:
+ level = ID2SYM(rb_intern("fatal"));
+ break;
+ default:
+ rb_raise(rb_eRuntimeError, "Unknown level: %" PRIu8, error->level);
+ }
+
  VALUE error_argv[] = {
  rb_enc_str_new_cstr(error->message, encoding),
  rb_class_new_instance(3, location_argv, rb_cPrismLocation),
- get_diagnostic_level_symbol(error->level)
+ level
  };
 
  rb_ary_push(errors, rb_class_new_instance(3, error_argv, rb_cPrismParseError));
@@ -442,10 +438,22 @@ parser_warnings(pm_parser_t *parser, rb_encoding *encoding, VALUE source) {
  LONG2FIX(warning->location.end - warning->location.start)
  };
 
+ VALUE level = Qnil;
+ switch (warning->level) {
+ case PM_WARNING_LEVEL_DEFAULT:
+ level = ID2SYM(rb_intern("default"));
+ break;
+ case PM_WARNING_LEVEL_VERBOSE:
+ level = ID2SYM(rb_intern("verbose"));
+ break;
+ default:
+ rb_raise(rb_eRuntimeError, "Unknown level: %" PRIu8, warning->level);
+ }
+
  VALUE warning_argv[] = {
  rb_enc_str_new_cstr(warning->message, encoding),
  rb_class_new_instance(3, location_argv, rb_cPrismLocation),
- get_diagnostic_level_symbol(warning->level)
+ level
  };
 
  rb_ary_push(warnings, rb_class_new_instance(3, warning_argv, rb_cPrismParseWarning));

include/prism/diagnostic.h

@@ -15,16 +15,22 @@
 #include <assert.h>
 
 /**
- * This enum represents the level of diagnostics generated during parsing.
+ * The levels of errors generated during parsing.
+ */
+typedef enum {
+ /** For errors that cannot be recovered from. */
+ PM_ERROR_LEVEL_FATAL = 0
+} pm_error_level_t;
+
+/**
+ * The levels of warnings generated during parsing.
  */
 typedef enum {
- /** The default level for errors. */
- PM_ERROR_DEFAULT = 0,
  /** For warnings which should be emitted if $VERBOSE != nil. */
- PM_WARNING_VERBOSE_NOT_NIL = 1,
+ PM_WARNING_LEVEL_DEFAULT = 0,
  /** For warnings which should be emitted if $VERBOSE == true. */
- PM_WARNING_VERBOSE_TRUE = 2
-} pm_diagnostic_level_t;
+ PM_WARNING_LEVEL_VERBOSE = 1
+} pm_warning_level_t;
 
 /**
  * This struct represents a diagnostic generated during parsing.
@@ -48,7 +54,10 @@ typedef struct {
  */
  bool owned;
 
- /** The level of the diagnostic, see pm_diagnostic_level_t for possible values. */
+ /**
+ * The level of the diagnostic, see `pm_error_level_t` and
+ * `pm_warning_level_t` for possible values.
+ */
  uint8_t level;
 } pm_diagnostic_t;
 

java/org/prism/ParseResult.java

@@ -13,35 +13,40 @@ public MagicComment(Nodes.Location keyLocation, Nodes.Location valueLocation) {
  }
  }
 
- public enum DiagnosticLevel {
- /** The default level for errors. */
- ERROR_DEFAULT,
- /** For warnings which should be emitted if $VERBOSE != nil. */
- WARNING_VERBOSE_NOT_NIL,
- /** For warnings which should be emitted if $VERBOSE == true. */
- WARNING_VERBOSE_TRUE
+ public enum ErrorLevel {
+ /** For errors that cannot be recovered from. */
+ ERROR_FATAL
  }
 
- public static DiagnosticLevel[] DIAGNOSTIC_LEVELS = DiagnosticLevel.values();
+ public static ErrorLevel[] ERROR_LEVELS = ErrorLevel.values();
 
  public static final class Error {
  public final String message;
  public final Nodes.Location location;
- public final DiagnosticLevel level;
+ public final ErrorLevel level;
 
- public Error(String message, Nodes.Location location, DiagnosticLevel level) {
+ public Error(String message, Nodes.Location location, ErrorLevel level) {
  this.message = message;
  this.location = location;
  this.level = level;
  }
  }
 
+ public enum WarningLevel {
+ /** For warnings which should be emitted if $VERBOSE != nil. */
+ WARNING_DEFAULT,
+ /** For warnings which should be emitted if $VERBOSE == true. */
+ WARNING_VERBOSE
+ }
+
+ public static WarningLevel[] WARNING_LEVELS = WarningLevel.values();
+
  public static final class Warning {
  public final String message;
  public final Nodes.Location location;
- public final DiagnosticLevel level;
+ public final WarningLevel level;
 
- public Warning(String message, Nodes.Location location, DiagnosticLevel level) {
+ public Warning(String message, Nodes.Location location, WarningLevel level) {
  this.message = message;
  this.location = location;
  this.level = level;

lib/prism/parse_result.rb

@@ -312,7 +312,7 @@ class ParseError
  # A Location object representing the location of this error in the source.
  attr_reader :location
 
- # The level of this error
+ # The level of this error.
  attr_reader :level
 
  # Create a new error object with the given message and location.
@@ -324,7 +324,7 @@ def initialize(message, location, level)
 
  # Implement the hash pattern matching interface for ParseError.
  def deconstruct_keys(keys)
- { message: message, location: location }
+ { message: message, location: location, level: level }
  end
 
  # Returns a string representation of this error.
@@ -341,7 +341,7 @@ class ParseWarning
  # A Location object representing the location of this warning in the source.
  attr_reader :location
 
- # The level of this warning
+ # The level of this warning.
  attr_reader :level
 
  # Create a new warning object with the given message and location.
@@ -353,7 +353,7 @@ def initialize(message, location, level)
 
  # Implement the hash pattern matching interface for ParseWarning.
  def deconstruct_keys(keys)
- { message: message, location: location, verbose_only: verbose_only }
+ { message: message, location: location, level: level }
  end
 
  # Returns a string representation of this warning.