Implement lint unconstructible_pub_struct

Author: mu001999

compiler/rustc_lint_defs/src/builtin.rs

@@ -110,6 +110,7 @@ declare_lint_pass! {
         TYVAR_BEHIND_RAW_POINTER,
         UNCONDITIONAL_PANIC,
         UNCONDITIONAL_RECURSION,
+        UNCONSTRUCTIBLE_PUB_STRUCT,
         UNCOVERED_PARAM_IN_PROJECTION,
         UNEXPECTED_CFGS,
         UNFULFILLED_LINT_EXPECTATIONS,
@@ -755,6 +756,38 @@ declare_lint! {
     "detect unused, unexported items"
 }
 
+declare_lint! {
+    /// The `unconstructible_pub_struct` lint detects public structs that
+    /// are unused locally and cannot be constructed externally.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(unconstructible_pub_struct)]
+    ///
+    /// pub struct Foo(i32);
+    /// # fn main() {}
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Unconstructible pub structs may signal a mistake or unfinished code.
+    /// To silence the warning for individual items, prefix the name with an
+    /// underscore such as `_Foo`.
+    ///
+    /// To preserve this lint, add a field with unit or never types that
+    /// indicate that the behavior is intentional, or use `PhantomData` as
+    /// field types if the struct is only used at the type level to check
+    /// things like well-formedness.
+    ///
+    /// Otherwise, consider removing it if the struct is no longer in use.
+    pub UNCONSTRUCTIBLE_PUB_STRUCT,
+    Allow,
+    "detects pub structs that are unused locally and cannot be constructed externally"
+}
+
 declare_lint! {
     /// The `unused_attributes` lint detects attributes that were not used by
     /// the compiler.

compiler/rustc_middle/src/query/mod.rs

@@ -1205,6 +1205,7 @@ rustc_queries! {
     query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx (
         LocalDefIdSet,
         LocalDefIdMap<FxIndexSet<DefId>>,
+        LocalDefIdSet,
     ) {
         arena_cache
         desc { "finding live symbols in crate" }

compiler/rustc_passes/messages.ftl

@@ -590,6 +590,10 @@ passes_trait_impl_const_stable =
 passes_transparent_incompatible =
     transparent {$target} cannot have other repr hints
 
+passes_unconstructible_pub_struct =
+    pub struct `{$name}` is unconstructible externally and never constructed locally
+    .help = this struct may be unused locally and also externally, consider removing it
+
 passes_unexportable_adt_with_private_fields = ADT types with private fields are not exportable
     .note = `{$field_name}` is private
 

compiler/rustc_passes/src/dead.rs

@@ -9,7 +9,7 @@ use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
 use rustc_abi::FieldIdx;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::MultiSpan;
-use rustc_hir::def::{CtorOf, DefKind, Res};
+use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{self as hir, Node, PatKind, QPath};
@@ -18,12 +18,13 @@ use rustc_middle::middle::privacy::Level;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::{self, AssocTag, TyCtxt};
 use rustc_middle::{bug, span_bug};
-use rustc_session::lint::builtin::DEAD_CODE;
+use rustc_session::lint::builtin::{DEAD_CODE, UNCONSTRUCTIBLE_PUB_STRUCT};
 use rustc_session::lint::{self, LintExpectationId};
 use rustc_span::{Symbol, kw, sym};
 
 use crate::errors::{
-    ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment,
+    ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UnconstructiblePubStruct,
+    UselessAssignment,
 };
 
 /// Any local definition that may call something in its body block should be explored. For example,
@@ -66,6 +67,38 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     }
 }
 
+fn struct_can_be_constructed_directly(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
+    let adt_def = tcx.adt_def(id);
+
+    // Skip types contain fields of unit and never type,
+    // it's usually intentional to make the type not constructible
+    if adt_def.all_fields().any(|field| {
+        let field_type = tcx.type_of(field.did).instantiate_identity();
+        field_type.is_unit() || field_type.is_never()
+    }) {
+        return true;
+    }
+
+    return adt_def.all_fields().all(|field| {
+        let field_type = tcx.type_of(field.did).instantiate_identity();
+        // Skip fields of PhantomData,
+        // cause it's a common way to check things like well-formedness
+        if field_type.is_phantom_data() {
+            return true;
+        }
+
+        field.vis.is_public()
+    });
+}
+
+fn method_has_no_receiver(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
+    if let Some(fn_decl) = tcx.hir_fn_decl_by_hir_id(tcx.local_def_id_to_hir_id(id)) {
+        !fn_decl.implicit_self.has_implicit_self()
+    } else {
+        true
+    }
+}
+
 /// Determine if a work from the worklist is coming from a `#[allow]`
 /// or a `#[expect]` of `dead_code`
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
@@ -370,6 +403,28 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
         }
     }
 
