Fix PropertyTag::setPropertyName() deprecation warning

[lifinsky]

ErrorException: Deprecated: ltrim(): Passing null to parameter #1 ($string) of type string is deprecated
#18 /vendor/laminas/laminas-code/src/Generator/DocBlock/Tag/ParamTag.php(55): Laminas\Code\Generator\DocBlock\Tag\ParamTag::setVariableName
#17 /vendor/laminas/laminas-code/src/Generator/DocBlock/TagManager.php(59): Laminas\Code\Generator\DocBlock\TagManager::createTagFromReflection
#16 /vendor/laminas/laminas-code/src/Generator/DocBlockGenerator.php(48): Laminas\Code\Generator\DocBlockGenerator::fromReflection
#15 /vendor/laminas/laminas-code/src/Generator/MethodGenerator.php(47): Laminas\Code\Generator\MethodGenerator::fromReflection

[lifinsky]

@alexmerlin

[dgafka]

@Ocramius are you able to help to push that forward?

[lifinsky]

@gsteel

[gsteel]

@lifinsky null is not an acceptable parameter here.

Simply casting with (string) because it might receive a null argument introduces the probability that someone will send an array and we'll have a PHP warning for array to string conversion.

This method should either defend against invalid parameters with assertions, or do nothing (as the caller is simply using it wrong) and wait for someone to refactor this class to be an immutable object with strict type guarantees.

[lifinsky]

@lifinsky null is not an acceptable parameter here.

Simply casting with (string) because it might receive a null argument introduces the probability that someone will send an array and we'll have a PHP warning for array to string conversion.

Done

[lifinsky]

@gsteel The linter didn’t allow casting to (string) because it’s redundant — PHPDoc already defines it as a string. I replaced it with !empty, as done elsewhere.

[samsonasik]

Suggested change

	$this->propertyName = ! empty($propertyName) ? ltrim($propertyName, '$') : '';
	$this->propertyName = ltrim((string) $propertyName, '$');

[lifinsky]

@samsonasik linter does not allow this. Please approve my version.

[lifinsky]

As you can see other methods also have this expression.

[lifinsky]

#207 (comment)

[samsonasik]

I think the issue is why user pass null on the exact signature string, which should never happen, while docblock, should be followed. The notice is expected as it not follow the signature, the refactor then imo should be on userland.

[lifinsky]

I think the issue is why user pass null on the exact signature string, which should never happen, while docblock, should be followed. The notice is expected as it not follow the signature, the refactor then imo should be on userland.

The stack trace was provided above.
This may occur due to incomplete PHPDoc.
In any case, it’s an issue within one of the library’s classes.
The current solution only suppresses the notice and doesn’t break anything internally.
It’s clear that the library needs stricter type control by using strict_types.

[samsonasik]

That's nothing todo with strict_types. That's php feature of deprecating null support on string functions on php 8.1+.

nullable property is related with initialization, while filling its value should follow the doc as a signature.

The empty value already checked on construct via empty, when filling via setter, exact string per signature should be followed.

[lifinsky]

@dgafka maybe we should fix our "client" side is it possible

[samsonasik]

@gsteel use cast is needed to keep existing behaviour, except it needs to go to 4.0.0

[samsonasik]

I am going to close this, this is expected, as signature is string, which not allow null, even docblock should be respected by the app that uses it.

The refactor will require major version.

Thank you for understanding.