Skip to content

Commit 077a484

Browse files
committed
fix: narrow enum advisory to fire only when zero cases carry #[EnumValue]
Partial annotation is the mechanism that will hide internal cases from the public schema once the default flips to opt-in; leaving some cases unannotated is deliberate and must not produce a warning. Under the previous logic the advisory fired for every partial annotation, which would punish exactly the pattern the migration encourages. New rule: fire the E_USER_DEPRECATED advisory only when a #[Type]-mapped enum declares zero #[EnumValue] attributes across all its cases — the signal that the developer has not yet engaged with the opt-in model. Annotating even a single case acknowledges the migration and silences the notice; from that point any combination of annotated and unannotated cases is correct and intentional. Implementation: EnumTypeMapper::mapByClassName() now tracks a single `sawAnyEnumValueAttribute` flag while iterating cases and emits the notice only when the flag stays false. Message rewritten to explain the "at least one case" requirement and point the reader at the intended migration path — including the fact that a bare #[EnumValue] on a single case is a valid acknowledgement. Tests: - Replaces the previous "any case missing" assertion with testEnumWithZeroEnumValueAttributesTriggersDeprecation using a new DescriptionLegacyEnum/Era fixture that declares no #[EnumValue] at all. - Adds testEnumWithPartialEnumValueAttributesIsSilent to lock in the partial-annotation contract against regressions. - Pre-existing Color/Size/Position fixtures gain a bare #[EnumValue] on their first case so their integration tests stop triggering the advisory — demonstrates the minimal upgrade path recommended in the docs. Full suite 539/539 green across cs-check, phpstan, and phpunit, with exactly 1 intentional deprecation coming from the dedicated advisory test. The docs in descriptions.md are updated to reflect the corrected semantics — "partial annotation is intentional and silent".
1 parent 0fb27ec commit 077a484

8 files changed

Lines changed: 133 additions & 46 deletions

File tree

src/Mappers/Root/EnumTypeMapper.php

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
use function array_values;
3131
use function assert;
3232
use function enum_exists;
33-
use function implode;
3433
use function ltrim;
3534
use function sprintf;
3635
use function trigger_error;
@@ -148,15 +147,14 @@ private function mapByClassName(string $enumClass): EnumType|null
148147
$enumCaseDescriptions = [];
149148
/** @var array<string, string> $enumCaseDeprecationReasons */
150149
$enumCaseDeprecationReasons = [];
151-
/** @var list<string> $casesMissingEnumValueAttribute */
152-
$casesMissingEnumValueAttribute = [];
150+
$sawAnyEnumValueAttribute = false;
153151

154152
foreach ($reflectionEnum->getCases() as $reflectionEnumCase) {
155153
$docBlock = $this->docBlockFactory->create($reflectionEnumCase);
156154
$enumValueAttribute = $this->annotationReader->getEnumValueAnnotation($reflectionEnumCase);
157155

158-
if ($enumValueAttribute === null) {
159-
$casesMissingEnumValueAttribute[] = $reflectionEnumCase->getName();
156+
if ($enumValueAttribute !== null) {
157+
$sawAnyEnumValueAttribute = true;
160158
}
161159

162160
$enumCaseDescriptions[$reflectionEnumCase->getName()] = $this->descriptionResolver->resolve(
@@ -183,40 +181,39 @@ private function mapByClassName(string $enumClass): EnumType|null
183181
}
184182
}
185183

186-
$this->warnAboutCasesMissingEnumValueAttribute($enumClass, $casesMissingEnumValueAttribute);
184+
if (! $sawAnyEnumValueAttribute) {
185+
$this->warnEnumHasNoEnumValueAttributes($enumClass);
186+
}
187187

188188
$type = new EnumType($enumClass, $typeName, $enumDescription, $enumCaseDescriptions, $enumCaseDeprecationReasons, $useValues);
189189

190190
return $this->cacheByName[$type->name] = $this->cacheByClass[$enumClass] = $type;
191191
}
192192

