Add no-url-suffix linter rule#4541
Conversation
microsoft-github-policy-service
Bot
added
int:azure-specs
haiyuazhang
force-pushed
the
haiyzhan/linter-url-to-uri
branch
from
ee45b72 to
42061ef
Compare
|
All changed packages have been documented.
Show changes
|
haiyuazhang
force-pushed
the
haiyzhan/linter-url-to-uri
branch
from
42061ef to
8041bab
Compare
commit: |
|
You can try these changes here
|
⚡ Benchmark Results✅ No performance regressions detected. Full details – comparing
|
| Metric | Baseline | Current | Change |
|---|---|---|---|
| total | 🔴 636.2ms | 🔴 615.4ms | -3.3% |
| loader | 🟢 187.7ms | 🟢 182.1ms | -3.0% |
| resolver | 🟢 18.3ms | 🟢 18.3ms | -0.0% |
| checker | 🟡 204.4ms | 🟢 198.3ms | -3.0% |
| validation | 🟢 43.4ms | 🟢 41.5ms | -4.5% |
| ↳ validation/@azure-tools/typespec-azure-core | 🟢 7.0ms | 🟢 6.9ms | -1.1% |
| ↳ validation/@typespec/http | 🟢 6.0ms | 🟢 5.9ms | -2.8% |
| ↳ validation/@typespec/rest | 🟢 0.5ms | 🟢 0.5ms | -9.6% |
| ↳ validation/@typespec/versioning | 🔴 28.4ms | 🔴 26.6ms | -6.4% 🟢 |
| ↳ validation/compiler | 🟢 1.4ms | 🟢 1.3ms | -8.0% |
| linter | 🟢 142.1ms | 🟢 139.0ms | -2.2% |
| ↳ linter/@azure-tools/typespec-azure-core/auth-required | 🟢 0.0ms | 🟢 0.0ms | -8.4% |
| ↳ linter/@azure-tools/typespec-azure-core/bad-record-type | 🟢 0.2ms | 🟢 0.2ms | -4.0% |
| ↳ linter/@azure-tools/typespec-azure-core/byos | 🟢 6.2ms | 🟢 5.7ms | -8.1% |
| ↳ linter/@azure-tools/typespec-azure-core/casing-style | 🟢 0.7ms | 🟢 0.7ms | -1.5% |
| ↳ linter/@azure-tools/typespec-azure-core/composition-over-inheritance | 🟢 0.1ms | 🟢 0.1ms | -8.3% |
| ↳ linter/@azure-tools/typespec-azure-core/documentation-required | 🟢 0.8ms | 🟢 0.8ms | -3.2% |
| ↳ linter/@azure-tools/typespec-azure-core/friendly-name | 🟢 0.6ms | 🟢 0.6ms | -0.4% |
| ↳ linter/@azure-tools/typespec-azure-core/key-visibility-required | 🟢 0.2ms | 🟢 0.2ms | -1.9% |
| ↳ linter/@azure-tools/typespec-azure-core/known-encoding | 🟢 0.3ms | 🟢 0.3ms | -6.4% |
| ↳ linter/@azure-tools/typespec-azure-core/long-running-polling-operation-required | 🟢 0.3ms | 🟢 0.3ms | -1.4% |
| ↳ linter/@azure-tools/typespec-azure-core/no-case-mismatch | 🟢 0.3ms | 🟢 0.2ms | -2.4% |
| ↳ linter/@azure-tools/typespec-azure-core/no-closed-literal-union | 🟢 0.3ms | 🟢 0.2ms | -2.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-enum | 🟢 0.0ms | 🟢 0.0ms | -8.9% |
| ↳ linter/@azure-tools/typespec-azure-core/no-error-status-codes | 🟢 0.1ms | 🟢 0.1ms | -1.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops | 🟢 0.1ms | 🟢 0.1ms | -4.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-format | 🟢 0.5ms | 🟢 0.5ms | -1.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-generic-numeric | 🟢 0.4ms | 🟢 0.4ms | -4.3% |
| ↳ linter/@azure-tools/typespec-azure-core/no-header-explode | 🔴 21.1ms | 🔴 20.4ms | -3.4% |
| ↳ linter/@azure-tools/typespec-azure-core/no-legacy-usage | 🟢 1.1ms | 🟢 1.1ms | -0.6% |
| ↳ linter/@azure-tools/typespec-azure-core/no-multiple-discriminator | 🟢 0.1ms | 🟢 0.1ms | -1.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-nullable | 🟢 0.2ms | 🟢 0.2ms | -1.6% |
| ↳ linter/@azure-tools/typespec-azure-core/no-offsetdatetime | 🟢 1.2ms | 🟢 1.2ms | +2.1% |
| ↳ linter/@azure-tools/typespec-azure-core/no-openapi | 🟢 2.0ms | 🟢 1.9ms | -4.8% |
| ↳ linter/@azure-tools/typespec-azure-core/no-private-usage | 🟢 1.8ms | 🟢 1.8ms | -0.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-query-explode | 🔴 21.3ms | 🔴 20.6ms | -3.2% |
| ↳ linter/@azure-tools/typespec-azure-core/no-response-body | 🔴 25.2ms | 🔴 25.2ms | -0.0% |
| ↳ linter/@azure-tools/typespec-azure-core/no-rest-library-interfaces | 🟢 0.0ms | 🟢 0.0ms | -12.1% |
| ↳ linter/@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch | 🟢 5.3ms | 🟢 5.3ms | -0.7% |
| ↳ linter/@azure-tools/typespec-azure-core/no-rpc-path-params | 🟢 0.2ms | 🟢 0.2ms | -1.6% |
| ↳ linter/@azure-tools/typespec-azure-core/no-string-discriminator | 🟢 0.0ms | 🟢 0.0ms | +4.1% |
| ↳ linter/@azure-tools/typespec-azure-core/no-unknown | 🟢 0.2ms | 🟢 0.2ms | -1.5% |
| ↳ linter/@azure-tools/typespec-azure-core/no-unnamed-union | 🟢 0.3ms | 🟢 0.3ms | -1.5% |
| ↳ linter/@azure-tools/typespec-azure-core/operation-missing-api-version | 🟢 0.2ms | 🟢 0.1ms | -11.0% |
| ↳ linter/@azure-tools/typespec-azure-core/request-body-problem | 🟢 0.3ms | 🟢 0.3ms | -7.6% |
| ↳ linter/@azure-tools/typespec-azure-core/require-versioned | 🟢 0.0ms | 🟢 0.0ms | -20.1% |
| ↳ linter/@azure-tools/typespec-azure-core/response-schema-problem | 🔴 23.6ms | 🔴 23.7ms | +0.3% |
| ↳ linter/@azure-tools/typespec-azure-core/rpc-operation-request-body | 🟢 0.3ms | 🟢 0.4ms | +7.9% |
| ↳ linter/@azure-tools/typespec-azure-core/spread-discriminated-model | 🟢 0.3ms | 🟢 0.3ms | -5.1% |
| ↳ linter/@azure-tools/typespec-azure-core/use-standard-names | 🟢 5.7ms | 🟢 5.3ms | -5.9% |
| ↳ linter/@azure-tools/typespec-azure-core/use-standard-operations | 🟢 0.1ms | 🟢 0.1ms | +0.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-common-types-version | 🟢 4.4ms | 🟢 4.1ms | -6.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key | 🟢 0.1ms | 🟢 0.1ms | +7.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage | 🟢 0.1ms | 🟢 0.1ms | -8.6% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes | 🟢 5.6ms | 🟢 5.5ms | -1.3% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts | 🟢 4.5ms | 🟢 4.2ms | -6.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-record | 🟢 0.4ms | 🟢 0.3ms | -5.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes | 🟢 0.5ms | 🟢 0.5ms | +7.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes | 🟢 0.0ms | 🟢 0.0ms | -9.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment | 🟢 0.2ms | 🟢 0.2ms | -1.3% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property | 🟢 0.1ms | 🟢 0.1ms | -2.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator | 🟢 0.0ms | 🟢 0.0ms | -0.1% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb | 🟢 0.1ms | 🟢 0.1ms | -2.3% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property | 🟢 0.1ms | 🟢 0.1ms | +5.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format | 🟢 0.0ms | 🟢 0.0ms | +0.6% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars | 🟢 0.2ms | 🟢 0.2ms | +0.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern | 🟢 0.0ms | 🟢 0.0ms | -16.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation | 🟢 0.2ms | 🟢 0.2ms | -7.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response | 🟢 5.2ms | 🟢 4.5ms | -13.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-patch | 🟢 0.3ms | 🟢 0.3ms | -1.7% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars | 🟢 0.2ms | 🟢 0.2ms | -1.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state | 🟢 0.1ms | 🟢 0.1ms | -3.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels | 🟢 0.1ms | 🟢 0.1ms | +0.0% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/empty-updateable-properties | 🟢 0.1ms | 🟢 0.1ms | -2.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation | 🟢 0.0ms | 🟢 0.0ms | -1.9% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/lro-location-header | 🟡 14.7ms | 🟡 15.1ms | +2.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint | 🟢 0.0ms | 🟢 0.0ms | -13.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers | 🟢 0.3ms | 🟢 0.3ms | -7.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-empty-model | 🟢 0.1ms | 🟢 0.1ms | -10.2% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-override-props | 🟢 0.1ms | 🟢 0.1ms | -3.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation | 🟢 0.2ms | 🟢 0.2ms | +2.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/no-response-body | 🔴 22.3ms | 🔴 21.2ms | -4.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/patch-envelope | 🟢 0.1ms | 🟢 0.1ms | -3.5% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/resource-name | 🟢 0.1ms | 🟢 0.1ms | -3.3% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/secret-prop | 🟢 2.2ms | 🟢 1.9ms | -14.8% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/unsupported-type | 🟢 0.4ms | 🟢 0.4ms | +3.4% |
| ↳ linter/@azure-tools/typespec-azure-resource-manager/version-progression | 🟢 0.0ms | 🟢 0.0ms | -4.8% |
| ↳ linter/@azure-tools/typespec-client-generator-core/csharp-no-url-suffix | 🟢 0.0ms | 🟢 0.5ms | +100.0% |
| ↳ linter/@azure-tools/typespec-client-generator-core/property-name-conflict | 🟢 1.1ms | 🟢 1.1ms | -2.4% |
| ↳ linter/@azure-tools/typespec-client-generator-core/require-client-suffix | 🟢 0.2ms | 🟢 0.2ms | -13.4% |
| emit | 🔴 6.18s | 🔴 6.01s | -2.7% |
| ↳ emit/@azure-tools/typespec-autorest | 🟡 206.8ms | 🟡 201.1ms | -2.7% |
| ↳ emit/@azure-tools/typespec-python | 🔴 4.51s | 🔴 4.40s | -2.3% |
| ↳ emit/@typespec/http-client-js | 🔴 1.19s | 🔴 1.16s | -2.9% |
| ↳ emit/@typespec/openapi3 | 🟢 157.2ms | 🟢 153.1ms | -2.6% |
| ↳ emit/@typespec/openapi3/compute | 🟢 138.0ms | 🟢 133.8ms | -3.1% |
| ↳ emit/@typespec/openapi3/write | 🟢 19.4ms | 🟢 19.3ms | -0.4% |
Averaged across 3 specs (azure-arm-resource-manager, azure-core-dataplane, azure-full).
Threshold: changes > ±5% are highlighted.
🟢 Fast · 🟡 Moderate (stages >200ms, rules >10ms) · 🔴 Slow (stages >400ms, rules >20ms)
haiyuazhang
force-pushed
the
haiyzhan/linter-url-to-uri
branch
11 times, most recently
from
53343bc to
0569c91
Compare
haiyuazhang
requested review from
bterlson,
iscai-msft,
joheredi,
lmazuel,
m-nash,
markcowl,
srnagar and
timotheeguerin
as code owners
…n.tsp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rary tests - Rename rule ID from no-url-suffix to dotnet-no-url-suffix - Rename doc file to dotnet-no-url-suffix.md - Update changesets, README, linter.md references - Fix empty namespace FQN producing invalid .Foo.imageUrl - Remove redundant node_modules filter (compiler handles this) - Add codefix integration tests (resolveCodeFix + applyCodeFix) - Add round-trip test (codefix applied → recompile → no diagnostic) - Add library types test (compiler filters library diagnostics) - 20 tests total, all passing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the dotnet- prefix to align with existing TCGC rule naming conventions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- createAugmentDecoratorCodeFix(): same-file @@decorator, uses short ref - createClientTspAugmentDecoratorCodeFix(): cross-file to client.tsp, uses FQN - Follows compiler's createAddDecoratorCodeFix() API pattern - Rule simplified to ~5 lines of codefix code Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Extract reusable test helper to test-codefix-helpers.ts - Accepts rule + libraryName so any rule can use it - Tests reduced from ~110 lines to ~20 lines - Follows standard tester pattern per reviewer feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nherited test - Use program.sourceFiles instead of async host.readFile (no cast needed) - Add using <namespace> to client.tsp for short refs instead of FQN - Remove library types test (standard linter engine behavior) - Fix inherited test to use @clientName + toBeValid per reviewer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Update core submodule to include multi-file applyCodeFix().toEqual() support (microsoft/typespec#10897) - Migrate codefix tests to use standard tester pattern - Remove custom test-codefix-helpers.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
haiyuazhang
force-pushed
the
haiyzhan/linter-url-to-uri
branch
from
78b2994 to
cc7a534
Compare
- Replace sourceFiles loop with direct .get(clientTspPath) lookup (O(1)) - Move client.tsp content read outside fix closure - Use headerLines array with join for header instead of string concatenation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Types are already in scope when client.tsp is imported via tspconfig. Remove computeRelativeImportPath and model import line per reviewer feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds C# client-name customizations where they can be isolated in a new client.tsp and suppresses existing URL-suffix usages where importing existing client customizations would risk unrelated SDK or generated-output changes. Prepares existing specs for Azure/typespec-azure#4541. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure/azure-rest-api-specs#43818 is the PR fixing the existing violations in current specs. |
Make language scope explicit per reviewer feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds C# client-name customizations where they can be isolated in a new client.tsp and suppresses existing URL-suffix usages where importing existing client customizations would risk unrelated SDK or generated-output changes. Prepares existing specs for Azure/typespec-azure#4541. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Please help me understand why this is blocking rather than a future enhancement. This tool will be impactful in helping partners be successful in authoring Azure SDK packages and remove a non-trivial amount of friction and delay in the process. I'm not well-versed in this space, but my understanding is that the concern here involves the need to emit a line to the TSP config that we would prefer not be present but which also causes no issues. Unless there's a real-world impact that I'm overlooking, we should prioritize moving this forward. If I'm missing something, please help me understand what. //cc: @lmazuel |
|
client.tsp not being imported means it is not part of the main compilation(neither during tsp compile . or in the IDE) which means the rule can never be successful. Not sure what line in the tspconfig you are refering to. |
It's working today in prototype form. So, yes, it can be successful. |
|
If the fixer adds an import line to the tspconfig.yaml as well as the @@clientName in client.tsp then it works fine. Eventually if we ever do this dependency invert we can always remove that part of the fixer but it will be no harm / redundant. |
|
importing that line in the tspconfig.y aml is exactaly the same as importing client.tsp from main.tsp.... |
|
and the "workaround" doesn't even work, it works for the demo but as soon as this rule is enabled and merged every service needs to pass, which means every service needs to add the suppression or read the client.tsp. I have 4 prs migrating all but 6 services for which importing client.tsp cause more issues and the tspconfig import still will cause those issues |
I'm good with whatever path this takes, so long as we've got prioritization and the timelines are equivalent. This tool solves a very real problem and if the fix that we'd prefer is going to be a significant timeline delay, then I'd like to push on a phased approach to get this in now and do the fix after. |
|
if that simplify this PR cannot merge until this check pass |
|
so yes this is why i have split #4564 into multiple phases. THe last 6 services will require more change and we might want to disable the rules for them until we have them migrated but this would allow the many others from benfiting from it from next release |
|
Thanks for the clarification. I do think if we need to push forward with those 6 specs opted out in the short term lets do that as this will be a pretty big win for our partner teams to get the signal with an auto fixer at the earliest stage possible. |
5 participants
Closes #4561
Add
dotnet-no-url-suffixlinter rule that flags model properties ending withUrland suggests usingUrisuffix instead, to follow .NET SDK naming conventions.