Skip to content

Commit 19327d7

Browse files
committed
fix(alias): place generated args correctly for run wrappers
cargo-fc generates Cargo args (--target, --no-default-features, --features=..., --message-format) per invocation. For aliases that expand through `cargo run -- ...` wrappers these were placed and detected incorrectly: generated args could land after the wrapper's `--` and reach the wrapped program instead of Cargo, and wrapper/program flags named like Cargo's were mistaken for cargo-fc input. Extract the forwarded-arg splitting and generated-arg placement into invocation_args.rs and: - Insert generated args before Cargo's `--`, but preserve an alias-defined `--` boundary so a wrapped command (e.g. `lint = "run --package wrapper -- lint"`) receives them. - Detect explicit `--target` and `--message-format` only on Cargo's own flags, so wrapper/program flags no longer suppress diagnostics mode or skip target injection. - Keep user program arguments after `--` out of target and feature-matrix planning.
1 parent 7d65205 commit 19327d7

7 files changed

Lines changed: 985 additions & 302 deletions

File tree

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,25 @@
77
- Fixed `--aggregate-targets` for aliases that expand through `cargo run -- ...`
88
wrappers, avoiding a misleading direct-`run` fallback note when target args
99
are passed to the wrapped command.
10+
- Fixed `--diagnostics-only` and `--dedupe` for aliases that expand through
11+
`cargo run -- ...` wrappers so the generated `--message-format` argument is
12+
passed to the wrapped command instead of the wrapper package.
13+
- Fixed run aliases such as `serve = "run --package app"` so user program
14+
arguments after `--` do not make cargo-fc pass generated matrix arguments to
15+
the program instead of Cargo.
16+
- Fixed explicit `--target` detection for aliases that expand through
17+
`cargo run -- ...` wrappers.
18+
- Fixed alias-defined wrapper `cargo run` flags such as `--target` and
19+
`--features` so they continue to configure the wrapper package instead of
20+
being treated as cargo-fc target planning or target-package feature matrix
21+
input.
22+
- Fixed diagnostics suppression so only Cargo's actual `--message-format` flag
23+
disables cargo-fc diagnostics mode; similarly named wrapper/program flags are
24+
left alone.
25+
- Fixed wrapper-package `--message-format` flags so they do not suppress
26+
diagnostics mode for the wrapped command.
27+
- Fixed aliases whose wrapped command has its own `--` separator so generated
28+
cargo-fc args are inserted before the wrapped command's program arguments.
1029

1130
## [0.2.1]
1231

src/cargo_alias.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ const MAX_ALIAS_EXPANSIONS: usize = 50;
2020
pub(crate) struct AliasExpansion {
2121
pub(crate) args: Vec<String>,
2222
pub(crate) expanded: bool,
23+
/// Whether a resolved alias body, not the user's trailing args, contributed
24+
/// a `--` separator. This is true for
25+
/// `lint = "run --package wrapper -- lint"` and false for
26+
/// `serve = "run --package app"` invoked as `cargo fc serve -- arg`.
27+
pub(crate) alias_provided_double_dash: bool,
2328
}
2429

2530
#[derive(Debug, Deserialize)]
@@ -108,6 +113,7 @@ pub(crate) fn expand_aliases_with_info(args: Vec<String>, workspace_root: &Path)
108113
return AliasExpansion {
109114
args,
110115
expanded: false,
116+
alias_provided_double_dash: false,
111117
};
112118
};
113119
match args.get(idx) {
@@ -116,6 +122,7 @@ pub(crate) fn expand_aliases_with_info(args: Vec<String>, workspace_root: &Path)
116122
return AliasExpansion {
117123
args,
118124
expanded: false,
125+
alias_provided_double_dash: false,
119126
};
120127
}
121128
}
@@ -124,6 +131,7 @@ pub(crate) fn expand_aliases_with_info(args: Vec<String>, workspace_root: &Path)
124131
return AliasExpansion {
125132
args,
126133
expanded: false,
134+
alias_provided_double_dash: false,
127135
};
128136
}
129137