193193
/**
194-
* Emits a deprecation notice when a GraphQL-mapped enum exposes one or more cases without a
195-
* matching {@see EnumValue} attribute.
194+
* Emits a deprecation notice when a GraphQL-mapped enum declares zero {@see EnumValue}
195+
* attributes across its cases — the signal that the developer has not yet engaged with
196+
* the opt-in model that a future major release will require.
196197
*
197-
* Today every case is automatically exposed in the schema — this call site keeps that
198-
* behaviour intact. The notice announces the planned migration: a future major release will
199-
* require `#[EnumValue]` on each case that should participate in the schema, mirroring
200-
* `#[Field]`'s opt-in model on classes. Until then, adopters can start annotating cases
201-
* incrementally; the warning lists the specific cases that would be dropped after the
202-
* future default flip so the migration path is mechanical.
198+
* Today every case is automatically exposed in the schema regardless of `#[EnumValue]` —
199+
* this call site keeps that behaviour intact. The notice announces the planned migration:
200+
* a future major release will require at least one `#[EnumValue]`-annotated case per
201+
* `#[Type]`-mapped enum, and only annotated cases will participate in the schema
202+
* (mirroring `#[Field]`'s opt-in model on classes). Partial annotation is deliberately
203+
* allowed and intentionally silent — leaving some cases unannotated is the mechanism that
204+
* hides internal cases from the public schema once the default flips, so it must not
205+
* itself produce a warning.
203206
*
204-
* @param class-string<UnitEnum> $enumClass
205-
* @param list<string> $casesMissingAttribute
207+
* @param class-string<UnitEnum> $enumClass
206208
*/
207-
private function warnAboutCasesMissingEnumValueAttribute(string $enumClass, array $casesMissingAttribute): void
209+
private function warnEnumHasNoEnumValueAttributes(string $enumClass): void
208210
{
209-
if ($casesMissingAttribute === []) {
210-
return;
211-
}
212-
213211
trigger_error(
214212
sprintf(
215-
'Enum "%s" is mapped to a GraphQL enum type but exposes one or more cases without a #[EnumValue] attribute. '
216-
. 'Today every case is automatically exposed; a future major release will require #[EnumValue] on each case that should participate in the schema, mirroring #[Field]\'s opt-in model. '
217-
. 'Add #[EnumValue] to each case you want to keep exposed. Cases currently exposed without an attribute: %s.',
213+
'Enum "%s" is mapped to a GraphQL enum type but declares no #[EnumValue] attributes on any case. '
214+
. 'Today every case is automatically exposed; a future major release will require at least one #[EnumValue]-annotated case per #[Type]-mapped enum, and only annotated cases will participate in the schema (mirroring #[Field]\'s opt-in model on classes). '
215+
. 'Add #[EnumValue] to the case(s) you want to expose — annotating at least one case acknowledges the opt-in model and silences this notice. Cases left unannotated will be hidden from the schema after the future default flip, which is the intended way to keep internal values out of the public API.',
218216
$enumClass,
219-
implode(', ', $casesMissingAttribute),
220217
),
221218
E_USER_DEPRECATED,
222219
);
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TheCodingMachine\GraphQLite\Fixtures\DescriptionLegacyEnum;
6+
7+
use TheCodingMachine\GraphQLite\Annotations\Type;
8+
9+
/**
10+
* Fixture for the deprecation advisory: this enum intentionally declares zero #[EnumValue]
11+
* attributes so the advisory fires. Under the future opt-in default it would have no cases
12+
* exposed at all — which is exactly what the notice is warning about.
13+
*/
14+
#[Type]
15+
enum Era: string
16+
{
17+
case Classical = 'classical';
18+
case Modern = 'modern';
19+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TheCodingMachine\GraphQLite\Fixtures\DescriptionLegacyEnum;
6+
7+
use TheCodingMachine\GraphQLite\Annotations\Query;
8+
9+
class EraController
10+
{
11+
#[Query]
12+
public function era(): Era
13+
{
14+
return Era::Modern;
15+
}
16+
}

tests/Fixtures/Integration/Models/Color.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Models;
66

7+
use TheCodingMachine\GraphQLite\Annotations\EnumValue;
78
use TheCodingMachine\GraphQLite\Annotations\Type;
89

910
#[Type(
@@ -12,6 +13,10 @@
1213
)]
1314
enum Color: string
1415
{
16+
// A bare #[EnumValue] acknowledges the opt-in migration without altering runtime behaviour.
17+
// Remove the attribute or annotate specific cases once the enum actually needs per-case
18+
// description/deprecation metadata.
19+
#[EnumValue]
1520
case Green = 'green';
1621
case Red = 'red';
1722
}

