Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ptr_arg]: recognize methods that also exist on slices #11817

Merged
merged 2 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion clippy_lints/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
use std::{fmt, iter};

use crate::vec::is_allowed_vec_method;

declare_clippy_lint! {
/// ### What it does
/// This lint checks for function arguments of type `&String`, `&Vec`,
Expand Down Expand Up @@ -661,7 +663,7 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &'tcx Body<'_>, args:
},
// If the types match check for methods which exist on both types. e.g. `Vec::len` and
// `slice::len`
ty::Adt(def, _) if def.did() == args.ty_did => {
ty::Adt(def, _) if def.did() == args.ty_did && !is_allowed_vec_method(self.cx, e) => {
set_skip_flag();
},
_ => (),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn adjusts_to_slice(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
/// Checks if the given expression is a method call to a `Vec` method
/// that also exists on slices. If this returns true, it means that
/// this expression does not actually require a `Vec` and could just work with an array.
fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
pub fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
const ALLOWED_METHOD_NAMES: &[&str] = &["len", "as_ptr", "is_empty"];

if let ExprKind::MethodCall(path, ..) = e.kind {
Expand Down
4 changes: 2 additions & 2 deletions lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl Crate {
target_dir_index: &AtomicUsize,
total_crates_to_lint: usize,
config: &LintcheckConfig,
lint_filter: &Vec<String>,
lint_filter: &[String],
server: &Option<LintcheckServer>,
) -> Vec<ClippyWarning> {
// advance the atomic index by one
Expand Down Expand Up @@ -728,7 +728,7 @@ fn read_stats_from_file(file_path: &Path) -> HashMap<String, usize> {
}

/// print how lint counts changed between runs
fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, usize>, lint_filter: &Vec<String>) {
fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, usize>, lint_filter: &[String]) {
let same_in_both_hashmaps = old_stats
.iter()
.filter(|(old_key, old_val)| new_stats.get::<&String>(old_key) == Some(old_val))
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
#![allow(clippy::if_same_then_else, clippy::no_effect, clippy::redundant_closure_call)]
#![allow(
clippy::if_same_then_else,
clippy::no_effect,
clippy::redundant_closure_call,
clippy::ptr_arg
)]
#![warn(clippy::needless_pass_by_ref_mut)]
#![feature(lint_reasons)]
//@no-rustfix
Expand Down
42 changes: 21 additions & 21 deletions tests/ui/needless_pass_by_ref_mut.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:7:11
--> $DIR/needless_pass_by_ref_mut.rs:12:11
|
LL | fn foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`
Expand All @@ -8,131 +8,131 @@ LL | fn foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
= help: to override `-D warnings` add `#[allow(clippy::needless_pass_by_ref_mut)]`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:32:12
--> $DIR/needless_pass_by_ref_mut.rs:37:12
|
LL | fn foo6(s: &mut Vec<u32>) {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:45:29
--> $DIR/needless_pass_by_ref_mut.rs:50:29
|
LL | fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:50:31
--> $DIR/needless_pass_by_ref_mut.rs:55:31
|
LL | fn badger(&mut self, vec: &mut Vec<i32>) -> usize {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:127:16
--> $DIR/needless_pass_by_ref_mut.rs:132:16
|
LL | async fn a1(x: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:131:16
--> $DIR/needless_pass_by_ref_mut.rs:136:16
|
LL | async fn a2(x: &mut i32, y: String) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:135:16
--> $DIR/needless_pass_by_ref_mut.rs:140:16
|
LL | async fn a3(x: &mut i32, y: String, z: String) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:139:16
--> $DIR/needless_pass_by_ref_mut.rs:144:16
|
LL | async fn a4(x: &mut i32, y: i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:143:24
--> $DIR/needless_pass_by_ref_mut.rs:148:24
|
LL | async fn a5(x: i32, y: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:147:24
--> $DIR/needless_pass_by_ref_mut.rs:152:24
|
LL | async fn a6(x: i32, y: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:151:32
--> $DIR/needless_pass_by_ref_mut.rs:156:32
|
LL | async fn a7(x: i32, y: i32, z: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:155:24
--> $DIR/needless_pass_by_ref_mut.rs:160:24
|
LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:155:45
--> $DIR/needless_pass_by_ref_mut.rs:160:45
|
LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:189:16
--> $DIR/needless_pass_by_ref_mut.rs:194:16
|
LL | fn cfg_warn(s: &mut u32) {}
| ^^^^^^^^ help: consider changing to: `&u32`
|
= note: this is cfg-gated and may require further changes

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:195:20
--> $DIR/needless_pass_by_ref_mut.rs:200:20
|
LL | fn cfg_warn(s: &mut u32) {}
| ^^^^^^^^ help: consider changing to: `&u32`
|
= note: this is cfg-gated and may require further changes

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:209:39
--> $DIR/needless_pass_by_ref_mut.rs:214:39
|
LL | async fn inner_async2(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:217:26
--> $DIR/needless_pass_by_ref_mut.rs:222:26
|
LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:236:34
--> $DIR/needless_pass_by_ref_mut.rs:241:34
|
LL | pub async fn call_in_closure1(n: &mut str) {
| ^^^^^^^^ help: consider changing to: `&str`
|
= warning: changing this function will impact semver compatibility

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:248:25
--> $DIR/needless_pass_by_ref_mut.rs:253:25
|
LL | pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
| ^^^^^^^^^^ help: consider changing to: `&usize`
|
= warning: changing this function will impact semver compatibility

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:255:20
--> $DIR/needless_pass_by_ref_mut.rs:260:20
|
LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
| ^^^^^^^^^^ help: consider changing to: `&usize`
|
= warning: changing this function will impact semver compatibility

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:266:26
--> $DIR/needless_pass_by_ref_mut.rs:271:26
|
LL | pub async fn closure4(n: &mut usize) {
| ^^^^^^^^^^ help: consider changing to: `&usize`
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/ptr_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ fn do_vec_mut(x: &mut Vec<i64>) {
//Nothing here
}

fn do_vec_mut2(x: &mut Vec<i64>) {
//~^ ERROR: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice w
x.len();
x.is_empty();
}

fn do_str(x: &String) {
//~^ ERROR: writing `&String` instead of `&str` involves a new object where a slice will d
//Nothing here either
Expand Down