Skip to content

DirectiveNode extends TextNode but never initializes the typed $content property #198

Description

@robert-stanciu

Blaze version

v1.0.14

Description

DirectiveNode extends TextNode and declares its own constructor without calling parent::__construct(). TextNode promotes a typed, non-nullable public string $content with no default, so on a DirectiveNode that property is never initialized.

src/Parser/Nodes/DirectiveNode.php:

class DirectiveNode extends TextNode
{
    public function __construct(
        public string $name,
        public string $original,
        public ?string $arguments = null,
    ) {
    }

    public function render(): string
    {
        return $this->original;
    }
}

src/Parser/Nodes/TextNode.php:

class TextNode extends Node
{
    public function __construct(
        public string $content,
    ) {}

    public function render(): string
    {
        return $this->content;
    }
}

Reproduction

$n = new Livewire\Blaze\Parser\Nodes\DirectiveNode('if', '@if(true)', '(true)');

$n->render();  // "@if(true)" — fine, render() is overridden
$n->content;   // Error: Typed property Livewire\Blaze\Parser\Nodes\TextNode::$content
               //        must not be accessed before initialization

Impact

Currently latent, not a live bug. DirectiveNode::render() is overridden and returns $original, and I could not find any code path in src/ that reads ->content on a generically-typed node — the existing ->content usages are either SlotNode::content() method calls or Tokenizer's own $this->content.

The risk is future-facing: any code that treats a TextNode polymorphically and reads ->content (a reasonable thing to do given the inheritance) will fatal when handed a DirectiveNode. It also means DirectiveNode doesn't actually satisfy the TextNode contract it declares.

Possible fixes

Either initialize the parent property:

public function __construct(
    public string $name,
    public string $original,
    public ?string $arguments = null,
) {
    parent::__construct($original);
}

…or, if a directive isn't conceptually text, extend Node directly instead of TextNode.

Found while verifying the 1.0.12 → 1.0.14 upgrade against a large Blade codebase — the upgrade itself was clean, this was just noticed in passing.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions