Skip to content

Commit 6990bcf

Browse files
committed
MQE-590: Fix PHP Mess Detector Errors
- DataObjectHandler refactoring for cyclomatic complexity. - Additional Suppression Flags - Ruleset change to allow unused foreach variables (as key=>value)
1 parent c8e48d8 commit 6990bcf

File tree

8 files changed

+84
-34
lines changed

8 files changed

+84
-34
lines changed

dev/tests/static/Magento/CodeMessDetector/ruleset.xml

+4-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@
2626
</rule>
2727

2828
<!-- Unused code rules -->
29-
<rule ref="rulesets/unusedcode.xml" >
29+
<rule ref="rulesets/unusedcode.xml/UnusedPrivateField" />
30+
<rule ref="rulesets/unusedcode.xml/UnusedPrivateMethod" />
31+
<rule ref="rulesets/unusedcode.xml/UnusedFormalParameter" />
32+
<rule ref="rulesets/unusedcode.xml/UnusedLocalVariable" >
3033
<properties>
3134
<property name="allow-unused-foreach-variables" value="true"/>
3235
</properties>

src/Magento/FunctionalTestingFramework/Config/FileResolver/Module.php

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class Module implements FileResolverInterface
2525
/**
2626
* Module constructor.
2727
* @param ModuleResolver|null $moduleResolver
28+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
2829
*/
2930
public function __construct(ModuleResolver $moduleResolver = null)
3031
{

src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/DataObjectHandler.php

+70-23
Original file line numberDiff line numberDiff line change
@@ -173,26 +173,12 @@ private function parseDataEntities()
173173
$uniquenessValues = [];
174174

175175
if (array_key_exists(self::DATA_VALUES, $entity)) {
176-
foreach ($entity[self::DATA_VALUES] as $dataElement) {
177-
$dataElementKey = strtolower($dataElement[self::DATA_ELEMENT_KEY]);
178-
$dataElementValue = $dataElement[self::DATA_ELEMENT_VALUE] ?? "";
179-
if (array_key_exists(self::DATA_ELEMENT_UNIQUENESS_ATTR, $dataElement)) {
180-
$uniquenessValues[$dataElementKey] = $dataElement[self::DATA_ELEMENT_UNIQUENESS_ATTR];
181-
}
182-
183-
$dataValues[$dataElementKey] = $dataElementValue;
184-
}
185-
unset($dataElement);
176+
$dataValues = $this->parseDataElements($entity);
177+
$uniquenessValues = $this->parseUniquenessValues($entity);
186178
}
187179

188180
if (array_key_exists(self::REQUIRED_ENTITY, $entity)) {
189-
foreach ($entity[self::REQUIRED_ENTITY] as $linkedEntity) {
190-
$linkedEntityName = $linkedEntity[self::REQUIRED_ENTITY_VALUE];
191-
$linkedEntityType = $linkedEntity[self::REQUIRED_ENTITY_TYPE];
192-
193-
$linkedEntities[$linkedEntityName] = $linkedEntityType;
194-
}
195-
unset($linkedEntity);
181+
$linkedEntities = $this->parseRequiredEntityElements($entity);
196182
}
197183

198184
if (array_key_exists(self::ARRAY_VALUES, $entity)) {
@@ -207,11 +193,7 @@ private function parseDataEntities()
207193
}
208194

209195
if (array_key_exists(self::VAR_VALUES, $entity)) {
210-
foreach ($entity[self::VAR_VALUES] as $varElement) {
211-
$varKey = $varElement[self::VAR_KEY];
212-
$varValue = $varElement[self::VAR_ENTITY] . self::VAR_ENTITY_FIELD_SEPARATOR . $varElement[self::VAR_FIELD];
213-
$vars[$varKey] = $varValue;
214-
}
196+
$vars = $this->parseVarElements($entity);
215197
}
216198

217199
$entityDataObject = new EntityDataObject(
@@ -224,9 +206,74 @@ private function parseDataEntities()
224206
);
225207

226208
$this->data[$entityDataObject->getName()] = $entityDataObject;
227-
228209
}
229210
unset($entityName);
230211
unset($entity);
231212
}
213+
214+
/**
215+
* Parses <data> elements in an entity, and returns them as an array of "lowerKey"=>value.
216+
* @param array $entityData
217+
* @return array
218+
*/
219+
private function parseDataElements($entityData)
220+
{
221+
$dataValues = [];
222+
foreach ($entityData[self::DATA_VALUES] as $dataElement) {
223+
$dataElementKey = strtolower($dataElement[self::DATA_ELEMENT_KEY]);
224+
$dataElementValue = $dataElement[self::DATA_ELEMENT_VALUE] ?? "";
225+
$dataValues[$dataElementKey] = $dataElementValue;
226+
}
227+
return $dataValues;
228+
}
229+
230+
/**
231+
* Parses through <data> elements in an entity to return an array of "DataKey" => "UniquenessAttribute"
232+
* @param array $entityData
233+
* @return array
234+
*/
235+
private function parseUniquenessValues($entityData)
236+
{
237+
$uniquenessValues = [];
238+
foreach ($entityData[self::DATA_VALUES] as $dataElement) {
239+
if (array_key_exists(self::DATA_ELEMENT_UNIQUENESS_ATTR, $dataElement)) {
240+
$dataElementKey = strtolower($dataElement[self::DATA_ELEMENT_KEY]);
241+
$uniquenessValues[$dataElementKey] = $dataElement[self::DATA_ELEMENT_UNIQUENESS_ATTR];
242+
}
243+
}
244+
return $uniquenessValues;
245+
}
246+
247+
/**
248+
* Parses <required-entity> elements given entity, and returns them as an array of "EntityValue"=>"EntityType"
249+
* @param array $entityData
250+
* @return array
251+
*/
252+
private function parseRequiredEntityElements($entityData)
253+
{
254+
$linkedEntities = [];
255+
foreach ($entityData[self::REQUIRED_ENTITY] as $linkedEntity) {
256+
$linkedEntityName = $linkedEntity[self::REQUIRED_ENTITY_VALUE];
257+
$linkedEntityType = $linkedEntity[self::REQUIRED_ENTITY_TYPE];
258+
259+
$linkedEntities[$linkedEntityName] = $linkedEntityType;
260+
}
261+
return $linkedEntities;
262+
}
263+
264+
/**
265+
* Parses <var> elements in given entity, and returns them as an array of "Key"=> entityType -> entityKey
266+
* @param array $entityData
267+
* @return array
268+
*/
269+
private function parseVarElements($entityData)
270+
{
271+
$vars = [];
272+
foreach ($entityData[self::VAR_VALUES] as $varElement) {
273+
$varKey = $varElement[self::VAR_KEY];
274+
$varValue = $varElement[self::VAR_ENTITY] . self::VAR_ENTITY_FIELD_SEPARATOR . $varElement[self::VAR_FIELD];
275+
$vars[$varKey] = $varValue;
276+
}
277+
return $vars;
278+
}
232279
}

