From 6c2bfa936ad16b9a7303af7c173ab5f5091f9ae8 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike.gerwitz@ryansg.com>
Date: Fri, 4 Aug 2023 00:20:56 -0400
Subject: [PATCH] tamer: asg::graph::Asg::try_map_obj: Make private

Only internal modules ought to be able to mutate objects.  This is important
now with the use of the `f::Map` trait, since traits don't allow for private
methods, which were previously the means of maintaining encapsulation.

DEV-13163
---
 tamer/src/asg/graph.rs        | 10 +++++++++-
 tamer/src/asg/graph/object.rs |  8 ++++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/tamer/src/asg/graph.rs b/tamer/src/asg/graph.rs
index c6023f7b..920dc9ee 100644
--- a/tamer/src/asg/graph.rs
+++ b/tamer/src/asg/graph.rs
@@ -287,6 +287,14 @@ impl Asg {
     ///     see the [`object` module documentation](object) for more
     ///     information and rationale on this behavior.
     ///
+    /// This method is intentionally private:
+    ///   while it is possible for external systems to receive immutable
+    ///   references to objects,
+    ///     mutation of those objects is intended to remain encapsulated.
+    /// This is especially important given the use of the [`Map`] trait by
+    ///   objects,
+    ///     since traits yield public APIs.
+    ///
     /// Panics
     /// ======
     /// This method chooses to simplify the API by choosing panics for
@@ -302,7 +310,7 @@ impl Asg {
     ///        representing a type mismatch between what the caller thinks
     ///        this object represents and what the object actually is.
     #[must_use = "returned ObjectIndex has a possibly-updated and more relevant span"]
-    pub(super) fn try_map_obj<O: ObjectKind, E>(
+    fn try_map_obj<O: ObjectKind, E>(
         &mut self,
         index: ObjectIndex<O>,
         f: impl FnOnce(O) -> Result<O, (O, E)>,
diff --git a/tamer/src/asg/graph/object.rs b/tamer/src/asg/graph/object.rs
index ee2e7be2..8506eff6 100644
--- a/tamer/src/asg/graph/object.rs
+++ b/tamer/src/asg/graph/object.rs
@@ -758,7 +758,7 @@ impl<O: ObjectKind> ObjectIndex<O> {
     ///
     /// If this operation is [`Infallible`],
     ///   see [`Self::map_obj`].
-    pub fn try_map_obj<E>(
+    fn try_map_obj<E>(
         self,
         asg: &mut Asg,
         f: impl FnOnce(O) -> Result<O, (O, E)>,
@@ -775,7 +775,7 @@ impl<O: ObjectKind> ObjectIndex<O> {
     ///
     /// If this operation is [`Infallible`],
     ///   see [`Self::map_obj_inner`].
-    pub fn try_map_obj_inner<T, E>(
+    fn try_map_obj_inner<T, E>(
         self,
         asg: &mut Asg,
         f: impl FnOnce(T) -> <O as TryMap<T>>::FnResult<E>,
@@ -792,7 +792,7 @@ impl<O: ObjectKind> ObjectIndex<O> {
     ///
     /// If this operation is _not_ [`Infallible`],
     ///   see [`Self::try_map_obj`].
-    pub fn map_obj(self, asg: &mut Asg, f: impl FnOnce(O) -> O) -> Self {
+    fn map_obj(self, asg: &mut Asg, f: impl FnOnce(O) -> O) -> Self {
         // This verbose notation (in place of e.g. `unwrap`) is intentional
         //   to emphasize why it's unreachable and to verify our assumptions
         //   at every point.
@@ -811,7 +811,7 @@ impl<O: ObjectKind> ObjectIndex<O> {
     ///
     /// If this operation is _not_ [`Infallible`],
     ///   see [`Self::try_map_obj_inner`].
-    pub fn map_obj_inner<T>(self, asg: &mut Asg, f: impl FnOnce(T) -> T) -> Self
+    fn map_obj_inner<T>(self, asg: &mut Asg, f: impl FnOnce(T) -> T) -> Self
     where
         O: Map<T, Target = O>,
     {