Skip to content

Commit 723ec03

Browse files
mtk3dmtk3d
authored
Refactor/static analyse improvement (#51)
* Update phpstan and install phpstan symfony plugin * Add phpstan extension to configuration * Remove unused property from test class * Change static:: to self:: for const refs * Remove unecessery psalm suppressions * Refactor to use phpstan symfony plugin There is a problem in phpstan with getting the right type of root node from tree builder. It can be fixed using phpstan symfony plugin, but it requires to instantiate TreeBuilder in place where you get root node, to allow phpstan to check second parameter of TreeBuilder. This change is fixing some problems that was ignored before. * Replace TreeBuilder instancing to shorter notation
1 parent f52dd35 commit 723ec03

File tree

6 files changed

+23
-62
lines changed

6 files changed

+23
-62
lines changed

‎composer.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@
4444
"mikey179/vfsstream": "^1.6",
4545
"nyholm/psr7": "^1.4",
4646
"phan/phan": "^4.1",
47-
"phpstan/phpstan": "^0.12.50",
48-
"phpstan/phpstan-phpunit": "^0.12.16",
47+
"phpstan/phpstan": "^1.4",
48+
"phpstan/phpstan-phpunit": "^1.0",
49+
"phpstan/phpstan-symfony": "^1.1",
4950
"phpunit/phpunit": "^9.5",
5051
"psalm/plugin-phpunit": "^0.13.0",
5152
"symfony/config": "^4.4|^5.0|^6.0",

‎phpstan.neon.dist‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
includes:
22
- vendor/phpstan/phpstan-phpunit/extension.neon
3+
- vendor/phpstan/phpstan-symfony/extension.neon
34

45
parameters:
56
tmpDir: var/cache/phpstan

‎src/Symfony/OtelSdkBundle/DependencyInjection/Configuration.php‎

Lines changed: 13 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use Psr\Log\LoggerInterface;
1010
use ReflectionClass;
1111
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
12-
use Symfony\Component\Config\Definition\Builder\NodeBuilder;
1312
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
1413
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
1514
use Symfony\Component\Config\Definition\ConfigurationInterface;
@@ -103,14 +102,12 @@ class Configuration implements ConfigurationInterface
103102
];
104103

105104
// PRIVATE CONSTANTS
106-
private const ARRAY_NODE_TYPE = 'array';
107105
private const SCALAR_NODE_TYPE = 'scalar';
108106
private const RESOURCE_XML = 'resource';
109107
private const PROCESSORS_XML = 'processor';
110108
private const EXPORTERS_XML = 'exporter';
111109
private const OPTIONS_XML = 'option';
112110
private const ENV_PREFIX = 'env_';
113-
private const SERVICE_PREFIX = '@';
114111
private const EXPORTER_HR = 'span exporter';
115112
private const PROCESSOR_HR = 'span processor';
116113

