Skip to content

Commit

Permalink
introduce name and crossLanguageDefinitionId to SdkBuiltInType,…
Browse files Browse the repository at this point in the history
… `SdkDateTimeType` and `SdkDurationType` (#1015)

Fixes #1010
Fixes #997
Fixes #1106

# Changes and migration guides

## Changes

1. remove the following types from `SdkBuiltInType`:
   - `uuid` (azure type)
   - `ipV4Address` (azure type)
   - `ipV6Address` (azure type)
   - `eTag` (azure type)
   - `armId` (azure type)
   - `azureLocation` (azure type)
   - `password` (removed)
   - `guid` (removed)
   - `uri` (removed)
   - `ipAddress` (removed)
2. `@format` can no longer change a string type to above azure types
(`uuid`, `eTag`, etc).
3. add `name`, `crossLanguageDefinitionId` and `baseType` to
`SdkBuiltInType`, `SdkDateTimeType` and `SdkDurationType`.
4. now scalars defined using `scalar` keyword will be parsed into either
`SdkBuiltInType`, `SdkDateTimeType` or `SdkDurationType` depending on
the base type, with its `name` and `crossLanguageDefinitionId`.
`@encode` will be added to the scalar type, and will not propagate to
its base type.
5. `isSdkFloatKind` now returns `false` for `decimal` and `decimal128`.

## Migration guides for emitters

The major breaking change introduced by this change is the removal of
azure related kinds for `SdkBuiltInType`. Therefore for code such as:
```typescript
if (type.kind === "azureLocation")
{
  // do something for azure-location
}
```
should be changed to the following if the emitter is doing something
special for azure-location:
```typescript
if (type.kind === "string" && type.crossLanguageDefinitionId === "Azure.Core.azureLocation")
{
  // do something for azure-location
}
```
or just change it to this:
```typescript
if (type.kind === "string")
{
  // treat azure-location as a string
}
```

For those removed kinds, they no longer exist therefore it should be
safe just remove related code snippets.

# Summary

This PR includes the following changes:

1. remove the following types from `SdkBuiltInType`:
   - `uuid` (azure type)
   - `ipV4Address` (azure type)
   - `ipV6Address` (azure type)
   - `eTag` (azure type)
   - `armId` (azure type)
   - `azureLocation` (azure type)
   - `password` (removed)
   - `guid` (removed)
   - `uri` (removed)
   - `ipAddress` (removed)
2. `@format` can no longer change a string type to above azure types
(`uuid`, `eTag`, etc).
3. add `name`, `crossLanguageDefinitionId` and `baseType` to
`SdkBuiltInType`, `SdkDatetimeType` and `SdkDurationType`.
4. now scalars defined using `scalar` keyword will be parsed into either
`SdkBuiltInType`, `SdkDatetimeType` or `SdkDurationType` depending on
the base type, with its `name` and `crossLanguageDefinitionId`.
`@encode` will be added to the scalar type, and will not propagate to
its base type.
5. `isSdkFloatKind` now returns `false` for `decimal` and `decimal128`.

# Reasons and motivations

TCGC should be "linguistic neutrality" which does not favor or disfavor
any language and only represent the structures defined in a tsp
file/files.
Also I think it should be neutral for 3rd party extensions, and azure
should be regarded as our first 3rd party extension. Therefore we need a
way to be able to handle those azure types without making them special
in TCGC's code.

The solution that this PR is applying is similar to the solution we have
for models in azure core template.
Instead of creating a special kind for models in azure core template
(such as the `ResponseError`, `TrackedResource`, `Resource` or
`ExtendedLocation`), we just parse them as regular models with their own
name and tsp namespace, so that the downstream genreators could know
where these types are defined, and if they want, they could do a filter,
to handle those special types.
Same thing could apply to scalars, for a scalar defined in azure core
template:
```typespec
namespace Azure.Core;
scalar azureLocation extends string;
```
for those would care this, this stands for an "azure location", for
those would not care, this is just a string.
Therefore this PR introduces name and `crossLanguageDefinitionId` to
`SdkBuiltInType` just like we have in models, this scalar now will be
parsed as:
```
{
	kind: "string",
	name: "azureLocation",
	tspNamespace: "Azure.Core",
	baseType: {
		kind: "string",
		name: "string",
		tspNamespace: "TypeSpec"
	}
}
```
The `crossLanguageDefinitionId` is important for downstream generators
to know whether this scalar is defined.
`baseType` here is also important and we must have this, because we do
not want to see if users write scalars extending some of the scalars
from azure core template, and it ends up with a string.
For example, based on the fact that we have the above `azureLocation`
defined in our azure core template, and one could write:
```typespec
scalar myLocation extends azureLocation;

model Foo {
	bar: myLocation;
}
```
in this case, I think the generator should generate the `bar` property
of `Foo` model as `AzureLocation` type. But if we do not have the
`baseType`, we only have this:
```typespec
{
	kind: "string",
	name: "myLocation",
	tspNamespace: "My.Service"
}
```
which has nothing with `azureLocation` which is not acceptable.

---------

Co-authored-by: tadelesh <tadelesh.shi@live.cn>
  • Loading branch information
ArcturusZhang and tadelesh committed Jul 23, 2024
1 parent c521e6f commit 524d459
Show file tree
Hide file tree
Showing 13 changed files with 375 additions and 972 deletions.
8 changes: 8 additions & 0 deletions .chronus/changes/add-scalar-type-2024-5-14-13-23-7.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: breaking
packages:
- "@azure-tools/typespec-client-generator-core"
---

refactor tcgc build-in types, please refer pr's description for details and migration guides
37 changes: 22 additions & 15 deletions docs/howtos/DataPlane Generation - DPG/07tcgcTypes.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,9 @@ interface SdkTypeBase {
// created by tcgc. Those won't have an original type
__raw?: Type;
kind: string;
deprecations?: string;
deprecation?: string;
description?: string;
details?: string;
}
```

Expand All @@ -1236,16 +1238,22 @@ There is a one-to-one mapping between the TypeSpec scalar kinds and the `SdkBuil
export interface SdkBuiltInType extends SdkTypeBase {
kind: SdkBuiltInKinds;
encode: string;
name: string;
baseType?: SdkBuiltInType;
crossLanguageDefinitionId: string;
}
```

### SdkDateTimeType

```ts
interface SdkDatetimeTypeBase extends SdkTypeBase {
name: string;
baseType?: SdkDateTimeType;
encode: DateTimeKnownEncoding;
// what we send over the wire. Often it's string
wireType: SdkBuiltInType;
crossLanguageDefinitionId: string;
}

interface SdkUtcDatetimeType extends SdkDatetimeTypeBase {
Expand All @@ -1262,9 +1270,12 @@ interface SdkOffsetDatetimeType extends SdkDatetimeTypeBase {
```ts
interface SdkDurationType extends SdkTypeBase {
kind: "duration";
name: string;
baseType?: SdkDurationType;
encode: DurationKnownEncoding;
// What we send over the wire. It's usually either a string or a float
wireType: SdkBuiltInType;
crossLanguageDefinitionId: string;
}
```

Expand All @@ -1273,8 +1284,9 @@ interface SdkDurationType extends SdkTypeBase {
```ts
interface SdkArrayType extends SdkTypeBase {
kind: "array";
name: string;
valueType: SdkType;
nullableValues: boolean;
crossLanguageDefinitionId: string;
}
```

Expand All @@ -1285,7 +1297,6 @@ interface SdkDictionaryType extends SdkTypeBase {
kind: "dict";
keyType: SdkType; // currently can only be string
valueType: SdkType;
nullableValues: boolean;
}
```

Expand All @@ -1296,17 +1307,16 @@ export interface SdkEnumType extends SdkTypeBase {
kind: "enum";
name: string;
// Determines whether the name was generated or not
generatedName: boolean;
isGeneratedName: boolean;
valueType: SdkBuiltInType;
values: SdkEnumValueType[];
isFixed: boolean;
description?: string;
details?: string;
isFlags: boolean;
usage: UsageFlags;
access: AccessFlags;
crossLanguageDefinitionId: string;
apiVersions: string[];
isUnionAsEnum: boolean;
}
```

Expand All @@ -1318,9 +1328,7 @@ export interface SdkEnumValueType extends SdkTypeBase {
name: string;
value: string | number;
enumType: SdkEnumType;
valueType: SdkType;
description?: string;
details?: string;
valueType: SdkBuiltInType;
}
```

Expand All @@ -1331,6 +1339,8 @@ export interface SdkConstantType extends SdkTypeBase {
kind: "constant";
value: string | number | boolean | null;
valueType: SdkBuiltInType;
name: string;
isGeneratedName: boolean;
}
```

Expand All @@ -1340,9 +1350,10 @@ export interface SdkConstantType extends SdkTypeBase {
export interface SdkUnionType extends SdkTypeBase {
name: string;
// determines if the union name was generated or not
generatedName: boolean;
isGeneratedName: boolean;
kind: "union";
values: SdkType[];
crossLanguageDefinitionId: string;
}
```

Expand All @@ -1353,13 +1364,9 @@ export interface SdkModelType extends SdkTypeBase {
kind: "model";
// purposely can also be header / query params for fidelity with TypeSpec
properties: SdkModelPropertyType[];
isFormDataType: boolean;
isError: boolean;
// we will always have a name. generatedName determines if it's generated or not.
name: string;
generatedName: string;
description?: string;
details?: string;
isGeneratedName: boolean;
access: AccessFlags;
usage: UsageFlags;
additionalProperties?: SdkType;
Expand Down
Loading

0 comments on commit 524d459

Please sign in to comment.