@@ -133,6 +141,7 @@ pub(crate) fn expand_aliases_with_info(args: Vec<String>, workspace_root: &Path)
133141
let mut rest: Vec<String> = args.get(idx..).unwrap_or_default().to_vec();
134142
let mut expansions = 0;
135143
let mut changed = false;
144+
let mut alias_provided_double_dash = false;
136145
for _ in 0..MAX_ALIAS_EXPANSIONS {
137146
let Some(token) = rest.first().cloned() else {
138147
break;
@@ -149,6 +158,7 @@ pub(crate) fn expand_aliases_with_info(args: Vec<String>, workspace_root: &Path)
149158
break;
150159
}
151160
let mut expanded = expansion.clone();
161+
alias_provided_double_dash |= expanded.iter().any(|arg| arg == "--");
152162
expanded.extend_from_slice(rest.get(1..).unwrap_or_default());
153163
rest = expanded;
154164
expansions += 1;
@@ -167,12 +177,14 @@ pub(crate) fn expand_aliases_with_info(args: Vec<String>, workspace_root: &Path)
167177
);
168178
rest = original_rest;
169179
changed = false;
180+
alias_provided_double_dash = false;
170181
}
171182

172183
head.extend(rest);
173184
AliasExpansion {
174185
args: head,
175186
expanded: changed,
187+
alias_provided_double_dash,
176188
}
177189
}
178190

@@ -277,6 +289,31 @@ mod tests {
277289
Ok(())
278290
}
279291

