diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index b860385c0..288c860da 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -15,6 +15,7 @@ use super::{ use crate::{ Span, header::{GrmtoolsSectionParser, HeaderError, HeaderErrorKind, HeaderValue}, + yacc::YaccOriginalActionKind, }; /// Any error from the Yacc parser returns an instance of this struct. @@ -55,7 +56,9 @@ impl ASTWithValidityInfo { let mut yp = YaccParser::new(yacc_kind, s); yp.parse().map_err(|e| errs.extend(e)).ok(); let mut ast = yp.build(); - ast.complete_and_validate().map_err(|e| errs.push(e)).ok(); + ast.complete_and_validate(Some(yacc_kind)) + .map_err(|e| errs.push(e)) + .ok(); ast }; ASTWithValidityInfo { @@ -122,7 +125,9 @@ impl FromStr for ASTWithValidityInfo { let mut yp = YaccParser::new(yacc_kind, src); yp.parse().map_err(|e| errs.extend(e)).ok(); let mut ast = yp.build(); - ast.complete_and_validate().map_err(|e| errs.push(e)).ok(); + ast.complete_and_validate(Some(yacc_kind)) + .map_err(|e| errs.push(e)) + .ok(); ast }; Ok(ASTWithValidityInfo { @@ -307,9 +312,19 @@ impl GrammarAST { /// 3) Every token reference references a declared token /// 4) If a production has a precedence token, then it references a declared token /// 5) Every token declared with %epp matches a known token - /// - /// If the validation succeeds, None is returned. - pub(crate) fn complete_and_validate(&mut self) -> Result<(), YaccGrammarError> { + /// 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, + ) -> Result<(), YaccGrammarError> { + let kind_requires_action_checks = matches!( + yacc_kind, + Some(YaccKind::Original(YaccOriginalActionKind::UserAction)) | Some(YaccKind::Grmtools) + ); + match self.start { None => { return Err(YaccGrammarError { @@ -327,8 +342,48 @@ impl GrammarAST { } } for rule in self.rules.values() { + if kind_requires_action_checks && rule.actiont.is_none() { + return Err(YaccGrammarError { + kind: YaccGrammarErrorKind::MissingActionType, + spans: vec![rule.name.1], + }); + } for &pidx in &rule.pidxs { let prod = &self.prods[pidx]; + 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 { @@ -510,7 +565,7 @@ mod test { #[test] fn test_empty_grammar() { let mut grm = GrammarAST::new(); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::NoStartRule, .. @@ -526,7 +581,7 @@ mod test { grm.start = Some(("A".to_string(), empty_span)); grm.add_rule(("B".to_string(), empty_span), None); grm.add_prod("B".to_string(), vec![], None, None, empty_span); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::InvalidStartRule(_), .. @@ -542,7 +597,7 @@ mod test { grm.start = Some(("A".to_string(), empty_span)); grm.add_rule(("A".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![], None, None, empty_span); - assert!(grm.complete_and_validate().is_ok()); + assert!(grm.complete_and_validate(None).is_ok()); } #[test] @@ -554,7 +609,7 @@ mod test { grm.add_rule(("B".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![rule("B")], None, None, empty_span); grm.add_prod("B".to_string(), vec![], None, None, empty_span); - assert!(grm.complete_and_validate().is_ok()); + assert!(grm.complete_and_validate(None).is_ok()); } #[test] @@ -564,7 +619,7 @@ mod test { grm.start = Some(("A".to_string(), empty_span)); grm.add_rule(("A".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![rule("B")], None, None, empty_span); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(_), .. @@ -581,7 +636,7 @@ mod test { grm.start = Some(("A".to_string(), empty_span)); grm.add_rule(("A".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![token("b")], None, None, empty_span); - assert!(grm.complete_and_validate().is_ok()); + assert!(grm.complete_and_validate(None).is_ok()); } #[test] @@ -594,7 +649,7 @@ mod test { grm.start = Some(("A".to_string(), empty_span)); grm.add_rule(("A".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![rule("b")], None, None, empty_span); - assert!(grm.complete_and_validate().is_err()); + assert!(grm.complete_and_validate(None).is_err()); } #[test] @@ -604,7 +659,7 @@ mod test { grm.start = Some(("A".to_string(), empty_span)); grm.add_rule(("A".to_string(), empty_span), None); grm.add_prod("A".to_string(), vec![token("b")], None, None, empty_span); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(_), .. @@ -626,7 +681,7 @@ mod test { None, Span::new(0, 2), ); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownRuleRef(_), .. @@ -644,7 +699,7 @@ mod test { grm.add_prod("A".to_string(), vec![], None, None, empty_span); grm.epp .insert("k".to_owned(), (empty_span, ("v".to_owned(), empty_span))); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownEPP(_), spans, @@ -677,7 +732,7 @@ mod test { None, empty_span, ); - assert!(grm.complete_and_validate().is_ok()); + assert!(grm.complete_and_validate(None).is_ok()); } #[test] @@ -693,7 +748,7 @@ mod test { None, empty_span, ); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::UnknownToken(_), .. @@ -701,7 +756,7 @@ mod test { _ => panic!("Validation error"), } grm.tokens.insert("b".to_string()); - match grm.complete_and_validate() { + match grm.complete_and_validate(None) { Err(YaccGrammarError { kind: YaccGrammarErrorKind::NoPrecForToken(_), .. @@ -737,7 +792,6 @@ mod test { #[test] fn token_rule_confusion_issue_557() { use super::*; - use crate::yacc::*; let ast_validity = ASTWithValidityInfo::new( YaccKind::Original(YaccOriginalActionKind::GenericParseTree), r#" @@ -785,7 +839,6 @@ mod test { #[test] fn test_token_directives() { use super::*; - use crate::yacc::*; // Testing that `%token a` after `%left "a"` still ends up in let ast_validity = ASTWithValidityInfo::new( @@ -815,7 +868,6 @@ mod test { #[test] fn clone_ast_changing_start_rule() { use super::*; - use crate::yacc::*; let y_src = r#" %start AStart %token A B C @@ -837,4 +889,101 @@ mod test { Some(&bstart_rule.name) ); } + + #[test] + fn test_missing_actiont() { + use super::*; + let ast_validity = ASTWithValidityInfo::new( + YaccKind::Original(YaccOriginalActionKind::UserAction), + r#" +%token a +%% +start: "a"; +"#, + ); + assert_eq!( + ast_validity.errors(), + vec![YaccGrammarError { + kind: YaccGrammarErrorKind::MissingActionType, + spans: vec![Span::new(13, 18)], + }] + ); + + let ast_validity = ASTWithValidityInfo::new( + YaccKind::Original(YaccOriginalActionKind::UserAction), + r#" +%actiontype () +%token a +%% +start: "a" { }; +"#, + ); + assert!(ast_validity.errors().is_empty()); + + let mut grm = GrammarAST::new(); + let empty_span = Span::new(0, 0); + let rule_span = Span::new(255, 255); + grm.start = Some(("A".to_string(), empty_span)); + grm.add_rule(("A".to_string(), rule_span), None); + grm.add_prod("A".to_string(), vec![], None, None, empty_span); + assert_eq!( + grm.complete_and_validate(Some(YaccKind::Grmtools)), + Err(YaccGrammarError { + kind: YaccGrammarErrorKind::MissingActionType, + spans: vec![rule_span], + }) + ); + } + + #[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)], + }] + ); + } } diff --git a/cfgrammar/src/lib/yacc/grammar.rs b/cfgrammar/src/lib/yacc/grammar.rs index 2bfae2efc..a03010ee7 100644 --- a/cfgrammar/src/lib/yacc/grammar.rs +++ b/cfgrammar/src/lib/yacc/grammar.rs @@ -1573,13 +1573,13 @@ mod test { "%grmtools{yacckind: YaccKind::Original(yaccoriginalactionkind::useraction)} %actiontype () %% - Start: ;", + Start: {};", "%grmtools{yacckind: Original(YACCOriginalActionKind::NoAction)} %% Start: ;", "%grmtools{yacckind: YaccKind::Grmtools} %% - Start -> () : ;", + Start -> () : {};", ]; for src in srcs { YaccGrammar::::from_str(src).unwrap(); @@ -1623,7 +1623,7 @@ mod test { yacckind: YaccKind::Grmtools, } %% - Start -> () : ; + Start -> () : {}; "#; YaccGrammar::::from_str(src).unwrap(); let src = r#" @@ -1631,7 +1631,7 @@ mod test { yacckind: YaccKind::Grmtools } %% - Start -> () : ; + Start -> () : {}; "#; YaccGrammar::::from_str(src).unwrap(); } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index c452d4ae9..6d76e0f8a 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -36,6 +36,8 @@ pub enum YaccGrammarErrorKind { IncompleteRule, IncompleteComment, IncompleteAction, + MissingActionCode, + MissingActionType, MissingColon, MissingRightArrow, MismatchedBrace, @@ -57,6 +59,7 @@ pub enum YaccGrammarErrorKind { InvalidString, NoStartRule, UnknownSymbol, + UnrecognisedActionVariable, InvalidStartRule(String), UnknownRuleRef(String), UnknownToken(String), @@ -97,6 +100,8 @@ impl fmt::Display for YaccGrammarErrorKind { YaccGrammarErrorKind::IncompleteRule => "Incomplete rule", YaccGrammarErrorKind::IncompleteComment => "Incomplete comment", YaccGrammarErrorKind::IncompleteAction => "Incomplete action", + YaccGrammarErrorKind::MissingActionCode => "Production is missing action code", + YaccGrammarErrorKind::MissingActionType => "Missing action type", YaccGrammarErrorKind::MissingColon => "Missing ':'", YaccGrammarErrorKind::MissingRightArrow => "Missing '->'", YaccGrammarErrorKind::MismatchedBrace => "Mismatched brace", @@ -107,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 => { + "Unrecognized action variable following '$'" + } YaccGrammarErrorKind::DuplicateAvoidInsertDeclaration => { "Duplicated %avoid_insert declaration" } @@ -245,6 +253,8 @@ impl Spanned for YaccGrammarError { | YaccGrammarErrorKind::IncompleteRule | YaccGrammarErrorKind::IncompleteComment | YaccGrammarErrorKind::IncompleteAction + | YaccGrammarErrorKind::MissingActionCode + | YaccGrammarErrorKind::MissingActionType | YaccGrammarErrorKind::MissingColon | YaccGrammarErrorKind::MissingRightArrow | YaccGrammarErrorKind::MismatchedBrace @@ -258,6 +268,7 @@ impl Spanned for YaccGrammarError { | YaccGrammarErrorKind::InvalidString | YaccGrammarErrorKind::NoStartRule | YaccGrammarErrorKind::UnknownSymbol + | YaccGrammarErrorKind::UnrecognisedActionVariable | YaccGrammarErrorKind::InvalidStartRule(_) | YaccGrammarErrorKind::UnknownRuleRef(_) | YaccGrammarErrorKind::UnknownToken(_) @@ -286,7 +297,7 @@ pub(crate) struct YaccParser<'a> { global_actiontype: Option<(String, Span)>, } -static RE_NAME: LazyLock = +pub(crate) static RE_NAME: LazyLock = LazyLock::new(|| Regex::new(r"^[a-zA-Z_.][a-zA-Z0-9_.]*").unwrap()); static RE_TOKEN: LazyLock = LazyLock::new(|| Regex::new("^(?:(\".+?\")|('.+?')|([a-zA-Z_][a-zA-Z_0-9]*))").unwrap()); @@ -842,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)) } } @@ -2287,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()))) diff --git a/lrpar/src/lib/ctbuilder.rs b/lrpar/src/lib/ctbuilder.rs index 830558116..4ef940758 100644 --- a/lrpar/src/lib/ctbuilder.rs +++ b/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> { 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, - diag: &SpannedDiagnosticFormatter, - ) -> Result> { + fn gen_user_actions(&self, grm: &YaccGrammar) -> Result> { let programs = grm .programs() .as_ref() @@ -1649,20 +1643,10 @@ where for i in 0..grm.prod(pidx).len() { let argt = match grm.prod(pidx)[i] { Symbol::Rule(ref_ridx) => { - if let Some(action_type) = grm.actiontype(ref_ridx).as_ref() { - str::parse::(action_type)? - } else { - let mut s = String::from("\n"); - let rule_span = grm.rule_name_span(ref_ridx); - s.push_str(&diag.file_location_msg("Error", Some(rule_span))); - s.push('\n'); - s.push_str(&diag.underline_span_with_text( - rule_span, - "Rule missing action type".to_string(), - '^', - )); - return Err(ErrorString(s).into()); - } + let action_type = grm.actiontype(ref_ridx) + .as_ref() + .expect("actiontype should have been checked during complete_and_validate for this YaccKind"); + str::parse::(action_type)? } Symbol::Token(_) => { let lexemet = @@ -1700,18 +1684,7 @@ where // Iterate over all $-arguments and replace them with their respective // element from the argument vector (e.g. $1 is replaced by args[0]). - let pre_action = grm.action(pidx).as_ref().ok_or_else(|| { - let mut s = String::from("\n"); - let span = grm.prod_span(pidx); - s.push_str(&diag.file_location_msg("Error", Some(span))); - s.push('\n'); - s.push_str(&diag.underline_span_with_text( - span, - "Production is missing action code".to_string(), - '^', - )); - ErrorString(s) - })?; + let pre_action = grm.action(pidx).as_ref().expect("action code should have been checked during complete_and_validate for this YaccKind"); let mut last = 0; let mut outs = String::new(); loop { @@ -1735,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 => {