Move action '$' variable checks to complete_and_validate

Author: ratmice

cfgrammar/src/lib/yacc/ast.rs

@@ -315,6 +315,7 @@ impl GrammarAST {
     ///   6) If `yacc_kind` is specified, perform any kind specific validation.
     ///      * If the kind requires an action type, check that each rule has one
     ///      * That each production has action code
+    ///      * That `$` variables referred to in action code are recognised.
     pub(crate) fn complete_and_validate(
         &mut self,
         yacc_kind: Option<YaccKind>,
@@ -349,12 +350,40 @@ impl GrammarAST {
             }
             for &pidx in &rule.pidxs {
                 let prod = &self.prods[pidx];
-                if kind_requires_action_checks && prod.action.is_none() {
-                    return Err(YaccGrammarError {
-                        kind: YaccGrammarErrorKind::MissingActionCode,
-                        spans: vec![prod.prod_span]
-                    });
+                if kind_requires_action_checks {
+                    if let Some((action_code, action_span)) = prod.action.as_ref() {
+                        let mut last = 0;
+                        while let Some(off) = action_code[last..].find('$') {
+                            if !(action_code[last + off..].starts_with("$$")
+                                || action_code[last + off..].starts_with("$lexer")
+                                || action_code[last + off..].starts_with("$span")
+                                || (last + off + 1 < action_code.len()
+                                    && action_code[last + off + 1..]
+                                        .starts_with(|c: char| c.is_numeric())))
+                            {
+                                // Starting from the `$` find the end of a variable name, otherwise default to the span of the `$`
+                                let m = crate::yacc::parser::RE_NAME
+                                    .find(&action_code[last + off + 1..]);
+                                let var_start_pos = action_span.start() + last + off;
+                                let var_end_pos = m
+                                    .map(|m| var_start_pos + 1 + m.end())
+                                    .unwrap_or(var_start_pos + 1);
+                                return Err(YaccGrammarError {
+                                    kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
+                                    spans: vec![Span::new(var_start_pos, var_end_pos)],
+                                });
+                            } else {
+                                last = last + off + 2;
+                            }
+                        }
+                    } else {
+                        return Err(YaccGrammarError {
+                            kind: YaccGrammarErrorKind::MissingActionCode,
+                            spans: vec![prod.prod_span],
+                        });
+                    }
                 }
+
                 if let Some(ref n) = prod.precedence {
                     if !self.tokens.contains(n) {
                         return Err(YaccGrammarError {
@@ -905,4 +934,56 @@ start: "a" { };
             })
         );
     }
+
+    #[test]
+    fn test_unrecognized_action_variable() {
+        use super::*;
+        let ast_validity = ASTWithValidityInfo::new(
+            YaccKind::Grmtools,
+            r#"
+%token a
+%%
+start -> () : "a" { $foo; };
+"#,
+        );
+        assert_eq!(
+            ast_validity.errors(),
+            vec![YaccGrammarError {
+                kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
+                spans: vec![Span::new(33, 37)],
+            }]
+        );
+
+        let ast_validity = ASTWithValidityInfo::new(
+            YaccKind::Grmtools,
+            r#"
+%token a
+%%
+start -> () : "a" {$};
+"#,
+        );
+        assert_eq!(
+            ast_validity.errors(),
+            vec![YaccGrammarError {
+                kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
+                spans: vec![Span::new(32, 33)],
+            }]
+        );
+
+        let ast_validity = ASTWithValidityInfo::new(
+            YaccKind::Grmtools,
+            r#"
+%token a
+%%
+start -> () : "a" {$;;;; };
+"#,
+        );
+        assert_eq!(
+            ast_validity.errors(),
+            vec![YaccGrammarError {
+                kind: YaccGrammarErrorKind::UnrecognisedActionVariable,
+                spans: vec![Span::new(32, 33)],
+            }]
+        );
+    }
 }

cfgrammar/src/lib/yacc/parser.rs

@@ -59,6 +59,7 @@ pub enum YaccGrammarErrorKind {
     InvalidString,
     NoStartRule,
     UnknownSymbol,
+    UnrecognisedActionVariable,
     InvalidStartRule(String),
     UnknownRuleRef(String),
     UnknownToken(String),
@@ -111,6 +112,9 @@ impl fmt::Display for YaccGrammarErrorKind {
             YaccGrammarErrorKind::UnknownDeclaration => "Unknown declaration",
             YaccGrammarErrorKind::DuplicatePrecedence => "Token has multiple precedences specified",
             YaccGrammarErrorKind::PrecNotFollowedByToken => "%prec not followed by token name",
+            YaccGrammarErrorKind::UnrecognisedActionVariable => {
+                "Unrecognised action variable following '$'"
+            }
             YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration => {
                 "Duplicated %avoid_insert declaration"
             }
@@ -264,6 +268,7 @@ impl Spanned for YaccGrammarError {
             | YaccGrammarErrorKind::InvalidString
             | YaccGrammarErrorKind::NoStartRule
             | YaccGrammarErrorKind::UnknownSymbol
+            | YaccGrammarErrorKind::UnrecognisedActionVariable
             | YaccGrammarErrorKind::InvalidStartRule(_)
             | YaccGrammarErrorKind::UnknownRuleRef(_)
             | YaccGrammarErrorKind::UnknownToken(_)
@@ -292,7 +297,7 @@ pub(crate) struct YaccParser<'a> {
     global_actiontype: Option<(String, Span)>,
 }
 
-static RE_NAME: LazyLock<Regex> =
+pub(crate) static RE_NAME: LazyLock<Regex> =
     LazyLock::new(|| Regex::new(r"^[a-zA-Z_.][a-zA-Z0-9_.]*").unwrap());
 static RE_TOKEN: LazyLock<Regex> =
     LazyLock::new(|| Regex::new("^(?:(\".+?\")|('.+?')|([a-zA-Z_][a-zA-Z_0-9]*))").unwrap());
@@ -848,7 +853,7 @@ impl YaccParser<'_> {
             Err(self.mk_error(YaccGrammarErrorKind::IncompleteAction, i))
         } else {
             debug_assert!(self.lookahead_is("}", j).is_some());
-            let s = self.src[i + '{'.len_utf8()..j].trim().to_string();
+            let s = self.src[i + '{'.len_utf8()..j].to_string();
             Ok((j + '}'.len_utf8(), s))
         }
     }
@@ -2293,12 +2298,12 @@ x"
           ",
         )
         .unwrap();
-        let action_str = "println!(\"test\");".to_string();
+        let action_str = " println!(\"test\"); ".to_string();
         assert_eq!(
             grm.prods[grm.rules["A"].pidxs[0]].action,
             Some((action_str.clone(), Span::new(34, 34 + action_str.len())))
         );
-        let action_str = "add($1, $2);".to_string();
+        let action_str = " add($1, $2); ".to_string();
         assert_eq!(
             grm.prods[grm.rules["B"].pidxs[0]].action,
             Some((action_str.clone(), Span::new(90, 90 + action_str.len())))

lrpar/src/lib/ctbuilder.rs

@@ -23,7 +23,7 @@ use crate::{
 use crate::unstable_api::UnstableApi;
 
 use cfgrammar::{
-    Location, RIdx, Span, Symbol,
+    Location, RIdx, Symbol,
     header::{
         GrmtoolsSectionParser, Header, HeaderError, HeaderErrorKind, HeaderValue, Namespaced,
         Setting, Value,
@@ -954,7 +954,6 @@ where
             &derived_mod_name,
             outp,
             &format!("/* CACHE INFORMATION {} */\n", cache),
-            &yacc_diag,
         )?;
         let conflicts = if stable.conflicts().is_some() {
             Some((sgraph, stable))
@@ -1083,14 +1082,13 @@ where
         mod_name: &str,
         outp_rs: P,
         cache: &str,
-        diag: &SpannedDiagnosticFormatter,
     ) -> Result<(), Box<dyn Error>> {
         let visibility = self.visibility.clone();
         let user_actions = if let Some(
             YaccKind::Original(YaccOriginalActionKind::UserAction) | YaccKind::Grmtools,
         ) = self.yacckind
         {
-            Some(self.gen_user_actions(grm, diag)?)
+            Some(self.gen_user_actions(grm)?)
         } else {
             None
         };
@@ -1611,11 +1609,7 @@ where
     }
 
     /// Generate the user action functions (if any).
-    fn gen_user_actions(
-        &self,
-        grm: &YaccGrammar<StorageT>,
-        diag: &SpannedDiagnosticFormatter,
-    ) -> Result<TokenStream, Box<dyn Error>> {
+    fn gen_user_actions(&self, grm: &YaccGrammar<StorageT>) -> Result<TokenStream, Box<dyn Error>> {
         let programs = grm
             .programs()
             .as_ref()
@@ -1714,18 +1708,7 @@ where
                             write!(outs, "{prefix}arg_", prefix = ACTION_PREFIX).ok();
                             last = last + off + "$".len();
                         } else {
-                            let span = grm.action_span(pidx).unwrap();
-                            let inner_span =
-                                Span::new(span.start() + last + off + "$".len(), span.end());
-                            let mut s = String::from("\n");
-                            s.push_str(&diag.file_location_msg("Error", Some(inner_span)));
-                            s.push('\n');
-                            s.push_str(&diag.underline_span_with_text(
-                                inner_span,
-                                "Unknown text following '$'".to_string(),
-                                '^',
-                            ));
-                            return Err(ErrorString(s).into());
+                            unreachable!("action variables checked during complete_and_validate");
                         }
                     }
                     None => {