From da76c06209451fa01d5248374869b689116ae05e Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 9 Oct 2021 05:58:05 +0200 Subject: [PATCH 1/3] Add `--msrv` option to `new_lint` command --- clippy_dev/src/main.rs | 6 +++ clippy_dev/src/new_lint.rs | 108 +++++++++++++++++++++++++++++++------ 2 files changed, 98 insertions(+), 16 deletions(-) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 8fdeba9842af..b5c04efce3bc 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -28,6 +28,7 @@ fn main() { matches.value_of("pass"), matches.value_of("name"), matches.value_of("category"), + matches.is_present("msrv"), ) { Ok(_) => update_lints::run(update_lints::UpdateMode::Change), Err(e) => eprintln!("Unable to create lint: {}", e), @@ -147,6 +148,11 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { "internal_warn", ]) .takes_value(true), + ) + .arg( + Arg::with_name("msrv") + .long("msrv") + .help("Add MSRV config code to the lint"), ), ) .subcommand( diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 3a81aaba6de0..ebbe26c46f7f 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -32,7 +32,7 @@ impl Context for io::Result { /// # Errors /// /// This function errors out if the files couldn't be created or written to. -pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> io::Result<()> { +pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>, msrv: bool) -> io::Result<()> { let lint = LintData { pass: pass.expect("`pass` argument is validated by clap"), name: lint_name.expect("`name` argument is validated by clap"), @@ -40,11 +40,11 @@ pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str project_root: clippy_project_root(), }; - create_lint(&lint).context("Unable to create lint implementation")?; + create_lint(&lint, msrv).context("Unable to create lint implementation")?; create_test(&lint).context("Unable to create a test for the new lint") } -fn create_lint(lint: &LintData<'_>) -> io::Result<()> { +fn create_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> { let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass { "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"), "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"), @@ -55,6 +55,7 @@ fn create_lint(lint: &LintData<'_>) -> io::Result<()> { let camel_case_name = to_camel_case(lint.name); let lint_contents = get_lint_file_contents( + lint.pass, pass_type, pass_lifetimes, lint.name, @@ -62,6 +63,7 @@ fn create_lint(lint: &LintData<'_>) -> io::Result<()> { lint.category, pass_import, context_import, + enable_msrv, ); let lint_path = format!("clippy_lints/src/{}.rs", lint.name); @@ -155,6 +157,7 @@ publish = false } fn get_lint_file_contents( + pass_name: &str, pass_type: &str, pass_lifetimes: &str, lint_name: &str, @@ -162,13 +165,41 @@ fn get_lint_file_contents( category: &str, pass_import: &str, context_import: &str, + enable_msrv: bool, ) -> String { - format!( - "use rustc_lint::{{{type}, {context_import}}}; -use rustc_session::{{declare_lint_pass, declare_tool_lint}}; + let mut result = String::new(); + + let name_camel = camel_case_name; + let name_upper = lint_name.to_uppercase(); + + result.push_str(&if enable_msrv { + format!( + "use clippy_utils::msrvs; {pass_import} +use rustc_lint::{{{context_import}, {pass_type}, LintContext}}; +use rustc_semver::RustcVersion; +use rustc_session::{{declare_tool_lint, impl_lint_pass}}; + +", + pass_type = pass_type, + pass_import = pass_import, + context_import = context_import, + ) + } else { + format!( + "{pass_import} +use rustc_lint::{{{context_import}, {pass_type}}}; +use rustc_session::{{declare_lint_pass, declare_tool_lint}}; + +", + pass_import = pass_import, + pass_type = pass_type, + context_import = context_import + ) + }); -declare_clippy_lint! {{ + result.push_str(&format!( + "declare_clippy_lint! {{ /// ### What it does /// /// ### Why is this bad? @@ -184,20 +215,65 @@ declare_clippy_lint! {{ pub {name_upper}, {category}, \"default lint description\" +}}", + name_upper = name_upper, + category = category, + )); + + result.push_str(&if enable_msrv { + format!( + " +pub struct {name_camel} {{ + msrv: Option, +}} + +impl {name_camel} {{ + #[must_use] + pub fn new(msrv: Option) -> Self {{ + Self {{ msrv }} + }} +}} + +impl_lint_pass!({name_camel} => [{name_upper}]); + +impl {pass_type}{pass_lifetimes} for {name_camel} {{ + extract_msrv_attr!({context_import}); }} +// TODO: Register the lint pass in `clippy_lints/src/lib.rs`, +// e.g. store.register_{pass_name}_pass(move || Box::new({module_name}::{name_camel}::new(msrv))); +// TODO: Add MSRV level to `clippy_utils/src/msrvs.rs` if needed. +// TODO: Add MSRV test to `tests/ui/min_rust_version_attr.rs`. +// TODO: Update msrv config comment in `clippy_lints/src/utils/conf.rs` +", + pass_type = pass_type, + pass_lifetimes = pass_lifetimes, + pass_name = pass_name, + name_upper = name_upper, + name_camel = name_camel, + module_name = lint_name, + context_import = context_import, + ) + } else { + format!( + " declare_lint_pass!({name_camel} => [{name_upper}]); -impl {type}{lifetimes} for {name_camel} {{}} +impl {pass_type}{pass_lifetimes} for {name_camel} {{}} +// +// TODO: Register the lint pass in `clippy_lints/src/lib.rs`, +// e.g. store.register_{pass_name}_pass(|| Box::new({module_name}::{name_camel})); ", - type=pass_type, - lifetimes=pass_lifetimes, - name_upper=lint_name.to_uppercase(), - name_camel=camel_case_name, - category=category, - pass_import=pass_import, - context_import=context_import - ) + pass_type = pass_type, + pass_lifetimes = pass_lifetimes, + pass_name = pass_name, + name_upper = name_upper, + name_camel = name_camel, + module_name = lint_name, + ) + }); + + result } #[test] From b8782257ae54674101b08caeee59c0d51ff760c6 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 9 Oct 2021 05:58:05 +0200 Subject: [PATCH 2/3] Fix `clippy::too-many-arguments` violation --- clippy_dev/src/new_lint.rs | 46 ++++++++++++-------------------------- 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index ebbe26c46f7f..50297efde21e 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -45,26 +45,7 @@ pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str } fn create_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> { - let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass { - "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"), - "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"), - _ => { - unreachable!("`pass_type` should only ever be `early` or `late`!"); - }, - }; - - let camel_case_name = to_camel_case(lint.name); - let lint_contents = get_lint_file_contents( - lint.pass, - pass_type, - pass_lifetimes, - lint.name, - &camel_case_name, - lint.category, - pass_import, - context_import, - enable_msrv, - ); + let lint_contents = get_lint_file_contents(lint, enable_msrv); let lint_path = format!("clippy_lints/src/{}.rs", lint.name); write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes()) @@ -156,20 +137,21 @@ publish = false ) } -fn get_lint_file_contents( - pass_name: &str, - pass_type: &str, - pass_lifetimes: &str, - lint_name: &str, - camel_case_name: &str, - category: &str, - pass_import: &str, - context_import: &str, - enable_msrv: bool, -) -> String { +fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String { let mut result = String::new(); - let name_camel = camel_case_name; + let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass { + "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"), + "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"), + _ => { + unreachable!("`pass_type` should only ever be `early` or `late`!"); + }, + }; + + let lint_name = lint.name; + let pass_name = lint.pass; + let category = lint.category; + let name_camel = to_camel_case(lint.name); let name_upper = lint_name.to_uppercase(); result.push_str(&if enable_msrv { From 7d9b21b90bc77ab26c8afefc78d6087aff34e4ad Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 9 Oct 2021 05:58:05 +0200 Subject: [PATCH 3/3] Use `indoc` for formatting --- clippy_dev/Cargo.toml | 1 + clippy_dev/src/new_lint.rs | 152 +++++++++++++++++++------------------ 2 files changed, 80 insertions(+), 73 deletions(-) diff --git a/clippy_dev/Cargo.toml b/clippy_dev/Cargo.toml index 4a13a4524097..affb283017c8 100644 --- a/clippy_dev/Cargo.toml +++ b/clippy_dev/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" [dependencies] bytecount = "0.6" clap = "2.33" +indoc = "1.0" itertools = "0.10" opener = "0.5" regex = "1.5" diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 50297efde21e..25320907bb49 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -1,4 +1,5 @@ use crate::clippy_project_root; +use indoc::indoc; use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::io::{self, ErrorKind}; @@ -105,12 +106,13 @@ fn to_camel_case(name: &str) -> String { fn get_test_file_contents(lint_name: &str, header_commands: Option<&str>) -> String { let mut contents = format!( - "#![warn(clippy::{})] + indoc! {" + #![warn(clippy::{})] -fn main() {{ - // test code goes here -}} -", + fn main() {{ + // test code goes here + }} + "}, lint_name ); @@ -123,16 +125,16 @@ fn main() {{ fn get_manifest_contents(lint_name: &str, hint: &str) -> String { format!( - r#" -# {} + indoc! {r#" + # {} -[package] -name = "{}" -version = "0.1.0" -publish = false + [package] + name = "{}" + version = "0.1.0" + publish = false -[workspace] -"#, + [workspace] + "#}, hint, lint_name ) } @@ -156,24 +158,26 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String { result.push_str(&if enable_msrv { format!( - "use clippy_utils::msrvs; -{pass_import} -use rustc_lint::{{{context_import}, {pass_type}, LintContext}}; -use rustc_semver::RustcVersion; -use rustc_session::{{declare_tool_lint, impl_lint_pass}}; - -", + indoc! {" + use clippy_utils::msrvs; + {pass_import} + use rustc_lint::{{{context_import}, {pass_type}, LintContext}}; + use rustc_semver::RustcVersion; + use rustc_session::{{declare_tool_lint, impl_lint_pass}}; + + "}, pass_type = pass_type, pass_import = pass_import, context_import = context_import, ) } else { format!( - "{pass_import} -use rustc_lint::{{{context_import}, {pass_type}}}; -use rustc_session::{{declare_lint_pass, declare_tool_lint}}; + indoc! {" + {pass_import} + use rustc_lint::{{{context_import}, {pass_type}}}; + use rustc_session::{{declare_lint_pass, declare_tool_lint}}; -", + "}, pass_import = pass_import, pass_type = pass_type, context_import = context_import @@ -181,53 +185,55 @@ use rustc_session::{{declare_lint_pass, declare_tool_lint}}; }); result.push_str(&format!( - "declare_clippy_lint! {{ - /// ### What it does - /// - /// ### Why is this bad? - /// - /// ### Example - /// ```rust - /// // example code where clippy issues a warning - /// ``` - /// Use instead: - /// ```rust - /// // example code which does not raise clippy warning - /// ``` - pub {name_upper}, - {category}, - \"default lint description\" -}}", + indoc! {" + declare_clippy_lint! {{ + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + pub {name_upper}, + {category}, + \"default lint description\" + }} + "}, name_upper = name_upper, category = category, )); result.push_str(&if enable_msrv { format!( - " -pub struct {name_camel} {{ - msrv: Option, -}} - -impl {name_camel} {{ - #[must_use] - pub fn new(msrv: Option) -> Self {{ - Self {{ msrv }} - }} -}} - -impl_lint_pass!({name_camel} => [{name_upper}]); - -impl {pass_type}{pass_lifetimes} for {name_camel} {{ - extract_msrv_attr!({context_import}); -}} - -// TODO: Register the lint pass in `clippy_lints/src/lib.rs`, -// e.g. store.register_{pass_name}_pass(move || Box::new({module_name}::{name_camel}::new(msrv))); -// TODO: Add MSRV level to `clippy_utils/src/msrvs.rs` if needed. -// TODO: Add MSRV test to `tests/ui/min_rust_version_attr.rs`. -// TODO: Update msrv config comment in `clippy_lints/src/utils/conf.rs` -", + indoc! {" + pub struct {name_camel} {{ + msrv: Option, + }} + + impl {name_camel} {{ + #[must_use] + pub fn new(msrv: Option) -> Self {{ + Self {{ msrv }} + }} + }} + + impl_lint_pass!({name_camel} => [{name_upper}]); + + impl {pass_type}{pass_lifetimes} for {name_camel} {{ + extract_msrv_attr!({context_import}); + }} + + // TODO: Register the lint pass in `clippy_lints/src/lib.rs`, + // e.g. store.register_{pass_name}_pass(move || Box::new({module_name}::{name_camel}::new(msrv))); + // TODO: Add MSRV level to `clippy_utils/src/msrvs.rs` if needed. + // TODO: Add MSRV test to `tests/ui/min_rust_version_attr.rs`. + // TODO: Update msrv config comment in `clippy_lints/src/utils/conf.rs` + "}, pass_type = pass_type, pass_lifetimes = pass_lifetimes, pass_name = pass_name, @@ -238,14 +244,14 @@ impl {pass_type}{pass_lifetimes} for {name_camel} {{ ) } else { format!( - " -declare_lint_pass!({name_camel} => [{name_upper}]); - -impl {pass_type}{pass_lifetimes} for {name_camel} {{}} -// -// TODO: Register the lint pass in `clippy_lints/src/lib.rs`, -// e.g. store.register_{pass_name}_pass(|| Box::new({module_name}::{name_camel})); -", + indoc! {" + declare_lint_pass!({name_camel} => [{name_upper}]); + + impl {pass_type}{pass_lifetimes} for {name_camel} {{}} + // + // TODO: Register the lint pass in `clippy_lints/src/lib.rs`, + // e.g. store.register_{pass_name}_pass(|| Box::new({module_name}::{name_camel})); + "}, pass_type = pass_type, pass_lifetimes = pass_lifetimes, pass_name = pass_name,