From 601a61fe276a92bbde6e74d60fc493e064677d9d Mon Sep 17 00:00:00 2001 From: xFrednet Date: Tue, 9 Jul 2024 23:51:31 +0200 Subject: [PATCH 1/2] Lintcheck: Refactor `CrateSource` --- lintcheck/src/input.rs | 65 ++++++++++++++++++++---------------------- lintcheck/src/main.rs | 10 ++----- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/lintcheck/src/input.rs b/lintcheck/src/input.rs index 3d034391c280..8e048615f43e 100644 --- a/lintcheck/src/input.rs +++ b/lintcheck/src/input.rs @@ -37,28 +37,22 @@ struct TomlCrate { /// Represents an archive we download from crates.io, or a git repo, or a local repo/folder /// Once processed (downloaded/extracted/cloned/copied...), this will be translated into a `Crate` +#[derive(Debug, Deserialize, Eq, Hash, PartialEq, Ord, PartialOrd)] +pub struct CrateWithSource { + pub name: String, + pub source: CrateSource, + pub options: Option>, +} + #[derive(Debug, Deserialize, Eq, Hash, PartialEq, Ord, PartialOrd)] pub enum CrateSource { - CratesIo { - name: String, - version: String, - options: Option>, - }, - Git { - name: String, - url: String, - commit: String, - options: Option>, - }, - Path { - name: String, - path: PathBuf, - options: Option>, - }, + CratesIo { version: String }, + Git { url: String, commit: String }, + Path { path: PathBuf }, } /// Read a `lintcheck_crates.toml` file -pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) { +pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) { let toml_content: String = fs::read_to_string(toml_path).unwrap_or_else(|_| panic!("Failed to read {}", toml_path.display())); let crate_list: SourceList = @@ -71,23 +65,29 @@ pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) { let mut crate_sources = Vec::new(); for tk in tomlcrates { if let Some(ref path) = tk.path { - crate_sources.push(CrateSource::Path { + crate_sources.push(CrateWithSource { name: tk.name.clone(), - path: PathBuf::from(path), + source: CrateSource::Path { + path: PathBuf::from(path), + }, options: tk.options.clone(), }); } else if let Some(ref version) = tk.version { - crate_sources.push(CrateSource::CratesIo { + crate_sources.push(CrateWithSource { name: tk.name.clone(), - version: version.to_string(), + source: CrateSource::CratesIo { + version: version.to_string(), + }, options: tk.options.clone(), }); } else if tk.git_url.is_some() && tk.git_hash.is_some() { // otherwise, we should have a git source - crate_sources.push(CrateSource::Git { + crate_sources.push(CrateWithSource { name: tk.name.clone(), - url: tk.git_url.clone().unwrap(), - commit: tk.git_hash.clone().unwrap(), + source: CrateSource::Git { + url: tk.git_url.clone().unwrap(), + commit: tk.git_hash.clone().unwrap(), + }, options: tk.options.clone(), }); } else { @@ -117,7 +117,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) { (crate_sources, crate_list.recursive) } -impl CrateSource { +impl CrateWithSource { /// Makes the sources available on the disk for clippy to check. /// Clones a git repo and checks out the specified commit or downloads a crate from crates.io or /// copies a local folder @@ -139,8 +139,10 @@ impl CrateSource { retries += 1; } } - match self { - CrateSource::CratesIo { name, version, options } => { + let name = &self.name; + let options = &self.options; + match &self.source { + CrateSource::CratesIo { version } => { let extract_dir = PathBuf::from(LINTCHECK_SOURCES); let krate_download_dir = PathBuf::from(LINTCHECK_DOWNLOADS); @@ -173,12 +175,7 @@ impl CrateSource { options: options.clone(), } }, - CrateSource::Git { - name, - url, - commit, - options, - } => { + CrateSource::Git { url, commit } => { let repo_path = { let mut repo_path = PathBuf::from(LINTCHECK_SOURCES); // add a -git suffix in case we have the same crate from crates.io and a git repo @@ -219,7 +216,7 @@ impl CrateSource { options: options.clone(), } }, - CrateSource::Path { name, path, options } => { + CrateSource::Path { path } => { fn is_cache_dir(entry: &DirEntry) -> bool { fs::read(entry.path().join("CACHEDIR.TAG")) .map(|x| x.starts_with(b"Signature: 8a477f597d28d172789f06886806bc55")) diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index e37ffab13ac8..e85d70f11ed6 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -38,7 +38,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::{env, fs}; use cargo_metadata::Message; -use input::{read_crates, CrateSource}; +use input::read_crates; use output::{ClippyCheckOutput, ClippyWarning, RustcIce}; use rayon::prelude::*; @@ -292,13 +292,7 @@ fn lintcheck(config: LintcheckConfig) { .into_iter() .filter(|krate| { if let Some(only_one_crate) = &config.only { - let name = match krate { - CrateSource::CratesIo { name, .. } - | CrateSource::Git { name, .. } - | CrateSource::Path { name, .. } => name, - }; - - name == only_one_crate + krate.name == *only_one_crate } else { true } From 0e3d19799ccd2279ac3f1e672d954566ce2e77df Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 10 Jul 2024 00:13:33 +0200 Subject: [PATCH 2/2] Lintcheck: Construct links for sources --- lintcheck/lintcheck_crates.toml | 2 +- lintcheck/src/driver.rs | 1 + lintcheck/src/input.rs | 45 +++++++++++++++++++++++++++++++++ lintcheck/src/json.rs | 12 ++++++--- lintcheck/src/main.rs | 3 ++- lintcheck/src/output.rs | 20 ++++++++++++--- lintcheck/src/recursive.rs | 10 +++++++- 7 files changed, 83 insertions(+), 10 deletions(-) diff --git a/lintcheck/lintcheck_crates.toml b/lintcheck/lintcheck_crates.toml index ff608e6f9359..6e8e1e726d66 100644 --- a/lintcheck/lintcheck_crates.toml +++ b/lintcheck/lintcheck_crates.toml @@ -1,6 +1,6 @@ [crates] # some of these are from cargotest -cargo = {name = "cargo", version = '0.64.0'} +cargo = {name = "cargo", version = '0.64.0', online_link = 'https://docs.rs/cargo/{version}/src/{file}.html#{line}'} iron = {name = "iron", version = '0.6.1'} ripgrep = {name = "ripgrep", version = '12.1.1'} xsv = {name = "xsv", version = '0.13.0'} diff --git a/lintcheck/src/driver.rs b/lintcheck/src/driver.rs index 041be5081f28..2fda2b00f873 100644 --- a/lintcheck/src/driver.rs +++ b/lintcheck/src/driver.rs @@ -11,6 +11,7 @@ use std::{env, mem}; fn run_clippy(addr: &str) -> Option { let driver_info = DriverInfo { package_name: env::var("CARGO_PKG_NAME").ok()?, + version: env::var("CARGO_PKG_VERSION").ok()?, }; let mut stream = BufReader::new(TcpStream::connect(addr).unwrap()); diff --git a/lintcheck/src/input.rs b/lintcheck/src/input.rs index 8e048615f43e..5aa261d7f6d1 100644 --- a/lintcheck/src/input.rs +++ b/lintcheck/src/input.rs @@ -10,6 +10,10 @@ use walkdir::{DirEntry, WalkDir}; use crate::{Crate, LINTCHECK_DOWNLOADS, LINTCHECK_SOURCES}; +const DEFAULT_DOCS_LINK: &str = "https://docs.rs/{krate}/{version}/src/{krate}/{file}.html#{line}"; +const DEFAULT_GITHUB_LINK: &str = "{url}/blob/{hash}/src/{file}#L{line}"; +const DEFAULT_PATH_LINK: &str = "{path}/src/{file}:{line}"; + /// List of sources to check, loaded from a .toml file #[derive(Debug, Deserialize)] pub struct SourceList { @@ -33,6 +37,39 @@ struct TomlCrate { git_hash: Option, path: Option, options: Option>, + /// Magic values: + /// * `{krate}` will be replaced by `self.name` + /// * `{version}` will be replaced by `self.version` + /// * `{url}` will be replaced with `self.git_url` + /// * `{hash}` will be replaced with `self.git_hash` + /// * `{path}` will be replaced with `self.path` + /// * `{file}` will be replaced by the path after `src/` + /// * `{line}` will be replaced by the line + /// + /// If unset, this will be filled by [`read_crates`] since it depends on + /// the source. + online_link: Option, +} + +impl TomlCrate { + fn file_link(&self, default: &str) -> String { + let mut link = self.online_link.clone().unwrap_or_else(|| default.to_string()); + link = link.replace("{krate}", &self.name); + + if let Some(version) = &self.version { + link = link.replace("{version}", version); + } + if let Some(url) = &self.git_url { + link = link.replace("{url}", url); + } + if let Some(hash) = &self.git_hash { + link = link.replace("{hash}", hash); + } + if let Some(path) = &self.path { + link = link.replace("{path}", path); + } + link + } } /// Represents an archive we download from crates.io, or a git repo, or a local repo/folder @@ -41,6 +78,7 @@ struct TomlCrate { pub struct CrateWithSource { pub name: String, pub source: CrateSource, + pub file_link: String, pub options: Option>, } @@ -70,6 +108,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) source: CrateSource::Path { path: PathBuf::from(path), }, + file_link: tk.file_link(DEFAULT_PATH_LINK), options: tk.options.clone(), }); } else if let Some(ref version) = tk.version { @@ -78,6 +117,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) source: CrateSource::CratesIo { version: version.to_string(), }, + file_link: tk.file_link(DEFAULT_DOCS_LINK), options: tk.options.clone(), }); } else if tk.git_url.is_some() && tk.git_hash.is_some() { @@ -88,6 +128,7 @@ pub fn read_crates(toml_path: &Path) -> (Vec, RecursiveOptions) url: tk.git_url.clone().unwrap(), commit: tk.git_hash.clone().unwrap(), }, + file_link: tk.file_link(DEFAULT_GITHUB_LINK), options: tk.options.clone(), }); } else { @@ -141,6 +182,7 @@ impl CrateWithSource { } let name = &self.name; let options = &self.options; + let file_link = &self.file_link; match &self.source { CrateSource::CratesIo { version } => { let extract_dir = PathBuf::from(LINTCHECK_SOURCES); @@ -173,6 +215,7 @@ impl CrateWithSource { name: name.clone(), path: extract_dir.join(format!("{name}-{version}/")), options: options.clone(), + base_url: file_link.clone(), } }, CrateSource::Git { url, commit } => { @@ -214,6 +257,7 @@ impl CrateWithSource { name: name.clone(), path: repo_path, options: options.clone(), + base_url: file_link.clone(), } }, CrateSource::Path { path } => { @@ -253,6 +297,7 @@ impl CrateWithSource { name: name.clone(), path: dest_crate_root, options: options.clone(), + base_url: file_link.clone(), } }, } diff --git a/lintcheck/src/json.rs b/lintcheck/src/json.rs index 1a652927988e..410f19f27b84 100644 --- a/lintcheck/src/json.rs +++ b/lintcheck/src/json.rs @@ -11,6 +11,7 @@ struct LintJson { lint: String, file_name: String, byte_pos: (u32, u32), + file_link: String, rendered: String, } @@ -29,6 +30,7 @@ pub(crate) fn output(clippy_warnings: Vec) -> String { LintJson { file_name: span.file_name.clone(), byte_pos: (span.byte_start, span.byte_end), + file_link: warning.url, lint: warning.lint, rendered: warning.diag.rendered.unwrap(), } @@ -50,11 +52,12 @@ fn print_warnings(title: &str, warnings: &[LintJson]) { } println!("### {title}"); - println!("```"); for warning in warnings { + println!("{title} `{}` at {}", warning.lint, warning.file_link); + println!("```"); print!("{}", warning.rendered); + println!("```"); } - println!("```"); } fn print_changed_diff(changed: &[(LintJson, LintJson)]) { @@ -63,8 +66,9 @@ fn print_changed_diff(changed: &[(LintJson, LintJson)]) { } println!("### Changed"); - println!("```diff"); for (old, new) in changed { + println!("Changed `{}` at {}", new.lint, new.file_link); + println!("```diff"); for change in diff::lines(&old.rendered, &new.rendered) { use diff::Result::{Both, Left, Right}; @@ -80,8 +84,8 @@ fn print_changed_diff(changed: &[(LintJson, LintJson)]) { }, } } + println!("```"); } - println!("```"); } pub(crate) fn diff(old_path: &Path, new_path: &Path) { diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index e85d70f11ed6..ba4ced4e4692 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -53,6 +53,7 @@ struct Crate { // path to the extracted sources that clippy can check path: PathBuf, options: Option>, + base_url: String, } impl Crate { @@ -185,7 +186,7 @@ impl Crate { // get all clippy warnings and ICEs let mut entries: Vec = Message::parse_stream(stdout.as_bytes()) .filter_map(|msg| match msg { - Ok(Message::CompilerMessage(message)) => ClippyWarning::new(message.message), + Ok(Message::CompilerMessage(message)) => ClippyWarning::new(message.message, &self.base_url), _ => None, }) .map(ClippyCheckOutput::ClippyWarning) diff --git a/lintcheck/src/output.rs b/lintcheck/src/output.rs index 4bfc554ef9e6..2fd09242f8ad 100644 --- a/lintcheck/src/output.rs +++ b/lintcheck/src/output.rs @@ -53,11 +53,12 @@ impl RustcIce { pub struct ClippyWarning { pub lint: String, pub diag: Diagnostic, + /// The URL that points to the file and line of the lint emission + pub url: String, } -#[allow(unused)] impl ClippyWarning { - pub fn new(mut diag: Diagnostic) -> Option { + pub fn new(mut diag: Diagnostic, base_url: &str) -> Option { let lint = diag.code.clone()?.code; if !(lint.contains("clippy") || diag.message.contains("clippy")) || diag.message.contains("could not read cargo metadata") @@ -69,7 +70,20 @@ impl ClippyWarning { let rendered = diag.rendered.as_mut().unwrap(); *rendered = strip_ansi_escapes::strip_str(&rendered); - Some(Self { lint, diag }) + let span = diag.spans.iter().find(|span| span.is_primary).unwrap(); + let file = &span.file_name; + let url = if let Some(src_split) = file.find("/src/") { + // This removes the inital `target/lintcheck/sources/-/` + let src_split = src_split + "/src/".len(); + let (_, file) = file.split_at(src_split); + + let line_no = span.line_start; + base_url.replace("{file}", file).replace("{line}", &line_no.to_string()) + } else { + file.clone() + }; + + Some(Self { lint, diag, url }) } pub fn span(&self) -> &DiagnosticSpan { diff --git a/lintcheck/src/recursive.rs b/lintcheck/src/recursive.rs index 373ca6f99184..6817d917b93b 100644 --- a/lintcheck/src/recursive.rs +++ b/lintcheck/src/recursive.rs @@ -20,6 +20,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Eq, Hash, PartialEq, Clone, Serialize, Deserialize)] pub(crate) struct DriverInfo { pub package_name: String, + pub version: String, } pub(crate) fn serialize_line(value: &T, writer: &mut W) @@ -61,10 +62,17 @@ fn process_stream( let mut stderr = String::new(); stream.read_to_string(&mut stderr).unwrap(); + // It's 99% likely that dependencies compiled with recursive mode are on crates.io + // and therefore on docs.rs. This links to the sources directly, do avoid invalid + // links due to remaped paths. See rust-lang/docs.rs#2551 for more details. + let base_url = format!( + "https://docs.rs/crate/{}/{}/source/src/{{file}}#{{line}}", + driver_info.package_name, driver_info.version + ); let messages = stderr .lines() .filter_map(|json_msg| serde_json::from_str::(json_msg).ok()) - .filter_map(ClippyWarning::new); + .filter_map(|diag| ClippyWarning::new(diag, &base_url)); for message in messages { sender.send(message).unwrap();