Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 188 additions & 68 deletions cfgrammar/src/lib/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,21 @@ use crate::{
},
};
use regex::{Regex, RegexBuilder};
use std::{error::Error, fmt, sync::LazyLock};
use std::{
collections::{HashMap, HashSet},
error::Error,
fmt,
sync::LazyLock,
};

#[derive(Debug)]
#[doc(hidden)]
pub struct FileHeaders {
pub grmtools: Header<Span>,
pub grmtools_span: Span,
pub user_section: HashMap<String, (Span, UserSectionValue)>,
pub user_section_span: Span,
}

/// An error regarding the `%grmtools` header section.
///
Expand Down Expand Up @@ -69,6 +83,15 @@ pub enum HeaderErrorKind {
DuplicateEntry,
InvalidEntry(&'static str),
ConversionError(&'static str, &'static str),
InvalidUserSectionValueType,
}

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub enum UserSectionValue {
String(String, Span),
Num(u64, Span),
Bool(bool, Span),
Array(Vec<UserSectionValue>, Span),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if we wanted to go super simple e.g. HashMap<String, String>, or a serde_json::Value inspired enum like this, I went with the latter for now because it was easy enough to do, and allows us to add types later if we need?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the right way to go.

}

impl fmt::Display for HeaderErrorKind {
Expand All @@ -82,6 +105,9 @@ impl fmt::Display for HeaderErrorKind {
}
HeaderErrorKind::InvalidEntry(s) => &format!("Invalid entry: '{}'", s),
HeaderErrorKind::DuplicateEntry => "Duplicate Entry",
HeaderErrorKind::InvalidUserSectionValueType => {
"Invalid value type for the %user section"
}
HeaderErrorKind::ConversionError(t, err_str) => {
&format!("Converting header value to type '{}': {}", t, err_str)
}
Expand Down Expand Up @@ -239,16 +265,14 @@ impl<T> Namespaced<T> {
static RE_LEADING_WS: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"^[\p{Pattern_White_Space}]*").unwrap());
static RE_NAME: LazyLock<Regex> = LazyLock::new(|| {
RegexBuilder::new(r"^[A-Z][A-Z_]*")
RegexBuilder::new(r"^[A-Z][A-Z_\.]*")
.case_insensitive(true)
.build()
.unwrap()
});
static RE_DIGITS: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^[0-9]+").unwrap());
static RE_STRING: LazyLock<Regex> = LazyLock::new(|| Regex::new(r#"^\"(\\.|[^"\\])*\""#).unwrap());

const MAGIC: &str = "%grmtools";

fn add_duplicate_occurrence<T: Eq + PartialEq + Clone>(
errs: &mut Vec<HeaderError<T>>,
kind: HeaderErrorKind,
Expand Down Expand Up @@ -418,83 +442,179 @@ impl<'input> GrmtoolsSectionParser<'input> {
Self { src, required }
}

#[allow(clippy::type_complexity)]
pub fn parse(&'_ self) -> Result<(Header<Span>, usize), Vec<HeaderError<Span>>> {
pub fn parse(&'_ self) -> Result<(FileHeaders, usize), Vec<HeaderError<Span>>> {
let mut sections_lookup = HashSet::from_iter(["%grmtools", "%user"]);

@ratmice ratmice Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I was thinking about the dotted keys vs named sections
Technically it seems like we could have both a named section and a declaration of the same name,
giving users the ability to specify the name of their section (via sections_lookup here rather than hard coding %user

We could have both:

%foo {
  // The user specified a foo section
}

%foo later-grmtools-added-a-foo-declaration

The commitment would need to make is that we agree not to add sections other than %grmtools.
There I suppose is an obscure conflict in that it is assuming that the %foo declaration is not starting with a {, at the very beginning of the file at least.

This would reduce some duplication of the tool name

%user {
    nimbleparse_lsp.input_file_extension: ".foo"
    nimbleparse_lsp.grammar_path: "./foo.y"
}

could turn into:

%nimbleparse_lsp {
   input_file_extension: "foo",
   grammar_path: "./foo.y",
}

Actually it's probably a bit more complex than just adding it here, we'd need to ignore it for cases like nimbleparse which aren't expecting/passing any additional section_lookup entries. The approach taken in this patch, definitely eliminates that complexity.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raises another possibility (off the top of my head: not properly thought through etc etc!). Perhaps we only need a %grmtools section and we namespace all directives in there (accepting, for backwards compatibility reasons, that not everything will have a grmtools. prefix at first).

@ratmice ratmice Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is actually still a bit of a reason to have separate directives in that we can check that each declaration in the %grmtools section is used easily, while it's more difficult for the %user section (because we aren't documenting/exposing the markmap that allows marking them as used)

Well I guess check that the grmtools. prefixed items are used only, and filter out other prefixes in the unused check, we'd have to split the . during the filter (only filtering out items with a non-grmtools non-empty prefix).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other thing I would say is that as it is currently written, with the hashmap like API, and the string grmtools.yacc_kind being a different key than yacckind, it's definitely going to double a lot of checks in the codebase while we still have legacy entries without the namespace. It seems like it might be quite a bit.

I don't know if trying to change the way that lookup is handled to use a default prefix/search defaulting to using a grmtools namespace would be viable. But that might avoid adding a second lookup including the namespace.

let mut cur_pos = 0;
let mut headers: HashMap<&'static str, (Header<Span>, Span)> = HashMap::new();
// This will error when a duplicate section is encountered, but only in a round about fashion.
// Because we remove the section from `sections_lookup`, the next go around it'll be unrecognized.
// As such, it's likely to be an `unrecognized declaration` instead of a nicer error.
while let (Some((header, section)), pos) = self.parse_sections(cur_pos, &sections_lookup)? {
sections_lookup.remove(section);
headers.insert(section, (header, Span::new(cur_pos, pos)));
cur_pos = pos;
}

// When a default empty header is produced, we currently give it an empty span at (0, 0) for convenience
let (grmtools, grmtools_span) = headers
.remove("%grmtools")
.unwrap_or_else(|| (Header::new(), Span::new(0, 0)));
let (user_header, user_section_span) = headers
.remove("%user")
.unwrap_or_else(|| (Header::new(), Span::new(0, 0)));
let mut user_section = HashMap::new();
let mut errs = Vec::new();
if let Some(mut i) = self.lookahead_is(MAGIC, self.parse_ws(0)) {
let mut ret = Header::new();
i = self.parse_ws(i);
let section_start_pos = i;
if let Some(j) = self.lookahead_is("{", i) {
i = self.parse_ws(j);
while self.lookahead_is("}", i).is_none() && i < self.src.len() {
let (key, key_loc, val, j) = match self.parse_key_value(i) {
Ok((key, key_loc, val, pos)) => (key, key_loc, val, pos),
Err(e) => {
errs.push(e);
return Err(errs);
for (key, HeaderValue(key_span, value)) in user_header.into_iter() {
match value {
Value::Flag(flag, val_span) => {
user_section.insert(
key.clone(),
(*key_span, UserSectionValue::Bool(*flag, *val_span)),
);
}
Value::Setting(setting) => match setting {
Setting::Array(v, start_span, _) => {
let mut out = Vec::with_capacity(v.capacity());
for val in v {
match val {
Setting::String(s, val_span) => {
out.push(UserSectionValue::String(s.clone(), *val_span))
}
Setting::Num(n, val_span) => {
out.push(UserSectionValue::Num(*n, *val_span))
}
_ => {
errs.push(HeaderError {
kind: HeaderErrorKind::InvalidUserSectionValueType,
locations: vec![*key_span],
});
}
}
}
};
match ret.entry(key) {
Entry::Occupied(orig) => {
let HeaderValue(orig_loc, _): &HeaderValue<Span> = orig.get();
add_duplicate_occurrence(
&mut errs,
HeaderErrorKind::DuplicateEntry,
*orig_loc,
key_loc,
)
user_section.insert(
key.clone(),
(*key_span, UserSectionValue::Array(out, *start_span)),
);
}
Setting::String(s, val_span) => {
user_section.insert(
key.clone(),
(*key_span, UserSectionValue::String(s.clone(), *val_span)),
);
}
Setting::Num(n, val_span) => {
user_section.insert(
key.clone(),
(*key_span, UserSectionValue::Num(*n, *val_span)),
);
}

_ => {
errs.push(HeaderError {
kind: HeaderErrorKind::InvalidUserSectionValueType,
locations: vec![*key_span],
});
}
},
}
}
if !errs.is_empty() {
return Err(errs);
}
Ok((
FileHeaders {
grmtools,
grmtools_span,
user_section,
user_section_span,
},
cur_pos,
))
}

#[allow(clippy::type_complexity)]
pub fn parse_sections(
&'_ self,
start_pos: usize,
sections: &HashSet<&'static str>,
) -> Result<(Option<(Header<Span>, &'static str)>, usize), Vec<HeaderError<Span>>> {
for magic_string in sections {
let grmtools_required = self.required && magic_string == &"%grmtools";
let mut errs = Vec::new();
if let Some(mut i) = self.lookahead_is(magic_string, self.parse_ws(start_pos)) {
let mut ret = Header::new();
i = self.parse_ws(i);
let section_start_pos = i;
if let Some(j) = self.lookahead_is("{", i) {
i = self.parse_ws(j);
while self.lookahead_is("}", i).is_none() && i < self.src.len() {
let (key, key_loc, val, j) = match self.parse_key_value(i) {
Ok((key, key_loc, val, pos)) => (key, key_loc, val, pos),
Err(e) => {
errs.push(e);
return Err(errs);
}
};
match ret.entry(key) {
Entry::Occupied(orig) => {
let HeaderValue(orig_loc, _): &HeaderValue<Span> = orig.get();
add_duplicate_occurrence(
&mut errs,
HeaderErrorKind::DuplicateEntry,
*orig_loc,
key_loc,
)
}
Entry::Vacant(entry) => {
entry.insert(HeaderValue(key_loc, val));
}
}
Entry::Vacant(entry) => {
entry.insert(HeaderValue(key_loc, val));
if let Some(j) = self.lookahead_is(",", j) {
i = self.parse_ws(j);
continue;
} else {
i = self.parse_ws(j);
break;
}
}
if let Some(j) = self.lookahead_is(",", j) {
i = self.parse_ws(j);
continue;
} else {
i = self.parse_ws(j);
break;
}
}
if let Some(j) = self.lookahead_is("*", i) {
errs.push(HeaderError {
kind: HeaderErrorKind::UnexpectedToken(
'*',
"perhaps this is a glob, in which case it requires string quoting.",
),
locations: vec![Span::new(i, j)],
});
Err(errs)
} else if let Some(i) = self.lookahead_is("}", i) {
if errs.is_empty() {
Ok((ret, i))
if let Some(j) = self.lookahead_is("*", i) {
errs.push(HeaderError {
kind: HeaderErrorKind::UnexpectedToken(
'*',
"perhaps this is a glob, in which case it requires string quoting.",
),
locations: vec![Span::new(i, j)],
});
return Err(errs);
} else if let Some(i) = self.lookahead_is("}", i) {
if errs.is_empty() {
return Ok((Some((ret, magic_string)), i));
} else {
return Err(errs);
}
} else {
Err(errs)
errs.push(HeaderError {
kind: HeaderErrorKind::ExpectedToken('}'),
locations: vec![Span::new(section_start_pos, i)],
});
return Err(errs);
}
} else {
errs.push(HeaderError {
kind: HeaderErrorKind::ExpectedToken('}'),
locations: vec![Span::new(section_start_pos, i)],
kind: HeaderErrorKind::ExpectedToken('{'),
locations: vec![Span::new(i, i)],
});
Err(errs)
return Err(errs);
}
} else {
} else if grmtools_required {
errs.push(HeaderError {
kind: HeaderErrorKind::ExpectedToken('{'),
locations: vec![Span::new(i, i)],
kind: HeaderErrorKind::MissingGrmtoolsSection,
locations: vec![Span::new(0, 0)],
});
Err(errs)
return Err(errs);
}
} else if self.required {
errs.push(HeaderError {
kind: HeaderErrorKind::MissingGrmtoolsSection,
locations: vec![Span::new(0, 0)],
});
Err(errs)
} else {
Ok((Header::new(), 0))
}
Ok((None, start_pos))
}

fn parse_name(&self, i: usize) -> Result<(String, usize), HeaderError<Span>> {
Expand Down Expand Up @@ -755,12 +875,12 @@ mod test {
let res = parser.parse();
let errs = res.unwrap_err();
assert_eq!(errs.len(), 1);
match errs[0] {
match &errs[0] {
HeaderError {
kind: HeaderErrorKind::UnexpectedToken('*', _),
locations: _,
} => (),
_ => panic!("Expected glob specific error"),
e => panic!("Expected glob specific error got '{e}'"),
}
}
}
Expand Down
Loading