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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

suspicious_double_ref_op: don't lint on .borrow() #112517

Merged
merged 2 commits into from
Jun 16, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -478,13 +478,11 @@ lint_requested_level = requested on the command line with `{$level} {$lint_name}
lint_supertrait_as_deref_target = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target
.label = target type is set here

lint_suspicious_double_ref_op =
using `.{$call}()` on a double reference, which returns `{$ty}` instead of {$op ->
*[should_not_happen] [{$op}]
[deref] dereferencing
[borrow] borrowing
[clone] cloning
} the inner type
lint_suspicious_double_ref_clone =
using `.clone()` on a double reference, which returns `{$ty}` instead of cloning the inner type

lint_suspicious_double_ref_deref =
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type

lint_trivial_untranslatable_diag = diagnostic with static strings only

Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1231,11 +1231,15 @@ pub struct NoopMethodCallDiag<'a> {
}

#[derive(LintDiagnostic)]
#[diag(lint_suspicious_double_ref_op)]
pub struct SuspiciousDoubleRefDiag<'a> {
pub call: Symbol,
#[diag(lint_suspicious_double_ref_deref)]
pub struct SuspiciousDoubleRefDerefDiag<'a> {
pub ty: Ty<'a>,
}

#[derive(LintDiagnostic)]
#[diag(lint_suspicious_double_ref_clone)]
pub struct SuspiciousDoubleRefCloneDiag<'a> {
pub ty: Ty<'a>,
pub op: &'static str,
}

// pass_by_value.rs
Expand Down
62 changes: 34 additions & 28 deletions compiler/rustc_lint/src/noop_method_call.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::context::LintContext;
use crate::lints::{NoopMethodCallDiag, SuspiciousDoubleRefDiag};
use crate::lints::{
NoopMethodCallDiag, SuspiciousDoubleRefCloneDiag, SuspiciousDoubleRefDerefDiag,
};
use crate::LateContext;
use crate::LateLintPass;
use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -76,22 +78,22 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {

// We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
// traits and ignore any other method call.
let did = match cx.typeck_results().type_dependent_def(expr.hir_id) {
// Verify we are dealing with a method/associated function.
Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
// Check that we're dealing with a trait method for one of the traits we care about.
Some(trait_id)
if matches!(
cx.tcx.get_diagnostic_name(trait_id),
Some(sym::Borrow | sym::Clone | sym::Deref)
) =>
{
did
}
_ => return,
},
_ => return,

let Some((DefKind::AssocFn, did)) =
cx.typeck_results().type_dependent_def(expr.hir_id)
else {
return;
};

let Some(trait_id) = cx.tcx.trait_of_item(did) else { return };

if !matches!(
cx.tcx.get_diagnostic_name(trait_id),
Some(sym::Borrow | sym::Clone | sym::Deref)
) {
return;
};

let substs = cx
.tcx
.normalize_erasing_regions(cx.param_env, cx.typeck_results().node_substs(expr.hir_id));
Expand All @@ -102,13 +104,6 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
// (Re)check that it implements the noop diagnostic.
let Some(name) = cx.tcx.get_diagnostic_name(i.def_id()) else { return };

let op = match name {
sym::noop_method_borrow => "borrow",
sym::noop_method_clone => "clone",
sym::noop_method_deref => "deref",
_ => return,
};

let receiver_ty = cx.typeck_results().expr_ty(receiver);
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
let arg_adjustments = cx.typeck_results().expr_adjustments(receiver);
Expand All @@ -129,11 +124,22 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span },
);
} else {
cx.emit_spanned_lint(
SUSPICIOUS_DOUBLE_REF_OP,
span,
SuspiciousDoubleRefDiag { call: call.ident.name, ty: expr_ty, op },
)
match name {
// If `type_of(x) == T` and `x.borrow()` is used to get `&T`,
// then that should be allowed
sym::noop_method_borrow => return,
sym::noop_method_clone => cx.emit_spanned_lint(
SUSPICIOUS_DOUBLE_REF_OP,
span,
SuspiciousDoubleRefCloneDiag { ty: expr_ty },
),
sym::noop_method_deref => cx.emit_spanned_lint(
SUSPICIOUS_DOUBLE_REF_OP,
span,
SuspiciousDoubleRefDerefDiag { ty: expr_ty },
),
_ => return,
}
}
}
}
17 changes: 17 additions & 0 deletions tests/ui/lint/issue-112489.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// check-pass
use std::borrow::Borrow;

struct S;

trait T: Sized {
fn foo(self) {}
}

impl T for S {}
impl T for &S {}

fn main() {
let s = S;
s.borrow().foo();
s.foo();
}