When the character next to `{}` is "shifted" (when mapping a byte index
in the format string to span) we should avoid shifting the span end
index, so first map the index of `}` to span, then bump the span,
instead of first mapping the next byte index to a span (which causes
bumping the end span too much).
Regression test added.
Fixes#83344
This renames the variants in HIR UnOp from
enum UnOp {
UnDeref,
UnNot,
UnNeg,
}
to
enum UnOp {
Deref,
Not,
Neg,
}
Motivations:
- This is more consistent with the rest of the code base where most enum
variants don't have a prefix.
- These variants are never used without the `UnOp` prefix so the extra
`Un` prefix doesn't help with readability. E.g. we don't have any
`UnDeref`s in the code, we only have `UnOp::UnDeref`.
- MIR `UnOp` type variants don't have a prefix so this is more
consistent with MIR types.
- "un" prefix reads like "inverse" or "reverse", so as a beginner in
rustc code base when I see "UnDeref" what comes to my mind is
something like "&*" instead of just "*".
Implement Rust 2021 panic
This implements the Rust 2021 versions of `panic!()`. See https://github.com/rust-lang/rust/issues/80162 and https://github.com/rust-lang/rfcs/pull/3007.
It does so by replacing `{std, core}::panic!()` by a bulitin macro that expands to either `$crate::panic::panic_2015!(..)` or `$crate::panic::panic_2021!(..)` depending on the edition of the caller.
This does not yet make std's panic an alias for core's panic on Rust 2021 as the RFC proposes. That will be a separate change: c5273bdfb2 That change is blocked on figuring out what to do with https://github.com/rust-lang/rust/issues/80846 first.
Fix formatting for removed lints
- Don't add backticks for the reason a lint was removed. This is almost
never a code block, and when it is the backticks should be in the reason
itself.
- Don't assume clippy is the only tool that needs to be checked for
backwards compatibility
I split this out of https://github.com/rust-lang/rust/pull/80527/ because it kept causing tests to fail, and it's a good change to have anyway.
r? `@flip1995`
- Don't add backticks for the reason a lint was removed. This is almost
never a code block, and when it is the backticks should be in the reason
itself.
- Don't assume clippy is the only tool that needs to be checked for
backwards compatibility
make MIR graphviz generation use gsgdt
gsgdt [https://crates.io/crates/gsgdt] is a crate which provides an
interface for stringly typed graphs. It also provides generation of
graphviz dot format from said graph.
This is the first in a series of PRs on moving graphviz code out of rustc into normal crates and then implementating graph diffing on top of these crates.
r? `@oli-obk`
Update error to reflect that integer literals can have float suffixes
For example, `1` is parsed as an integer literal, but it can be turned
into a float with the suffix `f32`. Now the error calls them "numeric
literals" and notes that you can add a float suffix since they can be
either integers or floats.
Part of #68490.
Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.
For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
For example, `1` is parsed as an integer literal, but it can be turned
into a float with the suffix `f32`. Now the error calls them "numeric
literals" and notes that you can add a float suffix since they can be
either integers or floats.
Qualify `panic!` as `core::panic!` in non-built-in `core` macros
Fixes#78333.
-----
Otherwise code like this
#![no_implicit_prelude]
fn main() {
::std::todo!();
::std::unimplemented!();
}
will fail to compile, which is unfortunate and presumably unintended.
This changes many invocations of `panic!` in a `macro_rules!` definition
to invocations of `$crate::panic!`, which makes the invocations hygienic.
Note that this does not make the built-in macro `assert!` hygienic.
Otherwise code like this
#![no_implicit_prelude]
fn main() {
::std::todo!();
::std::unimplemented!();
}
will fail to compile, which is unfortunate and presumably unintended.
This changes many invocations of `panic!` in a `macro_rules!` definition
to invocations of `$crate::panic!`, which makes the invocations hygienic.
Note that this does not make the built-in macro `assert!` hygienic.
This should make Clippy more resilient and will unblock #78343.
This PR is made against rust-lang/rust to avoid the need for a subtree
sync at @flip1995's suggestion in rust-lang/rust-clippy#6310.
add error_occured field to ConstQualifs,
fix#76064
I wasn't sure what `in_return_place` actually did and not sure why it returns `ConstQualifs` while it's sibling functions return `bool`. So I tried to make as minimal changes to the structure as possible. Please point out whether I have to refactor it or not.
r? `@oli-obk`
cc `@RalfJung`
Stabilize some Result methods as const
Stabilize the following methods of Result as const:
- `is_ok`
- `is_err`
- `as_ref`
A test is also included, analogous to the test for `const_option`.
These methods are currently const under the unstable feature `const_result` (tracking issue: #67520).
I believe these methods to be eligible for stabilization because of the stabilization of #49146 (Allow if and match in constants) and the trivial implementations, see also: [PR#75463](https://github.com/rust-lang/rust/pull/75463) and [PR#76135](https://github.com/rust-lang/rust/pull/76135).
Note: these methods are the only methods currently under the `const_result` feature, thus this PR results in the removal of the feature.
Related: #76225
Update the test `redundant_pattern_matching`: check if `is_ok` and `is_err` are suggested within const contexts.
Also removes the `redundant_pattern_matching_const_result` test, as it is no longer needed.
We no longer lint assignments to const item fields in the
`temporary_assignment` lint, since this is now covered by the
`CONST_ITEM_MUTATION` lint.
Additionally, we `#![allow(const_item_mutation)]` in the
`borrow_interior_mutable_const.rs` test. Clippy UI tests are run with
`-D warnings`, which seems to cause builtin lints to prevent Clippy
lints from running.
Check whether locals are too large instead of whether accesses into them are too large
Essentially this stops const prop from attempting to optimize
```rust
let mut x = [0_u8; 5000];
x[42] = 3;
```
I don't expect this to be a perf improvement without #73656 (which is also where the lack of this PR will be a perf regression).
r? @wesleywiser
The reason we do not trigger these lints anymore is that clippy sets the mir-opt-level to 0, and the recent changes subtly changed how the const propagator works.
Stabilize `transmute` in constants and statics but not const fn
cc #53605 (leaving issue open so we can add `transmute` to `const fn` later)
Previous attempt: #64011
r? @RalfJung
cc @rust-lang/wg-const-eval
New lint `match_vec_item`
Added new lint to warn a match on index item which can panic. It's always better to use `get(..)` instead.
Closes#5500
changelog: New lint `match_on_vec_items`
- Show just one error message with multiple suggestions in case of
using multiple times an OS in target family position
- Only suggest #[cfg(unix)] when the OS is in the Unix family
- Test all the operating systems
Don't trigger while_let_on_iterator when the iterator is recreated every iteration
r? @phansch
Fixes#1654
changelog: Fix false positive in [`while_let_on_iterator`]
Downgrade match_bool to pedantic
I don't quite buy the justification in https://rust-lang.github.io/rust-clippy/. The justification is:
> It makes the code less readable.
In the Rust codebases I've worked in, I have found people were comfortable using `match bool` (selectively) to make code more readable. For example, initializing struct fields is a place where the indentation of `match` can work better than the indentation of `if`:
```rust
let _ = Struct {
v: {
...
},
w: match doing_w {
true => ...,
false => ...,
},
x: Nested {
c: ...,
b: ...,
a: ...,
},
y: if doing_y {
...
} else { // :(
...
},
z: ...,
};
```
Or sometimes people prefer something a bit less pithy than `if` when the meaning of the bool doesn't read off clearly from the condition:
```rust
if set.insert(...) {
... // ???
} else {
...
}
match set.insert(...) {
// set.insert returns false if already present
false => ...,
true => ...,
}
```
Or `match` can be a better fit when the bool is playing the role more of a value than a branch condition:
```rust
impl ErrorCodes {
pub fn from(b: bool) -> Self {
match b {
true => ErrorCodes::Yes,
false => ErrorCodes::No,
}
}
}
```
And then there's plain old it's-1-line-shorter, which means we get 25% more content on a screen when stacking a sequence of conditions:
```rust
let old_noun = match old_binding.is_import() {
true => "import",
false => "definition",
};
let new_participle = match new_binding.is_import() {
true => "imported",
false => "defined",
};
```
Bottom line is I think this lint fits the bill better as a pedantic lint; I don't think linting on this by default is justified.
changelog: Remove match_bool from default set of enabled lints
Fixes#4226
This introduces the lint await_holding_lock. For async functions, we iterate
over all types in generator_interior_types and look for types named MutexGuard,
RwLockReadGuard, or RwLockWriteGuard. If we find one then we emit a lint.
If let else mutex
changelog: Adds lint to catch incorrect use of `Mutex::lock` in `if let` expressions with lock calls in any of the blocks.
closes: #5219
Fix issue #2907.
Update the "borrow box" lint to avoid recommending the following
conversion:
```
// Old
pub fn f(&mut Box<T>) {...}
// New
pub fn f(&mut T) {...}
```
Given a mutable reference to a box, functions may want to change
"which" object the Box is pointing at.
This change avoids recommending removing the "Box" parameter
for mutable references.
changelog: Don't trigger [`borrow_box`] lint on `&mut Box` references
Update the "borrow box" lint to avoid recommending the following
conversion:
```
// Old
pub fn f(&mut Box<T>) {...}
// New
pub fn f(&mut T) {...}
```
Given a mutable reference to a box, functions may want to change
"which" object the Box is pointing at.
This change avoids recommending removing the "Box" parameter
for mutable references.
add lint futures_not_send
changelog: add lint futures_not_send
fixes#5379
~Remark: one thing that can (should?) still be improved is to directly include the error message from the `Send` check so that the programmer stays in the flow. Currently, getting the actual error message requires a restructuring of the code to make the `Send` constraint explicit.~
It now shows all unmet constraints for allowing the Future to be Send.
Do not lint in macros for match lints
Don't lint in macros for match lints, more precisely in `check_pat` and `check_local` where it was not the case.
changelog: none
fixes: #5362
large_enum_variant: Report sizes of variants
This reports the sizes of the largest and second-largest variants.
Closes#5459
changelog: `large_enum_variant`: Report the sizes of the largest and second-largest variants.
Disallow bit-shifting in integer_arithmetic
Make the `integer_arithmetic` lint detect all the operations that are defined as being capable of overflow in the [Rust Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), by also linting for bit-shifting operations (`<<`, `>>`).
changelog: Disallow bit-shifting in `integer_arithmetic`
Add lint on large non scalar const
This PR adds the new lint `non_scalar_const` that aims to warn against `const` declaration of large arrays. For performance, because of inlining, large arrays should be preferably declared as `static`.
Note: i made this one to warn on all const arrays, whether they are in a body function or not. I don't know if this is really necessary, i could just reduce this lint to variables out of function scope.
Fixes: #400
changelog: add new lint for large non-scalar types declared as const
Add lint for float in array comparison
Fixes#4277
changelog:
- Added new handler for expression of index kind (e.g. `arr[i]`). It returns a constant when both array and index are constant, or when the array is constant and all values are equal.
- Trigger float_cmp and float_cmp_const lint when comparing arrays. Allow for comparison when one of the arrays contains only zeros or infinities.
- Added appropriate tests for such cases.
This change adds a check to the `inconsistent_digit_grouping` to add a check for
NumericLiterals that follow the UUID format of 8-4-4-4-12.
If the NumericLiteral matches the UUID format, no further inconsistent grouping checks
will be performed.
Closes#5431
Check for clone-on-copy in argument positions
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.
Fixes#5436
changelog: apply clone_on_copy lint to func args as well
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.
Fixes#5436
Downgrade implicit_hasher to pedantic
From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher), this lint is intended to suggest:
```diff
- pub fn foo(map: &mut HashMap<i32, i32>) { }
+ pub fn foo<S: BuildHasher>(map: &mut HashMap<i32, i32, S>) { }
```
I think this is pedantic. I get that this lint can benefit core libraries like serde, but that's exactly the use case for pedantic lints; a library like serde will [enable clippy_pedantic](fd6741f4b0/src/lib.rs (L304)) and take the time to go through everything possible. Similar for libraries doing a libz blitz style checkup before committing to a 1.0 release; it would make sense to run through all the available pedantic lints then.
But otherwise, for most codebases and certainly for industrial codebases, the above suggested change just makes the codebase more obtuse for questionable benefit.
changelog: Remove implicit_hasher from default set of enabled lints
Check fn header along with decl when suggesting to implement trait
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.
Fixes#5413, #4290
changelog: check fn header along with decl for should_implement_trait
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.
Fixes#5413, #4290
Downgrade unreadable_literal to pedantic
As motivated by #5418. This is the top most commonly suppressed Clippy style lint, which indicates that the community has decided they don't share Clippy's opinion on the best style of this.
I've left the lint in as pedantic, though it could be that "restriction" would be better -- I can see this lint being useful as an opt-in restriction in some codebases.
changelog: Remove unreadable_literal from default set of enabled lints
Add new lint for `Result<T, E>.map_or(None, Some(T))`
Fixes#5414
PR Checklist
---
- [x] Followed lint naming conventions (the name is a bit awkward, but it seems to conform)
- [x] Added passing UI tests (including committed .stderr file)
- [x] cargo test passes locally
- [x] Executed cargo dev update_lints
- [x] Added lint documentation
- [x] Run cargo dev fmt
`Result<T, E>` has an [`ok()`](https://doc.rust-lang.org/std/result/enum.Result.html#method.ok) method that adapts a `Result<T,E>` into an `Option<T>`.
It's possible to get around this adapter by writing `Result<T,E>.map_or(None, Some)`.
This lint is implemented as a new variant of the existing [`option_map_none` lint](https://github.com/rust-lang/rust-clippy/pull/2128)
