Skip to content

Commit 9074da0

Browse files
committed
Auto merge of rust-lang#10359 - mladedav:dm/private/is-empty, r=llogiq
Include async functions in the len_without_is_empty fixes rust-lang#7232 Changes done to the functionality: Allowing different error types for the functions was disallowed. So the following was linted before but is not after this change ``` impl Foo { pub len(&self) -> Result<usize, Error1> { todo!(); } pub is_empty(&self) -> Result<bool, Error2> { todo!(); } } ``` --- changelog: Enhancement: [`len_without_is_empty`]: Now also detects `async` functions [rust-lang#10359](rust-lang/rust-clippy#10359) <!-- changelog_checked -->
2 parents a45f712 + 5369052 commit 9074da0

File tree

3 files changed

+202
-20
lines changed

3 files changed

+202
-20
lines changed

clippy_lints/src/len_zero.rs

+91-19
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ use rustc_ast::ast::LitKind;
66
use rustc_errors::Applicability;
77
use rustc_hir::def_id::DefIdSet;
88
use rustc_hir::{
9-
def_id::DefId, AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, ImplItem, ImplItemKind, ImplicitSelfKind, Item,
10-
ItemKind, Mutability, Node, TraitItemRef, TyKind,
9+
def::Res, def_id::DefId, lang_items::LangItem, AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, GenericArg,
10+
GenericBound, ImplItem, ImplItemKind, ImplicitSelfKind, Item, ItemKind, Mutability, Node, PathSegment, PrimTy,
11+
QPath, TraitItemRef, TyKind, TypeBindingKind,
1112
};
1213
use rustc_lint::{LateContext, LateLintPass};
1314
use rustc_middle::ty::{self, AssocKind, FnSig, Ty};
@@ -250,33 +251,98 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items
250251
}
251252

252253
#[derive(Debug, Clone, Copy)]
253-
enum LenOutput<'tcx> {
254+
enum LenOutput {
254255
Integral,
255256
Option(DefId),
256-
Result(DefId, Ty<'tcx>),
257+
Result(DefId),
257258
}
258-
fn parse_len_output<'tcx>(cx: &LateContext<'_>, sig: FnSig<'tcx>) -> Option<LenOutput<'tcx>> {
259+
260+
fn extract_future_output<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx PathSegment<'tcx>> {
261+
if let ty::Alias(_, alias_ty) = ty.kind() &&
262+
let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(alias_ty.def_id) &&
263+
let Item { kind: ItemKind::OpaqueTy(opaque), .. } = item &&
264+
opaque.bounds.len() == 1 &&
265+
let GenericBound::LangItemTrait(LangItem::Future, _, _, generic_args) = &opaque.bounds[0] &&
266+
generic_args.bindings.len() == 1 &&
267+
let TypeBindingKind::Equality {
268+
term: rustc_hir::Term::Ty(rustc_hir::Ty {kind: TyKind::Path(QPath::Resolved(_, path)), .. }),
269+
} = &generic_args.bindings[0].kind &&
270+
path.segments.len() == 1 {
271+
return Some(&path.segments[0]);
272+
}
273+
274+
None
275+
}
276+
277+
fn is_first_generic_integral<'tcx>(segment: &'tcx PathSegment<'tcx>) -> bool {
278+
if let Some(generic_args) = segment.args {
279+
if generic_args.args.is_empty() {
280+
return false;
281+
}
282+
let arg = &generic_args.args[0];
283+
if let GenericArg::Type(rustc_hir::Ty {
284+
kind: TyKind::Path(QPath::Resolved(_, path)),
285+
..
286+
}) = arg
287+
{
288+
let segments = &path.segments;
289+
let segment = &segments[0];
290+
let res = &segment.res;
291+
if matches!(res, Res::PrimTy(PrimTy::Uint(_))) || matches!(res, Res::PrimTy(PrimTy::Int(_))) {
292+
return true;
293+
}
294+
}
295+
}
296+
297+
false
298+
}
299+
300+
fn parse_len_output<'tcx>(cx: &LateContext<'tcx>, sig: FnSig<'tcx>) -> Option<LenOutput> {
301+
if let Some(segment) = extract_future_output(cx, sig.output()) {
302+
let res = segment.res;
303+
304+
if matches!(res, Res::PrimTy(PrimTy::Uint(_))) || matches!(res, Res::PrimTy(PrimTy::Int(_))) {
305+
return Some(LenOutput::Integral);
306+
}
307+
308+
if let Res::Def(_, def_id) = res {
309+
if cx.tcx.is_diagnostic_item(sym::Option, def_id) && is_first_generic_integral(segment) {
310+
return Some(LenOutput::Option(def_id));
311+
} else if cx.tcx.is_diagnostic_item(sym::Result, def_id) && is_first_generic_integral(segment) {
312+
return Some(LenOutput::Result(def_id));
313+
}
314+
}
315+
316+
return None;
317+
}
318+
259319
match *sig.output().kind() {
260320
ty::Int(_) | ty::Uint(_) => Some(LenOutput::Integral),
261321
ty::Adt(adt, subs) if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) => {
262322
subs.type_at(0).is_integral().then(|| LenOutput::Option(adt.did()))
263323
},
264-
ty::Adt(adt, subs) if cx.tcx.is_diagnostic_item(sym::Result, adt.did()) => subs
265-
.type_at(0)
266-
.is_integral()
267-
.then(|| LenOutput::Result(adt.did(), subs.type_at(1))),
324+
ty::Adt(adt, subs) if cx.tcx.is_diagnostic_item(sym::Result, adt.did()) => {
325+
subs.type_at(0).is_integral().then(|| LenOutput::Result(adt.did()))
326+
},
268327
_ => None,
269328
}
270329
}
271330

