[prophet-report] ExplicitDataFactoryProphet: The [AUTO-FIXABLE] auto-fix is a no-op for the object-typed ::from() sites it…

[jessegall]

Reported automatically by a code-commandments agent.

Prophet	ExplicitDataFactoryProphet
Location	domain/Shop/Projections/Shop.php:242

What's wrong

The [AUTO-FIXABLE] auto-fix is a no-op for the object-typed ::from() sites it detects. 'judge' flags 'ShopData::from($this)' (Shop::toData) as an ExplicitDataFactory SIN ('given a non-array (object)'), but 'commandments repent --prophet=ExplicitDataFactoryProphet' makes NO changes for it (reports 'No changes made'/'No sins to fix') — so the promised migration to an explicit fromX() factory never happens; the runtime FromArrayOnly assert then breaks ~30 test classes. Repent should generate the fromShop() factory and rewrite the call site. Secondary: the closure param-hint case 'fn (Shop $shop) => ShopData::from($shop)' (AppContext.php:163) is not even detected, though the scripture lists 'parameter hints' as resolvable. Repro: set ExplicitDataFactoryProphet severity=sin, run repent --prophet=ExplicitDataFactoryProphet.

Flagged code

return ShopData::from($this);

For the fixer

Decide whether this is a false positive (tighten/guard the prophet), a wrong rule (adjust the rule or its config), or correct-but-unclear (improve the message/scripture). Add a fixture from the flagged code above.

[jessegall]

Done in v1.114.0 — object from() is now genuinely auto-fixable.

repent now performs the migration: XData::from($obj) → XData::from{Type}($obj), and it synthesises the factory on XData:

public static function from{Type}(\Fqcn\Type $x): static
{
    return static::from($x->toArray());
}

Cross-file by design (your call): the factory lands wherever XData is defined — the same file, or any other file, located through the codebase index and written via RepentanceResult::createdFiles. So repenting a flagged call site (e.g. Shop::toData) fixes the whole migration even when ShopData is outside the scoped/flagged set. The call rewrite is gated on successful factory placement — an unreachable Data class (e.g. a vendor one) is left untouched, never a dangling reference.

Secondary (detection): from($param) inside closures / arrow functions is now detected too (your AppContext.php:163 case) — that shipped in v1.113.2.

New DataFactorySynthesizer (NameResolver-qualified FQCNs, idempotent, dedups types, resolves $this/param/property/new/closure-param). Tests cover same-file, cross-file, dedup, idempotency, and the unreachable-gate. Both consumers updated to v1.114.0.

[jessegall]

ExplicitDataFactoryProphet auto-fix is a no-op for the object-typed ::from() sites it detects — repent reports "manual fix required" instead of generating the fromX() factory

Summary

ExplicitDataFactoryProphet is advertised as [AUTO-FIXABLE with: commandments repent], and it correctly detects object-typed ::from() call sites. But commandments repent --prophet=ExplicitDataFactoryProphet cannot fix the most common cases — it makes no changes and reports FAILED (manual fix required). Because this rule is paired with the auto-applied DataClassFromArrayOnly sin (which adds the FromArrayOnly runtime assert), the net effect is a codebase-wide breakage: repent adds the array-only guard to every Data class, but leaves the object call sites unconverted, so the runtime assert(is_array(...)) fires everywhere a Data object is hydrated from a model/request.

