Don't check [print/write]_with_newline on raw strings
Some tests for #3778 and some maybe-not-the-greatest code that passes those tests!
I didn't run `fmt` because a) it doesn't seem to install on nightly for me, and b) on stable it wanted to apply formatting to over 90 files. Happy to make any tweaks though!
I suspect this contribution may require more than just tweaks. I'm still sort of new to rust so it may not be idiomatic, and the specific approach I took feels a little heavy-handed and brittle. I'm happy to make changes with some guidance, or equally happy if this gives a starting place for someone else to do it better :)
Add a lint to warn on `T: Drop` bounds
**What it does:** Checks for generics with `std::ops::Drop` as bounds.
**Why is this bad?** `Drop` bounds do not really accomplish anything.
A type may have compiler-generated drop glue without implementing the
`Drop` trait itself. The `Drop` trait also only has one method,
`Drop::drop`, and that function is by fiat not callable in user code.
So there is really no use case for using `Drop` in trait bounds.
**Known problems:** None.
**Example:**
```rust
fn foo<T: Drop>() {}
```
Fixes#3773
Both regular strings and raw strings can contain literal newlines. This commit
extends the lint to also warn about terminating strings with these.
Behaviour handling for raw strings is also moved into `check_newlines` by
passing in the `is_raw` boolean from `check_tts` as
[suggested](https://github.com/rust-lang/rust-clippy/pull/3781#pullrequestreview-204663732)
Pass tests for #3778, {print,write}_with_newline false positive
This change guards the lint from checking newlines with a sort of complicated
check to see if it's a raw string. Raw strings shouldn't be newline-checked,
since r"\n" is literally \-n, not a newline. I think it's ok not to check for
_literal_ newlines at the end of raw strings, but maybe that's debatable.
I... don't think this code is that great. I wanted to write the check after
`check_tts`, but that was too late -- raw string type info is lost (or I
couldn't find it). Putting it inside `check_tts` feels heavy-duty and the check
itself feels like a brittle reach possibly into places it shouldn't.
Maybe someone can fix this up :)
**What it does:** Checks for generics with `std::ops::Drop` as bounds.
**Why is this bad?** `Drop` bounds do not really accomplish anything.
A type may have compiler-generated drop glue without implementing the
`Drop` trait itself. The `Drop` trait also only has one method,
`Drop::drop`, and that function is by fiat not callable in user code.
So there is really no use case for using `Drop` in trait bounds.
**Known problems:** None.
**Example:**
```rust
fn foo<T: Drop>() {}
```
Fix ICE in needless_pass_by_value lint
If I understand it correctly, we were first creating a type with a
`RegionKind::ReErased` region and then deleted it again in
`util::implements_trait` with:
cx.tcx.erase_regions(&ty);
causing the type query to fail.
It looks like using `ReEmpty` works around that deletion.
Fixes#3144
Macro check for assertion_on_constants lint
The `assertion_on_constants` lint currently has following output for this code [Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6f2c9df6fc50baf847212d3b5136ee97):
```rust
macro_rules! assert_const {
($len:expr) => {
assert!($len > 0);
}
}
fn main() {
assert_const!(3);
assert_const!(-1);
}
```
```
warning: assert!(const: true) will be optimized out by the compiler
--> src/main.rs:3:9
|
3 | assert!($len > 0);
| ^^^^^^^^^^^^^^^^^^
...
8 | assert_const!(3);
| ---------------- in this macro invocation
|
= note: #[warn(clippy::assertions_on_constants)] on by default
= help: remove it
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
warning: assert!(const: false) should probably be replaced
--> src/main.rs:3:9
|
3 | assert!($len > 0);
| ^^^^^^^^^^^^^^^^^^
...
9 | assert_const!(-1);
| ----------------- in this macro invocation
|
= help: use panic!() or unreachable!()
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
```
This is contradictory. This lint should not trigger if the `assert!` is in a macro itself.
Add a uitest subcommand to simplify UI test invocation
This makes running single tests a lot easier.
It's now
`TESTNAME=xxx cargo uitest`
instead of
`TESTNAME=xxx cargo test --test compile-test`
Start making clippy easier to invoke in non-cargo contexts
Clippy (clippy-driver) currently has a couple of strong but unnecessary couplings with cargo. This series:
1. makes detection of check builds more robust, and
2. make clippy-driver use the --sysroot specified on the command line as its internal sysroot.
If I understand it correctly, we were first creating a type with a
`RegionKind::ReErased` region and then deleted it again in
`util::implements_trait` with:
cx.tcx.erase_regions(&ty);
causing the type query to fail.
It looks like using `ReEmpty` works around that deletion.
Fix `cast_sign_loss` false positive
This checks if the value is a non-negative constant before linting about
losing the sign.
Because the `constant` function doesn't handle const functions, we check if
the value is from a call to a `max_value` function directly. A utility method
called `get_def_path` was added to make checking for the function paths
easier.
Fixes#2728
The rustc change added HirId to a few nodes. As I understand it, the plan is
to remove the NodeId from these nodes eventually. Where the NodeId was
not being matched, I used `..` to try and avoid further breakage. Where it
was, I used `_` to make the fix easier when NodeId is removed.
Adding lint test for excessive LOC.
This is a WIP for #2377. Just wanted to pull in because I had a few questions:
1. Is it okay that I'm approaching this via counting by looking at each line in the snippet instead of looking at the AST tree? If there's another way to do it, I want to make sure I'm doing the correct way, but I wasn't sure since the output AST JSON doesn't seem to contain whitespace.
2. My function is definitely going to trigger the lint, so also wanted to see if there was something obvious I could do to reduce it.
3. Are the two tests fine, or is there something obvious I'm missing?
4. Obviously bigger question - am I approaching the line count correctly. Current strategy is count a line if it contains some code, so skip if it's just comments or empty.
`hir::Ty` doesn't seem to know anything about type bounds and
`cx.tcx.type_of(def_id)` caused an ICE when it was passed a generic type
with a bound:
```
src/librustc_typeck/collect.rs:1311: unexpected non-type Node::GenericParam: Type { default: None, synthetic: None }
```
Converting it to a proper `Ty` fixes the ICE and catches a few more
places where the lint applies.
This checks if the value is a non-negative constant before linting about
losing the sign.
Because the `constant` function doesn't handle const functions, we check if
the value is from a call to a `max_value` function directly. A utility method
called `get_def_path` was added to make checking for the function paths
easier.
Fixes#2728
Add initial version of const_fn lint
This adds an initial version of a lint that can tell if a function could be `const`.
TODO:
- [x] Finish up the docs
- [x] Fix the ICE
cc #2440
Prevent incorrect cast_lossless suggestion in const_fn
`::from` is not a const fn, so applying the suggestion of
`cast_lossless` would fail to compile. The fix is to skip the lint if
the cast is found inside a const fn.
Fixes#3656
Fix documentation for `slow_vector_initialization`
This PR fixes the documentation for the lint `slow_vector_initialization`. The documentation recommended writing `vec![len; 0]` but the correct solution is `vec![0; len]`.
for file in `fd \.rs$` ; do sed -i s/span_suggestion_with_applicability/span_suggestion/g $file ; done
for file in `fd \.rs$` ; do sed -i s/span_suggestion_short_with_applicability/span_suggestion_short/g $file ; done
for file in `fd \.rs$` ; do sed -i s/span_suggestions_with_applicability/span_suggestions/g $file ; done
Fix `expect_fun_call` lint suggestions
This commit corrects some bad suggestions produced by the
`expect_fun_call` lint and enables `rust-fix` checking on the tests.
Addresses #3630
`::from` is not a const fn, so applying the suggestion of
`cast_lossless` would fail to compile. The fix is to skip the lint if
the cast is found inside a const fn.
* master: (58 commits)
Rustfmt all the things
Don't make decisions on values that don't represent the decision
Improving comments.
Rustup
Added rustfix to the test.
Improve span shortening.
Added "make_return" and "blockify" convenience methods in Sugg and used them in "needless_bool".
Actually check for constants.
Fixed potential mistakes with nesting. Added tests.
formatting fix
Update clippy_lints/src/needless_bool.rs
formatting fix
Fixing typo in CONTRIBUTING.md
Fix breakage due to rust-lang/rust#57651
needless bool lint suggestion is wrapped in brackets if it is an "else" clause of an "if-else" statement
Fix automatic suggestion on `use_self`.
Remove negative integer literal checks.
Fix `implicit_return` false positives.
Run rustfmt
Fixed breakage due to rust-lang/rust#57489
...
Fix automatic suggestion on `use_self`.
In an example like this:
```rust
impl Example {
fn fun_1() { }
fn fun_2() {
Example::fun_1();
}
}
```
Clippy tries to replace `Example::fun_1` with `Self`, loosing `::fun_1` in the process, it should rather try to replace `Example` with `Self`.
**Question**
- There may be other paths that need the same treatment, but I'm not sure I understand them fully:
- e648adf086/clippy_lints/src/use_self.rs (L94-L96)
- e648adf086/clippy_lints/src/use_self.rs (L225-L229)
Fix `implicit_return` false positives.
Fixes the following false positives:
- linting on `if let` without `else` in a `loop` even with a present `return`
- linting on `unreachable!()`
Catch up with `format_args` change
Catches up with a change in rust-lang/rust#57537. (Since the optimization is optional, this clippy PR can be merged before the rustc PR.)
Happened to fix a bug in `expect_fun_call`, that is the lint ignores more than
one arguments to `format`.
```
warning: use of `expect` followed by a function call
--> src/main.rs:2:17
|
2 | Some("foo").expect(format!("{} {}", 1, 2).as_ref());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{} {}", 1))`
|
```
Catches up with a change in rust-lang/rust#57537
Happened to fix a bug in `expect_fun_call`, that is the lint ignores more than
one arguments to `format`.
`cloned` requires that the elements of the iterator must be references. This
change determines if that is the case by examining the type of the closure
argument and suggesting `.cloned` only if it is a reference. When the closure
argument is not a reference, it suggests removing the `map` call instead.
A minor problem with this change is that the new check sometimes overlaps
with the `clone_on_copy` lint.
Fixes#498
