Fix/quirks#27
Merged
Merged
Conversation
…edupe Two bugs were riding the same fixture, both blocking the canonical goparsing/classification corpus from scanning standalone post-Stream M: 1. Lexer indent loss. internal/parsers/grammar/lexer.go collectRawBlock only treated `extensions` / `infoExtensions` as YAML-structural-indent- preserving bodies; `securityDefinitions` rode the Text view, which drops leading whitespace per line. Both schemes' fields collapsed into one flat top-level mapping. Fix: extend the yamlBody predicate to include `securityDefinitions`. 2. Strict-mode duplicate-key abort. Even with indent intact, real-world user godoc can carry copy-paste typos producing duplicate mapping keys; v1's gopkg.in/yaml.v2 silently last-wins-deduped, grammar2's go.yaml.in/yaml/v3 aborts the whole scan. yaml.v3 exposes no public UniqueKeys knob (uniqueKeys is hardcoded true in newDecoder), but decoding into *yaml.Node bypasses the mapping-time check entirely. New helper internal/parsers/yaml/dedupe.go parses to a *yaml.Node, walks the AST dropping earlier-occurrence duplicate keys (last-wins, matching v2 semantics), then node.Decode into the caller's target. All four call sites (Parse, ParseInto, TypedExtensions, UnmarshalBody) route through it. Diagnostic emission on duplicates is intentionally deferred to the yaml-library swap tracked in .claude/plans/forthcoming-features.md §3.1 — the position-tracking library is the right home for AST-with-positions infra; no point building two parallel diagnostic surfaces. Witness: fixtures/enhancements/meta-securitydefs-duplicate-keys/ with api_key declaring `type:` twice on purpose so the dedupe path is locked in alongside the lexer-indent path. Sibling bug discovered during witness construction (blank-line-before-first-body-line trips RemoveIndent's first-line anchor with an empty dedent width) is deferred — no user repro yet. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Building the witness fixture for Q26 (Terms Of Service raw block absorbing the adjacent Schemes keyword instead of terminating at it) revealed the bug no longer reproduces on master. Verified directly against the upstream go-swagger/examples/generated/restapi/doc.go — termsOfService captures cleanly, schemes is its own populated keyword. The fix landed somewhere in Stream M's merge sequence (probably the lexer terminator audit that accompanied M6.5-D's KwSchemes asCommaList → asRawBlock widening — both keywords now share the CtxMeta family, so isSiblingTerminatorFor's family-based overlap fires). The exact commit was not isolated. This fixture is therefore landing as a regression-detector golden rather than a witness-then-fix: the source mirrors go-swagger's generated meta shape exactly (tab-indented overall, 2-space body indent under each keyword, no blank line between TOS body and Schemes). Any future change to the terminator rule, KwSchemes shape, or familyOf categorisation that re-introduces the absorption will turn this test red. Q26 marked RESOLVED. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…n: comparison go-swagger's own codegen emits parameter annotations like `in: Body` (capital B), but the scanner's three doc-text capture sites all did a strict-case map lookup against the closed vocabulary. Mixed-case forms silently miscategorised — `in: Body` defaulted to `query` for parameters, fell into the invalidIn branch for responses, and emitted a "not one of …" diagnostic for routebody. Real-world compat break for anyone scanning generated server stubs. Three sites identified, all reading `in:` by doc-text line scan (the structural reason for not going through grammar's typed KwIn property is documented in internal/builders/parameters/README.md §in-discriminator: grammar attaches pre-annotation lines to prose, not to the property list): - internal/builders/parameters/doc_signals.go scanInLocation - internal/builders/responses/doc_signals.go scanInLocation - internal/parsers/routebody/parameters.go applyParamLine Fix: single canonical helper grammar.NormalizeIn(raw, allowFormAlias) (string, bool) lives in internal/parsers/grammar/ — the same package that already owns KwIn's asEnumOption vocabulary (query/path/header/body/formData). All three sites route through it. Case-insensitive match via strings.EqualFold; canonical return value from the vocabulary slice ensures the emitted spec always carries the OAS v2 canonical spelling regardless of source casing. Q27's `form → formData` v1 affordance for swagger:route inline parameters stays scope-contained: only routebody passes allowFormAlias=true; parameters and responses pass false so the canonical OAS v2 closed vocabulary remains the single source of truth in the rest of the codebase. Removed now-unused validParamIn / validResponseIn / validIn maps and the dead inQuery/inPath/inHeader/inFormData constants from parameters' doc_signals.go. Witness: fixtures/enhancements/in-case-insensitive/ declares one parameter at every standard location with a non-canonical case (in: Body, in: QUERY, in: Path, in: Header, in: FORMDATA) plus a response with mixed-case in: Body. Pre-fix the golden showed every parameter at the `query` default (silent miscategorisation), payload parameter without schema, response schema nil. Post-fix each parameter lands at its canonical location and the response routes through the body branch. Unit coverage: 23 cases in internal/parsers/grammar/in_normalize_test.go covering canonical lowercase, all-caps, mixed-case, whitespace trimming, form-alias on/off semantics, and near-miss rejection. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…ipled asymmetry
Q9's observed-quirks description ("interface methods stay PascalCase
because methods can't carry struct tags") turned out to be stale.
Empirical inspection: interface methods DO auto-jsonify via the
swag/mangling NameMangler.ToJSONName transform (lowercase-first
acronym-aware camelCase — CreatedAt → createdAt, ID → id,
ExternalID → externalId), and have done so since somewhere in Stream M
development. The status update was missed at the time; corrected here.
The real story is a principled asymmetry between the two paths:
- Struct fields mirror encoding/json's tag-or-verbatim rule via
resolvers.ParseJSONTag. A field with no json tag emits its Go name
literally, because that is what json.Marshal would actually produce
on the wire. Auto-mangling the struct path would silently disagree
with the user's running program.
- Interface methods have no natural serialization (encoding/json can't
marshal them without a custom MarshalJSON). The mangler is therefore
a documentation-convention default, not a serialization mirror.
When the author provides swagger:name X on an interface method, X is
emitted exactly as written — the mangler is bypassed. fields.go:
methodCarrier checks fd.JSONName first; the mangler only runs when it
is empty. This contract matters for non-camelCase user input.
Witness: fixtures/enhancements/interface-name-verbatim/ +
integration/coverage_interface_name_verbatim_test.go pin the verbatim
guarantee with PascalCase, snake_case, SCREAMING_CASE, and hyphenated
user inputs alongside a no-override default-path control. Any future
regression that runs the mangler over user-provided JSONName will turn
the test red on the spelling of those four entries.
Documentation: extended internal/builders/schema/README.md
§method-mangler with the principled-asymmetry rationale and the
swagger:name verbatim contract. A future "skip-jsonify-interfaces"
opt-out for the one-size-fits-all mangler is tracked separately as a
v2 candidate.
Q9 marked RESOLVED.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #27 +/- ##
==========================================
+ Coverage 79.55% 79.84% +0.29%
==========================================
Files 69 71 +2
Lines 5738 5786 +48
==========================================
+ Hits 4565 4620 +55
+ Misses 881 875 -6
+ Partials 292 291 -1 ☔ View full report in Codecov by Sentry. |
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
1 participant
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.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist