fix(naming): respect casing of top-level names that collide with acronym dictionary#3023
Open
schani wants to merge 7 commits into
Open
fix(naming): respect casing of top-level names that collide with acronym dictionary#3023schani wants to merge 7 commits into
schani wants to merge 7 commits into
Conversation
…nym dictionary (#2328) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
The #2328 fix made splitIntoWords treat a word as an acronym only when it was written in all-uppercase, which also stopped uniformly lowercase words that match the known-acronym dictionary (e.g. "json", "id", "asm") from being styled as acronyms. That regressed generated class names from "JSON" to "Json", "ID" to "Id", etc., breaking compilation for several languages — most visibly Kotlin/Klaxon, where a generated `data class Json` shadows Klaxon's imported `@Json` annotation in the keywords.json fixture. Narrow the exemption to mixed-case words only: a word carrying an explicit capitalization ("Foobar", "Acme") is never forced to an acronym, while a uniformly cased word ("foobar", "FOOBAR", "json") keeps the previous dictionary behavior. This keeps issue #2328 fixed (top-level "Acme" no longer becomes "ACME") and restores keywords.json output byte-for-byte to its pre-change form for java, kotlin, kotlin-jackson, csharp, haskell, elixir and scala3. Update the top-level-name unit test accordingly: lowercase "acme" has no capitalization to respect and stays "ACME", consistent with "json" → "JSON". Co-Authored-By: Claude <noreply@anthropic.com>
Generated-output differences1 files differ — 0 modified, 1 new, 0 deleted |
Generated-output differences1 files differ — 0 modified, 1 new, 0 deleted |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Top-level type name capitalization was not respected when the name (case-insensitively) matched an entry in quicktype's built-in acronym dictionary. For example:
produced
export interface ACME { ... }instead of the expectedexport interface Acme { ... }, because"acme"happens to be in the acronym dictionary and the default TypeScript pascal acronym style all-uppercases any word flagged as an acronym — even when the input word was not actually written in all-caps.Root cause
In
splitIntoWords(packages/quicktype-core/src/support/Strings.ts), a word was markedisAcronymif it was either an all-uppercase run or matched the known-acronym dictionary, independently of casing:The second clause matched the dictionary regardless of whether the source word was actually all-uppercase, so mixed-case input like
Acmeor lowercaseacmewas still flagged as an acronym and re-cased toACMEby the pascal acronym style.Fix
Only treat a dictionary match as an acronym when the word was actually written in all-uppercase in the source name:
This preserves the existing behavior for genuinely all-caps input (
ACMEstaysACME,HTMLParserstill treatsHTMLas an acronym) while no longer forcing mixed-case or lowercase names that happen to collide with the dictionary into all-caps.Test coverage
test/inputs/json/priority/name-style.json(which runs for every language fixture) with a top-level-style-relevant property,"Acme": { "HTMLParser": true }, exercising the same word-splitting path across all languages.test/unit/top-level-name-capitalization.test.ts, which asserts the exact emitted TypeScript identifier for top-level namesAcme,acme,ACME, andHTMLParser— this is needed because round-trip fixtures don't detect identifier casing (generated identifiers are self-consistent), so a unit test is the right complement per repo conventions.Verification
npm run buildpasses.export interface Acme { ... }(previouslyACME); also manually checkedacme→Acme,ACME→ACME,HTMLParser→HTMLParser,Foobar→Foobar,foobar→Foobar.npx vitest run test/unit/— all 167 unit tests pass (26 files).QUICKTEST=true FIXTURE=typescript script/test— all 78/78 TypeScript fixtures pass, including the updatedname-style.jsonunder every renderer-option combination (e.g.acronym-style: pascal).Fixes #2328
🤖 Generated with Claude Code