tests/Fixtures/Integration/Models/Position.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44

55
namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Models;
66

7+
use TheCodingMachine\GraphQLite\Annotations\EnumValue;
78
use TheCodingMachine\GraphQLite\Annotations\Type;
89

910
#[Type]
1011
enum Position: int
1112
{
13+
#[EnumValue]
1214
case Off = 0;
1315
case On = 1;
1416
}

tests/Fixtures/Integration/Models/Size.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44

55
namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Models;
66

7+
use TheCodingMachine\GraphQLite\Annotations\EnumValue;
78
use TheCodingMachine\GraphQLite\Annotations\Type;
89

910
#[Type]
1011
enum Size
1112
{
13+
#[EnumValue]
1214
case S;
1315
case M;
1416
case L;

tests/Integration/DescriptionTest.php

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use TheCodingMachine\GraphQLite\Containers\EmptyContainer;
1414
use TheCodingMachine\GraphQLite\Fixtures\Description\Book;
1515
use TheCodingMachine\GraphQLite\Fixtures\DescriptionDuplicate\Book as DuplicateBook;
16+
use TheCodingMachine\GraphQLite\Fixtures\DescriptionLegacyEnum\Era;
1617
use TheCodingMachine\GraphQLite\Schema;
1718
use TheCodingMachine\GraphQLite\SchemaFactory;
1819
use TheCodingMachine\GraphQLite\Security\VoidAuthenticationService;
@@ -105,18 +106,47 @@ public function testEnumValueAttributeProvidesCaseDescription(): void
105106
$this->assertSame('Fiction works including novels and short stories.', $fictionValue->description);
106107
}
107108

108-
public function testEnumWithCasesMissingEnumValueAttributeTriggersDeprecation(): void
109+
public function testEnumWithZeroEnumValueAttributesTriggersDeprecation(): void
109110
{
110-
// The Genre fixture deliberately leaves NonFiction without #[EnumValue] to exercise the
111-
// docblock-fallback path and to surface the deprecation announcing the future opt-in
112-
// migration. PHPUnit 11's expectUserDeprecationMessageMatches hooks into the library's
113-
// own error handler so the assertion works consistently with how deprecations surface
114-
// in CI output.
115-
$this->expectUserDeprecationMessageMatches('/#\[EnumValue\].*future major.*NonFiction/s');
111+
// The Era fixture deliberately declares zero #[EnumValue] attributes — the signal that
112+
// the developer has not yet engaged with the opt-in migration. That is the scenario the
113+
// advisory targets; partial annotation on other enums (like Genre in the Description
114+
// namespace) is deliberately silent because it already acknowledges the new model.
115+
$this->expectUserDeprecationMessageMatches('/declares no #\[EnumValue\] attributes.*future major/s');
116116

117-
$schema = $this->buildSchema(Book::class);
117+
$schema = $this->buildSchema(Era::class);
118118
// Force enum resolution — types are lazy-mapped until referenced.
119-
$schema->getType('Genre');
119+
$schema->getType('Era');
120+
}
121+
122+
public function testEnumWithPartialEnumValueAttributesIsSilent(): void
123+
{
124+
// Genre has #[EnumValue] on Fiction and Poetry but not NonFiction. Partial annotation is
125+
// deliberately OK: leaving a case unannotated is the mechanism for hiding it from the
126+
// public schema after the future default flip, and therefore must not itself produce an
127+
// advisory. Asserting no deprecation here locks that contract in.
128+
$captured = [];
129+
set_error_handler(
130+
static function (int $errno, string $errstr) use (&$captured): bool {
131+
if ($errno === E_USER_DEPRECATED && str_contains($errstr, 'EnumValue')) {
132+
$captured[] = $errstr;
133+
134+
return true;
135+
}
136+
137+
return false;
138+
},
139+
E_USER_DEPRECATED,
140+
);
141+
142+
try {
143+
$schema = $this->buildSchema(Book::class);
144+
$schema->getType('Genre');
145+
} finally {
146+
restore_error_handler();
147+
}
148+
149+
$this->assertSame([], $captured, 'Partial #[EnumValue] annotation must not trigger the advisory notice.');
120150
}
121151

