From 239d2735ca0a6d278e604b957116969fcb43a817 Mon Sep 17 00:00:00 2001 From: Jonas Elfering Date: Fri, 3 Jan 2020 16:37:41 +0100 Subject: [PATCH] Apply suggestion to dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableNotDirectlyDependetRule.php --- dev-ops/analyze/composer.json | 5 ++ dev-ops/analyze/phpstan-config-generator.php | 9 +- .../Rules/AnnotationBasedRuleHelper.php | 17 ++++ .../DecoratableDoesNotAddPublicMethodRule.php | 50 +++++++++++ ...oratableDoesNotCallOwnPublicMethodRule.php | 46 ++++++++++ .../DecoratableImplementsInterfaceRule.php | 70 +++++++++++++++ .../DecoratableNotDirectlyDependetRule.php | 90 +++++++++++++++++++ dev-ops/common/actions/static-analyze.sh | 2 +- dev-ops/pre-commit | 4 +- 9 files changed, 288 insertions(+), 5 deletions(-) create mode 100644 dev-ops/analyze/src/PHPStan/Rules/AnnotationBasedRuleHelper.php create mode 100644 dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotAddPublicMethodRule.php create mode 100644 dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotCallOwnPublicMethodRule.php create mode 100644 dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableImplementsInterfaceRule.php create mode 100644 dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableNotDirectlyDependetRule.php diff --git a/dev-ops/analyze/composer.json b/dev-ops/analyze/composer.json index 3f3675fa..db451e6e 100644 --- a/dev-ops/analyze/composer.json +++ b/dev-ops/analyze/composer.json @@ -10,5 +10,10 @@ "scripts": { "post-install-cmd": ["@composer bin all install --ansi"], "post-update-cmd": ["@composer bin all update --ansi"] + }, + "autoload": { + "psr-4": { + "Shopware\\Development\\Analyze\\": "src/" + } } } diff --git a/dev-ops/analyze/phpstan-config-generator.php b/dev-ops/analyze/phpstan-config-generator.php index 83638708..c73a77b1 100644 --- a/dev-ops/analyze/phpstan-config-generator.php +++ b/dev-ops/analyze/phpstan-config-generator.php @@ -5,7 +5,8 @@ use Shopware\Development\Kernel; use Symfony\Component\Dotenv\Dotenv; -$classLoader = require __DIR__ . '/../../vendor/autoload.php'; +$autoLoadFile = __DIR__ . '/../../vendor/autoload.php'; +$classLoader = require $autoLoadFile; (new Dotenv(true))->load(__DIR__ . '/../../.env'); $shopwareVersion = Versions::getVersion('shopware/platform'); @@ -30,11 +31,15 @@ $phpStanConfig = str_replace( [ "\n # the placeholder \"%ShopwareHashedCacheDir%\" will be replaced on execution by dev-ops/analyze/phpstan-config-generator.php script", + "\n # the placeholder \"%ShopwareAutoloadFile%\" will be replaced on execution by dev-ops/analyze/phpstan-config-generator.php script", '%ShopwareHashedCacheDir%', + '%ShopwareAutoloadFile%' ], [ '', - $relativeCacheDir + '', + $relativeCacheDir, + $autoLoadFile ], $phpStanConfigDist ); diff --git a/dev-ops/analyze/src/PHPStan/Rules/AnnotationBasedRuleHelper.php b/dev-ops/analyze/src/PHPStan/Rules/AnnotationBasedRuleHelper.php new file mode 100644 index 00000000..c91b0f99 --- /dev/null +++ b/dev-ops/analyze/src/PHPStan/Rules/AnnotationBasedRuleHelper.php @@ -0,0 +1,17 @@ +getNativeReflection(); + + return $reflection->getDocComment() && strpos($reflection->getDocComment(), '@' . $annotation) !== false; + } +} diff --git a/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotAddPublicMethodRule.php b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotAddPublicMethodRule.php new file mode 100644 index 00000000..41dbde26 --- /dev/null +++ b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotAddPublicMethodRule.php @@ -0,0 +1,50 @@ +isInClass()) { + // skip + return []; + } + + $class = $scope->getClassReflection(); + if (!AnnotationBasedRuleHelper::isClassTaggedWithAnnotation($class, AnnotationBasedRuleHelper::DECORATABLE_ANNOTATION)) { + return []; + } + + if (!$node->isPublic() || $node->isMagic()) { + return []; + } + + $method = $class->getMethod($node->name->name, $scope); + + if ($method->getPrototype()->getDeclaringClass()->isInterface()) { + return []; + } + return [ + sprintf( + 'The service "%s" is marked as "@Decoratable", but adds public method "%s", that is not defined by any Interface.', + $class->getName(), + $method->getName() + ) + ]; + } +} diff --git a/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotCallOwnPublicMethodRule.php b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotCallOwnPublicMethodRule.php new file mode 100644 index 00000000..54e9fbf4 --- /dev/null +++ b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableDoesNotCallOwnPublicMethodRule.php @@ -0,0 +1,46 @@ +isInClass()) { + // skip + return []; + } + + $class = $scope->getClassReflection(); + if (!AnnotationBasedRuleHelper::isClassTaggedWithAnnotation($class, AnnotationBasedRuleHelper::DECORATABLE_ANNOTATION)) { + return []; + } + + $method = $scope->getType($node->var)->getMethod($node->name->name, $scope); + if (!$method->isPublic() || $method->getDeclaringClass()->getName() !== $class->getName()) { + return []; + } + + return [ + sprintf( + 'The service "%s" is marked as "@Decoratable", but calls it\'s own public method "%s", which breaks decoration.', + $class->getName(), + $method->getName() + ) + ]; + } +} diff --git a/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableImplementsInterfaceRule.php b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableImplementsInterfaceRule.php new file mode 100644 index 00000000..d8f1d692 --- /dev/null +++ b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableImplementsInterfaceRule.php @@ -0,0 +1,70 @@ +broker = $broker; + } + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + */ + public function processNode(Node $node, Scope $scope): array + { + if (!$node->namespacedName) { + // skip anonymous classes + return []; + } + + $class = $this->broker->getClass($scope->resolveName($node->namespacedName)); + + if (!AnnotationBasedRuleHelper::isClassTaggedWithAnnotation($class, AnnotationBasedRuleHelper::DECORATABLE_ANNOTATION)) { + return []; + } + + if ($this->implementsInterface($class)) { + return []; + } + + return [ + sprintf( + 'The service "%s" is marked as "@Decoratable", but does not implement an interface.', + $class->getName() + ) + ]; + } + + private function implementsInterface(ClassReflection $class): bool + { + if (!empty($class->getInterfaces())) { + return true; + } + + if ($class->getParentClass()) { + return $this->implementsInterface($class->getParentClass()); + } + + return false; + } +} diff --git a/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableNotDirectlyDependetRule.php b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableNotDirectlyDependetRule.php new file mode 100644 index 00000000..fb253851 --- /dev/null +++ b/dev-ops/analyze/src/PHPStan/Rules/Decoratable/DecoratableNotDirectlyDependetRule.php @@ -0,0 +1,90 @@ +broker = $broker; + } + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + */ + public function processNode(Node $node, Scope $scope): array + { + if (!$node->namespacedName) { + // skip anonymous classes + return []; + } + + $class = $this->broker->getClass($scope->resolveName($node->namespacedName)); + $errors = []; + + foreach ($node->getProperties() as $property) { + foreach ($property->props as $prop) { + $propReflections = $class->getProperty($prop->name->name, $scope); + $this->containsDecoratableTypeDependence($propReflections->getReadableType(), $errors, $class->getName(), $property->getStartLine()); + $this->containsDecoratableTypeDependence($propReflections->getWritableType(), $errors, $class->getName(), $property->getStartLine()); + } + } + + foreach ($node->getMethods() as $method) { + $methodReflection = $class->getMethod($method->name->name, $scope); + foreach ($methodReflection->getVariants() as $variant) { + $this->containsDecoratableTypeDependence($variant->getReturnType(), $errors, $class->getName(), $method->getStartLine()); + + /** @var ParameterReflection $param */ + foreach ($variant->getParameters() as $param) { + $this->containsDecoratableTypeDependence($param->getType(), $errors, $class->getName(), $method->getStartLine()); + } + } + } + + return $errors; + } + + /** + * @param string[]|RuleError[] $errors + */ + private function containsDecoratableTypeDependence(Type $type, array &$errors, string $originalClassname, int $startLine): void + { + foreach ($type->getReferencedClasses() as $className) { + $class = $this->broker->getClass($className); + if (!$class->isInterface() && AnnotationBasedRuleHelper::isClassTaggedWithAnnotation($class, AnnotationBasedRuleHelper::DECORATABLE_ANNOTATION)) { + $errors[] = RuleErrorBuilder::message( + sprintf( + 'The service "%s" has a direct dependency on decoratable service "%s", but must only depend on it\'s interface.', + $originalClassname, + $class->getName() + ) + )->line($startLine) + ->build(); + } + } + } +} diff --git a/dev-ops/common/actions/static-analyze.sh b/dev-ops/common/actions/static-analyze.sh index a5334346..93bbf3cc 100644 --- a/dev-ops/common/actions/static-analyze.sh +++ b/dev-ops/common/actions/static-analyze.sh @@ -2,5 +2,5 @@ #DESCRIPTION: Run static code analysis on core php dev-ops/analyze/phpstan-config-generator.php -php dev-ops/analyze/vendor/bin/phpstan analyze --configuration vendor/shopware/platform/phpstan.neon +php dev-ops/analyze/vendor/bin/phpstan analyze --autoload-file=dev-ops/analyze/vendor/autoload.php --configuration vendor/shopware/platform/phpstan.neon php dev-ops/analyze/vendor/bin/psalm --config=vendor/shopware/platform/psalm.xml --threads=4 --show-info=false diff --git a/dev-ops/pre-commit b/dev-ops/pre-commit index b4a75212..bef2d30f 100755 --- a/dev-ops/pre-commit +++ b/dev-ops/pre-commit @@ -2,7 +2,7 @@ PLATFORM_ROOT="$(git rev-parse --show-toplevel)" PROJECT_ROOT="${PROJECT_ROOT:-"$(cd "$PLATFORM_ROOT"/.. && git rev-parse --show-toplevel)"}" -AUTOLOAD_FILE="$PROJECT_ROOT/vendor/autoload.php" +AUTOLOAD_FILE="$PROJECT_ROOT/dev-ops/analyze/vendor/autoload.php" function onExit { if [[ $? != 0 ]] @@ -64,7 +64,7 @@ then php ${PROJECT_ROOT}/dev-ops/analyze/vendor/bin/phpstan analyze --no-progress --configuration ${PROJECT_ROOT}/platform/phpstan.neon --autoload-file="$AUTOLOAD_FILE" ${PHP_FILES} else docker-compose exec -u $(id -u) -T -w /app/platform app_server php /app/dev-ops/analyze/phpstan-config-generator.php - docker-compose exec -u $(id -u) -T -w /app/platform app_server php /app/dev-ops/analyze/vendor/bin/phpstan analyze --no-progress --configuration /app/platform/phpstan.neon --autoload-file="/app/vendor/autoload.php" ${PHP_FILES} + docker-compose exec -u $(id -u) -T -w /app/platform app_server php /app/dev-ops/analyze/vendor/bin/phpstan analyze --no-progress --configuration /app/platform/phpstan.neon --autoload-file="/app/dev-ops/analyze/vendor/autoload.php" ${PHP_FILES} fi php ${PROJECT_ROOT}/dev-ops/analyze/vendor/bin/psalm --config=${PROJECT_ROOT}/vendor/shopware/platform/psalm.xml --threads=2 --show-info=false --root=${PROJECT_ROOT} ${PHP_FILES}