292+
#[test]
293+
fn tracks_only_alias_provided_double_dash() -> eyre::Result<()> {
294+
let tmp = TempDir::new()?;
295+
write(
296+
&tmp,
297+
"[alias]\nwrapper = \"run --package wrapper -- lint\"\nserve = \"run --package app\"\n",
298+
)?;
299+
300+
let wrapper = expand_aliases_with_info(vec!["wrapper".to_string()], tmp.path());
301+
let serve = expand_aliases_with_info(
302+
vec![
303+
"serve".to_string(),
304+
"--".to_string(),
305+
"program-arg".to_string(),
306+
],
307+
tmp.path(),
308+
);
309+
310+
// Only separators from alias bodies select after-separator placement;
311+
// user-provided trailing args stay program argv for a normal run alias.
312+
assert!(wrapper.alias_provided_double_dash);
313+
assert!(!serve.alias_provided_double_dash);
314+
Ok(())
315+
}
316+
280317
#[test]
281318
fn leaves_builtin_subcommand_untouched() -> eyre::Result<()> {
282319
let tmp = TempDir::new()?;

src/cli.rs

Lines changed: 99 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ trait ArgumentParser {
5656
///
5757
/// When `has_value` is `true`, this matches `--flag value` and
5858
/// `--flag=value` forms and returns the value part. When `has_value` is
59-
/// `false`, it matches bare flags like `--flag`.
59+
/// `false`, it matches bare flags like `--flag`. In both modes, `--`
60+
/// remains the forwarded-argument boundary and is never consumed as a flag
61+
/// value.
6062
fn get_all(&self, arg: &str, has_value: bool)
6163
-> Vec<(std::ops::RangeInclusive<usize>, String)>;
6264
}
@@ -73,13 +75,13 @@ impl ArgumentParser for Vec<String> {
7375
break;
7476
}
7577
match (a, self.get(idx + 1)) {
76-
(key, Some(value)) if key == arg && has_value => {
78+
(key, Some(value)) if key == arg && has_value && value != "--" => {
7779
matched.push((idx..=idx + 1, value.clone()));
7880
}
7981
(key, _) if key == arg && !has_value => {
8082
matched.push((idx..=idx, key.clone()));
8183
}
82-
(key, _) if key.starts_with(&format!("{arg}=")) => {
84+
(key, _) if has_value && key.starts_with(&format!("{arg}=")) => {
8385
let value = key.trim_start_matches(&format!("{arg}="));
8486
matched.push((idx..=idx, value.to_string()));
8587
}
@@ -164,6 +166,9 @@ pub(crate) fn subcommand_token_index(args: &[impl AsRef<str>]) -> Option<usize>
164166
for (idx, arg) in args.iter().map(AsRef::as_ref).enumerate() {
165167
if skip_next {
166168
skip_next = false;
169+
if arg == "--" {
170+
return None;
171+
}
167172
continue;
168173
}
169174
if arg == "--" {
@@ -625,6 +630,9 @@ pub fn parse_arguments(bin_name: &str) -> eyre::Result<(Options, Vec<String>)> {
625630
..Options::default()
626631
};
627632

633+
// Every cargo-fc option extracted below must use `get_all` or `mark_flag`.
634+
// Those helpers stop at the first `--`, so `cargo fc run -- --help` keeps
635+
// `--help` as a binary argument instead of turning into cargo-fc help.
628636
// Extract path to manifest to operate on
629637
for (span, manifest_path) in args.get_all("--manifest-path", true) {
630638
let manifest_path = PathBuf::from(manifest_path);
@@ -836,15 +844,71 @@ mod test {
836844
let args = vec![
837845
"run".to_string(),
838846
"--".to_string(),
847+
"--manifest-path".to_string(),
848+
"Cargo.toml".to_string(),
849+
"--package".to_string(),
850+
"pkg".to_string(),
851+
"-p".to_string(),
852+
"pkg".to_string(),
853+
"--exclude-package".to_string(),
854+
"pkg".to_string(),
839855
"--help".to_string(),
840856
"matrix".to_string(),
857+
"--pretty".to_string(),
841858
"--driver".to_string(),
842859
"cross".to_string(),
860+
"--only-packages-with-lib-target".to_string(),
861+
"--pedantic".to_string(),
862+
"--errors-only".to_string(),
863+
"--packages-only".to_string(),
864+
"--diagnostics-only".to_string(),
865+
"--fail-fast".to_string(),
866+
"--no-prune-implied".to_string(),
867+
"--show-pruned".to_string(),
868+
"--aggregate-targets".to_string(),
869+
"--no-targets".to_string(),
870+
"--install-missing-targets".to_string(),
871+
"--dedupe".to_string(),
872+
"--dedup".to_string(),
873+
"--summary-only".to_string(),
874+
"--summary".to_string(),
875+
"--silent".to_string(),
876+
"--workspace".to_string(),
843877
];
844878

845-
assert!(args.get_all("--help", false).is_empty());
846-
assert!(args.get_all("matrix", false).is_empty());
847-
assert!(args.get_all("--driver", true).is_empty());
879+
for flag in [
880+
"--manifest-path",
881+
"--package",
882+
"-p",
883+
"--exclude-package",
884+
"--driver",
885+
] {
886+
assert!(args.get_all(flag, true).is_empty(), "{flag}");
887+
}
888+
for flag in [
889+
"--help",
890+
"matrix",
891+
"--pretty",
892+
"--only-packages-with-lib-target",
893+
"--pedantic",
894+
"--errors-only",
895+
"--packages-only",
896+
"--diagnostics-only",
897+
"--fail-fast",
898+
"--no-prune-implied",
899+
"--show-pruned",
900+
"--aggregate-targets",
901+
"--no-targets",
902+
"--install-missing-targets",
903+
"--dedupe",
904+
"--dedup",
905+
"--summary-only",
906+
"--summary",
907+
"--silent",
908+
"--workspace",
909+
] {
910+
assert!(args.get_all(flag, false).is_empty(), "{flag}");
911+
}
848912
}
849913

850914
#[test]
@@ -863,6 +927,35 @@ mod test {
863927
assert_eq!(matches[0].1, "cargo");
864928
}
865929

930+
#[test]
931+
fn argument_parser_does_not_treat_inline_values_as_boolean_flags() {
932+
let args = vec![
933+
"--summary-only=false".to_string(),
934+
"--driver=cargo".to_string(),
935+
];
936+
937+
assert!(args.get_all("--summary-only", false).is_empty());
938+
let matches = args.get_all("--driver", true);
939+
assert_eq!(matches.len(), 1);
940+
assert_eq!(matches[0].1, "cargo");
941+
}
942+
943+
#[test]
944+
fn argument_parser_does_not_consume_double_dash_as_value() {
945+
let args = vec![
946+
"--driver".to_string(),
947+
"--".to_string(),
948+
"program-arg".to_string(),
949+
];
950+
951+
assert!(args.get_all("--driver", true).is_empty());
952+
}
953+
954+
#[test]
955+
fn cargo_subcommand_token_stops_at_double_dash_after_value_flag() {
956+
sim_assert_eq!(cargo_subcommand_token(&["--config", "--", "clippy"]), None);
957+
}
958+
866959
#[test]
867960
fn cargo_subcommand_treats_unknown_aliases_as_other() {
868961
sim_assert_eq!(cargo_subcommand(&["lint"]), CargoSubcommand::Other);

0 commit comments

Comments
 (0)