Skip to content

Commit 4ca87ae

Browse files
committed
AC-3483:: SVC false-positive: modifying system.xml file from another module
1 parent 3d2e6e5 commit 4ca87ae

File tree

2 files changed

+105
-1
lines changed

2 files changed

+105
-1
lines changed

src/Analyzer/SystemXml/Analyzer.php

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Magento\SemanticVersionChecker\Registry\XmlRegistry;
2626
use PHPSemVerChecker\Registry\Registry;
2727
use PHPSemVerChecker\Report\Report;
28+
use Magento\SemanticVersionChecker\Operation\SystemXml\FieldDuplicated;
2829

2930
/**
3031
* Analyzes <kbd>system.xml</kbd> files:
@@ -92,14 +93,63 @@ public function analyze($registryBefore, $registryAfter)
9293
$beforeFile = $registryBefore->mapping[XmlRegistry::NODES_KEY][$moduleName];
9394
$this->reportRemovedNodes($beforeFile, $removedNodes);
9495
}
96+
print_r('Added Nodes');
97+
print_r($addedNodes);
9598
if ($addedNodes) {
9699
$afterFile = $registryAfter->mapping[XmlRegistry::NODES_KEY][$moduleName];
97-
$this->reportAddedNodes($afterFile, $addedNodes);
100+
$duplicateNode = $this->reportAddedNodesWithDuplicateCheck($afterFile, $addedNodes, $moduleNodesBefore);
101+
print_r('Duplicate node '.$duplicateNode.' found');
102+
print_r("After file ". $afterFile );
103+
if ($duplicateNode) {
104+
$this->reportDuplicateNodes($afterFile, $addedNodes);
105+
} else {
106+
$this->reportAddedNodes($afterFile, $addedNodes);
107+
}
98108
}
99109
}
100110
return $this->report;
101111
}
102112

113+
/**
114+
* Check and Report duplicate nodes
115+
*
116+
* @param $afterFile
117+
* @param $addedNodes
118+
* @param $moduleNodesBefore
119+
* @return bool|void
120+
*/
121+
private function reportAddedNodesWithDuplicateCheck($afterFile, $addedNodes, $moduleNodesBefore)
122+
{
123+
print_r('Report Added Nodes function called.');
124+
foreach ($addedNodes as $nodeId => $node) {
125+
// Check for duplicates by comparing node content except for 'id'
126+
$isDuplicate = false;
127+
foreach ($moduleNodesBefore as $existingNodeId => $existingNode) {
128+
if ($this->isDuplicateNode($node, $existingNode)) {
129+
$isDuplicate = true;
130+
break;
131+
}
132+
}
133+
return $isDuplicate;
134+
}
135+
}
136+
137+
/**
138+
* Check if node is duplicate or not
139+
*
140+
* @param $node
141+
* @param $existingNode
142+
* @return bool
143+
*/
144+
private function isDuplicateNode($node, $existingNode)
145+
{
146+
// Remove 'id' key for comparison
147+
unset($node['id'], $existingNode['id']);
148+
149+
// Compare the remaining parts of the nodes
150+
return $node == $existingNode;
151+
}
152+
103153
/**
104154
* Extracts the node from <var>$registry</var> as an associative array.
105155
*
@@ -164,6 +214,26 @@ private function reportAddedNodes(string $file, array $nodes)
164214
}
165215
}
166216

217+
/**
218+
* Creates reports for <var>$nodes</var> considering that they have been duplicated.
219+
*
220+
* @param string $file
221+
* @param NodeInterface[] $nodes
222+
*/
223+
private function reportDuplicateNodes(string $file, array $nodes)
224+
{
225+
print_r('Duplicate Nodes switch case');
226+
foreach ($nodes as $node) {
227+
echo "<br/> $node->getPath() <br/>";
228+
switch (true) {
229+
case $node instanceof Field:
230+
$this->report->add('system', new FieldDuplicated($file, $node->getPath()));
231+
break;
232+
default:
233+
}
234+
}
235+
}
236+
167237
/**
168238
* Creates reports for <var>$modules</var> considering that <kbd>system.xml</kbd> has been removed from them.
169239
*
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace Magento\SemanticVersionChecker\Operation\SystemXml;
11+
12+
use Magento\SemanticVersionChecker\Operation\AbstractOperation;
13+
use PHPSemVerChecker\SemanticVersioning\Level;
14+
15+
/**
16+
* When a <kbd>field</kbd> node is added.
17+
*/
18+
class FieldDuplicated extends AbstractOperation
19+
{
20+
/**
21+
* @var string
22+
*/
23+
protected $code = 'M302';
24+
25+
/**
26+
* @var int
27+
*/
28+
protected $level = Level::PATCH;
29+
30+
/**
31+
* @var string
32+
*/
33+
protected $reason = 'A field-node was duplicated';
34+
}

0 commit comments

Comments
 (0)