Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-14 08:57:30 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #9182 - Serial-ATA:new_lint-type-flag, r=xFrednet

Browse source 
Add `--type` flag to `dev new_lint`

changelog: none

This only works with directories, `--type={copies, doc, ...}` won't work. Should they be counted?
This commit is contained in:
bors 2022-07-27 22:17:29 +00:00
commit cdd4dfc6d6
7 changed files with 355 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
book/src/development/adding_lints.md
View file

@ -10,6 +10,10 @@ because that's clearly a non-descriptive name.
- [Adding a new lint](#adding-a-new-lint)
- [Setup](#setup)
- [Getting Started](#getting-started)
- [Defining Our Lint](#defining-our-lint)
- [Standalone](#standalone)
- [Specific Type](#specific-type)
- [Tests Location](#tests-location)
- [Testing](#testing)
- [Cargo lints](#cargo-lints)
- [Rustfix tests](#rustfix-tests)
@ -36,17 +40,38 @@ See the [Basics](basics.md#get-the-code) documentation.
## Getting Started
There is a bit of boilerplate code that needs to be set up when creating a new
lint. Fortunately, you can use the clippy dev tools to handle this for you. We
lint. Fortunately, you can use the Clippy dev tools to handle this for you. We
are naming our new lint `foo_functions` (lints are generally written in snake
case), and we don't need type information so it will have an early pass type
(more on this later on). If you're not sure if the name you chose fits the lint,
take a look at our [lint naming guidelines][lint_naming]. To get started on this
lint you can run `cargo dev new_lint --name=foo_functions --pass=early
--category=pedantic` (category will default to nursery if not provided). This
command will create two files: `tests/ui/foo_functions.rs` and
`clippy_lints/src/foo_functions.rs`, as well as [registering the
lint](#lint-registration). For cargo lints, two project hierarchies (fail/pass)
will be created by default under `tests/ui-cargo`.
case), and we don't need type information, so it will have an early pass type
(more on this later). If you're unsure if the name you chose fits the lint,
take a look at our [lint naming guidelines][lint_naming].
## Defining Our Lint
To get started, there are two ways to define our lint.
### Standalone
Command: `cargo dev new_lint --name=foo_functions --pass=early --category=pedantic`
(category will default to nursery if not provided)
This command will create a new file: `clippy_lints/src/foo_functions.rs`, as well
as [register the lint](#lint-registration).
### Specific Type
Command: `cargo dev new_lint --name=foo_functions --type=functions --category=pedantic`
This command will create a new file: `clippy_lints/src/{type}/foo_functions.rs`.
Notice how this command has a `--type` flag instead of `--pass`. Unlike a standalone
definition, this lint won't be registered in the traditional sense. Instead, you will
call your lint from within the type's lint pass, found in `clippy_lints/src/{type}/mod.rs`.
A "type" is just the name of a directory in `clippy_lints/src`, like `functions` in
the example command. These are groupings of lints with common behaviors, so if your
lint falls into one, it would be best to add it to that type.
### Tests Location
Both commands will create a file: `tests/ui/foo_functions.rs`. For cargo lints,
two project hierarchies (fail/pass) will be created by default under `tests/ui-cargo`.
Next, we'll open up these files and add our lint!

11
clippy_dev/src/main.rs
View file

@ -36,7 +36,8 @@ fn main() {
match new_lint::create(
matches.get_one::<String>("pass"),
matches.get_one::<String>("name"),
matches.get_one::<String>("category"),
matches.get_one::<String>("category").map(String::as_str),
matches.get_one::<String>("type").map(String::as_str),
matches.contains_id("msrv"),
) {
Ok(_) => update_lints::update(update_lints::UpdateMode::Change),
@ -157,7 +158,8 @@ fn get_clap_config() -> ArgMatches {
.help("Specify whether the lint runs during the early or late pass")
.takes_value(true)
.value_parser([PossibleValue::new("early"), PossibleValue::new("late")])
.required(true),
.conflicts_with("type")
.required_unless_present("type"),
Arg::new("name")
.short('n')
.long("name")
@ -183,6 +185,11 @@ fn get_clap_config() -> ArgMatches {
PossibleValue::new("internal_warn"),
])
.takes_value(true),
Arg::new("type")
.long("type")
.help("What directory the lint belongs in")
.takes_value(true)
.required(false),
Arg::new("msrv").long("msrv").help("Add MSRV config code to the lint"),
]),
Command::new("setup")

321
clippy_dev/src/new_lint.rs
View file

@ -1,5 +1,5 @@
use crate::clippy_project_root;
use indoc::indoc;
use indoc::{indoc, writedoc};
use std::fmt::Write as _;
use std::fs::{self, OpenOptions};
use std::io::prelude::*;
@ -10,6 +10,7 @@ struct LintData<'a> {
pass: &'a str,
name: &'a str,
category: &'a str,
ty: Option<&'a str>,
project_root: PathBuf,
}
@ -37,26 +38,44 @@ impl<T> Context for io::Result<T> {
pub fn create(
pass: Option<&String>,
lint_name: Option<&String>,
category: Option<&String>,
category: Option<&str>,
mut ty: Option<&str>,
msrv: bool,
) -> io::Result<()> {
if category == Some("cargo") && ty.is_none() {
// `cargo` is a special category, these lints should always be in `clippy_lints/src/cargo`
ty = Some("cargo");
}
let lint = LintData {
pass: pass.expect("`pass` argument is validated by clap"),
pass: pass.map_or("", String::as_str),
name: lint_name.expect("`name` argument is validated by clap"),
category: category.expect("`category` argument is validated by clap"),
ty,
project_root: clippy_project_root(),
};
create_lint(&lint, msrv).context("Unable to create lint implementation")?;
create_test(&lint).context("Unable to create a test for the new lint")?;
add_lint(&lint, msrv).context("Unable to add lint to clippy_lints/src/lib.rs")
if lint.ty.is_none() {
add_lint(&lint, msrv).context("Unable to add lint to clippy_lints/src/lib.rs")?;
}
Ok(())
}
fn create_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> {
let lint_contents = get_lint_file_contents(lint, enable_msrv);
if let Some(ty) = lint.ty {
create_lint_for_ty(lint, enable_msrv, ty)
} else {
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())?;
println!("Generated lint file: `{}`", lint_path);
let lint_path = format!("clippy_lints/src/{}.rs", lint.name);
write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes())
Ok(())
}
}
fn create_test(lint: &LintData<'_>) -> io::Result<()> {
@ -75,16 +94,22 @@ fn create_test(lint: &LintData<'_>) -> io::Result<()> {
if lint.category == "cargo" {
let relative_test_dir = format!("tests/ui-cargo/{}", lint.name);
let test_dir = lint.project_root.join(relative_test_dir);
let test_dir = lint.project_root.join(&relative_test_dir);
fs::create_dir(&test_dir)?;
create_project_layout(lint.name, &test_dir, "fail", "Content that triggers the lint goes here")?;
create_project_layout(lint.name, &test_dir, "pass", "This file should not trigger the lint")
create_project_layout(lint.name, &test_dir, "pass", "This file should not trigger the lint")?;
println!("Generated test directories: `{relative_test_dir}/pass`, `{relative_test_dir}/fail`");
} else {
let test_path = format!("tests/ui/{}.rs", lint.name);
let test_contents = get_test_file_contents(lint.name, None);
write_file(lint.project_root.join(test_path), test_contents)
write_file(lint.project_root.join(&test_path), test_contents)?;
println!("Generated test file: `{}`", test_path);
}
Ok(())
}
fn add_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> {
@ -204,7 +229,6 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
},
};
let version = get_stabilization_version();
let lint_name = lint.name;
let category = lint.category;
let name_camel = to_camel_case(lint.name);
@ -238,32 +262,7 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
)
});
let _ = write!(
result,
indoc! {r#"
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
/// ```
#[clippy::version = "{version}"]
pub {name_upper},
{category},
"default lint description"
}}
"#},
version = version,
name_upper = name_upper,
category = category,
);
let _ = write!(result, "{}", get_lint_declaration(&name_upper, category));
result.push_str(&if enable_msrv {
format!(
@ -312,6 +311,254 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
result
}
fn get_lint_declaration(name_upper: &str, category: &str) -> String {
format!(
indoc! {r#"
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
/// ```
#[clippy::version = "{version}"]
pub {name_upper},
{category},
"default lint description"
}}
"#},
version = get_stabilization_version(),
name_upper = name_upper,
category = category,
)
}
fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::Result<()> {
match ty {
"cargo" => assert_eq!(
lint.category, "cargo",
"Lints of type `cargo` must have the `cargo` category"
),
_ if lint.category == "cargo" => panic!("Lints of category `cargo` must have the `cargo` type"),
_ => {},
}
let ty_dir = lint.project_root.join(format!("clippy_lints/src/{}", ty));
assert!(
ty_dir.exists() && ty_dir.is_dir(),
"Directory `{}` does not exist!",
ty_dir.display()
);
let lint_file_path = ty_dir.join(format!("{}.rs", lint.name));
assert!(
!lint_file_path.exists(),
"File `{}` already exists",
lint_file_path.display()
);
let mod_file_path = ty_dir.join("mod.rs");
let context_import = setup_mod_file(&mod_file_path, lint)?;
let name_upper = lint.name.to_uppercase();
let mut lint_file_contents = String::new();
if enable_msrv {
let _ = writedoc!(
lint_file_contents,
r#"
use clippy_utils::{{meets_msrv, msrvs}};
use rustc_lint::{{{context_import}, LintContext}};
use rustc_semver::RustcVersion;
use super::{name_upper};
// TODO: Adjust the parameters as necessary
pub(super) fn check(cx: &{context_import}, msrv: Option<RustcVersion>) {{
if !meets_msrv(msrv, todo!("Add a new entry in `clippy_utils/src/msrvs`")) {{
return;
}}
todo!();
}}
"#,
context_import = context_import,
name_upper = name_upper,
);
} else {
let _ = writedoc!(
lint_file_contents,
r#"
use rustc_lint::{{{context_import}, LintContext}};
use super::{name_upper};
// TODO: Adjust the parameters as necessary
pub(super) fn check(cx: &{context_import}) {{
todo!();
}}
"#,
context_import = context_import,
name_upper = name_upper,
);
}
write_file(lint_file_path.as_path(), lint_file_contents)?;
println!("Generated lint file: `clippy_lints/src/{}/{}.rs`", ty, lint.name);
println!(
"Be sure to add a call to `{}::check` in `clippy_lints/src/{}/mod.rs`!",
lint.name, ty
);
Ok(())
}
#[allow(clippy::too_many_lines)]
fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str> {
use super::update_lints::{match_tokens, LintDeclSearchResult};
use rustc_lexer::TokenKind;
let lint_name_upper = lint.name.to_uppercase();
let mut file_contents = fs::read_to_string(path)?;
assert!(
!file_contents.contains(&lint_name_upper),
"Lint `{}` already defined in `{}`",
lint.name,
path.display()
);
let mut offset = 0usize;
let mut last_decl_curly_offset = None;
let mut lint_context = None;
let mut iter = rustc_lexer::tokenize(&file_contents).map(|t| {
let range = offset..offset + t.len;
offset = range.end;
LintDeclSearchResult {
token_kind: t.kind,
content: &file_contents[range.clone()],
range,
}
});
// Find both the last lint declaration (declare_clippy_lint!) and the lint pass impl
while let Some(LintDeclSearchResult { content, .. }) = iter.find(|result| result.token_kind == TokenKind::Ident) {
let mut iter = iter
.by_ref()
.filter(|t| !matches!(t.token_kind, TokenKind::Whitespace | TokenKind::LineComment { .. }));
match content {
"declare_clippy_lint" => {
// matches `!{`
match_tokens!(iter, Bang OpenBrace);
if let Some(LintDeclSearchResult { range, .. }) =
iter.find(|result| result.token_kind == TokenKind::CloseBrace)
{
last_decl_curly_offset = Some(range.end);
}
},
"impl" => {
let mut token = iter.next();
match token {
// matches <'foo>
Some(LintDeclSearchResult {
token_kind: TokenKind::Lt,
..
}) => {
match_tokens!(iter, Lifetime { .. } Gt);
token = iter.next();
},
None => break,
_ => {},
}
if let Some(LintDeclSearchResult {
token_kind: TokenKind::Ident,
content,
..
}) = token
{
// Get the appropriate lint context struct
lint_context = match content {
"LateLintPass" => Some("LateContext"),
"EarlyLintPass" => Some("EarlyContext"),
_ => continue,
};
}
},
_ => {},
}
}
drop(iter);
let last_decl_curly_offset =
last_decl_curly_offset.unwrap_or_else(|| panic!("No lint declarations found in `{}`", path.display()));
let lint_context =
lint_context.unwrap_or_else(|| panic!("No lint pass implementation found in `{}`", path.display()));
// Add the lint declaration to `mod.rs`
file_contents.replace_range(
// Remove the trailing newline, which should always be present
last_decl_curly_offset..=last_decl_curly_offset,
&format!("\n\n{}", get_lint_declaration(&lint_name_upper, lint.category)),
);
// Add the lint to `impl_lint_pass`/`declare_lint_pass`
let impl_lint_pass_start = file_contents.find("impl_lint_pass!").unwrap_or_else(|| {
file_contents
.find("declare_lint_pass!")
.unwrap_or_else(|| panic!("failed to find `impl_lint_pass`/`declare_lint_pass`"))
});
let mut arr_start = file_contents[impl_lint_pass_start..].find('[').unwrap_or_else(|| {
panic!("malformed `impl_lint_pass`/`declare_lint_pass`");
});
arr_start += impl_lint_pass_start;
let mut arr_end = file_contents[arr_start..]
.find(']')
.expect("failed to find `impl_lint_pass` terminator");
arr_end += arr_start;
let mut arr_content = file_contents[arr_start + 1..arr_end].to_string();
arr_content.retain(|c| !c.is_whitespace());
let mut new_arr_content = String::new();
for ident in arr_content
.split(',')
.chain(std::iter::once(&*lint_name_upper))
.filter(|s| !s.is_empty())
{
let _ = write!(new_arr_content, "\n {},", ident);
}
new_arr_content.push('\n');
file_contents.replace_range(arr_start + 1..arr_end, &new_arr_content);
// Just add the mod declaration at the top, it'll be fixed by rustfmt
file_contents.insert_str(0, &format!("mod {};\n", &lint.name));
let mut file = OpenOptions::new()
.write(true)
.truncate(true)
.open(path)
.context(format!("trying to open: `{}`", path.display()))?;
file.write_all(file_contents.as_bytes())
.context(format!("writing to file: `{}`", path.display()))?;
Ok(lint_context)
}
#[test]
fn test_camel_case() {
let s = "a_lint";

10
clippy_dev/src/update_lints.rs
View file

@ -824,10 +824,12 @@ macro_rules! match_tokens {
}
}
struct LintDeclSearchResult<'a> {
token_kind: TokenKind,
content: &'a str,
range: Range<usize>,
pub(crate) use match_tokens;
pub(crate) struct LintDeclSearchResult<'a> {
pub token_kind: TokenKind,
pub content: &'a str,
pub range: Range<usize>,
}
/// Parse a source file looking for `declare_clippy_lint` macro invocations.

10
clippy_lints/src/cargo/mod.rs
View file

@ -1,3 +1,8 @@
mod common_metadata;
mod feature_name;
mod multiple_crate_versions;
mod wildcard_dependencies;
use cargo_metadata::MetadataCommand;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_lint_allowed;
@ -6,11 +11,6 @@ use rustc_lint::{LateContext, LateLintPass, Lint};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::DUMMY_SP;
mod common_metadata;
mod feature_name;
mod multiple_crate_versions;
mod wildcard_dependencies;
declare_clippy_lint! {
/// ### What it does
/// Checks to see if all common metadata is defined in

20
clippy_lints/src/matches/mod.rs
View file

@ -1,13 +1,3 @@
use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context};
use clippy_utils::{higher, in_constant, meets_msrv, msrvs};
use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{Span, SpanData, SyntaxContext};
mod collapsible_match;
mod infallible_destructuring_match;
mod manual_map;
@ -31,6 +21,16 @@ mod single_match;
mod try_err;
mod wild_in_or_pats;
use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context};
use clippy_utils::{higher, in_constant, meets_msrv, msrvs};
use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{Span, SpanData, SyntaxContext};
declare_clippy_lint! {
/// ### What it does
/// Checks for matches with a single arm where an `if let`

12
clippy_lints/src/operators/mod.rs
View file

@ -1,9 +1,3 @@
use rustc_hir::{Body, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
pub(crate) mod arithmetic;
mod absurd_extreme_comparisons;
mod assign_op_pattern;
mod bit_mask;
@ -27,6 +21,12 @@ mod ptr_eq;
mod self_assignment;
mod verbose_bit_mask;
pub(crate) mod arithmetic;
use rustc_hir::{Body, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
declare_clippy_lint! {
/// ### What it does
/// Checks for comparisons where one side of the relation is