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

Rollup of 11 pull requests #5438

Merged
merged 39 commits into from
Apr 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
adcaa1b
Downgrade let_unit_value to pedantic
dtolnay Apr 3, 2020
94154ca
Downgrade trivially_copy_pass_by_ref to pedantic
dtolnay Apr 3, 2020
e26ae7a
Downgrade inefficient_to_string to pedantic
dtolnay Apr 3, 2020
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
91d8a80
result_map_or_into_option: add lint to catch manually adpating Result…
nickrtorres Apr 4, 2020
2481d3c
Rename rustc -> rustc_middle in doc links
flip1995 Apr 4, 2020
fac5a41
Update CONTRIBUTING.md
flip1995 Apr 4, 2020
be34bc4
Downgrade unreadable_literal to pedantic
dtolnay Apr 4, 2020
560c8c9
Downgrade new_ret_no_self to pedantic
dtolnay Apr 4, 2020
91759a7
result_map_or_into_option: explicitly note absence of known problems
nickrtorres Apr 4, 2020
3a29aed
result_map_or_into_option: add good and bad examples
nickrtorres Apr 4, 2020
d0738bd
result_map_or_into_option: destructure lint tuple or return early
nickrtorres Apr 4, 2020
fb276dc
result_map_or_into_option: add `opt.map_or(None, |_| Some(y))` test
nickrtorres Apr 4, 2020
acc3bc1
result_map_or_into_option: move arg checks into tuple assignment
nickrtorres Apr 4, 2020
2533f56
result_map_or_into_option: fix `cargo dev fmt --check` errors
nickrtorres Apr 4, 2020
fecdb72
CONTRIBUTING.md: fix broken triage link
nickrtorres Apr 4, 2020
325d0b6
result_map_or_into: fix dogfood_clippy error => {h,l}int
nickrtorres Apr 4, 2020
5d54fbb
result_map_or_into_option: fix syntax error in example
nickrtorres Apr 5, 2020
9c9af1d
Include OpAssign in suspicious_op_assign_impl
jpospychala Apr 5, 2020
4f14826
Lint on opt.as_ref().map(|x| &**x).
xiongmao86 Apr 6, 2020
d7056f8
Refine lint message.
xiongmao86 Apr 7, 2020
46337cb
Rollup merge of #5406 - flip1995:update_lints_fix, r=flip1995
flip1995 Apr 8, 2020
935b45d
Rollup merge of #5409 - dtolnay:letunit, r=flip1995
flip1995 Apr 8, 2020
1e1bd51
Rollup merge of #5410 - dtolnay:trivially, r=flip1995
flip1995 Apr 8, 2020
5ea4771
Rollup merge of #5412 - dtolnay:tostring, r=flip1995
flip1995 Apr 8, 2020
a1e49f9
Rollup merge of #5415 - nickrtorres:master, r=flip1995
flip1995 Apr 8, 2020
f84b72b
Rollup merge of #5417 - flip1995:doc_update, r=flip1995
flip1995 Apr 8, 2020
2011d9a
Rollup merge of #5419 - dtolnay:unreadable, r=flip1995
flip1995 Apr 8, 2020
7cb5180
Rollup merge of #5420 - dtolnay:newret, r=flip1995
flip1995 Apr 8, 2020
b42bd19
Rollup merge of #5422 - nickrtorres:contributing-triage, r=flip1995
flip1995 Apr 8, 2020
8fc592a
Rollup merge of #5424 - jpospychala:suspicious_op_assign_impl, r=flip…
flip1995 Apr 8, 2020
79d1521
Rollup merge of #5425 - xiongmao86:issue5367, r=flip1995
flip1995 Apr 8, 2020
381f9cb
Run fmt and update test
flip1995 Apr 8, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,7 @@ Released 2018-09-13
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
[`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
[`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
Expand Down
11 changes: 5 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ High level approach:

### Finding something to fix/improve

All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @llogiq, @mcarton or @oli-obk.
All issues on Clippy are mentored, if you want help with a bug just ask
@Manishearth, @flip1995, @phansch or @yaahc.

Some issues are easier than others. The [`good first issue`] label can be used to find the easy issues.
If you want to work on an issue, please leave a comment so that we can assign it to you!
Expand Down Expand Up @@ -70,24 +71,22 @@ an AST expression). `match_def_path()` in Clippy's `utils` module can also be us
[`T-AST`]: https://github.com/rust-lang/rust-clippy/labels/T-AST
[`T-middle`]: https://github.com/rust-lang/rust-clippy/labels/T-middle
[`E-medium`]: https://github.com/rust-lang/rust-clippy/labels/E-medium
[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty
[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty
[nodes in the AST docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/
[deep-nesting]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/mem_forget.rs#L29-L43
[if_chain]: https://docs.rs/if_chain/*/if_chain
[nest-less]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/bit_mask.rs#L124-L150

## Writing code

Have a look at the [docs for writing lints][adding_lints] for more details. [Llogiq's blog post on lints]
is also a nice primer to lint-writing, though it does get into advanced stuff and may be a bit outdated.
Have a look at the [docs for writing lints][adding_lints] for more details.

If you want to add a new lint or change existing ones apart from bugfixing, it's
also a good idea to give the [stability guarantees][rfc_stability] and
[lint categories][rfc_lint_cats] sections of the [Clippy 1.0 RFC][clippy_rfc] a
quick read.

[adding_lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md
[Llogiq's blog post on lints]: https://llogiq.github.io/2015/06/04/workflows.html
[clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md
[rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees
[rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories
Expand Down Expand Up @@ -223,7 +222,7 @@ You can find the Clippy bors queue [here][homu_queue].
If you have @bors permissions, you can find an overview of the available
commands [here][homu_instructions].

[triage]: https://forge.rust-lang.org/triage-procedure.html
[triage]: https://forge.rust-lang.org/release/triage-procedure.html
[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A
[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A
[homu]: https://github.com/rust-lang/homu
Expand Down
128 changes: 53 additions & 75 deletions clippy_dev/src/lib.rs
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
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