Improve Twig-sourced code generation and restrict usage#39
Merged
Conversation
Lefthook invokes php-cs-fixer with explicit `{staged_files}`, which
overrides the Finder-level `notPath(['Generated'])` exclusion in
`.php-cs-fixer.php`. That let the fixer reformat generated fixtures
(stripping blank lines between class members) whenever any file
inside a `Generated/` dir was staged, causing the generator and
fixer to fight each turn.
Add a hook-level `exclude: ["**/Generated/**"]` so generated output
is left untouched — it's regenerated by the tool, not hand-edited.
Split the single `FileSource` into `GraphQLFileSource` (for `.graphql` files) and `TwigFileSource` (for `.twig` templates) so the generator can distinguish them and treat them differently. Twig templates render by having the framework instantiate the generated fragment classes for them — userland code shouldn't be constructing or subclassing these. Mark Twig-sourced classes with `restricted: true, restrictInstantiation: true` on the `#[Generated]` attribute so the PHPStan `RestrictedUsageExtension` blocks direct use outside the generating template. `.graphql`-sourced operations stay unrestricted since they're meant to be called explicitly from application code. `OperationClassPlan` keeps the narrower `GraphQLFileSource | InlineSource` union because Twig templates only contain fragment definitions, never operations; `planOperation` throws early if a Twig source ever reaches it.
The PHPStan extension that enforces `#[Generated(restricted: true,
restrictInstantiation: true)]` had no direct coverage — only
implicit via fixtures asserting the attribute was emitted. A
regression in the extension itself (wrong scope check, missing
namespace filter, broken attribute parsing) would pass every
existing test.
Add a second PHPStan run driven by `tests/PHPStan/phpstan.neon`,
invoked as a regular PHPUnit test so it runs as part of the normal
test suite:
- `tests/PHPStan/Generated/{Data,SomeQuery}.php` — fake generated
classes with `#[Generated]` pointing at `AllowedController` as
the source. `Data` is fully restricted; `SomeQuery` is restricted
but not `restrictInstantiation`, so anyone can `new` it.
- `tests/PHPStan/Fixtures/AllowedController.php` — the declared
source; must produce zero restricted-usage errors.
- `tests/PHPStan/Fixtures/NotAllowedController.php` — any other
caller; must produce exactly four errors (method, instantiation,
and two property accesses).
- `tests/PHPStan/RestrictedUsageExtensionTest.php` — single
PHPUnit test that shells out to `vendor/bin/phpstan` with the
test neon and asserts exit 0.
Expected errors live in the neon`s `ignoreErrors` block with exact
messages and `count:` values. Combined with
`reportUnmatchedIgnoredErrors` (default true), the run fails if
any expected error stops firing or an unexpected one appears.
Silence `shipmonk.deadMethod` for the fixture dir in the root
`phpstan.php` so the main run stays clean.
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.
Exclude Generated dirs from php-cs-fixer hook
Lefthook invokes php-cs-fixer with explicit
{staged_files}, which overrides the Finder-levelnotPath(['Generated'])exclusion in.php-cs-fixer.php. That let the fixer reformat generated fixtures (stripping blank lines between class members) whenever any file inside aGenerated/dir was staged, causing the generator and fixer to fight each turn.Add a hook-level
exclude: ["**/Generated/**"]so generated output is left untouched — it's regenerated by the tool, not hand-edited.Restrict Twig-sourced generated classes
Split the single
FileSourceintoGraphQLFileSource(for.graphqlfiles) andTwigFileSource(for.twigtemplates) so the generator can distinguish them and treat them differently.Twig templates render by having the framework instantiate the generated fragment classes for them — userland code shouldn't be constructing or subclassing these. Mark Twig-sourced classes with
restricted: true, restrictInstantiation: trueon the#[Generated]attribute so the PHPStanRestrictedUsageExtensionblocks direct use outside the generating template..graphql-sourced operations stay unrestricted since they're meant to be called explicitly from application code.OperationClassPlankeeps the narrowerGraphQLFileSource | InlineSourceunion because Twig templates only contain fragment definitions, never operations;planOperationthrows early if a Twig source ever reaches it.Add functional tests for RestrictedUsageExtension
The PHPStan extension that enforces
#[Generated(restricted: true, restrictInstantiation: true)]had no direct coverage — only implicit via fixtures asserting the attribute was emitted. A regression in the extension itself (wrong scope check, missing namespace filter, broken attribute parsing) would pass every existing test.Add a second PHPStan run driven by
tests/PHPStan/phpstan.neon, invoked as a regular PHPUnit test so it runs as part of the normal test suite:tests/PHPStan/Generated/{Data,SomeQuery}.php— fake generated classes with#[Generated]pointing atAllowedControlleras the source.Datais fully restricted;SomeQueryis restricted but notrestrictInstantiation, so anyone cannewit.tests/PHPStan/Fixtures/AllowedController.php— the declared source; must produce zero restricted-usage errors.tests/PHPStan/Fixtures/NotAllowedController.php— any other caller; must produce exactly four errors (method, instantiation, and two property accesses).tests/PHPStan/RestrictedUsageExtensionTest.php— single PHPUnit test that shells out tovendor/bin/phpstanwith the test neon and asserts exit 0.Expected errors live in the neon
signoreErrorsblock with exact messages andcount:values. Combined withreportUnmatchedIgnoredErrors` (default true), the run fails if any expected error stops firing or an unexpected one appears.Silence
shipmonk.deadMethodfor the fixture dir in the rootphpstan.phpso the main run stays clean.