[core] Even more aggressively deduplicate the results of visibility transforms. (#7295)

fix #7294
This changes the visibility mutators to use replacement functions
instead of mutations, making it so that if the visibility transform does
not actually modify a model, it will return the same input model
instance. Integrated with the caching system, this should produce a
minimally-copied tree. The only node that is guaranteed to be duplicated
is the root node, since we cannot deduplicate the template instance.

Author: witemple-msft

.chronus/changes/witemple-msft-visibility-mutator-replace-2025-4-8-15-28-29.md

@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/compiler"
+---
+
+Corrected visibility filtering logic to even more aggressively deduplicate the models it visits when the applied visibility transform does not actually remove any properties from a model.

packages/compiler/src/lib/visibility.ts

@@ -565,7 +565,7 @@ function createVisibilityFilterMutator(
     name: "VisibilityFilterProperty",
     ModelProperty: {
       filter: () => MutatorFlow.DoNotRecur,
-      mutate: (prop, clone, program) => {
+      replace: (prop, clone, program) => {
         let modified = false;
 
         // We need to create a copy of the decorators array to avoid modifying the original.
@@ -613,22 +613,19 @@ function createVisibilityFilterMutator(
           modified ||= clone.type !== prop.type;
         }
 
-        if (modified) {
-          return clone;
-        } else {
-          return prop;
-        }
+        return modified ? clone : prop;
       },
     },
   };
+
   const self: Mutator = {
     name: "VisibilityFilter",
     Union: {
       filter: () => MutatorFlow.DoNotRecur,
-      mutate: (union, clone, program) => {
+      replace: (union, clone, program) => {
         let modified = false;
         for (const [key, member] of union.variants) {
-          if (member.type.kind === "Model" || member.type.kind === "Union") {
+          if (isVisibilitySubject(member.type)) {
             const variant: UnionVariant = {
               ...member,
               type: cachedMutateSubgraph(program, self, member.type).type,
@@ -639,17 +636,14 @@ function createVisibilityFilterMutator(
           }
         }
 
-        if (modified) {
-          rename(clone, options.nameTemplate);
-          return clone;
-        } else {
-          return union;
-        }
+        rename(clone, options.nameTemplate);
+
+        return modified ? clone : union;
       },
     },
     Model: {
       filter: () => MutatorFlow.DoNotRecur,
-      mutate: (model, clone, program, realm) => {
+      replace: (model, clone, program, realm) => {
         let modified = false;
 
         if (model.indexer && isVisibilitySubject(model.indexer.value)) {
@@ -684,38 +678,44 @@ function createVisibilityFilterMutator(
           modified ||= clone.decorators.length !== model.decorators.length;
         }
 
-        if (modified) {
-          rename(clone, options.nameTemplate);
-          return clone;
-        } else {
-          return model;
-        }
+        rename(clone, options.nameTemplate);
+
+        return modified ? clone : model;
       },
     },
     ModelProperty: {
       filter: () => MutatorFlow.DoNotRecur,
-      mutate: (prop, clone, program) => {
+      replace: (prop, clone, program) => {
         if (isVisibilitySubject(prop.type)) {
           clone.type = cachedMutateSubgraph(program, self, prop.type).type;
         }
+
+        return clone.type !== prop.type ? clone : prop;
       },
     },
     UnionVariant: {
       filter: () => MutatorFlow.DoNotRecur,
-      mutate: (variant, clone, program) => {
+      replace: (variant, clone, program) => {
         if (isVisibilitySubject(variant.type)) {
           clone.type = cachedMutateSubgraph(program, self, variant.type).type;
         }
+
+        return clone.type !== variant.type ? clone : variant;
       },
     },
     Tuple: {
       filter: () => MutatorFlow.DoNotRecur,
-      mutate: (tuple, clone, program) => {
+      replace: (tuple, clone, program) => {
+        let modified = false;
         for (const [index, element] of tuple.values.entries()) {
           if (isVisibilitySubject(element)) {
             clone.values[index] = cachedMutateSubgraph(program, self, element).type;
+
+            modified ||= clone.values[index] !== element;
           }
         }
+
+        return modified ? clone : tuple;
       },
     },
   };

packages/compiler/test/visibility.test.ts

@@ -1109,6 +1109,34 @@ describe("compiler: visibility core", () => {
     ok(C.properties.has("c"));
   });
 
+  it("correctly caches and deduplicates instances that are not transformed", async () => {
+    const { example, B } = (await runner.compile(`
+      @test op example(): Read<A>;
+
+      model A {
+        b: B;
+      }
+      
+      @test
+      model B {
+        c: string;
+      }
+    `)) as { example: Operation; B: Model };
+
+    ok(example);
+    strictEqual(example.kind, "Operation");
+
+    const ReadA = example.returnType as Model;
+
+    strictEqual(ReadA.kind, "Model");
+
+    const aB = ReadA.properties.get("b")!.type as Model;
+
+    strictEqual(aB.kind, "Model");
+
+    ok(aB === B);
+  });
+
   it("correctly transforms arrays and records", async () => {
     const { Result } = (await runner.compile(`
       model A {