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

Auto merge of #4994 - bradsherman:new_lint_gen, r=flip1995

Browse source 
Autogenerate new lints

Add option in clippy_dev to automatically generate boilerplate code for adding new lints

example:

`./util/dev new_lint --name=iter_nth_zero --type=late`

Fixes #4942

changelog: none
This commit is contained in:
bors 2020-01-16 16:24:41 +00:00
commit 8b72b7251c
7 changed files with 381 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
clippy_dev/src/main.rs
View file

@ -4,6 +4,7 @@ use clap::{App, Arg, SubCommand};
use clippy_dev::*;
mod fmt;
mod new_lint;
mod stderr_length_check;
#[derive(PartialEq)]
@ -51,6 +52,47 @@ fn main() {
.help("Checks that util/dev update_lints has been run. Used on CI."),
),
)
.subcommand(
SubCommand::with_name("new_lint")
.about("Create new lint and run util/dev update_lints")
.arg(
Arg::with_name("pass")
.short("p")
.long("pass")
.help("Specify whether the lint runs during the early or late pass")
.takes_value(true)
.possible_values(&["early", "late"])
.required(true),
)
.arg(
Arg::with_name("name")
.short("n")
.long("name")
.help("Name of the new lint in snake case, ex: fn_too_long")
.takes_value(true)
.required(true),
)
.arg(
Arg::with_name("category")
.short("c")
.long("category")
.help("What category the lint belongs to")
.default_value("nursery")
.possible_values(&[
"style",
"correctness",
"complexity",
"perf",
"pedantic",
"restriction",
"cargo",
"nursery",
"internal",
"internal_warn",
])
.takes_value(true),
),
)
.arg(
Arg::with_name("limit-stderr-length")
.long("limit-stderr-length")
@ -75,6 +117,16 @@ fn main() {
update_lints(&UpdateMode::Change);
}
},
("new_lint", Some(matches)) => {
match new_lint::create(
matches.value_of("pass"),
matches.value_of("name"),
matches.value_of("category"),
) {
Ok(_) => update_lints(&UpdateMode::Change),
Err(e) => eprintln!("Unable to create lint: {}", e),
}
},
_ => {},
}
}

182
clippy_dev/src/new_lint.rs Normal file
View file

