Having multiple allowExceptCaseInsensitiveParams together with disallowedAttributes does not work

[ruudk]

I'm trying to prevent the following:

class Entity {
    #[Column(name: 'start_date_parsed', type: 'datetime', nullable: true)]
    private ?DateTimeInterface $parsedStartDate;
}

I can do that with the following config:

<?php

declare(strict_types=1);

return [
    'parameters' => [
        'disallowedAttributes' => [
            [
                'attribute' => Doctrine\ORM\Mapping\Column::class,
                'message' => 'use `utc_datetime_immutable` instead.',
                'allowExceptCaseInsensitiveParams' => [
                    [
                        'name' => 'type',
                        'position' => 2,
                        'value' => 'datetime',
                    ],
                ],
            ],
        ],
    ],
];

It produces:

Attribute Doctrine\ORM\Mapping\Column is forbidden, use `utc_datetime_immutable` instead.

But as soon as I add another disallowed attribute, it stops working:

<?php

declare(strict_types=1);

return [
    'parameters' => [
        'disallowedAttributes' => [
            [
                'attribute' => Doctrine\ORM\Mapping\Column::class,
                'message' => 'use `utc_datetime_immutable` instead.',
                'allowExceptCaseInsensitiveParams' => [
                    [
                        'name' => 'type',
                        'position' => 2,
                        'value' => 'datetime',
                    ],
                ],
            ],
            [
                'attribute' => Doctrine\ORM\Mapping\Column::class,
                'message' => 'use `utc_datetime_immutable` instead.',
                'allowExceptCaseInsensitiveParams' => [
                    [
                        'name' => 'type',
                        'position' => 2,
                        'value' => 'datetime_immutable',
                    ],
                ],
            ],
        ],
    ],
];

It's interesting, because this does seem to work for normal functions:

phpstan-disallowed-calls/disallowed-insecure-calls.neon

Lines 15 to 24 in 90fdf80

	-
	function: 'hash()'
	message: 'use hash() with at least SHA-256 for secure hash, or password_hash() for passwords'
	allowExceptCaseInsensitiveParams:
	1: 'md5'
	-
	function: 'hash()'
	message: 'use hash() with at least SHA-256 for secure hash, or password_hash() for passwords'
	allowExceptCaseInsensitiveParams:
	1: 'sha1'

[ruudk]

For now, this seems to work:

<?php

declare(strict_types=1);

return [
    'parameters' => [
        'disallowedAttributes' => [
            [
                'attribute' => Doctrine\ORM\Mapping\Column::class,
                'message' => 'use `utc_datetime_immutable` instead.',
                'allowExceptParams' => [
                    [
                        'name' => 'type',
                        'position' => 2,
                        'typeString' => '"datetime"|"datetime_immutable"',
                    ],
                ],
            ],
        ],
    ],
];

[spaze]

Thanks for the bug, nice catch! For some reason the isAllowed method is called differently for functions/methods and for attributes:

phpstan-disallowed-calls/src/RuleErrors/DisallowedCallsRuleErrors.php

Lines 56 to 60 in 90fdf80

	if (
	$this->identifier->matches($disallowedCall->getCall(), $name, $disallowedCall->getExcludes())
	&& $this->definedInMatches($disallowedCall, $definedIn)
	&& !$this->allowed->isAllowed($scope, isset($node) ? $node->getArgs() : null, $disallowedCall)
	) {

phpstan-disallowed-calls/src/RuleErrors/DisallowedAttributeRuleErrors.php

Lines 49 to 51 in 90fdf80

	if ($this->allowed->isAllowed($scope, $attribute->args, $disallowedAttribute)) {
	continue;
	}

So that's probably why it works for functions, but not for attributes. (Update: no, it's not, it's almost the same code except the definedInMatches call)

For now, I have written a test f22ee42, a failing one, will fix once there's a bit more brain power. And more sleep 😆

[spaze]

The reason is that attributes and most other "calls" use a simple key when adding items from the config (the name of the thing):

phpstan-disallowed-calls/src/DisallowedAttributeFactory.php

Line 51 in 90fdf80

$disallowedAttributes[$disallowedAttribute->getAttribute()] = $disallowedAttribute;

while calls use a composite key (the reason being that some functions/methods come pre-configured in .neon files and you may want to re-allow them based on the params used):

phpstan-disallowed-calls/src/DisallowedCallFactory.php

Line 65 in 90fdf80

$disallowedCalls[$disallowedCall->getKey()] = $disallowedCall;

This is not very clever but I'm not sure how to do it better, there's always a "but". So for now this will stay.

@ruudk You can use the typeString config as you found out, or you can merge those two attribute configs into one like

[
	'attribute' => AttributeColumn::class,
	'message' => 'use `utc_datetime_immutable` instead.',
	'allowExceptCaseInsensitiveParams' => [
		[
			'name' => 'type',
			'position' => 2,
			'value' => 'datetime',
		],
		[
			'name' => 'type',
			'position' => 2,
			'value' => 'datetime_immutable',
		],
	],
],

Notice there's just one 'attribute' => AttributeColumn::class item but the allowExceptCaseInsensitiveParams array has two records for the type param. I'll add a test for this case.