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
