neos / rector

Neos Rector Recipes for code migrations
6 stars 4 forks source link

TASK: Correctly migrate Neos\ContentRepository\Utility #54

Closed mhsdesign closed 5 months ago

mhsdesign commented 6 months ago

... at least partially ... to 100% equivalent would be to cast that to a string now but idk how.

NodeName::transliterateFromString($stuff)->value;
kdambekalns commented 6 months ago

Ok, once this ran, there is no chance to detect the places that woud still need a string cast. That might not be an issue, but when strict typing is in effect, it woud create hard-to-find bugs.

Any Rector expert that knows how we could "chain" changes and add the ->value?

kdambekalns commented 5 months ago

Ah, whatever, I guess those places can be found, so LGTM!

mhsdesign commented 5 months ago

Yeah no sorry its a hard nut everything. I assume one could create an all custom rule to do the low level lifting but it would be a bit too much imo

dlubitz commented 5 months ago

We have already something in place for: https://github.com/neos/rector/blob/29b338615e7eda32c172b523039c59bd2d2baee3/config/set/contentrepository-90.php#L425-L425

Not sure if it is suitable for this case, but maybe.

dlubitz commented 5 months ago

Actually it should already get handled: https://github.com/neos/rector/blob/29b338615e7eda32c172b523039c59bd2d2baee3/config/set/contentrepository-90.php#L436-L436

kdambekalns commented 5 months ago

Actually it should already get handled:

🤔 will that (ony) work when you run Rector twice?

dlubitz commented 5 months ago

🤔 will that only work when you run Rector twice?

No, it should also work on the first run, as the rule is defined below your rule.

🤔 will that ____ work when you run Rector twice?

It seems so. I quickly tested it by changing the test case locally to this scenario and it worked.

dlubitz commented 5 months ago

Ok, it's a bit different. This rule just handles string casts. But AFAIS it was already returning a string before.

$a = Utility::renderValidNodeName('foooo');
// becomes
$a = \Neos\ContentRepository\Core\SharedModel\Node\NodeName::transliterateFromString('foooo');

and

$a = (string) Utility::renderValidNodeName('foooo');
// becomes
$a = \Neos\ContentRepository\Core\SharedModel\Node\NodeName::transliterateFromString('foooo')->value;
dlubitz commented 5 months ago

Followup https://github.com/neos/rector/pull/69