Add a new lint UNCONSTRUCTABLE_PUB_STRUCT to detect unconstructable public structs

compiler/rustc_lint/src/lib.rs

@@ -295,6 +295,7 @@ fn register_builtins(store: &mut LintStore) {
         UNUSED_VISIBILITIES,
         UNUSED_ASSIGNMENTS,
         DEAD_CODE,
+        UNUSED_UNCONSTRUCTABLE_PUB_STRUCT,
         UNUSED_MUT,
         // FIXME: add this lint when it becomes stable,
         // see https://github.com/rust-lang/rust/issues/115585.

compiler/rustc_lint_defs/src/builtin.rs

@@ -150,6 +150,7 @@ declare_lint_pass! {
         UNUSED_MACRO_RULES,
         UNUSED_MUT,
         UNUSED_QUALIFICATIONS,
+        UNUSED_UNCONSTRUCTABLE_PUB_STRUCT,
         UNUSED_UNSAFE,
         UNUSED_VARIABLES,
         UNUSED_VISIBILITIES,
@@ -821,6 +822,48 @@ declare_lint! {
     crate_level_only
 }
 
+declare_lint! {
+    /// The `unused_unconstructable_pub_struct` lint detects public structs
+    /// with private fields that cannot be constructed or otherwise used through
+    /// the external API.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(unused_unconstructable_pub_struct)]
+    ///
+    /// pub struct Foo(i32);
+    /// # fn main() {}
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// This lint is emitted for a `pub` struct when:
+    ///
+    /// * It has private fields and no public constructor, so downstream crates
+    ///   cannot construct a value of the type.
+    /// * It is not used locally and does not appear in any reachable path from
+    ///   external APIs other than the public struct item itself.
+    /// * It has no field that marks intentional unconstructability, such as a
+    ///   field with unit or never type.
+    ///
+    /// Such structs may have been unused for a long time, but are now effectively dead code.
+    /// This lint helps find those items.
+    ///
+    /// To silence the warning for individual items, prefix the name with an
+    /// underscore such as `_Foo`.
+    ///
+    /// To indicate that the behavior is intentional, add a field with unit or
+    /// never type if the struct is only used at the type level.
+    ///
+    /// Otherwise, consider removing it if the struct is no longer in use.
+    pub UNUSED_UNCONSTRUCTABLE_PUB_STRUCT,
+    Deny,
+    "detects public structs with private fields that cannot be constructed or otherwise used through the external API"
+}
+
 declare_lint! {
     /// The `unused_attributes` lint detects attributes that were not used by
     /// the compiler.

compiler/rustc_middle/src/middle/dead_code.rs

@@ -11,13 +11,17 @@ pub struct DeadCodeLivenessSnapshot {
     pub ignored_derived_traits: LocalDefIdMap<FxIndexSet<DefId>>,
 }
 
-/// Dead-code liveness data for both analysis phases.
+/// Dead-code liveness data for different analysis phases.
 ///
 /// `pre_deferred_seeding` is computed before reachable-public and `#[allow(dead_code)]` seeding,
 /// and is used for lint `dead_code_pub_in_binary`.
+/// `pre_unconstructable_pubs` is computed before reachable public structs that cannot be
+/// directly constructed externally are seeded, and is used for lint
+/// `unused_unconstructable_pub_struct`.
 /// `final_result` is the final liveness snapshot used for lint `dead_code`.
 #[derive(Clone, Debug, StableHash)]
 pub struct DeadCodeLivenessSummary {
     pub pre_deferred_seeding: DeadCodeLivenessSnapshot,
+    pub pre_unconstructable_pubs: DeadCodeLivenessSnapshot,
     pub final_result: DeadCodeLivenessSnapshot,
 }

compiler/rustc_passes/src/dead.rs

@@ -10,7 +10,7 @@ use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
 use rustc_abi::FieldIdx;
 use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
 use rustc_errors::{ErrorGuaranteed, 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, ForeignItemId, ItemId, Node, PatKind, QPath, find_attr};
@@ -21,13 +21,15 @@ use rustc_middle::query::Providers;
 use rustc_middle::ty::{self, AssocTag, TyCtxt};
 use rustc_middle::{bug, span_bug};
 use rustc_session::config::CrateType;
-use rustc_session::lint::builtin::{DEAD_CODE, DEAD_CODE_PUB_IN_BINARY};
+use rustc_session::lint::builtin::{
+    DEAD_CODE, DEAD_CODE_PUB_IN_BINARY, UNUSED_UNCONSTRUCTABLE_PUB_STRUCT,
+};
 use rustc_session::lint::{self, Lint, LintExpectationId};
 use rustc_span::{Symbol, kw};
 
 use crate::errors::{
     ChangeFields, DeadCodePubInBinaryNote, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo,
-    UselessAssignment,
+    UnusedUnconstructablePubStructNote, UselessAssignment,
 };
 
 /// Any local definition that may call something in its body block should be explored. For example,
@@ -70,6 +72,42 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     }
 }
 
+fn struct_can_be_constructed_directly(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
+    // Skip language items
+    if tcx.as_lang_item(id.to_def_id()).is_some() {
+        return true;
+    }
+
+    let adt_def = tcx.adt_def(id.to_def_id());
+
+    // We only care about structs for now
+    if !adt_def.is_struct() {
+        return true;
+    }
+
+    // Such types often declared in Rust but constructed by FFI, so ignore
+    if adt_def.repr().c() || adt_def.repr().transparent() {
+        return true;
+    }
+
+    // Skip types contain fields of unit, never or PhantomData,
+    // 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).skip_binder();
+        field_type.is_unit() || field_type.is_never()
+    }) {
+        return true;
+    }
+
+    adt_def.all_fields().all(|field| field.vis.is_public())
+        || adt_def.all_fields().all(|field| tcx.type_of(field.did).skip_binder().is_phantom_data())
+}
+
+fn method_has_receiver(tcx: TyCtxt<'_>, id: LocalDefId) -> bool {
+    tcx.hir_fn_decl_by_hir_id(tcx.local_def_id_to_hir_id(id))
+        .is_some_and(|fn_decl| fn_decl.implicit_self().has_implicit_self())
+}
+
 /// 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)]
@@ -561,9 +599,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                 (self.tcx.local_parent(local_def_id), trait_item_id)
             }
             // impl items are live if the corresponding traits are live
-            DefKind::Impl { of_trait: true } => {
-                (local_def_id, self.tcx.impl_trait_id(local_def_id).as_local())
-            }
+            DefKind::Impl { of_trait } => (
+                local_def_id,
+                of_trait
+                    .then(|| self.tcx.impl_trait_id(local_def_id))
+                    .and_then(|did| did.as_local()),
+            ),
             _ => bug!(),
         };
 
@@ -921,12 +962,14 @@ fn maybe_record_as_seed<'tcx>(
 struct SeedWorklists {
     worklist: Vec<WorkItem>,
     deferred_seeds: Vec<WorkItem>,
+    deferred_unconstructable_pubs: Vec<WorkItem>,
     unsolved_items: Vec<LocalDefId>,
 }
 
 fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> SeedWorklists {
     let mut unsolved_items = Vec::new();
     let mut deferred_seeds = Vec::new();
+    let mut deferred_unconstructable_pubs = Vec::new();
     let mut worklist = Vec::new();
 
     if let Some((def_id, _)) = tcx.entry_fn(())
@@ -940,12 +983,47 @@ fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> SeedWorklists {
     }
 
     for (id, effective_vis) in tcx.effective_visibilities(()).iter() {
-        if effective_vis.is_public_at_level(Level::Reachable) {
-            deferred_seeds.push(WorkItem {
-                id: *id,
-                propagated: ComesFromAllowExpect::No,
-                own: ComesFromAllowExpect::No,
-            });
+        if !effective_vis.is_public_at_level(Level::Reachable) {
+            continue;
+        }
+
+        let work_item = WorkItem {
+            id: *id,
+            propagated: ComesFromAllowExpect::No,
+            own: ComesFromAllowExpect::No,
+        };
+
+        match tcx.def_kind(*id) {
+            DefKind::Struct if !struct_can_be_constructed_directly(tcx, *id) => {
+                deferred_unconstructable_pubs.push(work_item);
+            }
+            DefKind::Ctor(CtorOf::Struct, CtorKind::Fn)
+                if !struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)) =>
+            {
+                deferred_unconstructable_pubs.push(work_item);
+            }
+
+            DefKind::Impl { of_trait } => {
+                if !of_trait {
+                    unsolved_items.push(*id);
+                }
+
+                deferred_unconstructable_pubs.push(work_item);
+            }
+            DefKind::AssocFn
+                if let DefKind::Impl { of_trait } = tcx.def_kind(tcx.local_parent(*id))
+                    && method_has_receiver(tcx, *id) =>
+            {
+                if !of_trait {
+                    unsolved_items.push(*id);
+                }
+
+                deferred_unconstructable_pubs.push(work_item);
+            }
+
+            _ => {
+                deferred_seeds.push(work_item);
+            }
         }
     }
 
