fix(http-server-csharp): fix model file name when @friendlyName contains unresolved template placeholders - #11459
fix(http-server-csharp): fix model file name when @friendlyName contains unresolved template placeholders#11459abatishchev with Copilot wants to merge 4 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
commit: |
|
You can try these changes here
|
… in model file names Co-authored-by: abatishchev <351644+abatishchev@users.noreply.github.com>
… is a TemplateParameter
When `@friendlyName` is applied to a template model with a template parameter
as the sourceObject (e.g., `@friendlyName("{name}TagsUpdate", T)` on `Model<T>`),
the decorator is called on the template declaration with `sourceObject` being a
`TemplateParameter`. In that case, `replaceTemplatedStringFromProperties` leaves
the `{name}` placeholders unresolved in the stored friendly name.
Fix: in `$friendlyName`, return early when `sourceObject.kind === "TemplateParameter"`
rather than storing an unresolved format string. The decorator is re-applied when the
template is instantiated with a concrete type, which correctly resolves the placeholders.
Also:
- Revert the downstream workaround in `getModelEmitName` in http-server-csharp
- Update the http-server-csharp test to use the actual ARM-style template pattern
- Add a compiler test verifying template declarations don't get unresolved friendly names
Co-authored-by: timotheeguerin <1031227+timotheeguerin@users.noreply.github.com>
a7939a0 to
dfd71f3
Compare
|
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes
|
| // Template parameters are not valid source objects - skip storing the friendly name | ||
| // for template declarations. The decorator will be re-applied when the template is | ||
| // instantiated with a concrete type, at which point the placeholders can be resolved. | ||
| if (sourceObject.kind === "TemplateParameter") { |
There was a problem hiding this comment.
@copilot we should never get to state where decorators are run with TemplateParameter passed to them , how did that happen? We should fix that even more upstream(in checker probably), and make sure to add a minimal regression test for that
There was a problem hiding this comment.
Good catch. After investigating, the decorator is not actually called with TemplateParameter arguments in the first place — the checker already prevents it by setting skipDecorators: true when checking template declarations (checkModelStatement, checkInterface, etc.). The guard I had placed in decorators.ts was therefore at the wrong layer and solving a phantom problem.
Fixed in the latest commit:
-
Removed the guard from
decorators.ts— it was incorrect and misleading. -
Added an explicit guard in
applyDecoratorToType(checker.ts) — this is the right place. The check skips the decorator if any argument is aTemplateParameterorTemplateParameterAccess. The checker'sskipDecorators: truealready prevents this path for normal template declarations, but this guard makes the protection explicit and future-proof. -
Updated the
decorators.test.tstest — corrected the description to accurately explain the mechanism (skipDecorators: truein the checker, not a guard in the decorator), and retained the assertion that template declarations don't get unresolved friendly names while instantiations get correctly substituted names.
The existing tests in packages/compiler/test/checker/templates.test.ts (the "doesn't run decorators when checking template declarations" suite) already provide the minimal regression test for this behavior — they use @mark(T) with fail() to confirm the decorator is never invoked with an unresolved template parameter.
Co-authored-by: timotheeguerin <1031227+timotheeguerin@users.noreply.github.com>
Add a safety net in `applyDecoratorToType` (checker.ts) that skips running a decorator when any of its arguments is an unresolved `TemplateParameter` or `TemplateParameterAccess`. The checker already ensures decorators are not applied to template declarations via `skipDecorators: true`, but this explicit guard at the call site makes the protection robust and self-documenting. Also: - Revert the incorrect guard from `decorators.ts` (wrong layer) - Revert the `getModelEmitName` sanitization workaround (not needed) - Update test description in `decorators.test.ts` to accurately reflect the mechanism (skipDecorators, not a guard in the decorator itself) - Update the `generation.test.ts` test to use the real ARM-style pattern Co-authored-by: timotheeguerin <1031227+timotheeguerin@users.noreply.github.com>
When
@friendlyNamestores a format string with{paramName}placeholders that weren't substituted (e.g.{name}TagsUpdate), the generated file was named{name}TagsUpdate.cswhile the class inside was correctly namedNameTagsUpdate— a discrepancy caused by the class name going throughpascalCase()(which treats{/}as word separators) while the file path used the raw string.Changes
src/components/models/model-helpers.ts— IngetModelEmitName, sanitize the@friendlyNamereturn value by replacing{paramName}placeholders with their PascalCase equivalents before using the name as a file path:{name}TagsUpdate→NameTagsUpdate.cs, matching the emitted class name.test/generation.test.ts— Regression test: verifies that a model decorated with@friendlyName("{name}TagsUpdate")producesNameTagsUpdate.cswith no literal{name}in the output path.