Address PR #2358 review: minimize identifier churn Greptile's review flagged that the original PR shipped several unconditional behaviour changes to the generated API surface, none of which were intrinsic to the bug fixes: * `genServerURLWithVariablesFunctionParams` gained a third argument, breaking any user-supplied custom `server-urls.tmpl` override that called it with the previous two-argument form. * Digit-leading enum values acquired an `N` prefix (e.g. `Variable8443` became `VariableN8443`), an incidental side-effect of routing through `SchemaNameToTypeName` even though the enclosing type prefix already provided a leading letter. * The default-pointer constant was unconditionally renamed from `Default` to `DefaultValue` for every enum-typed variable, breaking adopters whose specs had no collision and whose code compiled cleanly under the old codegen. Greptile also identified a latent bug: when two enum values fold to the same identifier suffix (e.g. `enum: ["foo", "Foo"]`, both `ucFirst` to `Foo`), `SanitizeEnumNames`' numeric-suffix dedup produced `Foo` and `Foo1`, but the template's default-pointer emitted `{{ $v.Default | schemaNameToTypeName | sanitizeGoIdentity }}` without the suffix, so the pointer referenced an undeclared identifier. Changes: - Revert `genServerURLWithVariablesFunctionParams` to its original two-argument signature. A new `ServerObjectDefinition.NewServerFunctionParams` method now returns the full parameter list (typed declared variables plus plain-`string` undeclared placeholders, sorted together alphabetically), and the template calls `{{ .NewServerFunctionParams }}` instead of the helper. Any user-supplied custom template that calls `genServerURLWithVariablesFunctionParams` directly keeps working. - Replace `SanitizeEnumNames` (which forces digit-leading values through `SchemaNameToTypeName` and adds the `N` prefix) with a small in-package helper `serverURLEnumKeys` that uses `UppercaseFirstCharacter` directly, plus the same ``/`1`/`2` numeric-suffix dedup. Digit-leading values stay digit-leading; identifiers for non-colliding specs are byte-for-byte identical to what the pre-PR template produced. - Make the `Default` to `DefaultValue` rename asymmetric. A new `ServerObjectDefinition.EnumDefaultPointers` method pre-computes each enum-typed variable's default-pointer info; it switches to `DefaultValue` only when an enum value's identifier suffix is the literal string "Default" (i.e. exactly the spec pattern that produced #2003's duplicate-const compile error before this fix). Adopters whose specs compiled cleanly under the old codegen keep their `Default` constant. - The same `EnumDefaultPointers` data carries the post-dedup target identifier, so the template emits `const Default = Foo1` rather than recomputing via `schemaNameToTypeName | sanitizeGoIdentity`. The pointer always agrees with whatever name the const-block actually produced. - Drop the `schemaNameToTypeName` and `sanitizeGoIdentity` calls from `server-urls.tmpl` accordingly. The template now iterates pre-computed `.EnumDefaultPointers` and `.NewServerFunctionParams` rather than reconstructing identifier names itself. Test coverage: - `pkg/codegen/server_urls_test.go` gains four `TestEnumDefaultPointers` subtests pinning the asymmetric-rename behaviour, the post-dedup reference, and the absence of the `N` prefix. - `examples/generate/serverurls/api.yaml` adds a server with `enum: ["foo", "Foo"]` exercising the dedup case end-to-end. - `examples/generate/serverurls/gen_test.go` gains `TestServerUrlCaseOnlyEnumCollision` confirming the generated code compiles, the post-dedup constants are distinct, and the default-pointer references the right one. Net effect for adopters of the original `fix/enums` PR vs. `main`: * Helper signature unchanged. * Enum value identifiers unchanged for non-colliding specs. * Default-pointer constant name unchanged for non-colliding specs. * `noDefault`-style declared-but-unused parameters are still dropped (the #2004 fix; the parameter was a no-op). * Function calls `.Valid()` on enum-typed parameters (the #2006 fix). * Specs that previously failed to compile (#2003 collision, case-only-different enum values) now compile. Co-Authored-By: Claude Opus 4.7 (1M context)

Uh oh!

Uh oh!

Client Server URLs: don't generate variables that aren't used #2004

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions