[detector-report] NewDataObjectDetector

[jessegall]

Detector: NewDataObjectDetector

Report (why the flagged code is CORRECT and the detector is wrong):
A Data object built from values the caller already holds as typed arguments is a construction, not a hydration. new Node(typename: ..., id: ..., slots: ...) is checked by the compiler on every argument; Node::from([...]) replaces that with an untyped array. ::from converts one loose source (a request, a decoded document) into the type; where the source IS the type system there is nothing to convert.

Cleanest design the reporter can conceive:
A Data class constructed with named arguments when every value is already typed at the call site, and ::from() reserved for turning one loose source into the type. That is what these call sites already are.

⚖️ Maintainer litmus: a valid detector-report needs the flagged code to ALREADY BE the
cleanest design. If the design above differs from the flagged code at all, THAT design is
the owed fix — close this report; the fix is still owed.

Where: src/View/Normalizers/RenderableNormalizer.php:62

Code (src/View/Normalizers/RenderableNormalizer.php:62):

  59       */
  60      public function describe(Renderable $renderable, array $slots, array $spawned, bool $gone = false): Node
  61      {
→ 62          return new Node(
  63              typename: Inspector::for($renderable)->kebabName(),
  64              id: $renderable instanceof Identifiable ? $renderable->id : '',
  65              tag: $this->tag($renderable),
  66              style: $this->style($renderable),
  67              state: $renderable instanceof Component ? $renderable->getState() : [],
  68              dynamic: $this->isDynamic($renderable),
  69              gone: $gone,
  70              slots: array_map($this->slot(...), $slots),
  71              modifiers: $this->modifiers($renderable),
  72              transitions: $this->transitions($renderable),
  73              interactions: $this->interactions($renderable, $spawned),
  74              hotkeys: $renderable instanceof Surface ? $renderable->hotkeys() : [],
  75          );
  76      }
  77  
  78      private function isDynamic(Renderable $renderable): bool
  79      {
  80          if ($renderable instanceof Subjected)
  81          {
  82              return true;
  83          }
  84  
  85          // what can be PUT UP can also go, whether or not it remembers anything of its own — and if it
  86          // never says it stands, the next answer cannot know it is still there

Where: src/View/Normalizers/RenderableNormalizer.php:150

Code (src/View/Normalizers/RenderableNormalizer.php:150):

  147  
  148      private function slot(mixed $held): Slot
  149      {
→ 150          return new Slot(el: $held instanceof Node ? $held : (string) $held);
  151      }
  152  
  153      /**
  154       * @param  list<mixed>  $spawned
  155       * @return list<Interaction>
  156       */
  157      private function interactions(Renderable $renderable, array $spawned): array
  158      {
  159          if (! $renderable instanceof Interactive || ! $renderable instanceof Identifiable)
  160          {
  161              return [];
  162          }
  163  
  164          $asked = array_map(
  165              static fn (string $reported) => new Interaction(
  166                  event: $reported::event(),
  167                  action: new Dispatch($reported, $renderable->id, self::asked($reported))->asAction(),
  168                  propagate: false,
  169              ),
  170              $renderable->reports(),
  171          );
  172  
  173          $puts = [];
  174          $place = 0;

Where: src/View/Normalizers/RenderableNormalizer.php:165

Code (src/View/Normalizers/RenderableNormalizer.php:165):

  162          }
  163  
  164          $asked = array_map(
→ 165              static fn (string $reported) => new Interaction(
  166                  event: $reported::event(),
  167                  action: new Dispatch($reported, $renderable->id, self::asked($reported))->asAction(),
  168                  propagate: false,
  169              ),
  170              $renderable->reports(),
  171          );
  172  
  173          $puts = [];
  174          $place = 0;
  175  
  176          foreach ($renderable->spawns() as $spawn)
  177          {
  178              if ($spawn->isCarried())
  179              {
  180                  $puts[] = $this->spawning($spawn, $spawned[$place++] ?? null);
  181              }
  182          }
  183  
  184          $goes = array_map(
  185              static fn (Visit $visit) => new Interaction(
  186                  event: $visit->on::event(),
  187                  action: new Action(ClientAction::Visit->value, ['url' => $visit->url]),
  188                  propagate: false,
  189              ),

Where: src/View/Normalizers/RenderableNormalizer.php:230

Code (src/View/Normalizers/RenderableNormalizer.php:230):

  227  
  228      private function spawning(Spawn $spawn, mixed $described): Interaction
  229      {
→ 230          return new Interaction(
  231              event: $spawn->on::event(),
  232              action: new Action(ClientAction::Spawn->value, [
  233                  'el' => $this->wire($described),
  234                  'anchor' => $spawn->anchor->jsonSerialize(),
  235                  'standing' => $this->stands($described),
  236              ]),
  237              propagate: false,
  238          );
  239      }
  240  
  241      private function stands(mixed $described): bool
  242      {
  243          return $described instanceof Node && $this->state->stands($described->id);
  244      }
  245  
  246      private function tag(Renderable $renderable): string
  247      {
  248          return method_exists($renderable, 'getTag') ? $renderable->getTag()->value : Tag::Div->value;
  249      }
  250  
  251      /**
  252       * @return array<string, string>
  253       */
  254      private function style(Renderable $renderable): array

Where: src/View/Surface.php:105

Code (src/View/Surface.php:105):

  102  
  103      private function offered(Contract $hotkey): Hotkey
  104      {
→ 105          return new Hotkey(
  106              $hotkey->chord()->said,
  107              $hotkey->chord()->written(),
  108              $hotkey->says(),
  109              new Dispatch(HotkeyPressed::class, $this->id, new PressedKey($hotkey::class))->asAction(),
  110          );
  111      }
  112  
  113      protected function compose(): Renderable | array | null
  114      {
  115          $this->foreground->place(...$this->standing->all());
  116  
  117          return [$this->background, $this->content, $this->chrome, $this->foreground, $this->notices];
  118      }
  119  
  120  }

Filed via commandments report from a consumer project.

[jessegall]

Closing: the finding stands — and your own later code agrees with it.

The rule is narrower than the report reads it. NewDataObjectDetector fires only on a rich Data class — one that declares a cast, a name map, nested-Data hydration, or a magic from<X>() factory. That machinery is exactly what new skips. So the finding isn't "prefer ::from on principle"; it is "this class declares hydration, and this call site bypasses it", which means the mapping the class owns is being re-done (or silently dropped) by the caller.

new Node(slots: array_map($this->slot(...), $slots), ...) is that shape: the element derivation sits at the call site rather than in the cast/collection the class declares.

On the litmus — a valid report needs the flagged code to already BE the cleanest design. The same file in #438 and #440 reads Node::from([...]) and Interaction::from([...]), so the flagged construction was superseded by the fix within the day. That's the strongest evidence available that the finding was right and actionable.

If a specific class here is not rich and still fired, that would be a real bug — reopen with the Data class itself and I'll dig in.