Implement sealed trait analysis and expose it in the adapter. (#343) (#…

…346)

src/adapter/mod.rs

@@ -134,7 +134,12 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
                 "AttributeMetaItem" => {
                     properties::resolve_attribute_meta_item_property(contexts, property_name)
                 }
-                "Trait" => properties::resolve_trait_property(contexts, property_name),
+                "Trait" => properties::resolve_trait_property(
+                    contexts,
+                    property_name,
+                    self.current_crate,
+                    self.previous_crate,
+                ),
                 "ImplementedTrait" => {
                     properties::resolve_implemented_trait_property(contexts, property_name)
                 }

src/adapter/properties.rs

@@ -7,9 +7,9 @@ use trustfall::{
     FieldValue,
 };
 
-use crate::attributes::Attribute;
+use crate::{attributes::Attribute, IndexedCrate};
 
-use super::vertex::Vertex;
+use super::{origin::Origin, vertex::Vertex};
 
 pub(super) fn resolve_crate_property<'a, V: AsVertex<Vertex<'a>> + 'a>(
     contexts: ContextIterator<'a, V>,
@@ -456,10 +456,23 @@ pub(super) fn resolve_raw_type_property<'a, V: AsVertex<Vertex<'a>> + 'a>(
 pub(super) fn resolve_trait_property<'a, V: AsVertex<Vertex<'a>> + 'a>(
     contexts: ContextIterator<'a, V>,
     property_name: &str,
+    current_crate: &'a IndexedCrate<'a>,
+    previous_crate: Option<&'a IndexedCrate<'a>>,
 ) -> ContextOutcomeIterator<'a, V, FieldValue> {
     match property_name {
         "unsafe" => resolve_property_with(contexts, field_property!(as_trait, is_unsafe)),
         "object_safe" => resolve_property_with(contexts, field_property!(as_trait, is_object_safe)),
+        "sealed" => resolve_property_with(contexts, move |vertex| {
+            let trait_item = vertex.as_item().expect("not an Item");
+            let origin = vertex.origin;
+
+            let indexed_crate = match origin {
+                Origin::CurrentCrate => current_crate,
+                Origin::PreviousCrate => previous_crate.expect("no previous crate provided"),
+            };
+
+            indexed_crate.is_trait_sealed(&trait_item.id).into()
+        }),
         _ => unreachable!("Trait property {property_name}"),
     }
 }

src/adapter/tests.rs

@@ -1,3 +1,8 @@
+// The Trustfall API requires the adapter to be passed in as an Arc.
+// Our adapter is not Send/Sync (it doesn't need it),
+// but there's currently nothing we can do about this lint.
+#![allow(clippy::arc_with_non_send_sync)]
+
 use std::collections::BTreeMap;
 use std::sync::Arc;
 
@@ -154,6 +159,123 @@ fn rustdoc_finds_supertrait() {
     );
 }
 
+#[test]
+fn rustdoc_sealed_traits() {
+    let path = "./localdata/test_data/sealed_traits/rustdoc.json";
+    let content = std::fs::read_to_string(path)
+        .with_context(|| format!("Could not load {path} file, did you forget to run ./scripts/regenerate_test_rustdocs.sh ?"))
+        .expect("failed to load rustdoc");
+
+    let crate_ = serde_json::from_str(&content).expect("failed to parse rustdoc");
+    let indexed_crate = IndexedCrate::new(&crate_);
+    let adapter = RustdocAdapter::new(&indexed_crate, None);
+
+    let query = r#"
+{
+    Crate {
+        item {
+            ... on Trait {
+                name @output
+                sealed @output
+            }
+        }
+    }
+}
+"#;
+
+    let variables: BTreeMap<&str, &str> = BTreeMap::default();
+
+    let schema =
+        Schema::parse(include_str!("../rustdoc_schema.graphql")).expect("schema failed to parse");
+
+    #[derive(Debug, PartialOrd, Ord, PartialEq, Eq, serde::Deserialize)]
+    struct Output {
+        name: String,
+        sealed: bool,
+    }
+
+    let mut results: Vec<_> =
+        trustfall::execute_query(&schema, adapter.into(), query, variables.clone())
+            .expect("failed to run query")
+            .map(|row| row.try_into_struct().expect("shape mismatch"))
+            .collect();
+    results.sort_unstable();
+
+    let mut expected_results = vec![
+        Output {
+            name: "Sealed".into(),
+            sealed: true,
+        },
+        Output {
+            name: "InternalMarker".into(),
+            sealed: true,
+        },
+        Output {
+            name: "DirectlyTraitSealed".into(),
+            sealed: true,
+        },
+        Output {
+            name: "TransitivelyTraitSealed".into(),
+            sealed: true,
+        },
+        Output {
+            name: "Unsealed".into(),
+            sealed: false,
+        },
+        Output {
+            name: "MethodSealed".into(),
+            sealed: true,
+        },
+        Output {
+            name: "TransitivelyMethodSealed".into(),
+            sealed: true,
+        },
+        Output {
+            name: "NotMethodSealedBecauseOfDefaultImpl".into(),
+            sealed: false,
+        },
+        Output {
+            name: "NotTransitivelySealed".into(),
+            sealed: false,
+        },
+        Output {
+            name: "GenericSealed".into(),
+            sealed: true,
+        },
+        Output {
+            name: "NotGenericSealedBecauseOfDefaultImpl".into(),
+            sealed: false,
+        },
+        Output {
+            name: "IteratorExt".into(),
+            sealed: false,
+        },
+        Output {
+            name: "Iterator".into(),
+            sealed: true,
+        },
+        Output {
+            name: "ShadowedSubIterator".into(),
+            sealed: true,
+        },
+        Output {
+            name: "Super".into(),
+            sealed: false,
+        },
+        Output {
+            name: "Marker".into(),
+            sealed: true,
+        },
+        Output {
+            name: "NotGenericSealedBecauseOfPubSupertrait".into(),
+            sealed: false,
+        },
+    ];
+    expected_results.sort_unstable();
+
+    similar_asserts::assert_eq!(expected_results, results,);
+}
+
 #[test]
 fn rustdoc_finds_consts() {
     let path = "./localdata/test_data/consts/rustdoc.json";

src/indexed_crate.rs

@@ -1,11 +1,12 @@
 use std::{
     borrow::Borrow,
+    cell::RefCell,
     collections::{BTreeSet, HashMap},
 };
 
 use rustdoc_types::{Crate, Id, Item};
 
-use crate::{adapter::supported_item_kind, visibility_tracker::VisibilityTracker};
+use crate::{adapter::supported_item_kind, sealed_trait, visibility_tracker::VisibilityTracker};
 
 /// The rustdoc for a crate, together with associated indexed data to speed up common operations.
 ///
@@ -25,6 +26,11 @@ pub struct IndexedCrate<'a> {
     /// index: impl owner + impl'd item name -> list of (impl itself, the named item))
     pub(crate) impl_index: Option<HashMap<ImplEntry<'a>, Vec<(&'a Item, &'a Item)>>>,
 
+    /// Caching whether a trait by the given `Id`` is known to be sealed, or known to be unsealed.
+    ///
+    /// If the `Id` isn't present in the cache, the trait's sealed status has not yet been computed.
+    sealed_trait_cache: RefCell<HashMap<&'a Id, bool>>,
+
     /// Trait items defined in external crates are not present in the `inner: &Crate` field,
     /// even if they are implemented by a type in that crate. This also includes
     /// Rust's built-in traits like `Debug, Send, Eq` etc.
@@ -49,6 +55,7 @@ impl<'a> IndexedCrate<'a> {
             manually_inlined_builtin_traits: create_manually_inlined_builtin_traits(crate_),
             imports_index: None,
             impl_index: None,
+            sealed_trait_cache: Default::default(),
         };
 
         let mut imports_index: HashMap<Path, Vec<(&Item, Modifiers)>> =
@@ -153,6 +160,63 @@ impl<'a> IndexedCrate<'a> {
             Default::default()
         }
     }
