mglaman / phpstan-drupal

Extension for PHPStan to allow analysis of Drupal code.
https://phpstan-drupal.mglaman.dev/
MIT License
189 stars 72 forks source link

Detect usage of `Symfony\Component\Yaml\Yaml::parse` and suggest `Drupal\Core\Serialization\Yaml::decode` #642

Open Kingdutch opened 7 months ago

Kingdutch commented 7 months ago

Feature request

When developers work with Yaml the autocomplete from IDEs often suggests Yaml::parse which will give them the Symfony component and they may be unaware of the existence of Drupal's Yaml serialization component which is strictly better.

Developers should prefer calling Drupal\Component\Serialization\Yaml::decode which can delegate to Symfony, but will prefer the PHP Yaml extension if it's available which provides a free performance benefit.

mglaman commented 7 months ago

Actually, it should recommend https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Core/Serialization/Yaml.php?ref_type=heads

I know. We have two. But this respects the ability to override it

mglaman commented 7 months ago

@Kingdutch actually, I think we should open a Drupal core issue to kill this class. Drupal\Component\Serialization\Yaml should have a setSerializer method. DrupalKernel as part of bootEnvironment process could call the method to set the serializer.

Drupal\Component\Serialization\Yaml::setSerializer(Settings::get('yaml_parser_class'))

What do you think?

mglaman commented 7 months ago

I went an opened one https://www.drupal.org/project/drupal/issues/3401665

Kingdutch commented 7 months ago

Yeah that sounds like a good Drupal core issue :) I still agree with the title change to get people to switch to the most flexible class first, then they'll eventually automatically get the "deprecation" warning to move to the component when that is more flexible.

We'll need to include some logic in the rule that suggests Drupal\Core\Serialization\Yaml, to check whether that class is not deprecated and suggest Drupal\Component\Serialization\Yaml otherwise (or we risk people getting stuck between a rock and a hard place when the deprecation lands in Drupal).