Skip to content

Commit

Permalink
Merge pull request #2283 from ruby/error-follow-up
Browse files Browse the repository at this point in the history
Error follow-up
  • Loading branch information
kddnewton authored Jan 27, 2024
2 parents 44d5b63 + bd3eeb3 commit d69b9da
Show file tree
Hide file tree
Showing 11 changed files with 320 additions and 287 deletions.
1 change: 0 additions & 1 deletion bin/parse
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

$:.unshift(File.expand_path("../lib", __dir__))
require "prism"
require "pp"

if ARGV[0] == "-e"
result = Prism.parse(ARGV[1])
Expand Down
4 changes: 2 additions & 2 deletions docs/serialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
38 changes: 23 additions & 15 deletions ext/prism/extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down
23 changes: 16 additions & 7 deletions include/prism/diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;

Expand Down
29 changes: 17 additions & 12 deletions java/org/prism/ParseResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions lib/prism/parse_result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
Loading

0 comments on commit d69b9da

Please sign in to comment.