Skip to content

Commit d462382

Browse files
authored
Merge branch 'main' into copilot/fix-issue-with-typespec
2 parents db2b9af + 3e578c2 commit d462382

15 files changed

Lines changed: 353 additions & 55 deletions

File tree

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+
Hide cursor during spinner animation to prevent flicker on Windows
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
changeKind: feature
3+
packages:
4+
- "@typespec/protobuf"
5+
---
6+
7+
Map TypeSpec optionality (`?`) to protobuf `optional` where appropriate.
8+
9+
- `optional` is applied to fields with protobuf scalar types to set explicit presence.
10+
- `optional` is _not_ applied to fields with message types, because they _always_ have explicit presence.
11+
- Attempting to convert a TypeSpec optional property where the type is an array or `Protobuf.Map` instance produces a warning, because protobuf cannot differentiate between "empty" and "unset" `repeated`/`map`-typed fields.

packages/compiler/src/core/logger/dynamic-task.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export class DynamicTask implements TrackActionTask {
4242

4343
start() {
4444
if (this.#isTTY) {
45+
this.#stream.write("\x1B[?25l"); // Hide cursor to prevent flicker
4546
this.#interval = setInterval(() => {
4647
this.#printProgress();
4748
}, 100);
@@ -71,6 +72,9 @@ export class DynamicTask implements TrackActionTask {
7172
this.#interval = undefined;
7273
}
7374
this.#clear();
75+
if (this.#isTTY) {
76+
this.#stream.write("\x1B[?25h"); // Show cursor
77+
}
7478
this.#stream.write(`${StatusIcons[status]} ${this.#message}\n`);
7579
}
7680

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,53 @@ protected override string BuildNamespace() => string.IsNullOrEmpty(_inputModel.N
187187

188188
protected override CSharpType? BuildBaseType()
189189
{
190-
return BaseModelProvider?.Type;
190+
if (CustomCodeView?.BaseType != null)
191+
{
192+
var customBase = CustomCodeView.BaseType;
193+
194+
// If the custom base type doesn't have a resolved namespace, then try to resolve it from the input model map.
195+
// This will happen if a model is customized to inherit from another generated model, but that generated model
196+
// was not also defined in custom code so Roslyn does not recognize it.
197+
if (string.IsNullOrEmpty(customBase.Namespace))
198+
{
199+
if (CodeModelGenerator.Instance.TypeFactory.TypeProvidersByName.TryGetValue(
200+
customBase.Name, out var resolvedProvider) &&
201+
resolvedProvider is ModelProvider resolvedModel)
202+
{
203+
return resolvedModel.Type;
204+
}
205+
206+
// Force-create all input models so that visitors run (which may rename models
207+
// via TypeProvider.Update) and TypeProvidersByName is fully populated.
208+
foreach (var model in CodeModelGenerator.Instance.InputLibrary.InputNamespace.Models)
209+
{
210+
CodeModelGenerator.Instance.TypeFactory.CreateModel(model);
211+
}
212+
213+
if (CodeModelGenerator.Instance.TypeFactory.TypeProvidersByName.TryGetValue(
214+
customBase.Name, out resolvedProvider) &&
215+
resolvedProvider is ModelProvider resolvedAfterCreate)
216+
{
217+
return resolvedAfterCreate.Type;
218+
}
219+
}
220+
221+
if (CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(
222+
customBase, out var mappedProvider) &&
223+
mappedProvider is ModelProvider mappedModel)
224+
{
225+
return mappedModel.Type;
226+
}
227+
228+
return customBase;
229+
}
230+
231+
if (_inputModel.BaseModel == null)
232+
{
233+
return null;
234+
}
235+
236+
return CodeModelGenerator.Instance.TypeFactory.CreateModel(_inputModel.BaseModel)?.Type;
191237
}
192238

193239
protected override TypeProvider[] BuildSerializationProviders()
@@ -293,63 +339,16 @@ private static bool IsDiscriminator(InputProperty property)
293339

294340
private ModelProvider? BuildBaseModelProvider()
295341
{
296-
// consider models that have been customized to inherit from a different generated model
297-
if (CustomCodeView?.BaseType != null)
298-
{
299-
var baseType = CustomCodeView.BaseType;
300-
301-
// If the custom base type doesn't have a resolved namespace, then try to resolve it from the input model map.
302-
// This will happen if a model is customized to inherit from another generated model, but that generated model
303-
// was not also defined in custom code so Roslyn does not recognize it.
304-
if (string.IsNullOrEmpty(baseType.Namespace))
305-
{
306-
// Cheap check: the base model may already be created and registered under the right name.
307-
if (CodeModelGenerator.Instance.TypeFactory.TypeProvidersByName.TryGetValue(
308-
baseType.Name, out var resolvedProvider) &&
309-
resolvedProvider is ModelProvider resolvedModel)
310-
{
311-
return resolvedModel;
312-
}
313-
314-
// Force-create all input models so that visitors run (which may rename models
315-
// via TypeProvider.Update) and TypeProvidersByName is fully populated.
316-
// This is a no-op for models that have already been created.
317-
foreach (var model in CodeModelGenerator.Instance.InputLibrary.InputNamespace.Models)
318-
{
319-
CodeModelGenerator.Instance.TypeFactory.CreateModel(model);
320-
}
321-
322-
if (CodeModelGenerator.Instance.TypeFactory.TypeProvidersByName.TryGetValue(
323-
baseType.Name, out resolvedProvider) &&
324-
resolvedProvider is ModelProvider resolvedAfterCreate)
325-
{
326-
return resolvedAfterCreate;
327-
}
328-
}
329-
330-
// Try to find the base type in the CSharpTypeMap
331-
if (baseType != null && CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(
332-
baseType,
333-
out var customBaseType) &&
334-
customBaseType is ModelProvider customBaseModel)
335-
{
336-
return customBaseModel;
337-
}
338-
339-
// If the custom base type has a namespace (external type), we don't return it here
340-
// as it's handled by BuildBaseTypeProvider() which returns a TypeProvider
341-
if (!string.IsNullOrEmpty(baseType?.Namespace))
342-
{
343-
return null;
344-
}
345-
}
346-
347-
if (_inputModel.BaseModel == null)
342+
var baseType = BaseType;
343+
if (baseType is null)
348344
{
349345
return null;
350346
}
351347

352-
return CodeModelGenerator.Instance.TypeFactory.CreateModel(_inputModel.BaseModel);
348+
return CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(baseType, out var provider)
349+
&& provider is ModelProvider modelProvider
350+
? modelProvider
351+
: null;
353352
}
354353

355354
private List<FieldProvider> BuildAdditionalPropertyFields()

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,107 @@ public void BuildBaseType()
422422
Assert.AreEqual(baseModel!.Type, derivedModel!.Type.BaseType);
423423
}
424424

