From fdadebe6721a0016b87063d7a54e48c3db6bab82 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Sat, 28 May 2022 15:27:53 -0400 Subject: [PATCH] Add lint output to lint list --- Cargo.toml | 2 +- clippy_lints/Cargo.toml | 4 +- clippy_lints/src/collapsible_if.rs | 12 +- clippy_lints/src/doc.rs | 4 +- clippy_lints/src/large_include_file.rs | 3 +- clippy_lints/src/literal_representation.rs | 1 + clippy_lints/src/methods/mod.rs | 1 + .../internal_lints/metadata_collector.rs | 228 ++++++++++++++++-- tests/dogfood.rs | 4 +- util/gh-pages/index.html | 20 ++ 10 files changed, 247 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d23d681df00c..42455998fe79 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -59,7 +59,7 @@ rustc_tools_util = { version = "0.2", path = "rustc_tools_util" } [features] deny-warnings = ["clippy_lints/deny-warnings"] integration = ["tempfile"] -internal = ["clippy_lints/internal"] +internal = ["clippy_lints/internal", "tempfile"] [package.metadata.rust-analyzer] # This package uses #[feature(rustc_private)] diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 0a3f04da3570..eb4582b93f2c 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -10,6 +10,7 @@ edition = "2021" [dependencies] cargo_metadata = "0.14" +clippy_dev = { path = "../clippy_dev", optional = true } clippy_utils = { path = "../clippy_utils" } if_chain = "1.0" itertools = "0.10.1" @@ -18,6 +19,7 @@ quine-mc_cluskey = "0.2" regex-syntax = "0.6" serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", optional = true } +tempfile = { version = "3.3.0", optional = true } toml = "0.5" unicode-normalization = "0.1" unicode-script = { version = "0.5", default-features = false } @@ -30,7 +32,7 @@ url = { version = "2.2", features = ["serde"] } [features] deny-warnings = ["clippy_utils/deny-warnings"] # build clippy with internal lints enabled, off by default -internal = ["clippy_utils/internal", "serde_json"] +internal = ["clippy_utils/internal", "serde_json", "tempfile", "clippy_dev"] [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)] diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 3eceb848822e..90430b71a0ef 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -32,20 +32,20 @@ declare_clippy_lint! { /// makes code look more complex than it really is. /// /// ### Example - /// ```rust,ignore + /// ```rust + /// # let (x, y) = (true, true); /// if x { /// if y { - /// … + /// // … /// } /// } - /// /// ``` /// /// Use instead: - /// - /// ```rust,ignore + /// ```rust + /// # let (x, y) = (true, true); /// if x && y { - /// … + /// // … /// } /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index aaec88f50c77..bf8c810a3e75 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -178,7 +178,7 @@ declare_clippy_lint! { /// if the `fn main()` is left implicit. /// /// ### Examples - /// ``````rust + /// ```rust /// /// An example of a doctest with a `main()` function /// /// /// /// # Examples @@ -191,7 +191,7 @@ declare_clippy_lint! { /// fn needless_main() { /// unimplemented!(); /// } - /// `````` + /// ``` #[clippy::version = "1.40.0"] pub NEEDLESS_DOCTEST_MAIN, style, diff --git a/clippy_lints/src/large_include_file.rs b/clippy_lints/src/large_include_file.rs index 8bef13c682db..84dd61a1e4b0 100644 --- a/clippy_lints/src/large_include_file.rs +++ b/clippy_lints/src/large_include_file.rs @@ -22,10 +22,11 @@ declare_clippy_lint! { /// let included_bytes = include_bytes!("very_large_file.txt"); /// ``` /// - /// Instead, you can load the file at runtime: + /// Use instead: /// ```rust,ignore /// use std::fs; /// + /// // You can load the file at runtime /// let string = fs::read_to_string("very_large_file.txt")?; /// let bytes = fs::read("very_large_file.txt")?; /// ``` diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 9998712b8527..2a4e1262276d 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -46,6 +46,7 @@ declare_clippy_lint! { /// - Does not match on `_127` since that is a valid grouping for decimal and octal numbers /// /// ### Example + /// ```ignore /// `2_32` => `2_i32` /// `250_8 => `250_u8` /// ``` diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7308e74c323e..904454037ead 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1039,6 +1039,7 @@ declare_clippy_lint! { /// /// // Good /// _.split('x'); + /// ``` #[clippy::version = "pre 1.29.0"] pub SINGLE_CHAR_PATTERN, perf, diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index cf2de6a42af3..069d8374b268 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -31,6 +31,8 @@ use std::fmt::Write as _; use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::path::Path; +use std::path::PathBuf; +use std::process::Command; /// This is the output file of the lint collector. const OUTPUT_FILE: &str = "../util/gh-pages/lints.json"; @@ -180,6 +182,7 @@ pub struct MetadataCollector { lints: BinaryHeap, applicability_info: FxHashMap, config: Vec, + clippy_project_root: PathBuf, } impl MetadataCollector { @@ -188,6 +191,7 @@ impl MetadataCollector { lints: BinaryHeap::::default(), applicability_info: FxHashMap::::default(), config: collect_configs(), + clippy_project_root: clippy_dev::clippy_project_root(), } } @@ -215,11 +219,13 @@ impl Drop for MetadataCollector { // Mapping the final data let mut lints = std::mem::take(&mut self.lints).into_sorted_vec(); - collect_renames(&mut lints); for x in &mut lints { x.applicability = Some(applicability_info.remove(&x.id).unwrap_or_default()); + replace_produces(&x.id, &mut x.docs, &self.clippy_project_root); } + collect_renames(&mut lints); + // Outputting if Path::new(OUTPUT_FILE).exists() { fs::remove_file(OUTPUT_FILE).unwrap(); @@ -263,14 +269,193 @@ impl LintMetadata { } } +fn replace_produces(lint_name: &str, docs: &mut String, clippy_project_root: &Path) { + let mut doc_lines = docs.lines().map(ToString::to_string).collect::>(); + let mut lines = doc_lines.iter_mut(); + + 'outer: loop { + // Find the start of the example + + // ```rust + loop { + match lines.next() { + Some(line) if line.trim_start().starts_with("```rust") => { + if line.contains("ignore") || line.contains("no_run") { + // A {{produces}} marker may have been put on a ignored code block by mistake, + // just seek to the end of the code block and continue checking. + if lines.any(|line| line.trim_start().starts_with("```")) { + continue; + } + + panic!("lint `{}` has an unterminated code block", lint_name) + } + + break; + }, + Some(line) if line.trim_start() == "{{produces}}" => { + panic!( + "lint `{}` has marker {{{{produces}}}} with an ignored or missing code block", + lint_name + ) + }, + Some(line) => { + let line = line.trim(); + // These are the two most common markers of the corrections section + if line.eq_ignore_ascii_case("Use instead:") || line.eq_ignore_ascii_case("Could be written as:") { + break 'outer; + } + }, + None => break 'outer, + } + } + + // Collect the example + let mut example = Vec::new(); + loop { + match lines.next() { + Some(line) if line.trim_start() == "```" => break, + Some(line) => example.push(line), + None => panic!("lint `{}` has an unterminated code block", lint_name), + } + } + + // Find the {{produces}} and attempt to generate the output + loop { + match lines.next() { + Some(line) if line.is_empty() => {}, + Some(line) if line.trim() == "{{produces}}" => { + let output = get_lint_output(lint_name, &example, clippy_project_root); + line.replace_range( + .., + &format!( + "
\ + Produces\n\ + \n\ + ```text\n\ + {}\n\ + ```\n\ +
", + output + ), + ); + + break; + }, + // No {{produces}}, we can move on to the next example + Some(_) => break, + None => break 'outer, + } + } + } + + *docs = cleanup_docs(&doc_lines); +} + +fn get_lint_output(lint_name: &str, example: &[&mut String], clippy_project_root: &Path) -> String { + let dir = tempfile::tempdir().unwrap_or_else(|e| panic!("failed to create temp dir: {e}")); + let file = dir.path().join("lint_example.rs"); + + let mut source = String::new(); + let unhidden = example + .iter() + .map(|line| line.trim_start().strip_prefix("# ").unwrap_or(line)); + + // Get any attributes + let mut lines = unhidden.peekable(); + while let Some(line) = lines.peek() { + if line.starts_with("#!") { + source.push_str(line); + source.push('\n'); + lines.next(); + } else { + break; + } + } + + let needs_main = !example.iter().any(|line| line.contains("fn main")); + if needs_main { + source.push_str("fn main() {\n"); + } + + for line in lines { + source.push_str(line); + source.push('\n'); + } + + if needs_main { + source.push_str("}\n"); + } + + if let Err(e) = fs::write(&file, &source) { + panic!("failed to write to `{}`: {e}", file.as_path().to_string_lossy()); + } + + let prefixed_name = format!("{}{lint_name}", CLIPPY_LINT_GROUP_PREFIX); + + let mut cmd = Command::new("cargo"); + + cmd.current_dir(clippy_project_root) + .env("CARGO_INCREMENTAL", "0") + .env("CLIPPY_ARGS", "") + .env("CLIPPY_DISABLE_DOCS_LINKS", "1") + // We need to disable this to enable all lints + .env("ENABLE_METADATA_COLLECTION", "0") + .args(["run", "--bin", "clippy-driver"]) + .args(["--target-dir", "./clippy_lints/target"]) + .args(["--", "--error-format=json"]) + .args(["--edition", "2021"]) + .arg("-Cdebuginfo=0") + .args(["-A", "clippy::all"]) + .args(["-W", &prefixed_name]) + .args(["-L", "./target/debug"]) + .args(["-Z", "no-codegen"]); + + let output = cmd + .arg(file.as_path()) + .output() + .unwrap_or_else(|e| panic!("failed to run `{:?}`: {e}", cmd)); + + let tmp_file_path = file.to_string_lossy(); + let stderr = std::str::from_utf8(&output.stderr).unwrap(); + let msgs = stderr + .lines() + .filter(|line| line.starts_with('{')) + .map(|line| serde_json::from_str(line).unwrap()) + .collect::>(); + + let mut rendered = String::new(); + let iter = msgs + .iter() + .filter(|msg| matches!(&msg["code"]["code"], serde_json::Value::String(s) if s == &prefixed_name)); + + for message in iter { + let rendered_part = message["rendered"].as_str().expect("rendered field should exist"); + rendered.push_str(rendered_part); + } + + if rendered.is_empty() { + let rendered: Vec<&str> = msgs.iter().filter_map(|msg| msg["rendered"].as_str()).collect(); + let non_json: Vec<&str> = stderr.lines().filter(|line| !line.starts_with('{')).collect(); + panic!( + "did not find lint `{}` in output of example, got:\n{}\n{}", + lint_name, + non_json.join("\n"), + rendered.join("\n") + ); + } + + // The reader doesn't need to see `/tmp/.tmpfiy2Qd/lint_example.rs` :) + rendered.trim_end().replace(&*tmp_file_path, "lint_example.rs") +} + #[derive(Debug, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)] struct SerializableSpan { path: String, line: usize, } -impl std::fmt::Display for SerializableSpan { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for SerializableSpan { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}:{}", self.path.rsplit('/').next().unwrap_or_default(), self.line) } } @@ -435,10 +620,10 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); // metadata extraction if let Some((group, level)) = get_lint_group_and_level_or_lint(cx, &lint_name, item); - if let Some(mut docs) = extract_attr_docs_or_lint(cx, item); + if let Some(mut raw_docs) = extract_attr_docs_or_lint(cx, item); then { if let Some(configuration_section) = self.get_lint_configs(&lint_name) { - docs.push_str(&configuration_section); + raw_docs.push_str(&configuration_section); } let version = get_lint_version(cx, item); @@ -448,7 +633,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { group, level, version, - docs, + raw_docs, )); } } @@ -459,7 +644,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { let lint_name = sym_to_string(item.ident.name).to_ascii_lowercase(); if !BLACK_LISTED_LINTS.contains(&lint_name.as_str()); // Metadata the little we can get from a deprecated lint - if let Some(docs) = extract_attr_docs_or_lint(cx, item); + if let Some(raw_docs) = extract_attr_docs_or_lint(cx, item); then { let version = get_lint_version(cx, item); @@ -469,7 +654,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { DEPRECATED_LINT_GROUP_STR.to_string(), DEPRECATED_LINT_LEVEL, version, - docs, + raw_docs, )); } } @@ -535,22 +720,28 @@ fn extract_attr_docs_or_lint(cx: &LateContext<'_>, item: &Item<'_>) -> Option, item: &Item<'_>) -> Option { + let attrs = cx.tcx.hir().attrs(item.hir_id()); + let mut lines = attrs.iter().filter_map(ast::Attribute::doc_str); + + if let Some(line) = lines.next() { + let raw_docs = lines.fold(String::from(line.as_str()) + "\n", |s, line| s + line.as_str() + "\n"); + return Some(raw_docs); + } + + None +} + /// This function may modify the doc comment to ensure that the string can be displayed using a /// markdown viewer in Clippy's lint list. The following modifications could be applied: /// * Removal of leading space after a new line. (Important to display tables) /// * Ensures that code blocks only contain language information -fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option { - let attrs = cx.tcx.hir().attrs(item.hir_id()); - let mut lines = attrs.iter().filter_map(ast::Attribute::doc_str); - let mut docs = String::from(lines.next()?.as_str()); +fn cleanup_docs(docs_collection: &Vec) -> String { let mut in_code_block = false; let mut is_code_block_rust = false; - for line in lines { - let line = line.as_str(); + let mut docs = String::new(); + for line in docs_collection { // Rustdoc hides code lines starting with `# ` and this removes them from Clippy's lint list :) if is_code_block_rust && line.trim_start().starts_with("# ") { continue; @@ -583,7 +774,8 @@ fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option { docs.push_str(line); } } - Some(docs) + + docs } fn get_lint_version(cx: &LateContext<'_>, item: &Item<'_>) -> String { diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 9da80518ce91..fffc53603424 100644 --- a/tests/dogfood.rs +++ b/tests/dogfood.rs @@ -21,7 +21,7 @@ fn dogfood_clippy() { // "" is the root package for package in &["", "clippy_dev", "clippy_lints", "clippy_utils", "rustc_tools_util"] { - run_clippy_for_package(package, &[]); + run_clippy_for_package(package, &["-D", "clippy::all", "-D", "clippy::pedantic"]); } } @@ -77,8 +77,6 @@ fn run_clippy_for_package(project: &str, args: &[&str]) { .arg("--all-features") .arg("--") .args(args) - .args(&["-D", "clippy::all"]) - .args(&["-D", "clippy::pedantic"]) .arg("-Cdebuginfo=0"); // disable debuginfo to generate less data in the target dir if cfg!(feature = "internal") { diff --git a/util/gh-pages/index.html b/util/gh-pages/index.html index 2076d1299783..4999cce75114 100644 --- a/util/gh-pages/index.html +++ b/util/gh-pages/index.html @@ -206,6 +206,26 @@ margin: auto 5px; font-family: monospace; } + + details { + border-radius: 4px; + padding: .5em .5em 0; + } + + code { + white-space: pre !important; + } + + summary { + font-weight: bold; + margin: -.5em -.5em 0; + padding: .5em; + display: revert; + } + + details[open] { + padding: .5em; + }