Skip to content

Commit e7d5080

Browse files
committed
Add disallowed_pub_return_types lint
This lint checks the return values of all `pub` functions, methods, and trait methods within a crate, emitting a warning if any proscribed types are returned. Configurable via `disallowed-pub-return-types` in `clippy.toml`. This is intended to prevent internal error types (e.g., `anyhow::Result`) from leaking into public APIs. changelog: [`disallowed_pub_return_types`]: New lint
1 parent f763854 commit e7d5080

11 files changed

Lines changed: 250 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6651,6 +6651,7 @@ Released 2018-09-13
66516651
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
66526652
[`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
66536653
[`disallowed_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names
6654+
[`disallowed_pub_return_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_pub_return_types
66546655
[`disallowed_script_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_script_idents
66556656
[`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type
66566657
[`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types
@@ -7480,6 +7481,7 @@ Released 2018-09-13
74807481
[`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros
74817482
[`disallowed-methods`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods
74827483
[`disallowed-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-names
7484+
[`disallowed-pub-return-types`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-pub-return-types
74837485
[`disallowed-types`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-types
74847486
[`doc-valid-idents`]: https://doc.rust-lang.org/clippy/lint_configuration.html#doc-valid-idents
74857487
[`enable-raw-pointer-heuristic-for-send`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enable-raw-pointer-heuristic-for-send

book/src/lint_configuration.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,23 @@ default configuration of Clippy. By default, any configuration will replace the
585585
* [`disallowed_names`](https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names)
586586

587587

588+
## `disallowed-pub-return-types`
589+
The list of disallowed return types for pub functions, written as fully qualified paths.
590+
591+
**Fields:**
592+
- `path` (required): the fully qualified path to the type that should be disallowed
593+
- `reason` (optional): explanation why this type is disallowed
594+
- `replacement` (optional): suggested alternative type
595+
- `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
596+
if the path doesn't exist, instead of emitting an error
597+
598+
**Default Value:** `[]`
599+
600+
---
601+
**Affected lints:**
602+
* [`disallowed_pub_return_types`](https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_pub_return_types)
603+
604+
588605
## `disallowed-types`
589606
The list of disallowed types, written as fully qualified paths.
590607

clippy_config/src/conf.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,17 @@ define_Conf! {
628628
/// default configuration of Clippy. By default, any configuration will replace the default value.
629629
#[lints(disallowed_names)]
630630
disallowed_names: Vec<String> = DEFAULT_DISALLOWED_NAMES.iter().map(ToString::to_string).collect(),
631+
/// The list of disallowed return types for pub functions, written as fully qualified paths.
632+
///
633+
/// **Fields:**
634+
/// - `path` (required): the fully qualified path to the type that should be disallowed
635+
/// - `reason` (optional): explanation why this type is disallowed
636+
/// - `replacement` (optional): suggested alternative type
637+
/// - `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
638+
/// if the path doesn't exist, instead of emitting an error
639+
#[disallowed_paths_allow_replacements = true]
640+
#[lints(disallowed_pub_return_types)]
641+
disallowed_pub_return_types: Vec<DisallowedPath> = Vec::new(),
631642
/// The list of disallowed types, written as fully qualified paths.
632643
///
633644
/// **Fields:**

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
109109
crate::disallowed_macros::DISALLOWED_MACROS_INFO,
110110
crate::disallowed_methods::DISALLOWED_METHODS_INFO,
111111
crate::disallowed_names::DISALLOWED_NAMES_INFO,
112+
crate::disallowed_pub_return_types::DISALLOWED_PUB_RETURN_TYPES_INFO,
112113
crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
113114
crate::disallowed_types::DISALLOWED_TYPES_INFO,
114115
crate::doc::DOC_BROKEN_LINK_INFO,
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
use clippy_config::Conf;
2+
use clippy_config::types::{DisallowedPath, create_disallowed_map};
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use clippy_utils::paths::PathNS;
5+
use rustc_data_structures::fx::FxHashMap;
6+
use rustc_hir::def::{DefKind, Res};
7+
use rustc_hir::def_id::DefIdMap;
8+
use rustc_hir::intravisit::{Visitor, VisitorExt, walk_ty};
9+
use rustc_hir::{AmbigArg, FnRetTy, ImplItem, ImplItemKind, Item, ItemKind, TraitItem, TraitItemKind, Ty, TyKind};
10+
use rustc_lint::{LateContext, LateLintPass};
11+
use rustc_middle::ty::TyCtxt;
12+
use rustc_session::impl_lint_pass;
13+
use rustc_span::Span;
14+
15+
declare_clippy_lint! {
16+
/// ### What it does
17+
/// Denies the configured types in clippy.toml from being returned by pub functions.
18+
///
19+
/// ### Why is this bad?
20+
/// Some types are undesirable in public APIs (e.g. `anyhow::Result`).
21+
///
22+
/// ### Example
23+
/// ```rust,ignore
24+
/// pub fn foo() -> anyhow::Result<()> {
25+
/// Ok(())
26+
/// }
27+
/// ```
28+
#[clippy::version = "1.80.0"]
29+
pub DISALLOWED_PUB_RETURN_TYPES,
30+
style,
31+
"use of disallowed types in pub function return types"
32+
}
33+
34+
impl_lint_pass!(DisallowedPubReturnTypes => [DISALLOWED_PUB_RETURN_TYPES]);
35+
36+
pub struct DisallowedPubReturnTypes {
37+
def_ids: DefIdMap<(&'static str, &'static DisallowedPath)>,
38+
prim_tys: FxHashMap<rustc_hir::PrimTy, (&'static str, &'static DisallowedPath)>,
39+
}
40+
41+
impl DisallowedPubReturnTypes {
42+
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
43+
let (def_ids, prim_tys) = create_disallowed_map(
44+
tcx,
45+
&conf.disallowed_pub_return_types,
46+
PathNS::Type,
47+
def_kind_predicate,
48+
"type",
49+
true,
50+
);
51+
Self { def_ids, prim_tys }
52+
}
53+
54+
fn check_res_emit(&self, cx: &LateContext<'_>, res: &Res, span: Span) {
55+
let (path, disallowed_path) = match res {
56+
Res::Def(_, did) if let Some(&x) = self.def_ids.get(did) => x,
57+
Res::PrimTy(prim) if let Some(&x) = self.prim_tys.get(prim) => x,
58+
_ => return,
59+
};
60+
span_lint_and_then(
61+
cx,
62+
DISALLOWED_PUB_RETURN_TYPES,
63+
span,
64+
format!("returning a disallowed type `{path}` in a public API"),
65+
disallowed_path.diag_amendment(span),
66+
);
67+
}
68+
}
69+
70+
pub fn def_kind_predicate(def_kind: DefKind) -> bool {
71+
matches!(
72+
def_kind,
73+
DefKind::Struct
74+
| DefKind::Union
75+
| DefKind::Enum
76+
| DefKind::Trait
77+
| DefKind::TyAlias
78+
| DefKind::ForeignTy
79+
| DefKind::AssocTy
80+
)
81+
}
82+
83+
struct TyVisitor<'a, 'tcx> {
84+
cx: &'a LateContext<'tcx>,
85+
lint: &'a DisallowedPubReturnTypes,
86+
}
87+
88+
impl<'tcx> Visitor<'tcx> for TyVisitor<'_, 'tcx> {
89+
fn visit_ty(&mut self, ty: &'tcx Ty<'tcx, AmbigArg>) {
90+
if let TyKind::Path(path) = &ty.kind {
91+
let res = self.cx.qpath_res(path, ty.hir_id);
92+
self.lint.check_res_emit(self.cx, &res, ty.span);
93+
}
94+
walk_ty(self, ty);
95+
}
96+
}
97+
98+
impl<'tcx> LateLintPass<'tcx> for DisallowedPubReturnTypes {
99+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
100+
if let ItemKind::Fn { ref sig, .. } = item.kind
101+
&& cx.effective_visibilities.is_exported(item.owner_id.def_id)
102+
&& let FnRetTy::Return(ty) = sig.decl.output
103+
{
104+
TyVisitor { cx, lint: self }.visit_ty_unambig(ty);
105+
}
106+
}
107+
108+
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
109+
if let ImplItemKind::Fn(ref sig, _) = item.kind
110+
&& clippy_utils::trait_ref_of_method(cx, item.owner_id).is_none()
111+
&& cx.effective_visibilities.is_exported(item.owner_id.def_id)
112+
&& let FnRetTy::Return(ty) = sig.decl.output
113+
{
114+
TyVisitor { cx, lint: self }.visit_ty_unambig(ty);
115+
}
116+
}
117+
118+
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) {
119+
if let TraitItemKind::Fn(ref sig, _) = item.kind
120+
&& cx.effective_visibilities.is_exported(item.owner_id.def_id)
121+
&& let FnRetTy::Return(ty) = sig.decl.output
122+
{
123+
TyVisitor { cx, lint: self }.visit_ty_unambig(ty);
124+
}
125+
}
126+
}

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ mod disallowed_fields;
106106
mod disallowed_macros;
107107
mod disallowed_methods;
108108
mod disallowed_names;
109+
mod disallowed_pub_return_types;
109110
mod disallowed_script_idents;
110111
mod disallowed_types;
111112
mod doc;
@@ -717,6 +718,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
717718
Box::new(move |_| Box::new(if_then_some_else_none::IfThenSomeElseNone::new(conf))),
718719
Box::new(|_| Box::new(bool_assert_comparison::BoolAssertComparison)),
719720
Box::new(|_| Box::<unused_async::UnusedAsync>::default()),
721+
Box::new(move |tcx| Box::new(disallowed_pub_return_types::DisallowedPubReturnTypes::new(tcx, conf))),
720722
Box::new(move |tcx| Box::new(disallowed_types::DisallowedTypes::new(tcx, conf))),
721723
Box::new(move |tcx| Box::new(missing_enforced_import_rename::ImportRename::new(tcx, conf))),
722724
Box::new(move |_| Box::new(strlen_on_c_strings::StrlenOnCStrings::new(conf))),

clippy_utils/src/paths.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ pub fn find_crates(tcx: TyCtxt<'_>, name: Symbol) -> &'static [DefId] {
202202
let map = BY_NAME.get_or_init(|| {
203203
let mut map = FxHashMap::default();
204204
map.insert(tcx.crate_name(LOCAL_CRATE), vec![LOCAL_CRATE.as_def_id()]);
205+
map.insert(rustc_span::symbol::kw::Crate, vec![LOCAL_CRATE.as_def_id()]);
205206
for &num in tcx.crates(()) {
206207
map.entry(tcx.crate_name(num)).or_default().push(num.as_def_id());
207208
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
disallowed-pub-return-types = [
2+
"std::result::Result",
3+
"std::option::Option",
4+
"crate::InputOnly",
5+
]
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
#![warn(clippy::disallowed_pub_return_types)]
2+
#![allow(clippy::result_unit_err)]
3+
4+
pub fn bad_pub_fn() -> Result<(), ()> {
5+
//~^ disallowed_pub_return_types
6+
Ok(())
7+
}
8+
9+
fn private_fn() -> Result<(), ()> {
10+
// should not trigger
11+
Ok(())
12+
}
13+
14+
pub struct Struct;
15+
16+
impl Struct {
17+
pub fn bad_method(&self) -> Option<i32> {
18+
//~^ disallowed_pub_return_types
19+
Some(42)
20+
}
21+
22+
fn private_method(&self) -> Option<i32> {
23+
Some(42)
24+
}
25+
}
26+
27+
pub trait Trait {
28+
fn bad_trait_method() -> Result<(), ()>;
29+
//~^ disallowed_pub_return_types
30+
}
31+
32+
pub enum NestedStruct {
33+
One(Result<(), ()>),
34+
Two(Option<i32>),
35+
Three(String),
36+
}
37+
38+
pub fn get_nested_struct() -> NestedStruct {
39+
// Should not trigger. Wrapping the type is fine
40+
NestedStruct::One(Ok(()))
41+
}
42+
43+
pub enum InputOnly {
44+
Pretty,
45+
Raw,
46+
}
47+
48+
pub fn get_input_type() -> crate::InputOnly {
49+
//~^ disallowed_pub_return_types
50+
InputOnly::Pretty
51+
}
52+
53+
fn main() {}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
error: returning a disallowed type `std::result::Result` in a public API
2+
--> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:4:24
3+
|
4+
LL | pub fn bad_pub_fn() -> Result<(), ()> {
5+
| ^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::disallowed-pub-return-types` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::disallowed_pub_return_types)]`
9+
10+
error: returning a disallowed type `std::option::Option` in a public API
11+
--> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:17:33
12+
|
13+
LL | pub fn bad_method(&self) -> Option<i32> {
14+
| ^^^^^^^^^^^
15+
16+
error: returning a disallowed type `std::result::Result` in a public API
17+
--> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:28:30
18+
|
19+
LL | fn bad_trait_method() -> Result<(), ()>;
20+
| ^^^^^^^^^^^^^^
21+
22+
error: returning a disallowed type `crate::InputOnly` in a public API
23+
--> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:48:28
24+
|
25+
LL | pub fn get_input_type() -> crate::InputOnly {
26+
| ^^^^^^^^^^^^^^^^
27+
28+
error: aborting due to 4 previous errors
29+

0 commit comments

Comments
 (0)