425+
[Test]
426+
public void OverridingBuildBaseType_AutoResolvesBaseModelProviderForGeneratedModel()
427+
{
428+
var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []);
429+
var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: []);
430+
ModelProvider? baseProvider = null;
431+
MockHelpers.LoadMockGenerator(createModelCore: input =>
432+
{
433+
if (input == inputBase)
434+
{
435+
return baseProvider = new ModelProvider(input);
436+
}
437+
if (input == inputDerived)
438+
{
439+
return new BuildBaseTypeOverridingModelProvider(input, baseProvider!.Type);
440+
}
441+
return null;
442+
});
443+
444+
var actualBase = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase);
445+
var actualDerived = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputDerived);
446+
447+
Assert.IsNotNull(actualBase);
448+
Assert.IsNotNull(actualDerived);
449+
Assert.AreEqual(actualBase!.Type, actualDerived!.BaseType);
450+
Assert.AreSame(actualBase, actualDerived.BaseModelProvider);
451+
}
452+
453+
[Test]
454+
public void OverridingBuildBaseType_AutoResolvesBaseModelProviderToNullForFrameworkType()
455+
{
456+
var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: []);
457+
var frameworkBase = new CSharpType(typeof(InvalidOperationException));
458+
MockHelpers.LoadMockGenerator(createModelCore: input =>
459+
input == inputDerived ? new BuildBaseTypeOverridingModelProvider(input, frameworkBase) : null);
460+
461+
var actualDerived = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputDerived);
462+
463+
Assert.IsNotNull(actualDerived);
464+
Assert.AreEqual(frameworkBase, actualDerived!.BaseType);
465+
Assert.IsNull(actualDerived.BaseModelProvider);
466+
}
467+
468+
[Test]
469+
public void BaseModelProvider_DefaultResolvesViaCSharpTypeMap()
470+
{
471+
var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []);
472+
var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase);
473+
474+
var derivedProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputDerived);
475+
Assert.IsNotNull(derivedProvider);
476+
Assert.IsNotNull(derivedProvider!.BaseModelProvider);
477+
Assert.AreEqual(derivedProvider.BaseModelProvider!.Type, derivedProvider.BaseType);
478+
}
479+
480+
[Test]
481+
public void BaseModelProvider_NullWhenNoBase()
482+
{
483+
var inputModel = InputFactory.Model("standaloneModel", usage: InputModelTypeUsage.Input, properties: []);
484+
var modelProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel);
485+
486+
Assert.IsNotNull(modelProvider);
487+
Assert.IsNull(modelProvider!.BaseType);
488+
Assert.IsNull(modelProvider.BaseModelProvider);
489+
}
490+
491+
[Test]
492+
public void OverridingBuildBaseType_AutoResolvesBaseModelProviderToNullForNonModelTypeProvider()
493+
{
494+
var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: []);
495+
var nonModelTypeProvider = new NonModelTypeProvider();
496+
MockHelpers.LoadMockGenerator(createModelCore: input =>
497+
input == inputDerived ? new BuildBaseTypeOverridingModelProvider(input, nonModelTypeProvider.Type) : null);
498+
CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap[nonModelTypeProvider.Type] = nonModelTypeProvider;
499+
500+
var actualDerived = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputDerived);
501+
502+
Assert.IsNotNull(actualDerived);
503+
Assert.AreEqual(nonModelTypeProvider.Type, actualDerived!.BaseType);
504+
Assert.IsNull(actualDerived.BaseModelProvider);
505+
}
506+
507+
private class NonModelTypeProvider : TypeProvider
508+
{
509+
protected override string BuildRelativeFilePath() => ".";
510+
protected override string BuildName() => "NonModelBase";
511+
protected override string BuildNamespace() => "Custom.Namespace";
512+
}
513+
514+
private class BuildBaseTypeOverridingModelProvider : ModelProvider
515+
{
516+
private readonly CSharpType? _redirectedBaseType;
517+
518+
public BuildBaseTypeOverridingModelProvider(InputModelType inputModel, CSharpType? redirectedBaseType) : base(inputModel)
519+
{
520+
_redirectedBaseType = redirectedBaseType;
521+
}
522+
523+
protected override CSharpType? BuildBaseType() => _redirectedBaseType;
524+
}
525+
425526
[Test]
426527
public void BuildModelAsStruct()
427528
{

packages/protobuf/src/ast.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,10 @@ export interface ProtoFieldDeclaration extends ProtoDeclarationCommon {
262262
* Whether or not the field is repeated (i.e. an array).
263263
*/
264264
repeated?: boolean;
265+
/**
266+
* Whether or not the field uses the proto3 `optional` label.
267+
*/
268+
optional?: boolean;
265269
options?: Partial<DefaultFieldOptions>;
266270
type: ProtoType;
267271
index: number;

packages/protobuf/src/lib.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,20 @@ export const TypeSpecProtobufLibrary = createTypeSpecLibrary({
112112
union: "a message field's type may not be a union",
113113
},
114114
},
115+
"optional-array-field": {
116+
severity: "warning",
117+
messages: {
118+
default:
119+
"optional array fields cannot preserve unset versus empty in protobuf; emitting a repeated field without the 'optional' label",
120+
},
121+
},
122+
"optional-map-field": {
123+
severity: "warning",
124+
messages: {
125+
default:
126+
"optional map fields cannot preserve unset versus empty in protobuf; emitting a map field without the 'optional' label",
127+
},
128+
},
115129
"namespace-collision": {
116130
severity: "error",
117131
messages: {

packages/protobuf/src/transform/index.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,10 +832,47 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P
832832

833833
// Determine if the property type is an array
834834
if (isArray(property.type)) field.repeated = true;
835+
field.optional = shouldEmitOptionalLabel(property);
835836

836837
return field;
837838
}
838839

840+
function shouldEmitOptionalLabel(property: ModelProperty): boolean {
841+
if (!property.optional) {
842+
return false;
843+
}
844+
845+
if (isArray(property.type)) {
846+
reportDiagnostic.once(program, {
847+
code: "optional-array-field",
848+
format: {},
849+
target: property,
850+
});
851+
return false;
852+
}
853+
854+
if (isMap(program, property.type)) {
855+
reportDiagnostic.once(program, {
856+
code: "optional-map-field",
857+
format: {},
858+
target: property,
859+
});
860+
return false;
861+
}
862+
863+
switch (property.type.kind) {
864+
case "Scalar":
865+
case "Enum":
866+
return true;
867+
case "Intrinsic":
868+
case "Model":
869+
case "Union":
870+
return false;
871+
default:
872+
return false;
873+
}
874+
}
875+
839876
/**
840877
* @param e - the Enum to convert
841878
* @returns a corresponding protobuf enum declaration

packages/protobuf/src/write.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ function writeVariant(decl: ProtoEnumVariantDeclaration, indentLevel: number): I
178178
}
179179

180180
function writeField(decl: ProtoFieldDeclaration, indentLevel: number): Iterable<string> {
181-
const prefix = decl.repeated ? "repeated " : "";
181+
const prefix = decl.repeated ? "repeated " : decl.optional ? "optional " : "";
182182
const output = prefix + `${writeType(decl.type)} ${decl.name} = ${decl.index};`;
183183

184184
return writeDocumentationCommentFlexible(decl, output, indentLevel);
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
/test/main.tsp:18:13 - warning @typespec/protobuf/optional-array-field: optional array fields cannot preserve unset versus empty in protobuf; emitting a repeated field without the 'optional' label
2+
/test/main.tsp:19:13 - warning @typespec/protobuf/optional-map-field: optional map fields cannot preserve unset versus empty in protobuf; emitting a map field without the 'optional' label

0 commit comments

Comments
 (0)