Skip to content

Commit f6aefb0

Browse files
committed
Allow cargo-fmt to handle formatting individual files.
Adds an additional command line option ``--src-file`` / ``-s`` Tests were added in ``src/cargo-fmt/test/mod.rs`` and integration tests were added to ``tests/cargo-fmt/main.rs``
1 parent ea199ba commit f6aefb0

File tree

3 files changed

+204
-3
lines changed

3 files changed

+204
-3
lines changed

src/cargo-fmt/main.rs

+64-3
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ pub struct Opts {
4444
#[structopt(short = "p", long = "package", value_name = "package")]
4545
packages: Vec<String>,
4646

47+
/// Specify a source file to format
48+
#[structopt(short = "s", long = "src-file", value_name = "src-file")]
49+
src_files: Vec<PathBuf>,
50+
4751
/// Specify path to Cargo.toml
4852
#[structopt(long = "manifest-path", value_name = "manifest-path")]
4953
manifest_path: Option<String>,
@@ -89,6 +93,16 @@ fn execute() -> i32 {
8993

9094
let opts = Opts::from_iter(args);
9195

96+
if !opts.src_files.is_empty() & !opts.packages.is_empty() {
97+
print_usage_to_stderr("cannot format source files and packages at the same time");
98+
return FAILURE;
99+
}
100+
101+
if !opts.src_files.is_empty() & opts.format_all {
102+
print_usage_to_stderr("cannot format all packages when specifying source files");
103+
return FAILURE;
104+
}
105+
92106
let verbosity = match (opts.verbose, opts.quiet) {
93107
(false, false) => Verbosity::Normal,
94108
(false, true) => Verbosity::Quiet,
@@ -314,17 +328,23 @@ pub enum CargoFmtStrategy {
314328
/// Format every packages and dependencies.
315329
All,
316330
/// Format packages that are specified by the command line argument.
317-
Some(Vec<String>),
331+
Packages(Vec<String>),
318332
/// Format the root packages only.
319333
Root,
334+
/// Format individual source files specified by the command line arguments.
335+
SourcFiles(Vec<PathBuf>),
320336
}
321337

322338
impl CargoFmtStrategy {
323339
pub fn from_opts(opts: &Opts) -> CargoFmtStrategy {
340+
if !opts.src_files.is_empty() {
341+
return CargoFmtStrategy::SourcFiles(opts.src_files.clone());
342+
}
343+
324344
match (opts.format_all, opts.packages.is_empty()) {
325345
(false, true) => CargoFmtStrategy::Root,
326346
(true, _) => CargoFmtStrategy::All,
327-
(false, false) => CargoFmtStrategy::Some(opts.packages.clone()),
347+
(false, false) => CargoFmtStrategy::Packages(opts.packages.clone()),
328348
}
329349
}
330350
}
@@ -341,9 +361,12 @@ fn get_targets(
341361
CargoFmtStrategy::All => {
342362
get_targets_recursive(manifest_path, &mut targets, &mut BTreeSet::new())?
343363
}
344-
CargoFmtStrategy::Some(ref hitlist) => {
364+
CargoFmtStrategy::Packages(ref hitlist) => {
345365
get_targets_with_hitlist(manifest_path, hitlist, &mut targets)?
346366
}
367+
CargoFmtStrategy::SourcFiles(ref src_files) => {
368+
get_targets_from_src_files(manifest_path, &src_files, &mut targets)?
369+
}
347370
}
348371

349372
if targets.is_empty() {
@@ -356,6 +379,44 @@ fn get_targets(
356379
}
357380
}
358381

382+
fn get_targets_from_src_files(
383+
manifest_path: Option<&Path>,
384+
src_files: &[PathBuf],
385+
targets: &mut BTreeSet<Target>,
386+
) -> Result<(), io::Error> {
387+
let metadata = get_cargo_metadata(manifest_path)?;
388+
let mut src_file_target_map: BTreeMap<PathBuf, (String, String)> = BTreeMap::new();
389+
390+
// Map each targt to it's parent directory.
391+
for target in metadata.packages.iter().map(|p| p.targets.iter()).flatten() {
392+
let path = fs::canonicalize(&target.src_path)?
393+
.parent()
394+
.expect("Target src_path should have a parent directory")
395+
.to_owned();
396+
397+
src_file_target_map.insert(path, (target.kind[0].clone(), target.edition.clone()));
398+
}
399+
400+
// Given the path to a source file, traverse it's ancestors until we find one that's
401+
// in the target map we just constructed, at which point we add it to the targets set.
402+
// This ensures that we only support formatting arbitrary files within a package.
403+
for file_path in src_files {
404+
let canonicalize = fs::canonicalize(file_path)?;
405+
for path in canonicalize.ancestors() {
406+
if let Some((kind, edition)) = src_file_target_map.get(path) {
407+
targets.insert(Target {
408+
path: file_path.to_owned(),
409+
kind: kind.clone(),
410+
edition: edition.clone(),
411+
});
412+
break;
413+
}
414+
}
415+
}
416+
417+
Ok(())
418+
}
419+
359420
fn get_targets_root_only(
360421
manifest_path: Option<&Path>,
361422
targets: &mut BTreeSet<Target>,

src/cargo-fmt/test/mod.rs

+72
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ fn default_options() {
1616
assert_eq!(false, o.format_all);
1717
assert_eq!(None, o.manifest_path);
1818
assert_eq!(None, o.message_format);
19+
assert!(o.src_files.is_empty());
1920
}
2021

2122
#[test]
@@ -27,6 +28,10 @@ fn good_options() {
2728
"p1",
2829
"-p",
2930
"p2",
31+
"-s",
32+
"a.rs",
33+
"-s",
34+
"b.rs",
3035
"--message-format",
3136
"short",
3237
"--check",
@@ -39,6 +44,13 @@ fn good_options() {
3944
assert_eq!(false, o.version);
4045
assert_eq!(true, o.check);
4146
assert_eq!(vec!["p1", "p2"], o.packages);
47+
assert_eq!(
48+
vec!["a.rs", "b.rs"],
49+
o.src_files
50+
.iter()
51+
.map(|p| p.display().to_string())
52+
.collect::<Vec<String>>()
53+
);
4254
assert_eq!(vec!["--edition", "2018"], o.rustfmt_options);
4355
assert_eq!(false, o.format_all);
4456
assert_eq!(Some(String::from("short")), o.message_format);
@@ -90,6 +102,20 @@ fn multiple_packages_one_by_one() {
90102
assert_eq!(3, o.packages.len());
91103
}
92104

105+
#[test]
106+
fn multiple_source_files_one_by_one() {
107+
let o = Opts::from_iter(&[
108+
"test",
109+
"-s",
110+
"src/a.rs",
111+
"--src-file",
112+
"src/b.rs",
113+
"-s",
114+
"src/c.rs",
115+
]);
116+
assert_eq!(3, o.src_files.len());
117+
}
118+
93119
#[test]
94120
fn multiple_packages_grouped() {
95121
let o = Opts::from_iter(&[
@@ -104,6 +130,20 @@ fn multiple_packages_grouped() {
104130
assert_eq!(4, o.packages.len());
105131
}
106132

133+
#[test]
134+
fn multiple_source_files_grouped() {
135+
let o = Opts::from_iter(&[
136+
"test",
137+
"--src-file",
138+
"src/a.rs",
139+
"src/b.rs",
140+
"-s",
141+
"src/c.rs",
142+
"src/d.rs",
143+
]);
144+
assert_eq!(4, o.src_files.len());
145+
}
146+
107147
#[test]
108148
fn empty_packages_1() {
109149
assert!(Opts::clap().get_matches_from_safe(&["test", "-p"]).is_err());
@@ -135,3 +175,35 @@ fn empty_packages_4() {
135175
.is_err()
136176
);
137177
}
178+
179+
#[test]
180+
fn empty_source_files_1() {
181+
assert!(Opts::clap().get_matches_from_safe(&["test", "-s"]).is_err());
182+
}
183+
184+
#[test]
185+
fn empty_source_files_2() {
186+
assert!(
187+
Opts::clap()
188+
.get_matches_from_safe(&["test", "-s", "--", "--check"])
189+
.is_err()
190+
);
191+
}
192+
193+
#[test]
194+
fn empty_source_files_3() {
195+
assert!(
196+
Opts::clap()
197+
.get_matches_from_safe(&["test", "-s", "--verbose"])
198+
.is_err()
199+
);
200+
}
201+
202+
#[test]
203+
fn empty_source_files_4() {
204+
assert!(
205+
Opts::clap()
206+
.get_matches_from_safe(&["test", "-s", "--check"])
207+
.is_err()
208+
);
209+
}

tests/cargo-fmt/main.rs

+68
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Integration tests for cargo-fmt.
22

33
use std::env;
4+
use std::path::PathBuf;
45
use std::process::Command;
56

67
/// Run the cargo-fmt executable and return its output.
@@ -71,3 +72,70 @@ fn rustfmt_help() {
7172
assert_that!(&["--", "-h"], contains("Format Rust code"));
7273
assert_that!(&["--", "--help=config"], contains("Configuration Options:"));
7374
}
75+
76+
fn extend_path<'a, P: AsRef<std::path::Path>>(root: P, relative_path: &str) -> PathBuf {
77+
let mut new_path = root.as_ref().to_owned();
78+
79+
for part in relative_path.split("/") {
80+
new_path = new_path.join(part)
81+
}
82+
83+
new_path
84+
}
85+
86+
#[test]
87+
fn specify_source_files_when_running_cargo_fmt() {
88+
// assumes tests are envoked from the root of the rustfmt project
89+
let rustfmt_root = env::current_dir().unwrap();
90+
let cargo_project_root = extend_path(
91+
rustfmt_root, "tests/cargo-fmt/source/workspaces/path-dep-above/e"
92+
);
93+
94+
let path =
95+
&extend_path(cargo_project_root, "src/main.rs")
96+
.display()
97+
.to_string();
98+
99+
// test that we can run cargo-fmt on a single src file
100+
assert_that!(&["-v", "--check", "-s", path], contains(path));
101+
}
102+
103+
104+
#[test]
105+
fn specify_multiple_source_files_in_a_workspace_when_running_cargo_fmt() {
106+
// assumes tests are envoked from the root of the rustfmt project
107+
let rustfmt_root = env::current_dir().unwrap();
108+
let cargo_workspace_root = extend_path(
109+
rustfmt_root, "tests/cargo-fmt/source/workspaces/path-dep-above/ws"
110+
);
111+
let workspace_path1 =
112+
&extend_path(&cargo_workspace_root, "a/src/main.rs")
113+
.display()
114+
.to_string();
115+
116+
let workspace_path2 =
117+
&extend_path(&cargo_workspace_root, "b/src/main.rs")
118+
.display()
119+
.to_string();
120+
121+
let args = ["-v", "--check", "-s", workspace_path1, workspace_path2];
122+
123+
let (stdout, _) = cargo_fmt(&args);
124+
125+
// test that we can run cargo-fmt on a multiple src files withing a cargo workspace
126+
assert!(stdout.contains(&format!("{:?}", workspace_path1)));
127+
assert!(stdout.contains(&format!("{:?}", workspace_path2)));
128+
}
129+
130+
131+
#[test]
132+
fn formatting_source_files_and_packages_at_the_same_time_is_not_supported() {
133+
let (_, stderr) = cargo_fmt(&["--check", "-s", "src/main.rs", "-p", "p1", "-p2"]);
134+
assert!(stderr.starts_with("cannot format source files and packages at the same time"))
135+
}
136+
137+
#[test]
138+
fn formatting_source_files_and_using_package_related_arguments_is_not_supported() {
139+
let (_, stderr) = cargo_fmt(&["--check", "--all", "-s", "src/main.rs"]);
140+
assert!(stderr.starts_with("cannot format all packages when specifying source files"))
141+
}

0 commit comments

Comments
 (0)