Skip to content

Commit 1c09cb9

Browse files
authored
Merge pull request #8 from InteractionDesignFoundation/refactor/align-init-props-with-plugin-laravel
refactor: align framework-init props with plugin-laravel
2 parents c282c6b + 2a52ad7 commit 1c09cb9

1 file changed

Lines changed: 46 additions & 31 deletions

File tree

src/NovaSuppressHandler.php

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -75,26 +75,30 @@ final class NovaSuppressHandler implements AfterCodebasePopulatedInterface
7575
];
7676

7777
/**
78-
* Nova convention properties declared on a base class via a `@var` docblock (or a native
79-
* nullable) with no default: `Field::$name`, a metric's `$name`, a trend result's `$prefix`,
80-
* etc. Nova assigns them through the constructor, a setter or reflection — never at
81-
* declaration — so every concrete subclass that does not redeclare them is reported as
82-
* PropertyNotSetInConstructor. Keyed by the FQCN the property is declared on (lowercased).
78+
* Nova convention properties Nova assigns through a constructor, a setter or reflection —
79+
* never at declaration — yet declares with a `@var` docblock (or a native nullable) and no
80+
* default. Every concrete subclass that does not redeclare them is reported
81+
* PropertyNotSetInConstructor. Keyed by the FQCN that declares the property.
8382
*
84-
* Marking each as "initialized" on its declaring storage (see markFrameworkInitialised) is
85-
* per-property precise: ClassAnalyzer reads the flag from the DECLARING class
86-
* (Psalm\Internal\Analyzer\ClassAnalyzer::checkPropertyInitialization), so every subclass
87-
* stops flagging that one property while a subclass's OWN uninitialised typed property still
88-
* flags. No stub default value (which would be fiction — the runtime value is never that) and
89-
* no class-level suppression (which would hide a subclass's real bugs) is needed.
83+
* `$name` is the display name on Fields, Actions, Metrics, Filters and Lenses; a Field's
84+
* `$attribute` and `$resource` are set during resolution; a trend result's `$prefix`/
85+
* `$suffix`/`$format` are set when the metric builds its result; `$dependentShouldEmitChangesEvent`
86+
* lives on the `DependentFields` trait and is populated lazily by `dependentShouldEmit()`.
87+
*
88+
* Marking each initialized on its declaring storage (see markFrameworkInitializedProperties)
89+
* mirrors psalm/plugin-laravel's `Handlers\Diagnostics\SuppressHandler`: it is per-property
90+
* precise, so a subclass's OWN un-initialised typed property still reports. Preferred over a
91+
* stub default (whose value would be fiction, and which trips NonInvariantPropertyType on
92+
* subclasses that redeclare the property untyped) and over class-level issue suppression
93+
* (which would hide real bugs).
9094
*/
91-
private const FRAMEWORK_INITIALISED_PROPERTIES = [
92-
'laravel\nova\fields\field' => ['name', 'attribute', 'resource', 'dependentShouldEmitChangesEvent'],
93-
'laravel\nova\actions\action' => ['name'],
94-
'laravel\nova\filters\filter' => ['name'],
95-
'laravel\nova\lenses\lens' => ['name'],
96-
'laravel\nova\metrics\metric' => ['name'],
97-
'laravel\nova\metrics\trendresult' => ['prefix', 'suffix', 'format'],
95+
private const FRAMEWORK_INITIALIZED_PROPERTIES_BY_FQCN = [
96+
'Laravel\Nova\Fields\Field' => ['name', 'attribute', 'resource', 'dependentShouldEmitChangesEvent'],
97+
'Laravel\Nova\Actions\Action' => ['name'],
98+
'Laravel\Nova\Filters\Filter' => ['name'],
99+
'Laravel\Nova\Lenses\Lens' => ['name'],
100+
'Laravel\Nova\Metrics\Metric' => ['name'],
101+
'Laravel\Nova\Metrics\TrendResult' => ['prefix', 'suffix', 'format'],
98102
];
99103

100104
#[\Override]
@@ -103,7 +107,7 @@ public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event
103107
$codebase = $event->getCodebase();
104108
$provider = $codebase->classlike_storage_provider;
105109

106-
self::markFrameworkInitialised($provider);
110+
self::markFrameworkInitializedProperties($provider);
107111

108112
foreach ($provider::getAll() as $classStorage) {
109113
if (!$classStorage->user_defined || $classStorage->is_interface) {
@@ -168,32 +172,43 @@ private static function hasStringCompatiblePolicyShape(
168172
* Flag each Nova convention property as initialised on the class that declares it, so
169173
* PropertyNotSetInConstructor is not raised on subclasses that inherit it without a redeclaration.
170174
*/
171-
private static function markFrameworkInitialised(ClassLikeStorageProvider $provider): void
175+
/**
176+
* Mark framework-initialized properties as initialized on the class that declares them.
177+
*
178+
* `declaring_property_ids` is populated by Psalm's Populator after inheritance resolution, so
179+
* a property inherited via a trait points at the trait's storage and a property declared on
180+
* the parent class points at the parent. Writing `initialized_properties[$name] = true` there
181+
* is the same signal Psalm emits for properties with a default value, which the
182+
* PropertyNotSetInConstructor check honours without further configuration. The user's own
183+
* declared (and genuinely un-initialised) properties are unaffected.
184+
*/
185+
private static function markFrameworkInitializedProperties(ClassLikeStorageProvider $provider): void
172186
{
173-
foreach (self::FRAMEWORK_INITIALISED_PROPERTIES as $baseClass => $propertyNames) {
174-
if (!$provider->has($baseClass)) {
187+
foreach (self::FRAMEWORK_INITIALIZED_PROPERTIES_BY_FQCN as $className => $propertyNames) {
188+
if (!$provider->has($className)) {
175189
continue;
176190
}
177191

178-
$baseStorage = $provider->get($baseClass);
192+
$classStorage = $provider->get($className);
193+
179194
foreach ($propertyNames as $propertyName) {
180-
// A property pulled in from a trait is declared on the trait, not the class using
181-
// it; resolve to the real declaring storage so the flag is read from the same place
182-
// ClassAnalyzer looks it up.
183-
$declaringClass = $baseStorage->declaring_property_ids[$propertyName] ?? $baseClass;
195+
$declaringClass = $classStorage->declaring_property_ids[$propertyName] ?? null;
196+
if ($declaringClass === null) {
197+
continue;
198+
}
199+
184200
if (!$provider->has($declaringClass)) {
185201
continue;
186202
}
187203

188-
self::markInitialised($provider->get($declaringClass), $propertyName);
204+
self::markPropertyInitialized($provider->get($declaringClass), $propertyName);
189205
}
190206
}
191207
}
192208

193-
/** Mutates the passed storage (kept separate so the side effect is on a parameter, not a call result). */
194-
private static function markInitialised(ClassLikeStorage $classStorage, string $propertyName): void
209+
private static function markPropertyInitialized(ClassLikeStorage $storage, string $propertyName): void
195210
{
196-
$classStorage->initialized_properties[$propertyName] = true;
211+
$storage->initialized_properties[$propertyName] = true;
197212
}
198213

199214
private static function suppressHookMethods(ClassLikeStorage $classStorage): void

0 commit comments

Comments
 (0)