Remove the configuration option

Also no longer lints non-exported types now
This commit is contained in:
Catherine 2023-07-18 10:25:58 -05:00 committed by Catherine
parent f4b3bb19c1
commit 19b0e84187
10 changed files with 60 additions and 92 deletions

View File

@ -5465,5 +5465,4 @@ Released 2018-09-13
[`accept-comment-above-statement`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-statement
[`accept-comment-above-attributes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-attributes
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
[`allow-private-error`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-private-error
<!-- end autogenerated links to configuration documentation -->

View File

@ -730,13 +730,3 @@ Whether to allow `r#""#` when `r""` can be used
* [`unnecessary_raw_string_hashes`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_raw_string_hashes)
## `allow-private-error`
Whether to allow private types named `Error` that implement `Error`
**Default Value:** `true` (`bool`)
---
**Affected lints:**
* [`error_impl_error`](https://rust-lang.github.io/rust-clippy/master/index.html#error_impl_error)

View File

@ -1,11 +1,12 @@
use clippy_utils::diagnostics::{span_lint, span_lint_hir_and_then};
use clippy_utils::path_res;
use clippy_utils::ty::implements_trait;
use rustc_hir::def_id::DefId;
use rustc_hir::{Item, ItemKind, Node};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{Item, ItemKind};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_middle::ty::Visibility;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! {
@ -30,48 +31,42 @@ declare_clippy_lint! {
#[clippy::version = "1.72.0"]
pub ERROR_IMPL_ERROR,
restriction,
"types named `Error` that implement `Error`"
}
impl_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]);
#[derive(Clone, Copy)]
pub struct ErrorImplError {
pub allow_private_error: bool,
"exported types named `Error` that implement `Error`"
}
declare_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]);
impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let Self { allow_private_error } = *self;
let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error) else {
return;
};
if allow_private_error && !cx.effective_visibilities.is_exported(item.owner_id.def_id) {
return;
}
match item.kind {
ItemKind::TyAlias(ty, _) if implements_trait(cx, hir_ty_to_ty(cx.tcx, ty), error_def_id, &[])
&& item.ident.name == sym::Error =>
&& item.ident.name == sym::Error
&& is_visible_outside_module(cx, item.owner_id.def_id) =>
{
span_lint(
cx,
ERROR_IMPL_ERROR,
item.ident.span,
"type alias named `Error` that implements `Error`",
"exported type alias named `Error` that implements `Error`",
);
},
ItemKind::Impl(imp) if let Some(trait_def_id) = imp.of_trait.and_then(|t| t.trait_def_id())
&& error_def_id == trait_def_id
&& let Some(def_id) = path_res(cx, imp.self_ty).opt_def_id().and_then(DefId::as_local)
&& let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id)
&& let Node::Item(ty_item) = cx.tcx.hir().get(hir_id)
&& ty_item.ident.name == sym::Error =>
&& let Some(ident) = cx.tcx.opt_item_ident(def_id.to_def_id())
&& ident.name == sym::Error
&& is_visible_outside_module(cx, def_id) =>
{
span_lint_hir_and_then(
cx,
ERROR_IMPL_ERROR,
hir_id,
ty_item.ident.span,
"type named `Error` that implements `Error`",
ident.span,
"exported type named `Error` that implements `Error`",
|diag| {
diag.span_note(item.span, "`Error` was implemented here");
}
@ -81,3 +76,12 @@ impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
}
}
}
/// Do not lint private `Error`s, i.e., ones without any `pub` (minus `pub(self)` of course) and
/// which aren't reexported
fn is_visible_outside_module(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
!matches!(
cx.tcx.visibility(def_id),
Visibility::Restricted(mod_def_id) if cx.tcx.parent_module_from_def_id(def_id).to_def_id() == mod_def_id
)
}

View File

@ -551,10 +551,6 @@ define_Conf! {
///
/// Whether to allow `r#""#` when `r""` can be used
(allow_one_hash_in_raw_strings: bool = false),
/// Lint: ERROR_IMPL_ERROR.
///
/// Whether to allow private types named `Error` that implement `Error`
(allow_private_error: bool = true),
}
/// Search for the configuration file.

View File

@ -1 +0,0 @@
allow-private-error = true

View File

@ -1 +0,0 @@
allow-private-error = false

View File

@ -1,33 +0,0 @@
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:10:16
|
LL | pub struct Error;
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:18:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `-D clippy::error-impl-error` implied by `-D warnings`
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:35:15
|
LL | pub union Error {
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:52:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type alias named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:56:14
|
LL | pub type Error = std::fmt::Error;
| ^^^^^
error: aborting due to 3 previous errors

View File

@ -6,7 +6,6 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
allow-mixed-uninlined-format-args
allow-one-hash-in-raw-strings
allow-print-in-tests
allow-private-error
allow-private-module-inception
allow-unwrap-in-tests
allowed-idents-below-min-chars
@ -76,7 +75,6 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
allow-mixed-uninlined-format-args
allow-one-hash-in-raw-strings
allow-print-in-tests
allow-private-error
allow-private-module-inception
allow-unwrap-in-tests
allowed-idents-below-min-chars

View File

@ -1,6 +1,3 @@
//@revisions: allow_private disallow_private
//@[allow_private] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/error_impl_error/allow_private
//@[disallow_private] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/error_impl_error/disallow_private
#![allow(unused)]
#![warn(clippy::error_impl_error)]
#![no_main]
@ -20,7 +17,7 @@ pub mod a {
mod b {
#[derive(Debug)]
enum Error {}
pub(super) enum Error {}
impl std::fmt::Display for Error {
fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@ -58,7 +55,7 @@ pub mod d {
mod e {
#[derive(Debug)]
struct MyError;
pub(super) struct MyError;
impl std::fmt::Display for MyError {
fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@ -69,6 +66,25 @@ mod e {
impl std::error::Error for MyError {}
}
mod f {
type MyError = std::fmt::Error;
pub mod f {
pub type MyError = std::fmt::Error;
}
// Do not lint module-private types
mod g {
#[derive(Debug)]
enum Error {}
impl std::fmt::Display for Error {
fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
todo!()
}
}
impl std::error::Error for Error {}
}
mod h {
type Error = std::fmt::Error;
}

View File

@ -1,42 +1,42 @@
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:10:16
error: exported type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:7:16
|
LL | pub struct Error;
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:18:5
--> $DIR/error_impl_error.rs:15:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `-D clippy::error-impl-error` implied by `-D warnings`
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:23:10
error: exported type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:20:21
|
LL | enum Error {}
| ^^^^^
LL | pub(super) enum Error {}
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:31:5
--> $DIR/error_impl_error.rs:28:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:35:15
error: exported type named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:32:15
|
LL | pub union Error {
| ^^^^^
|
note: `Error` was implemented here
--> $DIR/error_impl_error.rs:52:5
--> $DIR/error_impl_error.rs:49:5
|
LL | impl std::error::Error for Error {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: type alias named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:56:14
error: exported type alias named `Error` that implements `Error`
--> $DIR/error_impl_error.rs:53:14
|
LL | pub type Error = std::fmt::Error;
| ^^^^^