272-
impl<'tcx> LenOutput<'tcx> {
273-
fn matches_is_empty_output(self, ty: Ty<'tcx>) -> bool {
331+
impl LenOutput {
332+
fn matches_is_empty_output<'tcx>(self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
333+
if let Some(segment) = extract_future_output(cx, ty) {
334+
return match (self, segment.res) {
335+
(_, Res::PrimTy(PrimTy::Bool)) => true,
336+
(Self::Option(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Option, def_id) => true,
337+
(Self::Result(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Result, def_id) => true,
338+
_ => false,
339+
};
340+
}
341+
274342
match (self, ty.kind()) {
275343
(_, &ty::Bool) => true,
276344
(Self::Option(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(),
277-
(Self::Result(id, err_ty), &ty::Adt(adt, subs)) if id == adt.did() => {
278-
subs.type_at(0).is_bool() && subs.type_at(1) == err_ty
279-
},
345+
(Self::Result(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(),
280346
_ => false,
281347
}
282348
}
@@ -300,9 +366,14 @@ impl<'tcx> LenOutput<'tcx> {
300366
}
301367

302368
/// Checks if the given signature matches the expectations for `is_empty`
303-
fn check_is_empty_sig<'tcx>(sig: FnSig<'tcx>, self_kind: ImplicitSelfKind, len_output: LenOutput<'tcx>) -> bool {
369+
fn check_is_empty_sig<'tcx>(
370+
cx: &LateContext<'tcx>,
371+
sig: FnSig<'tcx>,
372+
self_kind: ImplicitSelfKind,
373+
len_output: LenOutput,
374+
) -> bool {
304375
match &**sig.inputs_and_output {
305-
[arg, res] if len_output.matches_is_empty_output(*res) => {
376+
[arg, res] if len_output.matches_is_empty_output(cx, *res) => {
306377
matches!(
307378
(arg.kind(), self_kind),
308379
(ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::ImmRef)
@@ -314,11 +385,11 @@ fn check_is_empty_sig<'tcx>(sig: FnSig<'tcx>, self_kind: ImplicitSelfKind, len_o
314385
}
315386

316387
/// Checks if the given type has an `is_empty` method with the appropriate signature.
317-
fn check_for_is_empty<'tcx>(
318-
cx: &LateContext<'tcx>,
388+
fn check_for_is_empty(
389+
cx: &LateContext<'_>,
319390
span: Span,
320391
self_kind: ImplicitSelfKind,
321-
output: LenOutput<'tcx>,
392+
output: LenOutput,
322393
impl_ty: DefId,
323394
item_name: Symbol,
324395
item_kind: &str,
@@ -351,6 +422,7 @@ fn check_for_is_empty<'tcx>(
351422
Some(is_empty)
352423
if !(is_empty.fn_has_self_parameter
353424
&& check_is_empty_sig(
425+
cx,
354426
cx.tcx.fn_sig(is_empty.def_id).subst_identity().skip_binder(),
355427
self_kind,
356428
output,

tests/ui/len_without_is_empty.rs

+92
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,87 @@ impl AsyncLen {
282282
}
283283
}
284284

285+
// issue #7232
286+
pub struct AsyncLenWithoutIsEmpty;
287+
impl AsyncLenWithoutIsEmpty {
288+
pub async fn async_task(&self) -> bool {
289+
true
290+
}
291+
292+
pub async fn len(&self) -> usize {
293+
usize::from(!self.async_task().await)
294+
}
295+
}
296+
297+
// issue #7232
298+
pub struct AsyncOptionLenWithoutIsEmpty;
299+
impl AsyncOptionLenWithoutIsEmpty {
300+
async fn async_task(&self) -> bool {
301+
true
302+
}
303+
304+
pub async fn len(&self) -> Option<usize> {
305+
None
306+
}
307+
}
308+
309+
// issue #7232
310+
pub struct AsyncOptionLenNonIntegral;
311+
impl AsyncOptionLenNonIntegral {
312+
// don't lint
313+
pub async fn len(&self) -> Option<String> {
314+
None
315+
}
316+
}
317+
318+
// issue #7232
319+
pub struct AsyncResultLenWithoutIsEmpty;
320+
impl AsyncResultLenWithoutIsEmpty {
321+
async fn async_task(&self) -> bool {
322+
true
323+
}
324+
325+
pub async fn len(&self) -> Result<usize, ()> {
326+
Err(())
327+
}
328+
}
329+
330+
// issue #7232
331+
pub struct AsyncOptionLen;
332+
impl AsyncOptionLen {
333+
async fn async_task(&self) -> bool {
334+
true
335+
}
336+
337+
pub async fn len(&self) -> Result<usize, ()> {
338+
Err(())
339+
}
340+
341+
pub async fn is_empty(&self) -> bool {
342+
true
343+
}
344+
}
345+
346+
pub struct AsyncLenSyncIsEmpty;
347+
impl AsyncLenSyncIsEmpty {
348+
pub async fn len(&self) -> u32 {
349+
0
350+
}
351+
352+
pub fn is_empty(&self) -> bool {
353+
true
354+
}
355+
}
356+
357+
// issue #9520
358+
pub struct NonStandardLen;
359+
impl NonStandardLen {
360+
// don't lint
361+
pub fn len(&self, something: usize) -> usize {
362+
something
363+
}
364+
}
365+
285366
// issue #9520
286367
pub struct NonStandardLenAndIsEmptySignature;
287368
impl NonStandardLenAndIsEmptySignature {
@@ -328,4 +409,15 @@ impl NonStandardSignatureWithGenerics {
328409
}
329410
}
330411

412+
pub struct DifferingErrors;
413+
impl DifferingErrors {
414+
pub fn len(&self) -> Result<usize, u8> {
415+
Ok(0)
416+
}
417+
418+
pub fn is_empty(&self) -> Result<bool, u16> {
419+
Ok(true)
420+
}
421+
}
422+
331423
fn main() {}

tests/ui/len_without_is_empty.stderr

+19-1
Original file line numberDiff line numberDiff line change
@@ -119,5 +119,23 @@ LL | pub fn len(&self) -> Result<usize, ()> {
119119
|
120120
= help: use a custom `Error` type instead
121121

122-
error: aborting due to 12 previous errors
122+
error: struct `AsyncLenWithoutIsEmpty` has a public `len` method, but no `is_empty` method
123+
--> $DIR/len_without_is_empty.rs:292:5
124+
|
125+
LL | pub async fn len(&self) -> usize {
126+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
127+
128+
error: struct `AsyncOptionLenWithoutIsEmpty` has a public `len` method, but no `is_empty` method
129+
--> $DIR/len_without_is_empty.rs:304:5
130+
|
131+
LL | pub async fn len(&self) -> Option<usize> {
132+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
133+
134+
error: struct `AsyncResultLenWithoutIsEmpty` has a public `len` method, but no `is_empty` method
135+
--> $DIR/len_without_is_empty.rs:325:5
136+
|
137+
LL | pub async fn len(&self) -> Result<usize, ()> {
138+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
139+
140+
error: aborting due to 15 previous errors
123141

0 commit comments

Comments
 (0)