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

add io_other_error lint #14022

Merged
merged 1 commit into from
Feb 21, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5726,6 +5726,7 @@ Released 2018-09-13
[`invalid_utf8_in_unchecked`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_utf8_in_unchecked
[`inverted_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#inverted_saturating_sub
[`invisible_characters`]: https://rust-lang.github.io/rust-clippy/master/index.html#invisible_characters
[`io_other_error`]: https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error
[`is_digit_ascii_radix`]: https://rust-lang.github.io/rust-clippy/master/index.html#is_digit_ascii_radix
[`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
[`items_after_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_test_module
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`from_over_into`](https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into)
* [`if_then_some_else_none`](https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none)
* [`index_refutable_slice`](https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice)
* [`io_other_error`](https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error)
* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
* [`lines_filter_map_ok`](https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok)
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ define_Conf! {
from_over_into,
if_then_some_else_none,
index_refutable_slice,
io_other_error,
iter_kv_map,
legacy_numeric_constants,
lines_filter_map_ok,
Expand Down
5 changes: 2 additions & 3 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use crate::utils::{clippy_project_root, clippy_version};
use indoc::{formatdoc, writedoc};
use std::fmt;
use std::fmt::Write as _;
use std::fs::{self, OpenOptions};
use std::io::prelude::*;
use std::io::{self, ErrorKind};
use std::path::{Path, PathBuf};
use std::{fmt, io};

struct LintData<'a> {
pass: &'a str,
Expand All @@ -25,7 +24,7 @@ impl<T> Context for io::Result<T> {
Ok(t) => Ok(t),
Err(e) => {
let message = format!("{}: {e}", text.as_ref());
Err(io::Error::new(ErrorKind::Other, message))
Err(io::Error::other(message))
},
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::methods::INEFFICIENT_TO_STRING_INFO,
crate::methods::INSPECT_FOR_EACH_INFO,
crate::methods::INTO_ITER_ON_REF_INFO,
crate::methods::IO_OTHER_ERROR_INFO,
crate::methods::IS_DIGIT_ASCII_RADIX_INFO,
crate::methods::ITERATOR_STEP_BY_ZERO_INFO,
crate::methods::ITER_CLONED_COLLECT_INFO,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/manual_div_ceil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl_lint_pass!(ManualDivCeil => [MANUAL_DIV_CEIL]);

impl<'tcx> LateLintPass<'tcx> for ManualDivCeil {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
if !self.msrv.meets(msrvs::DIV_CEIL) {
if !self.msrv.meets(msrvs::MANUAL_DIV_CEIL) {
return;
}

Expand Down
37 changes: 37 additions & 0 deletions clippy_lints/src/methods/io_other_error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::msrvs::{IO_ERROR_OTHER, Msrv};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::LateContext;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, path: &Expr<'_>, args: &[Expr<'_>], msrv: &Msrv) {
if let [error_kind, error] = args
&& !expr.span.from_expansion()
&& !error_kind.span.from_expansion()
&& clippy_utils::is_expr_path_def_path(cx, path, &clippy_utils::paths::IO_ERROR_NEW)
&& clippy_utils::is_expr_path_def_path(
cx,
clippy_utils::expr_or_init(cx, error_kind),
&clippy_utils::paths::IO_ERRORKIND_OTHER,
)
&& let ExprKind::Path(QPath::TypeRelative(_, new_segment)) = path.kind
&& msrv.meets(IO_ERROR_OTHER)
{
span_lint_and_then(
cx,
super::IO_OTHER_ERROR,
expr.span,
"this can be `std::io::Error::other(_)`",
|diag| {
diag.multipart_suggestion_verbose(
"use `std::io::Error::other`",
vec![
(new_segment.ident.span, "other".to_owned()),
(error_kind.span.until(error.span), String::new()),
],
Applicability::MachineApplicable,
);
},
);
}
}
25 changes: 25 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ mod implicit_clone;
mod inefficient_to_string;
mod inspect_for_each;
mod into_iter_on_ref;
mod io_other_error;
mod is_digit_ascii_radix;
mod is_empty;
mod iter_cloned_collect;
Expand Down Expand Up @@ -4461,6 +4462,28 @@ declare_clippy_lint! {
"unnecessary `iter().any()` on slices that can be replaced with `contains()`"
}

declare_clippy_lint! {
/// This lint warns on calling `io::Error::new(..)` with a kind of
/// `io::ErrorKind::Other`.
///
/// ### Why is this bad?
/// Since Rust 1.74, there's the `io::Error::other(_)` shortcut.
///
/// ### Example
/// ```no_run
/// use std::io;
/// let _ = io::Error::new(io::ErrorKind::Other, "bad".to_string());
/// ```
/// Use instead:
/// ```no_run
/// let _ = std::io::Error::other("bad".to_string());
/// ```
#[clippy::version = "1.86.0"]
pub IO_OTHER_ERROR,
style,
"calling `std::io::Error::new(std::io::ErrorKind::Other, _)`"
}

#[expect(clippy::struct_excessive_bools)]
pub struct Methods {
avoid_breaking_exported_api: bool,
Expand Down Expand Up @@ -4637,6 +4660,7 @@ impl_lint_pass!(Methods => [
RETURN_AND_THEN,
UNBUFFERED_BYTES,
MANUAL_CONTAINS,
IO_OTHER_ERROR,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4666,6 +4690,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
unnecessary_fallible_conversions::check_function(cx, expr, func);
manual_c_str_literals::check(cx, expr, func, args, &self.msrv);
useless_nonzero_new_unchecked::check(cx, expr, func, args, &self.msrv);
io_other_error::check(cx, expr, func, args, &self.msrv);
},
ExprKind::MethodCall(method_call, receiver, args, _) => {
let method_span = method_call.ident.span;
Expand Down
4 changes: 2 additions & 2 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ msrv_aliases! {
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,75,0 { OPTION_AS_SLICE }
1,74,0 { REPR_RUST }
1,73,0 { DIV_CEIL }
1,74,0 { REPR_RUST, IO_ERROR_OTHER }
1,73,0 { MANUAL_DIV_CEIL }
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
1,68,0 { PATH_MAIN_SEPARATOR_STR }
Expand Down
2 changes: 2 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"]

// Paths in `core`/`alloc`/`std`. This should be avoided and cleaned up by adding diagnostic items.
pub const CHAR_IS_ASCII: [&str; 5] = ["core", "char", "methods", "<impl char>", "is_ascii"];
pub const IO_ERROR_NEW: [&str; 5] = ["std", "io", "error", "Error", "new"];
pub const IO_ERRORKIND_OTHER: [&str; 5] = ["std", "io", "error", "ErrorKind", "Other"];

// Paths in clippy itself
pub const MSRV: [&str; 3] = ["clippy_utils", "msrvs", "Msrv"];
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/format_args_unfixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![allow(unused)]
#![allow(clippy::assertions_on_constants, clippy::eq_op, clippy::uninlined_format_args)]

use std::io::{Error, ErrorKind, Write, stdout};
use std::io::{Error, Write, stdout};
use std::ops::Deref;
use std::panic::Location;

Expand All @@ -20,7 +20,7 @@ macro_rules! my_other_macro {
}

fn main() {
let error = Error::new(ErrorKind::Other, "bad thing");
let error = Error::other("bad thing");
let x = 'x';

println!("error: {}", format!("something failed at {}", Location::caller()));
Expand Down Expand Up @@ -115,7 +115,7 @@ macro_rules! my_println2_args {
}

fn test2() {
let error = Error::new(ErrorKind::Other, "bad thing");
let error = Error::other("bad thing");

// None of these should be linted without the config change
my_println2!(true, "error: {}", format!("something failed at {}", Location::caller()));
Expand Down Expand Up @@ -145,7 +145,7 @@ macro_rules! usr_println {
}

fn user_format() {
let error = Error::new(ErrorKind::Other, "bad thing");
let error = Error::other("bad thing");
let x = 'x';

usr_println!(true, "error: {}", format!("boom at {}", Location::caller()));
Expand Down
55 changes: 55 additions & 0 deletions tests/ui/io_other_error.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#![warn(clippy::io_other_error)]
use std::fmt;

#[derive(Debug)]
struct E;

impl std::error::Error for E {}
impl fmt::Display for E {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str("E")
}
}

macro_rules! o {
{} => { std::io::ErrorKind::Other };
}

macro_rules! e {
{ $kind:expr } => { std::io::Error::new($kind, E) };
}

fn main() {
let _err = std::io::Error::other(E);
//~^ ERROR: this can be `std::io::Error::other(_)`
let other = std::io::ErrorKind::Other;
let _err = std::io::Error::other(E);
//~^ ERROR: this can be `std::io::Error::other(_)`

// not other
let _err = std::io::Error::new(std::io::ErrorKind::TimedOut, E);

// from expansion
let _err = e!(other);
let _err = std::io::Error::new(o!(), E);
let _err = e!(o!());

paths::short();
under_msrv();
}

mod paths {
use std::io::{self, Error, ErrorKind};

pub fn short() {
let _err = Error::other(super::E);
//~^ ERROR: this can be `std::io::Error::other(_)`
let _err = io::Error::other(super::E);
//~^ ERROR: this can be `std::io::Error::other(_)`
}
}

#[clippy::msrv = "1.73"]
fn under_msrv() {
let _err = std::io::Error::new(std::io::ErrorKind::Other, E);
}
55 changes: 55 additions & 0 deletions tests/ui/io_other_error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#![warn(clippy::io_other_error)]
use std::fmt;

#[derive(Debug)]
struct E;

impl std::error::Error for E {}
impl fmt::Display for E {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str("E")
}
}

macro_rules! o {
{} => { std::io::ErrorKind::Other };
}

macro_rules! e {
{ $kind:expr } => { std::io::Error::new($kind, E) };
}

fn main() {
let _err = std::io::Error::new(std::io::ErrorKind::Other, E);
//~^ ERROR: this can be `std::io::Error::other(_)`
let other = std::io::ErrorKind::Other;
let _err = std::io::Error::new(other, E);
//~^ ERROR: this can be `std::io::Error::other(_)`

// not other
let _err = std::io::Error::new(std::io::ErrorKind::TimedOut, E);

// from expansion
let _err = e!(other);
let _err = std::io::Error::new(o!(), E);
let _err = e!(o!());

paths::short();
under_msrv();
}

mod paths {
use std::io::{self, Error, ErrorKind};

pub fn short() {
let _err = Error::new(ErrorKind::Other, super::E);
//~^ ERROR: this can be `std::io::Error::other(_)`
let _err = io::Error::new(io::ErrorKind::Other, super::E);
//~^ ERROR: this can be `std::io::Error::other(_)`
}
}

#[clippy::msrv = "1.73"]
fn under_msrv() {
let _err = std::io::Error::new(std::io::ErrorKind::Other, E);
}
52 changes: 52 additions & 0 deletions tests/ui/io_other_error.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
error: this can be `std::io::Error::other(_)`
--> tests/ui/io_other_error.rs:23:16
|
LL | let _err = std::io::Error::new(std::io::ErrorKind::Other, E);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::io-other-error` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::io_other_error)]`
help: use `std::io::Error::other`
|
LL - let _err = std::io::Error::new(std::io::ErrorKind::Other, E);
LL + let _err = std::io::Error::other(E);
|

error: this can be `std::io::Error::other(_)`
--> tests/ui/io_other_error.rs:26:16
|
LL | let _err = std::io::Error::new(other, E);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `std::io::Error::other`
|
LL - let _err = std::io::Error::new(other, E);
LL + let _err = std::io::Error::other(E);
|

error: this can be `std::io::Error::other(_)`
--> tests/ui/io_other_error.rs:45:20
|
LL | let _err = Error::new(ErrorKind::Other, super::E);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `std::io::Error::other`
|
LL - let _err = Error::new(ErrorKind::Other, super::E);
LL + let _err = Error::other(super::E);
|

error: this can be `std::io::Error::other(_)`
--> tests/ui/io_other_error.rs:47:20
|
LL | let _err = io::Error::new(io::ErrorKind::Other, super::E);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `std::io::Error::other`
|
LL - let _err = io::Error::new(io::ErrorKind::Other, super::E);
LL + let _err = io::Error::other(super::E);
|

error: aborting due to 4 previous errors