Repository navigation
fix(ruby): stop sub-SDKs named after Ruby or SDK constants from shadowing them - #206
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 93 files
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack
View guided diff | Turn on auto-fix | Re-trigger cubic
23a4842 to
bc7275a
Compare
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes Ruby generator collisions by qualifying core constants with :: and suffix-renaming sub-SDKs that shadow SDK internals, with snapshot tests pinning the corrected output; the renames only affect specs that previously failed to load.
View guided diff | Turn on auto-fix | Re-trigger cubic
Generator Snapshot Testing
Snapshot results: https://github.com/speakeasy-api/openapi-generation-snapshots/issues/162#issuecomment-6755288845 Snapshot results are summarized above. Additional run context is linked for maintainers with access. |
5dd5d29 to
18babcb
Compare
| return `[DateTime](https://ruby-doc.org/stdlib-2.6.1/libdoc/date/rdoc/DateTime.html)`; | ||
| } | ||
| case "date": { | ||
| return `[DateTime](https://ruby-doc.org/stdlib-2.6.1/libdoc/date/rdoc/DateTime.html)`; | ||
| return `[Date](https://ruby-doc.org/stdlib-2.6.1/libdoc/date/rdoc/Date.html)`; |
There was a problem hiding this comment.
🔊 Drive-by fix, unrelated to the constant shadowing: date fields linked to DateTime and date-time fields to Date. Swapped here; covered by its own docs changeset, and the review SDK diff for it is limited to four docs pages.
There was a problem hiding this comment.
0 issues found across 6 reviewed files. 1 file intentionally excluded from review.
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes Ruby generator shadowing by qualifying core constants with :: and renaming colliding sub-SDKs; snapshot tests pin output and renames only affect specs that previously failed to load, so no supported callers break.
View guided diff | Turn on auto-fix | Re-trigger cubic
…wing them Generated Ruby code lives inside the SDK module, and every tag becomes a sub-SDK class directly under it. Unqualified references in the templates therefore resolved to a same-named sub-SDK instead of the intended constant: - `File.extname`, `Time.now`, `Date.iso8601`, `is_a? Hash` and similar failed `srb tc` when a spec had a `File`, `Time`, `Date` or `Hash` tag; - `include Kernel` made the gem fail to load with a `Kernel` tag; - a `Utils` or `SDKHooks` tag replaced the SDK's own module (`Utils is not a class`), and an `SDKConfiguration` tag overwrote `sdkconfiguration.rb` and the `@sdk_configuration` instance variable. Prefix Ruby core and stdlib constants with `::` in the SDK templates and in the TypeScript helpers that emit Ruby code. Usage snippets are top-level Ruby, so their date, date-time, file and environment variable example values stay unqualified; model defaults and generated tests keep the prefix. Sub-SDKs whose class or file name matches an SDK internal (`Utils`, `SDKHooks`, `SDKConfiguration`) get an `SDK` suffix, repeated until the name is free, and a sub-SDK accessor named `sdk_configuration` becomes `sdk_configuration_`. Schemas keep their names: models live under `Models::` and do not shadow these constants. Add snapshot tests for tags that collide with these constants, for reserved names in nested groups and next to a main SDK class that takes the first suffix, and for example values in usage snippets versus generated tests. Compiling them runs `srb tc` on the generated SDKs. Drive-by: Ruby docs linked `date` fields to `DateTime` and `date-time` fields to `Date`; swap the links. Regenerate the Ruby review SDK.
18babcb to
7ac2c05
Compare
speakeasybot
left a comment
There was a problem hiding this comment.
Auto-approved: review:bypass label applied by @2ynn. Required status checks still gate this merge.
Why
Generated Ruby code lives inside the SDK module, and every tag becomes a sub-SDK class directly under it. Unqualified constant references in the templates therefore resolved to a same-named sub-SDK instead of the intended constant:
File.extname,Time.now,Date.iso8601,is_a? Hashand similar failedsrb tcwhen a spec had aFile,Time,DateorHashtag.include Kernelmade the gem fail to load when a spec had aKerneltag.UtilsorSDKHookstag replaced the SDK's own module (Utils is not a class). AnSDKConfigurationtag overwrotesdkconfiguration.rband the@sdk_configurationinstance variable.What changed
::String,::JSON,::File,::StandardError, ...) are prefixed with::in the SDK templates and in the TypeScript helpers that emit Ruby code.rubyConstantRef). Model defaults and generated tests keep the prefix.Utils,SDKHooks,SDKConfiguration,sdkconfiguration.rb) get anSDKsuffix, repeated until the name is free of every other sub-SDK and the main SDK file (subSDKTypeName). A sub-SDK accessor namedsdk_configurationbecomessdk_configuration_(sanitizeSubSDKFieldName).Models::and do not shadow these constants.srb tcand rubocop:TestSnapRubySubSDKCoreConstantCollision: tagsFile,Time,Hash,Class,Kernel,Utils,Date,String,Array,SDKConfiguration,SDKHooks,UtilsSDK.TestSnapRubySubSDKReservedNameNesting:sdkClassName: UtilsSDKwith aUtilstag (sub-SDK becomesUtilsSDKSDK), plus nested groupsadmin.SDKConfigurationandadmin.SDKHooks.TestSnapRubyCoreConstantExampleValues: date, date-time and file example values stay unqualified inUSAGE.mdand use::in generated tests.::prefixes.::qualification, formatter re-wrapping and the matchinggen.lockchecksums.Drive-by: date and date-time doc links
Unrelated to the shadowing fix. Ruby docs linked
datefields toDateTimeanddate-timefields toDate.templates/templates/ruby/includes/documentation.tsnow swaps them back, with its own changeset (docs, patch). In the review SDK this only changes four docs pages and theirgen.lockchecksums.Testing
go test ./pkg/generate/snapshots/namespaces/ -run Ruby -count=1with Ruby 3.2.11 and Go 1.26.10: all 6 Ruby snapshot tests generate, compile (srb tc, rubocop) and match.make lintsteps: prettier,go mod tidy -diff,tsgofor the Ruby templates and golangci-lint all pass.Utils,SDKHooks,SDKConfiguration,sdk_configuration) was generated with main and with this branch, then loaded withSDK.new. On main, all four fail to load (Utils is not a class,SDKHooks is not a module,wrong number of arguments (given 1, expected 6)). On this branch they load and the accessors return the renamed sub-SDKs. The renames therefore only affect SDKs that could not be used before.TARGET=review make build-rubypasses (srb tc, rubocop, mock-server build and staticcheck).srb tcand rubocop (5039 files, no offenses).Known and out of scope: the tag pair
SDKConfiguration+sdkconfiguration(already renamed upstream toSDKConfiguration1/Sdkconfiguration2) still fails at runtime becauseattr_accessoris derived fromType.Namewhile the instance variable is derived fromFieldName. This mismatch predates this change.Public-safety check
.claude/skills/public-repo-communication/SKILL.md).git diff --check.