@ -0,0 +1,182 @@
use std::fs::{File, OpenOptions};
use std::io;
use std::io::prelude::*;
use std::io::ErrorKind;
use std::path::{Path, PathBuf};
pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> Result<(), io::Error> {
let pass = pass.expect("`pass` argument is validated by clap");
let lint_name = lint_name.expect("`name` argument is validated by clap");
let category = category.expect("`category` argument is validated by clap");
match open_files(lint_name) {
Ok((mut test_file, mut lint_file)) => {
let (pass_type, pass_import, context_import) = match pass {
"early" => ("EarlyLintPass", "use syntax::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);
if let Err(e) = test_file.write_all(get_test_file_contents(lint_name).as_bytes()) {
return Err(io::Error::new(
ErrorKind::Other,
format!("Could not write to test file: {}", e),
));
};
if let Err(e) = lint_file.write_all(
get_lint_file_contents(
pass_type,
lint_name,
&camel_case_name,
category,
pass_import,
context_import,
)
.as_bytes(),
) {
return Err(io::Error::new(
ErrorKind::Other,
format!("Could not write to lint file: {}", e),
));
}
Ok(())
},
Err(e) => Err(io::Error::new(
ErrorKind::Other,
format!("Unable to create lint: {}", e),
)),
}
}
fn open_files(lint_name: &str) -> Result<(File, File), io::Error> {
let project_root = project_root()?;
let test_file_path = project_root.join("tests").join("ui").join(format!("{}.rs", lint_name));
let lint_file_path = project_root
.join("clippy_lints")
.join("src")
.join(format!("{}.rs", lint_name));
if Path::new(&test_file_path).exists() {
return Err(io::Error::new(
ErrorKind::AlreadyExists,
format!("test file {:?} already exists", test_file_path),
));
}
if Path::new(&lint_file_path).exists() {
return Err(io::Error::new(
ErrorKind::AlreadyExists,
format!("lint file {:?} already exists", lint_file_path),
));
}
let test_file = OpenOptions::new().write(true).create_new(true).open(test_file_path)?;
let lint_file = OpenOptions::new().write(true).create_new(true).open(lint_file_path)?;
Ok((test_file, lint_file))
}
fn project_root() -> Result<PathBuf, io::Error> {
let current_dir = std::env::current_dir()?;
for path in current_dir.ancestors() {
let result = std::fs::read_to_string(path.join("Cargo.toml"));
if let Err(err) = &result {
if err.kind() == io::ErrorKind::NotFound {
continue;
}
}
let content = result?;
if content.contains("[package]\nname = \"clippy\"") {
return Ok(path.to_path_buf());
}
}
Err(io::Error::new(ErrorKind::Other, "Unable to find project root"))
}
fn to_camel_case(name: &str) -> String {
name.split('_')
.map(|s| {
if s.is_empty() {
String::from("")
} else {
[&s[0..1].to_uppercase(), &s[1..]].concat()
}
})
.collect()
}
fn get_test_file_contents(lint_name: &str) -> String {
format!(
"#![warn(clippy::{})]
fn main() {{
// test code goes here
}}
",
lint_name
)
}
fn get_lint_file_contents(
pass_type: &str,
lint_name: &str,
camel_case_name: &str,
category: &str,
pass_import: &str,
context_import: &str,
) -> String {
format!(
"use rustc::lint::{{LintArray, LintPass, {type}, {context_import}}};
use rustc_session::{{declare_lint_pass, declare_tool_lint}};
{pass_import}
declare_clippy_lint! {{
/// **What it does:**
///
/// **Why is this bad?**
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// // example code
/// ```
pub {name_upper},
{category},
\"default lint description\"
}}
declare_lint_pass!({name_camel} => [{name_upper}]);
impl {type} for {name_camel} {{}}
",
type=pass_type,
name_upper=lint_name.to_uppercase(),
name_camel=camel_case_name,
category=category,
pass_import=pass_import,
context_import=context_import
)
}
#[test]
fn test_camel_case() {
let s = "a_lint";
let s2 = to_camel_case(s);
assert_eq!(s2, "ALint");
let name = "a_really_long_new_lint";
let name2 = to_camel_case(name);
assert_eq!(name2, "AReallyLongNewLint");
let name3 = "lint__name";
let name4 = to_camel_case(name3);
assert_eq!(name4, "LintName");
}

1
clippy_lints/src/lib.rs
View file

@ -1090,6 +1090,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
LintId::of(&utils::internal_lints::PRODUCE_ICE),
LintId::of(&utils::internal_lints::DEFAULT_LINT),
]);
store.register_group(true, "clippy::all", Some("clippy"), vec![

53
clippy_lints/src/utils/internal_lints.rs
View file

@ -13,10 +13,10 @@ use rustc_hir::*;
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
use rustc_session::declare_tool_lint;
use rustc_session::{declare_lint_pass, impl_lint_pass};
use rustc_span::source_map::Span;
use rustc_span::source_map::{Span, Spanned};
use rustc_span::symbol::SymbolStr;
use syntax::ast;
use syntax::ast::{Crate as AstCrate, ItemKind, Name};
use syntax::ast::{Crate as AstCrate, ItemKind, LitKind, Name};
use syntax::visit::FnKind;
declare_clippy_lint! {
@ -121,6 +121,29 @@ declare_clippy_lint! {
"this message should not appear anywhere as we ICE before and don't emit the lint"
}
declare_clippy_lint! {
/// **What it does:** Checks for cases of an auto-generated lint without an updated description,
/// i.e. `default lint description`.
///
/// **Why is this bad?** Indicates that the lint is not finished.
///
/// **Known problems:** None
///
/// **Example:**
/// Bad:
/// ```rust,ignore
/// declare_lint! { pub COOL_LINT, nursery, "default lint description" }
/// ```
///
/// Good:
/// ```rust,ignore
/// declare_lint! { pub COOL_LINT, nursery, "a great new lint" }
/// ```
pub DEFAULT_LINT,
internal,
"found 'default lint description' in a lint declaration"
}
declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);
impl EarlyLintPass for ClippyLintsInternal {
@ -163,12 +186,34 @@ pub struct LintWithoutLintPass {
registered_lints: FxHashSet<Name>,
}
impl_lint_pass!(LintWithoutLintPass => [LINT_WITHOUT_LINT_PASS]);
impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
if let hir::ItemKind::Static(ref ty, Mutability::Not, _) = item.kind {
if let hir::ItemKind::Static(ref ty, Mutability::Not, body_id) = item.kind {
if is_lint_ref_type(cx, ty) {
let expr = &cx.tcx.hir().body(body_id).value;
if_chain! {
if let ExprKind::AddrOf(_, _, ref inner_exp) = expr.kind;
if let ExprKind::Struct(_, ref fields, _) = inner_exp.kind;
let field = fields.iter()
.find(|f| f.ident.as_str() == "desc")
.expect("lints must have a description field");
if let ExprKind::Lit(Spanned {
node: LitKind::Str(ref sym, _),
..
}) = field.expr.kind;
if sym.as_str() == "default lint description";
then {
span_lint(
cx,
DEFAULT_LINT,
item.span,
&format!("the lint `{}` has the default lint description", item.ident.name),
);
}
}
self.declared_lints.insert(item.ident.name, item.span);
}
} else if is_expn_of(item.span, "impl_lint_pass").is_some()

76
doc/adding_lints.md
View file

@ -9,6 +9,7 @@ because that's clearly a non-descriptive name.
- [Adding a new lint](#adding-a-new-lint)
- [Setup](#setup)
- [Getting Started](#getting-started)
- [Testing](#testing)
- [Rustfix tests](#rustfix-tests)
- [Edition 2018 tests](#edition-2018-tests)
@ -31,6 +32,19 @@ which can change rapidly. Make sure you're working near rust-clippy's master,
and use the `setup-toolchain.sh` script to configure the appropriate toolchain
for the Clippy directory.
### 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
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). To get started on this lint you can run
`./util/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 run `./util/dev update_lints` to register the new lint. Next, we'll
open up these files and add our lint!
### Testing
Let's write some tests first that we can execute while we iterate on our lint.
@ -41,11 +55,9 @@ we want to check. The output of Clippy is compared against a `.stderr` file.
Note that you don't have to create this file yourself, we'll get to
generating the `.stderr` files further down.
We start by creating the test file at `tests/ui/foo_functions.rs`. It doesn't
really matter what the file is called, but it's a good convention to name it
after the lint it is testing, so `foo_functions.rs` it is.
We start by opening the test file created at `tests/ui/foo_functions.rs`.
Inside the file we put some examples to get started:
Update the file with some examples to get started:
```rust
#![warn(clippy::foo_functions)]
@ -90,8 +102,8 @@ Once we are satisfied with the output, we need to run
`tests/ui/update-all-references.sh` to update the `.stderr` file for our lint.
Please note that, we should run `TESTNAME=foo_functions cargo uitest`
every time before running `tests/ui/update-all-references.sh`.
Running `TESTNAME=foo_functions cargo uitest` should pass then. When we
commit our lint, we need to commit the generated `.stderr` files, too.
Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit
our lint, we need to commit the generated `.stderr` files, too.
### Rustfix tests
@ -121,26 +133,42 @@ With tests in place, let's have a look at implementing our lint now.
### Lint declaration
We start by creating a new file in the `clippy_lints` crate. That's the crate
where all the lint code is. We are going to call the file
`clippy_lints/src/foo_functions.rs` and import some initial things we need:
Let's start by opening the new file created in the `clippy_lints` crate
at `clippy_lints/src/foo_functions.rs`. That's the crate where all the
lint code is. This file has already imported some initial things we will need:
```rust
use rustc::lint::{LintArray, LintPass, EarlyLintPass};
use rustc::lint::{LintArray, LintPass, EarlyLintPass, EarlyContext};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use syntax::ast::*;
```
The next step is to provide a lint declaration. Lints are declared using the
[`declare_clippy_lint!`][declare_clippy_lint] macro:
The next step is to update the lint declaration. Lints are declared using the
[`declare_clippy_lint!`][declare_clippy_lint] macro, and we just need to update
the auto-generated lint declaration to have a real description, something like this:
```rust
declare_clippy_lint! {
/// **What it does:**
///
/// **Why is this bad?**
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// // example code
/// ```
pub FOO_FUNCTIONS,
pedantic,
"function named `foo`, which is not a descriptive name"
}
```
* The section of lines prefixed with `///` constitutes the lint documentation
section. This is the default documentation style and will be displayed at
https://rust-lang.github.io/rust-clippy/master/index.html.
* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the [lint naming
guidelines][lint_naming] here when naming your lint. In short, the name should
state the thing that is being checked for and read well when used with
@ -150,8 +178,8 @@ state the thing that is being checked for and read well when used with
* The last part should be a text that explains what exactly is wrong with the
code
With our lint declaration done, we will now make sure that it is assigned to a
lint pass:
The rest of this file contains an empty implementation for our lint pass,
which in this case is `EarlyLintPass` and should look like this:
```rust
// clippy_lints/src/foo_functions.rs
@ -166,12 +194,9 @@ impl EarlyLintPass for FooFunctions {}
Don't worry about the `name` method here. As long as it includes the name of the
lint pass it should be fine.
Next we need to run `util/dev update_lints` to register the lint in various
places, mainly in `clippy_lints/src/lib.rs`.
While `update_lints` automates some things, it doesn't automate everything. We
will have to register our lint pass manually in the `register_plugins` function
in `clippy_lints/src/lib.rs`:
The new lint automation runs `update_lints`, which automates some things, but it
doesn't automate everything. We will have to register our lint pass manually in
the `register_plugins` function in `clippy_lints/src/lib.rs`:
```rust
reg.register_early_lint_pass(box foo_functions::FooFunctions);
@ -195,14 +220,9 @@ In short, the `LateLintPass` has access to type information while the
`EarlyLintPass`. The `EarlyLintPass` is also faster. However linting speed
hasn't really been a concern with Clippy so far.
Since we don't need type information for checking the function name, we are
going to use the `EarlyLintPass`. It has to be imported as well, changing our
imports to:
```rust
use rustc::lint::{LintArray, LintPass, EarlyLintPass, EarlyContext};
use rustc::{declare_tool_lint, lint_array};
```
Since we don't need type information for checking the function name, we used
`--pass=early` when running the new lint automation and all the imports were
added accordingly.
### Emitting a lint

28
tests/ui/default_lint.rs Normal file
View file

@ -0,0 +1,28 @@
#![deny(clippy::internal)]
#![feature(rustc_private)]
#[macro_use]
extern crate rustc;
#[macro_use]
extern crate rustc_session;
extern crate rustc_lint;
use rustc_lint::{LintArray, LintPass};
declare_tool_lint! {
pub clippy::TEST_LINT,
Warn,
"",
report_in_external_macro: true
}
declare_tool_lint! {
pub clippy::TEST_LINT_DEFAULT,
Warn,
"default lint description",
report_in_external_macro: true
}
declare_lint_pass!(Pass => [TEST_LINT]);
declare_lint_pass!(Pass2 => [TEST_LINT_DEFAULT]);
fn main() {}

21
tests/ui/default_lint.stderr Normal file
View file

@ -0,0 +1,21 @@
error: the lint `TEST_LINT_DEFAULT` has the default lint description
--> $DIR/default_lint.rs:18:1
|
LL | / declare_tool_lint! {
LL | | pub clippy::TEST_LINT_DEFAULT,
LL | | Warn,
LL | | "default lint description",
LL | | report_in_external_macro: true
LL | | }
| |_^
|
note: lint level defined here
--> $DIR/default_lint.rs:1:9
|
LL | #![deny(clippy::internal)]
| ^^^^^^^^^^^^^^^^
= note: `#[deny(clippy::default_lint)]` implied by `#[deny(clippy::internal)]`
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
error: aborting due to previous error