Skip to content

AC-206: Create UCT phpcs ruleset for customizations only #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .github/workflows/php.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
- name: Install dependencies
run: npm install
- name: Run ESLint
run: npm run eslint -- eslint/rules Magento2 --ignore-pattern 'Magento2/Tests/Eslint/*Test.js'
run: npm run eslint -- eslint/rules
- name: Run JSCS
run: npm run jscs eslint/rules Magento2

Expand All @@ -59,4 +59,7 @@ jobs:
run: vendor/bin/phpunit

- name: Run code style suite
run: vendor/bin/phpcs --standard=Magento2 Magento2/Helpers/ Magento2/Sniffs
run: vendor/bin/phpcs --standard=Magento2 Magento2/Helpers Magento2/Sniffs Magento2Framework/Sniffs

- name: Run framework suite
run: vendor/bin/phpcs --standard=Magento2Framework Magento2/Helpers Magento2/Sniffs Magento2Framework/Sniffs
37 changes: 3 additions & 34 deletions Magento2/Sniffs/Legacy/RestrictedCodeSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class RestrictedCodeSniff implements Sniff
*/
public function __construct()
{
$this->loadData('restricted_classes*.php');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did create this method exactly to avoid the warning thrown by PHPCS about using include. But I guess it is OK to break our own rules sometimes :)

Copy link
Member

@sivaschenko sivaschenko Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svera the warning about include was skipped using phpcs:ignore (that is preserved, but the actual rule that needs to be skipped is specified after refactoring)

// phpcs:ignore Magento2.Security.IncludeFile.FoundIncludeFile
$this->classes = include __DIR__ . '/_files/restricted_classes.php';
}

/**
Expand All @@ -56,7 +57,7 @@ public function register()
*/
public function process(File $phpcsFile, $stackPtr)
{
// phpcs:ignore
// phpcs:ignore Magento2.Functions.DiscouragedFunction
if (array_key_exists(basename($phpcsFile->getFilename()), $this->fixtureFiles)) {
return;
}
Expand Down Expand Up @@ -99,36 +100,4 @@ private function isExcluded(string $token, File $phpcsFile): bool
}
return false;
}

/**
* Loads and merges data from fixtures
*
* @param string $filePattern
* @return void
*/
private function loadData(string $filePattern)
{
// phpcs:ignore
foreach (glob(__DIR__ . '/_files/' . $filePattern) as $file) {
$relativePath = str_replace(
'\\',
'/',
str_replace(__DIR__ . DIRECTORY_SEPARATOR, '', $file)
);
array_push($this->fixtureFiles, $relativePath);
$this->classes = array_merge_recursive($this->classes, $this->readList($file));
}
}

/**
* Isolate including a file into a method to reduce scope
*
* @param string $file
* @return array
*/
private function readList($file)
{
// phpcs:ignore
return include $file;
}
}
13 changes: 10 additions & 3 deletions Magento2/Sniffs/Legacy/_files/restricted_classes.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
'exclude' => [
'Magento/Framework/DB/Select.php',
'Magento/Framework/DB/Adapter/Pdo/Mysql.php',
'Magento/Framework/Model/ResourceModel/Iterator.php'
'Magento/Framework/Model/ResourceModel/Iterator.php',
'Magento/ResourceConnections/DB/Adapter/Pdo/MysqlProxy.php'
]
],
'Zend_Db_Adapter_Pdo_Mysql' => [
Expand Down Expand Up @@ -48,7 +49,10 @@
'Magento/Framework/Flag.php',
'Magento/Widget/Setup/LayoutUpdateConverter.php',
'Magento/Cms/Setup/ContentConverter.php',
'Magento/Framework/Unserialize/Test/Unit/UnserializeTest.php'
'Magento/Framework/Unserialize/Test/Unit/UnserializeTest.php',
'Magento/Framework/Test/Unit/FlagTest.php',
'Magento/Staging/Test/Unit/Model/Update/FlagTest.php',
'Magento/Logging/Test/Unit/Setup/ObjectConverterTest.php'
]
],
'ArrayObject' => [
Expand All @@ -67,7 +71,10 @@
'Magento/Framework/Indexer/Test/Unit/BatchTest.php',
'Magento/Framework/View/Element/UiComponent/ArrayObjectFactory.php',
'Magento/Framework/View/Element/UiComponent/Config/Provider/Component/Definition.php',
'Magento/Framework/Indexer/Action/Base.php'
'Magento/Framework/Indexer/Action/Base.php',
'Magento/MultipleWishlist/Test/Unit/Model/Search/Strategy/EmailTest.php',
'Magento/Rma/Test/Unit/Model/RmaRepositoryTest.php',
'Magento/Rma/Test/Unit/Model/Status/HistoryRepositoryTest.php'
]
],
'Magento\Framework\View\Element\UiComponent\ArrayObjectFactory' => [
Expand Down
32 changes: 0 additions & 32 deletions Magento2/Sniffs/Legacy/_files/restricted_classes_ee.php

This file was deleted.

33 changes: 0 additions & 33 deletions Magento2/Tests/Legacy/AbstractJsSniffUnitTestCase.php

This file was deleted.

21 changes: 0 additions & 21 deletions Magento2/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,6 @@
<severity>8</severity>
<type>warning</type>
</rule>
<rule ref="Magento2.Legacy.License">
<severity>8</severity>
<type>warning</type>
</rule>

<!-- Severity 7 warnings: General code issues. -->
<rule ref="Generic.Arrays.DisallowLongArraySyntax">
Expand Down Expand Up @@ -726,24 +722,7 @@
<exclude-pattern>*/Test/*</exclude-pattern>
<exclude-pattern>*Test.php</exclude-pattern>
</rule>
<rule ref="Magento2.Legacy.CopyrightAnotherExtensionsFiles">
<severity>5</severity>
<type>warning</type>
<exclude-pattern>*\.php$</exclude-pattern>
<exclude-pattern>*\.phtml$</exclude-pattern>
</rule>
<rule ref="Magento2.Legacy.Copyright">
<severity>5</severity>
<type>warning</type>
<include-pattern>*\.php$</include-pattern>
<include-pattern>*\.phtml$</include-pattern>
</rule>
<rule ref="Internal.NoCodeFound">
<severity>0</severity>
</rule>
<!-- TODO: AC-1314 -->
<rule ref="Magento2.Legacy">
<exclude name="Magento2.Legacy.Copyright"/>
<exclude name="Magento2.Legacy.CopyrightAnotherExtensionsFiles"/>
</rule>
</ruleset>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
declare(strict_types = 1);

namespace Magento2\Sniffs\Legacy;
namespace Magento2Framework\Sniffs\Header;

use Magento2\Sniffs\Less\TokenizerSymbolsInterface;
use PHP_CodeSniffer\Files\File;
Expand All @@ -24,7 +24,7 @@ class CopyrightAnotherExtensionsFilesSniff implements Sniff
* @var array
*/
public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS, 'PHP'];