Tested on v1.114.0 (after the earlier #44 close). zend.assertions=1.

Reproduction

1. A Data class extending Data directly (gets FromArrayOnly via the DataClassFromArrayOnly auto-fix):

// domain/Shop/Data/ShopDetailsData.php
use App\Support\FromArrayOnly;

class ShopDetailsData extends Data
{
    use FromArrayOnly;           // added by `repent` (DataClassFromArrayOnly, a SIN)
    // ...
}

// domain/Shop/Data/ShopData.php
class ShopData extends ShopDetailsData { /* ... */ }

2. A call site that hydrates from an object (model / $this / request) — extremely common:

// domain/Shop/Projections/Shop.php
public function toData(): ShopData
{
    return ShopData::from($this);   // $this is the Shop projection (a Model)
}

3. judge flags it as an auto-fixable sin:

$ commandments judge --files=domain/Shop/Projections/Shop.php --prophet=ExplicitDataFactoryProphet
SINS: 1 in 1 files
- ExplicitDataFactoryProphet (1)
  domain/Shop/Projections/Shop.php:242
    ShopData::from() is given a non-array (object) — that is the magic dispatch; from() should take an array.

4. repent cannot fix it:

$ commandments repent --prophet=ExplicitDataFactoryProphet --file=domain/Shop/Projections/Shop.php
FAILED (manual fix required): 1
  domain/Shop/Projections/Shop.php (No changes made)

Across our codebase this is 119 sites reported FAILED (manual fix required); only ~6–35 trivially-typed sites auto-convert.

What the scripture promises vs. what happens

The ExplicitDataFactory scripture states:

Argument types are resolved from the AST (parameter hints, $this, property types, new, ->toArray()); when a type cannot be resolved the prophet stays silent rather than guess.

But in practice the auto-fix does not handle the cases it explicitly lists:

Call-site shape	judge detects?	repent fixes?
ShopData::from($this) ($this is the class)	✅ yes	❌ "manual fix required"
ShopData::from($this->_shop) (typed property)	✅ yes	❌ no change
fn (Shop $shop) => ShopData::from($shop) (closure param hint)	❌ not even detected	❌
SomeFilters::from($request) (request → all())	✅ yes	❌ no change

So there are two distinct defects:

1. Fixer gap: for sites it does detect ($this, typed property, typed param), repent reports FAILED (manual fix required) instead of generating the factory + rewriting the call. The expected auto-fix per the scripture is:

// expected: repent generates the factory on the Data class…
public static function fromShop(Shop $shop): self
{
    return static::from($shop->toArray());
}
// …and rewrites the call site:
return ShopData::fromShop($this);

2. Detection gap: closure-parameter type hints (fn (Shop $shop) => ...::from($shop)) are not detected at all, though the scripture lists "parameter hints" as resolvable. These then only blow up at runtime via the FromArrayOnly assert.

The deeper coupling problem (please consider)

Even a correct fromX() auto-fix of return static::from($model->toArray()) is not behavior-preserving for Data classes that rely on Spatie's magic from(Model). Example:

class ShopData extends ShopDetailsData
{
    #[LoadRelation]
    #[DataCollectionOf(ShopChannelData::class)]
    public array $shopChannels = [];

    #[LoadRelation]
    public CompanyDetailsData | null $companyDetails = null;
}

ShopData::from($shopModel) uses Spatie's model normalizer, which honours #[LoadRelation] and loads shopChannels / companyDetails from the model. A generated static::from($shopModel->toArray()) factory passes a plain array — #[LoadRelation] no longer fires, so those relations silently come back empty/null. So the mechanical ->toArray() factory the scripture suggests loses data for any class using #[LoadRelation], request all(), computed accessors, etc.

What needs to be fixed (suggestions)

1. Make DataClassFromArrayOnly depend on ExplicitDataFactory being clean. The FromArrayOnly sin must not be auto-applied (or must be downgraded to a warning) while ExplicitDataFactory sites remain. As written, repent applies the runtime guard (sin) before the call-site migration (warning, manual) — guaranteeing a broken tree. At minimum repent should refuse to add FromArrayOnly to a class that still has un-migrated object ::from() call sites.
2. Close the fixer gap: actually generate the fromX() factory + rewrite the call for the $this / typed-property / typed-param cases the scripture claims to support, instead of FAILED (manual fix required).
3. Close the detection gap: resolve closure-parameter type hints.
4. Account for magic-from(Model) semantics: a naive ->toArray() factory drops #[LoadRelation] / request all() behavior. Either generate a factory that preserves it, or detect these classes and surface that the migration is non-mechanical (don't mark them auto-fixable).

Impact

With DataClassFromArrayOnly configured as a sin (default), a single commandments repent sweep over a large codebase adds FromArrayOnly to hundreds of Data classes and breaks ~30 test classes at runtime, with no auto-fix path for the call sites. We are backing the trait out and parking the rule until this is resolved.