@@ -958,15 +1036,19 @@ fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> SeedWorklists {
         maybe_record_as_seed(tcx, id, &mut push_into_worklist, &mut unsolved_items);
     }
 
-    SeedWorklists { worklist, deferred_seeds, unsolved_items }
+    SeedWorklists { worklist, deferred_seeds, deferred_unconstructable_pubs, unsolved_items }
 }
 
 fn live_symbols_and_ignored_derived_traits(
     tcx: TyCtxt<'_>,
     (): (),
 ) -> Result<DeadCodeLivenessSummary, ErrorGuaranteed> {
-    let SeedWorklists { worklist, deferred_seeds, mut unsolved_items } =
-        create_and_seed_worklist(tcx);
+    let SeedWorklists {
+        worklist,
+        deferred_seeds,
+        deferred_unconstructable_pubs,
+        mut unsolved_items,
+    } = create_and_seed_worklist(tcx);
     let mut symbol_visitor = MarkSymbolVisitor {
         worklist,
         tcx,
@@ -989,8 +1071,17 @@ fn live_symbols_and_ignored_derived_traits(
     symbol_visitor.worklist.extend(deferred_seeds);
     mark_live_symbols_and_ignored_derived_traits(&mut symbol_visitor, &mut unsolved_items)?;
 
+    let pre_unconstructable_pubs = DeadCodeLivenessSnapshot {
+        live_symbols: symbol_visitor.live_symbols.clone(),
+        ignored_derived_traits: symbol_visitor.ignored_derived_traits.clone(),
+    };
+
+    symbol_visitor.worklist.extend(deferred_unconstructable_pubs);
+    mark_live_symbols_and_ignored_derived_traits(&mut symbol_visitor, &mut unsolved_items)?;
+
     Ok(DeadCodeLivenessSummary {
         pre_deferred_seeding,
+        pre_unconstructable_pubs,
         final_result: DeadCodeLivenessSnapshot {
             live_symbols: symbol_visitor.live_symbols,
             ignored_derived_traits: symbol_visitor.ignored_derived_traits,
@@ -1092,6 +1183,13 @@ impl<'tcx> DeadVisitor<'tcx> {
         self.target_lint.name.eq(DEAD_CODE_PUB_IN_BINARY.name).then_some(DeadCodePubInBinaryNote)
     }
 
+    fn unused_unconstructable_pub_struct_note(&self) -> Option<UnusedUnconstructablePubStructNote> {
+        self.target_lint
+            .name
+            .eq(UNUSED_UNCONSTRUCTABLE_PUB_STRUCT.name)
+            .then_some(UnusedUnconstructablePubStructNote)
+    }
+
     // # Panics
     // All `dead_codes` must have the same lint level, otherwise we will intentionally ICE.
     // This is because we emit a multi-spanned lint using the lint level of the `dead_codes`'s
@@ -1204,6 +1302,8 @@ impl<'tcx> DeadVisitor<'tcx> {
                     participle,
                     name_list,
                     dead_code_pub_in_binary_note: self.dead_code_pub_in_binary_note(),
+                    unused_unconstructable_pub_struct_note: self
+                        .unused_unconstructable_pub_struct_note(),
                     change_fields_suggestion: fields_suggestion,
                     parent_info,
                     ignored_derived_impls,
@@ -1241,6 +1341,8 @@ impl<'tcx> DeadVisitor<'tcx> {
                     participle,
                     name_list,
                     dead_code_pub_in_binary_note: self.dead_code_pub_in_binary_note(),
+                    unused_unconstructable_pub_struct_note: self
+                        .unused_unconstructable_pub_struct_note(),
                     parent_info,
                     ignored_derived_impls,
                     enum_variants_with_same_name,
@@ -1317,8 +1419,11 @@ impl<'tcx> DeadVisitor<'tcx> {
 }
 
 fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
-    let Ok(DeadCodeLivenessSummary { pre_deferred_seeding, final_result }) =
-        tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
+    let Ok(DeadCodeLivenessSummary {
+        pre_deferred_seeding,
+        pre_unconstructable_pubs,
+        final_result,
+    }) = tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
     else {
         return;
     };
@@ -1344,6 +1449,26 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
         );
     }
 
+    let is_unused_unconstructable_pub = |def_id| {
+        tcx.effective_visibilities(()).is_public_at_level(def_id, Level::Reachable)
+            && !pre_unconstructable_pubs.live_symbols.contains(&def_id)
+            && tcx.def_kind(def_id) == DefKind::Struct
+            && !struct_can_be_constructed_directly(tcx, def_id)
+    };
+    lint_dead_codes(
+        tcx,
+        UNUSED_UNCONSTRUCTABLE_PUB_STRUCT,
+        module,
+        &pre_unconstructable_pubs.live_symbols,
+        &pre_unconstructable_pubs.ignored_derived_traits,
+        module_items
+            .free_items()
+            .filter(|free_item| is_unused_unconstructable_pub(free_item.owner_id.def_id)),
+        module_items
+            .foreign_items()
+            .filter(|foreign_item| is_unused_unconstructable_pub(foreign_item.owner_id.def_id)),
+    );
+
     lint_dead_codes(
         tcx,
         DEAD_CODE,

compiler/rustc_passes/src/errors.rs

@@ -924,6 +924,8 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
         #[subdiagnostic]
         dead_code_pub_in_binary_note: Option<DeadCodePubInBinaryNote>,
         #[subdiagnostic]
+        unused_unconstructable_pub_struct_note: Option<UnusedUnconstructablePubStructNote>,
+        #[subdiagnostic]
         // only on DeadCodes since it's never a problem for tuple struct fields
         enum_variants_with_same_name: Vec<EnumVariantSameName<'tcx>>,
         #[subdiagnostic]
@@ -949,6 +951,8 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
         #[subdiagnostic]
         dead_code_pub_in_binary_note: Option<DeadCodePubInBinaryNote>,
         #[subdiagnostic]
+        unused_unconstructable_pub_struct_note: Option<UnusedUnconstructablePubStructNote>,
+        #[subdiagnostic]
         change_fields_suggestion: ChangeFields,
         #[subdiagnostic]
         parent_info: Option<ParentInfo<'tcx>>,
@@ -963,6 +967,12 @@ pub(crate) enum MultipleDeadCodes<'tcx> {
 )]
 pub(crate) struct DeadCodePubInBinaryNote;
 
+#[derive(Subdiagnostic)]
+#[note(
+    "this `pub` struct has private fields, no public constructor, and is not otherwise reachable through the external API, so consider providing a public constructor or removing it"
+)]
+pub(crate) struct UnusedUnconstructablePubStructNote;
+
 #[derive(Subdiagnostic)]
 #[note(
     "it is impossible to refer to the {$dead_descr} `{$dead_name}` because it is shadowed by this enum variant with the same name"