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.

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

Let update_lints also generate the internal lints #5156

Merged
merged 6 commits into from
Feb 15, 2020
Merged
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
50 changes: 29 additions & 21 deletions clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,15 @@ impl Lint {
lints.filter(|l| l.deprecation.is_none() && !l.is_internal())
}

/// 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")
}

/// Returns the lints in a `HashMap`, grouped by the different lint groups
#[must_use]
pub fn by_lint_group(lints: &[Self]) -> HashMap<String, Vec<Self>> {
lints
.iter()
.map(|lint| (lint.group.to_string(), lint.clone()))
.into_group_map()
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()
}

#[must_use]
Expand All @@ -82,7 +84,7 @@ pub fn gen_lint_group_list(lints: Vec<Lint>) -> Vec<String> {
lints
.into_iter()
.filter_map(|l| {
if l.is_internal() || l.deprecation.is_some() {
if l.deprecation.is_some() {
None
} else {
Some(format!(" LintId::of(&{}::{}),", l.module, l.name.to_uppercase()))
Expand Down Expand Up @@ -173,29 +175,34 @@ pub fn gather_all() -> impl Iterator<Item = Lint> {

fn gather_from_file(dir_entry: &walkdir::DirEntry) -> impl Iterator<Item = Lint> {
let content = fs::read_to_string(dir_entry.path()).unwrap();
let mut filename = dir_entry.path().file_stem().unwrap().to_str().unwrap();
let path = dir_entry.path();
let filename = path.file_stem().unwrap();
let path_buf = path.with_file_name(filename);
let mut rel_path = path_buf
.strip_prefix(clippy_project_root().join("clippy_lints/src"))
.expect("only files in `clippy_lints/src` should be looked at");
// If the lints are stored in mod.rs, we get the module name from
// the containing directory:
if filename == "mod" {
filename = dir_entry
.path()
.parent()
.unwrap()
.file_stem()
.unwrap()
.to_str()
.unwrap()
rel_path = rel_path.parent().unwrap();
}
parse_contents(&content, filename)

let module = rel_path
.components()
.map(|c| c.as_os_str().to_str().unwrap())
.collect::<Vec<_>>()
.join("::");

parse_contents(&content, &module)
}

fn parse_contents(content: &str, filename: &str) -> impl Iterator<Item = Lint> {
fn parse_contents(content: &str, module: &str) -> impl Iterator<Item = Lint> {
let lints = DEC_CLIPPY_LINT_RE
.captures_iter(content)
.map(|m| Lint::new(&m["name"], &m["cat"], &m["desc"], None, filename));
.map(|m| Lint::new(&m["name"], &m["cat"], &m["desc"], None, module));
let deprecated = DEC_DEPRECATED_LINT_RE
.captures_iter(content)
.map(|m| Lint::new(&m["name"], "Deprecated", &m["desc"], Some(&m["desc"]), filename));
.map(|m| Lint::new(&m["name"], "Deprecated", &m["desc"], Some(&m["desc"]), module));
// Removing the `.collect::<Vec<Lint>>().into_iter()` causes some lifetime issues due to the map
lints.chain(deprecated).collect::<Vec<Lint>>().into_iter()
}
Expand Down Expand Up @@ -449,7 +456,7 @@ fn test_by_lint_group() {
"group2".to_string(),
vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")],
);
assert_eq!(expected, Lint::by_lint_group(&lints));
assert_eq!(expected, Lint::by_lint_group(lints.into_iter()));
}

#[test]
Expand Down Expand Up @@ -522,10 +529,11 @@ fn test_gen_lint_group_list() {
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("incorrect_internal", "internal_style", "abc", None, "module_name"),
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));
Expand Down
14 changes: 8 additions & 6 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ fn main() {
fn print_lints() {
let lint_list = gather_all();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list).collect();
let lint_count = usable_lints.len();
let grouped_by_lint_group = Lint::by_lint_group(&usable_lints);
let usable_lint_count = usable_lints.len();
let grouped_by_lint_group = Lint::by_lint_group(usable_lints.into_iter());

for (lint_group, mut lints) in grouped_by_lint_group {
if lint_group == "Deprecated" {
Expand All @@ -157,15 +157,17 @@ fn print_lints() {
}
}

println!("there are {} lints", lint_count);
println!("there are {} lints", usable_lint_count);
}

#[allow(clippy::too_many_lines)]
fn update_lints(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 lint_count = usable_lints.len();
let usable_lint_count = usable_lints.len();

let mut sorted_usable_lints = usable_lints.clone();
sorted_usable_lints.sort_by_key(|lint| lint.name.clone());
Expand Down Expand Up @@ -198,7 +200,7 @@ fn update_lints(update_mode: UpdateMode) {
|| {
vec![format!(
"[There are {} lints included in this crate!]({})",
lint_count, DOCS_LINK
usable_lint_count, DOCS_LINK
)]
},
)
Expand Down Expand Up @@ -267,7 +269,7 @@ fn update_lints(update_mode: UpdateMode) {
.changed;

// Generate the list of lints for all other lint groups
for (lint_group, lints) in Lint::by_lint_group(&usable_lints) {
for (lint_group, lints) in Lint::by_lint_group(usable_lints.into_iter().chain(internal_lints)) {
file_change |= replace_region_in_file(
Path::new("clippy_lints/src/lib.rs"),
&format!("store.register_group\\(true, \"clippy::{}\"", lint_group),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,10 +1105,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![
LintId::of(&utils::internal_lints::CLIPPY_LINTS_INTERNAL),
LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
LintId::of(&utils::internal_lints::DEFAULT_LINT),
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![
Expand Down
14 changes: 7 additions & 7 deletions doc/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,15 @@ declare_lint_pass!(FooFunctions => [FOO_FUNCTIONS]);
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.

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`:
Normally after declaring the lint, we have to run `cargo dev update_lints`,
which updates some files, so Clippy knows about the new lint. Since we used
`cargo dev new_lint ...` to generate the lint declaration, this was done
automatically. While `update_lints` automates most of the 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`:

```rust
reg.register_early_lint_pass(box foo_functions::FooFunctions);
store.register_early_pass(box foo_functions::FooFunctions);
```

This should fix the `unknown clippy lint: clippy::foo_functions` error that we
Expand Down