remove FieldKind from DSL v2 (#687)

Makes the grammar a lot simpler, since it is no longer needed after enums were simplified in #610
NomicFoundation · Dec 4, 2023 · 0e38580 · 0e38580
1 parent 6c31653
commit 0e38580
Show file tree

Hide file tree

Showing 34 changed files with 5,299 additions and 2,318 deletions.
diff --git a/crates/codegen/language/definition/src/compiler/analysis/definitions.rs b/crates/codegen/language/definition/src/compiler/analysis/definitions.rs
@@ -87,12 +87,12 @@ fn check_precedence_items(analysis: &mut Analysis) {
         }
 
         for primary_expression in &item.primary_expressions {
-            let expression = &primary_expression.expression;
+            let reference = &primary_expression.reference;
 
-            if !current_expressions.insert(expression) {
+            if !current_expressions.insert(reference) {
                 analysis
                     .errors
-                    .add(expression, &Errors::ExistingExpression(expression));
+                    .add(reference, &Errors::ExistingExpression(reference));
             }
         }
     }

diff --git a/crates/codegen/language/definition/src/compiler/analysis/references.rs b/crates/codegen/language/definition/src/compiler/analysis/references.rs
@@ -2,18 +2,19 @@ use crate::{
     compiler::{analysis::Analysis, version_set::VersionSet},
     internals::Spanned,
     model::{
-        Identifier, SpannedEnumItem, SpannedEnumVariant, SpannedField, SpannedFieldKind,
-        SpannedFragmentItem, SpannedItem, SpannedKeywordDefinition, SpannedKeywordItem,
-        SpannedPrecedenceExpression, SpannedPrecedenceItem, SpannedPrecedenceOperator,
-        SpannedPrimaryExpression, SpannedRepeatedItem, SpannedScanner, SpannedSeparatedItem,
-        SpannedStructItem, SpannedTokenDefinition, SpannedTokenItem, SpannedTriviaItem,
-        SpannedTriviaParser, SpannedVersionSpecifier,
+        Identifier, SpannedEnumItem, SpannedEnumVariant, SpannedField, SpannedFragmentItem,
+        SpannedItem,
+        SpannedItemDiscriminants::{self, *},
+        SpannedKeywordDefinition, SpannedKeywordItem, SpannedPrecedenceExpression,
+        SpannedPrecedenceItem, SpannedPrecedenceOperator, SpannedPrimaryExpression,
+        SpannedRepeatedItem, SpannedScanner, SpannedSeparatedItem, SpannedStructItem,
+        SpannedTokenDefinition, SpannedTokenItem, SpannedTriviaItem, SpannedTriviaParser,
+        SpannedVersionSpecifier,
     },
 };
 use indexmap::IndexMap;
 use semver::Version;
 use std::fmt::Debug;
-use strum_macros::Display;
 
 pub(crate) fn analyze_references(analysis: &mut Analysis) {
     let language = analysis.language.clone();
@@ -26,7 +27,7 @@ pub(crate) fn analyze_references(analysis: &mut Analysis) {
         None,
         &language.root_item,
         &enablement,
-        ReferenceFilter::NonTerminals,
+        &[Struct, Enum, Repeated, Separated, Precedence],
     );
 
     check_trivia_parser(analysis, &language.leading_trivia, &enablement);
@@ -105,7 +106,9 @@ fn check_enum(analysis: &mut Analysis, item: &SpannedEnumItem, enablement: &Vers
             Some(name),
             reference,
             &enablement,
-            ReferenceFilter::Nodes,
+            &[
+                Struct, Enum, Repeated, Separated, Precedence, Keyword, Token,
+            ],
         );
     }
 }
@@ -124,7 +127,9 @@ fn check_repeated(analysis: &mut Analysis, item: &SpannedRepeatedItem, enablemen
         Some(name),
         repeated,
         &enablement,
-        ReferenceFilter::Nodes,
+        &[
+            Struct, Enum, Repeated, Separated, Precedence, Keyword, Token,
+        ],
     );
 }
 
