Remove unnecessarily obfuscating indirection

cucumber · Aug 16, 2023 · daf4340 · daf4340
1 parent dee52e8
commit daf4340
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 29 deletions.
diff --git a/perl/gherkin-perl.razor b/perl/gherkin-perl.razor
@@ -104,7 +104,6 @@ sub match_@(rule.Name.Replace("#", "")) {
     }
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_@(rule.Name.Replace("#", ""))( $token ) }
     );
 }

diff --git a/perl/lib/Gherkin/Generated/Parser.pm b/perl/lib/Gherkin/Generated/Parser.pm
@@ -116,7 +116,6 @@ sub match_EOF {
     my ($self, $context, $token) = @_;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_EOF( $token ) }
     );
 }
@@ -126,7 +125,6 @@ sub match_Empty {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_Empty( $token ) }
     );
 }
@@ -136,7 +134,6 @@ sub match_Comment {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_Comment( $token ) }
     );
 }
@@ -146,7 +143,6 @@ sub match_TagLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_TagLine( $token ) }
     );
 }
@@ -156,7 +152,6 @@ sub match_FeatureLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_FeatureLine( $token ) }
     );
 }
@@ -166,7 +161,6 @@ sub match_RuleLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_RuleLine( $token ) }
     );
 }
@@ -176,7 +170,6 @@ sub match_BackgroundLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_BackgroundLine( $token ) }
     );
 }
@@ -186,7 +179,6 @@ sub match_ScenarioLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_ScenarioLine( $token ) }
     );
 }
@@ -196,7 +188,6 @@ sub match_ExamplesLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_ExamplesLine( $token ) }
     );
 }
@@ -206,7 +197,6 @@ sub match_StepLine {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_StepLine( $token ) }
     );
 }
@@ -216,7 +206,6 @@ sub match_DocStringSeparator {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_DocStringSeparator( $token ) }
     );
 }
@@ -226,7 +215,6 @@ sub match_TableRow {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_TableRow( $token ) }
     );
 }
@@ -236,7 +224,6 @@ sub match_Language {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_Language( $token ) }
     );
 }
@@ -246,7 +233,6 @@ sub match_Other {
     return if $token->is_eof;
     return $self->handle_external_error(
         $context,
-        0, # Default return value
         sub { $context->token_matcher->match_Other( $token ) }
     );
 }

diff --git a/perl/lib/Gherkin/ParserBase.pm b/perl/lib/Gherkin/ParserBase.pm
@@ -29,6 +29,7 @@ sub get_result { return $_[0]->ast_builder->get_result }
 sub add_error {
     my ( $self, $context, $error ) = @_;
 
+    die $error if not ref $error; # rethrow if unstructured; not ours
     die $error if $self->stop_at_first_error;
     if ( ref $error eq 'Gherkin::Exceptions::CompositeParser' ) {
         $context->add_errors( @{ $error->errors } );
@@ -43,30 +44,28 @@ sub add_error {
 
 sub _start_rule {
     my ( $self, $context, $ruleType ) = @_;
-    $self->_handle_ast_error( $context, start_rule => $ruleType );
+
+    if (not eval { $self->ast_builder->start_rule( $ruleType ); 1 }) {
+        $self->add_error( $context, $@ );
+    }
 }
 
 sub _end_rule {
     my ( $self, $context, $ruleType ) = @_;
-    $self->_handle_ast_error( $context, end_rule => $ruleType );
+    if (not eval { $self->ast_builder->end_rule( $ruleType ); 1 }) {
+        $self->add_error( $context, $@ );
+    }
 }
 
 sub _build {
     my ( $self, $context, $token ) = @_;
-    $self->_handle_ast_error( $context, build => $token );
-}
-
-sub _handle_ast_error {
-    my ( $self, $context, $method_name, $arg ) = @_;
-    my $action = sub {
-        $self->ast_builder->$method_name($arg);
-    };
-
-    $self->handle_external_error( $context, 1, $action );
+    if (not eval { $self->ast_builder->build( $token ); 1 }) {
+        $self->add_error( $context, $@ );
+    }
 }
 
 sub handle_external_error {
-    my ( $self, $context, $default_value, $action ) = @_;
+    my ( $self, $context, $action ) = @_;
     return $action->() if $self->stop_at_first_error;
 
     my $result = eval { $action->() };
@@ -76,7 +75,7 @@ sub handle_external_error {
     die $@ unless ref $@;
 
     $self->add_error( $context, $@ );
-    return $default_value;
+    return 0; # failed
 }
 
 1;