Skip to content

Commit 7c14191

Browse files
committed
Allow cargo-fmt to handle formatting individual files.
Adds an additional command line option ``--src-file`` / ``-s`` It should be noted that this does not allow cargo-fmt to format an arbitrary rust file. The file must be contained with a target specified in a Cargo.toml file. Adjustments were also made to prevent cargo-fmt from passing along certain arguments to rustfmt. For example, users can no longer pass rust files to rustfmt through cargo-fmt. Additionally, users cannot pass --edition to cargo-fmt, since editions are read from the Cargo.toml. Tests were added in ``src/cargo-fmt/test/mod.rs`` and integration tests were added to ``tests/cargo-fmt/main.rs``
1 parent a37d3ab commit 7c14191

File tree

3 files changed

+188
-3
lines changed

3 files changed

+188
-3
lines changed

src/cargo-fmt/main.rs

+89-3
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ pub struct Opts {
4949
)]
5050
packages: Vec<String>,
5151

52+
/// Specify a source file to format
53+
#[clap(short = 's', long = "src-file", value_name = "src-file")]
54+
src_file: Option<PathBuf>,
55+
5256
/// Specify path to Cargo.toml
5357
#[clap(long = "manifest-path", value_name = "manifest-path")]
5458
manifest_path: Option<String>,
@@ -94,6 +98,28 @@ fn execute() -> i32 {
9498

9599
let opts = Opts::parse_from(args);
96100

101+
if opts.src_file.is_some() & !opts.packages.is_empty() {
102+
print_usage_to_stderr("cannot format source files and packages at the same time");
103+
return FAILURE;
104+
}
105+
106+
if opts.src_file.is_some() & opts.format_all {
107+
print_usage_to_stderr("cannot format all packages when specifying source files");
108+
return FAILURE;
109+
}
110+
111+
if opts.rustfmt_options.iter().any(|s| s.ends_with(".rs")) {
112+
print_usage_to_stderr(
113+
"cannot pass rust files to rustfmt through cargo-fmt. Use '--src-file' instead",
114+
);
115+
return FAILURE;
116+
}
117+
118+
if opts.rustfmt_options.iter().any(|s| s.contains("--edition")) {
119+
print_usage_to_stderr("cannot pass '--edition' to rustfmt through cargo-fmt");
120+
return FAILURE;
121+
}
122+
97123
let verbosity = match (opts.verbose, opts.quiet) {
98124
(false, false) => Verbosity::Normal,
99125
(false, true) => Verbosity::Quiet,
@@ -319,17 +345,23 @@ pub enum CargoFmtStrategy {
319345
/// Format every packages and dependencies.
320346
All,
321347
/// Format packages that are specified by the command line argument.
322-
Some(Vec<String>),
348+
Packages(Vec<String>),
323349
/// Format the root packages only.
324350
Root,
351+
/// Format individual source files specified by the command line arguments.
352+
SourceFile(PathBuf),
325353
}
326354

327355
impl CargoFmtStrategy {
328356
pub fn from_opts(opts: &Opts) -> CargoFmtStrategy {
357+
if let Some(ref src_file) = opts.src_file {
358+
return CargoFmtStrategy::SourceFile(src_file.clone());
359+
}
360+
329361
match (opts.format_all, opts.packages.is_empty()) {
330362
(false, true) => CargoFmtStrategy::Root,
331363
(true, _) => CargoFmtStrategy::All,
332-
(false, false) => CargoFmtStrategy::Some(opts.packages.clone()),
364+
(false, false) => CargoFmtStrategy::Packages(opts.packages.clone()),
333365
}
334366
}
335367
}
@@ -346,9 +378,12 @@ fn get_targets(
346378
CargoFmtStrategy::All => {
347379
get_targets_recursive(manifest_path, &mut targets, &mut BTreeSet::new())?
348380
}
349-
CargoFmtStrategy::Some(ref hitlist) => {
381+
CargoFmtStrategy::Packages(ref hitlist) => {
350382
get_targets_with_hitlist(manifest_path, hitlist, &mut targets)?
351383
}
384+
CargoFmtStrategy::SourceFile(ref src_file) => {
385+
get_target_from_src_file(manifest_path, &src_file, &mut targets)?
386+
}
352387
}
353388

354389
if targets.is_empty() {
@@ -361,6 +396,57 @@ fn get_targets(
361396
}
362397
}
363398

399+
fn get_target_from_src_file(
400+
manifest_path: Option<&Path>,
401+
src_file: &PathBuf,
402+
targets: &mut BTreeSet<Target>,
403+
) -> Result<(), io::Error> {
404+
let metadata = get_cargo_metadata(manifest_path)?;
405+
406+
let get_target = |src_path: &Path| {
407+
metadata
408+
.packages
409+
.iter()
410+
.map(|p| p.targets.iter())
411+
.flatten()
412+
.filter(|t| {
413+
let kind = &t.kind[0];
414+
// to prevent formatting any arbitrary file within the root of our
415+
// project we special case the build.rs script, becuase it's parent
416+
// is the root of the project and we would always select the custom-build
417+
// target in the event that we couldn't find a better target to associate
418+
// with our file.
419+
if kind == "custom-build" && !src_path.ends_with("build.rs") {
420+
return false;
421+
}
422+
423+
if let Ok(target_path) = fs::canonicalize(&t.src_path) {
424+
let target_dir = target_path
425+
.parent()
426+
.expect("Target src_path should have a parent directory");
427+
src_path.starts_with(target_dir)
428+
} else {
429+
false
430+
}
431+
})
432+
.max_by(|t1, t2| {
433+
let t1_len = t1.src_path.components().count();
434+
let t2_len = t2.src_path.components().count();
435+
t1_len.cmp(&t2_len)
436+
})
437+
};
438+
439+
let canonicalize = fs::canonicalize(&src_file)?;
440+
if let Some(target) = get_target(&canonicalize) {
441+
targets.insert(Target {
442+
path: src_file.to_owned(),
443+
kind: target.kind[0].clone(),
444+
edition: target.edition.clone(),
445+
});
446+
}
447+
Ok(())
448+
}
449+
364450
fn get_targets_root_only(
365451
manifest_path: Option<&Path>,
366452
targets: &mut BTreeSet<Target>,

src/cargo-fmt/test/mod.rs

+60
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use super::*;
2+
use std::path::PathBuf;
23

34
mod message_format;
45
mod targets;
@@ -16,6 +17,7 @@ fn default_options() {
1617
assert_eq!(false, o.format_all);
1718
assert_eq!(None, o.manifest_path);
1819
assert_eq!(None, o.message_format);
20+
assert_eq!(None, o.src_file);
1921
}
2022

2123
#[test]
@@ -27,6 +29,8 @@ fn good_options() {
2729
"p1",
2830
"-p",
2931
"p2",
32+
"-s",
33+
"file.rs",
3034
"--message-format",
3135
"short",
3236
"--check",
@@ -39,6 +43,7 @@ fn good_options() {
3943
assert_eq!(false, o.version);
4044
assert_eq!(true, o.check);
4145
assert_eq!(vec!["p1", "p2"], o.packages);
46+
assert_eq!(Some(PathBuf::from("file.rs")), o.src_file);
4247
assert_eq!(vec!["--edition", "2018"], o.rustfmt_options);
4348
assert_eq!(false, o.format_all);
4449
assert_eq!(Some(String::from("short")), o.message_format);
@@ -90,6 +95,25 @@ fn multiple_packages_one_by_one() {
9095
assert_eq!(3, o.packages.len());
9196
}
9297

98+
#[test]
99+
fn cant_list_more_than_one_source_file() {
100+
assert!(
101+
Opts::command()
102+
.try_get_matches_from(&["test", "-s", "src/a.rs", "--src-file", "src/b.rs"])
103+
.is_err()
104+
);
105+
assert!(
106+
!Opts::command()
107+
.try_get_matches_from(&["test", "-s", "src/a.rs"])
108+
.is_err()
109+
);
110+
assert!(
111+
!Opts::command()
112+
.try_get_matches_from(&["test", "--src-file", "src/b.rs"])
113+
.is_err()
114+
);
115+
}
116+
93117
#[test]
94118
fn multiple_packages_grouped() {
95119
let o = Opts::parse_from(&[
@@ -139,3 +163,39 @@ fn empty_packages_4() {
139163
.is_err()
140164
);
141165
}
166+
167+
#[test]
168+
fn empty_source_files_1() {
169+
assert!(
170+
Opts::command()
171+
.try_get_matches_from(&["test", "-s"])
172+
.is_err()
173+
);
174+
}
175+
176+
#[test]
177+
fn empty_source_files_2() {
178+
assert!(
179+
Opts::command()
180+
.try_get_matches_from(&["test", "-s", "--", "--check"])
181+
.is_err()
182+
);
183+
}
184+
185+
#[test]
186+
fn empty_source_files_3() {
187+
assert!(
188+
Opts::command()
189+
.try_get_matches_from(&["test", "-s", "--verbose"])
190+
.is_err()
191+
);
192+
}
193+
194+
#[test]
195+
fn empty_source_files_4() {
196+
assert!(
197+
Opts::command()
198+
.try_get_matches_from(&["test", "-s", "--check"])
199+
.is_err()
200+
);
201+
}

tests/cargo-fmt/main.rs

+39
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,42 @@ fn cargo_fmt_out_of_line_test_modules() {
9696
assert!(stdout.contains(&format!("Diff in {}", path.display())))
9797
}
9898
}
99+
100+
#[test]
101+
fn cannot_pass_rust_files_to_rustfmt_through_caro_fmt() {
102+
let (_, stderr) = cargo_fmt(&["--", "src/main.rs"]);
103+
assert!(stderr.starts_with(
104+
"cannot pass rust files to rustfmt through cargo-fmt. Use '--src-file' instead"
105+
))
106+
}
107+
108+
#[test]
109+
fn cannot_pass_edition_to_rustfmt_through_caro_fmt() {
110+
let (_, stderr) = cargo_fmt(&["--", "--edition", "2021"]);
111+
assert!(stderr.starts_with("cannot pass '--edition' to rustfmt through cargo-fmt"))
112+
}
113+
114+
#[test]
115+
fn specify_source_files_when_running_cargo_fmt() {
116+
let path = "tests/cargo-fmt/source/workspaces/path-dep-above/e/src/main.rs";
117+
// test that we can run cargo-fmt on a single src file
118+
assert_that!(&["-v", "--check", "-s", path], contains(path));
119+
}
120+
121+
#[test]
122+
fn specify_source_files_in_a_workspace_when_running_cargo_fmt() {
123+
let path = "tests/cargo-fmt/source/workspaces/path-dep-above/ws/a/src/main.rs";
124+
assert_that!(&["-v", "--check", "-s", path], contains(path));
125+
}
126+
127+
#[test]
128+
fn formatting_source_files_and_packages_at_the_same_time_is_not_supported() {
129+
let (_, stderr) = cargo_fmt(&["--check", "-s", "src/main.rs", "-p", "p1"]);
130+
assert!(stderr.starts_with("cannot format source files and packages at the same time"))
131+
}
132+
133+
#[test]
134+
fn formatting_source_files_and_using_package_related_arguments_is_not_supported() {
135+
let (_, stderr) = cargo_fmt(&["--check", "--all", "-s", "src/main.rs"]);
136+
assert!(stderr.starts_with("cannot format all packages when specifying source files"))
137+
}

0 commit comments

Comments
 (0)