Skip to content

Commit 3a2c1e5

Browse files
committed
Fix lintcheck --fix
There were several issues: - `--fix` was ignored as part of #9461 - `--filter` in conjunction to `--fix` was broken due to #9703 After `lintcheck --fix` is used, crates will be re-extracted since their content has been modified
1 parent 5461e99 commit 3a2c1e5

File tree

1 file changed

+39
-15
lines changed

1 file changed

+39
-15
lines changed

lintcheck/src/main.rs

+39-15
Original file line numberDiff line numberDiff line change
@@ -182,33 +182,35 @@ impl CrateSource {
182182
CrateSource::CratesIo { name, version, options } => {
183183
let extract_dir = PathBuf::from(LINTCHECK_SOURCES);
184184
let krate_download_dir = PathBuf::from(LINTCHECK_DOWNLOADS);
185+
let extracted_krate_dir = extract_dir.join(format!("{name}-{version}/"));
185186

186187
// url to download the crate from crates.io
187188
let url = format!("https://crates.io/api/v1/crates/{name}/{version}/download");
188189
println!("Downloading and extracting {name} {version} from {url}");
189190
create_dirs(&krate_download_dir, &extract_dir);
190191

191192
let krate_file_path = krate_download_dir.join(format!("{name}-{version}.crate.tar.gz"));
192-
// don't download/extract if we already have done so
193+
// don't download if we already have done so
193194
if !krate_file_path.is_file() {
194195
// create a file path to download and write the crate data into
195196
let mut krate_dest = std::fs::File::create(&krate_file_path).unwrap();
196197
let mut krate_req = get(&url).unwrap().into_reader();
197198
// copy the crate into the file
198199
std::io::copy(&mut krate_req, &mut krate_dest).unwrap();
200+
}
199201

200-
// unzip the tarball
201-
let ungz_tar = flate2::read::GzDecoder::new(std::fs::File::open(&krate_file_path).unwrap());
202-
// extract the tar archive
203-
let mut archive = tar::Archive::new(ungz_tar);
204-
archive.unpack(&extract_dir).expect("Failed to extract!");
202+
// if the source code was altered (previous use of `--fix`), we need to restore the crate
203+
// to its original state by re-extracting it
204+
if !extracted_krate_dir.exists() || has_been_modified(&extracted_krate_dir) {
205+
debug!("extracting {} {}", name, version);
206+
Self::extract(&extract_dir, &krate_file_path);
205207
}
206208
// crate is extracted, return a new Krate object which contains the path to the extracted
207209
// sources that clippy can check
208210
Crate {
209211
version: version.clone(),
210212
name: name.clone(),
211-
path: extract_dir.join(format!("{name}-{version}/")),
213+
path: extracted_krate_dir,
212214
options: options.clone(),
213215
}
214216
},
@@ -299,6 +301,14 @@ impl CrateSource {
299301
},
300302
}
301303
}
304+
305+
fn extract(extract_dir: &Path, krate_file_path: &Path) {
306+
// unzip the tarball
307+
let ungz_tar = flate2::read::GzDecoder::new(std::fs::File::open(krate_file_path).unwrap());
308+
// extract the tar archive
309+
let mut archive = tar::Archive::new(ungz_tar);
310+
archive.unpack(extract_dir).expect("Failed to extract!");
311+
}
302312
}
303313

304314
impl Crate {
@@ -338,7 +348,9 @@ impl Crate {
338348
let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir");
339349

340350
let mut cargo_clippy_args = if config.fix {
341-
vec!["--fix", "--"]
351+
// need to pass `clippy` arg even if already feeding `cargo-clippy`
352+
// see https://github.com/rust-lang/rust-clippy/pull/9461
353+
vec!["clippy", "--fix", "--allow-no-vcs", "--"]
342354
} else {
343355
vec!["--", "--message-format=json", "--"]
344356
};
@@ -348,16 +360,19 @@ impl Crate {
348360
for opt in options {
349361
clippy_args.push(opt);
350362
}
351-
} else {
352-
clippy_args.extend(["-Wclippy::pedantic", "-Wclippy::cargo"]);
353363
}
354364

355-
if lint_filter.is_empty() {
356-
clippy_args.push("--cap-lints=warn");
365+
// cap-lints flag is ignored when using `clippy --fix` for now
366+
// So it needs to be passed directly to rustc
367+
// see https://github.com/rust-lang/rust-clippy/issues/9703
368+
let rustc_flags = if lint_filter.is_empty() {
369+
clippy_args.extend(["-Wclippy::pedantic", "-Wclippy::cargo"]);
370+
"--cap-lints=warn".to_string()
357371
} else {
358-
clippy_args.push("--cap-lints=allow");
359-
clippy_args.extend(lint_filter.iter().map(std::string::String::as_str));
360-
}
372+
let mut lint_filter_buf = lint_filter.clone();
373+
lint_filter_buf.push("--cap-lints=allow".to_string());
374+
lint_filter_buf.join(" ")
375+
};
361376

362377
if let Some(server) = server {
363378
let target = shared_target_dir.join("recursive");
@@ -396,6 +411,7 @@ impl Crate {
396411
let all_output = Command::new(&cargo_clippy_path)
397412
// use the looping index to create individual target dirs
398413
.env("CARGO_TARGET_DIR", shared_target_dir.join(format!("_{thread_index:?}")))
414+
.env("RUSTFLAGS", rustc_flags)
399415
.args(&cargo_clippy_args)
400416
.current_dir(&self.path)
401417
.output()
@@ -807,6 +823,14 @@ fn clippy_project_root() -> &'static Path {
807823
Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap()
808824
}
809825

826+
fn has_been_modified(extracted_krate_dir: &Path) -> bool {
827+
extracted_krate_dir.metadata().map_or(false, |metadata| {
828+
metadata.created().map_or(false, |created| {
829+
metadata.modified().map_or(false, |modified| created != modified)
830+
})
831+
})
832+
}
833+
810834
#[test]
811835
fn lintcheck_test() {
812836
let args = [

0 commit comments

Comments
 (0)