feat(php): generate invocation-only dynamic snippets - #17407
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…snippets The invocation-only dynamic-snippets hook now returns a structured InvocationSnippetResponse (snippet + imports + clientName + errors) instead of a bare call string. Callers such as documentation code templates can now regenerate the imports and client instantiation alongside the call and keep them in sync across SDK renames. The branded-string-alias case, which previously returned undefined (dropping the call because it referenced an SDK import the caller couldn't supply), now returns the call plus the required import block. generateInvocationSync still returns undefined only for the "generator doesn't implement the hook" capability check the multi-language fan-out relies on. Co-Authored-By: Claude <noreply@anthropic.com>
Mirrors the finalized TypeScript structured contract (PR #17393) and the Python (#17402) / Go (#17403) / Java (#17404) / C# (#17405) ports: alongside the full snippet, the generator now returns InvocationSnippetResponse = { snippet, imports, clientName, errors } for callers (e.g. docs code templates) that render the invocation inside code they already own. - snippet: the bare call (honoring options.clientVariableName), no client construction, no `<?php` prefix or `namespace ...;` header, and no trailing `;`. - imports: the PHP `use ...;` block the call references, captured separately via a new AstNode.toStringWithoutImports helper (backed by Writer.importsToString) — the PHP analogue of the TS AST's toStringWithoutImports and the C#/Java AST helpers. Empty string when the call needs no imports; populated when the invocation constructs types from another namespace inline (e.g. an inlined request class or a DateTime body value). - clientName: the generated client class name (context.getRootClientClassName()). - errors: preserved from the existing error reporter. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
AI Review Summary
Clean port of the invocation-only snippet hook to PHP. Main concern: clientVariableName is passed straight into php.codeblock without normalizing the $ sigil, so a caller passing the language-agnostic client (as TS/Python/Go callers do) yields invalid PHP. Minor nit on the redundant importsToString wrapper.
- 🟡 1 warning(s)
- 🔵 1 suggestion(s)
| }): php.MethodInvocation { | ||
| return php.invokeMethod({ | ||
| on: php.codeblock(CLIENT_VAR_NAME), | ||
| on: php.codeblock(clientVariableName ?? CLIENT_VAR_NAME), |
There was a problem hiding this comment.
🟡 warning
options.clientVariableName comes from the shared, language-agnostic Options contract, so callers will typically pass client / mailchimp (no sigil) — the same value handed to the TS/Python/Go generators. Interpolating it raw produces invalid PHP (mailchimp->endpoints->...). The test only exercises the pre-sigiled "$mailchimp", so this slips through.
Normalize the sigil:
| on: php.codeblock(clientVariableName ?? CLIENT_VAR_NAME), | |
| on: php.codeblock( | |
| clientVariableName != null | |
| ? clientVariableName.startsWith("$") | |
| ? clientVariableName | |
| : `$${clientVariableName}` | |
| : CLIENT_VAR_NAME | |
| ), |
And add a test case passing clientVariableName: "mailchimp".
| */ | ||
| public importsToString(): string { | ||
| return this.stringifyImports(); | ||
| } |
There was a problem hiding this comment.
🔵 suggestion
importsToString is a pure pass-through to stringifyImports. Just widen stringifyImports to public (or rename it) rather than maintaining two names for the same thing.
There was a problem hiding this comment.
🟡 Custom client variable name produces invalid PHP when passed without a dollar sign
The requested client variable name is inserted verbatim into the generated call (php.codeblock(clientVariableName ?? CLIENT_VAR_NAME) at generators/php/dynamic-snippets/src/EndpointSnippetGenerator.ts:177) without adding PHP's required $ prefix, so a caller that passes a plain name gets a code sample that is not valid PHP.
Impact: Documentation samples rendered with a custom client name can be syntactically broken and won't run for users.
Cross-language option is language-agnostic while PHP variables require a leading `$`
clientVariableName is a shared, language-agnostic option ("The name of the variable the endpoint is invoked on", generators/browser-compatible-base/src/dynamic-snippets/Options.ts). The TypeScript port's test passes it as a bare identifier (clientVariableName: "mailchimp", generators/typescript-v2/dynamic-snippets/src/__test__/InvocationSnippet.test.ts:52), whereas the PHP test only exercises the pre-prefixed form ("$mailchimp", generators/php/dynamic-snippets/src/__test__/InvocationSnippet.test.ts:68). With a bare name the PHP generator emits mailchimp->endpoints->httpMethods->testGet(...), which is invalid PHP; the default path is unaffected because CLIENT_VAR_NAME is "$client". Normalizing (prepending $ when absent) would make the PHP generator robust to the shared contract; note PHP's own SDK context also returns the $-prefixed form (generators/php/sdk/src/readme/ReadmeSnippetBuilder.ts:152).
(Refers to lines 176-178)
Was this helpful? React with 👍 or 👎 to provide feedback.
741c4f6 to
c85a00c
Compare
Ports the invocation-only dynamic-snippet hook to the PHP generator, mirroring the finalized TypeScript contract (#17393) and the Python (#17402) / Go (#17403) / Java (#17404) / C# (#17405) ports. Stacked on
devin/1786644600-invocation-only-dynamic-snippets.Structured contract
Alongside the full snippet, the generator now returns
InvocationSnippetResponse = { snippet, imports, clientName, errors }:snippet— the bare call, honoringoptions.clientVariableName(default$client). No$client = new ...Client(...)construction, no<?phpprefix ornamespace ...;header, no trailing;(stripped). Example:$client->endpoints->httpMethods->testGet(\n 'id',\n).imports— the PHPuse ...;block the call references, rendered as a string (""when none).clientName— the generated PHP client class name (context.getRootClientClassName(), e.g.AcmeClient).errors— preserved from the existingErrorReporter.How
useimports are captured (AST change)PHP's
Writeralready tracks references and prepends anamespace ...;+use ...;block intoString(skipImports=false). Mirroring the C#/Java precedent, I split the render:Writer.importsToString()(new,generators/php/codegen/src/ast/core/Writer.ts) — exposes the rendereduse ...;block on its own (empty string when none).AstNode.toStringWithoutImports()(new,generators/php/codegen/src/ast/core/AstNode.ts) — a single write pass yields{ code, imports }:codeis the node body with nonamespace/useheader (writer.toString(true)),importsis theuse ...;block. This is the PHP analogue of the TS AST'stoStringWithoutImports.EndpointSnippetGenerator.callMethodis parameterized withclientVariableName, and the newgenerateInvocationSnippetSyncrenders only the invocation node throughtoStringWithoutImports. Invocations that construct types from another namespace inline (an inlined request class, or aDateTimebody value) surface thoseusestatements rather than bailing to the full snippet; a plain call returnsimports === "".Tests
generators/php/dynamic-snippets/src/__test__/InvocationSnippet.test.ts(5 tests):<?php/namespace/use, no trailing;imports === ""for a bare callclientName=AcmeClientclientVariableName($mailchimp)use Acme\Types\Object\Types\ObjectWithOptionalField;anduse DateTime;are surfacedVerification
pnpm turbo run compile --filter @fern-api/php-dynamic-snippets --filter @fern-api/php-codegen— greenpnpm turbo run test --filter @fern-api/php-dynamic-snippets— 43/43 green (5 new + 38 existing)npx biome check --writeon the 4 changed.tsfiles — clean, no fixesgenerators/php/sdk/changes/unreleased/invocation-only-dynamic-snippets.yml(type: feat);versions.ymluntouchedGenerated with Claude Code