Skip to content

fix some errors reported by PhpStorm + Php Inspections (EA Ultimate)#2113

Closed
voku wants to merge 7 commits intorectorphp:masterfrom
voku:phpstorm_inspection_fixes
Closed

fix some errors reported by PhpStorm + Php Inspections (EA Ultimate)#2113
voku wants to merge 7 commits intorectorphp:masterfrom
voku:phpstorm_inspection_fixes

Conversation

@voku
Copy link
Copy Markdown
Contributor

@voku voku commented Oct 5, 2019

This change is Reviewable

@TomasVotruba
Copy link
Copy Markdown
Member

Nice, those checks should be in Rector rules 👍

Comment thread packages/CodingStyle/src/Rector/Encapsed/EncapsedStringsToSprintfRector.php Outdated
Comment thread packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer.php

// is suffix in the same category, e.g. "Exception/SomeException.php"
$expectedLocationFilePattern = sprintf('#\/%s\/.+%s#', preg_quote($groupName), preg_quote($suffixPattern));
$expectedLocationFilePattern = sprintf('#\/%s\/.+%s#', preg_quote($groupName, '#'), preg_quote($suffixPattern, '#'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasVotruba Maybe the usage of "preg_quote" need to be checked in "ConsistentPregDelimiterRector"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Care for PR to add it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that "ConsistentPregDelimiterRector" will only work for simple strings without variables etc. So "preg_quote" is not effected, because it will not be replaced anyway, correct?


PS: I tried to debug some stuff but I don't know why I can't simply "var_dump" something in e.g. "ConsistentPregDelimiterRector"? My laptop will crash while running the phpunit tests, it also crash if I add a "exit(1)" after the "var_dump"? Is there a debug option or something like that?

e.g.:

        if (! $arg->value instanceof String_) {
var_dump($arg); exit();
            return;
        }

@voku voku requested a review from TomasVotruba October 5, 2019 22:36
}

if ($interfaceMethods !== []) {
$interfaceMethods = array_merge([], ...$interfaceMethods);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather confusing, same variable is used for different array nesting


if ($allTypes !== []) {
$allTypes = array_merge([], ...$allTypes);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


if ($autoloadDirectories !== []) {
$autoloadDirectories = array_merge([], ...$autoloadDirectories);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

$dataProviderClassMethods = $this->createDataProviderClassMethodsFromRecipes();

$node->stmts = array_Merge($node->stmts, $dataProviderClassMethods);
$node->stmts = array_merge($node->stmts, $dataProviderClassMethods);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


if ($collectedTypes !== []) {
$collectedTypes = array_merge([], ...$collectedTypes);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

return array_unique($previousMethodCallNames);
if ($previousMethodCallNames !== []) {
$previousMethodCallNames = array_unique(array_merge([], ...$previousMethodCallNames));
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


if ($absoluteDirectories !== []) {
$absoluteDirectories = array_merge([], ...$absoluteDirectories);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


if ($declareNode) {
$nodes = array_merge([$declareNode], [$node]);
$nodes = [$declareNode, $node];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one, let's make a Rector from this:
#2170

$expectedLocationFilePattern = sprintf(
'#\/%s\/.+%s#',
preg_quote($groupName, '#'),
preg_quote($suffixPattern, '#')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@TomasVotruba
Copy link
Copy Markdown
Member

Integrated as Rector rules in:

Thanks for your PR, it helped to identify patterns and automate them, so you won't have to do it manually ever again 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants