Literal error reporting cleanup
While doing some performance work, I noticed some code duplication in `librustc_parser/lexer/mod.rs`, so I cleaned it up.
This PR is probably best reviewed commit by commit.
I'm not sure what the API stability practices for `librustc_lexer` are. Four public methods in `unescape.rs` can be removed, but two are used by clippy, so I left them in for now.
I could open a PR for Rust-Analyzer when this one lands.
But how do I open a PR for clippy? (Git submodules are frustrating to work with)
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
Cleanup: `node_id` -> `hir_id`
This removes some more `node_id` terminology from Clippy and replaces one occurrence of `as_local_node_id` with `as_local_hir_id`, which should be doing the same for that particular case.
changelog: none
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.
Fixes issue #4892.
First contribution here 😊 ! Do not hesitate to correct me.
This PR is related to issue #4892 .
# Summary
```rust
-literal.method_call(args)
```
The main idea is to not trigger `clippy::precedence` when the method call is an odd function.
# Example
```rust
// should trigger lint
let _ = -1.0_f64.abs() //precedence of method call abs() and neg ('-') is ambiguous
// should not trigger lint
let _ = -1.0_f64.sin() // sin is an odd function => -sin(x) = sin(-x)
```
# Theory
Rust allows following literals:
- char
- string
- integers
- floats
- byte
- bool
Only integers/floats implements the relevant `std::ops::Neg`.
Following odd functions are implemented on i[8-128] and/or f[32-64]:
- `asin`
- `asinh`
- `atan`
- `atanh`
- `cbrt`
- `fract`
- `round`
- `signum`
- `sin`
- `sinh`
- `tan`
- `tanh `
- `to_degrees`
- `to_radians`
# Implementation
As suggested by `flip1995` in [comment](https://github.com/rust-lang/rust-clippy/issues/4892#issuecomment-568249683), this PR add a whitelist of odd functions and compare method call to the the whitelist before triggering lint.
changelog: Don't trigger [`clippy::precedence`] on odd functions.
question_mark: don't add `as_ref()` for a call expression
If a call returns a `!Copy` value, it does so regardless of whether `as_ref()` is added. For example, `foo.into_option().as_ref()?` can be simplified to `foo.into_option()?`.
---
changelog: Improved `question_mark` lint suggestion so that it doesn't add redundant `as_ref()`
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 explicit deref and deref_mut method calls
This PR adds the lint `explicit_deref_method` that suggests replacing `deref()` and `deref_mut()` with `&*a` and `&mut *a`.
It doesn't lint inside macros.
This PR is the continuation of #3258.
changelog: Add lint `explicit_deref_method`.
Fixes: #1566
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.
Refactor: Use rustc's `match_def_path`
This replaces our match_def_path implementation with the rustc one.
Note that we can't just use it in all call sites because of the
`&[&str]` / `&[Symbol]` difference in Clippy/rustc.
changelog: none
This replaces our match_def_path implementation with the rustc one.
Note that we can't just use it in all call sites because of the
`&[&str]` / `&[Symbol]` difference in Clippy/rustc.
Make use of more diagnostic items
This makes use of some (not all) already existing diagnostic items. Specifically:
* 79982a2: `core::mem::uninitialized`, `core::mem::zeroed`, `alloc::sync::Arc`, `alloc::sync::Rc`
* 83874d0: `Option` and `Result`
cc #5393
changelog: none
Fixes#5405: redundant clone false positive with arrays
Check whether slice elements implement Copy before suggesting to drop
the clone method
changelog: add a check for slice indexing on redundant_clone lint
Update documentation for new_ret_no_self
changelog: Update documentation for lint new_ret_no_self to reflect that the return type must only contain `Self`, not be `Self`
The lint was changed to be more lenient than the documentation implies in PR #3338 (Related issue #3313)
Change the existing hex bit mask (`0x1111`) to a binary one (`0b1111`).
The former does not seem to have anything to do with trailing zeros and is
probably a typo.
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
