Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…ndpoint_design
  • Loading branch information
iscai-msft committed Mar 22, 2024
2 parents 85a97ac + 3b30c1e commit d0ff3e8
Show file tree
Hide file tree
Showing 25 changed files with 289 additions and 242 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: internal
packages:
- "@azure-tools/typespec-autorest"
---

Cleanup autorest diagnostics
8 changes: 8 additions & 0 deletions .chronus/changes/enable-no-enum-rule-2024-2-21-16-42-36.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: feature
packages:
- "@azure-tools/typespec-azure-core"
---

Enable `no-enum` rule by default in `all` ruleset
7 changes: 7 additions & 0 deletions .chronus/changes/enum_fixed-2024-2-21-15-54-49.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: breaking
packages:
- "@azure-tools/typespec-client-generator-core"
---

enums are always fixed after we switch to use union to represent extensible enum
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-azure-core"
---

`no-enum` rule codefix now convert to named variant when the enum had not values (e.g. `enum E {a, b}`)
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: internal
packages:
- "@azure-tools/typespec-azure-resource-manager"
---

Ensure `arm-post-response-codes` rule is in the default ruleset.
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"
---

Update `property-name-conflict` linter rule to stop looking and recommending `@projectedName` in favor of `@clientName`
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
* @bterlson @daviwil @markcowl @allenjzhang @timotheeguerin
* @bterlson @markcowl @allenjzhang @timotheeguerin

/packages/typespec-client-generator-core @lmazuel @m-nash @iscai-msft @srnagar @joheredi

Expand Down
5 changes: 5 additions & 0 deletions docs/howtos/DataPlane Generation - DPG/04renaming.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import TabItem from "@theme/TabItem";

This page documents how to customize names for client generations in DPG. For an overview of the setup, please visit the setup page.

:::note
The TypeSpec compiler provides an `@encodedName` decorator that allows changing the name of the property for a given serialization format.
However in Azure we recommend that you define the property name as the value sent on the wire and use the `@clientName` decorator to change the name of the generated property.
:::

## Default behaviors

