Merge pull request #46 from crealoz/dev

Improve code quality - add sort for processors

Api/Processor/AuditProcessorInterface.php

@@ -35,4 +35,6 @@ public function getErroneousFiles(): array;
     public function prepopulateResults(): void;
 
     public function hasErrors(): bool;
+
+    public function getOrder(): int;
 }

Console/RunAuditCommand.php

@@ -54,7 +54,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         $ignoredModules = $input->getOption('ignored-modules');
         if (!empty($ignoredModules)) {
-            $this->auditStorage->setIgnoredModules(explode(',',$ignoredModules));
+            $this->auditStorage->setIgnoredModules(explode(',',(string) $ignoredModules));
         }
 
         $this->auditService->run($output, $language);

Controller/Adminhtml/Request/Download.php

@@ -67,6 +67,7 @@ public function execute()
             $this->messageManager->addErrorMessage(__('An error occurred while downloading the file.'));
             return $this->_redirect('*/*/index');
         }
+        return null;
     }
 
     /**

Exception/Processor/AuditProcessorException.php

@@ -10,7 +10,7 @@
  */
 class AuditProcessorException extends LocalizedException
 {
-    private string $erroneousFile;
+    private readonly string $erroneousFile;
 
     public function __construct(Phrase $phrase, string $erroneousFile, \Exception $cause = null, $code = 0)
     {

Exception/Processor/Plugins/MagentoFrameworkPluginExtension.php

@@ -10,7 +10,7 @@
  */
 class MagentoFrameworkPluginExtension extends AuditProcessorException
 {
-    private string $pluggedFile;
+    private readonly string $pluggedFile;
 
     public function __construct(Phrase $phrase, string $erroneousFile, string $pluggedFile, \Exception $cause = null, $code = 0)
     {

Model/ResourceModel/AuditRequest.php

@@ -54,7 +54,7 @@ protected function _afterSave(\Magento\Framework\Model\AbstractModel $object)
             return $this;
         }
         $files = $object->getFiles();
-        if (count($files) > 0) {
+        if ($files !== []) {
             $connection = $this->getConnection();
             $connection->delete(
                 $this->getTable('crealoz_easyaudit_request_file'),

Processor/Files/AbstractAuditProcessor.php

@@ -8,6 +8,7 @@
 
 abstract class AbstractAuditProcessor implements AuditProcessorInterface
 {
+    public const ORDER = 0;
 
     protected array $results = [];
 
@@ -70,6 +71,10 @@ protected function addErroneousFile(string $file, int $score): void
         } else {
             $this->erroneousFiles[$file] += $score;
         }
+    }
 
+    public function getOrder(): int
+    {
+        return static::ORDER;
     }
 }

Processor/Files/Code/BlockViewModelRatio.php

@@ -8,6 +8,7 @@
 
 class BlockViewModelRatio extends AbstractArrayProcessor implements ArrayProcessorInterface
 {
+    public const ORDER = 40;
 
     public function getProcessorName(): string
     {

Processor/Files/Code/HardWrittenSQL.php

@@ -11,6 +11,7 @@
 
 class HardWrittenSQL extends AbstractFileProcessor implements FileProcessorInterface
 {
+    public const ORDER = 30;
 
     public function getProcessorName(): string
     {
@@ -117,7 +118,7 @@ public function run(): void
              * Tries to find a FROM clause in the SQL query
              */
             preg_match('/SELECT.*FROM/', $code, $matches);
-            if (!empty($matches)) {
+            if ($matches !== []) {
                 $this->results['hasErrors'] = true;
                 $this->results['errors']['hardWrittenSQLSelect']['files'][] = $input;
                 $this->addErroneousFile($input, Audit::PRIORITY_AVERAGE);
@@ -128,7 +129,7 @@ public function run(): void
              * Tries to find an INTO clause in the SQL query
              */
             preg_match('/INSERT.*INTO/', $code, $matches);
-            if (!empty($matches)) {
+            if ($matches !== []) {
                 $this->results['hasErrors'] = true;
                 $this->results['warnings']['hardWrittenSQLInsert']['files'][] = $input;
                 $this->addErroneousFile($input, Audit::PRIORITY_AVERAGE);
@@ -139,7 +140,7 @@ public function run(): void
              * Tries to find a SET clause in the SQL query
              */
             preg_match('/UPDATE.*SET/', $code, $matches);
-            if (!empty($matches)) {
+            if ($matches !== []) {
                 $this->results['hasErrors'] = true;
                 $this->results['warnings']['hardWrittenSQLUpdate']['files'][] = $input;
                 $this->addErroneousFile($input, Audit::PRIORITY_AVERAGE);
@@ -150,7 +151,7 @@ public function run(): void
              * Tries to find a FROM clause in the SQL query
              */
             preg_match('/DELETE.*FROM/', $code, $matches);
-            if (!empty($matches)) {
+            if ($matches !== []) {
                 $this->results['hasErrors'] = true;
                 $this->results['errors']['hardWrittenSQLDelete']['files'][] = $input;
                 $this->addErroneousFile($input, Audit::PRIORITY_AVERAGE);
@@ -161,7 +162,7 @@ public function run(): void
              * Tries to find a JOIN sth ON clause in the SQL query
              */
             preg_match('/JOIN.*ON/', $code, $matches);
-            if (!empty($matches)) {
+            if ($matches !== []) {
                 $this->results['hasErrors'] = true;
                 $this->results['suggestions']['hardWrittenSQLJoin']['files'][] = $input;
             }

Processor/Files/Code/SpecificClassInjection.php

@@ -21,6 +21,8 @@
  */
 class SpecificClassInjection extends AbstractFileProcessor implements FileProcessorInterface
 {
+    public const ORDER = 20;
+
     private array $ignoredClass = [
         'Magento\Framework\Escaper',
         'Magento\Framework\Data\Collection\AbstractDb',
@@ -216,7 +218,7 @@ private function isArgumentIgnored($argumentName)
 
     private function isClassRepository($argumentName): bool
     {
-        return str_contains($argumentName, 'Repository');
+        return str_contains((string) $argumentName, 'Repository');
     }
 
     public function getProcessorTag(): string

Processor/Files/Code/UseOfRegistry.php

@@ -13,7 +13,7 @@
 
 class UseOfRegistry extends AbstractFileProcessor implements FileProcessorInterface
 {
-
+    public const ORDER = 10;
 
     public function __construct(
         AuditStorage $auditStorage,

Processor/Files/Di/Plugins.php

@@ -20,16 +20,7 @@
 
 class Plugins extends AbstractXmlProcessor implements FileProcessorInterface
 {
-
-    public function getProcessorName(): string
-    {
-        return __('Plugins');
-    }
-
-    public function getAuditSection(): string
-    {
-        return __('Dependency Injection (DI)');
-    }
+    public const ORDER = 10;
 
     public function __construct(
         AuditStorage $auditStorage,
@@ -43,6 +34,16 @@ public function __construct(
         parent::__construct($auditStorage);
     }
 
+    public function getProcessorName(): string
+    {
+        return __('Plugins');
+    }
+
+    public function getAuditSection(): string
+    {
+        return __('Dependency Injection (DI)');
+    }
+
     public function prepopulateResults(): void
     {
         parent::prepopulateResults();

Processor/Files/Logic/UnusedModules.php

@@ -10,6 +10,8 @@
 
 class UnusedModules extends AbstractArrayProcessor implements ArrayProcessorInterface
 {
+    public const ORDER = 10;
+
     public function getAuditSection(): string
     {
         return __('Logic');
@@ -59,7 +61,7 @@ private function getUnusedModulesEntry(): array
     public function run(): void
     {
         $unusedModules = $this->getModuleConfig->process($this->getArray());
-        if (!empty($unusedModules)) {
+        if ($unusedModules !== []) {
             foreach ($unusedModules as $module) {
                 $this->results['hasErrors'] = true;
                 $this->results['suggestions']['unusedModules']['files'][] = $module;

Processor/Files/View/Cacheable.php

@@ -7,6 +7,8 @@
 
 class Cacheable extends AbstractXmlProcessor implements FileProcessorInterface
 {
+    public const ORDER = 10;
+
     protected array $allowedAreas = ['sales', 'customer', 'gift', 'message'];
 
     public function getProcessorName(): string
@@ -51,7 +53,7 @@ public function run(): void
             foreach ($blocksNotCached as $block) {
                 $blockName = (string) $block->attributes()->name;
                 foreach ($this->allowedAreas as $area) {
-                    if (str_contains($blockName, $area)) {
+                    if (str_contains($blockName, (string) $area)) {
                         continue 2;
                     }
                 }

Processor/Results/ErroneousFiles.php

@@ -37,7 +37,7 @@ public function processResults(array $results): array
             $fileList[$scope][$filename] = $score;
         }
 
-        if (!empty($fileList)) {
+        if ($fileList !== []) {
             $summary[] = __('%1 files have a score equal to or higher than 10. These files must have really bad design pattern and/or not follow coding standards. Please check them with high priority.', $countHigherThan10);
             $summary[] = __('Beside that, %1 files have a score equal to or higher than 5. These files must be checked with medium priority.', $countHigherThan5);
 
@@ -60,13 +60,13 @@ public function processResults(array $results): array
      */
     private function getScope($file) : string
     {
-        if (str_contains($file, 'vendor')) {
+        if (str_contains((string) $file, 'vendor')) {
             return 'vendor';
         }
-        if (str_contains($file, 'code')) {
+        if (str_contains((string) $file, 'code')) {
             return 'code';
         }
-        if (str_contains($file, 'design')) {
+        if (str_contains((string) $file, 'design')) {
             return 'design';
         }
         return 'other';

Processor/Type/AbstractType.php

@@ -11,6 +11,7 @@
 
 abstract class AbstractType implements TypeInterface
 {
+    const ORDER = 0;
     protected array $results = [];
 
     protected array $fileGetters = [];
@@ -25,6 +26,11 @@ public function __construct(
     ) {
     }
 
+    public function getOrder(): int
+    {
+        return static::ORDER;
+    }
+
     public function hasErrors(): bool
     {
         return $this->hasErrors;
@@ -46,9 +52,9 @@ public function process(array $subTypes, string $type, OutputInterface $output =
         foreach ($subTypes as $subType => $processors) {
             $fileGetter = $this->getFileGetter($subType);
             $files = $fileGetter->execute();
-            if (!empty($files)) {
+            if ($files !== []) {
                 $progressBar = null;
-                if ($output) {
+                if ($output instanceof \Symfony\Component\Console\Output\OutputInterface) {
                     $output->writeln("\r\nProcessing $subType files...");
                     /** if we are in command line, we display a bar */
                     $progressBar = new ProgressBar($output, $this->getProgressBarCount($processors, $files));
@@ -59,7 +65,7 @@ public function process(array $subTypes, string $type, OutputInterface $output =
                     $this->hasErrors = true;
                 }
                 $this->manageResults($processors);
-                if ($output) {
+                if ($output instanceof \Symfony\Component\Console\Output\OutputInterface) {
                     $progressBar->finish();
                 }
             }
@@ -121,7 +127,7 @@ protected function manageResults(array $processors) : void
         foreach ($processors as $processor) {
             $results = $processor->getResults();
             $results['title'] = $processor->getProcessorName();
-            $this->results[$processor->getAuditSection()][$processor->getProcessorTag()] = $results;
+            $this->results[$processor->getAuditSection()][$processor->getOrder()] = $results;
             foreach ($processor->getErroneousFiles() as $file => $score) {
                 if (isset($this->erroneousFiles[$file])) {
                     $this->erroneousFiles[$file] += $score;

Processor/Type/Logic.php

@@ -8,6 +8,7 @@
 
 class Logic extends AbstractType implements TypeInterface
 {
+    const ORDER = 20;
 
     /**
      * @inheritDoc

Processor/Type/PHPCode.php

@@ -8,6 +8,9 @@
 
 class PHPCode extends AbstractType implements TypeInterface
 {
+
+    const ORDER = 10;
+
     /**
      * @inheritDoc
      */
@@ -16,7 +19,7 @@ protected function doProcess(array $processors, array $files, ProgressBar $progr
         $hasErrors = false;
         foreach ($files as $codeFile) {
             // ignores autoload.php and registration.php
-            if (str_contains($codeFile, 'autoload.php') || str_contains($codeFile, 'registration.php')) {
+            if (str_contains((string) $codeFile, 'autoload.php') || str_contains((string) $codeFile, 'registration.php')) {
                 continue;
             }
             foreach ($processors as $processor) {

Processor/Type/TypeInterface.php

@@ -14,4 +14,6 @@ public function getErroneousFiles(): array;
     public function initResults(array $subTypes): void;
 
     public function hasErrors(): bool;
+
+    public function getOrder(): int;
 }

Processor/Type/Xml.php

@@ -8,6 +8,7 @@
 
 class Xml extends AbstractType implements TypeInterface
 {
+    const ORDER = 30;
 
     /**
      * @inheritDoc

Service/Audit.php

@@ -76,7 +76,7 @@ public function run(OutputInterface $output = null, string $language = null, str
             $typeResults = $type->process($subTypes, $typeName, $output);
             if ($type->hasErrors()) {
                 $hasErrors = true;
-                $this->results = array_merge_recursive($typeResults, $this->results);
+                $this->results = $this->mergeResultsWithPreservedIndexes($this->results, $typeResults);
                 $erroneousFiles[$typeName] = $type->getErroneousFiles();
             }
         }
@@ -127,6 +127,18 @@ public function run(OutputInterface $output = null, string $language = null, str
         return '';
     }
 
+    private function mergeResultsWithPreservedIndexes(array $oldResults, array $newResults): array
+    {
+        foreach ($newResults as $key => $value) {
+            if (array_key_exists($key, $oldResults)) {
+                $oldResults[$key] = $this->mergeResultsWithPreservedIndexes($oldResults[$key], $value);
+            } else {
+                $oldResults[$key] = $value;
+            }
+        }
+        return $oldResults;
+    }
+
     /**
      * Initialize the results of the processors
      * @return void
@@ -150,7 +162,7 @@ private function consolidateResults($erroneousFiles): array
         foreach ($erroneousFiles as $files) {
             foreach ($files as $file => $score) {
                 // if file consists only in empty spaces and line feed, we skip it
-                if (trim($file) === '') {
+                if (trim((string) $file) === '') {
                     continue;
                 }