Skip to content

Commit 25e0bea

Browse files
mglamanclaude
andauthored
Add rule to detect Symfony\Component\Yaml\Yaml::parse() usage (#978)
* Add rule to detect Symfony\Component\Yaml\Yaml::parse() usage Flags direct calls to Symfony's YAML parser and suggests \Drupal\Component\Serialization\Yaml::decode() instead, which respects the yaml_parser_class setting in Drupal's settings.php. Closes #642 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix rule message: yaml_parser_class is no longer a Drupal setting The actual benefit of Drupal\Component\Serialization\Yaml::decode() over Symfony\Component\Yaml\Yaml::parse() is consistent exception handling (InvalidDataTypeException) and correct parse flags. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Make SymfonyYamlParseRule opt-in via drupal.rules.symfonyYamlParseRule Follows the same pattern as all other rules in this package: disabled by default, enabled in bleedingEdge.neon. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use Scope::resolveName() to handle all Name node variants Handles both FullyQualified and plain Name nodes (from use imports), making the class name resolution explicit rather than relying on the NameResolver having already run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8ddbcc3 commit 25e0bea

File tree

6 files changed

+104
-0
lines changed

6 files changed

+104
-0
lines changed

bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ parameters:
1212
hookRules: true
1313
loggerFromFactoryPropertyAssignmentRule: true
1414
entityStorageDirectInjectionRule: true
15+
symfonyYamlParseRule: true

extension.neon

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ parameters:
3737
hookRules: false
3838
loggerFromFactoryPropertyAssignmentRule: false
3939
entityStorageDirectInjectionRule: false
40+
symfonyYamlParseRule: false
4041
entityMapping:
4142
aggregator_feed:
4243
class: Drupal\aggregator\Entity\Feed
@@ -275,6 +276,7 @@ parametersSchema:
275276
hookRules: boolean()
276277
loggerFromFactoryPropertyAssignmentRule: boolean()
277278
entityStorageDirectInjectionRule: boolean()
279+
symfonyYamlParseRule: boolean()
278280
])
279281
entityMapping: arrayOf(anyOf(
280282
structure([

rules.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ conditionalTags:
3434
phpstan.rules.rule: %drupal.rules.entityStorageDirectInjectionRule%
3535
mglaman\PHPStanDrupal\Rules\Drupal\EntityStoragePropertyAssignmentRule:
3636
phpstan.rules.rule: %drupal.rules.entityStorageDirectInjectionRule%
37+
mglaman\PHPStanDrupal\Rules\Drupal\SymfonyYamlParseRule:
38+
phpstan.rules.rule: %drupal.rules.symfonyYamlParseRule%
3739

3840
services:
3941
-
@@ -58,3 +60,5 @@ services:
5860
class: mglaman\PHPStanDrupal\Rules\Drupal\EntityStorageDirectInjectionRule
5961
-
6062
class: mglaman\PHPStanDrupal\Rules\Drupal\EntityStoragePropertyAssignmentRule
63+
-
64+
class: mglaman\PHPStanDrupal\Rules\Drupal\SymfonyYamlParseRule
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Rules\Drupal;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\StaticCall;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\Rules\Rule;
9+
use PHPStan\Rules\RuleErrorBuilder;
10+
11+
/**
12+
* Detects direct usage of Symfony's YAML parser and suggests the Drupal wrapper.
13+
*
14+
* Drupal\Component\Serialization\Yaml::decode() should be used instead of
15+
* Symfony\Component\Yaml\Yaml::parse() because it wraps exceptions as
16+
* InvalidDataTypeException and applies consistent parse flags.
17+
*
18+
* @implements Rule<StaticCall>
19+
*/
20+
class SymfonyYamlParseRule implements Rule
21+
{
22+
public function getNodeType(): string
23+
{
24+
return StaticCall::class;
25+
}
26+
27+
public function processNode(Node $node, Scope $scope): array
28+
{
29+
if (!($node->class instanceof Node\Name)) {
30+
return [];
31+
}
32+
33+
$className = $scope->resolveName($node->class);
34+
if ($className !== 'Symfony\Component\Yaml\Yaml') {
35+
return [];
36+
}
37+
38+
if (!($node->name instanceof Node\Identifier)) {
39+
return [];
40+
}
41+
42+
if ((string) $node->name !== 'parse') {
43+
return [];
44+
}
45+
46+
return [
47+
RuleErrorBuilder::message(
48+
'Avoid calling Symfony\Component\Yaml\Yaml::parse() directly. Use \Drupal\Component\Serialization\Yaml::decode() instead, which handles exceptions consistently and applies the correct parse flags.'
49+
)
50+
->identifier('drupal.symfonyYamlParse')
51+
->build(),
52+
];
53+
}
54+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Tests\Rules;
4+
5+
use mglaman\PHPStanDrupal\Rules\Drupal\SymfonyYamlParseRule;
6+
use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
7+
8+
final class SymfonyYamlParseRuleTest extends DrupalRuleTestCase
9+
{
10+
protected function getRule(): \PHPStan\Rules\Rule
11+
{
12+
return new SymfonyYamlParseRule();
13+
}
14+
15+
public function testRule(): void
16+
{
17+
$this->analyse(
18+
[__DIR__ . '/data/symfony-yaml-parse.php'],
19+
[
20+
[
21+
'Avoid calling Symfony\Component\Yaml\Yaml::parse() directly. Use \Drupal\Component\Serialization\Yaml::decode() instead, which handles exceptions consistently and applies the correct parse flags.',
22+
6,
23+
],
24+
[
25+
'Avoid calling Symfony\Component\Yaml\Yaml::parse() directly. Use \Drupal\Component\Serialization\Yaml::decode() instead, which handles exceptions consistently and applies the correct parse flags.',
26+
9,
27+
],
28+
]
29+
);
30+
}
31+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
use Symfony\Component\Yaml\Yaml;
4+
5+
// Direct call — should be flagged.
6+
$data = Yaml::parse('foo: bar');
7+
8+
// Fully-qualified call — should be flagged.
9+
$data = \Symfony\Component\Yaml\Yaml::parse('foo: bar');
10+
11+
// Drupal's wrapper — should not be flagged.
12+
$data = \Drupal\Component\Serialization\Yaml::decode('foo: bar');

0 commit comments

Comments
 (0)