Skip to content

Create new lints with #[clippy::version = "nightly"] #14299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions book/src/development/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ declare_clippy_lint! {
/// ```rust
/// // example code
/// ```
#[clippy::version = "1.29.0"]
#[clippy::version = "nightly"]
pub FOO_FUNCTIONS,
pedantic,
"function named `foo`, which is not a descriptive name"
Expand Down Expand Up @@ -587,7 +587,7 @@ declare_clippy_lint! {
/// ```rust,ignore
/// // A short example of improved code that doesn't trigger the lint
/// ```
#[clippy::version = "1.29.0"]
#[clippy::version = "nightly"]
pub FOO_FUNCTIONS,
pedantic,
"function named `foo`, which is not a descriptive name"
Expand Down
4 changes: 2 additions & 2 deletions book/src/development/defining_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ declare_clippy_lint! {
/// ```rust
/// // example code which does not raise Clippy warning
/// ```
#[clippy::version = "1.70.0"] // <- In which version was this implemented, keep it up to date!
#[clippy::version = "nightly"]
pub LINT_NAME, // <- The lint name IN_ALL_CAPS
pedantic, // <- The lint group
"default lint description" // <- A lint description, e.g. "A function has an unit return type."
"default lint description" // <- A lint description, e.g. "A function has an unit return type"
}
```

Expand Down
9 changes: 7 additions & 2 deletions book/src/development/infrastructure/changelog_update.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ that label in the changelog. If you can, remove the `beta-accepted` labels

### 4. Update `clippy::version` attributes

Next, make sure to check that the `#[clippy::version]` attributes for the added
lints contain the correct version.
Next, make sure to check that the `#[clippy::version]` attributes for the newly
added and deprecated lints contain the version of the release you're writing the
changelog for.

In order to find lints that need a version update, go through the lints in the
"New Lints" section and run the following command for each lint name:

Expand All @@ -123,6 +125,9 @@ grep -rB1 "pub $LINT_NAME" .
The version shown should match the version of the release the changelog is
written for. If not, update the version to the changelog version.

Newly created lints will have `#[clippy::version = "nightly"]` and be handled
during the sync, but many existing PRs will still have an incorrect version.

[changelog]: https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md
[forge]: https://forge.rust-lang.org/
[rust_master_tools]: https://github.com/rust-lang/rust/tree/master/src/tools/clippy
Expand Down
11 changes: 3 additions & 8 deletions clippy_dev/src/deprecate_lint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::update_lints::{
DeprecatedLint, DeprecatedLints, Lint, find_lint_decls, generate_lint_files, read_deprecated_lints,
};
use crate::utils::{UpdateMode, Version};
use crate::utils::UpdateMode;
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::{fs, io};
Expand All @@ -15,7 +15,7 @@ use std::{fs, io};
/// # Panics
///
/// If a file path could not read from or written to
pub fn deprecate(clippy_version: Version, name: &str, reason: &str) {
pub fn deprecate(name: &str, reason: &str) {
let prefixed_name = if name.starts_with("clippy::") {
name.to_owned()
} else {
Expand Down Expand Up @@ -51,12 +51,7 @@ pub fn deprecate(clippy_version: Version, name: &str, reason: &str) {
if remove_lint_declaration(stripped_name, &mod_path, &mut lints).unwrap_or(false) {
deprecated_contents.insert_str(
deprecated_end as usize,
&format!(
" #[clippy::version = \"{}\"]\n (\"{}\", \"{}\"),\n",
clippy_version.rust_display(),
prefixed_name,
reason,
),
&format!(" #[clippy::version = \"nightly\"]\n (\"{prefixed_name}\", \"{reason}\"),\n"),
);
deprecated_file.replace_contents(deprecated_contents.as_bytes());
drop(deprecated_file);
Expand Down
11 changes: 3 additions & 8 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn main() {
category,
r#type,
msrv,
} => match new_lint::create(clippy.version, pass, &name, &category, r#type.as_deref(), msrv) {
} => match new_lint::create(pass, &name, &category, r#type.as_deref(), msrv) {
Ok(()) => update_lints::update(utils::UpdateMode::Change),
Err(e) => eprintln!("Unable to create lint: {e}"),
},
Expand Down Expand Up @@ -78,13 +78,8 @@ fn main() {
old_name,
new_name,
uplift,
} => rename_lint::rename(
clippy.version,
&old_name,
new_name.as_ref().unwrap_or(&old_name),
uplift,
),
DevCommand::Deprecate { name, reason } => deprecate_lint::deprecate(clippy.version, &name, &reason),
} => rename_lint::rename(&old_name, new_name.as_ref().unwrap_or(&old_name), uplift),
DevCommand::Deprecate { name, reason } => deprecate_lint::deprecate(&name, &reason),
DevCommand::Sync(SyncCommand { subcommand }) => match subcommand {
SyncSubcommand::UpdateNightly => sync::update_nightly(),
},
Expand Down
32 changes: 8 additions & 24 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{RustSearcher, Token, Version};
use crate::utils::{RustSearcher, Token};
use clap::ValueEnum;
use indoc::{formatdoc, writedoc};
use std::fmt::{self, Write as _};
Expand All @@ -22,7 +22,6 @@ impl fmt::Display for Pass {
}

struct LintData<'a> {
clippy_version: Version,
pass: Pass,
name: &'a str,
category: &'a str,
Expand Down Expand Up @@ -50,21 +49,13 @@ impl<T> Context for io::Result<T> {
/// # Errors
///
/// This function errors out if the files couldn't be created or written to.
pub fn create(
clippy_version: Version,
pass: Pass,
name: &str,
category: &str,
mut ty: Option<&str>,
msrv: bool,
) -> io::Result<()> {
pub fn create(pass: Pass, name: &str, category: &str, mut ty: Option<&str>, msrv: bool) -> io::Result<()> {
if category == "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 {
clippy_version,
pass,
name,
category,
Expand Down Expand Up @@ -293,11 +284,7 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
);
}

let _: fmt::Result = writeln!(
result,
"{}",
get_lint_declaration(lint.clippy_version, &name_upper, category)
);
let _: fmt::Result = writeln!(result, "{}", get_lint_declaration(&name_upper, category));

if enable_msrv {
let _: fmt::Result = writedoc!(
Expand Down Expand Up @@ -335,7 +322,7 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
result
}

fn get_lint_declaration(version: Version, name_upper: &str, category: &str) -> String {
fn get_lint_declaration(name_upper: &str, category: &str) -> String {
let justification_heading = if category == "restriction" {
"Why restrict this?"
} else {
Expand All @@ -356,12 +343,12 @@ fn get_lint_declaration(version: Version, name_upper: &str, category: &str) -> S
/// ```no_run
/// // example code which does not raise clippy warning
/// ```
#[clippy::version = "{}"]
#[clippy::version = "nightly"]
pub {name_upper},
{category},
"default lint description"
}}"#,
version.rust_display(),
}}
"#,
)
}

Expand Down Expand Up @@ -460,10 +447,7 @@ fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str>
// Add the lint declaration to `mod.rs`
file_contents.insert_str(
lint_decl_end,
&format!(
"\n\n{}",
get_lint_declaration(lint.clippy_version, &lint_name_upper, lint.category)
),
&format!("\n\n{}", get_lint_declaration(&lint_name_upper, lint.category)),
);

// Add the lint to `impl_lint_pass`/`declare_lint_pass`
Expand Down
10 changes: 4 additions & 6 deletions clippy_dev/src/rename_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::update_lints::{
DeprecatedLints, RenamedLint, find_lint_decls, gen_renamed_lints_test_fn, generate_lint_files,
read_deprecated_lints,
};
use crate::utils::{FileUpdater, StringReplacer, UpdateMode, Version, try_rename_file};
use crate::utils::{FileUpdater, StringReplacer, UpdateMode, try_rename_file};
use std::ffi::OsStr;
use std::path::Path;
use walkdir::WalkDir;
Expand All @@ -23,7 +23,7 @@ use walkdir::WalkDir;
/// * If `old_name` doesn't name an existing lint.
/// * If `old_name` names a deprecated or renamed lint.
#[allow(clippy::too_many_lines)]
pub fn rename(clippy_version: Version, old_name: &str, new_name: &str, uplift: bool) {
pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
if let Some((prefix, _)) = old_name.split_once("::") {
panic!("`{old_name}` should not contain the `{prefix}` prefix");
}
Expand Down Expand Up @@ -89,10 +89,8 @@ pub fn rename(clippy_version: Version, old_name: &str, new_name: &str, uplift: b
deprecated_contents.insert_str(
renamed_end as usize,
&format!(
" #[clippy::version = \"{}\"]\n (\"{}\", \"{}\"),\n",
clippy_version.rust_display(),
lint.old_name,
lint.new_name,
" #[clippy::version = \"nightly\"]\n (\"{}\", \"{}\"),\n",
lint.old_name, lint.new_name,
),
);
deprecated_file.replace_contents(deprecated_contents.as_bytes());
Expand Down
29 changes: 13 additions & 16 deletions clippy_dev/src/update_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,21 +244,16 @@ pub fn read_deprecated_lints() -> DeprecatedLints {
#[allow(clippy::enum_glob_use)]
use Token::*;
#[rustfmt::skip]
static DECL_TOKENS: &[Token] = &[
static VERSIONED_DECL: &[Token] = &[
// #[clippy::version = "version"]
Pound, OpenBracket, Ident("clippy"), DoubleColon, Ident("version"), Eq, LitStr, CloseBracket,
// ("first", "second"),
OpenParen, CaptureLitStr, Comma, CaptureLitStr, CloseParen, Comma,
];
#[rustfmt::skip]
static DEPRECATED_TOKENS: &[Token] = &[
// !{ DEPRECATED(DEPRECATED_VERSION) = [
Bang, OpenBrace, Ident("DEPRECATED"), OpenParen, Ident("DEPRECATED_VERSION"), CloseParen, Eq, OpenBracket,
];
#[rustfmt::skip]
static RENAMED_TOKENS: &[Token] = &[
// !{ RENAMED(RENAMED_VERSION) = [
Bang, OpenBrace, Ident("RENAMED"), OpenParen, Ident("RENAMED_VERSION"), CloseParen, Eq, OpenBracket,
static UNVERSIONED_DECL: &[Token] = &[
// ("first", "second"),
OpenParen, CaptureLitStr, Comma, CaptureLitStr, CloseParen, Comma,
];

let path = "clippy_lints/src/deprecated_lints.rs";
Expand All @@ -276,14 +271,14 @@ pub fn read_deprecated_lints() -> DeprecatedLints {

// First instance is the macro definition.
assert!(
searcher.find_token(Ident("declare_with_version")),
searcher.find_token(Ident("deprecated")),
"error reading deprecated lints"
);

if searcher.find_token(Ident("declare_with_version")) && searcher.match_tokens(DEPRECATED_TOKENS, &mut []) {
if searcher.find_token(Ident("deprecated")) && searcher.match_tokens(&[Bang, OpenBracket], &mut []) {
let mut name = "";
let mut reason = "";
while searcher.match_tokens(DECL_TOKENS, &mut [&mut name, &mut reason]) {
while searcher.match_tokens(VERSIONED_DECL, &mut [&mut name, &mut reason]) {
res.deprecated.push(DeprecatedLint {
name: parse_str_single_line(path.as_ref(), name),
reason: parse_str_single_line(path.as_ref(), reason),
Expand All @@ -292,13 +287,15 @@ pub fn read_deprecated_lints() -> DeprecatedLints {
} else {
panic!("error reading deprecated lints");
}
// position of the closing `]}` of `declare_with_version`
// position of the closing `];` of `deprecated`
res.deprecated_end = searcher.pos();

if searcher.find_token(Ident("declare_with_version")) && searcher.match_tokens(RENAMED_TOKENS, &mut []) {
// pub const RENAMED: &[(&str, &str)] = &[
// ^^^^^^^ ^ ^
if searcher.find_token(Ident("RENAMED")) && searcher.find_token(Eq) && searcher.find_token(OpenBracket) {
let mut old_name = "";
let mut new_name = "";
while searcher.match_tokens(DECL_TOKENS, &mut [&mut old_name, &mut new_name]) {
while searcher.match_tokens(UNVERSIONED_DECL, &mut [&mut old_name, &mut new_name]) {
res.renamed.push(RenamedLint {
old_name: parse_str_single_line(path.as_ref(), old_name),
new_name: parse_str_single_line(path.as_ref(), new_name),
Expand All @@ -307,7 +304,7 @@ pub fn read_deprecated_lints() -> DeprecatedLints {
} else {
panic!("error reading renamed lints");
}
// position of the closing `]}` of `declare_with_version`
// position of the closing `];` of `RENAMED`
res.renamed_end = searcher.pos();

res
Expand Down
Loading