Skip to content

Commit

Permalink
submodules: update clippy from a416c5e to fc24fce
Browse files Browse the repository at this point in the history
Fixes clippy tool state

Changes:
````
FIXME > TODO
rustup rust-lang/rust#56992
Document map_clone known problems rust-lang#498
Remove header link
test: panic at map_unit_fn.rs:202 for map() without args
rm unused file map_unit_fn.stderr
panic at map_unit_fn.rs:202 for map() without args
Change contrib.md hierarchy, link to it from readme
Workaround rust-lang/rust#43081
Teach `suspicious_else_formatting` about `if .. {..} {..}`
Link to `rustc_driver` crate in plugin
mutex_atomic: Correct location of AtomicBool and friends
Update README local run command to specify syspath
Do not mark as_ref as useless if it's followed by a method call
Changes lint sugg to bitwise and operator `&`
Run update_lints after renaming
Rename lint to MODULE_NAME_REPETITIONS
Add renaming tests
Move renaming to the right place
Implements lint for order comparisons against bool
fix(module_name_repeat): Try to register renamed lint, not valid yet
Fix an endless loop in the tests.
Fix `implicit_return` false positives.
chore(moduel_name_repeat): Rename stutter lint to module_name_repeat to avoid ableist language
Make integration tests fail on 'E0463'
base tests: make sure cargo-clippy binary can be called directly
Revert "Merge pull request rust-lang#3257 from o01eg/remove-sysroot"
````
  • Loading branch information
