Skip to content
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.

Sign up for GitHub

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

Fix update_lints #5406

Merged
merged 7 commits into from
Apr 8, 2020
Merged

Fix update_lints #5406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ffb2e41
Clean up update_lints
flip1995 Apr 2, 2020
da67982
Get rid of Lint::is_internal method
flip1995 Apr 2, 2020
98c30fe
Build lint lists once and the reuse them to update files
flip1995 Apr 2, 2020
a186d9f
Don't filter lints in code generation functions
flip1995 Apr 2, 2020
d89bb50
Make lint modules private
flip1995 Apr 2, 2020
045722a
Run update_lints
flip1995 Apr 2, 2020
30503a9
Move matches test in matches module
flip1995 Apr 3, 2020
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
128 changes: 53 additions & 75 deletions clippy_dev/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,110 +62,89 @@ impl Lint {
}

/// Returns all non-deprecated lints and non-internal lints
pub fn usable_lints(lints: impl Iterator<Item = Self>) -> impl Iterator<Item = Self> {
lints.filter(|l| l.deprecation.is_none() && !l.is_internal())
#[must_use]
pub fn usable_lints(lints: &[Self]) -> Vec<Self> {
lints
.iter()
.filter(|l| l.deprecation.is_none() && !l.group.starts_with("internal"))
.cloned()
.collect()
}

/// Returns all internal lints (not `internal_warn` lints)
pub fn internal_lints(lints: impl Iterator<Item = Self>) -> impl Iterator<Item = Self> {
lints.filter(|l| l.group == "internal")
#[must_use]
pub fn internal_lints(lints: &[Self]) -> Vec<Self> {
lints.iter().filter(|l| l.group == "internal").cloned().collect()
}

/// Returns the lints in a `HashMap`, grouped by the different lint groups
/// Returns all deprecated lints
#[must_use]
pub fn by_lint_group(lints: impl Iterator<Item = Self>) -> HashMap<String, Vec<Self>> {
lints.map(|lint| (lint.group.to_string(), lint)).into_group_map()
pub fn deprecated_lints(lints: &[Self]) -> Vec<Self> {
lints.iter().filter(|l| l.deprecation.is_some()).cloned().collect()
}

/// Returns the lints in a `HashMap`, grouped by the different lint groups
#[must_use]
pub fn is_internal(&self) -> bool {
self.group.starts_with("internal")
pub fn by_lint_group(lints: impl Iterator<Item = Self>) -> HashMap<String, Vec<Self>> {
lints.map(|lint| (lint.group.to_string(), lint)).into_group_map()
}
}

/// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`.
#[must_use]
pub fn gen_lint_group_list(lints: Vec<Lint>) -> Vec<String> {
pub fn gen_lint_group_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
lints
.into_iter()
.filter_map(|l| {
if l.deprecation.is_some() {
None
} else {
Some(format!(" LintId::of(&{}::{}),", l.module, l.name.to_uppercase()))
}
})
.map(|l| format!(" LintId::of(&{}::{}),", l.module, l.name.to_uppercase()))
.sorted()
.collect::<Vec<String>>()
}

/// Generates the `pub mod module_name` list in `clippy_lints/src/lib.rs`.
#[must_use]
pub fn gen_modules_list(lints: Vec<Lint>) -> Vec<String> {
pub fn gen_modules_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
lints
.into_iter()
.filter_map(|l| {
if l.is_internal() || l.deprecation.is_some() {
None
} else {
Some(l.module)
}
})
.map(|l| &l.module)
.unique()
.map(|module| format!("pub mod {};", module))
.map(|module| format!("mod {};", module))
.sorted()
.collect::<Vec<String>>()
}

/// Generates the list of lint links at the bottom of the README
#[must_use]
pub fn gen_changelog_lint_list(lints: Vec<Lint>) -> Vec<String> {
let mut lint_list_sorted: Vec<Lint> = lints;
lint_list_sorted.sort_by_key(|l| l.name.clone());
lint_list_sorted
.iter()
.filter_map(|l| {
if l.is_internal() {
None
} else {
Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name))
}
})
pub fn gen_changelog_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
lints
.sorted_by_key(|l| &l.name)
.map(|l| format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name))
.collect()
}

/// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`.
#[must_use]
pub fn gen_deprecated(lints: &[Lint]) -> Vec<String> {
pub fn gen_deprecated<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
lints
.iter()
.filter_map(|l| {
l.clone().deprecation.map(|depr_text| {
vec![
" store.register_removed(".to_string(),
format!(" \"clippy::{}\",", l.name),
format!(" \"{}\",", depr_text),
" );".to_string(),
]
})
.flat_map(|l| {
l.deprecation
.clone()
.map(|depr_text| {
vec![
" store.register_removed(".to_string(),
format!(" \"clippy::{}\",", l.name),
format!(" \"{}\",", depr_text),
" );".to_string(),
]
})
.expect("only deprecated lints should be passed")
})
.flatten()
.collect::<Vec<String>>()
}

#[must_use]
pub fn gen_register_lint_list(lints: &[Lint]) -> Vec<String> {
pub fn gen_register_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
let pre = " store.register_lints(&[".to_string();
let post = " ]);".to_string();
let mut inner = lints
.iter()
.filter_map(|l| {
if !l.is_internal() && l.deprecation.is_none() {
Some(format!(" &{}::{},", l.module, l.name.to_uppercase()))
} else {
None
}
})
.map(|l| format!(" &{}::{},", l.module, l.name.to_uppercase()))
.sorted()
.collect::<Vec<String>>();
inner.insert(0, pre);
Expand Down Expand Up @@ -439,7 +418,7 @@ fn test_usable_lints() {
None,
"module_name",
)];
assert_eq!(expected, Lint::usable_lints(lints.into_iter()).collect::<Vec<Lint>>());
assert_eq!(expected, Lint::usable_lints(&lints));
}

#[test]
Expand Down Expand Up @@ -469,13 +448,12 @@ fn test_gen_changelog_lint_list() {
let lints = vec![
Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"),
Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
];
let expected = vec![
format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()),
format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string()),
];
assert_eq!(expected, gen_changelog_lint_list(lints));
assert_eq!(expected, gen_changelog_lint_list(lints.iter()));
}