+    fn mark_live_symbols_until_unsolved_items_fixed(
+        &mut self,
+        unsolved_items: &mut Vec<LocalDefId>,
+    ) {
+        self.mark_live_symbols();
+
+        // We have marked the primary seeds as live. We now need to process unsolved items from traits
+        // and trait impls: add them to the work list if the trait or the implemented type is live.
+        let mut items_to_check: Vec<_> = unsolved_items
+            .extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id))
+            .collect();
+
+        while !items_to_check.is_empty() {
+            self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
+            self.mark_live_symbols();
+
+            items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
+                self.check_impl_or_impl_item_live(local_def_id)
+            }));
+        }
+    }
+
     /// Automatically generated items marked with `rustc_trivial_field_reads`
     /// will be ignored for the purposes of dead code analysis (see PR #85200
     /// for discussion).
@@ -492,7 +547,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                     .and_then(|def_id| def_id.as_local()),
             ),
             // impl items are live if the corresponding traits are live
-            DefKind::Impl { of_trait: true } => (
+            DefKind::Impl { .. } => (
                 local_def_id,
                 self.tcx
                     .impl_trait_ref(local_def_id)
@@ -684,6 +739,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
     }
 }
 
+fn has_allow_unconstructible_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
+    let hir_id = tcx.local_def_id_to_hir_id(def_id);
+    let lint_level = tcx.lint_level_at_node(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id).level;
+    matches!(lint_level, lint::Allow | lint::Expect)
+}
+
 fn has_allow_dead_code_or_lang_attr(
     tcx: TyCtxt<'_>,
     def_id: LocalDefId,
@@ -743,7 +804,9 @@ fn maybe_record_as_seed<'tcx>(
     unsolved_items: &mut Vec<LocalDefId>,
 ) {
     let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id);
-    if let Some(comes_from_allow) = allow_dead_code {
+    if let Some(comes_from_allow) = allow_dead_code
+        && !tcx.effective_visibilities(()).is_reachable(owner_id.def_id)
+    {
         worklist.push((owner_id.def_id, comes_from_allow));
     }
 
@@ -807,6 +870,33 @@ fn create_and_seed_worklist(
             effective_vis
                 .is_public_at_level(Level::Reachable)
                 .then_some(id)
+                .filter(|id| {
+                    let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) {
+                        DefKind::Impl { .. } => (false, true),
+                        DefKind::AssocFn => (
+                            !matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. })
+                                || method_has_no_receiver(tcx, *id),
+                            true,
+                        ),
+                        DefKind::Struct => (
+                            has_allow_unconstructible_pub_struct(tcx, *id)
+                                || struct_can_be_constructed_directly(tcx, *id),
+                            false,
+                        ),
+                        DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => (
+                            has_allow_unconstructible_pub_struct(tcx, tcx.local_parent(*id))
+                                || struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)),
+                            false,
+                        ),
+                        _ => (true, false),
+                    };
+
+                    if !is_seed && is_impl_or_impl_item {
+                        unsolved_impl_item.push(*id);
+                    }
+
+                    is_seed
+                })
                 .map(|id| (id, ComesFromAllowExpect::No))
         })
         // Seed entry point
