Skip to content

fixes issue 17162 false positive for never type impls #17163

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.

Sign up for GitHub

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

Open
durationextender wants to merge 1 commit into
base: master
Choose a base branch
from
+42 −11
Open

fixes issue 17162 false positive for never type impls #17163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 14 additions & 6 deletions clippy_lints/src/unused_async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,21 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
visitor.visit_nested_body(body_id);

if !visitor.found_await
&& let Some(builtin_crate) = clippy_utils::std_or_core(cx)
&& let Some(inner) = unpack_async_fn_body(cx, body)
// Find the tail expression contained in the async fn (if any),
// which will be wrapped in std::future::ready.
&& let ExprKind::Block(block, _) = inner.kind
&& let Some(tail_expr) = block.expr
&& let Some(builtin_crate) = clippy_utils::std_or_core(cx)
&& let Some(inner) = unpack_async_fn_body(cx, body)
// Find the tail expression contained in the async fn (if any),
// which will be wrapped in std::future::ready.
&& let ExprKind::Block(block, _) = inner.kind
&& let Some(tail_expr) = block.expr
Comment on lines +260 to +265

@Jarcho Jarcho Jun 6, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These shouldn't be changed.

View changes since the review

@durationextender durationextender Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk why those got changed tbh maybe because of cargo dev fmt?

{
let parent = cx.tcx.hir_get_parent_item(impl_item.hir_id()).def_id;
let item = cx.tcx.hir_expect_item(parent);
let self_ty = cx.tcx.type_of(item.owner_id).instantiate_identity().skip_norm_wip();
//check if self.ty is never type ! if it is, then we don't want to lint because the function can
// never be called and thus doesn't need to be async
if self_ty.is_never() {
return;
}
Comment on lines +267 to +274

@Jarcho Jarcho Jun 6, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual important thing to check is if any of the input types are uninhabited. You can use Ty::is_inhabited_from.

View changes since the review

@ada4a ada4a Jun 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would mean that this check should be applied to freestanding functions as well, right? So it would probably make sense to extract it to a separate function, and call that both here and in check_fn

@durationextender durationextender Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay i will change the self_ty.is_never() to use is_inhabited_from instead.

@durationextender durationextender Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you perhaps explain that func? the is_inhabited_from? because there is only 1 reference of it in the whole codebase and that ref is very vague of what is expects imo

@Jarcho Jarcho Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything you need is in LateContext. You can get the DefId of the innermost enclosing item from cx.enclosing_body.unwrap().hir_id.owner.def_id.to_def_id() which can be used directly.

The function returns how the item passed views a type's inhabitedness. A struct containing a private uninhabited field can only be seen as uninhabited from items that can access that field.

span_lint_and_then(
cx,
UNUSED_ASYNC_TRAIT_IMPL,
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/unused_async_trait_impl.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::unused_async_trait_impl)]
#![feature(never_type)]

trait HasAsyncMethod {
async fn do_something() -> u32;
Expand Down Expand Up @@ -107,3 +108,14 @@ mod macros {
}
}
}


mod issue17162 {
// Should not lint when async trait impl is for `!` (never type)

impl crate::HasAsyncMethod for ! {
async fn do_something() -> u32 {
unreachable!()
}
}
}
11 changes: 11 additions & 0 deletions tests/ui/unused_async_trait_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::unused_async_trait_impl)]
#![feature(never_type)]

trait HasAsyncMethod {
async fn do_something() -> u32;
Expand Down Expand Up @@ -107,3 +108,13 @@ mod macros {
}
}
}

mod issue17162 {
// Should not lint when async trait impl is for `!` (never type)

impl crate::HasAsyncMethod for ! {
async fn do_something() -> u32 {
unreachable!()
}
}
}
10 changes: 5 additions & 5 deletions tests/ui/unused_async_trait_impl.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: unused `async` for async trait impl function with no `.await` statements
--> tests/ui/unused_async_trait_impl.rs:16:9
--> tests/ui/unused_async_trait_impl.rs:17:9
|
LL | / async fn do_something() -> u32 {
LL | |
Expand All @@ -18,7 +18,7 @@ LL ~ std::future::ready(1)
|

error: unused `async` for async trait impl function with no `.await` statements
--> tests/ui/unused_async_trait_impl.rs:40:9
--> tests/ui/unused_async_trait_impl.rs:41:9
|
LL | / async fn do_something() -> u32 {
LL | |
Expand All @@ -40,7 +40,7 @@ LL ~ std::future::ready(x)
|

error: unused `async` for async trait impl function with no `.await` statements
--> tests/ui/unused_async_trait_impl.rs:63:9
--> tests/ui/unused_async_trait_impl.rs:64:9
|
LL | / async fn do_something() -> u32 {
LL | |
Expand All @@ -57,7 +57,7 @@ LL ~ std::future::ready(5)
|

error: unused `async` for async trait impl function with no `.await` statements
--> tests/ui/unused_async_trait_impl.rs:97:9
--> tests/ui/unused_async_trait_impl.rs:98:9
|
LL | / async fn do_something() -> vec_ty!(u32) {
LL | |
Expand All @@ -74,7 +74,7 @@ LL ~ std::future::ready(Vec::new())
|

error: unused `async` for async trait impl function with no `.await` statements
--> tests/ui/unused_async_trait_impl.rs:104:9
--> tests/ui/unused_async_trait_impl.rs:105:9
|
LL | / async fn do_something() -> Vec<u32> {
LL | |
Expand Down
Loading