Skip to content

Report a malformed class instance variable as a syntax error - #3149

Open
Halvanhelv wants to merge 1 commit into
ruby:masterfrom
Halvanhelv:class-instance-variable-syntax-error
Open

Report a malformed class instance variable as a syntax error#3149
Halvanhelv wants to merge 1 commit into
ruby:masterfrom
Halvanhelv:class-instance-variable-syntax-error

Conversation

@Halvanhelv

Copy link
Copy Markdown

Fixes #3148.

class M self. raises RuntimeError: Unexpected error instead of RBS::ParsingError, so it escapes rescue RBS::ParsingError and the CLI prints a raw Ruby backtrace. Every other malformed member in the same position reports correctly, and so does the same construct under interface:

input before
class M <<< RBS::ParsingError with location and caret
interface _I self. RBS::ParsingError
class M self. RuntimeError: Unexpected error
class M self.5 RuntimeError: Unexpected error

Cause

src/parser.c:2353, in the kSELF branch of parse_variable_member, passes false for the bool syntax_error parameter (include/rbs/parser.h:161). raise_error in ext/rbs_extension/main.c then discards the location, token and message and raises a bare RuntimeError:

if (!error->syntax_error) {
    rb_raise(rb_eRuntimeError, "Unexpected error");
}

49 other call sites in src/parser.c pass true and produce proper RBS::ParsingErrors.

Fix

Pass syntax_error with a specific message, in the wording used by the neighbouring sites.

The error is also set on next_token rather than current_token: the branch tests parser->next_token.type, so next_token is the offending token, and current_token would put the caret on the dot. Sibling sites that test next_token set the error on it the same way (lines 178, 288, 316, 325, 555, 665, 688, 704, 981).

Scope — why only this one site

src/parser.c has 12 other syntax_error = false sites. I checked them empirically rather than by eye: I tagged each with a unique marker, temporarily stopped main.c from discarding error->message, rebuilt, and ran targeted probes plus 25,000 mutation-fuzz iterations over the bundled signatures. Only 2353 was reached from user input. The rest are defensive default: arms over already-exhausted enums — e.g. the switch at 2383 covers exactly the four token types its caller dispatches on — so they indicate a parser bug, and RuntimeError is the correct semantic there. Left untouched.

Result

class M self.      1:13...1:13: unexpected token for class instance variable name, token=`` (pEOF)
module M self.     1:14...1:14: ... token=`` (pEOF)
class M self.5     1:13...1:14: ... token=`5` (tINTEGER)
class M self."x"   1:13...1:16: ... token=`"x"` (tDQSTRING)

CLI now prints a formatted error instead of a backtrace:

/tmp/bug.rbs:1:13...1:13: Syntax error: unexpected token for class instance variable name, token=`` (pEOF) (RBS::ParsingError)

  class M self.
               ^

Valid signatures are unchanged — self.@x, @x and @@x all still parse, and all 342 bundled core/, stdlib/ and sig/ files parse with no failures. Re-running the 25,000-input fuzz against the fixed build gives 23,649 RBS::ParsingError and 0 RuntimeError.

Tests

Added two tests to test/rbs/errors_test.rb: one asserting RBS::ParsingError across the four malformed forms, one checking the exact detailed_message and caret position. Both fail with RuntimeError(Unexpected error) without the fix.

Full suite before: 999 tests, 8198 assertions, 0 failures, 0 errors.
Full suite after: 1001 tests, 8204 assertions, 0 failures, 0 errors.

The `kSELF` branch of `parse_variable_member` reported a missing or
invalid variable name by calling `rbs_parser_set_error` with
`syntax_error` set to `false`. `raise_error` discards the location, token
and message in that case and raises a bare `RuntimeError`, so
`class M self.` escaped `rescue RBS::ParsingError` and the CLI printed a
Ruby backtrace instead of a formatted syntax error. Every other malformed
member in the same position, and the same construct under `interface`,
already reported an `RBS::ParsingError`.

Pass `syntax_error` and a specific message, as the other syntax error
sites do. Set the error on `next_token`, which is the token the branch
tests and therefore the offending one; `current_token` would point the
caret at the dot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Malformed class instance variable member raises RuntimeError instead of RBS::ParsingError

1 participant