#13434: Strict type static test should be preformed only for added files.

Volodymyr Kublytskyi · Volodymyr Kublytskyi · commit e949bdb2adfd · 2018-03-27T12:14:35.000+03:00
diff --git a/app/code/Magento/ImportExport/Model/Import/SampleFileProvider.php b/app/code/Magento/ImportExport/Model/Import/SampleFileProvider.php
@@ -3,6 +3,8 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\ImportExport\Model\Import;
 
 use Magento\Framework\Exception\NoSuchEntityException;
diff --git a/dev/tests/static/get_github_changes.php b/dev/tests/static/get_github_changes.php
@@ -42,6 +42,17 @@
 $changedFiles = getChangedFiles($changes, $fileExtensions);
 generateChangedFilesList($options['output-file'], $changedFiles);
 saveChangedFileContent($repo);
+
+$additions = retrieveNewFilesAcrossForks($mainline, $repo, $options['branch']);
+$addedFiles = getChangedFiles($additions, $fileExtensions);
+$additionsFile = pathinfo($options['output-file']);
+$additionsFile = $additionsFile['dirname']
+    . DIRECTORY_SEPARATOR
+    . $additionsFile['filename']
+    . '.added.'
+    . $additionsFile['extension'];
+generateChangedFilesList($additionsFile, $addedFiles);
+
 cleanup($repo, $mainline);
 
 /**
@@ -143,7 +154,18 @@ function getRepo($options, $mainline)
  */
 function retrieveChangesAcrossForks($mainline, GitRepo $repo, $branchName)
 {
-    return $repo->compareChanges($mainline, $branchName);
+    return $repo->compareChanges($mainline, $branchName, GitRepo::CHANGE_TYPE_ALL);
+}
+
+/**
+ * @param string $mainline
+ * @param GitRepo $repo
+ * @param string $branchName
+ * @return array
+ */
+function retrieveNewFilesAcrossForks($mainline, GitRepo $repo, $branchName)
+{
+    return $repo->compareChanges($mainline, $branchName, GitRepo::CHANGE_TYPE_ADDED);
 }
 
 /**
@@ -178,6 +200,10 @@ function validateInput(array $options, array $requiredOptions)
 class GitRepo
 // @codingStandardsIgnoreEnd
 {
+    const CHANGE_TYPE_ADDED = 1;
+    const CHANGE_TYPE_MODIFIED = 2;
+    const CHANGE_TYPE_ALL = 3;
+
     /**
      * Absolute path to git project
      *
@@ -274,9 +300,10 @@ public function getBranches($source = '--all')
      *
      * @param string $remoteAlias
      * @param string $remoteBranch
+     * @param int $changesType
      * @return array
      */