matthiaskrgr committed Dec 23, 2018
1 parent 9d15aaf commit aca8f94
Show file tree
Hide file tree
Showing 33 changed files with 552 additions and 417 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ All notable changes to this project will be documented in this file.
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
[`module_name_repetitions`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
[`modulo_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_one
[`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions
[`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl
Expand Down Expand Up @@ -850,7 +851,6 @@ All notable changes to this project will be documented in this file.
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
[`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
[`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
[`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting
[`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
Expand Down
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru
* [Running test suite](#running-test-suite)
* [Running rustfmt](#running-rustfmt)
* [Testing manually](#testing-manually)
* [How Clippy works](#how-clippy-works)
* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
* [How Clippy works](#how-clippy-works)
* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
* [Issue and PR Triage](#issue-and-pr-triage)
* [Bors and Homu](#bors-and-homu)
* [Contributions](#contributions)
Expand Down Expand Up @@ -168,7 +168,7 @@ Manually testing against an example file is useful if you have added some
local modifications, run `env CLIPPY_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs`
from the working copy root.

### How Clippy works
## How Clippy works

Clippy is a [rustc compiler plugin][compiler_plugin]. The main entry point is at [`src/lib.rs`][main_entry]. In there, the lint registration is delegated to the [`clippy_lints`][lint_crate] crate.

Expand Down Expand Up @@ -218,7 +218,7 @@ The difference between `EarlyLintPass` and `LateLintPass` is that the methods of

That's why the `else_if_without_else` example uses the `register_early_lint_pass` function. Because the [actual lint logic][else_if_without_else] does not depend on any type information.

### Fixing build failures caused by Rust
## Fixing build failures caused by Rust

Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures caused by Rust updates, can be a good way to learn about Rust internals.

Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ To have cargo compile your crate with Clippy without Clippy installation
in your code, you can use:

```terminal
cargo run --bin cargo-clippy --manifest-path=path_to_clippys_Cargo.toml
RUSTFLAGS=--sysroot=`rustc --print sysroot` cargo run --bin cargo-clippy --manifest-path=path_to_clippys_Cargo.toml
```

*[Note](https://github.com/rust-lang/rust-clippy/wiki#a-word-of-warning):*
Expand Down Expand Up @@ -151,6 +151,10 @@ Note: `deny` produces errors instead of warnings.

If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra flags to Clippy during the run: `cargo clippy -- -A clippy::lint_name` will run Clippy with `lint_name` disabled and `cargo clippy -- -W clippy::lint_name` will run it with that enabled. This also works with lint groups. For example you can run Clippy with warnings for all lints enabled: `cargo clippy -- -W clippy::pedantic`

## Contributing

If you want to contribute to Clippy, you can find more information in [CONTRIBUTING.md](https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md).

## License

Copyright 2014-2018 The Rust Project Developers
Expand Down
5 changes: 5 additions & 0 deletions ci/base-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ cd clippy_lints && cargo test && cd ..
cd rustc_tools_util && cargo test && cd ..
cd clippy_dev && cargo test && cd ..

# make sure clippy can be called via ./path/to/cargo-clippy
cd clippy_workspace_tests
../target/debug/cargo-clippy
cd ..

# Perform various checks for lint registration
./util/dev update_lints --check
cargo +nightly fmt --all -- --check
Expand Down
2 changes: 1 addition & 1 deletion ci/integration-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function check() {
# run clippy on a project, try to be verbose and trigger as many warnings as possible for greater coverage
RUST_BACKTRACE=full cargo clippy --all-targets --all-features -- --cap-lints warn -W clippy::pedantic -W clippy::nursery &> clippy_output
cat clippy_output
! cat clippy_output | grep -q "internal compiler error\|query stack during panic"
! cat clippy_output | grep -q "internal compiler error\|query stack during panic\|E0463"
if [[ $? != 0 ]]; then
return 1
fi
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,9 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
&name_lower,
Some(tool_name.as_str())
) {
CheckLintNameResult::NoLint => (),
// FIXME: can we suggest similar lint names here?
// https://github.com/rust-lang/rust/pull/56992
CheckLintNameResult::NoLint(None) => (),
_ => {
db.span_suggestion(lint.span,
"lowercase the lint name",
Expand Down
13 changes: 9 additions & 4 deletions clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ declare_clippy_lint! {
/// }
/// ```
declare_clippy_lint! {
pub STUTTER,
pub MODULE_NAME_REPETITIONS,
pedantic,
"type names prefixed/postfixed with their containing module's name"
}
Expand Down Expand Up @@ -126,7 +126,12 @@ impl EnumVariantNames {

impl LintPass for EnumVariantNames {
fn get_lints(&self) -> LintArray {
lint_array!(ENUM_VARIANT_NAMES, PUB_ENUM_VARIANT_NAMES, STUTTER, MODULE_INCEPTION)
lint_array!(
ENUM_VARIANT_NAMES,
PUB_ENUM_VARIANT_NAMES,
MODULE_NAME_REPETITIONS,
MODULE_INCEPTION
)
}
}

Expand Down Expand Up @@ -277,7 +282,7 @@ impl EarlyLintPass for EnumVariantNames {
match item_camel.chars().nth(nchars) {
Some(c) if is_word_beginning(c) => span_lint(
cx,
STUTTER,
MODULE_NAME_REPETITIONS,
item.span,
"item name starts with its containing module's name",
),
Expand All @@ -287,7 +292,7 @@ impl EarlyLintPass for EnumVariantNames {
if rmatching == nchars {
span_lint(
cx,
STUTTER,
MODULE_NAME_REPETITIONS,
item.span,
"item name ends with its containing module's name",
);
Expand Down
72 changes: 55 additions & 17 deletions clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ declare_clippy_lint! {
"suspicious formatting of `*=`, `-=` or `!=`"
}

/// **What it does:** Checks for formatting of `else if`. It lints if the `else`
/// and `if` are not on the same line or the `else` seems to be missing.
/// **What it does:** Checks for formatting of `else`. It lints if the `else`
/// is followed immediately by a newline or the `else` seems to be missing.
///
/// **Why is this bad?** This is probably some refactoring remnant, even if the
/// code is correct, it might look confusing.
Expand All @@ -42,19 +42,29 @@ declare_clippy_lint! {
/// **Example:**
/// ```rust,ignore
/// if foo {
/// } { // looks like an `else` is missing here
/// }
///
/// if foo {
/// } if bar { // looks like an `else` is missing here
/// }
///
/// if foo {
/// } else
///
/// { // this is the `else` block of the previous `if`, but should it be?
/// }
///
/// if foo {
/// } else
///
/// if bar { // this is the `else` block of the previous `if`, but should it be?
/// }
/// ```
declare_clippy_lint! {
pub SUSPICIOUS_ELSE_FORMATTING,
style,
"suspicious formatting of `else if`"
"suspicious formatting of `else`"
}

/// **What it does:** Checks for possible missing comma in an array. It lints if
Expand Down Expand Up @@ -96,7 +106,7 @@ impl EarlyLintPass for Formatting {
match (&w[0].node, &w[1].node) {
(&ast::StmtKind::Expr(ref first), &ast::StmtKind::Expr(ref second))
| (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Semi(ref second)) => {
check_consecutive_ifs(cx, first, second);
check_missing_else(cx, first, second);
},
_ => (),
}
Expand All @@ -105,7 +115,7 @@ impl EarlyLintPass for Formatting {

fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
check_assign(cx, expr);
check_else_if(cx, expr);
check_else(cx, expr);
check_array(cx, expr);
}
}
Expand Down Expand Up @@ -139,10 +149,18 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &ast::Expr) {
}
}

/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else if`.
fn check_else_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else`.
fn check_else(cx: &EarlyContext<'_>, expr: &ast::Expr) {
if let Some((then, &Some(ref else_))) = unsugar_if(expr) {
if unsugar_if(else_).is_some() && !differing_macro_contexts(then.span, else_.span) && !in_macro(then.span) {
if (is_block(else_) || unsugar_if(else_).is_some())
&& !differing_macro_contexts(then.span, else_.span)
&& !in_macro(then.span)
{
// workaround for rust-lang/rust#43081
if expr.span.lo().0 == 0 && expr.span.hi().0 == 0 {
return;
}

// this will be a span from the closing ‘}’ of the “then” block (excluding) to
// the
// “if” of the “else if” block (excluding)
Expand All @@ -154,14 +172,19 @@ fn check_else_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
let else_pos = else_snippet.find("else").expect("there must be a `else` here");

if else_snippet[else_pos..].contains('\n') {
let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" };

span_note_and_lint(
cx,
SUSPICIOUS_ELSE_FORMATTING,
else_span,
"this is an `else if` but the formatting might hide it",
&format!("this is an `else {}` but the formatting might hide it", else_desc),
else_span,
"to remove this lint, remove the `else` or remove the new line between `else` \
and `if`",
&format!(
"to remove this lint, remove the `else` or remove the new line between \
`else` and `{}`",
else_desc,
),
);
}
}
Expand Down Expand Up @@ -200,32 +223,47 @@ fn check_array(cx: &EarlyContext<'_>, expr: &ast::Expr) {
}
}

/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for consecutive ifs.
fn check_consecutive_ifs(cx: &EarlyContext<'_>, first: &ast::Expr, second: &ast::Expr) {
fn check_missing_else(cx: &EarlyContext<'_>, first: &ast::Expr, second: &ast::Expr) {
if !differing_macro_contexts(first.span, second.span)
&& !in_macro(first.span)
&& unsugar_if(first).is_some()
&& unsugar_if(second).is_some()
&& (is_block(second) || unsugar_if(second).is_some())
{
// where the else would be
let else_span = first.span.between(second.span);

if let Some(else_snippet) = snippet_opt(cx, else_span) {
if !else_snippet.contains('\n') {
let (looks_like, next_thing) = if unsugar_if(second).is_some() {
("an `else if`", "the second `if`")
} else {
("an `else {..}`", "the next block")
};

span_note_and_lint(
cx,
SUSPICIOUS_ELSE_FORMATTING,
else_span,
"this looks like an `else if` but the `else` is missing",
&format!("this looks like {} but the `else` is missing", looks_like),
else_span,
"to remove this lint, add the missing `else` or add a new line before the second \
`if`",
&format!(
"to remove this lint, add the missing `else` or add a new line before {}",
next_thing,
),
);
}
}
}
}

fn is_block(expr: &ast::Expr) -> bool {
if let ast::ExprKind::Block(..) = expr.node {
true
} else {
false
}
}

/// Match `if` or `if let` expressions and return the `then` and `else` block.
fn unsugar_if(expr: &ast::Expr) -> Option<(&P<ast::Block>, &Option<P<ast::Expr>>)> {
match expr.node {
Expand Down
42 changes: 21 additions & 21 deletions clippy_lints/src/implicit_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ declare_clippy_lint! {
pub struct Pass;

impl Pass {
fn lint(cx: &LateContext<'_, '_>, outer_span: syntax_pos::Span, inner_span: syntax_pos::Span, msg: &str) {
span_lint_and_then(cx, IMPLICIT_RETURN, outer_span, "missing return statement", |db| {
if let Some(snippet) = snippet_opt(cx, inner_span) {
db.span_suggestion_with_applicability(
outer_span,
msg,
format!("return {}", snippet),
Applicability::MachineApplicable,
);
}
});
}

fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) {
match &expr.node {
// loops could be using `break` instead of `return`
Expand All @@ -55,23 +68,19 @@ impl Pass {
// only needed in the case of `break` with `;` at the end
else if let Some(stmt) = block.stmts.last() {
if let rustc::hir::StmtKind::Semi(expr, ..) = &stmt.node {
Self::expr_match(cx, expr);
// make sure it's a break, otherwise we want to skip
if let ExprKind::Break(.., break_expr) = &expr.node {
if let Some(break_expr) = break_expr {
Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown");
}
}
}
}
},
// use `return` instead of `break`
ExprKind::Break(.., break_expr) => {
if let Some(break_expr) = break_expr {
span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
if let Some(snippet) = snippet_opt(cx, break_expr.span) {
db.span_suggestion_with_applicability(
expr.span,
"change `break` to `return` as shown",
format!("return {}", snippet),
Applicability::MachineApplicable,
);
}
});
Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown");
}
},
ExprKind::If(.., if_expr, else_expr) => {
Expand All @@ -89,16 +98,7 @@ impl Pass {
// skip if it already has a return statement
ExprKind::Ret(..) => (),
// everything else is missing `return`
_ => span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
if let Some(snippet) = snippet_opt(cx, expr.span) {
db.span_suggestion_with_applicability(
expr.span,
"add `return` as shown",
format!("return {}", snippet),
Applicability::MachineApplicable,
);
}
}),
_ => Self::lint(cx, expr.span, expr.span, "add `return` as shown"),
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
doc::DOC_MARKDOWN,
empty_enum::EMPTY_ENUM,
enum_glob_use::ENUM_GLOB_USE,
enum_variants::MODULE_NAME_REPETITIONS,
enum_variants::PUB_ENUM_VARIANT_NAMES,
enum_variants::STUTTER,
if_not_else::IF_NOT_ELSE,
infinite_iter::MAYBE_INFINITE_ITER,
items_after_statements::ITEMS_AFTER_STATEMENTS,
Expand Down Expand Up @@ -1030,6 +1030,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
]);
}

pub fn register_renamed(ls: &mut rustc::lint::LintStore) {
ls.register_renamed("clippy::stutter", "clippy::module_name_repetitions");
}

// only exists to let the dogfood integration test works.
// Don't run clippy as an executable directly
#[allow(dead_code)]
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/map_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ pub struct Pass;
///
/// **Why is this bad?** Readability, this can be written more concisely
///
/// **Known problems:** None.
/// **Known problems:** Sometimes `.cloned()` requires stricter trait
/// bound than `.map(|e| e.clone())` (which works because of the coercion).
/// See [#498](https://github.com/rust-lang-nursery/rust-clippy/issues/498).
///
/// **Example:**
///
Expand Down
Loading

0 comments on commit aca8f94

Please sign in to comment.