@@ -143,15 +148,12 @@ fn check_separated(analysis: &mut Analysis, item: &SpannedSeparatedItem, enablem
         Some(name),
         separated,
         &enablement,
-        ReferenceFilter::Nodes,
-    );
-    check_reference(
-        analysis,
-        Some(name),
-        separator,
-        &enablement,
-        ReferenceFilter::Tokens,
+        &[
+            Struct, Enum, Repeated, Separated, Precedence, Keyword, Token,
+        ],
     );
+
+    check_reference(analysis, Some(name), separator, &enablement, &[Token]);
 }
 
 fn check_precedence(
@@ -190,19 +192,18 @@ fn check_precedence(
     }
 
     for primary_expression in primary_expressions {
-        let SpannedPrimaryExpression {
-            expression,
-            enabled,
-        } = primary_expression;
+        let SpannedPrimaryExpression { reference, enabled } = primary_expression;
 
         let enablement = update_enablement(analysis, &enablement, enabled);
 
         check_reference(
             analysis,
             Some(name),
-            expression,
+            reference,
             &enablement,
-            ReferenceFilter::Nodes,
+            &[
+                Struct, Enum, Repeated, Separated, Precedence, Keyword, Token,
+            ],
         );
     }
 }
@@ -215,46 +216,34 @@ fn check_fields(
 ) {
     for field in fields.values() {
         match field {
-            SpannedField::Required { kind } => {
-                check_field_kind(analysis, source, kind, enablement);
+            SpannedField::Required { reference } => {
+                check_reference(
+                    analysis,
+                    source,
+                    reference,
+                    enablement,
+                    &[
+                        Struct, Enum, Repeated, Separated, Precedence, Keyword, Token,
+                    ],
+                );
             }
-            SpannedField::Optional { kind, enabled } => {
+            SpannedField::Optional { reference, enabled } => {
                 let enablement = update_enablement(analysis, enablement, enabled);
 
-                check_field_kind(analysis, source, kind, &enablement);
-            }
-        };
-    }
-}
-
-fn check_field_kind(
-    analysis: &mut Analysis,
-    source: Option<&Identifier>,
-    kind: &SpannedFieldKind,
-    enablement: &VersionSet,
-) {
-    match kind {
-        SpannedFieldKind::NonTerminal { item } => {
-            check_reference(
-                analysis,
-                source,
-                item,
-                enablement,
-                ReferenceFilter::NonTerminals,
-            );
-        }
-        SpannedFieldKind::Terminal { items } => {
-            for item in items {
                 check_reference(
                     analysis,
                     source,
-                    item,
-                    enablement,
-                    ReferenceFilter::Terminals,
+                    reference,
+                    &enablement,
+                    &[
+                        // TODO(#638): remove [Separated] and [Repeated] from here, once they are allowed to be empty.
+                        // Therefore, we ensure we always produce the parent node, even if it has no children.
+                        Struct, Enum, Repeated, Separated, Precedence, Keyword, Token,
+                    ],
                 );
             }
-        }
-    };
+        };
+    }
 }
 
 fn check_trivia_parser(
@@ -274,7 +263,7 @@ fn check_trivia_parser(
             check_trivia_parser(analysis, parser, enablement);
         }
         SpannedTriviaParser::Trivia { trivia } => {
-            check_reference(analysis, None, trivia, enablement, ReferenceFilter::Trivia);
+            check_reference(analysis, None, trivia, enablement, &[Trivia]);
         }
     };
 }
@@ -292,13 +281,7 @@ fn check_keyword(analysis: &mut Analysis, item: &SpannedKeywordItem, enablement:
         definitions,
     } = item;
 
-    check_reference(
-        analysis,
-        Some(name),
-        identifier,
-        enablement,
-        ReferenceFilter::Tokens,
-    );
+    check_reference(analysis, Some(name), identifier, enablement, &[Token]);
 
     for definition in definitions {
         let SpannedKeywordDefinition {
@@ -372,65 +355,17 @@ fn check_scanner(
             check_scanner(analysis, source, not_followed_by, enablement);
         }
         SpannedScanner::Fragment { reference } => {
-            check_reference(
-                analysis,
-                source,
-                reference,
-                enablement,
-                ReferenceFilter::Fragments,
-            );
+            check_reference(analysis, source, reference, enablement, &[Fragment]);
         }
     };
 }
 
