Skip to content

Commit 4a1825a

Browse files
committed
Auto merge of #6952 - Jarcho:new_ret_no_self_fp, r=Manishearth
Fix `new_ret_no_self` false positive fixes: #1724 changelog: Fix false positive with `new_ret_no_self` when returning `Self` with different generic arguments
2 parents 917b538 + 6e88900 commit 4a1825a

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

clippy_lints/src/methods/mod.rs

+11-3
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ mod zst_offset;
6161

6262
use bind_instead_of_map::BindInsteadOfMap;
6363
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
64-
use clippy_utils::ty::{contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
64+
use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
6565
use clippy_utils::{contains_return, get_trait_def_id, in_macro, iter_input_pats, method_calls, paths, return_ty};
6666
use if_chain::if_chain;
6767
use rustc_hir as hir;
@@ -1916,7 +1916,11 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
19161916
let ret_ty = return_ty(cx, impl_item.hir_id());
19171917

19181918
// walk the return type and check for Self (this does not check associated types)
1919-
if contains_ty(ret_ty, self_ty) {
1919+
if let Some(self_adt) = self_ty.ty_adt_def() {
1920+
if contains_adt_constructor(ret_ty, self_adt) {
1921+
return;
1922+
}
1923+
} else if contains_ty(ret_ty, self_ty) {
19201924
return;
19211925
}
19221926

@@ -1926,7 +1930,11 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
19261930
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
19271931
if let ty::PredicateKind::Projection(projection_predicate) = predicate.kind().skip_binder() {
19281932
// walk the associated type and check for Self
1929-
if contains_ty(projection_predicate.ty, self_ty) {
1933+
if let Some(self_adt) = self_ty.ty_adt_def() {
1934+
if contains_adt_constructor(projection_predicate.ty, self_adt) {
1935+
return;
1936+
}
1937+
} else if contains_ty(projection_predicate.ty, self_ty) {
19301938
return;
19311939
}
19321940
}

clippy_utils/src/ty.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rustc_hir::{TyKind, Unsafety};
1111
use rustc_infer::infer::TyCtxtInferExt;
1212
use rustc_lint::LateContext;
1313
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
14-
use rustc_middle::ty::{self, IntTy, Ty, TypeFoldable, UintTy};
14+
use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TypeFoldable, UintTy};
1515
use rustc_span::sym;
1616
use rustc_span::symbol::Symbol;
1717
use rustc_span::DUMMY_SP;
@@ -43,6 +43,15 @@ pub fn contains_ty(ty: Ty<'_>, other_ty: Ty<'_>) -> bool {
4343
})
4444
}
4545

46+
/// Walks into `ty` and returns `true` if any inner type is an instance of the given adt
47+
/// constructor.
48+
pub fn contains_adt_constructor(ty: Ty<'_>, adt: &AdtDef) -> bool {
49+
ty.walk().any(|inner| match inner.unpack() {
50+
GenericArgKind::Type(inner_ty) => inner_ty.ty_adt_def() == Some(adt),
51+
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
52+
})
53+
}
54+
4655
/// Returns true if ty has `iter` or `iter_mut` methods
4756
pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option<Symbol> {
4857
// FIXME: instead of this hard-coded list, we should check if `<adt>::iter`

tests/ui/new_ret_no_self.rs

+10
Original file line numberDiff line numberDiff line change
@@ -340,3 +340,13 @@ mod issue5435 {
340340
}
341341
}
342342
}
343+
344+
// issue #1724
345+
struct RetOtherSelf<T>(T);
346+
struct RetOtherSelfWrapper<T>(T);
347+
348+
impl RetOtherSelf<T> {
349+
fn new(t: T) -> RetOtherSelf<RetOtherSelfWrapper<T>> {
350+
RetOtherSelf(RetOtherSelfWrapper(t))
351+
}
352+
}

0 commit comments

Comments
 (0)