Skip to content

Commit 9faec05

Browse files
compiler: Exclude false scope members from encoded name duplication tracking (#8681)
This is a very strange bug that likely stems from the way mutators work. When mutating a model for visibility transforms, we create clones of the model's properties. These clones end up checked/finished, even if they are not actually _attached_ to the model, and vice versa (the original properties may be checked/finished even if the clone ends up attached). This runs stateful decorators, like `@encodedName`. Unfortunately, in these cases, it means that both the clone and the original property end up binding an encoded name within the same scope, which the validator later sees as a conflict. However, only one of these properties is actually a member of the scope at validation time. This PR uses that to exclude "false members". These are properties that exist in the state map (the decorator has been applied to them), but aren't actually bound to the parent they claim to be a member of. We can work around this mutator bug in the validator by checking if the target is actually within the set of member values (in this case, the properties actually present in the parent model, but this will also work for enum members and union variants). Closes #8599
1 parent 2693658 commit 9faec05

File tree

4 files changed

+32
-2
lines changed

4 files changed

+32
-2
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/compiler"
5+
---
6+
7+
Addressed a bug that could cause duplicate `@encodedName` applications to be detected when none actually exist.

packages/compiler/src/lib/encoded-names.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,16 @@ export function validateEncodedNamesConflicts(program: Program) {
106106

107107
for (const [target, map] of getEncodedNamesStateMap(program).entries()) {
108108
const scope = getScope(target);
109-
if (scope === undefined) {
109+
const memberValues = new Set(scope?.members.values());
110+
if (
111+
scope === undefined ||
112+
// Workaround: exclude members that aren't actually in the scope. This can happen when types of
113+
// properties, enum members, or union variants are cloned for one reason or another, but are not
114+
// actually attached to the parent type. This should probably be fixed in cloning logic (mutator
115+
// implementation) instead, but for now this workaround solves some problems with false positives
116+
// in duplication tracking here.
117+
!memberValues.has(target)
118+
) {
110119
return;
111120
}
112121
for (const [mimeType, name] of map.entries()) {

packages/compiler/src/lib/visibility.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,7 @@ function createVisibilityFilterMutator(
681681
realm.remove(clone);
682682
modified = true;
683683
} else {
684-
const mutated = mutateSubgraph(program, [mpMutator], prop);
684+
const mutated = cachedMutateSubgraph(program, mpMutator, prop);
685685

686686
clone.properties.set(key, mutated.type as ModelProperty);
687687

packages/compiler/test/visibility.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,20 @@ describe("compiler: visibility core", () => {
12391239
ok(arrA.properties.has("a"));
12401240
ok(!arrA.properties.has("invisible"));
12411241
});
1242+
1243+
it("does not duplicate encodedName metadata", async () => {
1244+
const diagnostics = await runner.diagnose(`
1245+
model SomeModel {
1246+
@visibility(Lifecycle.Read)
1247+
@encodedName("application/json", "some_other_name")
1248+
someOtherName: string;
1249+
}
1250+
1251+
alias ReadModel = Read<SomeModel>;
1252+
`);
1253+
1254+
expectDiagnosticEmpty(diagnostics);
1255+
});
12421256
});
12431257

12441258
function validateCreateOrUpdateTransform(

0 commit comments

Comments
 (0)