122152
public function testEnumCaseWithoutAttributeFallsBackToDocblock(): void

website/docs/descriptions.md

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,24 +129,40 @@ is an enum value.
129129
Omitting it falls back to the `@deprecated` tag on the case docblock. An explicit empty string
130130
`''` deliberately clears any inherited `@deprecated` tag.
131131

132-
### Future migration: `#[EnumValue]` will become required per case
132+
### Future migration: `#[EnumValue]` will become required per `#[Type]`-mapped enum
133133

134134
Today every case of a `#[Type]`-mapped enum is automatically exposed in the GraphQL schema. A
135-
future major release will flip this to an opt-in model: **only cases carrying an explicit
136-
`#[EnumValue]` attribute will be exposed**, mirroring the way `#[Field]` opts individual
137-
methods and properties into an object type.
135+
future major release will flip this to an opt-in model: **`#[Type]`-mapped enums will need at
136+
least one `#[EnumValue]`-annotated case, and only annotated cases will participate in the
137+
schema**, mirroring the way `#[Field]` opts individual methods and properties into an object
138+
type.
138139

139140
The benefit is selective exposure — today there is no way to map a subset of a PHP enum into
140141
GraphQL, which forces schema authors to split an enum into two or rename cases. Under the
141142
opt-in model, an internal enum case can simply omit `#[EnumValue]` to stay out of the public
142-
schema.
143-
144-
To surface the upcoming change, GraphQLite already emits a PHP `E_USER_DEPRECATED` notice at
145-
schema build time when an enum annotated with `#[Type]` exposes any case without an
146-
`#[EnumValue]` attribute. The notice names the specific cases that would be dropped after the
147-
flip so the migration path is mechanical: add `#[EnumValue]` to every case you want to keep.
148-
No runtime behaviour changes today — the notice only signals what the future default will
149-
require.
143+
schema. **Partial annotation is intentional and deliberately silent**: leaving some cases
144+
unannotated is the mechanism for hiding them from the public schema once the default flips,
145+
not a migration oversight.
146+
147+
To surface the upcoming change, GraphQLite emits a PHP `E_USER_DEPRECATED` notice at schema
148+
build time only when a `#[Type]`-mapped enum declares **zero** `#[EnumValue]` attributes
149+
across its cases — the signal that the developer has not yet engaged with the opt-in model at
150+
all. Annotating at least one case acknowledges the new model and silences the notice; after
151+
that, any combination of annotated and unannotated cases is correct and intentional.
152+
153+
Adopters who aren't ready to decide which cases to expose can silence the advisory with a
154+
bare `#[EnumValue]` on a single case — no description, no deprecation, just the acknowledgement:
155+
156+
```php
157+
#[Type]
158+
enum Size
159+
{
160+
#[EnumValue] // acknowledges the opt-in migration; runtime behaviour unchanged
161+
case S;
162+
case M;
163+
case L;
164+
}
165+
```
150166

151167
**Runtime implication after the flip.** Cases without `#[EnumValue]` will not exist in the
152168
GraphQL schema, so a resolver that returns such a case will trigger webonyx/graphql-php's

0 commit comments

Comments
 (0)