src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/OperationDefinitionObjectHandler.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,11 @@ public function getOperationDefinition($operation, $dataType)
125125
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
126126
* @SuppressWarnings(PHPMD.NPathComplexity)
127127
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
128+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
128129
*/
129130
private function initDataDefinitions()
130131
{
131-
//TODO: Reduce CyclomaticComplexity/NPathComplexity of method, remove warning suppression.
132+
//TODO: Reduce CyclomaticComplexity/NPathComplexity/Length of method, remove warning suppression.
132133
$objectManager = ObjectManagerFactory::getObjectManager();
133134
$metadataParser = $objectManager->create(OperationDefinitionParser::class);
134135
foreach ($metadataParser->readOperationMetadata()

src/Magento/FunctionalTestingFramework/DataGenerator/Objects/OperationDefinitionObject.php

+1
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ class OperationDefinitionObject
118118
* @param string $contentType
119119
* @param string $successRegex
120120
* @param string $returnRegex
121+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
121122
*/
122123
public function __construct(
123124
$name,

src/Magento/FunctionalTestingFramework/DataGenerator/Persist/DataPersistenceHandler.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public function deleteEntity($storeCode = null)
142142
$this->storeCode = $storeCode;
143143
}
144144
$curlHandler = new CurlHandler('delete', $this->createdObject, $this->storeCode);
145-
$result = $curlHandler->executeRequest($this->dependentObjects);
145+
$curlHandler->executeRequest($this->dependentObjects);
146146
}
147147

148148
/**

src/Magento/FunctionalTestingFramework/Test/Util/TestNameValidationUtil.php

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public static function validateName($testName)
5252
*
5353
* @param string $val
5454
* @return string
55+
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
5556
*/
5657
private static function nameMapper($val)
5758
{

src/Magento/FunctionalTestingFramework/Util/TestGenerator.php

+4-8
Original file line numberDiff line numberDiff line change
@@ -1234,14 +1234,10 @@ private function generateHooksPhp($hookObjects)
12341234
throw new TestReferenceException($e->getMessage() . " in Element \"" . $type . "\"");
12351235
}
12361236

1237-
if ($type == "after" or $type == "before") {
1238-
$hooks .= sprintf("\tpublic function _{$type}(%s)\n", $dependencies);
1239-
$hooks .= "\t{\n";
1240-
$hooks .= $steps;
1241-
$hooks .= "\t}\n\n";
1242-
}
1243-
1244-
$hooks .= "";
1237+
$hooks .= sprintf("\tpublic function _{$type}(%s)\n", $dependencies);
1238+
$hooks .= "\t{\n";
1239+
$hooks .= $steps;
1240+
$hooks .= "\t}\n\n";
12451241
}
12461242

12471243
return $hooks;

0 commit comments

Comments
 (0)