Add arm-no-path-casing-conflicts linter rule#4379
Conversation
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/5585c071-eacf-4c3c-94f5-147154f6ad06 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
microsoft-github-policy-service
Bot
added
lib:azure-resource-manager
| * Replace `{paramName}` placeholders with `{}` so that paths only differing | ||
| * by parameter name still get bucketed together. | ||
| */ | ||
| function normalizePath(path: string): string { |
There was a problem hiding this comment.
This is unnecessary and duplicative. We already have a candidate rule for paths that differ by parameter name, her we want paths that differ only by casing of static path segments.
There was a problem hiding this comment.
Updated in e8fdf16: parameter-name-only casing differences no longer trigger this rule. Bucketing and comparison now operate on static segments only (parameters normalized to {}), so paths that differ only by parameter name casing are skipped and left to the separate candidate rule. Test and docs updated accordingly.
| * a CodeFix that lowercases the matching `@segment("...")` decorator string | ||
| * literals reachable from this operation. | ||
| * | ||
| * If any differing token is a path parameter (e.g. `{ResourceName}` vs |
There was a problem hiding this comment.
We should just leave this comment out.
⚡ Benchmark ResultsComparing [
|
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/dcf050e1-c5e8-44f2-b464-412c5eb55b68 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
commit: |
|
@copilot Please pull the latest from main, rebuild and rebuild the docs for typespec-azure-resource-manager, fix any merge conflicts, verify that all files are correctly formatted and only the changes associated with this PR are in the pr diff |
…th-casing-rule # Conflicts: # website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/55aa9445-8282-4c42-9865-eead8e407804 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
Done in fe958be. Merged latest |
|
@copilot tests are failiong because the new rule hasn't been added to the azure-resource-manager ruleset. Please pull the latest from main, fix the issue, ensure all tests are passing and all files are correctly formatted, and the pr diff contains only changes from this pr. |
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/6a7ca720-e633-4d43-a5b1-57b04e670a42 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
Done in 5100b66. Added |
markcowl
left a comment
markcowl
left a comment
There was a problem hiding this comment.
Pull the latest from main, add tests to cover the suggested changes, then implement the changes. Ensure that all tests pass, all files are correctly formatted, all samples can be built cleanly and only the changes associated with this PR are in the PR diff.
| * Replace `{paramName}` placeholders with `{}` so that path-parameter naming | ||
| * differences are ignored when comparing paths. | ||
| */ | ||
| function normalizeParameters(path: string): string { |
There was a problem hiding this comment.
I previously told you to remove this. You should include the path parameters in your comparison
There was a problem hiding this comment.
Please formulate tests to verify that paths like:
/{resourceUri}/providers/Microsoft.Foo/widgets
and
/{scope}/providers/microsoft.foo/widgets
are in different buckets
However:
/{scope}/providers/microsoft.foo/widgets
and
/{Scope}/providers/Microsoft.Foo/widgets
should be in the same bucket
There was a problem hiding this comment.
Done in 0e7eafa. Removed the normalizeParameters helper so path parameter names are now part of the case-insensitive bucket key (op.path.toLowerCase()). Added two tests covering the requested cases: /{resourceUri}/providers/Microsoft.Foo/widgets vs /{scope}/providers/microsoft.foo/widgets end up in different buckets (no diagnostic), while /{scope}/providers/microsoft.foo/widgets vs /{Scope}/providers/Microsoft.Foo/widgets share a bucket and emit a diagnostic. Updated the existing parameter-name-casing test to assert a diagnostic accordingly. ARM rule tests pass 8/8, rulesets validate-rules-defined 3/3, samples build & tests 68/68, pnpm format clean.
…th-casing-rule Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/778bf744-f599-415c-937b-3e87d92678ca Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/35d76144-269f-421e-ae00-3873e8a437ef Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
This PR ( |
markcowl
left a comment
markcowl
left a comment
There was a problem hiding this comment.
Pull the latest from main, make the suggested changes, ensure that all tests pass, all files are formatted correctly, and only the changes associated with this PR are in the PR diff
| name: string; | ||
| } | ||
|
|
||
| model Bar is ProxyResource<FooProperties> { |
There was a problem hiding this comment.
The fixed version here will not compile, since the same operation verb and path are used. Instead, make one of the operations be a delete and the other a read
There was a problem hiding this comment.
Fixed in ee903fb. Changed Bars to delete is ArmResourceDeleteSync<Bar> (in both before and after blocks) so the post-fix snippet compiles — Foos.get and Bars.delete share the same path after the codefix but use different verbs.
| }); | ||
| }); | ||
|
|
||
| it("applies a codefix that lowercases the offending @segment value", async () => { |
There was a problem hiding this comment.
Also add two operations that use @route with different casing (one delete and one get)
There was a problem hiding this comment.
…th-casing-rule Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/32f08dce-2572-45d3-9c2c-8a54f2a6f364 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com>
arm-no-path-casing-conflicts linter rule
packages/typespec-azure-resource-manager/src/linter.ts@azure-tools/typespec-azure-rulesetsresource-manager ruleset{scope}vs{Scope}collide;{scope}vs{resourceUri}do not)/{resourceUri}/...vs/{scope}/...(different buckets) and/{scope}/...vs/{Scope}/...(same bucket)Barsresource now usesArmResourceDeleteSync<Bar>so the post-fix code compiles (different verb on same path asFoos.get is ArmResourceRead<Foo>)@routeoperations differing only by static-segment casing — one@delete, one@get— emits a diagnosticmain;pnpm formatclean; PR diff againstmaincontains only the 8 files for this changevalidate-rules-definedtest passes (3/3); ARM rule tests pass (9/9)