Reported automatically by a code-commandments agent.
|
|
| Prophet |
PreferEnumForClosedSetField |
| Location |
src/Workflow/Execution/WorkflowTimelineStep.php:17 |
What's wrong
AUTO-REPENT completeness: two gaps observed when driving repent --input create-enum-class=… --input cases=….
(1) NON-SPATIE-DATA CLASSES — retype is NOT enough; call sites are left broken. The repenter creates the enum and retypes the property, but it does not rewrite the field's literal string ASSIGNMENTS and COMPARISONS in the same/other classes. For a Spatie Data class this is invisible — ::from() casts string->backed-enum on the way in and serializes enum->value on the way out, so e.g. retyping NodeSocketData::$direction to a new SocketDirection enum left the 'input'/'output' literals in fromInputSocket()/fromOutputSocket() untouched and ALL 490 tests still passed. But a plain domain class has no such bridge: WorkflowTimelineStep (a normal class, not Data) sets $this->status = 'running'/'completed'/'skipped' and reads $step->status === 'processing'. Retyping $status to an enum there would TypeError on the string assignments and silently mismatch the === comparisons. So the auto-fix is complete for Data properties but incomplete (and runtime-breaking) for non-Data classes. Suggestion: either (a) also rewrite same-symbol literal string assignments/comparisons of the retyped field to enum cases, or (b) detect that the declaring class is not a Spatie Data subclass and emit a 'manual call-site fixup required' note instead of silently leaving it broken.
(2) REUSE AN EXISTING ENUM — there is no --input to point the field at an enum that already exists; create-enum-class only ever CREATES. The scripture's own examples are reuse cases (Field::$type -> SchemaFieldType, RunPreAnalysisData::$status -> WorkflowRunStatus where the value already comes from that enum), but those can only be done by hand today. A --input enum-class=ExistingEnum (retype only, skip creation) would let the auto-fixer handle the reuse cases the scripture endorses, instead of forcing a manual edit or a duplicate enum.
Evidence: repent on NodeSocketData::$direction (Spatie Data) -> created src/Http/Data/SocketDirection.php + retyped, 490 tests green with the from() string literals untouched (Spatie cast). The same move on WorkflowTimelineStep::$status (plain class) would break.
Flagged code
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.
Reported automatically by a code-commandments agent.
PreferEnumForClosedSetFieldsrc/Workflow/Execution/WorkflowTimelineStep.php:17What's wrong
AUTO-REPENT completeness: two gaps observed when driving
repent --input create-enum-class=… --input cases=….(1) NON-SPATIE-DATA CLASSES — retype is NOT enough; call sites are left broken. The repenter creates the enum and retypes the property, but it does not rewrite the field's literal string ASSIGNMENTS and COMPARISONS in the same/other classes. For a Spatie Data class this is invisible — ::from() casts string->backed-enum on the way in and serializes enum->value on the way out, so e.g. retyping NodeSocketData::$direction to a new SocketDirection enum left the 'input'/'output' literals in fromInputSocket()/fromOutputSocket() untouched and ALL 490 tests still passed. But a plain domain class has no such bridge: WorkflowTimelineStep (a normal class, not Data) sets $this->status = 'running'/'completed'/'skipped' and reads $step->status === 'processing'. Retyping $status to an enum there would TypeError on the string assignments and silently mismatch the === comparisons. So the auto-fix is complete for Data properties but incomplete (and runtime-breaking) for non-Data classes. Suggestion: either (a) also rewrite same-symbol literal string assignments/comparisons of the retyped field to enum cases, or (b) detect that the declaring class is not a Spatie Data subclass and emit a 'manual call-site fixup required' note instead of silently leaving it broken.
(2) REUSE AN EXISTING ENUM — there is no --input to point the field at an enum that already exists; create-enum-class only ever CREATES. The scripture's own examples are reuse cases (Field::$type -> SchemaFieldType, RunPreAnalysisData::$status -> WorkflowRunStatus where the value already comes from that enum), but those can only be done by hand today. A --input enum-class=ExistingEnum (retype only, skip creation) would let the auto-fixer handle the reuse cases the scripture endorses, instead of forcing a manual edit or a duplicate enum.
Evidence: repent on NodeSocketData::$direction (Spatie Data) -> created src/Http/Data/SocketDirection.php + retyped, 490 tests green with the from() string literals untouched (Spatie cast). The same move on WorkflowTimelineStep::$status (plain class) would break.
Flagged code
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.