-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Verify that versions in the changelog match the lint definitions #14821
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
#![feature(rustc_private)] | ||
|
||
use std::collections::HashMap; | ||
use std::fs; | ||
|
||
use clippy_lints::declared_lints::LINTS; | ||
use clippy_lints::deprecated_lints::RENAMED; | ||
use pulldown_cmark::{Event, HeadingLevel, Parser, Tag, TagEnd}; | ||
use test_utils::IS_RUSTC_TEST_SUITE; | ||
|
||
mod test_utils; | ||
|
||
#[test] | ||
fn versions_match_changelog() { | ||
if IS_RUSTC_TEST_SUITE { | ||
return; | ||
} | ||
|
||
let changelog = fs::read_to_string("CHANGELOG.md").unwrap(); | ||
|
||
let mut versions_by_name: HashMap<_, _> = LINTS.iter().map(|&lint| (lint.name_lower(), lint)).collect(); | ||
|
||
for (from, to) in RENAMED { | ||
let from = from.strip_prefix("clippy::").unwrap(); | ||
if let Some(to) = to.strip_prefix("clippy::") { | ||
versions_by_name.insert(from.to_owned(), versions_by_name[to]); | ||
} | ||
} | ||
|
||
let mut heading = None; | ||
let mut changelog_version = None; | ||
let mut in_new_lints = true; | ||
let mut checked = 0; | ||
|
||
for event in Parser::new(&changelog) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This parsing seems a bit brittle. It relies on the changelog format never changing. Do you have ideas how we can make this more robust? For example, add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add a note next to the template in the docs, it should be visible in the review diff if the |
||
match event { | ||
Event::Start(Tag::Heading { level, .. }) => { | ||
in_new_lints = false; | ||
heading = Some(level); | ||
}, | ||
Event::End(TagEnd::Heading(_)) => heading = None, | ||
Event::Text(text) => match heading { | ||
Some(HeadingLevel::H2) => { | ||
if let Some(v) = text.strip_prefix("Rust ") { | ||
changelog_version = Some(v.to_owned()); | ||
} | ||
}, | ||
Some(HeadingLevel::H3) => { | ||
in_new_lints = text.eq_ignore_ascii_case("new lints"); | ||
}, | ||
_ => {}, | ||
}, | ||
Event::Start(Tag::Link { id, .. }) if in_new_lints => { | ||
if let Some(name) = id.strip_prefix('`') | ||
&& let Some(name) = name.strip_suffix('`') | ||
&& let Some(&lint) = versions_by_name.get(name) | ||
{ | ||
let lint_version = lint.version.strip_suffix(".0").unwrap(); | ||
let changelog_version = changelog_version.as_deref().unwrap(); | ||
assert_eq!( | ||
lint_version, | ||
changelog_version, | ||
"{name} has version {lint_version} but appears in the changelog for {changelog_version}\n\ | ||
\n\ | ||
update {} to `#[clippy::version = \"{changelog_version}.0\"]`", | ||
lint.location_terminal(), | ||
); | ||
checked += 1; | ||
} | ||
}, | ||
_ => {}, | ||
} | ||
} | ||
|
||
assert!( | ||
checked > 400, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this magic number coming from? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was the number at the time rounded down to the nearest 100, just a basic check that if the format of the entire changelog ever changed or it became a stub linking to a site or something like that then the test would fail |
||
"only checked {checked} versions, did the changelog format change?" | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this now necessary 🤔
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
file!()
string literal has a strange span (e.g.clippy_lints/src/unicode.rs:56:1: 73:2 (#0)
), it points to the wholedeclare_clippy_lint! { ... }
callsite. The lints pick up that snippet and see the characters in the doc commentBefore it was part of a
concat
rather than its own expression