#[test]
Expand All @@ -495,7 +473,6 @@ fn test_gen_deprecated() {
Some("will be removed"),
"module_name",
),
Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"),
];
let expected: Vec<String> = vec![
" store.register_removed(",
Expand All @@ -510,36 +487,37 @@ fn test_gen_deprecated() {
.into_iter()
.map(String::from)
.collect();
assert_eq!(expected, gen_deprecated(&lints));
assert_eq!(expected, gen_deprecated(lints.iter()));
}

#[test]
#[should_panic]
fn test_gen_deprecated_fail() {
let lints = vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")];
let _ = gen_deprecated(lints.iter());
}

#[test]
fn test_gen_modules_list() {
let lints = vec![
Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"),
Lint::new("incorrect_stuff", "group3", "abc", None, "another_module"),
Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
];
let expected = vec![
"pub mod another_module;".to_string(),
"pub mod module_name;".to_string(),
];
assert_eq!(expected, gen_modules_list(lints));
let expected = vec!["mod another_module;".to_string(), "mod module_name;".to_string()];
assert_eq!(expected, gen_modules_list(lints.iter()));
}

#[test]
fn test_gen_lint_group_list() {
let lints = vec![
Lint::new("abc", "group1", "abc", None, "module_name"),
Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"),
Lint::new("internal", "internal_style", "abc", None, "module_name"),
];
let expected = vec![
" LintId::of(&module_name::ABC),".to_string(),
" LintId::of(&module_name::INTERNAL),".to_string(),
" LintId::of(&module_name::SHOULD_ASSERT_EQ),".to_string(),
];
assert_eq!(expected, gen_lint_group_list(lints));
assert_eq!(expected, gen_lint_group_list(lints.iter()));
}
34 changes: 15 additions & 19 deletions clippy_dev/src/update_lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ pub enum UpdateMode {
pub fn run(update_mode: UpdateMode) {
let lint_list: Vec<Lint> = gather_all().collect();

let internal_lints = Lint::internal_lints(lint_list.clone().into_iter());

let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list.clone().into_iter()).collect();
let usable_lint_count = round_to_fifty(usable_lints.len());

let internal_lints = Lint::internal_lints(&lint_list);
let deprecated_lints = Lint::deprecated_lints(&lint_list);
let usable_lints = Lint::usable_lints(&lint_list);
let mut sorted_usable_lints = usable_lints.clone();
sorted_usable_lints.sort_by_key(|lint| lint.name.clone());

let usable_lint_count = round_to_fifty(usable_lints.len());

let mut file_change = replace_region_in_file(
Path::new("src/lintlist/mod.rs"),
"begin lint list",
Expand Down Expand Up @@ -61,7 +61,7 @@ pub fn run(update_mode: UpdateMode) {
"<!-- end autogenerated links to lint list -->",
false,
update_mode == UpdateMode::Change,
|| gen_changelog_lint_list(lint_list.clone()),
|| gen_changelog_lint_list(usable_lints.iter().chain(deprecated_lints.iter())),
)
.changed;

Expand All @@ -71,7 +71,7 @@ pub fn run(update_mode: UpdateMode) {
"end deprecated lints",
false,
update_mode == UpdateMode::Change,
|| gen_deprecated(&lint_list),
|| gen_deprecated(deprecated_lints.iter()),
)
.changed;

Expand All @@ -81,7 +81,7 @@ pub fn run(update_mode: UpdateMode) {
"end register lints",
false,
update_mode == UpdateMode::Change,
|| gen_register_lint_list(&lint_list),
|| gen_register_lint_list(usable_lints.iter().chain(internal_lints.iter())),
)
.changed;

Expand All @@ -91,7 +91,7 @@ pub fn run(update_mode: UpdateMode) {
"end lints modules",
false,
update_mode == UpdateMode::Change,
|| gen_modules_list(lint_list.clone()),
|| gen_modules_list(usable_lints.iter()),
)
.changed;

Expand All @@ -104,13 +104,9 @@ pub fn run(update_mode: UpdateMode) {
update_mode == UpdateMode::Change,
|| {
// clippy::all should only include the following lint groups:
let all_group_lints = usable_lints
.clone()
.into_iter()
.filter(|l| {
l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf"
})
.collect();
let all_group_lints = usable_lints.iter().filter(|l| {
l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf"
});

gen_lint_group_list(all_group_lints)
},
Expand All @@ -125,7 +121,7 @@ pub fn run(update_mode: UpdateMode) {
r#"\]\);"#,
false,
update_mode == UpdateMode::Change,
|| gen_lint_group_list(lints.clone()),
|| gen_lint_group_list(lints.iter()),
)
.changed;
}
Expand All @@ -140,8 +136,8 @@ pub fn run(update_mode: UpdateMode) {
}

pub fn print_lints() {
let lint_list = gather_all();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list).collect();
let lint_list: Vec<Lint> = gather_all().collect();
let usable_lints = Lint::usable_lints(&lint_list);
let usable_lint_count = usable_lints.len();
let grouped_by_lint_group = Lint::by_lint_group(usable_lints.into_iter());

Expand Down
Loading