@@ -130,10 +127,8 @@ public function __construct(bool $debug = false, ?LoggerInterface $logger = null
130127
public function getConfigTreeBuilder(): TreeBuilder
131128
{
132129
$this->debug('Start building config tree in ' . __CLASS__);
133-
$treeBuilder = self::createTreeBuilder(static::ROOT_KEY);
134-
/** @phpstan-ignore-next-line */
130+
$treeBuilder = new TreeBuilder(self::ROOT_KEY);
135131
$rootNode = $treeBuilder->getRootNode()
136-
/** @psalm-suppress PossiblyUndefinedMethod **/
137132
->canBeDisabled()
138133
;
139134
self::addResourceSection($rootNode);
@@ -144,14 +139,6 @@ public function getConfigTreeBuilder(): TreeBuilder
144139
return $treeBuilder;
145140
}
146141

147-
private static function createTreeBuilder(
148-
string $name,
149-
string $type = self::ARRAY_NODE_TYPE,
150-
NodeBuilder $builder = null
151-
): TreeBuilder {
152-
return new TreeBuilder($name, $type, $builder);
153-
}
154-
155142
private static function addResourceSection(ArrayNodeDefinition $node): void
156143
{
157144
$node
@@ -178,10 +165,8 @@ private static function addTraceSection(ArrayNodeDefinition $node): void
178165

179166
private static function createResourceLimitsNode(): NodeDefinition
180167
{
181-
/** @phpstan-ignore-next-line */
182-
return self::createTreeBuilder(self::LIMITS_NODE)
168+
return (new TreeBuilder(self::LIMITS_NODE))
183169
->getRootNode()
184-
/** @psalm-suppress PossiblyUndefinedMethod **/
185170
->children()
186171
->integerNode(self::ATTR_COUNT_NODE)
187172
->defaultValue(self::LIMITS_COUNT_DEFAULT)
@@ -195,8 +180,7 @@ private static function createResourceLimitsNode(): NodeDefinition
195180

196181
private static function createResourceAttributesNode(): NodeDefinition
197182
{
198-
/** @phpstan-ignore-next-line */
199-
return self::createTreeBuilder(self::ATTRIBUTES_NODE)
183+
return (new TreeBuilder(self::ATTRIBUTES_NODE))
200184
->getRootNode()
201185
->beforeNormalization()
202186
->ifTrue(static function ($v) {
@@ -217,7 +201,6 @@ private static function createResourceAttributesNode(): NodeDefinition
217201
);
218202
})
219203
->end()
220-
/** @psalm-suppress PossiblyUndefinedMethod **/
221204
->fixXmlConfig(self::RESOURCE_XML)
222205
->useAttributeAsKey(self::NAME_KEY)
223206
->prototype(self::SCALAR_NODE_TYPE)
@@ -227,16 +210,14 @@ private static function createResourceAttributesNode(): NodeDefinition
227210

228211
private static function createSamplerSectionNode(): NodeDefinition
229212
{
230-
/** @phpstan-ignore-next-line */
231-
return self::createTreeBuilder(self::SAMPLER_NODE)
213+
return (new TreeBuilder(self::SAMPLER_NODE))
232214
->getRootNode()
233215
->beforeNormalization()
234216
->ifString()
235217
->then(static function ($v) {
236218
return [self::ROOT_NODE => $v];
237219
})
238220
->end()
239-
/** @psalm-suppress PossiblyUndefinedMethod **/
240221
->append(self::createSamplerNode(self::ROOT_NODE))
241222
->children()
242223
->arrayNode(self::REMOTE_NODE)
@@ -253,8 +234,7 @@ private static function createSamplerSectionNode(): NodeDefinition
253234

254235
private static function createSamplerNode(string $name, string $default = self::SAMPLER_NODE_DEFAULT): NodeDefinition
255236
{
256-
/** @phpstan-ignore-next-line */
257-
return self::createTreeBuilder($name)
237+
return (new TreeBuilder($name))
258238
->getRootNode()
259239
->beforeNormalization()
260240
->ifString()
@@ -290,7 +270,6 @@ private static function createSamplerNode(string $name, string $default = self::
290270
return $v;
291271
})
292272
->end()
293-
/** @psalm-suppress PossiblyUndefinedMethod **/
294273
->children()
295274
->append(self::createTypeNode($default))
296275
->append(self::createClassNode())
@@ -302,10 +281,8 @@ private static function createSamplerNode(string $name, string $default = self::
302281

303282
private static function createSpanSectionNode(): NodeDefinition
304283
{
305-
/** @phpstan-ignore-next-line */
306-
return self::createTreeBuilder(self::SPAN_NODE)
284+
return (new TreeBuilder(self::SPAN_NODE))
307285
->getRootNode()
308-
/** @psalm-suppress PossiblyUndefinedMethod **/
309286
->children()
310287
->append(self::createSpanLimitsNode())
311288
->append(self::createSpanProcessors())
@@ -315,10 +292,8 @@ private static function createSpanSectionNode(): NodeDefinition
315292

316293
private static function createSpanLimitsNode(): NodeDefinition
317294
{
318-
/** @phpstan-ignore-next-line */
319-
return self::createTreeBuilder(self::LIMITS_NODE)
295+
return (new TreeBuilder(self::LIMITS_NODE))
320296
->getRootNode()
321-
/** @psalm-suppress PossiblyUndefinedMethod **/
322297
->children()
323298
->integerNode(self::ATTR_COUNT_NODE)->defaultValue(self::LIMITS_COUNT_DEFAULT)->end()
324299
->integerNode(self::ATTR_VALUE_LENGTH_NODE)->defaultValue(PHP_INT_MAX)->end()
@@ -332,14 +307,12 @@ private static function createSpanLimitsNode(): NodeDefinition
332307

333308
private static function createSpanProcessors(): NodeDefinition
334309
{
335-
/** @phpstan-ignore-next-line */
336-
return self::createTreeBuilder(self::PROCESSORS_NODE)
310+
return (new TreeBuilder(self::PROCESSORS_NODE))
337311
->getRootNode()
338312
->beforeNormalization()
339313
->ifString()
340314
->castToArray()
341315
->end()
342-
/** @psalm-suppress PossiblyUndefinedMethod **/
343316
->fixXmlConfig(self::PROCESSORS_XML)
344317
->arrayPrototype()
345318
->beforeNormalization()
@@ -388,10 +361,8 @@ private static function createSpanProcessors(): NodeDefinition
388361

389362
private static function createExporters(): NodeDefinition
390363
{
391-
/** @phpstan-ignore-next-line */
392-
return self::createTreeBuilder(self::EXPORTERS_NODE)
364+
return (new TreeBuilder(self::EXPORTERS_NODE))
393365
->getRootNode()
394-
/** @phpstan-ignore-next-line */
395366
->requiresAtLeastOneElement()
396367
->beforeNormalization()
397368
->ifString()
@@ -423,20 +394,19 @@ private static function createExporters(): NodeDefinition
423394

424395
private static function createClassNode(): NodeDefinition
425396
{
426-
return self::createTreeBuilder(self::CLASS_NODE, self::SCALAR_NODE_TYPE)
397+
return (new TreeBuilder(self::CLASS_NODE, self::SCALAR_NODE_TYPE))
427398
->getRootNode();
428399
}
429400

430401
private static function createIdNode(): NodeDefinition
431402
{
432-
return self::createTreeBuilder(self::ID_NODE, self::SCALAR_NODE_TYPE)
403+
return (new TreeBuilder(self::ID_NODE, self::SCALAR_NODE_TYPE))
433404
->getRootNode();
434405
}
435406

436407
private static function createOptionsNodes(): NodeDefinition
437408
{
438-
/** @phpstan-ignore-next-line */
439-
return self::createTreeBuilder(self::OPTIONS_NODE, self::ARRAY_NODE_TYPE)
409+
return (new TreeBuilder(self::OPTIONS_NODE))
440410
->getRootNode()
441411
->fixXmlConfig(self::OPTIONS_XML)
442412
->useAttributeAsKey(self::NAME_KEY)
@@ -445,7 +415,7 @@ private static function createOptionsNodes(): NodeDefinition
445415

446416
private static function createTypeNode(?string $default = null): NodeDefinition
447417
{
448-
$node = self::createTreeBuilder(self::TYPE_NODE, self::SCALAR_NODE_TYPE)
418+
$node = (new TreeBuilder(self::TYPE_NODE, self::SCALAR_NODE_TYPE))
449419
->getRootNode()
450420
->isRequired();
451421

@@ -510,11 +480,6 @@ private static function isEnvVarReference(string $value): bool
510480
return stripos($value, self::ENV_PREFIX) === 0;
511481
}
512482

513-
private static function isServiceReference(string $value): bool
514-
{
515-
return stripos($value, self::SERVICE_PREFIX) === 0;
516-
}
517-
518483
private static function validateTypedExporterConfig(array $config)
519484
{
520485
// custom exporter

‎src/Symfony/OtelSdkBundle/DependencyInjection/OtelSdkExtension.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,6 @@ private function registerExporterFactoryDefinition(string $exporterKey, array $c
371371
$class = $this->resolveExporterClass($config);
372372

373373
$id = self::concatId(
374-
/** @psalm-suppress ArgumentTypeCoercion */
375374
ServiceHelper::classToId(SpanExporters::NAMESPACE) . '.' . self::FACTORY_SUFFIX,
376375
$exporterKey
377376
);
@@ -432,7 +431,6 @@ private function registerSpanProcessor(Definition $definition, string $processor
432431
{
433432
$id = sprintf(
434433
'%s.%s.%s',
435-
/** @psalm-suppress ArgumentTypeCoercion */
436434
ServiceHelper::classToId(SpanProcessors::NAMESPACE),
437435
$processorType,
438436
$exporterKey
@@ -620,14 +618,14 @@ private function registerService(string $id, Definition $definition, bool $class
620618
{
621619
$this->getContainer()->setDefinition($id, $definition);
622620
if ($classAlias === true) {
623-
if ($definition->getClass() === null) {
621+
$className = $definition->getClass();
622+
if ($className === null) {
624623
throw new RuntimeException(sprintf(
625624
'Cannot set class alias for id "%s". Definition has not class',
626625
$id
627626
));
628627
}
629-
/** @psalm-suppress PossiblyNullArgument **/
630-
$this->getContainer()->setAlias($definition->getClass(), $id);
628+
$this->getContainer()->setAlias($className, $id);
631629
}
632630
}
633631

‎tests/Integration/Symfony/OtelSdkBundle/DependencyInjection/ConfigurationTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public function configProvider(): array
8080
{
8181
$data = [];
8282

83-
foreach (static::CONFIG_VARIANTS as $variant) {
83+
foreach (self::CONFIG_VARIANTS as $variant) {
8484
$data[$variant] = $this->loadTestData($variant);
8585
}
8686

@@ -91,7 +91,7 @@ public function exceptionProvider(): array
9191
{
9292
$data = [];
9393

94-
foreach (static::EXCEPTIONS as $variant) {
94+
foreach (self::EXCEPTIONS as $variant) {
9595
$data[$variant] = $this->loadExceptionData($variant);
9696
}
9797

‎tests/Unit/Symfony/OtelSdkBundle/Trace/ExporterFactoryTest.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,22 +205,18 @@ class TestExporter implements SpanExporterInterface
205205

206206
private StreamFactoryInterface $streamFactory;
207207

208-
private $untyped;
209-
210208
public function __construct(
211209
string $name,
212210
string $endpointUrl,
213211
ClientInterface $client,
214212
RequestFactoryInterface $requestFactory,
215-
StreamFactoryInterface $streamFactory,
216-
$untyped = 'untyped'
213+
StreamFactoryInterface $streamFactory
217214
) {
218215
$this->name = $name;
219216
$this->endpointUrl = $endpointUrl;
220217
$this->client = $client;
221218
$this->requestFactory = $requestFactory;
222219
$this->streamFactory = $streamFactory;
223-
$this->untyped = $untyped;
224220
}
225221

226222
public static function fromConnectionString(string $endpointUrl, string $name, string $args)

0 commit comments

Comments
 (0)