-/// Used by different checks above to make sure that references to other items are valid.
-/// For example, struct fields can reference anything, but tokens can only reference fragments.
-#[derive(Debug, Display, PartialEq, Eq)]
-enum ReferenceFilter {
-    Nodes,
-
-    NonTerminals,
-    Terminals,
-
-    Trivia,
-    Tokens,
-
-    Fragments,
-}
-
-impl ReferenceFilter {
-    fn apply(&self, item: &SpannedItem) -> bool {
-        match (self, item) {
-            (Self::Nodes, item) => Self::NonTerminals.apply(item) || Self::Terminals.apply(item),
-            (
-                Self::NonTerminals,
-                SpannedItem::Struct { .. }
-                | SpannedItem::Enum { .. }
-                | SpannedItem::Repeated { .. }
-                | SpannedItem::Separated { .. }
-                | SpannedItem::Precedence { .. },
-            )
-            | (
-                Self::Terminals,
-                SpannedItem::Trivia { .. }
-                | SpannedItem::Keyword { .. }
-                | SpannedItem::Token { .. },
-            )
-            | (Self::Trivia, SpannedItem::Trivia { .. })
-            | (Self::Tokens, SpannedItem::Token { .. })
-            | (Self::Fragments, SpannedItem::Fragment { .. }) => true,
-
-            _ => false,
-        }
-    }
-}
-
 fn check_reference(
     analysis: &mut Analysis,
     source: Option<&Identifier>,
     reference: &Spanned<Identifier>,
     enablement: &VersionSet,
-    filter: ReferenceFilter,
+    expected_kinds: &[SpannedItemDiscriminants],
 ) {
     let target = match analysis.metadata.get_mut(&**reference) {
         Some(target) => target,
@@ -450,10 +385,11 @@ fn check_reference(
         );
     }
 
-    if !filter.apply(&target.item) {
+    let actual_kind: SpannedItemDiscriminants = target.item.as_ref().into();
+    if !expected_kinds.contains(&actual_kind) {
         analysis.errors.add(
             reference,
-            &Errors::InvalidReferenceFilter(reference, &filter),
+            &Errors::InvalidReferenceFilter(reference, &actual_kind, expected_kinds),
         );
     }
 
@@ -536,6 +472,10 @@ enum Errors<'err> {
     UnknownReference(&'err Identifier),
     #[error("Reference '{0}' is only defined in '{1}', but not in '{2}'.")]
     InvalidReferenceVersion(&'err Identifier, &'err VersionSet, &'err VersionSet),
-    #[error("Reference '{0}' is not valid. Expected: {1}.")]
-    InvalidReferenceFilter(&'err Identifier, &'err ReferenceFilter),
+    #[error("Reference '{0}' of kind '{1:?}' is not valid. Expected: {2:?}")]
+    InvalidReferenceFilter(
+        &'err Identifier,
+        &'err SpannedItemDiscriminants,
+        &'err [SpannedItemDiscriminants],
+    ),
 }
diff --git a/crates/codegen/language/definition/src/model/item.rs b/crates/codegen/language/definition/src/model/item.rs
@@ -4,9 +4,10 @@ use crate::model::{
 };
 use codegen_language_internal_macros::{derive_spanned_type, ParseInputTokens, WriteOutputTokens};
 use serde::{Deserialize, Serialize};
+use strum_macros::EnumDiscriminants;
 
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
-#[derive_spanned_type(ParseInputTokens, WriteOutputTokens)]
+#[derive_spanned_type(EnumDiscriminants, ParseInputTokens, WriteOutputTokens)]
 pub enum Item {
     Struct { item: StructItem },
     Enum { item: EnumItem },

diff --git a/crates/codegen/language/definition/src/model/non_terminals/field.rs b/crates/codegen/language/definition/src/model/non_terminals/field.rs
@@ -1,6 +1,5 @@
 use crate::model::{Identifier, VersionSpecifier};
 use codegen_language_internal_macros::{derive_spanned_type, ParseInputTokens, WriteOutputTokens};
-use indexmap::IndexSet;
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
@@ -21,18 +20,11 @@ pub struct FieldDelimiters {
 #[derive_spanned_type(ParseInputTokens, WriteOutputTokens)]
 pub enum Field {
     Required {
-        kind: FieldKind,
+        reference: Identifier,
     },
     Optional {
-        kind: FieldKind,
+        reference: Identifier,
 
         enabled: Option<VersionSpecifier>,
     },
 }
-
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
-#[derive_spanned_type(ParseInputTokens, WriteOutputTokens)]
-pub enum FieldKind {
-    NonTerminal { item: Identifier },
-    Terminal { items: IndexSet<Identifier> },
-}
diff --git a/crates/codegen/language/definition/src/model/non_terminals/precedence.rs b/crates/codegen/language/definition/src/model/non_terminals/precedence.rs
@@ -48,7 +48,7 @@ pub enum OperatorModel {
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(ParseInputTokens, WriteOutputTokens)]
 pub struct PrimaryExpression {
-    pub expression: Identifier,
+    pub reference: Identifier,
 
     pub enabled: Option<VersionSpecifier>,
 }
diff --git a/crates/codegen/language/internal_macros/src/derive/spanned.rs b/crates/codegen/language/internal_macros/src/derive/spanned.rs
@@ -112,7 +112,6 @@ fn get_spanned_type(input: Type) -> Type {
         | "EnumVariant"
         | "Field"
         | "FieldDelimiters"
-        | "FieldKind"
         | "FieldsErrorRecovery"
         | "FragmentItem"
         | "InputItem"

diff --git a/crates/codegen/language/tests/src/fail/definitions/duplicate_expression_name/test.rs b/crates/codegen/language/tests/src/fail/definitions/duplicate_expression_name/test.rs
@@ -18,7 +18,7 @@ codegen_language_macros::compile!(Language(
                         PrecedenceExpression(name = Expression2, rule_name = X, operators = []),
                         PrecedenceExpression(name = Expression1, rule_name = X, operators = [])
                     ],
-                    primary_expressions = [PrimaryExpression(expression = Baz)]
+                    primary_expressions = [PrimaryExpression(reference = Baz)]
                 ),
                 Token(
                     name = Baz,

diff --git a/crates/codegen/language/tests/src/fail/definitions/duplicate_item_name/test.rs b/crates/codegen/language/tests/src/fail/definitions/duplicate_item_name/test.rs
@@ -11,9 +11,9 @@ codegen_language_macros::compile!(Language(
         topics = [Topic(
             title = "Topic One",
             items = [
-                Struct(name = Bar1, fields = (field = Required(NonTerminal(Bar2)))),
-                Struct(name = Bar1, fields = (field = Required(NonTerminal(Bar2)))),
-                Struct(name = Bar2, fields = (field = Required(NonTerminal(Bar1))))
+                Struct(name = Bar1, fields = (field = Required(Bar2))),
+                Struct(name = Bar1, fields = (field = Required(Bar2))),
+                Struct(name = Bar2, fields = (field = Required(Bar1)))
             ]
         )]
     )]

diff --git a/crates/codegen/language/tests/src/fail/definitions/duplicate_item_name/test.stderr b/crates/codegen/language/tests/src/fail/definitions/duplicate_item_name/test.stderr
@@ -1,5 +1,5 @@
 error: An item with the name 'Bar1' already exists.
   --> src/fail/definitions/duplicate_item_name/test.rs:15:31
    |
-15 |                 Struct(name = Bar1, fields = (field = Required(NonTerminal(Bar2)))),
+15 |                 Struct(name = Bar1, fields = (field = Required(Bar2))),
    |                               ^^^^