From b3174af13a016001d01810a9e5bce3817d397557 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Thu, 24 Apr 2025 11:03:17 -0700 Subject: [PATCH 1/7] initial idea --- packages/compiler/src/core/checker.ts | 26 +++++++------- packages/compiler/test/checker/model.test.ts | 27 ++++++++++++++ .../compiler/test/checker/templates.test.ts | 36 +++++++++++++++++++ 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 583946d9bf0..f2e4917e257 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -548,7 +548,6 @@ export function createChecker(program: Program, resolver: NameResolver): Checker node: Node, mapper?: TypeMapper, constraint?: CheckValueConstraint, - options: { legacyTupleAndModelCast?: boolean } = {}, ): Value | null { const initial = checkNode(node, mapper, constraint); if (initial === null) { @@ -560,15 +559,21 @@ export function createChecker(program: Program, resolver: NameResolver): Checker } else { entity = initial; } - // if (options.legacyTupleAndModelCast && entity !== null && isType(entity)) { - // entity = legacy_tryTypeToValueCast(entity, constraint, node); - // } if (entity === null) { return null; } if (isValue(entity)) { return constraint ? inferScalarsFromConstraints(entity, constraint.type) : entity; } + // If a template parameter that can be a value is used in a template declaration then we allow it but we return null because we don't have an actual value. + if ( + entity.kind === "TemplateParameter" && + entity.constraint?.valueType && + entity.constraint.type === undefined && + mapper === undefined + ) { + return null; + } reportExpectedValue(node, entity); return null; } @@ -4664,15 +4669,10 @@ export function createChecker(program: Program, resolver: NameResolver): Checker // if the prop type is an error we don't need to validate again. return null; } - const defaultValue = getValueForNode( - defaultNode, - undefined, - { - kind: "assignment", - type, - }, - { legacyTupleAndModelCast: true }, - ); + const defaultValue = getValueForNode(defaultNode, undefined, { + kind: "assignment", + type, + }); if (defaultValue === null) { return null; } diff --git a/packages/compiler/test/checker/model.test.ts b/packages/compiler/test/checker/model.test.ts index 26f11a5e15a..b3498584b1e 100644 --- a/packages/compiler/test/checker/model.test.ts +++ b/packages/compiler/test/checker/model.test.ts @@ -194,6 +194,33 @@ describe("compiler: models", () => { deepStrictEqual(foo.defaultValue?.value, "up-value"); }); }); + + describe("using a template parameter", () => { + it(`set it with valid constraint`, async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + model A { @test foo?: string = T } + alias Test = A<"Abc">; + `, + ); + const { foo } = (await testHost.compile("main.tsp")) as { foo: ModelProperty }; + strictEqual(foo.defaultValue?.valueKind, "StringValue"); + }); + it(`error if constraint is not compatible with property type`, async () => { + testHost.addTypeSpecFile( + "main.tsp", + ` + model A { @test foo?: string = T } + `, + ); + const diagnostics = await testHost.diagnose("main.tsp"); + expectDiagnostics(diagnostics, { + code: "unassignable", + message: "Type 'T' is not assignable to type 'string'.", + }); + }); + }); }); describe("doesn't allow a default of different type than the property type", () => { diff --git a/packages/compiler/test/checker/templates.test.ts b/packages/compiler/test/checker/templates.test.ts index a0e0068895f..1fea0e5785a 100644 --- a/packages/compiler/test/checker/templates.test.ts +++ b/packages/compiler/test/checker/templates.test.ts @@ -982,4 +982,40 @@ describe("compiler: templates", () => { strictEqual(members[1][1].name, "string"); }); }); + + describe("template declaration passing values", () => { + it("allows passing to a decorator expecting that value", async () => { + testHost.addJsFile("effect.js", { + $call: () => null, + }); + + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./effect.js"; + extern dec call(target, arg: valueof string); + @call(T) model Dec {} + `, + ); + const diagnostics = await testHost.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + + it("allows passing to a decorator expecting a composed value", async () => { + testHost.addJsFile("effect.js", { + $call: () => null, + }); + + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./effect.js"; + extern dec call(target, arg: valueof unknown); + @call(#{foo: T}) model Dec {} + `, + ); + const diagnostics = await testHost.diagnose("main.tsp"); + expectDiagnosticEmpty(diagnostics); + }); + }); }); From c428dd845dc9132fb02f98aa4b1920b6b6c6d7eb Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 25 Apr 2025 15:45:50 -0700 Subject: [PATCH 2/7] Use nullvalue?? --- packages/compiler/src/core/checker.ts | 20 ++++++++++++++++---- packages/compiler/test/checker/model.test.ts | 3 ++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index f2e4917e257..b98a0854efa 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -572,7 +572,15 @@ export function createChecker(program: Program, resolver: NameResolver): Checker entity.constraint.type === undefined && mapper === undefined ) { - return null; + return createValue( + { + entityKind: "Value", + valueKind: "NullValue", + type: entity.constraint.valueType, + value: null, + }, + entity.constraint.valueType, + ); } reportExpectedValue(node, entity); return null; @@ -4627,7 +4635,7 @@ export function createChecker(program: Program, resolver: NameResolver): Checker pendingResolutions.start(sym, ResolutionKind.Type); type.type = getTypeForNode(prop.value, mapper); if (prop.default) { - const defaultValue = checkDefaultValue(prop.default, type.type); + const defaultValue = checkDefaultValue(prop.default, type.type, mapper); if (defaultValue !== null) { type.defaultValue = defaultValue; } @@ -4664,12 +4672,16 @@ export function createChecker(program: Program, resolver: NameResolver): Checker }; } - function checkDefaultValue(defaultNode: Node, type: Type): Value | null { + function checkDefaultValue( + defaultNode: Node, + type: Type, + mapper: TypeMapper | undefined, + ): Value | null { if (isErrorType(type)) { // if the prop type is an error we don't need to validate again. return null; } - const defaultValue = getValueForNode(defaultNode, undefined, { + const defaultValue = getValueForNode(defaultNode, mapper, { kind: "assignment", type, }); diff --git a/packages/compiler/test/checker/model.test.ts b/packages/compiler/test/checker/model.test.ts index b3498584b1e..65c789a4f77 100644 --- a/packages/compiler/test/checker/model.test.ts +++ b/packages/compiler/test/checker/model.test.ts @@ -207,6 +207,7 @@ describe("compiler: models", () => { const { foo } = (await testHost.compile("main.tsp")) as { foo: ModelProperty }; strictEqual(foo.defaultValue?.valueKind, "StringValue"); }); + it(`error if constraint is not compatible with property type`, async () => { testHost.addTypeSpecFile( "main.tsp", @@ -217,7 +218,7 @@ describe("compiler: models", () => { const diagnostics = await testHost.diagnose("main.tsp"); expectDiagnostics(diagnostics, { code: "unassignable", - message: "Type 'T' is not assignable to type 'string'.", + message: "Type 'int32' is not assignable to type 'string'", }); }); }); From 25317ff7180141f2db009caf3bc4ff71cfebcd86 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 25 Apr 2025 15:53:12 -0700 Subject: [PATCH 3/7] . --- .../compiler/test/checker/templates.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/compiler/test/checker/templates.test.ts b/packages/compiler/test/checker/templates.test.ts index 1fea0e5785a..f4c95daee50 100644 --- a/packages/compiler/test/checker/templates.test.ts +++ b/packages/compiler/test/checker/templates.test.ts @@ -1017,5 +1017,26 @@ describe("compiler: templates", () => { const diagnostics = await testHost.diagnose("main.tsp"); expectDiagnosticEmpty(diagnostics); }); + + it("validate incompatible composed values", async () => { + testHost.addJsFile("effect.js", { + $call: () => null, + }); + + testHost.addTypeSpecFile( + "main.tsp", + ` + import "./effect.js"; + extern dec call(target, arg: valueof {foo: int32}); + @call(#{foo: T}) model Dec {} + `, + ); + const diagnostics = await testHost.diagnose("main.tsp"); + expectDiagnostics(diagnostics, { + code: "invalid-argument", + message: + "Argument of type '{ foo: string }' is not assignable to parameter of type '{ foo: int32 }'", + }); + }); }); }); From 4534519845b6f9261aa3d53d35ad3e175c052c22 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 28 Apr 2025 12:40:59 -0700 Subject: [PATCH 4/7] try this way --- packages/compiler/src/core/checker.ts | 10 +++++----- packages/compiler/src/core/types.ts | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index b98a0854efa..9f2b7306bd4 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -156,6 +156,7 @@ import { UnknownType, UsingStatementNode, Value, + ValueWithTemplate, VoidType, } from "./types.js"; @@ -303,7 +304,7 @@ export function createChecker(program: Program, resolver: NameResolver): Checker TypeReferenceNode | MemberExpressionNode | IdentifierNode, Sym | undefined >(); - const valueExactTypes = new WeakMap(); + const valueExactTypes = new WeakMap(); let onCheckerDiagnostic: (diagnostic: Diagnostic) => void = (x: Diagnostic) => { program.reportDiagnostic(x); }; @@ -575,12 +576,11 @@ export function createChecker(program: Program, resolver: NameResolver): Checker return createValue( { entityKind: "Value", - valueKind: "NullValue", + valueKind: "TemplateValue", type: entity.constraint.valueType, - value: null, }, entity.constraint.valueType, - ); + ) as any; } reportExpectedValue(node, entity); return null; @@ -3823,7 +3823,7 @@ export function createChecker(program: Program, resolver: NameResolver): Checker ); } - function createValue(value: T, preciseType: Type): T { + function createValue(value: T, preciseType: Type): T { valueExactTypes.set(value, preciseType); return value; } diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts index a7010a8c3d7..ec64f0cc378 100644 --- a/packages/compiler/src/core/types.ts +++ b/packages/compiler/src/core/types.ts @@ -317,6 +317,9 @@ export type Value = | EnumValue | NullValue; +/** @internal */ +export type ValueWithTemplate = Value | TemplateValue; + interface BaseValue { readonly entityKind: "Value"; readonly valueKind: string; @@ -380,6 +383,15 @@ export interface NullValue extends BaseValue { value: null; } +/** + * This is an internal type that represent a value while in a template declaration. + * This type should currently never be exposed on the type graph(unlike TemplateParameter). + * @internal + */ +export interface TemplateValue extends BaseValue { + valueKind: "TemplateValue"; +} + //#endregion Values export interface Scalar extends BaseType, DecoratedType, TemplatedTypeBase { From 943562ad5606866e2a01b4d536bb15095b11d3c5 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 28 Apr 2025 12:57:04 -0700 Subject: [PATCH 5/7] Create tenative-fix-template-values-2025-3-28-19-41-46.md --- .../tenative-fix-template-values-2025-3-28-19-41-46.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/tenative-fix-template-values-2025-3-28-19-41-46.md diff --git a/.chronus/changes/tenative-fix-template-values-2025-3-28-19-41-46.md b/.chronus/changes/tenative-fix-template-values-2025-3-28-19-41-46.md new file mode 100644 index 00000000000..402d00d7692 --- /dev/null +++ b/.chronus/changes/tenative-fix-template-values-2025-3-28-19-41-46.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Allow passing template parameters as property defaults From ff5fa2c251caacabc0efdf177bc0ac1c5d9c2bd3 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 28 Apr 2025 12:57:43 -0700 Subject: [PATCH 6/7] Create tenative-fix-template-values-2025-3-28-19-41-47.md --- .../tenative-fix-template-values-2025-3-28-19-41-47.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/tenative-fix-template-values-2025-3-28-19-41-47.md diff --git a/.chronus/changes/tenative-fix-template-values-2025-3-28-19-41-47.md b/.chronus/changes/tenative-fix-template-values-2025-3-28-19-41-47.md new file mode 100644 index 00000000000..94d495ff103 --- /dev/null +++ b/.chronus/changes/tenative-fix-template-values-2025-3-28-19-41-47.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/compiler" +--- + +Allow passing tempalate parameter values in object and array values used inside the template From dee5834a29581fee770c66ac9c9eb550332546b1 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 28 Apr 2025 12:58:00 -0700 Subject: [PATCH 7/7] make sure its not exposed --- packages/compiler/src/core/checker.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 9f2b7306bd4..1bc688d17c2 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -4692,6 +4692,10 @@ export function createChecker(program: Program, resolver: NameResolver): Checker if (!related) { reportCheckerDiagnostics(diagnostics); return null; + } else if ((defaultValue.valueKind as any) === "TemplateValue") { + // Right now we don't want to expose `TemplateValue` in the type graph. + // And as interating with the template declaration is not a supported feature we can just drop it. + return null; } else { return { ...defaultValue, type }; }