Use ui_test's Windows path backslash heuristic
changelog: none
Instead of unconditionally replacing `\` with `/` we now use [`Match::PathBackslash`](https://docs.rs/ui_test/latest/ui_test/enum.Match.html#variant.PathBackslash) to only replace backslashes in paths that look like windows paths
`ui-toml` and `ui-cargo` tests still use the old way because they produce verbatim paths on windows in some tests (`\\?\C:\foo\...`) which was finnicky to get the replacement order correct with
Also removes the `ui_test` -> `compiletest` alias and `VarGuard`
Fix SPEEDTEST instructions and output
* `--nocapture` hasn't been needed anymore since forever (even before `ui_test`)
* the result was dividing by 1000 instead of the number of test runs, giving bogus (but still useful for the purpose) timing results.
changelog: fix SPEEDTEST instructions and output
redundant_locals: fix FPs on mutated shadows
Fixes#11290.
When a mutable binding is shadowed by
a mutable binding of the same name in a different scope, mutations in that scope have different meaning.
This PR fixes spurious `redundant_locals` emissions on such locals.
cc `@Centri3,` `@flip1995`
changelog: [`redundant_locals`]: fix false positives on mutated shadows
Rustup
r? `@ghost`
cc `@max-niederman` With the latest sync, I'm getting a lot of FP in the `redundant_locals` lint you recently added. Any ideas where this could come from?
changelog: none
When a mutable binding is shadowed by
a mutable binding of the same name in a different scope,
mutations in that scope have different meaning.
This commit fixes spurious `redundant_locals` emissions
on such locals.
[`redundant_guards`]: don't lint on float literals
Fixes#11304
changelog: [`redundant_guards`]: don't lint on float literals
r? `@Centri3` i figured you are probably a good reviewer for this since you implemented the lint ^^
redundant_type_annotations: only pass certain def kinds to type_of
Fixes#11190Fixesrust-lang/rust#113516
Also adds an `is_lint_allowed` check to skip the lint when it's not needed
changelog: none
Add `internal_features` lint
Implements https://github.com/rust-lang/compiler-team/issues/596
Also requires some more test blessing for codegen tests etc
`@jyn514` had the idea of just `allow`ing the lint by default in the test suite. I'm not sure whether this is a good idea, but it's definitely one worth considering. Additional input encouraged.
Fix `suspicious_xor_used_as_pow.rs` performance
The original `suspicious_xor_used_as_pow` lint had poor performance, so I fixed that + a little refactor so that module is readable.
**107 millis. -> 106 millis.** Using `SPEEDTEST` on Rust's VMs
fix#11060
changelog: [`suspicious_xor_used_as_pow`]: Improve performance by 0.934%
It lints against features that are inteded to be internal to the
compiler and standard library. Implements MCP #596.
We allow `internal_features` in the standard library and compiler as those
use many features and this _is_ the standard library from the "internal to the compiler and
standard library" after all.
Marking some features as internal wasn't exactly the most scientific approach, I just marked some
mostly obvious features. While there is a categorization in the macro,
it's not very well upheld (should probably be fixed in another PR).
We always pass `-Ainternal_features` in the testsuite
About 400 UI tests and several other tests use internal features.
Instead of throwing the attribute on each one, just always allow them.
There's nothing wrong with testing internal features^^
New lint `ignored_unit_patterns`
This idea comes from #11238. I've put the lint in `pedantic` as it might trigger numerous positives (three in Clippy itself).
changelog: [`ignored_unit_patterns`]: new lint
Suppress `question_mark` warning if `question_mark_used` is not allowed
Closes#11283
changelog: [`question_mark`]: Don't lint if `question_mark_used` is not allowed
Rename and allow `cast_ref_to_mut` lint
This PR is a small subset of https://github.com/rust-lang/rust/pull/112431, that is the renaming of the lint (`cast_ref_to_mut` -> `invalid_reference_casting`).
BUT also temporarily change the default level of the lint from deny-by-default to allow-by-default until https://github.com/rust-lang/rust/pull/112431 is merged.
r? `@Nilstrieb`
new lint: [`readonly_write_lock`]
Closes#8555
A new lint that catches `RwLock::write` calls to acquire a write lock only to read from it and not actually do any writes (mutations).
changelog: new lint: [`readonly_write_lock`]
Now `option_env_unwrap` warns even if a variable isn't set at compiletime
Fixes#10742
changelog: Fix false negative where `option_env_unwrap` wouldn't warn if the env variable isn't set at compile-time.
Currently, Clippy, Miri, Rustfmt, and rustc all use an environment variable to
indicate that output should be blessed, but they use different variable names.
In order to improve consistency, this patch applies the following changes:
- Emit `RUSTC_BLESS` within `prepare_cargo_test` so it is always
available
- Change usage of `MIRI_BLESS` in the Miri subtree to use `RUSTC_BLESS`
- Change usage of `BLESS` in the Clippy subtree to `RUSTC_BLESS`
- Change usage of `BLESS` in the Rustfmt subtree to `RUSTC_BLESS`
- Adjust the blessable test in `rustc_errors` to use this same
convention
- Update documentation where applicable
Any tools that uses `RUSTC_BLESS` should check that it is set to any value
other than `"0"`.
Fix integration tests #2
fix integration tests.
It turned out that the following tests fail to build at all:
chalk, combine, stdarch and hyper.
This is often a problem of passing `--all-targets --all-features`, in case of combine though, outdated deps were to blame.
I have opened tickets against combine and rustfmt
https://github.com/rust-lang/rustfmt/issues/5859https://github.com/Marwes/combine/issues/357
should we just remove the other failing repos? :/
changelog: fix integration tests on ci
[`slow_vector_initialization`]: catch `Vec::new()` followed by `.resize(len, 0)`
Closes#10938
changelog: [`slow_vector_initialization`]: catch `Vec::new()` followed by `.resize(len, 0)`
Remove Gha status emitter in compile-test
Disables the github specific output for now since it can be a bit confusing - https://github.com/oli-obk/ui_test/issues/109, in particular the truncation/repetition
r? `@flip1995`
changelog: none
New lint [`needless_return_with_try`]
Closes#10902
Rather than having a config option, this will just suggest removing the "return"; if `try_err` is used as well, then it'll be added again but without the `?`.
changelog: New lint [`needless_return_with_try`]
ptr_arg should ignore extern functions
Fixes: #11181
changelog: [`ptr_arg`]: ignore extern functions that are not
I am not sure whether we should ignore other Rust calling conventions like `rust-intrinsic`, `rust-call` or `rust-cold`.
`unwrap_or_else_default` -> `unwrap_or_default` and improve resulting lint
Resolves#10080 (though it doesn't implement exactly what's described there)
This PR does the following:
1. Merges `unwrap_or_else_default.rs`'s code into `or_fun_call.rs`
2. Extracts the code to handle `unwrap_or(/* default value */)` and similar, and moves it into `unwrap_or_else_default`
3. Implements the missing functionality from #9342, e.g.,, to handle `or_insert_with(Default::default)`
4. Renames `unwrap_or_else_default` to `unwrap_or_default` (since the "new" lint handles both `unwrap_or` and `unwrap_or_else`, it seemed sensible to use the shortened name)
This PR is currently two commits. The first implements 1-3, the second implements 4.
A word about 2: the `or_fun_call` lint currently produces warnings like the following:
```
error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:56:14
|
LL | with_new.unwrap_or(Vec::new());
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
```
To me, such warnings look like they should come from `unwrap_or_else_default`, not `or_fun_call`, especially since `or_fun_call` is [in the nursery](https://github.com/rust-lang/rust-clippy/pull/9829).
---
changelog: Move: Renamed `unwrap_or_else_default` to [`unwrap_or_default`]
[#10120](https://github.com/rust-lang/rust-clippy/pull/10120)
changelog: Enhancement: [`unwrap_or_default`]: Now handles more functions, like `or_insert_with`
[#10120](https://github.com/rust-lang/rust-clippy/pull/10120)
<!-- changelog_checked-->
check that the types are equal in `SpanlessEq::eq_expr`
Fixes#11213
changelog: [`if_same_then_else`]: don't lint for integer literals of different types
Fix async functions handling for `needless_pass_by_ref_mut` lint
Fixes https://github.com/rust-lang/rust-clippy/issues/11179.
The problem with async is that "internals" are actually inside a closure from the `ExprUseVisitor` point of view, meaning we need to actually run the check on the closures' body as well.
changelog: none
r? `@llogiq`
Make `comparison_to_empty` work on `if let`/`let` chains
This adds `LetChain` to `clippy_utils::higher`, other lints may benefit from such a change as well :D
changelog: Enhancement: [`comparison_to_empty`]: Now lints on `if let`
[`unused_async`]: don't lint if paths reference async fn without immediate call
Fixes#9695Fixes#9359
Clippy shouldn't lint unused `async` if there are paths referencing them if that path isn't the receiver of a function call, because that means that the function might be passed to some other function:
```rs
async fn f() {} // No await statements, so unused at this point
fn requires_fn_future<F: Future<Output = ()>>(_: fn() -> F) {}
requires_fn_future(f); // `f`'s asyncness is actually not unused.
```
(This isn't limited to just passing the function as a parameter to another function, it could also first be stored in a variable and later passed to another function as an argument)
This requires delaying the linting until post-crate and collecting path references to local async functions along the way.
changelog: [`unused_async`]: don't lint if paths reference async fn that require asyncness
fix dogfood lints in `redundant_local`
keep `redundant_local` from running in proc macros
rewrite `redundant_local` as late pass
make redundant_local's `find_binding` more readable
pluralize `redundant_locals` name
add test for `redundant_locals` in macros
test `redundant_locals` in proc macros
use more destructuring in `redundant_locals`
fix: format redundant_locals.rs
ignore needless_pass_by_mut_ref in redundant_locals test
Allow `Self::cmp(self, other)` as a correct impl
Fixes#11178
Also no longer checks if the method name is *just* cmp, but the path. That was an oversight on my part ^^
r? `@xFrednet`
(and `@blyxyas` too!)
changelog: [`incorrect_partial_ord_impl_on_ord_type`]: Now allows non-method calls to `cmp` like `Self::cmp(self, other)`
fix: false positive for `option_env!` in `ifs_same_cond`
Clippy had a false positive for with `ifs_same_cond` when two if-let expressions have an `option_env!` macro. The fix is similar to the `env!` macro fix.
The following example had a clippy error:
```rust
if let Some(env1) = option_env!("ENV1") {
// ...
} else if let Some(env2) = option_env!("ENV2") {
// ...
}
```
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01b85c61b56ddd900117fb247af04824
changelog: [`ifs_same_cond`]: fix false positive when using `option_env!` in if-let expressions.
[`unnecessary_literal_unwrap`]: Fix ICE on None.unwrap_or_default()
Fixes#11099Fixes#11064
I'm running into #11099 (cc `@y21)` on my Rust codebase. Clippy ICEs on this code when evaluating the `unnecessary_literal_unwrap` lint:
```rust
fn main() {
let val1: u8 = None.unwrap_or_default();
}
```
This fixes that ICE and adds an message specifically for that case:
```
error: used `unwrap_or_default()` on `None` value
--> $DIR/unnecessary_literal_unwrap.rs:26:5
|
LL | None::<String>.unwrap_or_default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap_or_default()`: `String::default()`
```
This PR also fixes the same ICE with `None.unwrap_or_else` (by giving the generic error message for the lint in that case).
changelog: Fix ICE in `unnecessary_literal_unwrap` on `None.unwrap_or_default()`
Clippy had a false positive for with `ifs_same_cond` when two
if-let expressions have an `option_env!` macro. The fix is similar to the
`env!` macro fix.
The following example had a clippy error:
```rust
if let Some(env1) = option_env!("ENV1") {
// ...
} else if let Some(env2) = option_env!("ENV2") {
// ...
}
```
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01b85c61b56ddd900117fb247af04824
changelog: Fix [`ifs_same_cond`] false positive when using `option_env!` in if-let expressions.
[`manual_filter_map`]: lint on `matches` and pattern matching
Fixes#8010
Previously this lint only worked specifically for a very limited set of methods on the filter call (`.filter(|opt| opt.is_some())` and `.filter(|res| res.is_ok())`). This PR extends it to also recognize `matches!` in the `filter` and pattern matching with `if let` or `match` in the `map`.
Example:
```rs
enum Enum {
A(i32),
B,
}
let _ = [Enum::A(123), Enum::B].into_iter()
.filter(|x| matches!(x, Enum::A(_)))
.map(|x| if let Enum::A(s) = x { s } else { unreachable!() });
```
Now suggests:
```diff
- .filter(|x| matches!(x, Enum::A(_))).map(if let Enum::A(s) = x { s } else { unreachable!() })
+ .filter_map(|x| match x { Enum::A(s) => Some(s), _ => None })
```
Adding this required a somewhat large change in code because it originally seemed to be specifically written with only method calls in the filter in mind, and `matches!` has different behavior in the map, so this new setup should make it possible to support more "generic" cases that need different handling for the filter and map calls.
changelog: [`manual_filter_map`]: lint on `matches` and pattern matching (and some internal refactoring)
Fix `unwrap_or_else_default` false positive
This PR fixes a false positive in the handling of `unwrap_or_else` with a default value when the value is needed for type inference.
An easy example to exhibit the false positive is the following:
```rust
let option = None;
option.unwrap_or_else(Vec::new).push(1);
```
The following code would not compile, because the fact that the value is a `Vec` has been lost:
```rust
let option = None;
option.unwrap_or_default().push(1);
```
The fix is to:
- implement a heuristic to tell whether an expression's type can be determined purely from its subexpressions, and the arguments and locals they use;
- apply the heuristic to `unwrap_or_else`'s receiver.
The heuristic returns false when applied to `option` in the above example, but it returns true when applied to `option` in either of the following examples:
```rust
let option: Option<Vec<u64>> = None;
option.unwrap_or_else(Vec::new).push(1);
```
```rust
let option = None::<Vec<u64>>;
option.unwrap_or_else(Vec::new).push(1);
```
(Aside: https://github.com/rust-lang/rust-clippy/pull/10120 unfairly contained multiple changes in one PR. I am trying to break that PR up into smaller pieces.)
---
changelog: FP: [`unwrap_or_else_default`]: No longer lints if the default value is needed for type inference
Remove `#![allow(unused)]` and `--crate-name` from `cargo dev new_lint` generated tests
changelog: none
Also removes some unused flags from `ui-cargo` tests because the entrypoint is now the `Cargo.toml`, not the `.rs` files
Rewrite [`tuple_array_conversions`]
Fixes#11100Fixes#11144Fixes#11124#11082 still needs discussion and #11085 likely can't be fixed.
changelog: [`tuple_array_conversions`]: Move to `pedantic`
changelog: [`tuple_array_conversions`]: Don't lint if mutability of references changes
changelog: [`tuple_array_conversions`]: Don't lint if bindings don't come from the exact same pattern
changelog: [`tuple_array_conversions`]: Don't lint if bindings are used for more than just the conversion
Add `imports_granularity = "Module"` to rustfmt.toml
This lets rustfmt split/merge imports, `Module` seems to be the most common style in clippy
https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#imports_granularity
changelog: none
Almost all the updates other than the config file change are from `cargo dev fmt` or blessed tests, the exceptions being
- `tests/ui/single_component_path_imports.rs`
- `tests/ui/single_component_path_imports_nested_first.rs`
- `tests/ui/single_component_path_imports_self_after.rs`
- `tests/ui/single_component_path_imports_self_before.rs`
- `tests/ui/unsafe_removed_from_name.rs` (added a test with merged imports as a drive by)
- `tests/ui/wildcard_imports.rs`
- `tests/ui/wildcard_imports_2021.rs`
[`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>`
Fixes#11145
This lint has a list of allowed types, one of which is `Wrapping<T>`, but it was only actually allowed if the type on the right hand side was also `Wrapping<T>`, which meant that, for example, `Wrapping<u32> += u32` would still lint. It now allows binary ops involving `Wrapping<T>` regardless of the type on the rhs.
These impls have only existed since Rust 1.60.0, so that is probably why the lint was previously not handling this correctly
changelog: [`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>` (e.g. `Wrapping<T> += T`)
[`unnecessary_literal_unwrap`]: also lint `unwrap_(err_)unchecked`
Closes#11093
changelog: [`unnecessary_literal_unwrap`]: also lint `unwrap_unchecked` and `unwrap_err_unchecked`
Use depinfo to discover UI test dependencies
changelog: none
context: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Building.20test.20dependencies
This restores [the old `EXTERN_FLAGS` method](4cf5bdc60c/tests/compile-test.rs (L67-L75)) of passing `--extern` flags for building UI tests with minor changes
- Unused deps were removed
- It's now a `Vec` of args instead of a command string
- It uses a `BTreeMap` so the extern flags are in alphabetical order and deterministic
I don't know if the `HOST_LIBS` part is still required, but I figured it best to leave it in for now. If the change is accepted we can take a look if it's needed in `rust-lang/rust` after the next sync
This isn't as pleasant as having a `Cargo.toml`, though there is something satisfying about knowing the dependencies are already built and not needing to invoke `cargo`
r? `@flip1995`
This commit fixes#11025 by removing checks for `todo!`,
`unimplemented!` and `unreachable!`.
Signed-off-by: Panagiotis Foliadis <pfoliadis@hotmail.com>
Uplift `clippy::fn_null_check` lint
This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.
## `incorrect_fn_null_checks`
(warn-by-default)
The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.
### Example
```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */
if (fn_ptr as *const ()).is_null() { /* ... */ }
```
### Explanation
Function pointers are assumed to be non-null, checking for their nullity is incorrect.
-----
Mostly followed the instructions for uplifting a clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
`@rustbot` label: +I-lang-nominated
r? compiler
[`unnecessary_literal_unwrap`]: don't lint if binding initializer comes from expansion
Fixes https://github.com/rust-lang/rust-clippy/discussions/11109
changelog: [`unnecessary_literal_unwrap`]: don't lint if binding initializer comes from expansion
"try this" -> "try"
Current help messages contain a mix of "try", "try this", and one "try this instead". In the spirit of #10631, this PR adopts the first, as it is the most concise.
It also updates the `lint_message_conventions` test to catch cases of "try this".
(Aside: #10120 unfairly contained multiple changes in one PR. I am trying to break that PR up into smaller pieces.)
changelog: Make help messages more concise ("try this" -> "try").
Add `needless_pass_by_ref_mut` lint
changelog: [`needless_pass_by_ref_mut`]: This PR add a new lint `needless_pass_by_ref_mut` which emits a warning in case a `&mut` function argument isn't used mutably. It doesn't warn on trait and trait impls functions.
Fixes#8863.
cargo dev fmt
cargo test passes
cargo test passes
refactor a lil
Update bool_comparison.stderr
heavily refactor + bump `clippy::version`
refactor
refactor
check bounds to increase accuracy, and add todos
new lint: `read_line_without_trim`
This adds a new lint that checks for calls to `Stdin::read_line` with a reference to a string that is then attempted to parse into an integer type without first trimming it, which is always going to fail at runtime.
This is something that I've seen happen a lot to beginners, because it's easy to run into when following the example of chapter 2 in the book where it shows how to program a guessing game.
It would be nice if we could point beginners to clippy and tell them "let's see what clippy has to say" and have clippy explain to them why it fails 👀
I think this lint can later be "generalized" to work not just for `Stdin` but also any `BufRead` (which seems to be where the guarantee about the trailing newline comes from) and also, matching/comparing it to a string slice that doesn't end in a newline character (e.g. `input == "foo"` is always going to fail)
changelog: new lint: [`read_line_without_trim`]
[`useless_vec`]: add more tests and don't lint inside of macros
Closes#11084.
I realized that the fix I added in #11081 itself also causes an error in a suggestion when inside of a macro. Example:
```rs
macro_rules! x {
() => {
for _ in vec![1, 2] {}
}
}
x!();
```
Here it would suggest replacing `vec![1, 2]` with `[x!()]`, because that's what the source callsite is (reminder: it does this to get the correct span of `x!()` for code like `for _ in vec![x!()]`), but that's wrong when *inside* macros, so I decided to make it not lint if the whole loop construct is inside a macro to avoid this issue.
changelog: [`useless_vec`]: add more tests and don't lint inside of macros
r? `@Alexendoo` since these were your tests, I figured it makes most sense to assign you
Don't lint manual_let_else in cases where ? would work
Don't lint `manual_let_else` where the question mark operator `?` would be sufficient, that is, mostly in cases like:
```Rust
let v = if let Some(v) = ex { v } else { return None };
```
Also, this PR emits the `question_mark` lint for `let...else` patterns that could be written with `?` (also, only `return None` like cases).
```
changelog: [`manual_let_else`]: don't lint in cases where question_mark already lints
changelog: [`question_mark`]: lint for `let Some(...) = ex else { return None };`
```
Fixes #8755
[`useless_vec`]: use the source span for initializer
Fixes#11075.
changelog: [`useless_vec`]: use the source span for the initializer expression when inside of a macro
[`arc_with_non_send_sync`]: don't lint if type has nested type parameters
Fixes#11076
changelog: [`arc_with_non_send_sync`]: don't lint if type has nested type parameters
r? `@Manishearth`
new lint: `type_id_on_box`
Closes#7687.
A new lint that detects calling `.type_id()` on `Box<dyn Any>` (and not on the underlying `dyn Any`), which can make up for some pretty confusing bugs!
changelog: new lint: [`type_id_on_box`]
Add `SPEEDTEST`
In the `master` branch, we currently don't have any way to test the performance of a single lint in changes.
This PR adds `SPEEDTEST`, the environment variable which lets you do a speed test on a lint / category of tests with various configuration options.
Maybe we should merge this with `lintcheck` 🤔
See the book page for more information.
changelog:none
`let_and_return`: lint 'static lifetimes, don't lint borrows in closures
Fixes#11056
Now also ignores functions returning `'static` lifetimes, since I noticed the `stdin.lock()` example was still being linted but doesn't need to be since https://github.com/rust-lang/rust/pull/93965
changelog: none
New lint [`tuple_array_conversions`]
Closes#10748
PS, the implementation is a bit ugly 😅 ~~I will likely refactor soon enough :)~~ Done :D
changelog: New lint [`tuple_array_conversions`]
Also, lint question_mark for `let...else` clauses that can be simplified to use `?`.
This lint isn't perfect as it doesn't support the unstable try blocks.
[significant_drop_tightening] Fix#10413Fix#10413
This is quite a rewrite that unfortunately took a large amount of time. I tried my best to comment what is going on to easy review but feel free to ask any question.
The problem basically is that the current algorithm is only taking into consideration single blocks which means that things like the following don't work or show unpredictable results.
```rust
let mutex = Mutex::new(1);
{
let lock = mutex.lock().unwrap();
{
let _ = *lock;
}
}
```
The solve the issue, each path that refers a lock is now being tracked individually.
```
changelog: [`significant_drop_tightening`]: Lift the restriction of only considerate single blocks
```
New lint [`redundant_at_rest_pattern`]
Closes#11011
It's always a great feeling when a new lint triggers on clippy itself 😄
changelog: New lint [`redundant_at_rest_pattern`]
suggests `is_some_and` over `map().unwrap`
changelog: Enhancement: [`option_map_unwrap_or`] now considers the [`msrv`] config when creating the suggestion.
* modified option_map_unwrap_or lint to recognise when an `Option<T>` is mapped to an `Option<bool>` with false being used when `None` is detected; suggests the use of `is_some_and` instead
* msrv is set to 1.70.0 for this lint; when `is_some_and` was stabilised
fixes#9125
[`question_mark`]: don't lint inside of `try` block
Fixes#8628.
Diff looks a bit noisy because I had to move the two functions into an impl, because they now need to access the structs `try_block_depth` field to see if they're inside a try block.
changelog: [`question_mark`]: don't lint inside of `try` block
[`option_if_let_else`]: suggest `.as_ref()` if scrutinee is of type `&Option<_>`
Fixes#10729
`Option::map_or` takes ownership, so if matching on an `&Option<_>`, we need to suggest `.as_ref()` before calling `map_or` to get the same effect and to not cause a borrowck error.
changelog: [`option_if_let_else`]: suggest `.as_ref()`/`.as_mut()` if scrutinee is of type `&Option<_>`/`&mut Option<_>`
[`unused_async`]: don't lint if function is part of a trait
Fixes#10459.
We shouldn't lint if the function is part of a trait, because the user won't be able to easily remove the `async`, as this will then not match with the function signature in the trait definition
changelog: [`unused_async`]: don't lint if function is part of a trait
Use substring matching for TESTNAME
Restores the previous behaviour of matching using a substring match rather than needing a full match
changelog: none
Add `BLESS` for compile-test and some cleanup
changelog: none
Allows passing the environment variable `BLESS` to bless tests, which is useful when you want to bless internal tests - `BLESS= cargo uitest -Finternal`
Also updates a place in the docs referring to `cargo dev bless` and removes some unused test deps
Port clippy away from compiletest to ui_test
Reasons to do this:
* runs completely on stable Rust
* is easier to extend with new features
* has its own dogfood test suite, so changes can be tested in [the `ui_test` repo](https://github.com/oli-obk/ui_test)
* supports dependencies from crates.io without having to manually fiddle with command line flags
* supports `ui-cargo`, `ui`, `ui-toml` out of the box, no need to find and run the tests ourselves
One thing that is a big difference to `compiletest` is that if a test emits *any* error, you need to mark all of them with `//~ ERROR:` annotations. Since many clippy tests did not have annotations, I changed many lints to be `warn` in their test so that only the `stderr` output is tested.
TODO:
* [ ] check that this still works as a subtree in the rustc repo
changelog: none
<!-- changelog_checked -->
Note: at present the latest changes needed for clippy are only available as a git dependency, but I expect to publish a new crates.io version soon
Check if `if` conditions always evaluate to true in `never_loop`
This fixes the example provided in #11004, but it shouldn't be closed as this is still an issue on like
```rust
let x = true;
if x { /* etc */ }`
```
This also makes `clippy_utils::consts::constant` handle `ConstBlock` and `DropTemps`.
changelog: [`never_loop`]: Check if `if` conditions always evaluate to true
Lint `mem_forget` if any fields are `Drop`
Closes#9298
I think this way of doing it (`needs_drop`) should be fine.
---
changelog: Enhancement: [`mem_forget`]: Now lints on types with fields that implement `Drop`
[#10996](https://github.com/rust-lang/rust-clippy/pull/10996)
[`format_push_string`]: look through `match` and `if` expressions
Closes#9493.
changelog: [`format_push_string`]: look through `match` and `if` expressions
[`get_unwrap`]: include a borrow in the suggestion if argument is not an integer literal
Fixes#9909
I have to say, I don't really understand what the previous logic was trying to do, but this fixes the linked bug.
It was checking if the argument passed to `.get()` can be parsed as a usize (i.e. if it's an integer literal, probably?), and if not, it wouldn't include a borrow? I don't know how we came to that conclusion, but that logic doesn't work:
```rs
let slice = &[1, 2];
let _r: &i32 = slice.get({ 1 }).unwrap();
// previous suggestion: slice[{ 1 }]
// the suggestion should be: &slice[{ 1 }]
```
Here the argument passed to it isn't an integer literal, but it should still include a borrow, because it would otherwise change the type from `&i32` to `i32`.
The exception is that if the parent of the `get().unwrap()` expr is a dereference or a method call or the like, we don't need an explicit borrow because it's automatically inserted by the compiler
changelog: [`get_unwrap`]: include a borrow in the suggestion if argument is not an integer literal
Don't lint [`iter_nth_zero`] in `next`
Closes#9820
This also *slightlyy* modifies the output of `iter_nth`, as I noticed the types' names weren't in backticks
changelog: [`iter_nth_zero`]: No longer lints in implementations of `Iterator::next`
[`single_match`]: don't lint if block contains comments
Fixes#8634
It now ignores matches with a comment in the "else" arm
changelog: [`single_match`]: don't lint if block contains comments
[`redundant_closure_call`]: handle nested closures
Fixes#9956.
This ended up being a much larger change than I'd thought, and I ended up having to pretty much rewrite it as a late lint pass, because it needs access to certain things that I don't think are available in early lint passes (e.g. getting the parent expr). I think this'll be required to fi-x #10922 anyway, so this is probably fine.
(edit: had to write "fi-x" because "fix" makes github think that this PR fixes it, which it doesn't 😅 )
Previously, it would suggest changing `(|| || 42)()()` to `|| 42()`, which is a type error (it needs parens: `(|| 42)()`). In my opinion, though, the suggested fix should have really been `42`, so that's what this PR changes.
changelog: [`redundant_closure_call`]: handle nested closures and rewrite as a late lint pass
Fix false positive of [self_named_module_files] and [mod_module_files]
changelog: [self_named_module_files] [mod_module_files]: No longer lints dependencies located in subdirectory of workspace
fixes#8887
---
First time contributor here, just read contribution guide today.
I have several questions:
1. ~Is it the correct way to use environment variable `CARGO_HOME` to get the location of cargo home directory?~
(Edit: Code no longer uses CARGO_HOME)
2. How to setup test for this PR? This involves multiple files and `CARGO_HOME` setup. ~Not sure how to do this.~
~Edit: Working on tests right now~ A workspace_test has been added
[`arithmetic_side_effects`] Fix#10792Fix#10792
```
changelog: [`arithmetic_side_effects`]: Retrieve field values of structures that are in constant environments
```
Ignore more type aliases in `unnecessary_cast`
This is potentially the worst code I've ever written, and even if not, it's very close to being on par with starb. This will ignore `call() as i32` and `local_obtained_from_call as i32` now.
This should fix every reasonable way to reproduce #10555, but likely not entirely.
changelog: Ignore more type aliases in `unnecessary_cast`
[`missing_panics_doc`]: pickup expect method
close#10240
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`missing_panics_doc`]: pickup expect method
new lint: `drain_collect`
Closes#10818.
This adds a new lint that looks for `.drain(..).collect()` and suggests replacing it with `mem::take`.
changelog: [`drain_collect`]: new lint
[`match_same_arms`]: don't lint if `non_exhaustive_omitted_patterns`
Fixes#10327
changelog: [`match_same_arms`]: Don't lint if `non_exhaustive_omitted_patterns` is `warn` or `deny`
from_over_into: Show suggestions for non-Self expanded paths
changelog: [`from_over_into`]: Show suggestions when the body contains macros not expanding to `Self`
Currently any path in a macro expansion causes the suggestion to be hidden, meaning most macro calls cause it to be hidden
Now it's only hidden if the expansion contains `Self`
[`unnecessary_fold`]: suggest turbofish if necessary
Fixes#10000
This adds turbofish `::<T>` to the suggestion in `unnecessary_fold`. This is necessary because the `Sum` trait is generic, which breaks inference when changing `fold()` to `sum()`.
changelog: [`unnecessary_fold`]: suggest turbofish if necessary
new lint [`single_range_in_vec_init`]
Lints on `vec![0..200]` (or `[0..200]`), suggesting either `(0..200).collect::<Vec<i32>>()` or `[0; 200]`.
Haven't tested it with anything that isn't primitive. Probably should!
Closes#10932
changelog: new lint [`single_range_in_vec_init`]
[`derivable_impls`]: don't lint if `default()` call expr unsize-coerces to trait object
Fixes#10158.
This fixes a FP where the derive-generated Default impl would have different behavior because of unsize coercion from `Box<T>` to `Box<dyn Trait>`:
```rs
struct S {
x: Box<dyn std::fmt::Debug>
}
impl Default for S {
fn default() -> Self {
Self {
x: Box::<()>::default()
// ^~ Box<()> coerces to Box<dyn Debug>
// #[derive(Default)] would call Box::<dyn Debug>::default()
}
}
}
```
(this intentionally only looks for trait objects `dyn` specifically, and not any unsize coercion, e.g. `&[i32; 5]` to `&[i32]`, because that breaks existing tests and isn't actually problematic, as far as I can tell)
changelog: [`derivable_impls`]: don't lint if `default()` call expression unsize-coerces to trait object
[`needless_doctest_main`]: ignore `main()` in `no_test` code fences
close#10491
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`needless_doctest_main`]: ignore `main()` in `no_test` code fence
[`map_unwrap_or`]: don't lint when referenced variable is moved
Fixes#10579.
The previous way of checking if changing `map(f).unwrap_or(a)` to `map_or(a, f)` is safe had a flaw when the argument to `unwrap_or` moves a binding and the `map` closure references that binding in some way.
It used to simply check if any of the identifiers in the `unwrap_or` argument are referenced in the `map` closure, but it didn't consider the case where the moved binding is referred to through references, for example:
```rs
let x = vec![1, 2];
let x_ref = &x;
Some(()).map(|_| x_ref.clone()).unwrap_or(x);
```
This compiles as is, but we cannot change it to `map_or`. This lint however did suggest changing it, because the simple way of checking if `x` is referenced anywhere in the `map` closure fails here. The safest thing to do here imo (what this PR does) is check if the moved value `x` is referenced *anywhere* in the body (before the `unwrap_or` call). One can always create a reference to the value and smuggle them into the closure, without actually referring to `x`. The original, linked issue shows another one such example:
```rs
let x = vec![1,2,3,0];
let y = x.strip_suffix(&[0]).map(|s| s.to_vec()).unwrap_or(x);
```
`x.strip_suffix(&[0])` creates a reference to `x` that is available through `s` inside of the `map` closure, so we can't change it to `map_or`.
changelog: [`map_unwrap_or`]: don't lint when referenced variable is moved
[`no_effect`]: Suggest adding `return` if applicable
Closes#10941
Unfortunately doesn't catch anything complex as `no_effect` already wouldn't, but I'm fine with that (it catches `ControlFlow` at least :D)
changelog: [`no_effect`]: Suggest adding `return` if statement has same type as function's return type and is the last statement in a block
Improve suggestion for [`needless_lifetimes`]
Fixes#10093
changelog: [`needless_lifetimes`]: Suggestion now points at the elidable lifetimes, rather than the entire function declaration
[`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`
this will check every local for `TerminatorKind::Drop` to ensure they can be evaluated at compile time, not sure if this is the best way to do this but MIR is confusing and it works so...
fixes#10617
changelog: [`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`
[`useless_vec`]: lint on `vec![_]` invocations that adjust to a slice
Fixes#2262 (well, actually my PR over at #10901 did do most of the stuff, but this PR implements the one last other case mentioned in the comments that my PR didn't fix)
Before this change, it would lint `(&vec![1]).iter().sum::<i32>()`, but not `vec![1].iter().sum::<i32>()`. This PR handles this case.
This also refactors a few things that I wanted to do in my other PR but forgot about.
changelog: [`useless_vec`]: lint on `vec![_]` invocations that adjust to a slice
Don't linting `as_conversions` in proc macros
Don't linting `as_conversions` if code was generated by procedural macro.
This PR fixes https://github.com/rust-lang/rust-clippy/issues/9657
I implemented the fix changing the lint code to be a `LateLintPass` in order to be able to use the `is_from_proc_macro` out of the box. If the reviwer thinks that it would be better to do the other way (implementing `WithSearchPat`) just let me know. I might need some help in implementing it for the `ustc_ast::ast::Expr`
changelog: [`as_conversions`] avoiding warnings in macro-generated code
Extend `explicit_iter_loop` and `explicit_into_iter_loop`
fixes#1518
Some included cleanups
* Split `for_loop` test into different files for each lint (partially).
* Move handling of some `into_iter` cases from `explicit_into_iter`.
---
changelog: Enhancement: [`explicit_iter_loop`]: Now also handles types that implement `IntoIterator`.
[#10416](https://github.com/rust-lang/rust-clippy/pull/10416)
changelog: Sugg: [`explicit_into_iter_loop`]: The suggestion now works on mutable references.
[#10416](https://github.com/rust-lang/rust-clippy/pull/10416)
<!-- changelog_checked -->
Add `needless_if` lint
first off: Sorry about the large diff. Seems a ton of tests do this (understandably so).
this is basically everything I wanted in #10868, while it doesn't lint *all* unnecessary empty blocks, it lints needless if statements; which are basically the crux of the issue (for me) anyway. I've committed code that includes this far too many times 😅 hopefully clippy can help me out soon
closes#10868
changelog: New lint [`needless_if`]
