Name collisions from structurally named schema now error

.changeset/social-bears-deny.md

@@ -0,0 +1,23 @@
+---
+"@fluidframework/tree": minor
+"fluid-framework": minor
+"__section": tree
+---
+Name collisions from structurally named schema now error
+
+It is legal to have multiple [TreeNodeSchema](https://fluidframework.com/docs/api/fluid-framework/treenodeschema-typealias) with the same name so long as they are not used together in the same tree.
+Using different schema with the same name when building otherwise identical [structurally named](https://fluidframework.com/docs/api/fluid-framework/schemafactory-class#schemafactory-remarks) in the same [SchemaFactory](https://fluidframework.com/docs/api/fluid-framework/schemafactory-class) is not valid, however.
+Previously doing this would not error, and instead return the first structurally named schema with that name.
+Now this case throws an informative error:
+
+```typescript
+const factory = new SchemaFactory(undefined);
+class Child1 extends factory.object("Child", {}) {}
+class Child2 extends factory.object("Child", {}) {}
+
+const a = factory.map(Child1);
+
+// Throws a UsageError with the message:
+// "Structurally named schema collision: two schema named "Array<["Child"]>" were defined with different input schema."
+const b = factory.array(Child2);
+```

packages/dds/tree/src/simple-tree/api/schemaFactory.ts

@@ -6,6 +6,7 @@
 import type { IFluidHandle } from "@fluidframework/core-interfaces";
 import { assert, unreachableCase } from "@fluidframework/core-utils/internal";
 import { isFluidHandle } from "@fluidframework/runtime-utils/internal";
+import { UsageError } from "@fluidframework/telemetry-utils/internal";
 
 import type { TreeValue } from "../../core/index.js";
 import type { NodeIdentifierManager } from "../../feature-libraries/index.js";
@@ -14,6 +15,7 @@ import type { NodeIdentifierManager } from "../../feature-libraries/index.js";
 import type { TreeAlpha } from "../../shared-tree/index.js";
 import {
 	type RestrictiveStringRecord,
+	compareSets,
 	getOrCreate,
 	isReadonlyArray,
 } from "../../util/index.js";
@@ -60,6 +62,7 @@ import {
 	type ImplicitAnnotatedAllowedTypes,
 	type UnannotateImplicitAllowedTypes,
 	type UnannotateSchemaRecord,
+	normalizeAllowedTypes,
 } from "../schemaTypes.js";
 
 import { createFieldSchemaUnsafe } from "./schemaFactoryRecursive.js";
@@ -766,9 +769,9 @@ export class SchemaFactory<
 		if (allowedTypes === undefined) {
 			const types = nameOrAllowedTypes as (T & TreeNodeSchema) | readonly TreeNodeSchema[];
 			const fullName = structuralName("Map", types);
-			return getOrCreate(
-				this.structuralTypes,
+			return this.getStructuralType(
 				fullName,
+				types,
 				() =>
 					this.namedMap(
 						fullName as TName,
@@ -954,7 +957,7 @@ export class SchemaFactory<
 		if (allowedTypes === undefined) {
 			const types = nameOrAllowedTypes as (T & TreeNodeSchema) | readonly TreeNodeSchema[];
 			const fullName = structuralName("Array", types);
-			return getOrCreate(this.structuralTypes, fullName, () =>
+			return this.getStructuralType(fullName, types, () =>
 				this.namedArray(fullName, nameOrAllowedTypes as T, false, true),
 			) as TreeNodeSchemaClass<
 				ScopedSchemaName<TScope, string>,
@@ -978,6 +981,35 @@ export class SchemaFactory<
 		return out;
 	}
 
+	/**
+	 * Retrieves or creates a structural {@link TreeNodeSchema} with the specified name and types.
+	 *
+	 * @param fullName - The name for the structural schema.
+	 * @param types - The input schema(s) used to define the structural schema.
+	 * @param builder - A function that builds the schema if it does not already exist.
+	 * @returns The structural {@link TreeNodeSchema} associated with the given name and types.
+	 * @throws `UsageError` if a schema structurally named schema with the same name is cached in `structuralTypes` but had different input types.
+	 */
+	private getStructuralType(
+		fullName: string,
+		types: TreeNodeSchema | readonly TreeNodeSchema[],
+		builder: () => TreeNodeSchema,
+	): TreeNodeSchema {
+		const structural = getOrCreate(this.structuralTypes, fullName, builder);
+		const inputTypes = new Set(normalizeAllowedTypes(types));
+		const outputTypes = new Set(
+			normalizeAllowedTypes(structural.info as TreeNodeSchema | readonly TreeNodeSchema[]),
+		);
+		// If our cached value had a different set of types then were requested, the user must have caused a collision.
+		const same = compareSets({ a: inputTypes, b: outputTypes });
+		if (!same) {
+			throw new UsageError(
+				`Structurally named schema collision: two schema named "${fullName}" were defined with different input schema.`,
+			);
+		}
+		return structural;
+	}
+
 	/**
 	 * Define a {@link TreeNodeSchema} for a {@link (TreeArrayNode:interface)}.
 	 *

packages/dds/tree/src/test/simple-tree/api/schemaFactory.spec.ts

@@ -1079,6 +1079,47 @@ describe("schemaFactory", () => {
 		assert.deepEqual(getKeys(arr), [0]);
 		assert.deepEqual(getKeys(mapNode), ["x"]);
 	});
+
+	it("structural type collision: single type", () => {
+		const factory = new SchemaFactory("");
+		class Child1 extends factory.object("Child", {}) {}
+		class Child2 extends factory.object("Child", {}) {}
+
+		const a = factory.array(Child1);
+		// No error: this type is the same as the one above.
+		assert.equal(factory.array(Child1), a);
+
+		// Error: this type is different from the one above.
+		assert.throws(
+			() => {
+				factory.array(Child2);
+			},
+			validateUsageError(/collision/),
+		);
+
+		assert.equal(factory.array([Child1]), a);
+		assert.throws(
+			() => {
+				factory.array([Child2]);
+			},
+			validateUsageError(/collision/),
+		);
+	});
+
+	it("structural type collision: multi type", () => {
+		const factory = new SchemaFactory("");
+		class Child1 extends factory.object("Child", {}) {}
+		class Child2 extends factory.object("Child", {}) {}
+
+		const a = factory.map([Child1, SchemaFactory.null]);
+		assert.equal(factory.map([SchemaFactory.null, Child1]), a);
+		assert.throws(
+			() => {
+				factory.map([Child2, SchemaFactory.null]);
+			},
+			validateUsageError(/collision/),
+		);
+	});
 });
 
 // kind based narrowing example