Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AttributeSet type #2419

Merged
merged 17 commits into from
Dec 10, 2024
4 changes: 2 additions & 2 deletions src/alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::*;
/// An alias, e.g. `name := target`
#[derive(Debug, PartialEq, Clone, Serialize)]
pub(crate) struct Alias<'src, T = Rc<Recipe<'src>>> {
pub(crate) attributes: BTreeSet<Attribute<'src>>,
pub(crate) attributes: AttributeSet<'src>,
pub(crate) name: Name<'src>,
#[serde(
bound(serialize = "T: Keyed<'src>"),
Expand All @@ -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.contains(AttributeDiscriminant::Private)
}
}

Expand Down
53 changes: 23 additions & 30 deletions src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,21 @@ impl<'run, 'src> Analyzer<'run, 'src> {
} => {
let mut doc_attr: Option<&str> = None;
let mut groups = Vec::new();
attributes.ensure_valid_attributes(
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should keep the error checking in the else below, otherwise the set of allowed attributes is in two places, here, where we call ensure_valid_attributes, and below, in the if / else statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we keep the error checking here, then we would have pretty similar error-generation code here and in the ensure_valid_attributes method. I added ensure_valid_attributes as a way of de-duplicating attribute error generation code. I modified the code below to add an unreachable and hopefully make it clearer what's going on.

"Module",
**name,
&[AttributeDiscriminant::Doc, AttributeDiscriminant::Group],
)?;

for attribute in attributes {
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(),
}));
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!(),
}
}

Expand Down Expand Up @@ -170,11 +174,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;
}
}

Expand Down Expand Up @@ -284,11 +286,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(),
Expand All @@ -301,16 +299,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(())
}

Expand Down
60 changes: 60 additions & 0 deletions src/attribute_set.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use {super::*, std::collections};

#[derive(Default, Debug, Clone, PartialEq, Serialize)]
pub(crate) struct AttributeSet<'src>(BTreeSet<Attribute<'src>>);

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<'a>(&'a self) -> collections::btree_set::Iter<'a, Attribute<'src>> {
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> FromIterator<Attribute<'src>> for AttributeSet<'src> {
fn from_iter<T: IntoIterator<Item = attribute::Attribute<'src>>>(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()
}
}
2 changes: 1 addition & 1 deletion src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub(crate) enum Item<'src> {
relative: StringLiteral<'src>,
},
Module {
attributes: BTreeSet<Attribute<'src>>,
attributes: AttributeSet<'src>,
absolute: Option<PathBuf>,
doc: Option<&'src str>,
name: Name<'src>,
Expand Down
116 changes: 91 additions & 25 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,96 @@

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},
attribute_set::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,
Expand Down Expand Up @@ -120,6 +185,7 @@ mod assignment;
mod assignment_resolver;
mod ast;
mod attribute;
mod attribute_set;
mod binding;
mod color;
mod color_display;
Expand Down
Loading
Loading