-    public function compareChanges($remoteAlias, $remoteBranch)
+    public function compareChanges($remoteAlias, $remoteBranch, $changesType = self::CHANGE_TYPE_ALL)
     {
         if (!isset($this->remoteList[$remoteAlias])) {
             throw new LogicException('Alias "' . $remoteAlias . '" is not defined');
@@ -288,7 +315,8 @@ public function compareChanges($remoteAlias, $remoteBranch)
             ? $this->filterChangedFiles(
                 $result,
                 $remoteAlias,
-                $remoteBranch
+                $remoteBranch,
+                $changesType
             )
             : [];
     }
@@ -299,50 +327,113 @@ public function compareChanges($remoteAlias, $remoteBranch)
      * @param array $changes
      * @param string $remoteAlias
      * @param string $remoteBranch
+     * @param int $changesType
      * @return array
      */
-    protected function filterChangedFiles(array $changes, $remoteAlias, $remoteBranch)
-    {
+    protected function filterChangedFiles(
+        array $changes,
+        $remoteAlias,
+        $remoteBranch,
+        $changesType = self::CHANGE_TYPE_ALL
+    ) {
         $countScannedFiles = 0;
-        $changedFilesMasks = [
-            'M' => "M\t",
-            'A' => "A\t"
-        ];
+        $changedFilesMasks = $this->buildChangedFilesMask($changesType);
         $filteredChanges = [];
         foreach ($changes as $fileName) {
             $countScannedFiles++;
             if (($countScannedFiles % 5000) == 0) {
                 echo $countScannedFiles . " files scanned so far\n";
             }
-            foreach ($changedFilesMasks as $mask) {
-                if (strpos($fileName, $mask) === 0) {
-                    $fileName = str_replace($mask, '', $fileName);
-                    $fileName = trim($fileName);
-                    if (!in_array($fileName, $filteredChanges) && is_file($this->workTree . '/' . $fileName)) {
-                        $result = $this->call(
-                            sprintf(
-                                'diff HEAD %s/%s -- %s',
-                                $remoteAlias,
-                                $remoteBranch,
-                                $this->workTree . '/' . $fileName
-                            )
-                        );
-                        if ($result) {
-                            if (!(isset($this->changedContentFiles[$fileName]))) {
-                                $this->setChangedContentFile($result, $fileName);
-                            }
-                            $filteredChanges[] = $fileName;
-                        }
-                    }
-                    break;
-                }
+
+            $changeTypeMask = $this->detectChangeTypeMask($fileName, $changedFilesMasks);
+            if (null === $changeTypeMask) {
+                continue;
             }
+
+            $fileName = trim(substr($fileName, strlen($changeTypeMask)));
+            if (in_array($fileName, $filteredChanges)) {
+                continue;
+            }
+
+            $fileChanges = $this->getFileChangeDetails($fileName, $remoteAlias, $remoteBranch);
+            if (empty($fileChanges)) {
+                continue;
+            }
+
+            if (!(isset($this->changedContentFiles[$fileName]))) {
+                $this->setChangedContentFile($fileChanges, $fileName);
+            }
+            $filteredChanges[] = $fileName;
         }
         echo $countScannedFiles . " files scanned\n";
 
         return $filteredChanges;
     }
 
+    /**
+     * Build mask of git diff report
+     *
+     * @param int $changesType
+     * @return array
+     */
+    private function buildChangedFilesMask(int $changesType): array
+    {
+        $changedFilesMasks = [];
+        foreach ([
+            self::CHANGE_TYPE_ADDED => "A\t",
+            self::CHANGE_TYPE_MODIFIED => "M\t",
+        ] as $changeType => $changedFilesMask) {
+            if ($changeType & $changesType) {
+                $changedFilesMasks[] = $changedFilesMask;
+            }
+        }
+        return $changedFilesMasks;
+    }
+
+    /**
+     * Find one of the allowed modification mask returned by git diff.
+     * Example of change record: "A path/to/added_file"
+     *
+     * @param string $changeRecord
+     * @param array $allowedMasks
+     * @return string|null
+     */
+    private function detectChangeTypeMask(string $changeRecord, array $allowedMasks)
+    {
+        foreach ($allowedMasks as $mask) {
+            if (strpos($changeRecord, $mask) === 0) {
+                return $mask;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Read detailed information about changes in a file
+     *
+     * @param string $fileName
+     * @param string $remoteAlias
+     * @param string $remoteBranch
+     * @return array
+     */
+    private function getFileChangeDetails(string $fileName, string $remoteAlias, string $remoteBranch): array
+    {
+        if (!is_file($this->workTree . '/' . $fileName)) {
+            return [];
+        }
+
+        $result = $this->call(
+            sprintf(
+                'diff HEAD %s/%s -- %s',
+                $remoteAlias,
+                $remoteBranch,
+                $this->workTree . '/' . $fileName
+            )
+        );
+
+        return $result;
+    }
+
     /**
      * Set changed content for file.
      *
diff --git a/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php b/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php
@@ -107,30 +107,74 @@ public static function getWhitelist(
      */
     private static function getChangedFilesList($changedFilesBaseDir)
     {
-        $changedFiles = [];
+        return self::getFilesFromListFile(
+            $changedFilesBaseDir,
+            'changed_files*',
+            function () {
+                // if no list files, probably, this is the dev environment
+                @exec('git diff --name-only', $changedFiles);
+                @exec('git diff --cached --name-only', $addedFiles);
+                $changedFiles = array_unique(array_merge($changedFiles, $addedFiles));
+                return $changedFiles;
+            }
+        );
+    }
+
+    /**
+     * This method loads list of added files.
+     *
+     * @param string $changedFilesBaseDir
+     * @return string[]
+     */
+    private static function getAddedFilesList($changedFilesBaseDir)
+    {
+        return self::getFilesFromListFile(
+            $changedFilesBaseDir,
+            'changed_files*.added.*',
+            function () {
+                // if no list files, probably, this is the dev environment
+                @exec('git diff --cached --name-only', $addedFiles);
+                return $addedFiles;
+            }
+        );
+    }
 
-        $globFilesListPattern = ($changedFilesBaseDir ?: self::getChangedFilesBaseDir()) . '/_files/changed_files*';
+    /**
+     * Read files from generated lists.
+     *
+     * @param string $listsBaseDir
+     * @param string $listFilePattern
+     * @param callable $noListCallback
+     * @return string[]
+     */
+    private static function getFilesFromListFile($listsBaseDir, $listFilePattern, $noListCallback)
+    {
+        $filesDefinedInList = [];
+
+        $globFilesListPattern = ($listsBaseDir ?: self::getChangedFilesBaseDir())
+            . '/_files/' . $listFilePattern;
         $listFiles = glob($globFilesListPattern);
         if (count($listFiles)) {
             foreach ($listFiles as $listFile) {
-                $changedFiles = array_merge(
-                    $changedFiles,
+                $filesDefinedInList = array_merge(
+                    $filesDefinedInList,
                     file($listFile, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES)
                 );
             }
         } else {
-            // if no list files, probably, this is the dev environment
-            @exec('git diff --name-only', $changedFiles);
+            $filesDefinedInList = call_user_func($noListCallback);
         }
 
         array_walk(
-            $changedFiles,
+            $filesDefinedInList,
             function (&$file) {
                 $file = BP . '/' . $file;
             }
         );
 
-        return $changedFiles;
+        $filesDefinedInList = array_values(array_unique($filesDefinedInList));
+
+        return $filesDefinedInList;
     }
 
     /**
@@ -279,7 +323,7 @@ public function testCopyPaste()
      */
     public function testStrictTypes()
     {
-        $changedFiles = self::getChangedFilesList('');
+        $changedFiles = self::getAddedFilesList('');
 
         $componentRegistrar = new ComponentRegistrar();
         $directoriesToCheck = $componentRegistrar->getPaths(ComponentRegistrar::MODULE);
diff --git a/dev/tests/static/testsuite/Magento/Test/Php/_files/blacklist/strict_type.txt b/dev/tests/static/testsuite/Magento/Test/Php/_files/blacklist/strict_type.txt
@@ -1,9 +1 @@
 # Format: <componentType=module|library|theme|language|*> <componentName> <globPattern> or simply <globPattern>
-
-app/code/Magento/CatalogImportExport/Model/Import/Product.php
-app/code/Magento/ImportExport/Controller/Adminhtml/Import/Download.php
-app/code/Magento/ImportExport/Model/Import/SampleFileProvider.php
-app/code/Magento/CatalogInventory/Model/StockManagement.php
-app/code/Magento/CatalogInventory/Observer/ItemsForReindex.php
-app/code/Magento/CatalogInventory/Observer/SubtractQuoteInventoryObserver.php
-app/code/Magento/Store/Api/Data/WebsiteInterface.php
