diff --git a/nextgen/config/src/inherited_tasks_config.rs b/nextgen/config/src/inherited_tasks_config.rs index 842ec202244..9be70a8c52c 100644 --- a/nextgen/config/src/inherited_tasks_config.rs +++ b/nextgen/config/src/inherited_tasks_config.rs @@ -1,6 +1,6 @@ use crate::language_platform::{LanguageType, PlatformType}; use crate::project::{validate_deps, TaskConfig}; -use crate::project_config::{validate_tasks, ProjectType}; +use crate::project_config::ProjectType; use crate::shapes::InputPath; use crate::validate::check_yml_extension; use moon_common::path::standardize_separators; @@ -51,7 +51,7 @@ cacheable!( #[setting(merge = merge::append_vec)] pub implicit_inputs: Vec, - #[setting(nested, merge = merge::merge_btreemap, validate = validate_tasks)] + #[setting(nested, merge = merge::merge_btreemap)] pub tasks: BTreeMap, } ); diff --git a/nextgen/config/src/project_config.rs b/nextgen/config/src/project_config.rs index d49e0c56455..4aa7bd6847a 100644 --- a/nextgen/config/src/project_config.rs +++ b/nextgen/config/src/project_config.rs @@ -19,25 +19,6 @@ fn validate_channel(value: &str, _data: &D, _ctx: &C) -> Result<(), Valida Ok(()) } -pub fn validate_tasks( - tasks: &BTreeMap, - _data: &D, - _ctx: &C, -) -> Result<(), ValidateError> { - for (id, config) in tasks { - if let Some(extends_from) = &config.extends { - if !tasks.contains_key(extends_from) { - return Err(ValidateError::new(format!( - "task {} is extending an unknown task {}", - id, extends_from - ))); - } - } - } - - Ok(()) -} - derive_enum!( #[derive(ConfigEnum, Copy, Default)] pub enum ProjectType { @@ -108,7 +89,7 @@ cacheable!( pub tags: Vec, - #[setting(nested, validate = validate_tasks)] + #[setting(nested)] pub tasks: BTreeMap, #[setting(nested)] diff --git a/nextgen/config/tests/project_config_test.rs b/nextgen/config/tests/project_config_test.rs index f0f34b2d52d..4d35e1f8076 100644 --- a/nextgen/config/tests/project_config_test.rs +++ b/nextgen/config/tests/project_config_test.rs @@ -578,23 +578,6 @@ tasks: assert!(config.tasks.contains_key("base")); assert!(config.tasks.contains_key("extender")); } - - #[test] - #[should_panic(expected = "task extender is extending an unknown task unknown")] - fn errors_if_extending_unknown_task() { - test_load_config( - CONFIG_PROJECT_FILENAME, - r" -tasks: - base: - command: 'base' - extender: - extends: 'unknown' - args: '--more' -", - |path| ProjectConfig::load_from(path, "."), - ); - } } mod toolchain { diff --git a/nextgen/task-builder/src/lib.rs b/nextgen/task-builder/src/lib.rs index ec931c60acf..1ca4da4d144 100644 --- a/nextgen/task-builder/src/lib.rs +++ b/nextgen/task-builder/src/lib.rs @@ -1,3 +1,5 @@ mod tasks_builder; +mod tasks_builder_error; pub use tasks_builder::*; +pub use tasks_builder_error::*; diff --git a/nextgen/task-builder/src/tasks_builder.rs b/nextgen/task-builder/src/tasks_builder.rs index 2ff7582ae0a..ae4058348a0 100644 --- a/nextgen/task-builder/src/tasks_builder.rs +++ b/nextgen/task-builder/src/tasks_builder.rs @@ -1,5 +1,6 @@ #![allow(dead_code)] +use crate::tasks_builder_error::TasksBuilderError; use moon_args::split_args; use moon_common::{color, Id}; use moon_config::{ @@ -23,21 +24,43 @@ struct ConfigChain<'proj> { // This is a standalone function as recursive closures are not possible! fn extract_config<'builder, 'proj>( - task_id: &'builder Id, - tasks_map: &'builder FxHashMap<&'proj Id, &'proj TaskConfig>, configs: &'builder mut Vec>, - global: bool, -) { - if let Some(config) = tasks_map.get(task_id) { + task_id: &'builder Id, + tasks: &'builder FxHashMap<&'proj Id, &'proj TaskConfig>, + extendable_tasks: &'builder FxHashMap<&'proj Id, &'proj TaskConfig>, + inherited: bool, + extended: bool, +) -> miette::Result<()> { + let config = if extended { + extendable_tasks.get(task_id) + } else { + tasks.get(task_id) + }; + + if let Some(config) = config { if let Some(extend_task_id) = &config.extends { - extract_config(extend_task_id, tasks_map, configs, global); + if !extendable_tasks.contains_key(extend_task_id) { + return Err(TasksBuilderError::UnknownExtendsSource { + source_id: task_id.to_owned(), + target_id: extend_task_id.to_owned(), + } + .into()); + } + + extract_config( + configs, + extend_task_id, + tasks, + extendable_tasks, + inherited, + true, + )?; } - configs.push(ConfigChain { - config, - inherited: global, - }); + configs.push(ConfigChain { config, inherited }); } + + Ok(()) } #[derive(Debug)] @@ -230,7 +253,7 @@ impl<'proj> TasksBuilder<'proj> { ); let mut task = Task::default(); - let chain = self.get_config_inherit_chain(id); + let chain = self.get_config_inherit_chain(id)?; // Determine command and args before building options and the task, // as we need to figure out if we're running in local mode or not. @@ -413,7 +436,7 @@ impl<'proj> TasksBuilder<'proj> { }; let configs = self - .get_config_inherit_chain(id) + .get_config_inherit_chain(id)? .iter() .map(|link| &link.config.options) .collect::>(); @@ -589,13 +612,34 @@ impl<'proj> TasksBuilder<'proj> { Ok((command, args)) } - fn get_config_inherit_chain(&self, id: &Id) -> Vec { + fn get_config_inherit_chain(&self, id: &Id) -> miette::Result> { let mut configs = vec![]; - extract_config(id, &self.global_tasks, &mut configs, true); - extract_config(id, &self.local_tasks, &mut configs, false); - - configs + let mut extendable_tasks = FxHashMap::default(); + extendable_tasks.extend(&self.global_tasks); + extendable_tasks.extend(&self.local_tasks); + + // Inherit all global first + extract_config( + &mut configs, + id, + &self.global_tasks, + &extendable_tasks, + true, + false, + )?; + + // Then all local + extract_config( + &mut configs, + id, + &self.local_tasks, + &extendable_tasks, + false, + false, + )?; + + Ok(configs) } fn apply_filters_to_deps(&self, deps: Vec) -> Vec { diff --git a/nextgen/task-builder/src/tasks_builder_error.rs b/nextgen/task-builder/src/tasks_builder_error.rs new file mode 100644 index 00000000000..8f3e20afdbd --- /dev/null +++ b/nextgen/task-builder/src/tasks_builder_error.rs @@ -0,0 +1,14 @@ +use miette::Diagnostic; +use moon_common::{Id, Style, Stylize}; +use thiserror::Error; + +#[derive(Error, Debug, Diagnostic)] +pub enum TasksBuilderError { + #[diagnostic(code(task_builder::unknown_extends))] + #[error( + "Task {} is extending an unknown task {}.", + .source_id.style(Style::Id), + .target_id.style(Style::Id), + )] + UnknownExtendsSource { source_id: Id, target_id: Id }, +} diff --git a/nextgen/task-builder/tests/__fixtures__/builder/extends-unknown/moon.yml b/nextgen/task-builder/tests/__fixtures__/builder/extends-unknown/moon.yml new file mode 100644 index 00000000000..0341d87fed7 --- /dev/null +++ b/nextgen/task-builder/tests/__fixtures__/builder/extends-unknown/moon.yml @@ -0,0 +1,5 @@ +tags: [extends] + +tasks: + base: + extends: unknown diff --git a/nextgen/task-builder/tests/__fixtures__/builder/extends/moon.yml b/nextgen/task-builder/tests/__fixtures__/builder/extends/moon.yml index 2ba37affad0..5fc008309ca 100644 --- a/nextgen/task-builder/tests/__fixtures__/builder/extends/moon.yml +++ b/nextgen/task-builder/tests/__fixtures__/builder/extends/moon.yml @@ -36,8 +36,6 @@ tasks: extends: base local: false - # Tests global inheritance - local-base: inputs: - 'local-base' @@ -52,3 +50,9 @@ tasks: - 'local-extender' options: mergeArgs: 'prepend' + + local-extends-global: + extends: 'global-base' + inputs: + - 'local-extender' + diff --git a/nextgen/task-builder/tests/tasks_builder_test.rs b/nextgen/task-builder/tests/tasks_builder_test.rs index 217d78fc5ea..76e54401f00 100644 --- a/nextgen/task-builder/tests/tasks_builder_test.rs +++ b/nextgen/task-builder/tests/tasks_builder_test.rs @@ -1212,6 +1212,15 @@ mod tasks_builder { mod extending { use super::*; + #[tokio::test] + #[should_panic(expected = "Task base is extending an unknown task unknown.")] + async fn errors_for_unknown_extend_task() { + let sandbox = create_sandbox("builder"); + let tasks = build_tasks(sandbox.path(), "extends-unknown/moon.yml").await; + + tasks.get("base").unwrap(); + } + #[tokio::test] async fn handles_args() { let sandbox = create_sandbox("builder"); @@ -1286,6 +1295,24 @@ mod tasks_builder { assert_eq!(task.options.retry_count, 3); } + #[tokio::test] + async fn can_extend_a_global_from_local() { + let sandbox = create_sandbox("builder"); + let tasks = build_tasks(sandbox.path(), "extends/moon.yml").await; + let task = tasks.get("local-extends-global").unwrap(); + + assert_eq!(task.command, "global-base"); + assert_eq!( + task.inputs, + vec![ + InputPath::ProjectFile("global-base".into()), + InputPath::ProjectFile("local-extender".into()), + InputPath::WorkspaceGlob(".moon/*.yml".into()), + InputPath::WorkspaceFile("global/tasks/tag-extends.yml".into()), + ] + ); + } + #[tokio::test] async fn can_create_extends_chains() { let sandbox = create_sandbox("builder"); diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 32db7728580..e6213b99683 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -16,6 +16,10 @@ - Updated `moon dep-graph` to support a task in closest project, similar to `moon run`. +#### 🐞 Fixes + +- Fixed an issue where local tasks could not extend global tasks using the `extends` setting. + ## 1.14.2 #### 🐞 Fixes