Repository navigation
docs: resolve the documentation drift reported in #63 - #68
Merged
Merged
Conversation
… examples The landing page called mapToClass() with a JSON string, but its signature is mapToClass(\stdClass $json, string $class), so the first snippet a new user sees fatals with a TypeError. The method that takes a string is mapToClassFromString(). The setup page built both builders with a static create(), which exists on neither: both are constructed with new(). It also called withJsonMapperClass() (the real name is withJsonMapperClassName()) and withProperyMapper(), which has never existed under that spelling — so the "sets the property mapper twice" chain was really one dead call, and it is dropped rather than corrected. DocBlockAnnotations requires a cache in its constructor, and the factory's create() adds no middleware of its own, so an empty call throws a BuilderException. Both are now shown correctly. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each of these fails before it maps anything: - architecture: DocBlockAnnotations needs a cache argument. - laravel-eloquent: the factory's create() with no arguments builds an empty middleware chain and throws a BuilderException; bestFit() is what was meant. - attributes: the example called getId()/getName() on a class declared with public properties and no getters. - laravel-usage: the class declared $mapper both as a property and as a promoted constructor argument, which is a redeclaration fatal. - value-transformation: the callback was the string 'srtolower'. - abstracts: the object was created without new. - interfaces: the chain never called build(), so $mapper was a builder. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Snippets referenced classes by short name with neither use statements nor fully-qualified names, and the pages were inconsistent about which convention they followed. Every snippet now opens with a use block. Four pages mapped onto \Tests\JsonMapper\Implementation\SimpleObject. The real fixture is JsonMapper\Tests\Implementation\SimpleObject, but a class from the package's test suite does not exist in a reader's application either way, so the examples now define the small class they map onto. The debugging page used Psr\Log\Test\TestLogger, which only ships with psr/log ^1.1 — one of the three majors JsonMapper accepts — so the example is written against a plain PSR-3 logger instead. The creating-middleware page described a MiddlewareInterface::process() taking and returning the object, and an addMiddleware() method on JsonMapper and JsonMapperFactory. The interface method is handle(), it amends its arguments in place, and middleware is added with the builder's withMiddleware(). Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The site tracked the development branch while the released package lagged, with no way for a reader to tell which was which. The docs index now names the release these pages describe, and each middleware and builder page carries an "Available since" line taken from the tag that first shipped the class. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The version was already stated inline in the prose; it moves to the same "Available since" line the other middleware pages now use. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The examples on this site have always written class names out in full, which keeps a snippet short enough to read without scrolling. The earlier commit replaced that with an import block per snippet, which added five or six lines above the code that actually does something. Reverted to the original convention. Snippets that represent a real file in the reader's application — the ones that declare their own namespace — keep their imports, since that is what a reader would write. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The page explained that state is written to a PSR-3 logger but never showed a record, so there was no way to tell what the middleware is useful for without wiring it up first. Adds the record shape (one debug entry per object, with the json, object and propertyMap context keys), the real output for the example above it, and the property map reformatted, since that is the part worth reading when a property is silently left unset. Two behaviours that are not obvious and are easy to get wrong: - Position in the chain decides what you see. Pushed, the property map is fully resolved; unshifted it is still empty, which is the view to use when you want the JSON as it arrived, before renaming or case conversion touched it. - The middleware runs once per object, not once per mapping call, so nested structures log a record per object. All output in this page was captured from a real run against 2.25.1. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The site documents v2, so a reader is assumed to be on it. Telling them the DocBlock annotations middleware exists since 0.1.0, or the debugger since 1.0.0, is noise — the answer is always yes. Drops the line from the seven pages whose middleware predates 2.0 and keeps it on the four APIs a v2 user could actually be missing: rename (2.2.0), value transformation (2.9.0), the strict scalar caster and property mapper builder (2.10.0) and the constructor middleware (2.14.0). Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The alternative was a trailing sentence with the mapping squeezed onto one line, which is where a reader is least likely to look and hardest to read. It now sits in the example next to the addMapping calls it replaces. Both forms verified to give the same result. Refs #63 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Resolves the site-side items of #63.
Every claim in the issue was checked by executing the documented snippets against the real package
(
JsonMapper/JsonMapper,develop, 4 commits past2.25.1) rather than by reading. That changedseveral of the issue's conclusions — see Findings below.
Findings that differ from the issue
mapToClass()is not unreleased. It has existed since 2.14.0. The defect is its signature:mapToClass(\stdClass $json, string $class). The landing page passed a JSON string, so the firstsnippet a new user sees died with a
TypeError. The method that takes a string ismapToClassFromString(). The site was never ahead of the release here.The builder's constructor is
new(), notcreate(). Item 1 resolves in favour of the landingpage.
JsonMapperBuilder::new()was already used correctly on five other pages; only the Setup pageused
::create(), which exists on neither builder.withProperyMapperis a docs-only typo — there is no API defect.git log -Sfinds no commitever introducing that spelling in
src/. No alias and no deprecation are needed, so the follow-upissue item 4 contemplated can be dropped. This also settles item 5: with the typo gone, the
"property mapper set twice" chain was one dead call plus one real one, so the dead call is removed
rather than corrected.
What was fixed
Code that could not run
mapToClass('{json}', …)→TypeErrormapToClassFromString()JsonMapperBuilder::create(),PropertyMapperBuilder::create()::new()withJsonMapperClass()withJsonMapperClassName()withProperyMapper(…)new DocBlockAnnotations()→ArgumentCountErrorcreate()→BuilderException(empty middleware chain)bestFit()getId()/getName()on a class with public properties and no getters->id/->name$mapperdeclared as both a property and a promoted constructor argument → redeclaration fatal'srtolower''strtolower'$object = AbstractShapeWrapper();— nonewnew AbstractShapeWrapper()build(), so$mapperwas a builder->build()addedThe last five were not in the issue.
Copy-pasteability (issue item 6)
The convention is settled as fully-qualified class names with no import block, applied site-wide.
That is what the site already did in most places, and it keeps a snippet short enough to read without
scrolling — an import block costs five or six lines before the reader reaches any working code.
Snippets that represent a real file in the reader's application — the ones declaring their own
namespace— keep their imports, since that is what a reader would actually write. The convention iswritten down in
CLAUDE.mdso it stops drifting.Four pages mapped onto
\Tests\JsonMapper\Implementation\SimpleObject. The real fixture isJsonMapper\Tests\Implementation\SimpleObject(the README's form), but as the issue notes, a classfrom the package's test suite does not exist in a reader's application either way — so those examples
now define the small class they map onto (issue item 3).
The debugging page used
Psr\Log\Test\TestLogger, which ships only withpsr/log ^1.1— one of thethree majors JsonMapper accepts — so it is written against a plain PSR-3 logger instead.
The creating-middleware page's prose was also wrong, which the issue did not catch: it described a
MiddlewareInterface::process()that takes and returns the object, and anaddMiddleware()method onJsonMapper/JsonMapperFactory. The interface method ishandle(), it amends its arguments in place,and middleware is added with the builder's
withMiddleware().Version annotations (issue item 8)
The docs index now names the release these pages describe, and an "Available since" line is added for
each API that landed during 2.x — taken from the tag that first shipped the class:
Middleware that predates 2.0 gets no line. The site documents v2, so a reader is assumed to be on it
and "available since 0.1.0" answers a question nobody has.
Verification
All 31
phpfences were extracted and run against the real package, with canned HTTP responses andstub files for reader-owned and framework classes:
php -l42John Doe,john doe,A Meowtain., …)_site/composer validate --strictis cleanNot executed: the Laravel and Symfony guides and the Eloquent page, which need those frameworks
installed. The Symfony page was checked by hand against the bundle's
services.xml— itsJsonMapper\JsonMappertype-hint matches the registered service id and is correct as written.What is left
Item 7 — the stale README feature list. Out of scope here: the README lives in
JsonMapper/JsonMapperand needs its own PR. It lists 6 features against the site's 11+, omittingconstructor support, rename, value transformation, PHP attributes, Laravel Eloquent, enums and
readonly properties. Its own code example is correct as written, so only the list is stale. Packagist
renders that README, so the package's most-visited page understates the feature set by roughly half.
The preventative measures. Also
JsonMapper/JsonMapperwork:built to verify this PR is a working starting point.
middleware index.
A convention to keep. New middleware and builder pages should carry the "Available since" line, and
the version named on the docs index needs bumping at each release.
Refs #63
🤖 Generated with Claude Code