Add test for #8855Fix#8855
Here is what I think is going on.
First, the expression `format!("{:>6} {:>6}", a, b.to_string())` expands to:
```rust
{
let res =
::alloc::fmt::format(::core::fmt::Arguments::new_v1_formatted(&["",
" "],
&[::core::fmt::ArgumentV1::new_display(&a),
::core::fmt::ArgumentV1::new_display(&b.to_string())],
&[::core::fmt::rt::v1::Argument {
position: 0usize,
format: ::core::fmt::rt::v1::FormatSpec {
fill: ' ',
align: ::core::fmt::rt::v1::Alignment::Right,
flags: 0u32,
precision: ::core::fmt::rt::v1::Count::Implied,
width: ::core::fmt::rt::v1::Count::Is(6usize),
},
},
::core::fmt::rt::v1::Argument {
position: 1usize,
format: ::core::fmt::rt::v1::FormatSpec {
fill: ' ',
align: ::core::fmt::rt::v1::Alignment::Right,
flags: 0u32,
precision: ::core::fmt::rt::v1::Count::Implied,
width: ::core::fmt::rt::v1::Count::Is(6usize),
},
}], unsafe { ::core::fmt::UnsafeArg::new() }));
res
}
```
When I dump the expressions that get past the call to `has_string_formatting` [here](b312ad7d0c/clippy_lints/src/format_args.rs (L83)), I see more than I would expect.
In particular, I see this subexpression of the above:
```
&[::core::fmt::ArgumentV1::new_display(&a),
::core::fmt::ArgumentV1::new_display(&b.to_string())],
```
This suggests to me that more expressions are getting past [this call](b312ad7d0c/clippy_lints/src/format_args.rs (L71)) to `FormatArgsExpn::parse` than should.
Those expressions are then visited, but no `::core::fmt::rt::v1::Argument`s are found and pushed [here](b312ad7d0c/clippy_utils/src/macros.rs (L407)).
As a result, the expressions appear unformatted, hence, the false positive.
My proposed fix is to restrict `FormatArgsExpn::parse` so that it only matches `Call` expressions.
cc: `@akanalytics`
changelog: none
Lint `collapsible_str_replace`
fixes#6651
```
changelog: [`collapsible_str_replace`]: create new lint `collapsible_str_replace`
```
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
Rework `only_used_in_recursion`
fixes#8782fixes#8629fixes#8560fixes#8556
This is a complete rewrite of the lint. This loses some capabilities of the old implementation. Namely the ability to track through tuple and slice patterns, as well as the ability to trace through assignments.
The two reported bugs are fixed with this. One was caused by using the name of the method rather than resolving to the `DefId` of the called method. The second was cause by using the existence of a cycle in the dependency graph to determine whether the parameter was used in recursion even though there were other ways to create a cycle in the graph.
Implementation wise this switches from using a visitor to walking up the tree from every use of each parameter until it has been determined the parameter is used for something other than recursion. This is likely to perform better as it avoids walking the entire function a second time, and it is unlikely to walk up the HIR tree very much. Some cases would perform worse though.
cc `@buttercrab`
changelog: Scale back `only_used_in_recursion` to fix false positives
changelog: Move `only_used_in_recursion` back to `complexity`
Refactor `FormatArgsExpn`
It now for each format argument `{..}` has:
- The `Expr` it points to, and how it does so (named/named inline/numbered/implicit)
- The parsed `FormatSpec` (format trait/fill/align/etc., the precision/width and any value they point to)
- Many spans
The caller no longer needs to pair up arguments to their value, or separately interpret the `specs` `Expr`s when it isn't `None`
The gist is that it combines the result of [`rustc_parse_format::Parser`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_parse_format/struct.Parser.html) with the macro expansion itself
This unfortunately makes the code a bit longer, however we need to use both as neither have all the information we're after. `rustc_parse_format` doesn't have the information to resolve named arguments to their values. The macro expansion doesn't contain whether the positions are implicit/numbered/named, or the spans for format arguments
Wanted by #9233 and #8518 to be able to port the changes from #9040
Also fixes#8643, previously the format args seem to have been paired up with the wrong values somehow
changelog: [`format_in_format_args`]: Fix false positive due to misattributed arguments
r? `@flip1995`
cc `@nyurik`
Don't lint on match pattern-binding in ´question_mark`
Fixes#9347
Technically it is possible to have a blank match-pattern that does nothing, and we fail to lint. But it's easier to be safe than sorry here.
changelog: [`question_mark`]: don't lint `if let`s with subpatterns
Enhance `needless_borrow` to consider trait implementations
The proposed enhancement causes `needless_borrow` to suggest removing `&` from `&e` when `&e` is an argument position requiring trait implementations, and `e` implements the required traits. Example:
```
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:131:51
|
LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
| ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
```
r? `@Jarcho`
changelog: Enhance `needless_borrow` to consider trait implementations
unwrap_used and expect_used: trigger on uses of their _err variants
changelog: [`unwrap_used`]: lint uses of `unwrap_err`
changelog: [`expect_used`]: lint uses of `expect_err`
fixes#9331
`transmute_undefined_repr` fix
changelog: Don't lint `transmute_undefined_repr` when the the first field of a `repr(C)` type is compatible with the other type
