From c99c8261615d57aba1a003306b2b4365a1990b0d Mon Sep 17 00:00:00 2001 From: tjprescott Date: Tue, 11 Apr 2023 13:44:08 -0700 Subject: [PATCH 1/3] Fixes #1706. --- .../core-AllowRequiredDefault_2023-04-11-20-44.json | 10 ++++++++++ packages/compiler/core/messages.ts | 6 ------ packages/compiler/core/parser.ts | 6 ------ packages/compiler/test/parser.test.ts | 8 ++++---- 4 files changed, 14 insertions(+), 16 deletions(-) create mode 100644 common/changes/@typespec/compiler/core-AllowRequiredDefault_2023-04-11-20-44.json diff --git a/common/changes/@typespec/compiler/core-AllowRequiredDefault_2023-04-11-20-44.json b/common/changes/@typespec/compiler/core-AllowRequiredDefault_2023-04-11-20-44.json new file mode 100644 index 00000000000..94ab7361731 --- /dev/null +++ b/common/changes/@typespec/compiler/core-AllowRequiredDefault_2023-04-11-20-44.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/compiler", + "comment": "Allow use of defaults on non-optional properties.", + "type": "none" + } + ], + "packageName": "@typespec/compiler" +} \ No newline at end of file diff --git a/packages/compiler/core/messages.ts b/packages/compiler/core/messages.ts index ee8f1cd76bb..cb710f231cb 100644 --- a/packages/compiler/core/messages.ts +++ b/packages/compiler/core/messages.ts @@ -117,12 +117,6 @@ const diagnostics = { topLevel: "Imports must be top-level and come prior to namespaces or other declarations.", }, }, - "default-optional": { - severity: "error", - messages: { - default: "Cannot use default with non optional properties", - }, - }, "token-expected": { severity: "error", messages: { diff --git a/packages/compiler/core/parser.ts b/packages/compiler/core/parser.ts index 63440f2f09c..6867a58fda5 100644 --- a/packages/compiler/core/parser.ts +++ b/packages/compiler/core/parser.ts @@ -853,9 +853,6 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa const value = parseExpression(); const hasDefault = parseOptional(Token.Equals); - if (hasDefault && !optional) { - error({ code: "default-optional" }); - } const defaultValue = hasDefault ? parseExpression() : undefined; return { kind: SyntaxKind.ModelProperty, @@ -2055,9 +2052,6 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa const value = parseProjectionExpression(); const hasDefault = parseOptional(Token.Equals); - if (hasDefault && !optional) { - error({ code: "default-optional" }); - } const defaultValue = hasDefault ? parseProjectionExpression() : undefined; return { kind: SyntaxKind.ProjectionModelProperty, diff --git a/packages/compiler/test/parser.test.ts b/packages/compiler/test/parser.test.ts index ee9858fc5b6..be75ad554a8 100644 --- a/packages/compiler/test/parser.test.ts +++ b/packages/compiler/test/parser.test.ts @@ -38,6 +38,10 @@ describe("compiler: parser", () => { prop2: string };`, + `model Car { + withDefaultButNotOptional: string = "foo" + }`, + `model Car { optional?: number; withDefault?: string = "my-default"; @@ -110,10 +114,6 @@ describe("compiler: parser", () => { ["model Car is Foo extends Bar { }", [/'{' expected/]], ["model Car extends Bar is Foo { }", [/'{' expected/]], ["model Car { withDefaultMissing?: string = }", [/Expression expected/]], - [ - `model Car { withDefaultButNotOptional: string = "foo" }`, - [/Cannot use default with non optional properties/], - ], ["model", [/Identifier expected/]], ["model Car is Vehicle", [/';', or '{' expected/]], ["model Car;", [/'{', '=', 'extends', or 'is' expected/]], From 7f7017c1f480a0492b699e2973384391c1e029e8 Mon Sep 17 00:00:00 2001 From: tjprescott Date: Tue, 11 Apr 2023 14:10:02 -0700 Subject: [PATCH 2/3] Add emitter documentation for handling default values. --- docs/extending-typespec/emitters-basics.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/extending-typespec/emitters-basics.md b/docs/extending-typespec/emitters-basics.md index b0e9575fc8b..344af507341 100644 --- a/docs/extending-typespec/emitters-basics.md +++ b/docs/extending-typespec/emitters-basics.md @@ -186,3 +186,25 @@ Since an emitter is a node library, you could use standard `fs` APIs to write fi Instead, use the compiler [`host` interface](#todo) to access the file system. The API is equivalent to the node API but works in a wider range of scenarios. In order to know where to emit files, the emitter context has a `emitterOutputDir` property that is automatically resolved using the `emitter-output-dir` built-in emitter options. This is set to `{cwd}/tsp-output/{emitter-name}` by default, but can be overridden by the user. Do not use the `compilerOptions.outputDir` + +## Handling Default Values + +Several TypeSpec types have a `default` property that can be used to specify a default value. For example, the following model has a default value of `true` for the `isActive` property: + +```tsp +model User { + isActive?: boolean = true; +} +``` + +These values can be accessed in the emitter using the `default` property on the `ModelProperty` type. + +```ts +const modelProp: ModelProperty = ...; // the isActive ModelProperty type +const defaultValue = modelProp.default; // value: true +``` + +It is important that emitters handle default values in a consistent way. The following guidance should be used when emitting default values: + +- operation parameters: use the `default` value, if provided, as the client default when no value is specified +- model properties: do nothing. Note that some emitters may interpret default values on model properties in a way that they essentially become operation parameters. For example, a Python emitter might generate an `__init__` method that includes all model properties and their default values. In this case, the emitter should follow the guidance for operation parameters. From 07870fddb5993b4d3e855506d0ce7d8ee5253d94 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Wed, 12 Apr 2023 14:20:06 -0700 Subject: [PATCH 3/3] Update documentation. --- docs/extending-typespec/emitters-basics.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/docs/extending-typespec/emitters-basics.md b/docs/extending-typespec/emitters-basics.md index 344af507341..ddd67f99f3d 100644 --- a/docs/extending-typespec/emitters-basics.md +++ b/docs/extending-typespec/emitters-basics.md @@ -204,7 +204,4 @@ const modelProp: ModelProperty = ...; // the isActive ModelProperty type const defaultValue = modelProp.default; // value: true ``` -It is important that emitters handle default values in a consistent way. The following guidance should be used when emitting default values: - -- operation parameters: use the `default` value, if provided, as the client default when no value is specified -- model properties: do nothing. Note that some emitters may interpret default values on model properties in a way that they essentially become operation parameters. For example, a Python emitter might generate an `__init__` method that includes all model properties and their default values. In this case, the emitter should follow the guidance for operation parameters. +It is important that emitters handle default values in a consistent way. Default values SHOULD NOT be used as client-side default values. Instead, they should be used as a way to specify a default value for the server-side implementation. For example, if a model property has a default value of `true`, the server-side implementation should use that value if the client does not provide a value. Default values SHOULD be expressed in documentation to properly communicate the service-side default.