Skip to content

Commit

Permalink
Fix crash when Traits builders gets passed non model (Azure#694)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheeguerin authored May 1, 2024
1 parent 2ff99d0 commit 7a86203
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 9 deletions.
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: fix
packages:
- "@azure-tools/typespec-azure-core"
---

Fix crash when `Traits` builders gets passed non model
2 changes: 1 addition & 1 deletion packages/typespec-azure-core/lib/operations.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ alias ExpectedResourceOperationTraits = [
*/
@ensureTraitsPresent(InterfaceTraits, ExpectedResourceOperationTraits)
interface ResourceOperations<
InterfaceTraits,
InterfaceTraits extends TypeSpec.Reflection.Model,
ErrorResponse = Azure.Core.Foundations.ErrorResponse
> {
/**
Expand Down
15 changes: 8 additions & 7 deletions packages/typespec-azure-core/lib/traits.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ model TraitOverride<Trait> {}
@trait
@added(Azure.Core.Versions.v1_0_Preview_2)
@Private.ensureAllQueryParams(Parameters)
model QueryParametersTrait<Parameters, Contexts = unknown> {
model QueryParametersTrait<Parameters extends Model, Contexts = unknown> {
@traitContext(Contexts)
queryParams: {
@traitLocation(TraitLocation.Parameters)
Expand All @@ -79,7 +79,8 @@ model QueryParametersTrait<Parameters, Contexts = unknown> {
*/
@trait
@added(Azure.Core.Versions.v1_0_Preview_2)
model ListQueryParametersTrait<Parameters> is QueryParametersTrait<Parameters, TraitContext.List>;
model ListQueryParametersTrait<Parameters extends Model>
is QueryParametersTrait<Parameters, TraitContext.List>;

/**
* Declares a trait that is applied as a request header parameter.
Expand All @@ -89,7 +90,7 @@ model ListQueryParametersTrait<Parameters> is QueryParametersTrait<Parameters, T
@trait
@added(Azure.Core.Versions.v1_0_Preview_2)
@Private.ensureAllHeaderParams(Headers)
model RequestHeadersTrait<Headers, Contexts = unknown> {
model RequestHeadersTrait<Headers extends Model, Contexts = unknown> {
@traitContext(Contexts)
requestHeaders: {
@traitLocation(TraitLocation.Parameters)
Expand All @@ -105,7 +106,7 @@ model RequestHeadersTrait<Headers, Contexts = unknown> {
@trait
@added(Azure.Core.Versions.v1_0_Preview_2)
@Private.ensureAllHeaderParams(Headers)
model ResponseHeadersTrait<Headers, Contexts = unknown> {
model ResponseHeadersTrait<Headers extends Model, Contexts = unknown> {
@traitContext(Contexts)
responseHeaders: {
@traitLocation(TraitLocation.Response)
Expand Down Expand Up @@ -311,7 +312,7 @@ namespace Azure {
*/
extern dec ensureTraitsPresent(
target: Interface | TypeSpec.Reflection.Operation,
traitModel: unknown,
traitModel: Model,
expectedTraits: ExpectedTrait[]
);

Expand All @@ -322,7 +323,7 @@ namespace Azure {
* @param target The model type where this check will be established.
* @param paramModel The actual model type to check for query parameters.
*/
extern dec ensureAllQueryParams(target: unknown, paramModel: unknown);
extern dec ensureAllQueryParams(target: unknown, paramModel: Model);

/**
* `@ensureAllHeaderParams` checks the properties of `paramModel` to ensure they all are marked
Expand All @@ -331,7 +332,7 @@ namespace Azure {
* @param target The model type where this check will be established.
* @param paramModel The actual model type to check for header properties.
*/
extern dec ensureAllHeaderParams(target: unknown, paramModel: unknown);
extern dec ensureAllHeaderParams(target: unknown, paramModel: Model);
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions packages/typespec-azure-core/test/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2897,4 +2897,23 @@ describe("typespec-azure-core: operation templates", () => {
]);
});
});

// Regression test for https://github.com/Azure/typespec-azure/issues/332
it("doesn't crash when passing non model to ServiceTraits", async () => {
const [_, diagnostics] = await getOperations(`
alias Operations = Azure.Core.ResourceOperations<abc>;
`);
expectDiagnostics(
diagnostics.filter((x) => x.severity === "error"),
[
{
code: "unknown-identifier",
message: "Unknown identifier abc",
},
{
code: "unassignable",
},
]
);
});
});
21 changes: 20 additions & 1 deletion packages/typespec-azure-core/test/traits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ describe("typespec-azure-core: service traits", () => {
{ trait: "MissingTrait", diagnostic: "conditional-requests-trait-missing" },
{ trait: "AnotherMissingTrait", diagnostic: "repeatable-requests-trait-missing" }
])
interface Operations<Traits> {}
interface Operations<Traits extends Reflection.Model> {}
interface Usage extends Operations<ContextTrait & NoContextTrait> {}
`);
Expand Down Expand Up @@ -302,6 +302,16 @@ describe("typespec-azure-core: service traits", () => {
},
]);
});

it("emits a diagnostic when using non model", async () => {
const diagnostics = await runner.diagnose(`
alias Test = Azure.Core.Traits.QueryParametersTrait<"abc">;
`);

expectDiagnostics(diagnostics, {
code: "unassignable",
});
});
});

describe("@ensureAllHeaderParams", () => {
Expand All @@ -326,5 +336,14 @@ describe("typespec-azure-core: service traits", () => {
},
]);
});
it("emits a diagnostic when using non model", async () => {
const diagnostics = await runner.diagnose(`
alias Test = Azure.Core.Traits.RequestHeadersTrait<"abc">;
`);

expectDiagnostics(diagnostics, {
code: "unassignable",
});
});
});
});

0 comments on commit 7a86203

Please sign in to comment.