diff --git a/packages/typespec-lintdiff/src/rules/consistent-patch-properties.ts b/packages/typespec-lintdiff/src/rules/consistent-patch-properties.ts index 7b2ff46da5..b772cbb256 100644 --- a/packages/typespec-lintdiff/src/rules/consistent-patch-properties.ts +++ b/packages/typespec-lintdiff/src/rules/consistent-patch-properties.ts @@ -1,9 +1,4 @@ import { resolveProviderNamespace } from "@azure-tools/typespec-azure-resource-manager"; -import { - createTCGCContext, - isInScope, - type TCGCContext, -} from "@azure-tools/typespec-client-generator-core"; import { createRule, getDiscriminator, @@ -13,6 +8,7 @@ import { resolveEncodedName, type Model, type ModelProperty, + type Program, type Type, } from "@typespec/compiler"; import { getAllHttpServices, type HttpOperation, type HttpOperationResponse } from "@typespec/http"; @@ -28,7 +24,6 @@ export const consistentPatchPropertiesRule = createRule({ create(context) { return { root: () => { - const emitterContext = createTCGCContext(context.program, "@azure-tools/typespec-autorest"); const [services] = getAllHttpServices(context.program); for (const service of services) { if (resolveProviderNamespace(context.program, service.namespace) === undefined) { @@ -36,15 +31,12 @@ export const consistentPatchPropertiesRule = createRule({ } for (const httpOperation of service.operations) { - if ( - httpOperation.verb !== "patch" || - !isInScope(emitterContext, httpOperation.operation) - ) { + if (httpOperation.verb !== "patch") { continue; } const patchBody = getObjectModel(httpOperation.parameters.body?.type); - const resourceType = getResourceType(emitterContext, httpOperation, service.operations); + const resourceType = getResourceType(httpOperation, service.operations); if (patchBody === undefined) { continue; } @@ -55,10 +47,10 @@ export const consistentPatchPropertiesRule = createRule({ const resourceModel = getObjectModel(resourceType); const invalidProperties = resourceModel === undefined - ? [...getPayloadProperties(emitterContext, patchBody)].map( + ? [...getPayloadProperties(context.program, patchBody)].map( ([jsonName, property]) => ({ path: [jsonName], target: property.target }), ) - : findInvalidPatchProperties(emitterContext, patchBody, resourceModel); + : findInvalidPatchProperties(context.program, patchBody, resourceModel); for (const invalidProperty of invalidProperties) { context.reportDiagnostic({ @@ -76,15 +68,11 @@ export const consistentPatchPropertiesRule = createRule({ }); function getResourceType( - emitterContext: TCGCContext, patchOperation: HttpOperation, operations: HttpOperation[], ): Type | undefined { const getOperation = operations.find( - (operation) => - operation.verb === "get" && - operation.path === patchOperation.path && - isInScope(emitterContext, operation.operation), + (operation) => operation.verb === "get" && operation.path === patchOperation.path, ); return ( @@ -112,7 +100,7 @@ function getResponseBodyType( } function findInvalidPatchProperties( - emitterContext: TCGCContext, + program: Program, patchModel: Model, resourceModel: Model, path: string[] = [], @@ -129,15 +117,15 @@ function findInvalidPatchProperties( } const invalidProperties: Array<{ path: string[]; target: Model | ModelProperty }> = []; - const resourceProperties = getPayloadProperties(emitterContext, resourceModel); + const resourceProperties = getPayloadProperties(program, resourceModel); - for (const [jsonName, patchProperty] of getPayloadProperties(emitterContext, patchModel)) { + for (const [jsonName, patchProperty] of getPayloadProperties(program, patchModel)) { const currentPath = [...path, jsonName]; const resourceProperty = resourceProperties.get(jsonName); if (resourceProperty === undefined) { invalidProperties.push( - ...collectPropertyPaths(emitterContext, patchProperty, currentPath, new Set()), + ...collectPropertyPaths(program, patchProperty, currentPath, new Set()), ); continue; } @@ -148,7 +136,7 @@ function findInvalidPatchProperties( if (resourcePropertyModel !== undefined) { invalidProperties.push( ...findInvalidPatchProperties( - emitterContext, + program, patchPropertyModel, resourcePropertyModel, currentPath, @@ -157,7 +145,7 @@ function findInvalidPatchProperties( ); } else { invalidProperties.push( - ...collectNestedPropertyPaths(emitterContext, patchPropertyModel, currentPath), + ...collectNestedPropertyPaths(program, patchPropertyModel, currentPath), ); } } @@ -168,18 +156,18 @@ function findInvalidPatchProperties( } function collectNestedPropertyPaths( - emitterContext: TCGCContext, + program: Program, model: Model, path: string[], ): Array<{ path: string[]; target: Model | ModelProperty }> { const visited = new Set([model]); - return [...getPayloadProperties(emitterContext, model)].flatMap(([jsonName, property]) => - collectPropertyPaths(emitterContext, property, [...path, jsonName], visited), + return [...getPayloadProperties(program, model)].flatMap(([jsonName, property]) => + collectPropertyPaths(program, property, [...path, jsonName], visited), ); } function collectPropertyPaths( - emitterContext: TCGCContext, + program: Program, property: PayloadProperty, path: string[], visited: Set, @@ -194,13 +182,13 @@ function collectPropertyPaths( } visited.add(propertyModel); - const nestedProperties = getPayloadProperties(emitterContext, propertyModel); + const nestedProperties = getPayloadProperties(program, propertyModel); if (nestedProperties.size === 0) { return [{ path, target: property.target }]; } const invalidProperties = [...nestedProperties].flatMap(([jsonName, nestedProperty]) => - collectPropertyPaths(emitterContext, nestedProperty, [...path, jsonName], visited), + collectPropertyPaths(program, nestedProperty, [...path, jsonName], visited), ); visited.delete(propertyModel); return invalidProperties; @@ -211,25 +199,18 @@ interface PayloadProperty { type?: Type; } -function getPayloadProperties( - emitterContext: TCGCContext, - model: Model, -): Map { +function getPayloadProperties(program: Program, model: Model): Map { const properties = new Map(); for (let current: Model | undefined = model; current !== undefined; current = current.baseModel) { for (const property of current.properties.values()) { - const jsonName = resolveEncodedName(emitterContext.program, property, "application/json"); - if ( - !properties.has(jsonName) && - !isNeverType(property.type) && - isInScope(emitterContext, property) - ) { + const jsonName = resolveEncodedName(program, property, "application/json"); + if (!properties.has(jsonName) && !isNeverType(property.type)) { properties.set(jsonName, { target: property, type: property.type }); } } - const discriminator = getDiscriminator(emitterContext.program, current); + const discriminator = getDiscriminator(program, current); if ( discriminator !== undefined && !current.properties.has(discriminator.propertyName) && diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/corpus-evidence.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/corpus-evidence.json index a05927a084..18badd6ea4 100644 --- a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/corpus-evidence.json +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/corpus-evidence.json @@ -1,25 +1,64 @@ { - "schemaVersion": 1, + "schemaVersion": 2, "rule": "ConsistentPatchProperties", "specsCommit": "f6b53f105b95da05276530a0754a1c71b4f16397", - "generatedAt": "2026-09-04T14:39:11.685Z", + "generatedAt": "2026-09-09T09:24:28.545Z", + "completedAt": "2026-09-09T17:28:59.7719922+08:00", + "sourceBaseCommit": "29c4a87b0799092be0ede854ffdf23a0a9648795", + "sourceBranch": "feature/lintdiff-consistent-patch-properties-native", + "sourceState": "Uncommitted native-boundary repair, before final formatting", + "localLinterFingerprint": "sha256:33b2527404c4fb88cc32a37f2dd7cb1d58b17789308bdce020e23635019bd6f1", + "coverageKind": "partial", "command": "pnpm --dir packages/typespec-lintdiff specs:typespec --specs-repo C:\\dev\\worktrees\\azure-rest-api-specs-lintdiff-consistent-patch-properties --concurrency 6", "fullRun": true, - "durationMs": 1261930, + "durationMs": 4678050, "sourceProjectCount": 468, "successfulProjectCount": 462, "failedProjectCount": 6, "validator": { "diagnostics": 151, "projects": 27, - "fileIndependentIdentities": 48 + "fileIndependentIdentities": 48, + "fileAndPathIdentities": 48 }, "typespec": { "diagnostics": 325, "sourceIdentities": 188, - "projects": 32, - "selectedVersionDiagnostics": 306, - "selectedVersionProjects": 28 + "projects": 32 + }, + "afterOneSidedVersionExclusions": { + "diagnostics": 306, + "projects": 28, + "excludedDiagnostics": 19, + "globallyProjected": false, + "scope": "Remove only the documented findings in the four older-version-only projects" + }, + "rawCountComparison": { + "equalOverlapProjects": 18, + "typeSpecHigherOverlapProjects": 9, + "validatorHigherOverlapProjects": 0, + "positiveDifferenceAllProjects": 174, + "negativeDifferenceAllProjects": 0 + }, + "deduplicatedCountComparison": { + "equalProjects": 9, + "typeSpecHigherProjects": 22, + "validatorHigherProjects": 1, + "positiveDifference": 142, + "negativeDifference": 2, + "identities": "Validator project + JSON path; TypeSpec project + source file + line + column" + }, + "oneSidedProjectCounts": { + "Batch": 2, + "Cdn": 1, + "Informatica": 22, + "ManagedNetworkFabric": 15, + "NetApp": 1 + }, + "uncertainty": { + "informatica": "Selected Swagger schema mismatches are verified; the validator-side omission mechanism is not isolated.", + "versions": "The remaining 306 diagnostics are not a globally latest-version-projected population.", + "scope": "Four fixtures prove intentional native/emitter contract differences despite unchanged corpus counts." }, "overlapProjects": [ "specification/apimanagement/resource-manager/Microsoft.ApiManagement/ApiManagement", @@ -62,6 +101,14 @@ "specification/netapp/resource-manager/Microsoft.NetApp/NetApp" ] }, + "compileFailureKinds": { + "DeviceProvisioningServices": "@typespec/http/duplicate-body", + "TenantActionGroups": "@typespec/http/missing-uri-param", + "Network": "@typespec/http/missing-uri-param", + "Quota": "@typespec/http/missing-uri-param", + "deployments": "@typespec/http/duplicate-body", + "ServiceLinker": "@typespec/http/duplicate-body" + }, "failedProjects": [ "specification/deviceprovisioningservices/resource-manager/Microsoft.Devices/DeviceProvisioningServices", "specification/monitor/resource-manager/Microsoft.Insights/Insights/TenantActionGroups", diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/migration.md b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/migration.md index 5e1fe782e5..6edd012244 100644 --- a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/migration.md +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/migration.md @@ -1,36 +1,71 @@ # ConsistentPatchProperties migration evidence -## Conclusion - -The migrated TypeSpec rule is functionally equivalent to the intended Swagger -`ConsistentPatchProperties` behavior over the aligned, successfully compiled -corpus. The final full run covers all 27 validator projects. There are no -validator-only projects. - -**TypeSpec rule update required:** yes. The previous implementation inspected -only registered ARM resource lifecycle updates. The Swagger rule inspects every -ARM PATCH operation, including legacy templates, custom provider operations, -and PATCH actions. The updated rule traverses ARM HTTP PATCH operations, selects -the PATCH response model containing status `200` or `201`, falls back to the -same-path GET response containing `200` or `201`, recursively compares body -properties at the same level, and reports the authored property. TypeSpec HTTP -can represent those statuses either as individual numbers or as members of a -status-code range. - -The five remaining raw TypeSpec-only projects are explained. Nineteen -diagnostics come from older-version declarations absent from the retained -latest Swagger. Informatica also contains genuine selected-version violations -missed by the Swagger validator. Raw diagnostic equality is not required -because Swagger reports emitted operation/schema occurrences while TypeSpec -reports authored properties. +## Result and gap summary + +The September 9 full production run has **151 Swagger diagnostics in 27 +projects versus 325 TypeSpec diagnostics in 32 projects**, over 462 successfully +compiled projects; six failures are excluded from both sides. All 27 validator +projects overlap. Counts are unchanged from September 4. + +The five TypeSpec-only projects contribute 19 removed/renamed-declaration +findings and 22 Informatica findings on selected-version schema mismatches. +The other 133 extra raw findings occur in overlapping projects, where reporting +granularity and repeated source targets differ. + +**Decision: native-boundary repair completed; Swagger equivalence is partial.** +Four scope fixtures prove three TypeSpec-only cases and one validator-only case: +native declarations remain visible when AutoRest omits them. The unchanged +corpus does not exercise away this contract difference. + +**Limits:** Informatica's validator-side omission mechanism remains unisolated. +Version exclusions establish the four one-sided-project exclusions, not a +globally latest-version-projected diagnostic total. + +## Required changes and native contract + +The repair replaces `TCGCContext` with compiler `Program`, removes +`createTCGCContext` and all `isInScope` calls, and preserves the native PATCH +body/response comparison. No emitter, generator context, private decorator +state, or generated OpenAPI is used to decide diagnostics. Compiler APIs still +provide JSON encoded names, inheritance, nullability, and discriminator metadata. + +PATCH `200`, then `201`, then same-path GET `200`/`201` selects the comparison +body. Exact response codes take precedence over a containing range. The lint +recursively compares same-level properties and reports authored targets; this +repair does not change diagnostic granularity, cycle handling, or API-version +policy. + +Directly related changes are the native regression tests, four scope-comparison +fixtures and snapshots (two existing, two new), and partial-coverage metadata. +The ARM provider check remains lintdiff-only isolation in a mixed runner; its +official-library adaptation is separate from this repair. + +The existing official `arm-resource-patch` rule remains only partial coverage: +it checks registered resource PATCH bodies without this recursive same-level +comparison or the complete custom-operation response/fallback selection. +The merged source PR #5399 is historical; this is an explicitly approved +follow-up repair, not a duplicate migration. ## Evidence revisions and populations -| Evidence | Revision and population | `ConsistentPatchProperties` row | -| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [External coverage snapshot](../../../docs/coverage_old.md) | The checked-in snapshot links to its source gist but records no date, spec commit, or generator revision. It reports 450 compiled projects and 210 rules. | `lint`; 303 validator projects; 25 local-lint projects; 0 official projects; 8.3%. Project identities cannot be reconstructed from this aggregate row. | -| [Checked-in observed report](../../../specs/coverage-breakdown.md) | Specs commit `f6b53f105b95da05276530a0754a1c71b4f16397`; 462/468 successfully compiled projects. | Before this change: `production`; 27 validator projects; 27 TypeSpec projects; 23 overlap; 4 validator-only; 4 TypeSpec-only; 151 validator and 122 TypeSpec diagnostics. | -| [Final retained evidence](./corpus-evidence.json) | Full review-fix run generated 2026-09-04 from the same specs commit; 462/468 projects compiled; duration 1,261,930 ms. | 27 validator projects; 32 raw TypeSpec projects; 27 overlap; 0 validator-only; 5 raw TypeSpec-only; 151 validator and 325 raw TypeSpec diagnostics. | +Upstream research uses `azure-openapi-validator` commit +`6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f`: +[implementation](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/packages/rulesets/src/spectral/functions/consistent-patch-properties.ts), +[`diffSchema` and GET lookup](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/packages/rulesets/src/spectral/functions/utils.ts), +[tests](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/packages/rulesets/src/spectral/test/consistent-patch-properties.test.ts), +and [documentation](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/docs/consistent-patch-properties.md). +The installed comparison engine is `@microsoft.azure/openapi-validator-rulesets` +2.2.6. Its resolved ARM selector is `$.paths.*.patch`, using the first body +parameter schema and the response precedence described above. Upstream tests +cover inherited missing properties, a matching subset, and asynchronous GET +fallback. The scope limitations occur before this validator, during emission. + +| Evidence | Revision and population | `ConsistentPatchProperties` row | +| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [External coverage snapshot](../../../docs/coverage_old.md) | The checked-in snapshot links to its source gist but records no date, spec commit, or generator revision. It reports 450 compiled projects and 210 rules. | `lint`; 303 validator projects; 25 local-lint projects; 0 official projects; 8.3%. Project identities cannot be reconstructed from this aggregate row. | +| [Checked-in observed report](../../../specs/coverage-breakdown.md) | Specs commit `f6b53f105b95da05276530a0754a1c71b4f16397`; 462/468 successfully compiled projects. | Before this change: `production`; 27 validator projects; 27 TypeSpec projects; 23 overlap; 4 validator-only; 4 TypeSpec-only; 151 validator and 122 TypeSpec diagnostics. | +| September 4 source evidence (historical) | Full review-fix run from the same specs commit; 462/468 projects compiled; duration 1,261,930 ms. | 27 validator projects; 32 TypeSpec projects; 27 overlap; 0 validator-only; 5 TypeSpec-only; 151 validator and 325 TypeSpec diagnostics. | +| [Final retained repair evidence](./corpus-evidence.json) | Full run generated 2026-09-09T09:24:28.545Z; completed 2026-09-09T17:28:59+08:00; same specs commit; 462/468 compiled; duration 4,678,050 ms. | `production`; `partial` semantic coverage; 27 validator projects; 32 raw TypeSpec projects; 27 overlap; 0 validator-only; 5 raw TypeSpec-only; 151 validator and 325 raw TypeSpec diagnostics. | The external report uses an unidentified older population and aggregate migration credit. The observed reports require same-project diagnostics on the @@ -38,6 +73,16 @@ pinned successful-project population. The final TypeSpec diagnostic count also includes every declared API version; the validator dataset retains one selected version per project. +The repair ran on `feature/lintdiff-consistent-patch-properties-native` from +target commit `29c4a87b0799092be0ede854ffdf23a0a9648795`, with the uncommitted +source repair included. The runner's source fingerprint is retained in +`corpus-evidence.json`. The scope is ARM, production validator execution, +successfully compiled projects only, no readme suppressions on retained +Swagger, and normal source-program TypeSpec diagnostics (including source +suppressions). No new global API-version projection or normalization was added. +The checked-in coverage report is deliberately historical: refreshed canonical +corpus artifacts are not included in this rule-repair PR. + Six compile failures were excluded from both sides: - `specification/deviceprovisioningservices/resource-manager/Microsoft.Devices/DeviceProvisioningServices` @@ -70,18 +115,30 @@ The raw TypeSpec-only projects are: These diagnostics are intentional; the Swagger validator silently misses the emitted violations. -The selected-version TypeSpec population therefore contains 306 diagnostics -across 28 projects: all 27 overlap projects plus one intentional TypeSpec-only -project. +The version attribution was rechecked against the pinned source: + +| Project | Selected API version | Raw findings excluded | Source evidence | +| -------------------- | -------------------- | --------------------: | ------------------------------------------------------------------------------------------------------------------------------------------ | +| Batch | `2025-06-01` | 2 | `models.tsp:1981-1995`: certificate model removed in `v2025_06_01`. | +| Cdn | `2026-04-01-preview` | 1 | `KeyGroup.tsp:46-81`: PATCH interface removed in `v2025_12_01`; diagnostic on `models.tsp:3593`. | +| ManagedNetworkFabric | `2025-07-15` | 15 | Deprecated PATCH properties or their containing property are removed/renamed in `v2024_06_15_preview` or `v2025_07_15`; see example below. | +| NetApp | `2026-05-15-preview` | 1 | `Volume.tsp:1330-1346`: `usageThreshold20250901` removed/renamed at the selected preview version. | + +Excluding these 19 findings leaves 306 diagnostics across 28 projects: all +27 overlap projects plus Informatica. This is a **one-sided version-filtered +population**, not a globally projected latest-version run. Versioning can also +rename properties, so the unprojected deprecated names do not prove violations +in the older emitted API versions either. ## Diagnostic cardinality -| Identity | Validator | TypeSpec | -| ------------------------------------------------ | --------: | -------: | -| Raw full-run diagnostics | 151 | 325 | -| Validator `project + JSON path` | 48 | N/A | -| TypeSpec `project + source file + line + column` | N/A | 188 | -| Selected-version diagnostics | 151 | 306 | +| Identity | Validator | TypeSpec | +| ------------------------------------------------- | --------: | -------: | +| Raw full-run diagnostics | 151 | 325 | +| Validator `project + Swagger file + JSON path` | 48 | N/A | +| Validator `project + JSON path` | 48 | N/A | +| TypeSpec `project + source file + line + column` | N/A | 188 | +| After the documented one-sided version exclusions | 151 | 306 | Eighteen overlap projects have equal raw counts. Across the other nine, TypeSpec has 133 additional raw diagnostics and Swagger has none. The largest @@ -90,6 +147,15 @@ plus diagnostic granularity: Swagger can report one parent property at an operation body path while TypeSpec reports its individual missing leaves. The two identity domains cannot be safely collapsed into a one-to-one key. +Across all 32 affected projects, raw positive differences sum to 174 and +negative differences to zero. After the separate identity-based deduplications, +nine projects have equal counts, 22 are TypeSpec-higher, and SQL is +validator-higher; positive differences sum to 142 and negative differences to +2, giving 188 versus 48. SQL has four operation paths but only two reused +authored `operations` properties. EdgeOrder is the largest deduplicated +TypeSpec-higher outlier: three parent-property messages share one Swagger path, +while TypeSpec reports 25 authored leaves. + ## Emission matrix AutoRest's `getSchemaOrRef` selects inline or referenced schemas, @@ -107,13 +173,15 @@ compares the resulting `properties` maps. | PATCH lacks `200`/`201`; same-path GET has `200` | GET `200` response is fallback | clean | clean | `async-get-fallback` | | PATCH has scalar `200` and model `201` responses | Existing `200` schema wins before its shape is interpreted | violation | violation | `response-precedence` | | PATCH response range contains `200` | AutoRest emits the full `2XX` range while TypeSpec HTTP retains `{ start: 200, end: 299 }` | validator miss | violation | focused rule unit tests | -| Exact PATCH `200` overlaps a containing range | Explicit `200` response takes precedence over the range regardless of declaration order | validator miss | violation | focused rule unit tests | +| Exact PATCH `200` overlaps a containing range | Explicit `200` response takes precedence over the range regardless of declaration order | violation | violation | focused rule unit tests | | Different source names encode to the same JSON name | `resolveProperty` uses the encoded property name | clean | clean | `payload-property-shape` | | Nullable object properties have different nested properties | nullable single-model unions emit object `properties` | violation | violation | `nullable-object-mismatch` | | Nullable object properties have matching nested properties | nullable single-model unions emit matching object `properties` | clean | clean | `nullable-object-match` | | Same-named array and scalar properties | neither property schema emits named `properties` | clean | clean | `non-model-property-shape` | -| PATCH-only property scoped to C# | AutoRest `isInScope` omits it from the PATCH schema | clean | clean | `scoped-property` | -| Same-path GET scoped to C# | AutoRest omits the GET route, so PATCH has no fallback schema | clean | clean | `scoped-get-fallback` | +| PATCH-only property scoped to C# | AutoRest `isInScope` omits it from the PATCH schema | clean | violation | `scoped-property` | +| Same-path GET scoped to C# | AutoRest omits the GET route, so PATCH has no fallback schema | clean | violation | `scoped-get-fallback` | +| PATCH operation scoped to C# | AutoRest filters the route; no PATCH operation reaches the validator | clean | violation | `scoped-patch-operation` | +| Matching response property scoped to C# | AutoRest omits the response property but retains the PATCH property | violation | clean | `scoped-response-property` | | Undeclared PATCH discriminator | `getSchemaForModel` synthesizes the discriminator as a required string property | violation | violation | `synthesized-discriminator` | | Authored property encodes to a synthesized discriminator name | `resolveProperty` overwrites the synthesized property with the authored property's schema | violation | violation | `encoded-discriminator-property` | | Same-level PATCH subset | corresponding property exists in response schema | clean | clean | `same-level-subset` | @@ -123,11 +191,135 @@ Inherited properties and spreads reach the same model/property emitter branches. Arrays, records, scalar leaves, and empty objects have no named `properties` at that point in the recursive comparison; neither rule treats their elements, arbitrary record keys, or scalar values as named PATCH -properties. Operation and property scope use AutoRest's TCGC emitter identity, -and undeclared discriminators are represented by the model that causes AutoRest +properties. Operation and property scope are deliberately not projected to an +emitter-specific contract. Undeclared discriminators use compiler metadata and +are represented by the model that causes AutoRest to synthesize them. Cycles are guarded by active model-pair traversal without suppressing repeated authored occurrences on sibling paths. +## Gap examples: emitter scope is outside the native contract + +The following examples are from the checked-in comparison fixtures at API +version `2024-01-01` for the two existing fixtures and `0000-00-00` for the two +new unversioned fixtures. Schema excerpts omit descriptions only. Each native +outcome also has a direct unit assertion; comparison snapshots alone are not +the acceptance criterion. + +### PATCH request property omitted by AutoRest + +- **Classification:** TypeSpec-only +- **Status:** intentional +- **Project/API version:** fixture `scoped-property` / `2024-01-01` +- **Source:** `scoped-property/main.tsp`, `WidgetPatchProperties.clientOnly` + +```typespec +model WidgetPatchProperties { + @scope("csharp") + clientOnly?: string; +} +``` + +AutoRest emits `"WidgetPatchProperties": { "type": "object" }`, with no +`properties` map. The native model still contains `clientOnly`. + +| Engine | Observed result | +| ----------------- | ------------------------------------------------------------------------ | +| Swagger validator | No finding: the property is absent from the PATCH schema. | +| TypeSpec lint | One finding for `properties.clientOnly`, absent from the response model. | + +**Disposition:** retain the native finding and partial-coverage classification; +do not import downstream scope helpers into an ARM lint. + +### GET fallback omitted by AutoRest + +- **Classification:** TypeSpec-only +- **Status:** intentional +- **Project/API version:** fixture `scoped-get-fallback` / `2024-01-01` +- **Source:** `scoped-get-fallback/main.tsp`, `CustomWidgetOperations.read` + +```typespec +@get +@scope("csharp") +read(...ResourceInstanceParameters): WidgetResponse | ErrorResponse; + +@patch +update(...ResourceInstanceParameters, @body body: WidgetPatchBody): + | AcceptedResponse + | ErrorResponse; +``` + +The emitted widget-item path has only `patch`; its responses are `202` and +`default`, with no eligible resource schema. The same-path GET exists in the +native HTTP graph and returns the resource whose `displayName` is nested under +`properties`, unlike the PATCH body. + +| Engine | Observed result | +| ----------------- | -------------------------------------------------------------------------------- | +| Swagger validator | No finding: neither PATCH nor an emitted GET supplies a `200`/`201` schema. | +| TypeSpec lint | One finding for the wrongly nested `displayName`, using the native GET fallback. | + +**Disposition:** retain native GET selection; emitted route visibility is not a +native semantic condition. + +### PATCH endpoint omitted by AutoRest + +- **Classification:** TypeSpec-only +- **Status:** intentional +- **Project/API version:** fixture `scoped-patch-operation` / `0000-00-00` +- **Source:** `scoped-patch-operation/main.tsp`, `update` + +```typespec +@route("/widgets") +@patch +@scope("csharp") +op update(@body body: WidgetUpdate): Widget; +``` + +`WidgetUpdate` has `extra`; `Widget` has only `name`. The emitted `/widgets` +path has only `get`, so there is no PATCH object for the validator selector. + +| Engine | Observed result | +| ----------------- | ----------------------------------------------------- | +| Swagger validator | No finding: the PATCH operation was not emitted. | +| TypeSpec lint | One finding for `extra` on the native PATCH endpoint. | + +**Disposition:** retain endpoint checking independent of client scope. + +### Matching response property omitted by AutoRest + +- **Classification:** validator-only +- **Status:** intentional +- **Project/API version:** fixture `scoped-response-property` / `0000-00-00` +- **Source:** `scoped-response-property/main.tsp`, `Widget.description` + +```typespec +model Widget { + name?: string; + + @scope("csharp") + description?: string; +} +model WidgetUpdate { + description?: string; +} +``` + +```json +{ + "Widget": { "type": "object", "properties": { "name": { "type": "string" } } }, + "WidgetUpdate": { "type": "object", "properties": { "description": { "type": "string" } } } +} +``` + +| Engine | Observed result | +| ----------------- | ------------------------------------------------------------------------- | +| Swagger validator | One finding for `description`, absent from the emitted response schema. | +| TypeSpec lint | No finding: `description` exists at the same level in both native models. | + +**Disposition:** retain native compliance and the explicit reviewed validator +expectation. The validator is correct for the emitted schema; this is not a +validator false positive or proof of complete Swagger equivalence. + ## Gap example: custom PATCH traversal - **Classification:** validator-only @@ -172,14 +364,14 @@ resource lifecycle update, so `getArmResources()` did not expose it. - **Classification:** TypeSpec-only - **Status:** population mismatch -- **Project/API version:** `Batch` / selected latest version after `2025-06-01` +- **Project/API version:** `Batch` / `2025-06-01` - **Source:** `models.tsp`, `CertificateCreateOrUpdateProperties` **TypeSpec source** ```typespec @removed(Versions.v2025_06_01) -model CertificateCreateOrUpdateProperties { +model CertificateCreateOrUpdateProperties extends CertificateBaseProperties { @visibility(Lifecycle.Read, Lifecycle.Update) data: string; @@ -201,6 +393,36 @@ from the retained latest Swagger. **Disposition:** exclude these diagnostics from selected-version comparison; do not weaken the production lint. +### Versioning subcase: removed and renamed PATCH properties + +- **Classification:** TypeSpec-only +- **Status:** population mismatch +- **Project/API version:** `ManagedNetworkFabric` / `2025-07-15` +- **Source:** `models/NetworkToNetworkInterconnect.tsp:260-262` + +```typespec +@removed(Versions.v2025_07_15) +@renamedFrom(Versions.v2025_07_15, "prefixLimits") +prefixLimitsDeprecated?: OptionBLayer3PrefixLimitPatchProperties[]; +``` + +The raw program reports +`properties.optionBLayer3Configuration.prefixLimitsDeprecated`. The selected +version removes this declaration and uses the separately added `prefixLimits` +property. The same pattern accounts for the deprecated properties in +`InternalNetwork.tsp`, `common.tsp`, `NetworkTapRule.tsp`, and +`L3IsolationDomain.tsp`; two findings under `aggregateRouteConfigurationDeprecated` +are removed with their containing property in `v2024_06_15_preview`. + +| Engine | Observed result | +| ----------------- | ----------------------------------------------------------------- | +| Swagger validator | No corresponding deprecated property in the selected emitted API. | +| TypeSpec lint | Fifteen raw findings across removed/renamed PATCH declarations. | + +**Disposition:** exclude these source-program findings from the selected-version +project comparison. A renamed source property is not evidence of an invalid +older emitted property name. + ## Gap example: validator misses an emitted violation - **Classification:** TypeSpec-only @@ -246,6 +468,16 @@ model OrganizationPropertiesCustomUpdate { **Disposition:** retain the TypeSpec findings; they enforce the documented same-level subset contract and expose a validator false negative. +The selected Swagger was rechecked in this repair: `Organizations_Update` +uses `InformaticaOrganizationResourceUpdate` as its body and +`InformaticaOrganizationResource` as its `200` response. The former references +`OrganizationPropertiesCustomUpdate`; the latter references +`OrganizationProperties`, which has `informaticaProperties` rather than +`informaticaOrganizationProperties` and no `existingResourceId`. Retained +validator output contains no execution error and no rule finding. This proves +the observed omission and schema mismatch, but not the internal reason the +validator omitted the findings; that mechanism remains unisolated. + ## Gap example: emitted occurrence versus source target - **Classification:** count-only @@ -271,11 +503,75 @@ same-level subset contract and expose a validator false negative. **Disposition:** preserve raw counts and source identities separately. Do not deduplicate by property name or require count equality. +### Count-only outlier: SQL shares two properties across four routes + +- **Classification:** count-only +- **Status:** intentional +- **Project/API version:** `SQL` / `2025-02-01-preview` +- **Source:** `models.tsp:11883-11885` and `11930-11932` + +```typespec +model SensitivityLabelUpdateList { + operations?: SensitivityLabelUpdate[]; +} +model RecommendedSensitivityLabelUpdateList { + operations?: RecommendedSensitivityLabelUpdate[]; +} +``` + +The validator reports `operations` at four distinct PATCH body schema paths: +`currentSensitivityLabels` and `recommendedSensitivityLabels`, each under both +`managedInstances/.../databases` and `servers/.../databases`. TypeSpec also +reports four raw findings, but repeats the two source locations above. + +| Engine | Observed result | +| ----------------- | ------------------------------------------------------- | +| Swagger validator | Four raw findings and four file-independent JSON paths. | +| TypeSpec lint | Four raw findings and two source identities. | + +**Disposition:** do not mistake source reuse for two missed operations. + +### Count-only outlier: EdgeOrder reports parents versus leaves + +- **Classification:** count-only +- **Status:** intentional +- **Project/API version:** `EdgeOrder` / `2024-02-01` +- **Source:** `models.tsp:2471-2485`, `OrderItemUpdateProperties` + +```typespec +model OrderItemUpdateProperties { + forwardAddress?: AddressProperties; + preferences?: Preferences; + notificationEmailList?: string[]; +} +``` + +The validator reports `properties.forwardAddress`, `properties.preferences`, +and `properties.notificationEmailList`, all at the same order-item PATCH body +schema path. Native lint expands the missing object properties to authored +leaves such as `properties.forwardAddress.shippingAddress.streetAddress1`; +the array `notificationEmailList` remains a single target. + +| Engine | Observed result | +| ----------------- | -------------------------------------------------------------- | +| Swagger validator | Three parent-property messages, sharing one JSON path. | +| TypeSpec lint | Twenty-five authored-property messages at 25 source locations. | + +**Disposition:** preserve this existing diagnostic granularity, not a fabricated +one-to-one identity between operation schema paths and source properties. + ## Focused validation -Sixteen fixture cases pass: nine violations and seven compliant controls. They cover -nested and moved properties, custom PATCH traversal, PATCH `201`, GET `200` and -`201` fallback, encoded JSON names, nullable objects, and a same-level subset. -Focused unit regressions additionally verify that the TypeSpec rule selects a -response status range containing `200` and prefers an overlapping exact `200` -response. The package build and diagnostic-noise audit also pass. +The repair's 18 fixture snapshots were refreshed and then checked without +snapshot updates. There are nine matching violation cases, five matching +rule-compliant controls, three intentional TypeSpec-only scope cases, and one +reviewed validator-only scope case. The harness labels the TypeSpec-only cases +as mapped diagnostics in validator-clean fixtures; that warning is explained, +not suppressed or reclassified as equivalence. One pre-existing non-model +control still has unreviewed ambient diagnostics, unrelated to this rule. + +Nine native unit cases pass: the two existing response-range regressions, +three emitter/client-generator-free model-comparison cases, and four client +scope regressions. Package build and changed-file lint pass. The full corpus +completed with exit code zero; the six project compile failures above are +retained and excluded, not counted as successful comparisons. diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/rule.md b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/rule.md index 34433ec5b6..ecab4d044d 100644 --- a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/rule.md +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/rule.md @@ -3,7 +3,7 @@ validatorRuleId: ConsistentPatchProperties engine: spectral tspLints: - tsp-lintdiff-local-linter/consistent-patch-properties -coverageKind: lint +coverageKind: partial officialTspLints: - "@azure-tools/typespec-azure-resource-manager/arm-resource-patch" --- @@ -24,6 +24,26 @@ should not contain properties that are not present in the resource definition. The local lint checks the PATCH body shape against the ARM resource model recursively and reports properties that are missing from the resource model or moved to a different nesting level. +## Native boundary and partial coverage + +The rule uses compiler and HTTP semantic APIs, not a client-generator context or +an emitter. It checks the authored HTTP contract regardless of TCGC `@scope`. +AutoRest can omit a scoped PATCH operation, GET fallback, request property, or +response property. The Swagger validator sees that emitted contract, whereas +this lint sees the native declarations. These differences are intentional and +make Swagger equivalence partial; project overlap does not prove full parity. + +`scoped-property`, `scoped-get-fallback`, and `scoped-patch-operation` retain +Swagger-compliant expectations and record a native diagnostic in their snapshots. +`scoped-response-property` is native-compliant and records the reviewed validator +discrepancy. Native unit tests assert the intended result for all four cases. +Do not add an emitter adapter or read TCGC private state to eliminate these gaps. + +The provider namespace check isolates this ARM rule in lintdiff's mixed ruleset; +it is not a requirement that every operation carry provider metadata. Evaluate +that isolation separately during official ARM promotion, with ordinary and +nested-namespace coverage. This repair does not change the applicability guard. + ## Semantic coverage notes - The official `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` lint partially @@ -43,8 +63,10 @@ properties that are missing from the resource model or moved to a different nest - different source names with the same emitted JSON name => valid - nullable object properties recurse like emitted object schemas => invalid when nested shapes differ, valid when they match - matching property names whose array/scalar schemas have no named properties => valid - - properties scoped away from the AutoRest emitter => valid - - a same-path GET scoped away from AutoRest is unavailable as a PATCH fallback => valid + - properties scoped away from AutoRest remain part of the native model => invalid when missing from the response + - a same-path GET scoped away from AutoRest remains a native PATCH fallback => invalid when the body is inconsistent + - a PATCH operation scoped away from AutoRest remains a native endpoint => invalid when the body is inconsistent + - response properties scoped away from AutoRest remain available for native comparison => valid when the PATCH body is a same-level subset - an undeclared discriminator synthesized into the PATCH schema => invalid when absent from the response schema - an encoded authored property replaces a same-named synthesized discriminator => compare the authored property shape - PATCH property subset at the same level => valid @@ -52,24 +74,26 @@ properties that are missing from the resource model or moved to a different nest ## Test Cases -| ID | Violation | Description | -| -------------------------------- | --------- | ------------------------------------------------------------------------------------------------------ | -| `inconsistent-patch` | yes | PATCH places `displayName` at the top level even though the resource model nests it under `properties` | -| `nested-extra-property` | yes | PATCH adds `properties.extraPatchOnly`, which does not exist in the resource model | -| `custom-patch-operation` | yes | A custom ARM PATCH operation places `displayName` at the wrong level | -| `patch-201-response` | yes | PATCH selects its `201` response model and finds a moved property | -| `get-201-fallback` | yes | PATCH falls back to the same-path GET `201` response model and finds a moved property | -| `response-precedence` | yes | A scalar PATCH `200` response takes precedence over the matching `201` resource response | -| `payload-property-shape` | no | Different source names encode to the same matching JSON name | -| `nullable-object-mismatch` | yes | Nullable request and response objects have different nested properties | -| `nullable-object-match` | no | Nullable request and response objects have the same nested properties | -| `non-model-property-shape` | no | Same-named array and scalar properties both emit no nested named properties | -| `scoped-property` | no | A PATCH-only property scoped to C# is omitted by AutoRest | -| `scoped-get-fallback` | no | A GET operation scoped to C# is not available as AutoRest's PATCH response fallback | -| `synthesized-discriminator` | yes | AutoRest synthesizes a PATCH discriminator property absent from the response model | -| `encoded-discriminator-property` | yes | An encoded authored property replaces the synthesized discriminator and has a mismatching nested shape | -| `same-level-subset` | no | PATCH updates only `properties.description`, which is a valid subset of the resource model | -| `async-get-fallback` | no | PATCH has only a `202` response, so the validator falls back to the GET resource model | +| ID | Violation | Description | +| -------------------------------- | ------------------------ | ------------------------------------------------------------------------------------------------------ | +| `inconsistent-patch` | yes | PATCH places `displayName` at the top level even though the resource model nests it under `properties` | +| `nested-extra-property` | yes | PATCH adds `properties.extraPatchOnly`, which does not exist in the resource model | +| `custom-patch-operation` | yes | A custom ARM PATCH operation places `displayName` at the wrong level | +| `patch-201-response` | yes | PATCH selects its `201` response model and finds a moved property | +| `get-201-fallback` | yes | PATCH falls back to the same-path GET `201` response model and finds a moved property | +| `response-precedence` | yes | A scalar PATCH `200` response takes precedence over the matching `201` resource response | +| `payload-property-shape` | no | Different source names encode to the same matching JSON name | +| `nullable-object-mismatch` | yes | Nullable request and response objects have different nested properties | +| `nullable-object-match` | no | Nullable request and response objects have the same nested properties | +| `non-model-property-shape` | no | Same-named array and scalar properties both emit no nested named properties | +| `scoped-property` | no (Swagger); native yes | AutoRest omits the PATCH-only property; native lint reports it | +| `scoped-get-fallback` | no (Swagger); native yes | AutoRest omits the GET fallback; native lint uses it | +| `scoped-patch-operation` | no (Swagger); native yes | AutoRest omits the PATCH endpoint; native lint checks it | +| `scoped-response-property` | yes (Swagger); native no | AutoRest omits a matching response property; native lint accepts it | +| `synthesized-discriminator` | yes | AutoRest synthesizes a PATCH discriminator property absent from the response model | +| `encoded-discriminator-property` | yes | An encoded authored property replaces the synthesized discriminator and has a mismatching nested shape | +| `same-level-subset` | no | PATCH updates only `properties.description`, which is a valid subset of the resource model | +| `async-get-fallback` | no | PATCH has only a `202` response, so the validator falls back to the GET resource model | Focused rule unit tests additionally cover the TypeSpec HTTP representation where one response carries a status-code range containing `200`, including the diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-get-fallback/tsp-diagnostics.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-get-fallback/tsp-diagnostics.json index 26773c6f88..e3fd5387d7 100644 --- a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-get-fallback/tsp-diagnostics.json +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-get-fallback/tsp-diagnostics.json @@ -4,6 +4,11 @@ "severity": "warning", "message": "The resource name parameter should be defined with a 'pattern' restriction. Please use 'ResourceNameParameter' to specify the name parameter with options to override default pattern RegEx expression." }, + { + "code": "tsp-lintdiff-local-linter/consistent-patch-properties", + "severity": "warning", + "message": "The property 'displayName' in the request body either does not appear in the resource model or is nested at the wrong level." + }, { "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state", "severity": "warning", diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/expect.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/expect.json new file mode 100644 index 0000000000..c8d62bd11a --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/expect.json @@ -0,0 +1 @@ +{ "violation": false } diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/main.tsp b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/main.tsp new file mode 100644 index 0000000000..4d3a8df859 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/main.tsp @@ -0,0 +1,38 @@ +import "../../lib/imports.tsp"; +import "@azure-tools/typespec-client-generator-core"; + +using TypeSpec.Http; +using Azure.ClientGenerator.Core; +using Azure.ResourceManager; + +@armProviderNamespace +@service(#{ title: "Scoped PATCH comparison" }) +namespace Microsoft.TestService; + +/** Resource returned by the service. */ +model Widget { + /** Resource name. */ + name?: string; +} + +/** Native update body with an extra property. */ +model WidgetUpdate { + /** Property absent from the resource. */ + extra?: string; +} + +interface Operations extends Azure.ResourceManager.Operations {} + +/** Read the widget. */ +@route("/widgets") +@get +op read(): Widget; + +/** Update the widget in the native HTTP contract, but not AutoRest output. */ +@route("/widgets") +@patch +@scope("csharp") +op update( + /** Update body. */ + @body body: WidgetUpdate, +): Widget; diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/output.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/output.json new file mode 100644 index 0000000000..ff2ad6c831 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/output.json @@ -0,0 +1,116 @@ +{ + "swagger": "2.0", + "info": { + "title": "Scoped PATCH comparison", + "version": "0000-00-00", + "x-typespec-generated": [ + { + "emitter": "@azure-tools/typespec-autorest" + } + ] + }, + "schemes": [ + "https" + ], + "host": "management.azure.com", + "produces": [ + "application/json" + ], + "consumes": [ + "application/json" + ], + "security": [ + { + "azure_auth": [ + "user_impersonation" + ] + } + ], + "securityDefinitions": { + "azure_auth": { + "type": "oauth2", + "description": "Azure Active Directory OAuth2 Flow.", + "flow": "implicit", + "authorizationUrl": "https://login.microsoftonline.com/common/oauth2/authorize", + "scopes": { + "user_impersonation": "impersonate your user account" + } + } + }, + "tags": [ + { + "name": "Operations" + } + ], + "paths": { + "/providers/Microsoft.TestService/operations": { + "get": { + "operationId": "Operations_List", + "tags": [ + "Operations" + ], + "description": "List the operations for the provider", + "parameters": [ + { + "$ref": "../../../../../common-types/resource-management/v3/types.json#/parameters/ApiVersionParameter" + } + ], + "responses": { + "200": { + "description": "Azure operation completed successfully.", + "schema": { + "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/OperationListResult" + } + }, + "default": { + "description": "An unexpected error response.", + "schema": { + "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/ErrorResponse" + } + } + }, + "x-ms-pageable": { + "nextLinkName": "nextLink" + } + } + }, + "/widgets": { + "get": { + "operationId": "Read", + "description": "Read the widget.", + "parameters": [], + "responses": { + "200": { + "description": "The request has succeeded.", + "schema": { + "$ref": "#/definitions/Widget" + } + } + } + } + } + }, + "definitions": { + "Widget": { + "type": "object", + "description": "Resource returned by the service.", + "properties": { + "name": { + "type": "string", + "description": "Resource name." + } + } + }, + "WidgetUpdate": { + "type": "object", + "description": "Native update body with an extra property.", + "properties": { + "extra": { + "type": "string", + "description": "Property absent from the resource." + } + } + } + }, + "parameters": {} +} diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/tsp-diagnostics.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/tsp-diagnostics.json new file mode 100644 index 0000000000..28fe2a6d11 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/tsp-diagnostics.json @@ -0,0 +1,67 @@ +[ + { + "code": "@azure-tools/typespec-azure-core/require-versioned", + "severity": "warning", + "message": "Azure services should use the versioning library to define versions for their services. Add the '@versioned' decorator to the service namespace." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-common-types-version", + "severity": "warning", + "message": "Specify the ARM common-types version using the @armCommonTypesVersion decorator on the service namespace or on each version of the service version enum." + }, + { + "code": "tsp-lintdiff-local-linter/latest-version-of-common-types-must-be-used", + "severity": "warning", + "message": "Use the latest ARM common-types version 'v6' instead of 'v3'." + }, + { + "code": "tsp-lintdiff-local-linter/consistent-patch-properties", + "severity": "warning", + "message": "The property 'extra' in the request body either does not appear in the resource model or is nested at the wrong level." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "All operations must be inside an interface declaration." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "All Resource operations must use an api-version parameter. Please include Azure.ResourceManager.ApiVersionParameter in the operation parameter list using the spread (...ApiVersionParameter) operator, or using one of the common resource parameter models." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "Resource GET operation must be decorated with @armResourceRead or @armResourceList." + }, + { + "code": "tsp-lintdiff-local-linter/get-collection-only-has-value-and-next-link", + "severity": "warning", + "message": "Get endpoints for collections of resources must only have the `value` and `nextLink` properties in their model." + }, + { + "code": "tsp-lintdiff-local-linter/get-in-operation-name", + "severity": "warning", + "message": "'GET' operation 'Read' should use method name 'Get' or method name starting with 'List'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change." + }, + { + "code": "tsp-lintdiff-local-linter/xms-examples-required", + "severity": "warning", + "message": "Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "All operations must be inside an interface declaration." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "All Resource operations must use an api-version parameter. Please include Azure.ResourceManager.ApiVersionParameter in the operation parameter list using the spread (...ApiVersionParameter) operator, or using one of the common resource parameter models." + }, + { + "code": "tsp-lintdiff-local-linter/xms-examples-required", + "severity": "warning", + "message": "Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations." + } +] diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/validator-diagnostics.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/validator-diagnostics.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-patch-operation/validator-diagnostics.json @@ -0,0 +1 @@ +[] diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-property/tsp-diagnostics.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-property/tsp-diagnostics.json index 4b37b64f1f..30e88f5971 100644 --- a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-property/tsp-diagnostics.json +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-property/tsp-diagnostics.json @@ -4,6 +4,11 @@ "severity": "warning", "message": "The resource name parameter should be defined with a 'pattern' restriction. Please use 'ResourceNameParameter' to specify the name parameter with options to override default pattern RegEx expression." }, + { + "code": "tsp-lintdiff-local-linter/consistent-patch-properties", + "severity": "warning", + "message": "The property 'properties.clientOnly' in the request body either does not appear in the resource model or is nested at the wrong level." + }, { "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state", "severity": "warning", diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/expect.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/expect.json new file mode 100644 index 0000000000..ab5abad994 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/expect.json @@ -0,0 +1,4 @@ +{ + "violation": false, + "validatorDiagnostics": [{ "code": "ConsistentPatchProperties", "count": 1 }] +} diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/main.tsp b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/main.tsp new file mode 100644 index 0000000000..1fb4f47a42 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/main.tsp @@ -0,0 +1,36 @@ +import "../../lib/imports.tsp"; +import "@azure-tools/typespec-client-generator-core"; + +using TypeSpec.Http; +using Azure.ClientGenerator.Core; +using Azure.ResourceManager; + +@armProviderNamespace +@service(#{ title: "Scoped response comparison" }) +namespace Microsoft.TestService; + +/** Resource with a property available in the native model only. */ +model Widget { + /** Resource name. */ + name?: string; + + /** Property omitted from AutoRest output. */ + @scope("csharp") + description?: string; +} + +/** Native update body is a same-level subset of Widget. */ +model WidgetUpdate { + /** Updated description. */ + description?: string; +} + +interface Operations extends Azure.ResourceManager.Operations {} + +/** Update the widget. */ +@route("/widgets") +@patch +op update( + /** Update body. */ + @body body: WidgetUpdate, +): Widget; diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/output.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/output.json new file mode 100644 index 0000000000..eb8ba1316e --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/output.json @@ -0,0 +1,126 @@ +{ + "swagger": "2.0", + "info": { + "title": "Scoped response comparison", + "version": "0000-00-00", + "x-typespec-generated": [ + { + "emitter": "@azure-tools/typespec-autorest" + } + ] + }, + "schemes": [ + "https" + ], + "host": "management.azure.com", + "produces": [ + "application/json" + ], + "consumes": [ + "application/json" + ], + "security": [ + { + "azure_auth": [ + "user_impersonation" + ] + } + ], + "securityDefinitions": { + "azure_auth": { + "type": "oauth2", + "description": "Azure Active Directory OAuth2 Flow.", + "flow": "implicit", + "authorizationUrl": "https://login.microsoftonline.com/common/oauth2/authorize", + "scopes": { + "user_impersonation": "impersonate your user account" + } + } + }, + "tags": [ + { + "name": "Operations" + } + ], + "paths": { + "/providers/Microsoft.TestService/operations": { + "get": { + "operationId": "Operations_List", + "tags": [ + "Operations" + ], + "description": "List the operations for the provider", + "parameters": [ + { + "$ref": "../../../../../common-types/resource-management/v3/types.json#/parameters/ApiVersionParameter" + } + ], + "responses": { + "200": { + "description": "Azure operation completed successfully.", + "schema": { + "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/OperationListResult" + } + }, + "default": { + "description": "An unexpected error response.", + "schema": { + "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/ErrorResponse" + } + } + }, + "x-ms-pageable": { + "nextLinkName": "nextLink" + } + } + }, + "/widgets": { + "patch": { + "operationId": "Update", + "description": "Update the widget.", + "parameters": [ + { + "name": "body", + "in": "body", + "description": "Update body.", + "required": true, + "schema": { + "$ref": "#/definitions/WidgetUpdate" + } + } + ], + "responses": { + "200": { + "description": "The request has succeeded.", + "schema": { + "$ref": "#/definitions/Widget" + } + } + } + } + } + }, + "definitions": { + "Widget": { + "type": "object", + "description": "Resource with a property available in the native model only.", + "properties": { + "name": { + "type": "string", + "description": "Resource name." + } + } + }, + "WidgetUpdate": { + "type": "object", + "description": "Native update body is a same-level subset of Widget.", + "properties": { + "description": { + "type": "string", + "description": "Updated description." + } + } + } + }, + "parameters": {} +} diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/tsp-diagnostics.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/tsp-diagnostics.json new file mode 100644 index 0000000000..46b4cef4c9 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/tsp-diagnostics.json @@ -0,0 +1,32 @@ +[ + { + "code": "@azure-tools/typespec-azure-core/require-versioned", + "severity": "warning", + "message": "Azure services should use the versioning library to define versions for their services. Add the '@versioned' decorator to the service namespace." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-common-types-version", + "severity": "warning", + "message": "Specify the ARM common-types version using the @armCommonTypesVersion decorator on the service namespace or on each version of the service version enum." + }, + { + "code": "tsp-lintdiff-local-linter/latest-version-of-common-types-must-be-used", + "severity": "warning", + "message": "Use the latest ARM common-types version 'v6' instead of 'v3'." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "All operations must be inside an interface declaration." + }, + { + "code": "@azure-tools/typespec-azure-resource-manager/arm-resource-operation", + "severity": "warning", + "message": "All Resource operations must use an api-version parameter. Please include Azure.ResourceManager.ApiVersionParameter in the operation parameter list using the spread (...ApiVersionParameter) operator, or using one of the common resource parameter models." + }, + { + "code": "tsp-lintdiff-local-linter/xms-examples-required", + "severity": "warning", + "message": "Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations." + } +] diff --git a/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/validator-diagnostics.json b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/validator-diagnostics.json new file mode 100644 index 0000000000..4c6be91df1 --- /dev/null +++ b/packages/typespec-lintdiff/test/fixtures/ConsistentPatchProperties/scoped-response-property/validator-diagnostics.json @@ -0,0 +1,15 @@ +[ + { + "code": "ConsistentPatchProperties", + "message": "The property 'description' in the request body either not apppear in the resource model or has the wrong level.", + "path": [ + "paths", + "/widgets", + "patch", + "parameters", + "0", + "schema" + ], + "severity": 0 + } +] diff --git a/packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts b/packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts index 432631372d..ef646a8636 100644 --- a/packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts +++ b/packages/typespec-lintdiff/test/rules/consistent-patch-properties.test.ts @@ -30,6 +30,69 @@ beforeEach(async () => { }); describe("consistent-patch-properties", () => { + it("checks nested PATCH properties without loading AutoRest or the client generator core", async () => { + await tester + .expect( + ` + using TypeSpec.Http; + @Azure.ResourceManager.armProviderNamespace + @service namespace Microsoft.TestService; + + model Resource { properties: { name?: string }; } + model Update { properties?: { extra?: string }; } + + @route("/widgets") @patch + op update(@body body: Update): Resource; + `, + ) + .toEmitDiagnostics({ + code: "tsp-lintdiff-local-linter/consistent-patch-properties", + message: + "The property 'properties.extra' in the request body either does not appear in the resource model or is nested at the wrong level.", + }); + }); + + it("accepts a same-level subset without loading AutoRest or the client generator core", async () => { + await tester + .expect( + ` + using TypeSpec.Http; + @Azure.ResourceManager.armProviderNamespace + @service namespace Microsoft.TestService; + + model Resource { properties: { name?: string; description?: string }; } + model Update { properties?: { name?: string }; } + + @route("/widgets") @patch + op update(@body body: Update): Resource; + `, + ) + .toBeValid(); + }); + + it("uses the same-path GET model without loading AutoRest or the client generator core", async () => { + await tester + .expect( + ` + using TypeSpec.Http; + @Azure.ResourceManager.armProviderNamespace + @service namespace Microsoft.TestService; + + model Resource { name?: string; } + model Accepted { @statusCode statusCode: 202; } + + @route("/widgets") @get op read(): Resource; + @route("/widgets") @patch + op update(@body body: { extra?: string }): Accepted; + `, + ) + .toEmitDiagnostics({ + code: "tsp-lintdiff-local-linter/consistent-patch-properties", + message: + "The property 'extra' in the request body either does not appear in the resource model or is nested at the wrong level.", + }); + }); + it("uses a response model from a status-code range containing 200", async () => { await tester .expect( @@ -71,6 +134,96 @@ describe("consistent-patch-properties", () => { }); }); + describe("client scope does not change the native PATCH contract", () => { + beforeEach(async () => { + const runner = await createTester(resolvePath(import.meta.dirname, "../.."), { + libraries: [ + "@typespec/http", + "@typespec/openapi", + "@typespec/rest", + "@typespec/versioning", + "@azure-tools/typespec-azure-core", + "@azure-tools/typespec-azure-resource-manager", + "@azure-tools/typespec-client-generator-core", + ], + }) + .importLibraries() + .createInstance(); + tester = createLinterRuleTester( + runner, + consistentPatchPropertiesRule, + "tsp-lintdiff-local-linter", + ); + }); + + const service = ` + using TypeSpec.Http; + using Azure.ClientGenerator.Core; + @Azure.ResourceManager.armProviderNamespace + @service namespace Microsoft.TestService; + model Resource { name?: string; } + `; + + it("checks a property scoped away from AutoRest", async () => { + await tester + .expect( + `${service} + model Update { @scope("csharp") extra?: string; } + @route("/widgets") @patch op update(@body body: Update): Resource; + `, + ) + .toEmitDiagnostics({ + code: "tsp-lintdiff-local-linter/consistent-patch-properties", + message: + "The property 'extra' in the request body either does not appear in the resource model or is nested at the wrong level.", + }); + }); + + it("checks a PATCH operation scoped away from AutoRest", async () => { + await tester + .expect( + `${service} + @route("/widgets") @patch @scope("csharp") + op update(@body body: { extra?: string }): Resource; + `, + ) + .toEmitDiagnostics({ + code: "tsp-lintdiff-local-linter/consistent-patch-properties", + message: + "The property 'extra' in the request body either does not appear in the resource model or is nested at the wrong level.", + }); + }); + + it("uses a GET fallback scoped away from AutoRest", async () => { + await tester + .expect( + `${service} + model Accepted { @statusCode statusCode: 202; } + @route("/widgets") @get @scope("csharp") op read(): Resource; + @route("/widgets") @patch + op update(@body body: { extra?: string }): Accepted; + `, + ) + .toEmitDiagnostics({ + code: "tsp-lintdiff-local-linter/consistent-patch-properties", + message: + "The property 'extra' in the request body either does not appear in the resource model or is nested at the wrong level.", + }); + }); + + it("accepts a matching response property scoped away from AutoRest", async () => { + await tester + .expect( + `${service} + model ScopedResource { @scope("csharp") name?: string; } + @route("/widgets") @patch + op update(@body body: { name?: string }): ScopedResource; + `, + ) + .toBeValid(); + }); + }); + it("prefers an exact response over an overlapping status-code range", async () => { await tester .expect(