@@ -827,7 +917,7 @@ fn create_and_seed_worklist(
 fn live_symbols_and_ignored_derived_traits(
     tcx: TyCtxt<'_>,
     (): (),
-) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>) {
+) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>, LocalDefIdSet) {
     let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
     let mut symbol_visitor = MarkSymbolVisitor {
         worklist,
@@ -841,28 +931,29 @@ fn live_symbols_and_ignored_derived_traits(
         ignore_variant_stack: vec![],
         ignored_derived_traits: Default::default(),
     };
-    symbol_visitor.mark_live_symbols();
+    symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items);
 
-    // We have marked the primary seeds as live. We now need to process unsolved items from traits
-    // and trait impls: add them to the work list if the trait or the implemented type is live.
-    let mut items_to_check: Vec<_> = unsolved_items
-        .extract_if(.., |&mut local_def_id| {
-            symbol_visitor.check_impl_or_impl_item_live(local_def_id)
-        })
-        .collect();
+    let reachable_items =
+        tcx.effective_visibilities(()).iter().filter_map(|(&id, effective_vis)| {
+            effective_vis.is_public_at_level(Level::Reachable).then_some(id)
+        });
 
-    while !items_to_check.is_empty() {
-        symbol_visitor
-            .worklist
-            .extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
-        symbol_visitor.mark_live_symbols();
+    let mut unstructurable_pub_structs = LocalDefIdSet::default();
+    for id in reachable_items {
+        if symbol_visitor.live_symbols.contains(&id) {
+            continue;
+        }
+
+        if matches!(tcx.def_kind(id), DefKind::Struct) {
+            unstructurable_pub_structs.insert(id);
+        }
 
-        items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
-            symbol_visitor.check_impl_or_impl_item_live(local_def_id)
-        }));
+        symbol_visitor.worklist.push((id, ComesFromAllowExpect::No));
     }
 
-    (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
+    symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items);
+
+    (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits, unstructurable_pub_structs)
 }
 
 struct DeadItem {
@@ -1142,7 +1233,8 @@ impl<'tcx> DeadVisitor<'tcx> {
 }
 
 fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
-    let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(());
+    let (live_symbols, ignored_derived_traits, unstructurable_pub_structs) =
+        tcx.live_symbols_and_ignored_derived_traits(());
     let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits };
 
     let module_items = tcx.hir_module_items(module);
@@ -1229,6 +1321,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
     for foreign_item in module_items.foreign_items() {
         visitor.check_definition(foreign_item.owner_id.def_id);
     }
+
+    for item in module_items.free_items() {
+        let def_id = item.owner_id.def_id;
+
+        if !unstructurable_pub_structs.contains(&def_id) {
+            continue;
+        }
+
+        let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else {
+            continue;
+        };
+
+        if name.as_str().starts_with('_') {
+            continue;
+        }
+
+        let hir_id = tcx.local_def_id_to_hir_id(def_id);
+        let vis_span = tcx.hir_node(hir_id).expect_item().vis_span;
+        let diag = UnconstructiblePubStruct { name, vis_span };
+        tcx.emit_node_span_lint(UNCONSTRUCTIBLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag);
+    }
 }
 
 pub(crate) fn provide(providers: &mut Providers) {

compiler/rustc_passes/src/errors.rs

@@ -1268,6 +1268,14 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
     },
 }
 
+#[derive(LintDiagnostic)]
+#[diag(passes_unconstructible_pub_struct)]
+pub(crate) struct UnconstructiblePubStruct {
+    pub name: Symbol,
+    #[help]
+    pub vis_span: Span,
+}
+
 #[derive(Subdiagnostic)]
 #[note(passes_enum_variant_same_name)]
 pub(crate) struct EnumVariantSameName<'tcx> {