diff --git a/cfgrammar/src/lib/header.rs b/cfgrammar/src/lib/header.rs index 645277f43..05a9ab19f 100644 --- a/cfgrammar/src/lib/header.rs +++ b/cfgrammar/src/lib/header.rs @@ -48,7 +48,7 @@ impl Spanned for HeaderError { // This is essentially a tuple that needs a newtype so we can implement `From` for it. // Thus we aren't worried about it being `pub`. -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone)] #[doc(hidden)] pub struct HeaderValue(pub T, pub Value); diff --git a/cfgrammar/src/lib/markmap.rs b/cfgrammar/src/lib/markmap.rs index b2f910290..9e2a3d245 100644 --- a/cfgrammar/src/lib/markmap.rs +++ b/cfgrammar/src/lib/markmap.rs @@ -21,7 +21,7 @@ use std::fmt; /// /// Merge behaviors configure how the merge operator handles cases where both `MarkMaps` being merged /// contain a particular key. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone)] #[doc(hidden)] pub struct MarkMap { default_merge_behavior: MergeBehavior, @@ -484,12 +484,15 @@ impl MarkMap { } /// Returns a `Vec` containing all the keys that are not marked as used. - pub fn unused(&self) -> Vec { + pub fn unused(&self) -> Vec<(K, V)> + where + V: Clone, + { let mut ret = Vec::new(); for (k, mark, v) in &self.contents { let used_mark = Mark::Used.repr(); if v.is_some() && mark & used_mark == 0 { - ret.push(k.to_owned()) + ret.push((k.to_owned(), v.as_ref().unwrap().clone())) } } ret @@ -711,7 +714,7 @@ mod test { assert!(mm.insert("a", "test").is_none()); mm.mark_used(&"a"); assert_eq!(mm.get_mark(&"a"), Some(Mark::Used.repr())); - let empty: &[&String] = &[]; + let empty: &[(&str, &str)] = &[]; assert_eq!(mm.unused().as_slice(), empty); } @@ -722,7 +725,7 @@ mod test { assert!(mm.insert("b", "unused").is_none()); assert_eq!(mm.get_mark(&"a"), Some(Mark::Used.repr())); assert_eq!(mm.get_mark(&"b"), Some(0)); - assert_eq!(mm.unused().as_slice(), &["b"]); + assert_eq!(mm.unused().as_slice(), &[("b", "unused")]); } } diff --git a/cfgrammar/src/lib/yacc/ast.rs b/cfgrammar/src/lib/yacc/ast.rs index fb1ef5cb2..1526958b0 100644 --- a/cfgrammar/src/lib/yacc/ast.rs +++ b/cfgrammar/src/lib/yacc/ast.rs @@ -14,7 +14,10 @@ use super::{ use crate::{ Span, - header::{GrmtoolsSectionParser, HeaderError, HeaderErrorKind, HeaderValue}, + header::{ + GrmtoolsSectionParser, Header, HeaderError, HeaderErrorKind, HeaderValue, RE_CRATE_DOT, + Value, + }, yacc::YaccOriginalActionKind, }; @@ -39,6 +42,7 @@ impl fmt::Display for ASTModificationError { pub struct ASTWithValidityInfo { yacc_kind: YaccKind, ast: GrammarAST, + grmtools_section: Header, errs: Vec, } @@ -52,18 +56,19 @@ impl ASTWithValidityInfo { /// already extracted the `YaccKind` if any. pub fn new(yacc_kind: YaccKind, s: &str) -> Self { let mut errs = Vec::new(); - let ast = { + let (ast, grmtools_section) = { let mut yp = YaccParser::new(yacc_kind, s); yp.parse().map_err(|e| errs.extend(e)).ok(); - let mut ast = yp.build(); + let (mut ast, grmtools_section) = yp.build(); ast.complete_and_validate(Some(yacc_kind)) .map_err(|e| errs.push(e)) .ok(); - ast + (ast, grmtools_section) }; ASTWithValidityInfo { ast, errs, + grmtools_section, yacc_kind, } } @@ -108,6 +113,48 @@ impl ASTWithValidityInfo { }) } } + + /// Performs a lookup in the grmtools section for an entry with the key `crate_name.key_name` and returns it. + /// If the entry is found it marks the key as `used`, for the purposes of `unused_header_keys_for_crate`. + pub fn header_value_for_crate( + &mut self, + crate_name: &str, + key_name: &str, + ) -> Option<(Span, &Value)> { + let key = format!("{crate_name}.{key_name}"); + self.grmtools_section.mark_used(&key); + if let Some(HeaderValue(span, value)) = self.grmtools_section.get(&key) { + Some((*span, value)) + } else { + None + } + } + + /// Returns all key names given in the header specified by a `%grmtools` directive with the + /// `crate_name.` prefix for the given crate. If the `crate_name` is None returns any unused + /// keys with no crate prefix specified. + pub fn unused_header_keys_for_crate(&self, crate_name: Option<&str>) -> Vec<(String, Span)> { + self.grmtools_section + .unused() + .iter() + .filter_map(|(key_name, HeaderValue(key_span, _))| { + if let Some(crate_name) = crate_name { + let crate_prefix = format!("{crate_name}."); + if key_name.starts_with(&crate_prefix) { + Some((key_name.clone(), *key_span)) + } else { + None + } + } else { + if !RE_CRATE_DOT.is_match(key_name) { + Some((key_name.clone(), *key_span)) + } else { + None + } + } + }) + .collect::>() + } } impl FromStr for ASTWithValidityInfo { @@ -120,19 +167,20 @@ impl FromStr for ASTWithValidityInfo { .map_err(|mut errs| errs.drain(..).map(|e| e.into()).collect::>())?; if let Some(HeaderValue(_, yk_val)) = header.get("cfgrammar.yacckind") { let yacc_kind = YaccKind::try_from(yk_val).map_err(|e| vec![e.into()])?; - let ast = { + let (ast, grmtools_section) = { // We don't want to strip off the header so that span's will be correct. let mut yp = YaccParser::new(yacc_kind, src); yp.parse().map_err(|e| errs.extend(e)).ok(); - let mut ast = yp.build(); + let (mut ast, grmtools_section) = yp.build(); ast.complete_and_validate(Some(yacc_kind)) .map_err(|e| errs.push(e)) .ok(); - ast + (ast, grmtools_section) }; Ok(ASTWithValidityInfo { ast, errs, + grmtools_section, yacc_kind, }) } else { @@ -984,4 +1032,205 @@ start -> () : "a" {$;;;; }; }] ); } + + #[test] + fn test_grmtools_section_values() { + use super::*; + use crate::header::Value; + let src = r#" +%grmtools { + yacckind: Grmtools, + lrpar.recoverer: CPCTPlus, + test.Flag, + !test.Negative, + test.string: "Foo", + test.vec: ["Aaaa", "Bbbb"], + test.num: 1234, + test.unused: 5678 +} +%token a +%% +start -> () : "a" { () }; +"#; + let mut ast_validity = ASTWithValidityInfo::from_str(src).unwrap(); + for (key, (expected_span, expected_value)) in [ + ( + "Flag", + ( + src.find_span("test.Flag"), + Value::Bool(true, src.find_span("test.Flag")), + ), + ), + ( + "Negative", + ( + src.find_span("test.Negative"), + Value::Bool(false, src.find_span("!test.Negative")), + ), + ), + ( + "string", + ( + src.find_span("test.string"), + Value::String("Foo".to_string(), src.find_span("Foo")), + ), + ), + ( + "vec", + ( + src.find_span("test.vec"), + Value::Array( + vec![ + Value::String("Aaaa".to_string(), src.find_span("Aaaa")), + Value::String("Bbbb".to_string(), src.find_span("Bbbb")), + ], + src.find_span("[\"Aaaa\", \"Bbbb\"]"), + ), + ), + ), + ( + "num", + ( + src.find_span("test.num"), + Value::Num(1234, src.find_span("1234")), + ), + ), + ] { + let value = ast_validity.header_value_for_crate("test", key); + assert_eq!(value, Some((expected_span, &expected_value))); + } + assert_eq!( + ast_validity.unused_header_keys_for_crate(Some("test")), + vec![("test.unused".to_string(), src.find_span("test.unused"))] + ); + assert_eq!( + ast_validity.header_value_for_crate("cfgrammar", "yacckind"), + Some(( + src.find_span("yacckind"), + &Value::Namespaced("Grmtools".to_string(), src.find_span("Grmtools")) + )) + ); + + assert!( + ast_validity + .unused_header_keys_for_crate(Some("cfgrammar")) + .is_empty() + ); + + assert_eq!( + ast_validity.header_value_for_crate("lrpar", "recoverer"), + Some(( + src.find_span("lrpar.recoverer"), + &Value::Namespaced("CPCTPlus".to_string(), src.find_span("CPCTPlus")) + )) + ); + + assert!( + ast_validity + .unused_header_keys_for_crate(Some("lrpar")) + .is_empty() + ); + } + + #[test] + fn test_grmtools_section_values2() { + use super::*; + let src = r#" +%grmtools { + yacckind: Original(YaccOriginalActionKind::UserAction), +} +%token a +%actiontype () +%% +start: "a" { () }; +"#; + let mut ast_validity = ASTWithValidityInfo::from_str(src).unwrap(); + assert_eq!( + ast_validity.header_value_for_crate("cfgrammar", "yacckind"), + Some(( + src.find_span("yacckind"), + &Value::Namespaced( + "Original(YaccOriginalActionKind::UserAction)".to_string(), + src.find_span("Original(YaccOriginalActionKind::UserAction)"), + ), + )) + ); + assert!( + ast_validity + .unused_header_keys_for_crate(Some("cfgrammar")) + .is_empty() + ); + } + + #[test] + fn test_grmtools_section_values3() { + use super::*; + let src = r#" +%grmtools { + yacckind: YaccKind::Original(YaccOriginalActionKind::UserAction), +} +%token a +%actiontype () +%% +start: "a" { () }; +"#; + let mut ast_validity = ASTWithValidityInfo::from_str(src).unwrap(); + assert_eq!( + ast_validity.header_value_for_crate("cfgrammar", "yacckind"), + Some(( + src.find_span("yacckind"), + &Value::Namespaced( + "YaccKind::Original(YaccOriginalActionKind::UserAction)".to_string(), + src.find_span("YaccKind::Original(YaccOriginalActionKind::UserAction)"), + ), + )) + ); + assert!( + ast_validity + .unused_header_keys_for_crate(Some("cfgrammar")) + .is_empty() + ); + } + + #[test] + fn test_grmtools_section_values4() { + use super::*; + let src = r#" +%grmtools { + yacckind: YaccKind::Original(UserAction), +} +%token a +%actiontype () +%% +start: "a" { () }; +"#; + let mut ast_validity = ASTWithValidityInfo::from_str(src).unwrap(); + assert_eq!( + ast_validity.header_value_for_crate("cfgrammar", "yacckind"), + Some(( + src.find_span("yacckind"), + &Value::Namespaced( + "YaccKind::Original(UserAction)".to_string(), + src.find_span("YaccKind::Original(UserAction)"), + ), + )) + ); + assert!( + ast_validity + .unused_header_keys_for_crate(Some("cfgrammar")) + .is_empty() + ); + } + + trait FindSpan { + fn find_span(&self, s: &str) -> Span; + } + + impl FindSpan for &'_ str { + #[track_caller] + fn find_span(&self, s: &str) -> Span { + let start_pos = self.find(s).unwrap(); + Span::new(start_pos, start_pos + s.len()) + } + } } diff --git a/cfgrammar/src/lib/yacc/parser.rs b/cfgrammar/src/lib/yacc/parser.rs index 4ee8eec99..39e8f60c9 100644 --- a/cfgrammar/src/lib/yacc/parser.rs +++ b/cfgrammar/src/lib/yacc/parser.rs @@ -16,7 +16,7 @@ use wincode::{SchemaRead, SchemaWrite}; use crate::{ Span, Spanned, - header::{GrmtoolsSectionParser, HeaderErrorKind}, + header::{CRATE_KEY_MAP, GrmtoolsSectionParser, Header, HeaderErrorKind}, }; pub type YaccGrammarResult = Result>; @@ -294,6 +294,7 @@ pub(crate) struct YaccParser<'a> { src: &'a str, num_newlines: usize, ast: GrammarAST, + header: Option>, global_actiontype: Option<(String, Span)>, } @@ -331,15 +332,17 @@ impl YaccParser<'_> { src, num_newlines: 0, ast: GrammarAST::new(), + header: None, global_actiontype: None, } } pub(crate) fn parse(&mut self) -> YaccGrammarResult { let mut errs = Vec::new(); - let (_, pos) = GrmtoolsSectionParser::new(self.src, false) + let (header, pos) = GrmtoolsSectionParser::new(self.src, false) .parse() .map_err(|mut errs| errs.drain(..).map(|e| e.into()).collect::>())?; + self.header = Some(header); // We pass around an index into the *bytes* of self.src. We guarantee that at all times // this points to the beginning of a UTF-8 character (since multibyte characters exist, not // every byte within the string is also a valid character). @@ -371,8 +374,19 @@ impl YaccParser<'_> { } } - pub(crate) fn build(self) -> GrammarAST { - self.ast + pub(crate) fn build(self) -> (GrammarAST, Header) { + let mut header = self.header.expect("set by parse()"); + // Preemptively mark the keys for lrpar and cfgrammar as used in the header. + // If a downstream crate checks the keys in the ast. The lrpar crate works on a + // local instance which merges the keys from ast with keys from the `CTBuilder`. + // + // It is difficult to do later due to shared references. + for (key_name, crate_name) in CRATE_KEY_MAP.iter() { + if ["cfgrammar", "lrpar"].contains(crate_name) { + header.mark_used(&format!("{crate_name}.{key_name}")); + } + } + (self.ast, header) } fn parse_declarations( @@ -1083,7 +1097,8 @@ mod test { fn parse(yacc_kind: YaccKind, s: &str) -> Result> { let mut yp = YaccParser::new(yacc_kind, s); yp.parse()?; - Ok(yp.build()) + let (ast, _) = yp.build(); + Ok(ast) } fn rule(n: &str) -> Symbol { diff --git a/lrlex/src/lib/ctbuilder.rs b/lrlex/src/lib/ctbuilder.rs index 7b6983e46..6a1eea4bd 100644 --- a/lrlex/src/lib/ctbuilder.rs +++ b/lrlex/src/lib/ctbuilder.rs @@ -520,7 +520,12 @@ where None }; - let unused_header_values = build_env.header().unused(); + let unused_header_values = build_env + .header() + .unused() + .iter() + .map(|(s, _)| s.to_string()) + .collect::>(); if !unused_header_values.is_empty() { return Err( format!("Unused header values: {}", unused_header_values.join(", ")).into(), diff --git a/lrlex/src/main.rs b/lrlex/src/main.rs index 30ebbb326..bb87599f2 100644 --- a/lrlex/src/main.rs +++ b/lrlex/src/main.rs @@ -123,7 +123,11 @@ fn main() -> Result<(), Box> { } }; { - let unused_header_values = header.unused(); + let unused_header_values = header + .unused() + .iter() + .map(|(s, _)| s.to_string()) + .collect::>(); if !unused_header_values.is_empty() { Err(ErrorString(format!( "Unused header values: {}", diff --git a/lrpar/src/lib/codegen.rs b/lrpar/src/lib/codegen.rs index ecb342641..1ad7ed751 100644 --- a/lrpar/src/lib/codegen.rs +++ b/lrpar/src/lib/codegen.rs @@ -13,7 +13,7 @@ use crate::{ use cfgrammar::{ Location, RIdx, Span, Symbol, - header::{GrmtoolsSectionParser, Header, HeaderError, HeaderValue}, + header::{GrmtoolsSectionParser, Header, HeaderError, HeaderValue, RE_CRATE_DOT}, markmap::MergeError, yacc::{ YaccGrammar, YaccGrammarError, YaccKind, YaccOriginalActionKind, ast::ASTWithValidityInfo, @@ -32,6 +32,7 @@ const ACTIONS_KIND: &str = "__GtActionsKind"; const ACTIONS_KIND_PREFIX: &str = "Ak"; const ACTIONS_KIND_HIDDEN: &str = "__GtActionsKindHidden"; +#[derive(Debug)] #[non_exhaustive] pub(crate) enum ParserSrcEnvError { GrmtoolsSectionParseError(Vec>), @@ -41,6 +42,7 @@ pub(crate) enum ParserSrcEnvError { MissingModName, } +#[derive(Debug)] #[non_exhaustive] pub(crate) enum ParserBuildEnvError where @@ -53,6 +55,7 @@ where GrmtoolsSectionMissingRequiredKeys(Vec), } +#[derive(Debug)] #[non_exhaustive] pub(crate) enum CodegenError { ProcMacro2Error(proc_macro2::LexError), @@ -456,8 +459,26 @@ where self.ast_with_validity_info.yacc_kind() } - pub(crate) fn check_unused_header_keys(&self) -> Result<(), ParserBuildEnvError> { - let unused_keys = self.header.unused(); + /// Returns an error if any unused keys specified in a `%grmtools` directive that begin with a + /// `crate_name.` prefix for `crate_name` value are found. If the `crate_name` is None returns + /// an error if any unused keys with no crate prefix specified are found. + pub(crate) fn check_unused_header_keys_for_crate( + &self, + crate_name: Option<&str>, + ) -> Result<(), ParserBuildEnvError> { + let unused_keys = self + .header + .unused() + .iter() + .filter(|(s, _)| { + if let Some(crate_name) = crate_name { + s.starts_with(&format!("{crate_name}.")) + } else { + !RE_CRATE_DOT.is_match(s) + } + }) + .map(|(s, _)| s.to_string()) + .collect::>(); if !unused_keys.is_empty() { return Err(ParserBuildEnvError::GrmtoolsSectionUnusedKeys(unused_keys)); } @@ -1294,3 +1315,177 @@ pub(crate) fn make_generics(parse_generics: Option<&str>) -> Result)) } } + +#[cfg(test)] +mod test { + use crate::test_utils::TestLexerTypes; + use cfgrammar::{header::Header, span::Location}; + + use super::*; + #[test] + fn test_unused_crate_header_entry() { + let src = r#" + %grmtools{ + yacckind: Grmtools, + test.foo: "test crate value", + } + %% + start -> () : "A" { () }; + "#; + let empty_header = Header::::new(); + let src_env = ParserSrcEnv::::new_with_header(src, None, empty_header); + let build_env = src_env + .build_env(ParserBuildEnvArgs::new().mod_name(Some("test_module"))) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("cfgrammar")) + .is_empty() + ); + build_env + .check_unused_header_keys_for_crate(Some("cfgrammar")) + .unwrap(); + build_env + .check_unused_header_keys_for_crate(Some("lrpar")) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("lrpar")) + .is_empty() + ); + build_env + .check_unused_header_keys_for_crate(Some("lrlex")) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("lrpar")) + .is_empty() + ); + build_env.check_unused_header_keys_for_crate(None).unwrap(); + let codegen = build_env.code_generator("timestamp").unwrap(); + let out = codegen.generate(&build_env).unwrap(); + assert!(!out.is_empty()); + } + + #[test] + fn test_unused_header_entry() { + let src = r#" + %grmtools{ + yacckind: Grmtools, + testfoo: "values which do not specify a crate origin should show up as unused", + } + %% + start -> () : "A" { () }; + "#; + let empty_header = Header::::new(); + let src_env = ParserSrcEnv::::new_with_header(src, None, empty_header); + let build_env = src_env + .build_env(ParserBuildEnvArgs::new().mod_name(Some("test_module"))) + .unwrap(); + build_env + .check_unused_header_keys_for_crate(Some("cfgrammar")) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("cfgrammar")) + .is_empty() + ); + build_env + .check_unused_header_keys_for_crate(Some("lrpar")) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("lrpar")) + .is_empty() + ); + build_env + .check_unused_header_keys_for_crate(Some("lrlex")) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("lrlex")) + .is_empty() + ); + match build_env.check_unused_header_keys_for_crate(None) { + Err(ParserBuildEnvError::GrmtoolsSectionUnusedKeys(keys)) + if keys == vec!["testfoo".to_string()] => {} + _ => panic!("Unexpected return value for unused header keys check"), + } + let codegen = build_env.code_generator("timestamp").unwrap(); + let out = codegen.generate(&build_env).unwrap(); + assert!(!out.is_empty()); + } + + #[test] + fn test_unused_grmtools_header_entry() { + let src = r#" + %grmtools{ + yacckind: Grmtools, + cfgrammar.unknown: "should be unused", + lrpar.unknown: "should be unused", + } + %% + start -> () : "A" { () }; + "#; + let empty_header = Header::::new(); + let src_env = ParserSrcEnv::::new_with_header(src, None, empty_header); + let build_env = src_env + .build_env(ParserBuildEnvArgs::new().mod_name(Some("test_module"))) + .unwrap(); + assert!( + build_env + .check_unused_header_keys_for_crate(Some("cfgrammar")) + .is_err() + ); + assert_eq!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("cfgrammar")), + vec![( + "cfgrammar.unknown".to_string(), + src.find_span("cfgrammar.unknown") + )] + ); + assert!( + build_env + .check_unused_header_keys_for_crate(Some("lrpar")) + .is_err() + ); + assert_eq!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("lrpar")), + vec![("lrpar.unknown".to_string(), src.find_span("lrpar.unknown"))] + ); + build_env + .check_unused_header_keys_for_crate(Some("lrlex")) + .unwrap(); + assert!( + build_env + .ast_with_validity_info() + .unused_header_keys_for_crate(Some("lrlex")) + .is_empty() + ); + let codegen = build_env.code_generator("timestamp").unwrap(); + let out = codegen.generate(&build_env).unwrap(); + assert!(!out.is_empty()); + } + + trait FindSpan { + fn find_span(&self, s: &str) -> Span; + } + + impl FindSpan for &'_ str { + #[track_caller] + fn find_span(&self, s: &str) -> Span { + let start_pos = self.find(s).unwrap(); + Span::new(start_pos, start_pos + s.len()) + } + } +} diff --git a/lrpar/src/lib/ctbuilder.rs b/lrpar/src/lib/ctbuilder.rs index 0df742103..1d7a1feaf 100644 --- a/lrpar/src/lib/ctbuilder.rs +++ b/lrpar/src/lib/ctbuilder.rs @@ -747,10 +747,21 @@ where inspector_rt(build_env.header_mut(), rt, &rule_ids, grmp)? } + // Catch any typos in key names for cfgrammar or lrpar build_env - .check_unused_header_keys() + .check_unused_header_keys_for_crate(Some("cfgrammar")) + .map_err(|e| ErrorString(e.to_string()))?; + build_env + .check_unused_header_keys_for_crate(Some("lrpar")) + .map_err(|e| ErrorString(e.to_string()))?; + // Catch any stray lrlex keys that accidentally make their way into the parser src. + build_env + .check_unused_header_keys_for_crate(Some("lrlex")) + .map_err(|e| ErrorString(e.to_string()))?; + // Catch any stray keys without a crate prefix. + build_env + .check_unused_header_keys_for_crate(None) .map_err(|e| ErrorString(e.to_string()))?; - self.output_file( &code_gen, outp,