Skip to content

Commit

Permalink
Merge pull request #56 from magento-gl/Hammer-SVC-False-Positive-Build
Browse files Browse the repository at this point in the history
AC-6945::SVC false-positive build failures
  • Loading branch information
sidolov authored Sep 9, 2024
2 parents 3d2e6e5 + fc4ac2c commit 6430b3c
Show file tree
Hide file tree
Showing 9 changed files with 263 additions and 10 deletions.
54 changes: 54 additions & 0 deletions dev/tests/Unit/ClassHierarchy/EntityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,21 @@ public function testIsTrait(string $type, bool $expected)
);
}

/**
* @dataProvider dataProviderIsEnum
* @param string $type
* @param bool $expected
*/
public function testIsEnum(string $type, bool $expected)
{
$entity = new Entity('myName', $type);

$this->assertEquals(
$expected,
$entity->isEnum()
);
}

/*
* Data providers
*/
Expand All @@ -503,6 +518,10 @@ public static function dataProviderIsClass()
Entity::TYPE_TRAIT,
false,
],
'entity-is-enum-returns-false' => [
Entity::TYPE_ENUM,
false,
],
];
}

Expand All @@ -526,6 +545,10 @@ public static function dataProviderIsInterface()
Entity::TYPE_TRAIT,
false,
],
'entity-is-enum-returns-false' => [
Entity::TYPE_ENUM,
false,
],
];
}

Expand All @@ -549,6 +572,37 @@ public static function dataProviderIsTrait()
Entity::TYPE_TRAIT,
true,
],
'entity-is-enum-returns-false' => [
Entity::TYPE_ENUM,
false,
],
];
}

/**
* Provides test data for {@link EntityTest::testIsEnum()}
*
* @return array
*/
public static function dataProviderIsEnum()
{
return [
'entity-is-class-returns-false' => [
Entity::TYPE_CLASS,
false,
],
'entity-is-interface-returns-false' => [
Entity::TYPE_INTERFACE,
false,
],
'entity-is-trait-returns-false' => [
Entity::TYPE_TRAIT,
false,
],
'entity-is-enum-returns-true' => [
Entity::TYPE_ENUM,
true,
],
];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ public static function changesDataProvider()
$pathToFixtures . '/new-method/source-code-before',
$pathToFixtures . '/new-method/source-code-after',
[
'Class (MINOR)',
'Class (PATCH)',
'Test\Vcs\TestClass::testMethod | [public] Method has been added. | V015'
],
'Minor change is detected.'
'Patch change is detected.'
],
'api-class-removed-class' => [
$pathToFixtures . '/removed-class/source-code-before',
Expand Down
64 changes: 62 additions & 2 deletions src/Analyzer/DiXml/VirtualTypeAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Magento\SemanticVersionChecker\Node\VirtualType;
use Magento\SemanticVersionChecker\Operation\DiXml\VirtualTypeChanged;
use Magento\SemanticVersionChecker\Operation\DiXml\VirtualTypeRemoved;
use Magento\SemanticVersionChecker\Operation\DiXml\VirtualTypeToTypeChanged;
use Magento\SemanticVersionChecker\Registry\XmlRegistry;
use PHPSemVerChecker\Registry\Registry;
use PHPSemVerChecker\Report\Report;
Expand Down Expand Up @@ -57,10 +58,15 @@ public function analyze($registryBefore, $registryAfter)
foreach ($nodesBefore as $moduleName => $moduleNodes) {
/* @var VirtualType $nodeBefore */
$fileBefore = $registryBefore->mapping[XmlRegistry::NODES_KEY][$moduleName];

// Check if $moduleName exists in $registryAfter->mapping[XmlRegistry::NODES_KEY]
if (!isset($registryAfter->mapping[XmlRegistry::NODES_KEY][$moduleName])) {
continue;
}
foreach ($moduleNodes as $name => $nodeBefore) {
// search nodesAfter the by name
$nodeAfter = $nodesAfter[$moduleName][$name] ?? false;

$fileAfter = $registryAfter->mapping[XmlRegistry::NODES_KEY][$moduleName];
if ($nodeAfter !== false && $nodeBefore !== $nodeAfter) {
/* @var VirtualType $nodeAfter */
$this->triggerNodeChange($nodeBefore, $nodeAfter, $fileBefore);
Expand All @@ -78,14 +84,68 @@ public function analyze($registryBefore, $registryAfter)
}
}

$operation = new VirtualTypeRemoved($fileBefore, $name);
$finalPath = $this->convertClassNameToFilePath($fileAfter, $name, '.php');

if (file_exists($finalPath)) {
$operation = new VirtualTypeToTypeChanged($fileBefore, $name);
} else {
$operation = new VirtualTypeRemoved($fileBefore, $name);
}
$this->report->add('di', $operation);
}
}

return $this->report;
}

/**
* Method to convert class name to file path
*
* @param string $filePath
* @param string $className
* @param string $extraString
* @return string
*/
private function convertClassNameToFilePath($filePath, $className, $extraString = ''): string
{
// Split the initial file path to get the base directory.
$parts = explode('/', $filePath);
$classParts = explode('\\', $className);

// Find the common part between the file path and class name.
$baseDirParts = [];
foreach ($parts as $part) {
$baseDirParts[] = $part;

if (in_array($part, $classParts)) {
break;
}
}

// Reconstruct the base directory path.
$baseDir = implode('/', $baseDirParts);

// Replace namespace separators with directory separators in the class name.
$classFilePath = str_replace('\\', '/', $className);

$position = strpos($classFilePath, "/");

if ($position !== false) {
$classFilePath = substr($classFilePath, $position);
}

// Combine the base directory and class file path.
$fullPath = rtrim($baseDir, '/') . $classFilePath;


// Append the extra string if provided.
if ($extraString) {
$fullPath .= $extraString;
}
return $fullPath;
}