/**
* @inheritDoc
*/
Expand All @@ -44,7 +44,7 @@ public function process(File $phpcsFile, $stackPtr)
if ($stackPtr > 0) {
return;
}

$fileText = $phpcsFile->getTokensAsString($stackPtr, count($phpcsFile->getTokens()));
$adobeCopyrightFound = preg_match(self::COPYRIGHT_ADOBE, $fileText);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,26 @@
*/
declare(strict_types = 1);

namespace Magento2\Sniffs\Legacy;
namespace Magento2Framework\Sniffs\Header;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;

class CopyrightSniff implements Sniff
{
private const WARNING_CODE = 'FoundCopyrightMissingOrWrongFormat';

private const COPYRIGHT_MAGENTO_TEXT = 'Copyright © Magento, Inc. All rights reserved.';
private const COPYRIGHT_ADOBE = '/Copyright \d+ Adobe/';

/**
* @inheritdoc
*/
public function register()
{
return [T_OPEN_TAG];
}

/**
* @inheritDoc
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
declare(strict_types = 1);

namespace Magento2\Sniffs\Legacy;
namespace Magento2Framework\Sniffs\Header;

use Magento2\Sniffs\Less\TokenizerSymbolsInterface;
use PHP_CodeSniffer\Files\File;
Expand All @@ -21,9 +21,9 @@ class LicenseSniff implements Sniff
public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS, 'PHP'];

private const WARNING_CODE = 'FoundLegacyTextInCopyright';

private const LEGACY_TEXTS = ['Irubin Consulting Inc', 'DBA Varien', 'Magento Inc'];

/**
* @inheritdoc
*/
Expand All @@ -34,15 +34,15 @@ public function register()
T_INLINE_HTML
];
}

/**
* @inheritDoc
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();
$content = null;

if ($tokens[$stackPtr]['code'] === T_DOC_COMMENT_STRING) {
$content = $tokens[$stackPtr]['content'];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,31 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento2\Tests\Legacy;
namespace Magento2Framework\Tests\Header;

class CopyrightAnotherExtensionsFilesUnitTest extends AbstractJsSniffUnitTestCase
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

class CopyrightAnotherExtensionsFilesUnitTest extends AbstractSniffUnitTest
{
/**
* @inheritDoc
*/
protected function setUp(): void
{
parent::setUp();

$config = new Config();
$config->extensions = array_merge(
$config->extensions,
[
'js' => 'PHP'
]
);

$GLOBALS['PHP_CODESNIFFER_CONFIG'] = $config;
}

/**
* @inheritdoc
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento2\Tests\Legacy;
namespace Magento2Framework\Tests\Header;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

Expand All @@ -25,7 +25,7 @@ public function getWarningList($testFile = ''): array
if ($testFile === 'CopyrightUnitTest.4.inc' || $testFile === 'CopyrightUnitTest.5.inc') {
return [];
}

if ($testFile === 'CopyrightUnitTest.1.inc') {
return [
1 => 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento2\Tests\Legacy;
namespace Magento2Framework\Tests\Header;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

Expand All @@ -25,7 +25,7 @@ public function getWarningList($testFile = ''): array
if ($testFile === 'LicenseUnitTest.1.inc' || $testFile === 'LicenseUnitTest.3.xml') {
return [];
}

if ($testFile === 'LicenseUnitTest.2.inc') {
return [
3 => 1,
Expand Down
20 changes: 20 additions & 0 deletions Magento2Framework/ruleset.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0"?>
<ruleset name="Magento2Framework">
<description>Magento Coding Standard sniffs applicable for the framework testing only</description>
<rule ref="Magento2Framework.Header.License">
<severity>5</severity>
<type>warning</type>
</rule>
<rule ref="Magento2Framework.Header.CopyrightAnotherExtensionsFiles">
<severity>5</severity>
<type>warning</type>
<exclude-pattern>*\.php$</exclude-pattern>
<exclude-pattern>*\.phtml$</exclude-pattern>
</rule>
<rule ref="Magento2Framework.Header.Copyright">
<severity>5</severity>
<type>warning</type>
<include-pattern>*\.php$</include-pattern>
<include-pattern>*\.phtml$</include-pattern>
</rule>
</ruleset>
Loading