From 722a8776be3d8afe90596a81c915e9e92bab781c Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Tue, 8 Oct 2024 14:28:07 -0700 Subject: [PATCH 01/13] AttributeSet type --- src/alias.rs | 4 +- src/analyzer.rs | 43 +++++++----------- src/attribute.rs | 67 ++++++++++++++++++++++++++++ src/item.rs | 2 +- src/lib.rs | 114 ++++++++++++++++++++++++++++++++++++----------- src/parser.rs | 34 +++++--------- src/recipe.rs | 82 ++++++++++++++++++---------------- 7 files changed, 229 insertions(+), 117 deletions(-) diff --git a/src/alias.rs b/src/alias.rs index 286cefd064..a1e433d8ff 100644 --- a/src/alias.rs +++ b/src/alias.rs @@ -3,7 +3,7 @@ use super::*; /// An alias, e.g. `name := target` #[derive(Debug, PartialEq, Clone, Serialize)] pub(crate) struct Alias<'src, T = Rc>> { - pub(crate) attributes: BTreeSet>, + pub(crate) attributes: AttributeSet<'src>, pub(crate) name: Name<'src>, #[serde( bound(serialize = "T: Keyed<'src>"), @@ -26,7 +26,7 @@ impl<'src> Alias<'src, Name<'src>> { impl Alias<'_> { pub(crate) fn is_private(&self) -> bool { - self.name.lexeme().starts_with('_') || self.attributes.contains(&Attribute::Private) + self.name.lexeme().starts_with('_') || self.attributes.private() } } diff --git a/src/analyzer.rs b/src/analyzer.rs index 0929294c5d..0576806b5f 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -72,17 +72,17 @@ impl<'run, 'src> Analyzer<'run, 'src> { } => { let mut doc_attr: Option<&str> = None; let mut groups = Vec::new(); - for attribute in attributes { + attributes.ensure_valid_attributes( + "Module", + **name, + &[AttributeDiscriminant::Doc, AttributeDiscriminant::Group], + )?; + + for attribute in attributes.iter() { if let Attribute::Doc(ref doc) = attribute { doc_attr = Some(doc.as_ref().map(|s| s.cooked.as_ref()).unwrap_or_default()); } else if let Attribute::Group(ref group) = attribute { groups.push(group.cooked.clone()); - } else { - return Err(name.token.error(InvalidAttribute { - item_kind: "Module", - item_name: name.lexeme(), - attribute: attribute.clone(), - })); } } @@ -170,11 +170,9 @@ impl<'run, 'src> Analyzer<'run, 'src> { } for recipe in recipes.values() { - for attribute in &recipe.attributes { - if let Attribute::Script(_) = attribute { - unstable_features.insert(UnstableFeature::ScriptAttribute); - break; - } + if recipe.attributes.contains(AttributeDiscriminant::Script) { + unstable_features.insert(UnstableFeature::ScriptAttribute); + break; } } @@ -284,11 +282,7 @@ impl<'run, 'src> Analyzer<'run, 'src> { } if !recipe.is_script() { - if let Some(attribute) = recipe - .attributes - .iter() - .find(|attribute| matches!(attribute, Attribute::Extension(_))) - { + if let Some(attribute) = recipe.attributes.get(AttributeDiscriminant::Extension) { return Err(recipe.name.error(InvalidAttribute { item_kind: "Recipe", item_name: recipe.name.lexeme(), @@ -301,16 +295,11 @@ impl<'run, 'src> Analyzer<'run, 'src> { } fn analyze_alias(alias: &Alias<'src, Name<'src>>) -> CompileResult<'src> { - for attribute in &alias.attributes { - if *attribute != Attribute::Private { - return Err(alias.name.token.error(InvalidAttribute { - item_kind: "Alias", - item_name: alias.name.lexeme(), - attribute: attribute.clone(), - })); - } - } - + alias.attributes.ensure_valid_attributes( + "Alias", + *alias.name, + &[AttributeDiscriminant::Private], + )?; Ok(()) } diff --git a/src/attribute.rs b/src/attribute.rs index ebdc212745..15b2cf494b 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -129,6 +129,73 @@ impl<'src> Display for Attribute<'src> { } } +#[derive(Default, Debug, Clone, PartialEq)] +pub(crate) struct AttributeSet<'src> { + inner: BTreeSet>, +} + +impl<'src> Serialize for AttributeSet<'src> { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.inner.serialize(serializer) + } +} + +impl<'src> AttributeSet<'src> { + pub(crate) fn from_iter(iter: impl IntoIterator>) -> Self { + Self { + inner: iter.into_iter().collect(), + } + } + + pub(crate) fn count(&self) -> usize { + self.inner.len() + } + + pub(crate) fn contains(&self, target: AttributeDiscriminant) -> bool { + self.inner.iter().any(|attr| { + let discriminant: AttributeDiscriminant = attr.into(); + discriminant == target + }) + } + + pub(crate) fn get(&self, discriminant: AttributeDiscriminant) -> Option<&Attribute<'src>> { + self.inner.iter().find(|attr| { + let item_discriminant: AttributeDiscriminant = (*attr).into(); + discriminant == item_discriminant + }) + } + + pub(crate) fn iter(&self) -> impl Iterator> { + self.inner.iter() + } + + pub(crate) fn private(&self) -> bool { + self.inner.contains(&Attribute::Private) + } + + pub(crate) fn ensure_valid_attributes( + &self, + item_kind: &'static str, + item_token: Token<'src>, + valid: &[AttributeDiscriminant], + ) -> Result<(), CompileError<'src>> { + for attribute in &self.inner { + let discriminant: AttributeDiscriminant = attribute.into(); + if !valid.contains(&discriminant) { + return Err(item_token.error(CompileErrorKind::InvalidAttribute { + item_kind, + item_name: item_token.lexeme(), + attribute: attribute.clone(), + })); + } + } + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/item.rs b/src/item.rs index 08d723fe70..7e8a99b9a4 100644 --- a/src/item.rs +++ b/src/item.rs @@ -13,7 +13,7 @@ pub(crate) enum Item<'src> { relative: StringLiteral<'src>, }, Module { - attributes: BTreeSet>, + attributes: AttributeSet<'src>, absolute: Option, doc: Option<&'src str>, name: Name<'src>, diff --git a/src/lib.rs b/src/lib.rs index 75e3332be4..9e1e79e063 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,31 +6,95 @@ pub(crate) use { crate::{ - alias::Alias, analyzer::Analyzer, argument_parser::ArgumentParser, assignment::Assignment, - assignment_resolver::AssignmentResolver, ast::Ast, attribute::Attribute, binding::Binding, - color::Color, color_display::ColorDisplay, command_color::CommandColor, - command_ext::CommandExt, compilation::Compilation, compile_error::CompileError, - compile_error_kind::CompileErrorKind, compiler::Compiler, condition::Condition, - conditional_operator::ConditionalOperator, config::Config, config_error::ConfigError, - constants::constants, count::Count, delimiter::Delimiter, dependency::Dependency, - dump_format::DumpFormat, enclosure::Enclosure, error::Error, evaluator::Evaluator, - execution_context::ExecutionContext, executor::Executor, expression::Expression, - fragment::Fragment, function::Function, interpreter::Interpreter, - interrupt_guard::InterruptGuard, interrupt_handler::InterruptHandler, item::Item, - justfile::Justfile, keyed::Keyed, keyword::Keyword, lexer::Lexer, line::Line, list::List, - load_dotenv::load_dotenv, loader::Loader, module_path::ModulePath, name::Name, - namepath::Namepath, ordinal::Ordinal, output::output, output_error::OutputError, - parameter::Parameter, parameter_kind::ParameterKind, parser::Parser, platform::Platform, - platform_interface::PlatformInterface, position::Position, positional::Positional, ran::Ran, - range_ext::RangeExt, recipe::Recipe, recipe_resolver::RecipeResolver, - recipe_signature::RecipeSignature, scope::Scope, search::Search, search_config::SearchConfig, - search_error::SearchError, set::Set, setting::Setting, settings::Settings, shebang::Shebang, - show_whitespace::ShowWhitespace, source::Source, string_delimiter::StringDelimiter, - string_kind::StringKind, string_literal::StringLiteral, subcommand::Subcommand, - suggestion::Suggestion, table::Table, thunk::Thunk, token::Token, token_kind::TokenKind, - unresolved_dependency::UnresolvedDependency, unresolved_recipe::UnresolvedRecipe, - unstable_feature::UnstableFeature, use_color::UseColor, variables::Variables, - verbosity::Verbosity, warning::Warning, + alias::Alias, + analyzer::Analyzer, + argument_parser::ArgumentParser, + assignment::Assignment, + assignment_resolver::AssignmentResolver, + ast::Ast, + attribute::{Attribute, AttributeDiscriminant, AttributeSet}, + binding::Binding, + color::Color, + color_display::ColorDisplay, + command_color::CommandColor, + command_ext::CommandExt, + compilation::Compilation, + compile_error::CompileError, + compile_error_kind::CompileErrorKind, + compiler::Compiler, + condition::Condition, + conditional_operator::ConditionalOperator, + config::Config, + config_error::ConfigError, + constants::constants, + count::Count, + delimiter::Delimiter, + dependency::Dependency, + dump_format::DumpFormat, + enclosure::Enclosure, + error::Error, + evaluator::Evaluator, + execution_context::ExecutionContext, + executor::Executor, + expression::Expression, + fragment::Fragment, + function::Function, + interpreter::Interpreter, + interrupt_guard::InterruptGuard, + interrupt_handler::InterruptHandler, + item::Item, + justfile::Justfile, + keyed::Keyed, + keyword::Keyword, + lexer::Lexer, + line::Line, + list::List, + load_dotenv::load_dotenv, + loader::Loader, + module_path::ModulePath, + name::Name, + namepath::Namepath, + ordinal::Ordinal, + output::output, + output_error::OutputError, + parameter::Parameter, + parameter_kind::ParameterKind, + parser::Parser, + platform::Platform, + platform_interface::PlatformInterface, + position::Position, + positional::Positional, + ran::Ran, + range_ext::RangeExt, + recipe::Recipe, + recipe_resolver::RecipeResolver, + recipe_signature::RecipeSignature, + scope::Scope, + search::Search, + search_config::SearchConfig, + search_error::SearchError, + set::Set, + setting::Setting, + settings::Settings, + shebang::Shebang, + show_whitespace::ShowWhitespace, + source::Source, + string_delimiter::StringDelimiter, + string_kind::StringKind, + string_literal::StringLiteral, + subcommand::Subcommand, + suggestion::Suggestion, + table::Table, + thunk::Thunk, + token::Token, + token_kind::TokenKind, + unresolved_dependency::UnresolvedDependency, + unresolved_recipe::UnresolvedRecipe, + unstable_feature::UnstableFeature, + use_color::UseColor, + variables::Variables, + verbosity::Verbosity, + warning::Warning, }, camino::Utf8Path, clap::ValueEnum, diff --git a/src/parser.rs b/src/parser.rs index e03ab7833f..cb49746635 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -439,7 +439,7 @@ impl<'run, 'src> Parser<'run, 'src> { if let Some((token, attributes)) = attributes { return Err(token.error(CompileErrorKind::ExtraneousAttributes { - count: attributes.len(), + count: attributes.count(), })); } } @@ -462,7 +462,7 @@ impl<'run, 'src> Parser<'run, 'src> { /// Parse an alias, e.g `alias name := target` fn parse_alias( &mut self, - attributes: BTreeSet>, + attributes: AttributeSet<'src>, ) -> CompileResult<'src, Alias<'src, Name<'src>>> { self.presume_keyword(Keyword::Alias)?; let name = self.parse_name()?; @@ -480,24 +480,15 @@ impl<'run, 'src> Parser<'run, 'src> { fn parse_assignment( &mut self, export: bool, - attributes: BTreeSet>, + attributes: AttributeSet<'src>, ) -> CompileResult<'src, Assignment<'src>> { let name = self.parse_name()?; self.presume(ColonEquals)?; let value = self.parse_expression()?; self.expect_eol()?; - let private = attributes.contains(&Attribute::Private); - - for attribute in attributes { - if attribute != Attribute::Private { - return Err(name.error(CompileErrorKind::InvalidAttribute { - item_kind: "Assignment", - item_name: name.lexeme(), - attribute, - })); - } - } + let private = attributes.private(); + attributes.ensure_valid_attributes("Assignment", *name, &[AttributeDiscriminant::Private])?; Ok(Assignment { file_depth: self.file_depth, @@ -862,7 +853,7 @@ impl<'run, 'src> Parser<'run, 'src> { &mut self, doc: Option<&'src str>, quiet: bool, - attributes: BTreeSet>, + attributes: AttributeSet<'src>, ) -> CompileResult<'src, UnresolvedRecipe<'src>> { let name = self.parse_name()?; @@ -923,9 +914,7 @@ impl<'run, 'src> Parser<'run, 'src> { let body = self.parse_body()?; let shebang = body.first().map_or(false, Line::is_shebang); - let script = attributes - .iter() - .any(|attribute| matches!(attribute, Attribute::Script(_))); + let script = attributes.contains(AttributeDiscriminant::Script); if shebang && script { return Err(name.error(CompileErrorKind::ShebangAndScriptAttribute { @@ -933,7 +922,7 @@ impl<'run, 'src> Parser<'run, 'src> { })); } - let private = name.lexeme().starts_with('_') || attributes.contains(&Attribute::Private); + let private = name.lexeme().starts_with('_') || attributes.private(); Ok(Recipe { shebang: shebang || script, @@ -1113,9 +1102,7 @@ impl<'run, 'src> Parser<'run, 'src> { } /// Item attributes, i.e., `[macos]` or `[confirm: "warning!"]` - fn parse_attributes( - &mut self, - ) -> CompileResult<'src, Option<(Token<'src>, BTreeSet>)>> { + fn parse_attributes(&mut self) -> CompileResult<'src, Option<(Token<'src>, AttributeSet<'src>)>> { let mut attributes = BTreeMap::new(); let mut token = None; @@ -1163,7 +1150,8 @@ impl<'run, 'src> Parser<'run, 'src> { if attributes.is_empty() { Ok(None) } else { - Ok(Some((token.unwrap(), attributes.into_keys().collect()))) + let attribute_set = AttributeSet::from_iter(attributes.into_keys()); + Ok(Some((token.unwrap(), attribute_set))) } } } diff --git a/src/recipe.rs b/src/recipe.rs index 05516225a6..5dde6be5a4 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -19,7 +19,7 @@ fn error_from_signal(recipe: &str, line_number: Option, exit_status: Exit /// A recipe, e.g. `foo: bar baz` #[derive(PartialEq, Debug, Clone, Serialize)] pub(crate) struct Recipe<'src, D = Dependency<'src>> { - pub(crate) attributes: BTreeSet>, + pub(crate) attributes: AttributeSet<'src>, pub(crate) body: Vec>, pub(crate) dependencies: Vec, pub(crate) doc: Option<&'src str>, @@ -66,20 +66,20 @@ impl<'src, D> Recipe<'src, D> { } pub(crate) fn confirm(&self) -> RunResult<'src, bool> { - for attribute in &self.attributes { - if let Attribute::Confirm(prompt) = attribute { - if let Some(prompt) = prompt { - eprint!("{} ", prompt.cooked); - } else { - eprint!("Run recipe `{}`? ", self.name); - } - let mut line = String::new(); - std::io::stdin() - .read_line(&mut line) - .map_err(|io_error| Error::GetConfirmation { io_error })?; - let line = line.trim().to_lowercase(); - return Ok(line == "y" || line == "yes"); + if let Some(Attribute::Confirm(ref prompt)) = + self.attributes.get(AttributeDiscriminant::Confirm) + { + if let Some(prompt) = prompt { + eprint!("{} ", prompt.cooked); + } else { + eprint!("Run recipe `{}`? ", self.name); } + let mut line = String::new(); + std::io::stdin() + .read_line(&mut line) + .map_err(|io_error| Error::GetConfirmation { io_error })?; + let line = line.trim().to_lowercase(); + return Ok(line == "y" || line == "yes"); } Ok(true) } @@ -97,7 +97,7 @@ impl<'src, D> Recipe<'src, D> { } pub(crate) fn is_public(&self) -> bool { - !self.private && !self.attributes.contains(&Attribute::Private) + !self.private && !self.attributes.private() } pub(crate) fn is_script(&self) -> bool { @@ -105,18 +105,21 @@ impl<'src, D> Recipe<'src, D> { } pub(crate) fn takes_positional_arguments(&self, settings: &Settings) -> bool { - settings.positional_arguments || self.attributes.contains(&Attribute::PositionalArguments) + settings.positional_arguments + || self + .attributes + .contains(AttributeDiscriminant::PositionalArguments) } pub(crate) fn change_directory(&self) -> bool { - !self.attributes.contains(&Attribute::NoCd) + !self.attributes.contains(AttributeDiscriminant::NoCd) } pub(crate) fn enabled(&self) -> bool { - let windows = self.attributes.contains(&Attribute::Windows); - let linux = self.attributes.contains(&Attribute::Linux); - let macos = self.attributes.contains(&Attribute::Macos); - let unix = self.attributes.contains(&Attribute::Unix); + let windows = self.attributes.contains(AttributeDiscriminant::Windows); + let linux = self.attributes.contains(AttributeDiscriminant::Linux); + let macos = self.attributes.contains(AttributeDiscriminant::Macos); + let unix = self.attributes.contains(AttributeDiscriminant::Unix); (!windows && !linux && !macos && !unix) || (cfg!(target_os = "windows") && windows) @@ -127,7 +130,9 @@ impl<'src, D> Recipe<'src, D> { } fn print_exit_message(&self) -> bool { - !self.attributes.contains(&Attribute::NoExitMessage) + !self + .attributes + .contains(AttributeDiscriminant::NoExitMessage) } fn working_directory<'a>(&'a self, context: &'a ExecutionContext) -> Option { @@ -139,7 +144,7 @@ impl<'src, D> Recipe<'src, D> { } fn no_quiet(&self) -> bool { - self.attributes.contains(&Attribute::NoQuiet) + self.attributes.contains(AttributeDiscriminant::NoQuiet) } pub(crate) fn run<'run>( @@ -341,10 +346,8 @@ impl<'src, D> Recipe<'src, D> { return Ok(()); } - let executor = if let Some(Attribute::Script(interpreter)) = self - .attributes - .iter() - .find(|attribute| matches!(attribute, Attribute::Script(_))) + let executor = if let Some(Attribute::Script(interpreter)) = + self.attributes.get(AttributeDiscriminant::Script) { Executor::Command( interpreter @@ -386,13 +389,16 @@ impl<'src, D> Recipe<'src, D> { })?; let mut path = tempdir.path().to_path_buf(); - let extension = self.attributes.iter().find_map(|attribute| { - if let Attribute::Extension(extension) = attribute { - Some(extension.cooked.as_str()) - } else { - None - } - }); + let extension = self + .attributes + .get(AttributeDiscriminant::Extension) + .and_then(|attribute| { + if let Attribute::Extension(extension) = attribute { + Some(extension.cooked.as_str()) + } else { + None + } + }); path.push(executor.script_filename(self.name(), extension)); @@ -460,10 +466,8 @@ impl<'src, D> Recipe<'src, D> { } pub(crate) fn doc(&self) -> Option<&str> { - for attribute in &self.attributes { - if let Attribute::Doc(doc) = attribute { - return doc.as_ref().map(|s| s.cooked.as_ref()); - } + if let Some(Attribute::Doc(doc)) = self.attributes.get(AttributeDiscriminant::Doc) { + return doc.as_ref().map(|s| s.cooked.as_ref()); } self.doc } @@ -479,7 +483,7 @@ impl<'src, D: Display> ColorDisplay for Recipe<'src, D> { writeln!(f, "# {doc}")?; } - for attribute in &self.attributes { + for attribute in self.attributes.iter() { writeln!(f, "[{attribute}]")?; } From 27648a97eb662b6a7702dd082e3f1e43c85318b2 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Wed, 30 Oct 2024 21:10:43 -0700 Subject: [PATCH 02/13] PR comments --- src/alias.rs | 2 +- src/analyzer.rs | 12 ++++++++---- src/attribute.rs | 47 +++++++++++++++++------------------------------ src/parser.rs | 7 ++++--- src/recipe.rs | 2 +- 5 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/alias.rs b/src/alias.rs index a1e433d8ff..4020c15714 100644 --- a/src/alias.rs +++ b/src/alias.rs @@ -26,7 +26,7 @@ impl<'src> Alias<'src, Name<'src>> { impl Alias<'_> { pub(crate) fn is_private(&self) -> bool { - self.name.lexeme().starts_with('_') || self.attributes.private() + self.name.lexeme().starts_with('_') || self.attributes.contains(AttributeDiscriminant::Private) } } diff --git a/src/analyzer.rs b/src/analyzer.rs index 0576806b5f..75fb8639f4 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -79,10 +79,14 @@ impl<'run, 'src> Analyzer<'run, 'src> { )?; for attribute in attributes.iter() { - if let Attribute::Doc(ref doc) = attribute { - doc_attr = Some(doc.as_ref().map(|s| s.cooked.as_ref()).unwrap_or_default()); - } else if let Attribute::Group(ref group) = attribute { - groups.push(group.cooked.clone()); + match attribute { + Attribute::Doc(ref doc) => { + doc_attr = Some(doc.as_ref().map(|s| s.cooked.as_ref()).unwrap_or_default()); + } + Attribute::Group(ref group) => { + groups.push(group.cooked.clone()); + } + _ => unreachable!(), } } diff --git a/src/attribute.rs b/src/attribute.rs index 15b2cf494b..618059d6b4 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -96,6 +96,10 @@ impl<'src> Attribute<'src> { }) } + pub(crate) fn discriminant(&self) -> AttributeDiscriminant { + self.into() + } + pub(crate) fn name(&self) -> &'static str { self.into() } @@ -129,51 +133,34 @@ impl<'src> Display for Attribute<'src> { } } -#[derive(Default, Debug, Clone, PartialEq)] -pub(crate) struct AttributeSet<'src> { - inner: BTreeSet>, -} - -impl<'src> Serialize for AttributeSet<'src> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.inner.serialize(serializer) - } -} +#[derive(Default, Debug, Clone, PartialEq, Serialize)] +pub(crate) struct AttributeSet<'src>(BTreeSet>); impl<'src> AttributeSet<'src> { pub(crate) fn from_iter(iter: impl IntoIterator>) -> Self { - Self { - inner: iter.into_iter().collect(), - } + Self(iter.into_iter().collect()) } - pub(crate) fn count(&self) -> usize { - self.inner.len() + pub(crate) fn len(&self) -> usize { + self.0.len() } pub(crate) fn contains(&self, target: AttributeDiscriminant) -> bool { - self.inner.iter().any(|attr| { + self.0.iter().any(|attr| { let discriminant: AttributeDiscriminant = attr.into(); discriminant == target }) } pub(crate) fn get(&self, discriminant: AttributeDiscriminant) -> Option<&Attribute<'src>> { - self.inner.iter().find(|attr| { - let item_discriminant: AttributeDiscriminant = (*attr).into(); - discriminant == item_discriminant - }) + self + .0 + .iter() + .find(|attr| discriminant == attr.discriminant()) } pub(crate) fn iter(&self) -> impl Iterator> { - self.inner.iter() - } - - pub(crate) fn private(&self) -> bool { - self.inner.contains(&Attribute::Private) + self.0.iter() } pub(crate) fn ensure_valid_attributes( @@ -182,8 +169,8 @@ impl<'src> AttributeSet<'src> { item_token: Token<'src>, valid: &[AttributeDiscriminant], ) -> Result<(), CompileError<'src>> { - for attribute in &self.inner { - let discriminant: AttributeDiscriminant = attribute.into(); + for attribute in &self.0 { + let discriminant = attribute.discriminant(); if !valid.contains(&discriminant) { return Err(item_token.error(CompileErrorKind::InvalidAttribute { item_kind, diff --git a/src/parser.rs b/src/parser.rs index cb49746635..2dae1cf072 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -439,7 +439,7 @@ impl<'run, 'src> Parser<'run, 'src> { if let Some((token, attributes)) = attributes { return Err(token.error(CompileErrorKind::ExtraneousAttributes { - count: attributes.count(), + count: attributes.len(), })); } } @@ -487,7 +487,7 @@ impl<'run, 'src> Parser<'run, 'src> { let value = self.parse_expression()?; self.expect_eol()?; - let private = attributes.private(); + let private = attributes.contains(AttributeDiscriminant::Private); attributes.ensure_valid_attributes("Assignment", *name, &[AttributeDiscriminant::Private])?; Ok(Assignment { @@ -922,7 +922,8 @@ impl<'run, 'src> Parser<'run, 'src> { })); } - let private = name.lexeme().starts_with('_') || attributes.private(); + let private = + name.lexeme().starts_with('_') || attributes.contains(AttributeDiscriminant::Private); Ok(Recipe { shebang: shebang || script, diff --git a/src/recipe.rs b/src/recipe.rs index 5dde6be5a4..ad0f2e9c21 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -97,7 +97,7 @@ impl<'src, D> Recipe<'src, D> { } pub(crate) fn is_public(&self) -> bool { - !self.private && !self.attributes.private() + !self.private && !self.attributes.contains(AttributeDiscriminant::Private) } pub(crate) fn is_script(&self) -> bool { From a452435052f8203bc40ee108a733ded079f7b977 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Wed, 30 Oct 2024 21:10:54 -0700 Subject: [PATCH 03/13] WIP PR comments --- src/attribute.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/attribute.rs b/src/attribute.rs index 618059d6b4..fd41aa270c 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -136,6 +136,16 @@ impl<'src> Display for Attribute<'src> { #[derive(Default, Debug, Clone, PartialEq, Serialize)] pub(crate) struct AttributeSet<'src>(BTreeSet>); +impl<'src> IntoIterator for &AttributeSet<'src> { + type Item = &'src Attribute<'src>; + + type IntoIter; + + fn into_iter(self) -> Self::IntoIter { + todo!() + } +} + impl<'src> AttributeSet<'src> { pub(crate) fn from_iter(iter: impl IntoIterator>) -> Self { Self(iter.into_iter().collect()) From b64704592bbb3d87118a3ea6b96e52a72d9c8eab Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Thu, 31 Oct 2024 23:03:29 -0700 Subject: [PATCH 04/13] More pr comments --- src/attribute.rs | 14 ++++++++------ src/recipe.rs | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index fd41aa270c..52d8ef8707 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -1,3 +1,5 @@ +use std::collections::{self}; + use super::*; #[derive( @@ -136,14 +138,14 @@ impl<'src> Display for Attribute<'src> { #[derive(Default, Debug, Clone, PartialEq, Serialize)] pub(crate) struct AttributeSet<'src>(BTreeSet>); -impl<'src> IntoIterator for &AttributeSet<'src> { - type Item = &'src Attribute<'src>; +impl<'src, 'a> IntoIterator for &'a AttributeSet<'src> { + type Item = &'a Attribute<'src>; - type IntoIter; + type IntoIter = collections::btree_set::Iter<'a, Attribute<'src>>; - fn into_iter(self) -> Self::IntoIter { - todo!() - } + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } } impl<'src> AttributeSet<'src> { diff --git a/src/recipe.rs b/src/recipe.rs index ad0f2e9c21..e2d477bd0f 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -483,7 +483,7 @@ impl<'src, D: Display> ColorDisplay for Recipe<'src, D> { writeln!(f, "# {doc}")?; } - for attribute in self.attributes.iter() { + for attribute in &self.attributes { writeln!(f, "[{attribute}]")?; } From 0716e62761958a11e4980286edcdebbc406fd9d9 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Fri, 1 Nov 2024 19:30:35 -0700 Subject: [PATCH 05/13] use discriminant --- src/attribute.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index 52d8ef8707..fd1d4b8f26 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -158,10 +158,7 @@ impl<'src> AttributeSet<'src> { } pub(crate) fn contains(&self, target: AttributeDiscriminant) -> bool { - self.0.iter().any(|attr| { - let discriminant: AttributeDiscriminant = attr.into(); - discriminant == target - }) + self.0.iter().any(|attr| attr.discriminant() == target) } pub(crate) fn get(&self, discriminant: AttributeDiscriminant) -> Option<&Attribute<'src>> { From aca47c722940e1691ab4dc66cfd1a700e8f19656 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Fri, 1 Nov 2024 21:24:32 -0700 Subject: [PATCH 06/13] fix --- src/recipe.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/recipe.rs b/src/recipe.rs index e2d477bd0f..3bdb105256 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -389,16 +389,13 @@ impl<'src, D> Recipe<'src, D> { })?; let mut path = tempdir.path().to_path_buf(); - let extension = self - .attributes - .get(AttributeDiscriminant::Extension) - .and_then(|attribute| { - if let Attribute::Extension(extension) = attribute { - Some(extension.cooked.as_str()) - } else { - None - } - }); + let extension = self.attributes.iter().find_map(|attribute| { + if let Attribute::Extension(extension) = attribute { + Some(extension.cooked.as_str()) + } else { + None + } + }); path.push(executor.script_filename(self.name(), extension)); From 90a1102ac14efe5ee668486ba148131c23edba88 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sat, 2 Nov 2024 00:21:56 -0700 Subject: [PATCH 07/13] tweak --- src/recipe.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/recipe.rs b/src/recipe.rs index 3bdb105256..5acdba509e 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -463,8 +463,10 @@ impl<'src, D> Recipe<'src, D> { } pub(crate) fn doc(&self) -> Option<&str> { - if let Some(Attribute::Doc(doc)) = self.attributes.get(AttributeDiscriminant::Doc) { - return doc.as_ref().map(|s| s.cooked.as_ref()); + for attribute in &self.attributes { + if let Attribute::Doc(doc) = attribute { + return doc.as_ref().map(|s| s.cooked.as_ref()); + } } self.doc } From 90537ca0026e64d73bf582836ce6c4c3f5172720 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 11 Nov 2024 15:39:32 -0800 Subject: [PATCH 08/13] Implement FromIterator for AttributeSet --- src/attribute.rs | 6 ++++-- src/parser.rs | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index fd1d4b8f26..04f8d9f9c6 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -148,11 +148,13 @@ impl<'src, 'a> IntoIterator for &'a AttributeSet<'src> { } } -impl<'src> AttributeSet<'src> { - pub(crate) fn from_iter(iter: impl IntoIterator>) -> Self { +impl<'src, 'a> FromIterator> for AttributeSet<'src> { + fn from_iter>>(iter: T) -> Self { Self(iter.into_iter().collect()) } +} +impl<'src> AttributeSet<'src> { pub(crate) fn len(&self) -> usize { self.0.len() } diff --git a/src/parser.rs b/src/parser.rs index 8adb986e40..a2e9749914 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1152,8 +1152,7 @@ impl<'run, 'src> Parser<'run, 'src> { if attributes.is_empty() { Ok(None) } else { - let attribute_set = AttributeSet::from_iter(attributes.into_keys()); - Ok(Some((token.unwrap(), attribute_set))) + Ok(Some((token.unwrap(), attributes.into_keys().collect()))) } } } From 43d4e4586c638317acab43d027bd4f2e34f88879 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 11 Nov 2024 15:43:08 -0800 Subject: [PATCH 09/13] Consolidate imports --- src/attribute.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index 04f8d9f9c6..feb59d5f8a 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -1,6 +1,4 @@ -use std::collections::{self}; - -use super::*; +use {super::*, std::collections}; #[derive( EnumDiscriminants, PartialEq, Debug, Clone, Serialize, Ord, PartialOrd, Eq, IntoStaticStr, From e439bdd9c702f92a32fcc633711309f924581f54 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 11 Nov 2024 15:48:49 -0800 Subject: [PATCH 10/13] Move AttributeSet into its own file --- src/attribute.rs | 59 ------------------------------------------- src/attribute_set.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 ++- 3 files changed, 63 insertions(+), 60 deletions(-) create mode 100644 src/attribute_set.rs diff --git a/src/attribute.rs b/src/attribute.rs index feb59d5f8a..4ea3997e60 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -133,65 +133,6 @@ impl<'src> Display for Attribute<'src> { } } -#[derive(Default, Debug, Clone, PartialEq, Serialize)] -pub(crate) struct AttributeSet<'src>(BTreeSet>); - -impl<'src, 'a> IntoIterator for &'a AttributeSet<'src> { - type Item = &'a Attribute<'src>; - - type IntoIter = collections::btree_set::Iter<'a, Attribute<'src>>; - - fn into_iter(self) -> Self::IntoIter { - self.0.iter() - } -} - -impl<'src, 'a> FromIterator> for AttributeSet<'src> { - fn from_iter>>(iter: T) -> Self { - Self(iter.into_iter().collect()) - } -} - -impl<'src> AttributeSet<'src> { - pub(crate) fn len(&self) -> usize { - self.0.len() - } - - pub(crate) fn contains(&self, target: AttributeDiscriminant) -> bool { - self.0.iter().any(|attr| attr.discriminant() == target) - } - - pub(crate) fn get(&self, discriminant: AttributeDiscriminant) -> Option<&Attribute<'src>> { - self - .0 - .iter() - .find(|attr| discriminant == attr.discriminant()) - } - - pub(crate) fn iter(&self) -> impl Iterator> { - self.0.iter() - } - - pub(crate) fn ensure_valid_attributes( - &self, - item_kind: &'static str, - item_token: Token<'src>, - valid: &[AttributeDiscriminant], - ) -> Result<(), CompileError<'src>> { - for attribute in &self.0 { - let discriminant = attribute.discriminant(); - if !valid.contains(&discriminant) { - return Err(item_token.error(CompileErrorKind::InvalidAttribute { - item_kind, - item_name: item_token.lexeme(), - attribute: attribute.clone(), - })); - } - } - Ok(()) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/attribute_set.rs b/src/attribute_set.rs new file mode 100644 index 0000000000..92517bff85 --- /dev/null +++ b/src/attribute_set.rs @@ -0,0 +1,60 @@ +use {super::*, std::collections}; + +#[derive(Default, Debug, Clone, PartialEq, Serialize)] +pub(crate) struct AttributeSet<'src>(BTreeSet>); + +impl<'src> AttributeSet<'src> { + pub(crate) fn len(&self) -> usize { + self.0.len() + } + + pub(crate) fn contains(&self, target: AttributeDiscriminant) -> bool { + self.0.iter().any(|attr| attr.discriminant() == target) + } + + pub(crate) fn get(&self, discriminant: AttributeDiscriminant) -> Option<&Attribute<'src>> { + self + .0 + .iter() + .find(|attr| discriminant == attr.discriminant()) + } + + pub(crate) fn iter(&self) -> impl Iterator> { + self.0.iter() + } + + pub(crate) fn ensure_valid_attributes( + &self, + item_kind: &'static str, + item_token: Token<'src>, + valid: &[AttributeDiscriminant], + ) -> Result<(), CompileError<'src>> { + for attribute in &self.0 { + let discriminant = attribute.discriminant(); + if !valid.contains(&discriminant) { + return Err(item_token.error(CompileErrorKind::InvalidAttribute { + item_kind, + item_name: item_token.lexeme(), + attribute: attribute.clone(), + })); + } + } + Ok(()) + } +} + +impl<'src, 'a> FromIterator> for AttributeSet<'src> { + fn from_iter>>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} + +impl<'src, 'a> IntoIterator for &'a AttributeSet<'src> { + type Item = &'a Attribute<'src>; + + type IntoIter = collections::btree_set::Iter<'a, Attribute<'src>>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} diff --git a/src/lib.rs b/src/lib.rs index 9e1e79e063..1bc5b2ce0f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,8 @@ pub(crate) use { assignment::Assignment, assignment_resolver::AssignmentResolver, ast::Ast, - attribute::{Attribute, AttributeDiscriminant, AttributeSet}, + attribute::{Attribute, AttributeDiscriminant}, + attribute_set::AttributeSet, binding::Binding, color::Color, color_display::ColorDisplay, @@ -177,6 +178,7 @@ mod assignment; mod assignment_resolver; mod ast; mod attribute; +mod attribute_set; mod binding; mod color; mod color_display; From 1379edc6609c3ed39237c8ebcf37b6b895148c94 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 11 Nov 2024 15:56:42 -0800 Subject: [PATCH 11/13] Add newline --- src/parser.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/parser.rs b/src/parser.rs index a2e9749914..da276e53ce 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -488,6 +488,7 @@ impl<'run, 'src> Parser<'run, 'src> { self.expect_eol()?; let private = attributes.contains(AttributeDiscriminant::Private); + attributes.ensure_valid_attributes("Assignment", *name, &[AttributeDiscriminant::Private])?; Ok(Assignment { From af508bf4c91040a836f52b0144fc5af1c9a43052 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Tue, 10 Dec 2024 15:26:32 -0800 Subject: [PATCH 12/13] Reform --- src/attribute_set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attribute_set.rs b/src/attribute_set.rs index 21adb7c44b..49d10d2e82 100644 --- a/src/attribute_set.rs +++ b/src/attribute_set.rs @@ -19,7 +19,7 @@ impl<'src> AttributeSet<'src> { .find(|attr| discriminant == attr.discriminant()) } - pub(crate) fn iter(&self) -> impl Iterator> { + pub(crate) fn iter<'a>(&'a self) -> collections::btree_set::Iter<'a, Attribute<'src>> { self.0.iter() } From b2b63ca21d4d406a082d3210c23ba5a521c38eda Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Tue, 10 Dec 2024 15:29:42 -0800 Subject: [PATCH 13/13] Amend --- src/analyzer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 75fb8639f4..ff5dd18754 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -78,7 +78,7 @@ impl<'run, 'src> Analyzer<'run, 'src> { &[AttributeDiscriminant::Doc, AttributeDiscriminant::Group], )?; - for attribute in attributes.iter() { + for attribute in attributes { match attribute { Attribute::Doc(ref doc) => { doc_attr = Some(doc.as_ref().map(|s| s.cooked.as_ref()).unwrap_or_default());