scramble icon indicating copy to clipboard operation
scramble copied to clipboard

[Bug]: Docblocks above dynamic rule keys are ignored in rules() method

Open hosni opened this issue 2 months ago • 2 comments

What happened?

When using a dynamic array key inside a FormRequest’s rules() method, Scramble does not include the PHPDoc description written above it. It works perfectly with static keys but fails when the key is generated dynamically.

Scramble should attach the docblock description to the field even when the key is defined using a dynamic expression, as long as the key can be resolved at runtime or the expression is documented above it.

How to reproduce the bug

Given a Laravel FormRequest:

public function rules()
{
    return [
            /**
             * The configuration for `fcm` channel.  
             * It is required when you send `fcm` in channels.
             */
            'fcm' => [
                Rule::requiredIf(fn(): bool => $this->needConfigurationForChannel(ChannelEnum::FCM)),
            ],
            /**
             * The Firebase registeration token of the user on it's device.  
             * This is the `token` field which is exists in result of call to:  
             *  https://fcmregistrations.googleapis.com/v1/projects/{PROJECT_ID}/registrations
             */
            'fcm.token' => [
                Rule::requiredIf(fn(): bool => $this->needConfigurationForChannel(ChannelEnum::FCM)),
                'string',
                'min:100',
                'max:200',
            ],
    ];
}

✅ Works perfectly — Scramble shows the above description:

Image

But this version loses the description:

public function rules()
{
    return [
            /**
             * The configuration for `fcm` channel.  
             * It is required when you send `fcm` in channels.
             */
            $this->channelName(ChannelEnum::FCM) => [
                Rule::requiredIf(fn(): bool => $this->needConfigurationForChannel(ChannelEnum::FCM)),
            ],
            /**
             * The Firebase registeration token of the user on it's device.  
             * This is the `token` field which is exists in result of call to:  
             *  https://fcmregistrations.googleapis.com/v1/projects/{PROJECT_ID}/registrations
             */
            $this->channelName(ChannelEnum::FCM, 'token') => [
                Rule::requiredIf(fn(): bool => $this->needConfigurationForChannel(ChannelEnum::FCM)),
                'string',
                'min:100',
                'max:200',
            ],
    ];
}

❌ The field appears in the docs (if Scramble can detect it), but the description is missing:

Image

Package Version

0.12.35

PHP Version

8.3.26

Laravel Version

12.31.1

Which operating systems does with happen with?

Linux

Notes

After inspecting the source, it seems this behavior is caused by how Scramble currently handles array keys and documentation extraction:

  1. In src/Infer/Handler/ArrayItemHandler.php, the key type is set only if it is a LiteralStringType, and there’s even a @todo comment indicating dynamic keys are not yet handled:

    $keyType instanceof LiteralStringType ? $keyType->value : null, // @todo handle cases when key is something dynamic
    

    As a result, any dynamic key (e.g., $this->channelName(ChannelEnum::FCM)) produces an ArrayItemType_ without a resolvable string key.

  2. Then, in src/Support/OperationExtensions/ParameterExtractor/TypeBasedRulesDocumentationRetriever.php, the method getDocNodes() ignores such entries because of this check:

    if (! $key = $this->getArrayItemKey($item)) {
        return []; // <- documentation for dynamic keys is discarded here
    }
    

Because the $key is null for dynamic expressions, the docblock is skipped entirely — even though the ArrayItemType_ may contain a valid docNode attribute.

A possible improvement would be to preserve the PHPDoc node for dynamic keys instead of discarding it, maybe by:

  • Storing it under a placeholder name (e.g. "dynamic_key_<line_number>")
  • Or evaluate the rule and get the key

hosni avatar Oct 15 '25 12:10 hosni

@hosni can you share the source of channelName method?

romalytvynenko avatar Oct 15 '25 18:10 romalytvynenko

@hosni can you share the source of channelName method?

@romalytvynenko Sure, it's a simple string cocat:

    protected function channelName(ChannelEnum $channel, string $append = ''): string
    {
        return $channel->value . ($append ? ".{$append}" : '');
    }

And generally, it does not work with anything other than literal strings.

hosni avatar Oct 19 '25 07:10 hosni

@hosni

Then, in src/Support/OperationExtensions/ParameterExtractor/TypeBasedRulesDocumentationRetriever.php, the method getDocNodes() ignores such entries because of this check

You can check the source of getArrayItemKey – such entries are not ignored due to this check. Scramble stores the type of the array key and tries to handle it even if it is not a string.

It is not easy

You see, the issue here is that it's not easy to extract PHPDoc and associate it with fields: array comments are not available at runtime when evaluating rules. To solve this, Scramble tries to find the corresponding field comment from the type of rules expression.

This works really well in most cases but starts failing when you add more logic (conditionals, concatenation, etc.) that is determined at runtime. Unless Scramble can figure out string literal type from whatever expression you use as the array key, it won't be able to associate comment with a field.

On suggested solutions

Solutions you've suggested won't work.

Using some placeholder name is not an option: it won't give you this link between expression/runtime data. You don't have a line number, for example, when you evaluate the array item value.

The idea with evaluating array keys separately looks good on the surface and will work for cases when rules expression is a single array literal, e.g:

public function rules()
{
    return [$this->getChannelName('foo') => 'integer'];
}

But it will not work for cases when rules expression is something else because you simply don't have access to array key expression nodes:

public function rules()
{
    return [$this->getChannelName('foo') => 'integer'] + (new SomeFormRequest)->rules();
}

I hope that makes sense on why this issue is complex and simple fixes won't suffice.

Possible solutions

There are possible solutions:

  • Store associated PHP parser's expression nodes with type nodes. With it, it will be possible to evaluate array keys even if the type cannot be inferred.
  • Implement type-based evaluator from the ground up.
  • Keep improving Scramble type inference system, adding concatenation and basic conditions support.

These solutions will work but they are really complex and time-consuming. The first option may also require a ton of memory usage and will limit the ability to optimize Scramble's memory footprint. The latest option is hard (conditionals part).

Realistic solution

So in the end, in such cases, I recommend implementing an extension for Scramble's type inference system.

For example, in your case you can describe the return type of the getChannelName method. You'll need to implement MethodReturnTypeExtension and register it as an extension. In it, you have access to arguments passed to the method.

Closing the issue for now. I will definitely consider improving this part, but for now I'd keep it the way it is.

romalytvynenko avatar Nov 16 '25 08:11 romalytvynenko

@romalytvynenko

Thanks for the detailed explanation, it makes sense why Scramble cannot reliably associate PHPDoc with dynamic array keys given how rules are evaluated and how AST nodes are lost when expressions are merged or composed.

I have one follow-up question to make sure I implement the recommended solution correctly.

You mentioned:

“in such cases, I recommend implementing an extension for Scramble's type inference system. You'll need to implement MethodReturnTypeExtension”

My method looks like this:

public function channelName(string $channel): string
{
    return $channel;
}

In my case the argument is always passed as a class constant (or Enums), e.g.:

$this->channelName(WatchChannel::FCM)

So the return value is always a string literal type resolved from the argument.

My questions:

  1. To infer the correct key, should my MethodReturnTypeExtension examine the argument and return a LiteralStringType($channel)?
  2. Will Scramble automatically propagate that string literal type into the ArrayItemType_ so the documentation retriever picks it up?
  3. Is there any minimal example of a working MethodReturnTypeExtension I can model mine on?

If this is the correct approach, I’ll implement the extension and report back.

Thanks again for your complete description.

hosni avatar Nov 16 '25 14:11 hosni