Added a default value for VersionSpecifier and some documentation (#1500)

Originally I wanted to stop wrapping `VersionSpecifier` in an `Option`,
and parse it as `Always` if it's not present, but that brings some
issues with the span information, and we'd need to differentiate between
an `Always` with span (explicit) and one without (implicit).

However, I think having the default value representable is good to
simplify some algorithms, in particular, the information of what a
missing field on the language definition means is now explicit.

I also added some documentation here and there.

---------

Co-authored-by: Gustavo Giráldez <ggiraldez@manas.tech>

Author: teofr

crates/codegen-v2/parser/src/lexer/builder.rs

@@ -95,16 +95,22 @@ impl LexerModelBuilder {
                     }
                     Lexeme::Keyword { kind, reserved, .. } => {
                         if match reserved {
-                            None => true,
-                            Some(VersionSpecifier::Never) => false,
-                            Some(VersionSpecifier::From { .. }) => true,
-                            Some(VersionSpecifier::Till { .. }) => true,
-                            Some(VersionSpecifier::Range { .. }) => true,
+                            VersionSpecifier::Always => true,
+                            VersionSpecifier::Never => false,
+                            VersionSpecifier::From { .. } => true,
+                            VersionSpecifier::Till { .. } => true,
+                            VersionSpecifier::Range { .. } => true,
                         } {
                             kinds.insert(format!("{kind}_Reserved"));
                         }
 
-                        if reserved.is_some() {
+                        if match reserved {
+                            VersionSpecifier::Always => false,
+                            VersionSpecifier::Never => true,
+                            VersionSpecifier::From { .. } => true,
+                            VersionSpecifier::Till { .. } => true,
+                            VersionSpecifier::Range { .. } => true,
+                        } {
                             kinds.insert(format!("{kind}_Unreserved"));
                         }
                     }
@@ -126,7 +132,7 @@ impl LexerModelBuilder {
         item.definitions.iter().map(|def| Lexeme::Keyword {
             kind: item.name.to_string(),
             regex: self.convert_keyword_value(&def.value),
-            reserved: def.reserved.clone(),
+            reserved: def.reserved.clone().unwrap_or_default(),
         })
     }
 

crates/codegen-v2/parser/src/lexer/mod.rs

@@ -39,6 +39,6 @@ pub enum Lexeme {
     Keyword {
         kind: String,
         regex: String,
-        reserved: Option<VersionSpecifier>,
+        reserved: VersionSpecifier,
     },
 }

crates/language-v2/definition/src/compiler/analysis/p2_version_specifiers/mod.rs

@@ -263,6 +263,7 @@ fn check_version_specifier(
     };
 
     match &**specifier {
+        SpannedVersionSpecifier::Always => {}
         SpannedVersionSpecifier::Never => {}
         SpannedVersionSpecifier::From { from } => {
             check_version(analysis, from);

crates/language-v2/definition/src/compiler/utils/version_set.rs

@@ -28,6 +28,9 @@ impl VersionSet {
         language: &SpannedLanguage,
     ) {
         match specifier {
+            SpannedVersionSpecifier::Always => {
+                self.add_version_range(&language.versions[0], &MAX_VERSION);
+            }
             SpannedVersionSpecifier::Never => {
                 // Do nothing.
             }

crates/language-v2/definition/src/model/item.rs

@@ -9,6 +9,9 @@ use crate::model::{
     StructItem, TokenItem, TriviaItem,
 };
 
+/// An item is the smallest unit of a language definition.
+///
+/// It represents both terminals and nonterminals.
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, EnumDiscriminants, ParseInputTokens, WriteOutputTokens)]
 #[serde(tag = "type")]

crates/language-v2/definition/src/model/manifest.rs

@@ -1,5 +1,4 @@
 use std::collections::BTreeSet;
-use std::path::PathBuf;
 
 use indexmap::IndexSet;
 use language_v2_internal_macros::{derive_spanned_type, ParseInputTokens, WriteOutputTokens};
@@ -9,21 +8,29 @@ use serde::{Deserialize, Serialize};
 use super::BuiltIn;
 use crate::model::{BuiltInContext, Field, Identifier, Item, TriviaParser, VersionSpecifier};
 
+/// A representation of a Language definition
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 pub struct Language {
+    /// The name of the language
     pub name: Identifier,
 
-    pub binding_rules_file: PathBuf,
-
+    /// Each language must have a single root item
     pub root_item: Identifier,
 
+    /// The leading trivia parser
     pub leading_trivia: TriviaParser,
+
+    /// The trailing trivia parser
     pub trailing_trivia: TriviaParser,
 
+    /// The supported versions of the language
     pub versions: IndexSet<Version>,
 
+    /// The sections of the language
     pub sections: Vec<Section>,
+
+    /// The built-in contexts
     pub built_ins: Vec<BuiltInContext>,
 }
 
@@ -57,7 +64,7 @@ impl Language {
 
         let mut add_spec = |spec: &Option<VersionSpecifier>| {
             if let Some(spec) = spec {
-                res.extend(spec.versions().cloned());
+                res.extend(spec.breaking_versions().cloned());
             }
         };
 
@@ -130,7 +137,7 @@ impl Language {
 
         let mut add_spec = |spec: &Option<VersionSpecifier>| {
             if let Some(spec) = spec {
-                res.extend(spec.versions().cloned());
+                res.extend(spec.breaking_versions().cloned());
             }
         };
 
@@ -160,6 +167,7 @@ impl Language {
     }
 }
 
+/// A section is a named container for topics, used for organizing the large grammar definition in user documentation.  
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 pub struct Section {
@@ -174,6 +182,7 @@ impl Section {
     }
 }
 
+/// A topic is a named container for items, used for organizing the large grammar definition in user documentation.  
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 pub struct Topic {

crates/language-v2/definition/src/model/nonterminals/field.rs

@@ -3,12 +3,28 @@ use serde::{Deserialize, Serialize};
 
 use crate::model::{Identifier, VersionSpecifier};
 
+/// Error recovery for fields, this is used by the parser to recover in case of missing or unexpected tokens.
+///
+/// Note: Some nonterminals have both a terminator and delimiters, ie `DoWhileStatement`.
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 pub struct FieldsErrorRecovery {
+    /// Error recovery happens at a terminator.
+    ///
+    /// For example `PragmaDirective` has a `Semicolon` terminator, that could
+    /// be used to recover from an invalid `pragma` directive like
+    /// ```
+    /// pragma soldity ^0.8.0;
+    /// ```
     #[serde(skip_serializing_if = "Option::is_none")]
     pub terminator: Option<Identifier>,
 
+    /// Error recovery happens at delimiters.
+    ///
+    /// For example `TupleExpression` has a `OpenParen` and `CloseParen` delimiters,
+    /// that could be used to recover from an invalid tuple like
+    /// `(pragma, bar)`
+    ///   ~~~~~ -> This is not a valid expression
     #[serde(skip_serializing_if = "Option::is_none")]
     pub delimiters: Option<FieldDelimiters>,
 }
@@ -29,6 +45,9 @@ pub struct FieldDelimiters {
     pub terminals_matched_acceptance_threshold: Option<u8>,
 }
 
+/// A `Field` of a nonterminal that can be either required or optional.
+///
+/// Note: `Optional` fields are versioned, `Required` fields are not.
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 #[serde(tag = "type")]

crates/language-v2/definition/src/model/nonterminals/struct_.rs

@@ -4,17 +4,22 @@ use serde::{Deserialize, Serialize};
 
 use crate::model::{Field, FieldsErrorRecovery, Identifier, VersionSpecifier};
 
+/// A `StructItem` is a nonterminal that can have fields.
+/// It roughly corresponds to a sequence of `Item`s.
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 pub struct StructItem {
     pub name: Identifier,
 
+    /// Whether the struct is enabled
     #[serde(skip_serializing_if = "Option::is_none")]
     pub enabled: Option<VersionSpecifier>,
 
+    /// Error recovery information if this struct supports it
     #[serde(skip_serializing_if = "Option::is_none")]
     pub error_recovery: Option<FieldsErrorRecovery>,
 
+    /// The fields of the struct, in the order they should appear in the source code
     #[serde(with = "indexmap::map::serde_seq")]
     pub fields: IndexMap<Identifier, Field>,
 }

crates/language-v2/definition/src/model/utils/version_specifier.rs

@@ -2,19 +2,29 @@ use language_v2_internal_macros::{derive_spanned_type, ParseInputTokens, WriteOu
 use semver::Version;
 use serde::{Deserialize, Serialize};
 
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
+#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
 #[derive_spanned_type(Clone, Debug, ParseInputTokens, WriteOutputTokens)]
 #[serde(tag = "type")]
 pub enum VersionSpecifier {
+    #[default]
+    Always,
     Never,
-    From { from: Version },
-    Till { till: Version },
-    Range { from: Version, till: Version },
+    From {
+        from: Version,
+    },
+    Till {
+        till: Version,
+    },
+    Range {
+        from: Version,
+        till: Version,
+    },
 }
 
 impl VersionSpecifier {
     pub fn contains(&self, version: &Version) -> bool {
         match self {
+            VersionSpecifier::Always => true,
             VersionSpecifier::Never => false,
             VersionSpecifier::From { from } => from <= version,
             VersionSpecifier::Till { till } => version < till,
@@ -23,9 +33,9 @@ impl VersionSpecifier {
     }
 
     /// Returns an iterator over the versions specified as the upper and lower bound.
-    pub fn versions(&self) -> impl Iterator<Item = &Version> {
+    pub fn breaking_versions(&self) -> impl Iterator<Item = &Version> {
         match self {
-            VersionSpecifier::Never => [None, None],
+            VersionSpecifier::Always | VersionSpecifier::Never => [None, None],
             VersionSpecifier::From { from } => [Some(from), None],
             VersionSpecifier::Till { till } => [None, Some(till)],
             VersionSpecifier::Range { from, till } => [Some(from), Some(till)],

crates/language-v2/tests/src/fail/p0_parsing/duplicate_map_key/test.rs

@@ -2,7 +2,6 @@
 
 language_v2_macros::compile!(Language(
     name = Foo,
-    binding_rules_file = "bindings/rules.msgb",
     root_item = Bar,
     leading_trivia = Sequence([]),
     trailing_trivia = Sequence([]),