By default, any language code generator will assume the TYPESPEC name is the client. For clarity, generators do not attempt to do any auto-magic rename.
Expand Down
2 changes: 1 addition & 1 deletion docs/libraries/azure-core/reference/linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-core/no-multiple-discriminator` | Classes should have at most one discriminator. |
| `@azure-tools/typespec-azure-core/no-rest-library-interfaces` | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. |
| `@azure-tools/typespec-azure-core/no-unknown` | Azure services must not have properties of type `unknown`. |
| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts. |
| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts between a property and a model of the same name. |
| `@azure-tools/typespec-azure-core/bad-record-type` | Identify bad record definitions. |
| `@azure-tools/typespec-azure-core/documentation-required` | Require documentation over enums, models, and operations. |
| `@azure-tools/typespec-azure-core/key-visibility-required` | Key properties need to have an explicit visibility setting. |
Expand Down
1 change: 1 addition & 0 deletions docs/libraries/azure-resource-manager/reference/linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/arm-common-types-version` | Specify the ARM common-types version using @armCommonTypesVersion. |
| [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](/libraries/azure-resource-manager/rules/delete-operation-response-codes.md) | Ensure delete operations have the appropriate status codes. |
| [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](/libraries/azure-resource-manager/rules/put-operation-response-codes.md) | Ensure put operations have the appropriate status codes. |
| [`@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes`](/libraries/azure-resource-manager/rules/post-operation-response-codes.md) | Ensure post operations have the appropriate status codes. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment` | `@armResourceAction` should not be used with `@segment`. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property` | Warn about duplicate properties in resources. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. |
Expand Down
36 changes: 0 additions & 36 deletions packages/typespec-autorest/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,24 +173,6 @@ const EmitterOptionsSchema: JSONSchemaType<AutorestEmitterOptions> = {
const libDef = {
name: "@azure-tools/typespec-autorest",
diagnostics: {
"security-service-namespace": {
severity: "error",
messages: {
default: "Cannot add security details to a namespace other than the service namespace.",
},
},
"resource-namespace": {
severity: "error",
messages: {
default: "Resource goes on namespace",
},
},
"missing-path-param": {
severity: "error",
messages: {
default: paramMessage`Path contains parameter ${"param"} but wasn't found in given parameters`,
},
},
"duplicate-body-types": {
severity: "error",
messages: {
Expand Down Expand Up @@ -242,24 +224,6 @@ const libDef = {
default: paramMessage`Invalid type '${"type"}' for a default value`,
},
},
"invalid-property-type-for-collection-format": {
severity: "error",
messages: {
default: "The collectionFormat can only be applied to model property with type 'string[]'.",
},
},
"invalid-collection-format": {
severity: "error",
messages: {
default: "The format should be one of 'csv','ssv','tsv','pipes' and 'multi'.",
},
},
"non-recommended-collection-format": {
severity: "warning",
messages: {
default: "The recommendation of collection format are 'csv' and 'multi'.",
},
},
"invalid-multi-collection-format": {
severity: "error",
messages: {
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-azure-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-core/no-multiple-discriminator` | Classes should have at most one discriminator. |
| `@azure-tools/typespec-azure-core/no-rest-library-interfaces` | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. |
| `@azure-tools/typespec-azure-core/no-unknown` | Azure services must not have properties of type `unknown`. |
| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts. |
| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts between a property and a model of the same name. |
| `@azure-tools/typespec-azure-core/bad-record-type` | Identify bad record definitions. |
| `@azure-tools/typespec-azure-core/documentation-required` | Require documentation over enums, models, and operations. |
| `@azure-tools/typespec-azure-core/key-visibility-required` | Key properties need to have an explicit visibility setting. |
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-azure-core/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export const $linter = defineLinter({
true,
[`@azure-tools/typespec-azure-core/${useStandardNames.name}`]: true,
[`@azure-tools/typespec-azure-core/${friendlyNameRule.name}`]: true,
[`@azure-tools/typespec-azure-core/${noEnumRule.name}`]: false,
[`@azure-tools/typespec-azure-core/${noEnumRule.name}`]: true,
},
extends: ["@typespec/http/all"],
},
Expand Down
8 changes: 5 additions & 3 deletions packages/typespec-azure-core/src/rules/no-enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ export const noEnumRule = createRule({
create(context) {
return {
enum: (en: Enum) => {
if (getVersionsForEnum(context.program, en).length > 0) {
const [_, versions] = getVersionsForEnum(context.program, en);
if (versions !== undefined && versions.getVersions()[0].enumMember.enum === en) {
return;
}

context.reportDiagnostic({
format: { enumName: en.name },
target: en,
Expand All @@ -48,7 +50,7 @@ function createEnumToExtensibleUnionCodeFix(en: Enum): CodeFix {
? node.value.value
: `"${node.value.value}"`
}`
: `"${node.id.sv}"`;
: `${node.id.sv}: "${node.id.sv}"`;
}
}

Expand Down Expand Up @@ -131,7 +133,7 @@ function getNodeAnnotations(node: Node): string {
}

for (let i = endOfTrivia; i < node.end; i++) {
if (source[i] === " ") {
if (source[i] === " " || source[i] === "\n") {
endOfTrivia = i + 1;
} else {
break;
Expand Down
22 changes: 4 additions & 18 deletions packages/typespec-azure-core/src/rules/property-naming.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,20 @@
import { ModelProperty, createRule, getProjectedNames, paramMessage } from "@typespec/compiler";
import { ModelProperty, createRule, paramMessage } from "@typespec/compiler";
import { isExcludedCoreType } from "./utils.js";

export const propertyNameRule = createRule({
name: "property-name-conflict",
description: "Avoid naming conflicts.",
description: "Avoid naming conflicts between a property and a model of the same name.",
severity: "warning",
messages: {
default: paramMessage`Property '${"propertyName"}' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`,
projectedName: paramMessage`Use of @projectedName on property '${"propertyName"}' results in '${"propertyName"}' having the same name as its enclosing type in C#. Please use a different @projectedName value.`,
default: paramMessage`Property '${"propertyName"}' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`,
},
create(context) {
return {
modelProperty: (property: ModelProperty) => {
if (isExcludedCoreType(context.program, property)) return;
const projectedNames = getProjectedNames(context.program, property);
const modelName = property.model?.name.toLocaleLowerCase();
const propertyName = property.name.toLocaleLowerCase();
const csharpProjection = projectedNames?.get("csharp")?.toLocaleLowerCase();
const clientProjection = projectedNames?.get("client")?.toLocaleLowerCase();
if (
csharpProjection === modelName || // csharp projection conflicts with model name
(clientProjection === modelName && !csharpProjection) // client projection conflicts with model name and there is no csharp projection
) {
context.reportDiagnostic({
messageId: "projectedName",
format: { propertyName: property.name },
target: property,
});
} else if (propertyName === modelName && !(csharpProjection || clientProjection)) {
// warning if the property name conflicts with the model name and there is no csharp or client projected name
if (propertyName === modelName) {
context.reportDiagnostic({
format: { propertyName: property.name },
target: property,
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-azure-core/src/rules/require-docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const requireDocumentation = createRule({
description: "Require documentation over enums, models, and operations.",
severity: "warning",
messages: {
default: paramMessage`The ${"kind"} named '${"name"}' should have a documentation or description, please use decorator @doc to add it.`,
default: paramMessage`The ${"kind"} named '${"name"}' should have a documentation or description, use doc comment /** */ to provide it.`,
},
create(context) {
return {
Expand Down
52 changes: 49 additions & 3 deletions packages/typespec-azure-core/test/rules/no-enum.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe("typespec-azure-core: no-enum rule", () => {
},
]);
});

it("allows the version enum", async () => {
await tester
.expect(
Expand All @@ -46,6 +47,27 @@ describe("typespec-azure-core: no-enum rule", () => {
.toBeValid();
});

it("emit warning about other enums in versioned service", async () => {
await tester
.expect(
`
@service
@versioned(Versions)
namespace Foo;
enum Versions {
v1, v2
}
enum Bar { a, b}
`
)
.toEmitDiagnostics([
{
code: "@azure-tools/typespec-azure-core/no-enum",
},
]);
});

describe("codefix", () => {
it("codefix simple enum", async () => {
await tester
Expand All @@ -60,7 +82,31 @@ describe("typespec-azure-core: no-enum rule", () => {
union PetKind {
string,
"cat", "dog",
cat: "cat", dog: "dog",
}
`);
});

it("keeps new lines", async () => {
await tester
.expect(
`
enum PetKind {
/** cat doc */
cat,
/** dog doc */
dog
}
`
)
.applyCodeFix("enum-to-extensible-union").toEqual(`
union PetKind {
string,
/** cat doc */
cat: "cat",
/** dog doc */
dog: "dog",
}
`);
});
Expand Down Expand Up @@ -115,14 +161,14 @@ describe("typespec-azure-core: no-enum rule", () => {
/** cat */
@doc("cat")
#suppress "cat"
"cat",
cat: "cat",
// dog
/** dog */
@doc("dog")
#suppress "dog"
"dog",
dog: "dog",
// end
}
Expand Down
Loading

0 comments on commit d0ff3e8

Please sign in to comment.