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

Author: orthros

CHANGELOG.md

@@ -6651,6 +6651,7 @@ Released 2018-09-13
 [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
 [`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
 [`disallowed_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names
+[`disallowed_pub_return_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_pub_return_types
 [`disallowed_script_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_script_idents
 [`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type
 [`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types

clippy_config/src/conf.rs

@@ -628,6 +628,17 @@ define_Conf! {
     /// default configuration of Clippy. By default, any configuration will replace the default value.
     #[lints(disallowed_names)]
     disallowed_names: Vec<String> = DEFAULT_DISALLOWED_NAMES.iter().map(ToString::to_string).collect(),
+    /// The list of disallowed return types for pub functions, written as fully qualified paths.
+    ///
+    /// **Fields:**
+    /// - `path` (required): the fully qualified path to the type that should be disallowed
+    /// - `reason` (optional): explanation why this type is disallowed
+    /// - `replacement` (optional): suggested alternative type
+    /// - `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
+    ///   if the path doesn't exist, instead of emitting an error
+    #[disallowed_paths_allow_replacements = true]
+    #[lints(disallowed_pub_return_types)]
+    disallowed_pub_return_types: Vec<DisallowedPath> = Vec::new(),
     /// The list of disallowed types, written as fully qualified paths.
     ///
     /// **Fields:**

clippy_lints/src/declared_lints.rs

@@ -109,6 +109,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::disallowed_macros::DISALLOWED_MACROS_INFO,
     crate::disallowed_methods::DISALLOWED_METHODS_INFO,
     crate::disallowed_names::DISALLOWED_NAMES_INFO,
+    crate::disallowed_pub_return_types::DISALLOWED_PUB_RETURN_TYPES_INFO,
     crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
     crate::disallowed_types::DISALLOWED_TYPES_INFO,
     crate::doc::DOC_BROKEN_LINK_INFO,

clippy_lints/src/disallowed_pub_return_types.rs

@@ -0,0 +1,130 @@
+use clippy_config::Conf;
+use clippy_config::types::{DisallowedPath, create_disallowed_map};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::paths::PathNS;
+use rustc_data_structures::fx::FxHashMap;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::def_id::DefIdMap;
+use rustc_hir::intravisit::{Visitor, VisitorExt, walk_ty};
+use rustc_hir::{AmbigArg, FnRetTy, ImplItem, ImplItemKind, Item, ItemKind, TraitItem, TraitItemKind, Ty, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::TyCtxt;
+use rustc_session::impl_lint_pass;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Denies the configured types in clippy.toml from being returned by pub functions.
+    ///
+    /// ### Why is this bad?
+    /// Some types are undesirable in public APIs (e.g. `anyhow::Result`).
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// pub fn foo() -> anyhow::Result<()> {
+    ///     Ok(())
+    /// }
+    /// ```
+    #[clippy::version = "1.80.0"]
+    pub DISALLOWED_PUB_RETURN_TYPES,
+    style,
+    "use of disallowed types in pub function return types"
+}
+
+impl_lint_pass!(DisallowedPubReturnTypes => [DISALLOWED_PUB_RETURN_TYPES]);
+
+pub struct DisallowedPubReturnTypes {
+    def_ids: DefIdMap<(&'static str, &'static DisallowedPath)>,
+    prim_tys: FxHashMap<rustc_hir::PrimTy, (&'static str, &'static DisallowedPath)>,
+}
+
+impl DisallowedPubReturnTypes {
+    pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
+        let (def_ids, prim_tys) = create_disallowed_map(
+            tcx,
+            &conf.disallowed_pub_return_types,
+            PathNS::Type,
+            def_kind_predicate,
+            "type",
+            true,
+        );
+        Self { def_ids, prim_tys }
+    }
+
+    fn check_res_emit(&self, cx: &LateContext<'_>, res: &Res, span: Span) {
+        let (path, disallowed_path) = match res {
+            Res::Def(_, did) if let Some(&x) = self.def_ids.get(did) => x,
+            Res::PrimTy(prim) if let Some(&x) = self.prim_tys.get(prim) => x,
+            _ => return,
+        };
+        span_lint_and_then(
+            cx,
+            DISALLOWED_PUB_RETURN_TYPES,
+            span,
+            format!("returning a disallowed type `{path}` in a public API"),
+            disallowed_path.diag_amendment(span),
+        );
+    }
+}
+
+pub fn def_kind_predicate(def_kind: DefKind) -> bool {
+    matches!(
+        def_kind,
+        DefKind::Struct
+            | DefKind::Union
+            | DefKind::Enum
+            | DefKind::Trait
+            | DefKind::TyAlias
+            | DefKind::ForeignTy
+            | DefKind::AssocTy
+    )
+}
+
+struct TyVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    lint: &'a DisallowedPubReturnTypes,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for TyVisitor<'a, 'tcx> {
+    fn visit_ty(&mut self, ty: &'tcx Ty<'tcx, AmbigArg>) {
+        if let TyKind::Path(path) = &ty.kind {
+            let res = self.cx.qpath_res(path, ty.hir_id);
+            self.lint.check_res_emit(self.cx, &res, ty.span);
+        }
+        walk_ty(self, ty);
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for DisallowedPubReturnTypes {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if let ItemKind::Fn { ref sig, .. } = item.kind {
+            if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
+                if let FnRetTy::Return(ty) = sig.decl.output {
+                    TyVisitor { cx, lint: self }.visit_ty_unambig(ty);
+                }
+            }
+        }
+    }
+
+    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
+        if let ImplItemKind::Fn(ref sig, _) = item.kind {
+            if clippy_utils::trait_ref_of_method(cx, item.owner_id).is_none() {
+                if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
+                    if let FnRetTy::Return(ty) = sig.decl.output {
+                        TyVisitor { cx, lint: self }.visit_ty_unambig(ty);
+                    }
+                }
+            }
+        }
+    }
+
+    fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) {
+        if let TraitItemKind::Fn(ref sig, _) = item.kind {
+            if cx.effective_visibilities.is_exported(item.owner_id.def_id) {
+                if let FnRetTy::Return(ty) = sig.decl.output {
+                    TyVisitor { cx, lint: self }.visit_ty_unambig(ty);
+                }
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -106,6 +106,7 @@ mod disallowed_fields;
 mod disallowed_macros;
 mod disallowed_methods;
 mod disallowed_names;
+mod disallowed_pub_return_types;
 mod disallowed_script_idents;
 mod disallowed_types;
 mod doc;
@@ -717,6 +718,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(move |_| Box::new(if_then_some_else_none::IfThenSomeElseNone::new(conf))),
         Box::new(|_| Box::new(bool_assert_comparison::BoolAssertComparison)),
         Box::new(|_| Box::<unused_async::UnusedAsync>::default()),
+        Box::new(move |tcx| Box::new(disallowed_pub_return_types::DisallowedPubReturnTypes::new(tcx, conf))),
         Box::new(move |tcx| Box::new(disallowed_types::DisallowedTypes::new(tcx, conf))),
         Box::new(move |tcx| Box::new(missing_enforced_import_rename::ImportRename::new(tcx, conf))),
         Box::new(move |_| Box::new(strlen_on_c_strings::StrlenOnCStrings::new(conf))),

tests/ui-toml/disallowed_pub_return_types/clippy.toml

@@ -0,0 +1,4 @@
+disallowed-pub-return-types = [
+    "std::result::Result",
+    "std::option::Option",
+]

tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs

@@ -0,0 +1,43 @@
+#![warn(clippy::disallowed_pub_return_types)]
+#![allow(clippy::result_unit_err)]
+
+pub fn bad_pub_fn() -> Result<(), ()> {
+    //~^ disallowed_pub_return_types
+    Ok(())
+}
+
+fn private_fn() -> Result<(), ()> {
+    // should not trigger
+    Ok(())
+}
+
+pub struct Struct;
+
+impl Struct {
+    pub fn bad_method(&self) -> Option<i32> {
+        //~^ disallowed_pub_return_types
+        Some(42)
+    }
+
+    fn private_method(&self) -> Option<i32> {
+        Some(42)
+    }
+}
+
+pub trait Trait {
+    fn bad_trait_method() -> Result<(), ()>;
+    //~^ disallowed_pub_return_types
+}
+
+pub enum NestedStruct {
+    One(Result<(), ()>),
+    Two(Option<i32>),
+    Three(String),
+}
+
+pub fn get_nested_struct() -> NestedStruct {
+    // Should not trigger. Wrapping the type is fine
+    NestedStruct::One(Ok(()))
+}
+
+fn main() {}

tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.stderr

@@ -0,0 +1,23 @@
+error: returning a disallowed type `std::result::Result` in a public API
+  --> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:4:24
+   |
+LL | pub fn bad_pub_fn() -> Result<(), ()> {
+   |                        ^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::disallowed-pub-return-types` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::disallowed_pub_return_types)]`
+
+error: returning a disallowed type `std::option::Option` in a public API
+  --> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:17:33
+   |
+LL |     pub fn bad_method(&self) -> Option<i32> {
+   |                                 ^^^^^^^^^^^
+
+error: returning a disallowed type `std::result::Result` in a public API
+  --> tests/ui-toml/disallowed_pub_return_types/disallowed_pub_return_types.rs:28:30
+   |
+LL |     fn bad_trait_method() -> Result<(), ()>;
+   |                              ^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -42,6 +42,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            disallowed-macros
            disallowed-methods
            disallowed-names
+           disallowed-pub-return-types
            disallowed-types
            doc-valid-idents
            enable-raw-pointer-heuristic-for-send
@@ -143,6 +144,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            disallowed-macros
            disallowed-methods
            disallowed-names
+           disallowed-pub-return-types
            disallowed-types
            doc-valid-idents
            enable-raw-pointer-heuristic-for-send
@@ -244,6 +246,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            disallowed-macros
            disallowed-methods
            disallowed-names
+           disallowed-pub-return-types
            disallowed-types
            doc-valid-idents
            enable-raw-pointer-heuristic-for-send