Skip to content

Commit

Permalink
Do not suggest using Error in no_std before Rust 1.81 (#13834)
Browse files Browse the repository at this point in the history
changelog: [`result_unit_err`]: do not suggest using `Error` in `no_std`
mode before Rust 1.81

Fix #9767
  • Loading branch information
llogiq authored Dec 16, 2024
2 parents 8da8da8 + 4c0177c commit 968669b
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 13 deletions.
11 changes: 8 additions & 3 deletions clippy_lints/src/functions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod too_many_lines;

use clippy_config::Conf;
use clippy_utils::def_path_def_ids;
use clippy_utils::msrvs::Msrv;
use rustc_hir as hir;
use rustc_hir::intravisit;
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -455,6 +456,7 @@ pub struct Functions {
/// A set of resolved `def_id` of traits that are configured to allow
/// function params renaming.
trait_ids: DefIdSet,
msrv: Msrv,
}

impl Functions {
Expand All @@ -469,6 +471,7 @@ impl Functions {
.iter()
.flat_map(|p| def_path_def_ids(tcx, &p.split("::").collect::<Vec<_>>()))
.collect(),
msrv: conf.msrv.clone(),
}
}
}
Expand Down Expand Up @@ -518,12 +521,12 @@ impl<'tcx> LateLintPass<'tcx> for Functions {

fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
must_use::check_item(cx, item);
result::check_item(cx, item, self.large_error_threshold);
result::check_item(cx, item, self.large_error_threshold, &self.msrv);
}

fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
must_use::check_impl_item(cx, item);
result::check_impl_item(cx, item, self.large_error_threshold);
result::check_impl_item(cx, item, self.large_error_threshold, &self.msrv);
impl_trait_in_params::check_impl_item(cx, item);
renamed_function_params::check_impl_item(cx, item, &self.trait_ids);
}
Expand All @@ -532,8 +535,10 @@ impl<'tcx> LateLintPass<'tcx> for Functions {
too_many_arguments::check_trait_item(cx, item, self.too_many_arguments_threshold);
not_unsafe_ptr_arg_deref::check_trait_item(cx, item);
must_use::check_trait_item(cx, item);
result::check_trait_item(cx, item, self.large_error_threshold);
result::check_trait_item(cx, item, self.large_error_threshold, &self.msrv);
impl_trait_in_params::check_trait_item(cx, item, self.avoid_breaking_exported_api);
ref_option::check_trait_item(cx, item, self.avoid_breaking_exported_api);
}

extract_msrv_attr!(LateContext);
}
29 changes: 20 additions & 9 deletions clippy_lints/src/functions/result.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use clippy_utils::msrvs::{self, Msrv};
use rustc_errors::Diag;
use rustc_hir as hir;
use rustc_lint::{LateContext, LintContext};
Expand All @@ -6,8 +7,8 @@ use rustc_middle::ty::{self, Ty};
use rustc_span::{Span, sym};

use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use clippy_utils::trait_ref_of_method;
use clippy_utils::ty::{AdtVariantInfo, approx_ty_size, is_type_diagnostic_item};
use clippy_utils::{is_no_std_crate, trait_ref_of_method};

use super::{RESULT_LARGE_ERR, RESULT_UNIT_ERR};

Expand All @@ -34,46 +35,56 @@ fn result_err_ty<'tcx>(
}
}

pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::Item<'tcx>, large_err_threshold: u64) {
pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::Item<'tcx>, large_err_threshold: u64, msrv: &Msrv) {
if let hir::ItemKind::Fn(ref sig, _generics, _) = item.kind
&& let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.owner_id.def_id, item.span)
{
if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
check_result_unit_err(cx, err_ty, fn_header_span);
check_result_unit_err(cx, err_ty, fn_header_span, msrv);
}
check_result_large_err(cx, err_ty, hir_ty.span, large_err_threshold);
}
}

pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::ImplItem<'tcx>, large_err_threshold: u64) {
pub(super) fn check_impl_item<'tcx>(
cx: &LateContext<'tcx>,
item: &hir::ImplItem<'tcx>,
large_err_threshold: u64,
msrv: &Msrv,
) {
// Don't lint if method is a trait's implementation, we can't do anything about those
if let hir::ImplItemKind::Fn(ref sig, _) = item.kind
&& let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.owner_id.def_id, item.span)
&& trait_ref_of_method(cx, item.owner_id.def_id).is_none()
{
if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
check_result_unit_err(cx, err_ty, fn_header_span);
check_result_unit_err(cx, err_ty, fn_header_span, msrv);
}
check_result_large_err(cx, err_ty, hir_ty.span, large_err_threshold);
}
}

pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::TraitItem<'tcx>, large_err_threshold: u64) {
pub(super) fn check_trait_item<'tcx>(
cx: &LateContext<'tcx>,
item: &hir::TraitItem<'tcx>,
large_err_threshold: u64,
msrv: &Msrv,
) {
if let hir::TraitItemKind::Fn(ref sig, _) = item.kind {
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
if let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.owner_id.def_id, item.span) {
if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
check_result_unit_err(cx, err_ty, fn_header_span);
check_result_unit_err(cx, err_ty, fn_header_span, msrv);
}
check_result_large_err(cx, err_ty, hir_ty.span, large_err_threshold);
}
}
}

fn check_result_unit_err(cx: &LateContext<'_>, err_ty: Ty<'_>, fn_header_span: Span) {
if err_ty.is_unit() {
fn check_result_unit_err(cx: &LateContext<'_>, err_ty: Ty<'_>, fn_header_span: Span, msrv: &Msrv) {
if err_ty.is_unit() && (!is_no_std_crate(cx) || msrv.meets(msrvs::ERROR_IN_CORE)) {
span_lint_and_help(
cx,
RESULT_UNIT_ERR,
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ macro_rules! msrv_aliases {
msrv_aliases! {
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY }
1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
1,81,0 { LINT_REASONS_STABILIZATION }
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE }
1,80,0 { BOX_INTO_ITER }
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
Expand Down
26 changes: 26 additions & 0 deletions tests/ui/result_unit_error_no_std.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![feature(lang_items, start, libc)]
#![no_std]
#![warn(clippy::result_unit_err)]

#[clippy::msrv = "1.80"]
pub fn returns_unit_error_no_lint() -> Result<u32, ()> {
Err(())
}

#[clippy::msrv = "1.81"]
pub fn returns_unit_error_lint() -> Result<u32, ()> {
Err(())
}

#[start]
fn main(_argc: isize, _argv: *const *const u8) -> isize {
0
}

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
loop {}
}

#[lang = "eh_personality"]
extern "C" fn eh_personality() {}
12 changes: 12 additions & 0 deletions tests/ui/result_unit_error_no_std.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error: this returns a `Result<_, ()>`
--> tests/ui/result_unit_error_no_std.rs:11:1
|
LL | pub fn returns_unit_error_lint() -> Result<u32, ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: use a custom `Error` type instead
= note: `-D clippy::result-unit-err` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::result_unit_err)]`

error: aborting due to 1 previous error

0 comments on commit 968669b

Please sign in to comment.