Skip to content

Commit

Permalink
Rollup merge of rust-lang#5406 - flip1995:update_lints_fix, r=flip1995
Browse files Browse the repository at this point in the history 
Fix update_lints

This fixes a bug in update_lints, where `internal` lints were not registered properly. This also cleans up some code. For example: The code generation functions no longer filter the lints the are given. This is now the task of the caller. This way, it is more obvious in the `replace_in_file` calls which lints will be included in which part of a file.

This also turns the lint modules private. There is no need for them to be public, since shared code should be in the utils module anyway.

And last but not least, this fixes the `register_lints` code generation, so also internal lints get registered.

changelog: none
  • Loading branch information
@flip1995
flip1995 authored Apr 7, 2020
2 parents acd4342 + 30503a9 commit 3da43d0
Show file tree
Hide file tree
Showing 5 changed files with 262 additions and 287 deletions.
128 changes: 53 additions & 75 deletions clippy_dev/src/lib.rs
View file
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
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

0 comments on commit 3da43d0

Please sign in to comment.