Skip to content

Commit

Permalink
Don't conflate recipes with the same name in different modules (#1825)
Browse files Browse the repository at this point in the history
  • Loading branch information
@casey
casey authored Jan 8, 2024
1 parent 0dbd5bf commit 1ea5e6a
Show file tree
Hide file tree
Showing 19 changed files with 257 additions and 159 deletions.
17 changes: 8 additions & 9 deletions src/analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<'src> Analyzer<'src> {
(*original, name)
};

return Err(redefinition.token().error(Redefinition {
return Err(redefinition.token.error(Redefinition {
first_type,
second_type,
name: name.lexeme(),
Expand Down Expand Up @@ -83,7 +83,7 @@ impl<'src> Analyzer<'src> {
if let Some(absolute) = absolute {
define(*name, "module", false)?;
modules.insert(
name.to_string(),
name.lexeme().into(),
(*name, Self::analyze(loaded, paths, asts, absolute)?),
);
}
Expand Down Expand Up @@ -160,7 +160,7 @@ impl<'src> Analyzer<'src> {

for parameter in &recipe.parameters {
if parameters.contains(parameter.name.lexeme()) {
return Err(parameter.name.token().error(DuplicateParameter {
return Err(parameter.name.token.error(DuplicateParameter {
recipe: recipe.name.lexeme(),
parameter: parameter.name.lexeme(),
}));
Expand All @@ -173,7 +173,7 @@ impl<'src> Analyzer<'src> {
return Err(
parameter
.name
.token()
.token
.error(RequiredParameterFollowsDefaultParameter {
parameter: parameter.name.lexeme(),
}),
Expand Down Expand Up @@ -201,7 +201,7 @@ impl<'src> Analyzer<'src> {

fn analyze_assignment(&self, assignment: &Assignment<'src>) -> CompileResult<'src> {
if self.assignments.contains_key(assignment.name.lexeme()) {
return Err(assignment.name.token().error(DuplicateVariable {
return Err(assignment.name.token.error(DuplicateVariable {
variable: assignment.name.lexeme(),
}));
}
Expand All @@ -213,7 +213,7 @@ impl<'src> Analyzer<'src> {

for attr in &alias.attributes {
if *attr != Attribute::Private {
return Err(alias.name.token().error(AliasInvalidAttribute {
return Err(alias.name.token.error(AliasInvalidAttribute {
alias: name,
attr: *attr,
}));
Expand All @@ -238,10 +238,9 @@ impl<'src> Analyzer<'src> {
recipes: &Table<'src, Rc<Recipe<'src>>>,
alias: Alias<'src, Name<'src>>,
) -> CompileResult<'src, Alias<'src>> {
let token = alias.name.token();
// Make sure the alias doesn't conflict with any recipe
if let Some(recipe) = recipes.get(alias.name.lexeme()) {
return Err(token.error(AliasShadowsRecipe {
return Err(alias.name.token.error(AliasShadowsRecipe {
alias: alias.name.lexeme(),
recipe_line: recipe.line_number(),
}));
Expand All @@ -250,7 +249,7 @@ impl<'src> Analyzer<'src> {
// Make sure the target recipe exists
match recipes.get(alias.target.lexeme()) {
Some(target) => Ok(alias.resolve(Rc::clone(target))),
None => Err(token.error(UnknownAliasTarget {
None => Err(alias.name.token.error(UnknownAliasTarget {
alias: alias.name.lexeme(),
target: alias.target.lexeme(),
})),
Expand Down
15 changes: 9 additions & 6 deletions src/assignment_resolver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,19 @@ impl<'src: 'run, 'run> AssignmentResolver<'src, 'run> {
if self.evaluated.contains(variable) {
Ok(())
} else if self.stack.contains(&variable) {
let token = self.assignments[variable].name.token();
self.stack.push(variable);
Err(token.error(CircularVariableDependency {
variable,
circle: self.stack.clone(),
}))
Err(
self.assignments[variable]
.name
.error(CircularVariableDependency {
variable,
circle: self.stack.clone(),
}),
)
} else if self.assignments.contains_key(variable) {
self.resolve_assignment(variable)
} else {
Err(name.token().error(UndefinedVariable { variable }))
Err(name.token.error(UndefinedVariable { variable }))
}
}
Expression::Call { thunk } => match thunk {
Expand Down
35 changes: 21 additions & 14 deletions src/compiler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ impl Compiler {
let mut srcs: HashMap<PathBuf, &str> = HashMap::new();
let mut loaded = Vec::new();

let mut stack: Vec<(PathBuf, u32)> = Vec::new();
stack.push((root.into(), 0));
let mut stack = Vec::new();
stack.push(Source::root(root));

while let Some((current, depth)) = stack.pop() {
let (relative, src) = loader.load(root, &current)?;
while let Some(current) = stack.pop() {
let (relative, src) = loader.load(root, &current.path)?;
loaded.push(relative.into());
let tokens = Lexer::lex(relative, src)?;
let mut ast = Parser::parse(depth, &current, &tokens)?;
let mut ast = Parser::parse(&current.path, &current.namepath, current.depth, &tokens)?;

paths.insert(current.clone(), relative.into());
srcs.insert(current.clone(), src);
paths.insert(current.path.clone(), relative.into());
srcs.insert(current.path.clone(), src);

for item in &mut ast.items {
match item {
Expand All @@ -39,7 +39,7 @@ impl Compiler {
});
}

let parent = current.parent().unwrap();
let parent = current.path.parent().unwrap();

let import = if let Some(relative) = relative {
let path = parent.join(Self::expand_tilde(&relative.cooked)?);
Expand All @@ -55,10 +55,13 @@ impl Compiler {

if let Some(import) = import {
if srcs.contains_key(&import) {
return Err(Error::CircularImport { current, import });
return Err(Error::CircularImport {
current: current.path,
import,
});
}
*absolute = Some(import.clone());
stack.push((import, depth + 1));
stack.push(current.module(*name, import));
} else if !*optional {
return Err(Error::MissingModuleFile { module: *name });
}
Expand All @@ -70,17 +73,21 @@ impl Compiler {
path,
} => {
let import = current
.path
.parent()
.unwrap()
.join(Self::expand_tilde(&relative.cooked)?)
.lexiclean();

if import.is_file() {
if srcs.contains_key(&import) {
return Err(Error::CircularImport { current, import });
return Err(Error::CircularImport {
current: current.path,
import,
});
}
*absolute = Some(import.clone());
stack.push((import, depth + 1));
stack.push(current.import(import));
} else if !*optional {
return Err(Error::MissingImportFile { path: *path });
}
Expand All @@ -89,7 +96,7 @@ impl Compiler {
}
}

asts.insert(current.clone(), ast.clone());
asts.insert(current.path, ast.clone());
}

let justfile = Analyzer::analyze(&loaded, &paths, &asts, root)?;
Expand Down Expand Up @@ -155,7 +162,7 @@ impl Compiler {
#[cfg(test)]
pub(crate) fn test_compile(src: &str) -> CompileResult<Justfile> {
let tokens = Lexer::test_lex(src)?;
let ast = Parser::parse(0, &PathBuf::new(), &tokens)?;
let ast = Parser::parse(&PathBuf::new(), &Namepath::default(), 0, &tokens)?;
let root = PathBuf::from("justfile");
let mut asts: HashMap<PathBuf, Ast> = HashMap::new();
asts.insert(root.clone(), ast);
Expand Down
4 changes: 2 additions & 2 deletions src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ impl<'src> Error<'src> {
fn context(&self) -> Option<Token<'src>> {
match self {
Self::AmbiguousModuleFile { module, .. } | Self::MissingModuleFile { module, .. } => {
Some(module.token())
Some(module.token)
}
Self::Backtick { token, .. } => Some(*token),
Self::Compile { compile_error } => Some(compile_error.context()),
Self::FunctionCall { function, .. } => Some(function.token()),
Self::FunctionCall { function, .. } => Some(function.token),
Self::MissingImportFile { path } => Some(*path),
_ => None,
}
Expand Down
6 changes: 3 additions & 3 deletions src/evaluator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
config: &'run Config,
dotenv: &'run BTreeMap<String, String>,
parameters: &[Parameter<'src>],
arguments: &[&str],
arguments: &[String],
scope: &'run Scope<'src, 'run>,
settings: &'run Settings,
search: &'run Search,
Expand Down Expand Up @@ -289,13 +289,13 @@ impl<'src, 'run> Evaluator<'src, 'run> {
}
} else if parameter.kind.is_variadic() {
for value in rest {
positional.push((*value).to_owned());
positional.push(value.clone());
}
let value = rest.to_vec().join(" ");
rest = &[];
value
} else {
let value = rest[0].to_owned();
let value = rest[0].clone();
positional.push(value.clone());
rest = &rest[1..];
value
Expand Down
42 changes: 14 additions & 28 deletions src/justfile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl<'src> Justfile<'src> {
});
}

let mut ran = BTreeSet::new();
let mut ran = Ran::default();
for invocation in invocations {
let context = RecipeContext {
settings: invocation.settings,
Expand All @@ -283,7 +283,12 @@ impl<'src> Justfile<'src> {
Self::run_recipe(
&context,
invocation.recipe,
&invocation.arguments,
&invocation
.arguments
.iter()
.copied()
.map(str::to_string)
.collect::<Vec<String>>(),
&dotenv,
search,
&mut ran,
Expand Down Expand Up @@ -399,17 +404,12 @@ impl<'src> Justfile<'src> {
fn run_recipe(
context: &RecipeContext<'src, '_>,
recipe: &Recipe<'src>,
arguments: &[&str],
arguments: &[String],
dotenv: &BTreeMap<String, String>,
search: &Search,
ran: &mut BTreeSet<Vec<String>>,
ran: &mut Ran<'src>,
) -> RunResult<'src> {
let mut invocation = vec![recipe.name().to_owned()];
for argument in arguments {
invocation.push((*argument).to_string());
}

if ran.contains(&invocation) {
if ran.has_run(&recipe.namepath, arguments) {
return Ok(());
}

Expand Down Expand Up @@ -440,20 +440,13 @@ impl<'src> Justfile<'src> {
.map(|argument| evaluator.evaluate_expression(argument))
.collect::<RunResult<Vec<String>>>()?;

Self::run_recipe(
context,
recipe,
&arguments.iter().map(String::as_ref).collect::<Vec<&str>>(),
dotenv,
search,
ran,
)?;
Self::run_recipe(context, recipe, &arguments, dotenv, search, ran)?;
}

recipe.run(context, dotenv, scope.child(), search, &positional)?;

{
let mut ran = BTreeSet::new();
let mut ran = Ran::default();

for Dependency { recipe, arguments } in recipe.dependencies.iter().skip(recipe.priors) {
let mut evaluated = Vec::new();
Expand All @@ -462,18 +455,11 @@ impl<'src> Justfile<'src> {
evaluated.push(evaluator.evaluate_expression(argument)?);
}

Self::run_recipe(
context,
recipe,
&evaluated.iter().map(String::as_ref).collect::<Vec<&str>>(),
dotenv,
search,
&mut ran,
)?;
Self::run_recipe(context, recipe, &evaluated, dotenv, search, &mut ran)?;
}
}

ran.insert(invocation);
ran.ran(&recipe.namepath, arguments.to_vec());
Ok(())
}

Expand Down
26 changes: 15 additions & 11 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ pub(crate) use {
fragment::Fragment, function::Function, function_context::FunctionContext,
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, name::Name, ordinal::Ordinal, output::output,
output_error::OutputError, parameter::Parameter, parameter_kind::ParameterKind, parser::Parser,
platform::Platform, platform_interface::PlatformInterface, position::Position,
positional::Positional, range_ext::RangeExt, recipe::Recipe, recipe_context::RecipeContext,
recipe_resolver::RecipeResolver, scope::Scope, search::Search, search_config::SearchConfig,
search_error::SearchError, set::Set, setting::Setting, settings::Settings, shebang::Shebang,
shell::Shell, show_whitespace::ShowWhitespace, 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, use_color::UseColor, variables::Variables,
verbosity::Verbosity, warning::Warning,
load_dotenv::load_dotenv, loader::Loader, 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_context::RecipeContext, recipe_resolver::RecipeResolver, scope::Scope, search::Search,
search_config::SearchConfig, search_error::SearchError, set::Set, setting::Setting,
settings::Settings, shebang::Shebang, shell::Shell, show_whitespace::ShowWhitespace,
source::Source, 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,
use_color::UseColor, variables::Variables, verbosity::Verbosity, warning::Warning,
},
std::{
cmp,
Expand All @@ -47,6 +47,7 @@ pub(crate) use {
io::{self, Cursor, Write},
iter::{self, FromIterator},
mem,
ops::Deref,
ops::{Index, Range, RangeInclusive},
path::{self, Path, PathBuf},
process::{self, Command, ExitStatus, Stdio},
Expand Down Expand Up @@ -149,6 +150,7 @@ mod list;
mod load_dotenv;
mod loader;
mod name;
mod namepath;
mod ordinal;
mod output;
mod output_error;
Expand All @@ -159,6 +161,7 @@ mod platform;
mod platform_interface;
mod position;
mod positional;
mod ran;
mod range_ext;
mod recipe;
mod recipe_context;
Expand All @@ -174,6 +177,7 @@ mod settings;
mod shebang;
mod shell;
mod show_whitespace;
mod source;
mod string_kind;
mod string_literal;
mod subcommand;
Expand Down
Loading

0 comments on commit 1ea5e6a

Please sign in to comment.