diff --git a/src/rust/engine/options/src/lib.rs b/src/rust/engine/options/src/lib.rs index 47400d05ec2..801a0a14aff 100644 --- a/src/rust/engine/options/src/lib.rs +++ b/src/rust/engine/options/src/lib.rs @@ -489,19 +489,30 @@ impl OptionParser { } derivation = Some(derivations); } + + // Removals from any source apply after adds from any source (but are themselves + // overridden by later replacements), so we collect them here and apply them later. + let mut removal_lists: Vec> = vec![]; + let mut highest_priority_source = Source::Default; for (source_type, source) in self.sources.iter() { if let Some(list_edits) = getter(source, id)? { highest_priority_source = source_type.clone(); for list_edit in list_edits { match list_edit.action { - ListEditAction::Replace => list = list_edit.items, + ListEditAction::Replace => { + list = list_edit.items; + removal_lists.clear(); + } ListEditAction::Add => list.extend(list_edit.items), - ListEditAction::Remove => remover(&mut list, &list_edit.items), + ListEditAction::Remove => removal_lists.push(list_edit.items), } } } } + for removals in removal_lists { + remover(&mut list, &removals); + } Ok(ListOptionValue { derivation, source: highest_priority_source, diff --git a/src/rust/engine/options/src/tests.rs b/src/rust/engine/options/src/tests.rs index 2de7ba1ace5..eb2e2e5e367 100644 --- a/src/rust/engine/options/src/tests.rs +++ b/src/rust/engine/options/src/tests.rs @@ -280,7 +280,7 @@ fn test_parse_list_options() { ); check( - vec![1, 3, 4], + vec![1, 3], vec![ (Source::Default, vec![replace(vec![0])]), (config_source(), vec![replace(vec![1, 2])]), @@ -290,7 +290,7 @@ fn test_parse_list_options() { vec![], vec![("PANTS_SCOPE_FOO", "+[3, 4]")], "[scope]\nfoo = [1, 2]", - "[scope]\nfoo = '-[2, 4]'", // 2 should be removed, but not 4, since env has precedence. + "[scope]\nfoo = '-[2, 4]'", ); check( @@ -330,6 +330,63 @@ fn test_parse_list_options() { "", "", ); + + // Filtering all instances of repeated values. + check( + vec![1, 2, 2, 4], + vec![ + (Source::Default, vec![replace(vec![0])]), + (config_source(), vec![add(vec![1, 2, 3, 2, 0, 3, 3, 4])]), + (Source::Env, vec![remove(vec![0, 3])]), + ], + vec![], + vec![("PANTS_SCOPE_FOO", "-[0, 3]")], + "[scope]\nfoo.add = [1, 2, 3, 2, 0, 3, 3, 4]", + "", + ); + + // Filtering a value even though it was appended again at a higher rank. + check( + vec![0, 2], + vec![ + (Source::Default, vec![replace(vec![0])]), + (config_source(), vec![add(vec![1, 2])]), + (Source::Env, vec![remove(vec![1])]), + (Source::Flag, vec![add(vec![1])]), + ], + vec!["--scope-foo=+[1]"], + vec![("PANTS_SCOPE_FOO", "-[1]")], + "[scope]\nfoo.add = [1, 2]", + "", + ); + + // Filtering a value even though it was appended again at the same rank. + check( + vec![0, 2], + vec![ + (Source::Default, vec![replace(vec![0])]), + (config_source(), vec![add(vec![1, 2])]), + (Source::Env, vec![remove(vec![1]), add(vec![1])]), + ], + vec![], + vec![("PANTS_SCOPE_FOO", "-[1],+[1]")], + "[scope]\nfoo.add = [1, 2]", + "", + ); + + // Overwriting cancels filters. + check( + vec![0], + vec![ + (Source::Default, vec![replace(vec![0])]), + (config_source(), vec![remove(vec![0])]), + (Source::Env, vec![replace(vec![0])]), + ], + vec![], + vec![("PANTS_SCOPE_FOO", "[0]")], + "[scope]\nfoo.remove = [0]", + "", + ); } #[test]