+
+    /// Return `true` if our analysis indicates the trait is sealed, and `false` otherwise.
+    ///
+    /// Our analysis is conservative: it has false-negatives but no false-positives.
+    /// If this method returns `true`, the trait is *definitely* sealed or else you've found a bug.
+    /// It may be possible to construct traits that *technically* are sealed for which our analysis
+    /// returns `false`.
+    ///
+    /// The goal of this method is to reflect author intent, not technicalities.
+    /// When Rustaceans seal traits on purpose, they do so with a limited number of techniques
+    /// that are well-defined and immediately recognizable to readers in the community:
+    /// - <https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/>
+    /// - <https://jack.wrenn.fyi/blog/private-trait-methods/>
+    ///
+    /// The analysis here looks for such techniques, which are always applied at the type signature
+    /// level. It does not inspect function bodies or do interprocedural analysis.
+    ///
+    /// ## Panics
+    ///
+    /// This method will panic if the provided `id` is not an item in this crate,
+    /// or does not correspond to a trait in this crate.
+    ///
+    /// ## Re-entrancy
+    ///
+    /// This method is re-entrant: calling it may cause additional calls to itself, inquiring about
+    /// the sealed-ness of a trait's supertraits.
+    ///
+    /// We rely on rustc to reject supertrait cycles in order to prevent infinite loops.
+    /// Here's a supertrait cycle that must be rejected by rustc:
+    /// ```compile_fail
+    /// pub trait First: Third {}
+    ///
+    /// pub trait Second: First {}
+    ///
+    /// pub trait Third: Second {}
+    /// ```
+    pub fn is_trait_sealed(&self, id: &'a Id) -> bool {
+        let cache_ref = self.sealed_trait_cache.borrow();
+        if let Some(cached) = cache_ref.get(id) {
+            return *cached;
+        }
+
+        // Explicitly drop the ref, because this method is re-entrant
+        // and may need to write in a subsequent re-entrant call.
+        drop(cache_ref);
+
+        let trait_item = &self.inner.index[id];
+        let decision = sealed_trait::is_trait_sealed(self, trait_item);
+
+        // Unfortunately there's no easy way to avoid the double-hashing here.
+        // This method is re-entrant (we may need to decide if a supertrait is sealed *first*),
+        // and the borrow-checker doesn't like holding the `entry()` API's mut borrow
+        // while resolving the answer in `.or_insert_with()`.
+        self.sealed_trait_cache.borrow_mut().insert(id, decision);
+
+        decision
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]

src/lib.rs

@@ -1,12 +1,12 @@
 mod adapter;
 mod attributes;
 mod indexed_crate;
+mod sealed_trait;
+mod visibility_tracker;
 
 #[cfg(test)]
 pub(crate) mod test_util;
 
-mod visibility_tracker;
-
 // Re-export the Crate type so we can deserialize it.
 pub use rustdoc_types::Crate;
 

src/rustdoc_schema.graphql

@@ -902,9 +902,24 @@ type Trait implements Item & Importable {
   visibility_limit: String!
 
   # own properties
+  """
+  Whether this trait is defined as `unsafe` and requires the `unsafe` keyword when implementing.
+  """
   unsafe: Boolean!
+
+  """
+  Whether this trait can be used as `dyn Trait`.
+  """
   object_safe: Boolean!
 
+  """
+  Whether downstream crates are prevented from implementing this trait themselves.
+
+  Required implementation items can be added to sealed traits without causing a breaking change,
+  since no implementations of that trait may exist in other crates.
+  """
+  sealed: Boolean!
+
   # edges from Item
   span: Span
   attribute: [Attribute!]