Merge pull request #58 from magento-gl/Hammer-PlatformHealth-30sep24

Hammer platform health 30sep24

.github/workflows/php.yml

@@ -17,6 +17,7 @@ jobs:
           - "8.1"
           - "8.2"
           - "8.3"
+          - "8.4"
         dependencies:
           - "lowest"
           - "highest"

composer.json

@@ -8,7 +8,7 @@
     ],
     "bin": ["bin/svc"],
     "require": {
-        "php": "~8.1.0||~8.2.0||~8.3.0",
+        "php": "~8.1.0||~8.2.0||~8.3.0||~8.4.0",
         "ext-json": "*",
         "laminas/laminas-stdlib": "^3.18",
         "nikic/php-parser": "^4.15",

dev/tests/Unit/Console/Command/CompareSourceCommandApiClassesTest.php

@@ -59,10 +59,10 @@ public static function changesDataProvider()
                 $pathToFixtures . '/new-method/source-code-before',
                 $pathToFixtures . '/new-method/source-code-after',
                 [
-                    'Class (PATCH)',
+                    'Class (MINOR)',
                     'Test\Vcs\TestClass::testMethod | [public] Method has been added. | V015'
                 ],
-                'Patch change is detected.'
+                'Minor change is detected.'
             ],
             'api-class-removed-class' => [
                 $pathToFixtures . '/removed-class/source-code-before',

dev/tests/Unit/bootstrap.php

@@ -37,7 +37,6 @@ function ($errNo, $errStr, $errFile, $errLine) {
                     E_USER_ERROR => 'User Error',
                     E_USER_WARNING => 'User Warning',
                     E_USER_NOTICE => 'User Notice',
-                    E_STRICT => 'Strict',
                     E_RECOVERABLE_ERROR => 'Recoverable Error',
                     E_DEPRECATED => 'Deprecated',
                     E_USER_DEPRECATED => 'User Deprecated',

src/Analyzer/SystemXml/Analyzer.php

@@ -25,6 +25,8 @@
 use Magento\SemanticVersionChecker\Registry\XmlRegistry;
 use PHPSemVerChecker\Registry\Registry;
 use PHPSemVerChecker\Report\Report;
+use Magento\SemanticVersionChecker\Operation\SystemXml\DuplicateFieldAdded;
+use RecursiveDirectoryIterator;
 
 /**
  * Analyzes <kbd>system.xml</kbd> files:
@@ -92,14 +94,152 @@ public function analyze($registryBefore, $registryAfter)
                 $beforeFile = $registryBefore->mapping[XmlRegistry::NODES_KEY][$moduleName];
                 $this->reportRemovedNodes($beforeFile, $removedNodes);
             }
+
             if ($addedNodes) {
                 $afterFile = $registryAfter->mapping[XmlRegistry::NODES_KEY][$moduleName];
-                $this->reportAddedNodes($afterFile, $addedNodes);
+                if (strpos($afterFile, '_files') !== false) {
+                    $this->reportAddedNodes($afterFile, $addedNodes);
+                } else {
+                    $baseDir = $this->getBaseDir($afterFile);
+                    foreach ($addedNodes as $nodeId => $node) {
+                        $newNodeData = $this->getNodeData($node);
+                        $nodePath = $newNodeData['path'];
+
+                        // Extract section, group, and fieldId with error handling
+                        $extractedData = $this->extractSectionGroupField($nodePath);
+                        if ($extractedData === null) {
+                            // Skip the node if its path is invalid
+                            continue;
+                        }
+
+                        // Extract section, group, and fieldId
+                        list($sectionId, $groupId, $fieldId) = $extractedData;
+
+                        // Call function to check if this field is duplicated in other system.xml files
+                        $isDuplicated = $this->isDuplicatedFieldInXml(
+                            $baseDir,
+                            $sectionId,
+                            $groupId,
+                            $fieldId,
+                            $afterFile
+                        );
+
+                        foreach ($isDuplicated as $isDuplicatedItem) {
+                            if ($isDuplicatedItem['status'] === 'duplicate') {
+                                $this->reportDuplicateNodes($afterFile, [$nodeId => $node]);
+                            } else {
+                                $this->reportAddedNodes($afterFile, [$nodeId => $node]);
+                            }
+                        }
+                    }
+                }
             }
         }
         return $this->report;
     }
 
+    /**
+     * Get Magento Base directory from the path
+     *
+     * @param string $filePath
+     * @return string|null
+     */
+    private function getBaseDir(string $filePath): ?string
+    {
+        $currentDir = dirname($filePath);
+        while ($currentDir !== '/' && $currentDir !== false) {
+            // Check if current directory contains files unique to Magento root
+            if (file_exists($currentDir . '/SECURITY.md')) {
+                return $currentDir; // Found the Magento base directory
+            }
+            $currentDir = dirname($currentDir);
+        }
+        return null;
+    }
+
+  /**
+   * Search for system.xml files in both app/code and vendor directories, excluding the provided file.
+   *
+   * @param string $magentoBaseDir The base directory of Magento.
+   * @param string|null $excludeFile The file to exclude from the search.
+   * @return array An array of paths to system.xml files, excluding the specified file.
+   */
+    private function getSystemXmlFiles(string $magentoBaseDir, ?string $excludeFile = null): array
+    {
+        $systemXmlFiles = [];
+        $directoryToSearch = [
+            $magentoBaseDir . '/app/code'
+        ];
+
+        // Check if 'vendor' directory exists, and only add it if it does
+        if (is_dir($magentoBaseDir . '/vendor')) {
+            $directoriesToSearch[] = $magentoBaseDir . '/vendor';
+        }
+        foreach ($directoryToSearch as $directory) {
+            $iterator = new \RecursiveIteratorIterator(new RecursiveDirectoryIterator($directory));
+            foreach ($iterator as $file) {
+                if ($file->getfileName() === 'system.xml') {
+                    $filePath = $file->getRealPath();
+                    if ($filePath !== $excludeFile) {
+                        $systemXmlFiles[] = $file->getRealPath();
+                    }
+                }
+            }
+        }
+        return $systemXmlFiles;
+    }
+
+  /**
+   * Method to extract section, group and field from the Node
+   *
+   * @param string $nodePath
+   * @return array|null
+   */
+    private function extractSectionGroupField(string $nodePath): ?array
+    {
+        $parts = explode('/', $nodePath);
+
+        if (count($parts) < 3) {
+            // Invalid path if there are fewer than 3 parts
+            return null;
+        }
+
+        $sectionId = $parts[0];
+        $groupId = $parts[1];
+        $fieldId = $parts[2];
+
+        return [$sectionId, $groupId, $fieldId];
+    }
+
+  /**
+   * Method to get Node Data using reflection class
+   *
+   * @param object|string $node
+   * @return array
+   * @throws \ReflectionException
+   */
+    private function getNodeData(object|string $node): array
+    {
+        $data = [];
+
+        // Use reflection to get accessible properties
+        $reflection = new \ReflectionClass($node);
+        foreach ($reflection->getMethods() as $method) {
+            // Skip 'getId' and 'getParent' methods for comparison
+            if ($method->getName() === 'getId' || $method->getName() === 'getParent') {
+                continue;
+            }
+
+            // Dynamically call the getter methods
+            if (strpos($method->getName(), 'get') === 0) {
+                $propertyName = lcfirst(str_replace('get', '', $method->getName()));
+                $data[$propertyName] = $method->invoke($node);
+            }
+        }
+
+        return $data;
+    }
+
     /**
      * Extracts the node from <var>$registry</var> as an associative array.
      *
@@ -164,13 +304,32 @@ private function reportAddedNodes(string $file, array $nodes)
         }
     }
 
+    /**
+     * Creates reports for <var>$nodes</var> considering that they have been duplicated.
+     *
+     * @param string $file
+     * @param NodeInterface[] $nodes
+     * @return void
+     */
+    private function reportDuplicateNodes(string $file, array $nodes): void
+    {
+        foreach ($nodes as $node) {
+            switch (true) {
+                case $node instanceof Field:
+                    $this->report->add('system', new DuplicateFieldAdded($file, $node->getPath()));
+                    break;
+            }
+        }
+    }
+
     /**
      * Creates reports for <var>$modules</var> considering that <kbd>system.xml</kbd> has been removed from them.
      *
      * @param array $modules
      * @param XmlRegistry $registryBefore
+     * @return void
      */
-    private function reportRemovedFiles(array $modules, XmlRegistry $registryBefore)
+    private function reportRemovedFiles(array $modules, XmlRegistry $registryBefore): void
     {
         foreach ($modules as $module) {
             $beforeFile = $registryBefore->mapping[XmlRegistry::NODES_KEY][$module];
@@ -183,8 +342,9 @@ private function reportRemovedFiles(array $modules, XmlRegistry $registryBefore)
      *
      * @param string $file
      * @param NodeInterface[] $nodes
+     * @return void
      */
-    private function reportRemovedNodes(string $file, array $nodes)
+    private function reportRemovedNodes(string $file, array $nodes): void
     {
         foreach ($nodes as $node) {
             switch (true) {
@@ -202,4 +362,56 @@ private function reportRemovedNodes(string $file, array $nodes)
             }
         }
     }
+
+  /**
+   * @param string|null $baseDir
+   * @param string $sectionId
+   * @param string $groupId
+   * @param string|null $fieldId
+   * @param string $afterFile
+   * @return array
+   */
+    private function isDuplicatedFieldInXml(
+        ?string $baseDir,
+        string $sectionId,
+        string $groupId,
+        ?string $fieldId,
+        string $afterFile
+    ): array {
+        $hasDuplicate = false;
+
+        $result = [
+            'status' => 'minor',
+            'field'  => $fieldId
+        ];
+
+        if ($baseDir) {
+            $systemXmlFiles = $this->getSystemXmlFiles($baseDir, $afterFile);
+
+            foreach ($systemXmlFiles as $systemXmlFile) {
+                $xmlContent = file_get_contents($systemXmlFile);
+                try {
+                    $xml = new \SimpleXMLElement($xmlContent);
+                } catch (\Exception $e) {
+                    continue; // Skip this file if there's a parsing error
+                }
+                // Find <field> nodes with the given field ID
+                // XPath to search for <field> within a specific section and group
+                $fields = $xml->xpath("//section[@id='$sectionId']/group[@id='$groupId']/field[@id='$fieldId']");
+                if (!empty($fields)) {
+                    $hasDuplicate = true; // Set the duplicate flag to true if a match is found
+                    break; // Since we found a duplicate, we don't need to check further for this field
+                }
+            }
+            if ($hasDuplicate) {
+                return [
+                    [
+                        'status' => 'duplicate',
+                        'field'  => $fieldId
+                    ]
+                ];
+            }
+        }
+        return [$result];
+    }
 }

src/Operation/SystemXml/DuplicateFieldAdded.php

@@ -0,0 +1,34 @@
+<?php
+
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Magento\SemanticVersionChecker\Operation\SystemXml;
+
+use Magento\SemanticVersionChecker\Operation\AbstractOperation;
+use PHPSemVerChecker\SemanticVersioning\Level;
+
+/**
+ * When a <kbd>field</kbd> node is added.
+ */
+class DuplicateFieldAdded extends AbstractOperation
+{
+    /**
+     * @var string
+     */
+    protected $code = 'M302';
+
+    /**
+     * @var int
+     */
+    protected $level = Level::PATCH;
+
+    /**
+     * @var string
+     */
+    protected $reason = 'A field-node was duplicated';
+}

src/ReportBuilder.php

@@ -126,7 +126,7 @@ protected function makeVersionReport()
         // Customize severity level of some @api changes
         LevelMapping::setOverrides(
             [
-                'V015' => Level::PATCH, // Add public method
+                'V015' => Level::MINOR, // Add public method
                 'V016' => Level::PATCH, // Add protected method
                 'V019' => Level::MINOR, // Add public property
                 'V020' => Level::MINOR, // Add protected property