/**
* Return a filtered node list from type {@link VirtualType}
*
Expand Down
16 changes: 16 additions & 0 deletions src/ClassHierarchy/DependencyGraph.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,20 @@ public function findOrCreateTrait(string $fullyQualifiedName): Entity

return $trait;
}

/**
* @param string $fullyQualifiedName
* @return Entity
*/
public function findOrCreateEnum(string $fullyQualifiedName): Entity
{
$enum = $this->findEntityByName($fullyQualifiedName);

if (!$enum) {
$enum = $this->entityFactory->createEnum($fullyQualifiedName);
$this->addEntity($enum);
}

return $enum;
}
}
10 changes: 7 additions & 3 deletions src/ClassHierarchy/DependencyInspectionVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use Magento\SemanticVersionChecker\Helper\Node as NodeHelper;
use PhpParser\Node;
use PhpParser\Node\Stmt\Enum_ as EnumNode;
use PhpParser\Node\Stmt\Class_ as ClassNode;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
Expand All @@ -22,7 +23,7 @@
use PhpParser\NodeVisitorAbstract;

/**
* Implements a visitor for `class`, `interface` and `trait` nodes that generates a dependency graph.
* Implements a visitor for `class`, `interface`, `trait` and `enum` nodes that generates a dependency graph.
*/
class DependencyInspectionVisitor extends NodeVisitorAbstract
{
Expand Down Expand Up @@ -94,8 +95,8 @@ public function enterNode(Node $node)
}

/**
* Handles Class, Interface, and Traits nodes. Sets currentClassLike entity and will populate extends, implements,
* and API information
* Handles Class, Interface, Traits and Enum nodes. Sets currentClassLike entity and will populate extends,
* implements, and API information
*
* @param ClassLike $node
* @return int|null
Expand Down Expand Up @@ -135,6 +136,9 @@ private function handleClassLike(ClassLike $node)
case $node instanceof TraitNode:
$this->currentClassLike = $this->dependencyGraph->findOrCreateTrait((string)$namespacedName);
break;
case $node instanceof EnumNode:
$this->currentClassLike = $this->dependencyGraph->findOrCreateEnum((string)$namespacedName);
break;
}
$this->currentClassLike->setIsApi($this->nodeHelper->isApiNode($node));
return null;
Expand Down
13 changes: 12 additions & 1 deletion src/ClassHierarchy/Entity.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use PhpParser\Node\Stmt\PropertyProperty;

/**
* Implements an entity that reflects a `class`, `interface` or `trait` and its dependencies.
* Implements an entity that reflects a `class`, `interface`, `enum` or `trait` and its dependencies.
*/
class Entity
{
Expand All @@ -24,6 +24,7 @@ class Entity
public const TYPE_CLASS = 'class';
public const TYPE_INTERFACE = 'interface';
public const TYPE_TRAIT = 'trait';
public const TYPE_ENUM = 'enum';
/**#@-*/

/**
Expand Down Expand Up @@ -327,6 +328,16 @@ public function isTrait(): bool
return $this->type === self::TYPE_TRAIT;
}

/**
* Reflects whether current entity reflects an `enum`.
*
* @return bool
*/
public function isEnum(): bool
{
return $this->type === self::TYPE_ENUM;
}

/*
* Private methods
*/
Expand Down
9 changes: 9 additions & 0 deletions src/ClassHierarchy/EntityFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,13 @@ public function createTrait(string $name): Entity
{
return new Entity($name, Entity::TYPE_TRAIT);
}

/**
* @param string $name
* @return Entity
*/
public function createEnum(string $name): Entity
{
return new Entity($name, Entity::TYPE_ENUM);
}
}
99 changes: 99 additions & 0 deletions src/Operation/DiXml/VirtualTypeToTypeChanged.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\SemanticVersionChecker\Operation\DiXml;

use PHPSemVerChecker\Operation\Operation;
use PHPSemVerChecker\SemanticVersioning\Level;

/**
* When a virtual type was changed.
*/
class VirtualTypeToTypeChanged extends Operation
{
/**
* Error codes.
*
* @var array
*/
protected $code = 'M201';

/**
* Change level.
*
* @var int
*/
protected $level = Level::PATCH;

/**
* Operation message.
*
* @var string
*/
protected $reason = 'Virtual Type was changed to type';
/**
* File path before changes.
*
* @var string
*/
protected $fileBefore;

/**
* Property context before changes.
*
* @var \PhpParser\Node\Stmt
*/
protected $contextBefore;

/**
* Property before changes.
*
* @var \PhpParser\Node\Stmt\Property
*/
protected $propertyBefore;

/**
* @param string $fileBefore
* @param string $target
*/
public function __construct($fileBefore, $target)
{
$this->fileBefore = $fileBefore;
$this->target = $target;
}

/**
* Returns file path before changes.
*
* @return string
*/
public function getLocation(): string
{
return $this->fileBefore;
}

/**
* Returns line position of existed property.
*
* @return int
*/
public function getLine(): int
{
return 0;
}
/**
* Get level.
*
* @return mixed
*/
public function getLevel(): int
{
return $this->level;
}
}
Loading

0 comments on commit 6430b3c

Please sign in to comment.