Skip to content

Commit

Permalink
psl: simplify attribute list validation (#4237)
Browse files Browse the repository at this point in the history
Since type aliases are not a thing,
`AttributesValidationState.attributes` does not need to be a Vec anymore
(we were accumulating attributes defined on the model and on aliases),
it can be an Option. This is simpler to reason about.
  • Loading branch information
tomhoule committed Sep 14, 2023
1 parent d4650df commit 3ef05ff
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
24 changes: 12 additions & 12 deletions psl/parser-database/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,15 @@ impl<'db> Context<'db> {
/// - When you are done validating an attribute set, you must call
/// `validate_visited_attributes()`. Otherwise, Context will helpfully panic.
pub(super) fn visit_attributes(&mut self, ast_attributes: ast::AttributeContainer) {
if !self.attributes.attributes.is_empty() || !self.attributes.unused_attributes.is_empty() {
if self.attributes.attributes.is_some() || !self.attributes.unused_attributes.is_empty() {
panic!(
"`ctx.visit_attributes() called with {:?} while the Context is still validating previous attribute set on {:?}`",
ast_attributes,
self.attributes.attributes
);
}

self.attributes.attributes.clear();
self.attributes.unused_attributes.clear();
self.attributes.extend_attributes(ast_attributes, self.ast);
self.attributes.set_attributes(ast_attributes, self.ast);
}

/// Look for an optional attribute with a name of the form
Expand All @@ -139,8 +137,8 @@ impl<'db> Context<'db> {
/// with a default that can be first, but with native types, arguments are
/// purely positional.
pub(crate) fn visit_datasource_scoped(&mut self) -> Option<(StringId, StringId, ast::AttributeId)> {
let attrs =
iter_attributes(&self.attributes.attributes, self.ast).filter(|(_, attr)| attr.name.name.contains('.'));
let attrs = iter_attributes(self.attributes.attributes.as_ref(), self.ast)
.filter(|(_, attr)| attr.name.name.contains('.'));
let mut native_type_attr = None;
let diagnostics = &mut self.diagnostics;

Expand Down Expand Up @@ -173,7 +171,8 @@ impl<'db> Context<'db> {
/// is defined.
#[must_use]
pub(crate) fn visit_optional_single_attr(&mut self, name: &'static str) -> bool {
let mut attrs = iter_attributes(&self.attributes.attributes, self.ast).filter(|(_, a)| a.name.name == name);
let mut attrs =
iter_attributes(self.attributes.attributes.as_ref(), self.ast).filter(|(_, a)| a.name.name == name);
let (first_idx, first) = match attrs.next() {
Some(first) => first,
None => return false,
Expand All @@ -182,7 +181,7 @@ impl<'db> Context<'db> {

if attrs.next().is_some() {
for (idx, attr) in
iter_attributes(&self.attributes.attributes, self.ast).filter(|(_, a)| a.name.name == name)
iter_attributes(self.attributes.attributes.as_ref(), self.ast).filter(|(_, a)| a.name.name == name)
{
diagnostics.push_error(DatamodelError::new_duplicate_attribute_error(
&attr.name.name,
Expand All @@ -206,7 +205,7 @@ impl<'db> Context<'db> {
let mut has_valid_attribute = false;

while !has_valid_attribute {
let first_attr = iter_attributes(&self.attributes.attributes, self.ast)
let first_attr = iter_attributes(self.attributes.attributes.as_ref(), self.ast)
.filter(|(_, attr)| attr.name.name == name)
.find(|(attr_id, _)| self.attributes.unused_attributes.contains(attr_id));
let (attr_id, attr) = if let Some(first_attr) = first_attr {
Expand Down Expand Up @@ -297,7 +296,8 @@ impl<'db> Context<'db> {
attribute.span,
))
}
self.attributes.attributes.clear();

self.attributes.attributes = None;
self.attributes.unused_attributes.clear();
}

Expand Down Expand Up @@ -430,11 +430,11 @@ impl<'db> Context<'db> {

// Implementation detail. Used for arguments validation.
fn iter_attributes<'a, 'ast: 'a>(
attrs: &'a [ast::AttributeContainer],
attrs: Option<&'a ast::AttributeContainer>,
ast: &'ast ast::SchemaAst,
) -> impl Iterator<Item = (ast::AttributeId, &'ast ast::Attribute)> + 'a {
attrs
.iter()
.into_iter()
.flat_map(move |container| ast[*container].iter().enumerate().map(|a| (a, *container)))
.map(|((idx, attr), container)| (ast::AttributeId::new_in_container(container, idx), attr))
}
Expand Down
7 changes: 4 additions & 3 deletions psl/parser-database/src/context/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::interner::StringId;
#[derive(Default, Debug)]
pub(super) struct AttributesValidationState {
/// The attributes list being validated.
pub(super) attributes: Vec<ast::AttributeContainer>,
pub(super) attributes: Option<ast::AttributeContainer>,
pub(super) unused_attributes: HashSet<ast::AttributeId>, // the _remaining_ attributes

/// The attribute being validated.
Expand All @@ -13,10 +13,11 @@ pub(super) struct AttributesValidationState {
}

impl AttributesValidationState {
pub(super) fn extend_attributes(&mut self, attributes: ast::AttributeContainer, ast: &ast::SchemaAst) {
pub(super) fn set_attributes(&mut self, attributes: ast::AttributeContainer, ast: &ast::SchemaAst) {
let attribute_ids = (0..ast[attributes].len()).map(|idx| ast::AttributeId::new_in_container(attributes, idx));
self.unused_attributes.clear();
self.unused_attributes.extend(attribute_ids);

self.attributes.push(attributes);
self.attributes = Some(attributes);
}
}

0 comments on commit 3ef05ff

Please sign in to comment.