Skip to content

Commit

Permalink
Merge pull request magento#149 from konarshankar07/142-import-test-rule
Browse files Browse the repository at this point in the history
magento#142 : [New Rule] No imports from static tests namespaces
  • Loading branch information
lenaorobei authored Dec 11, 2019
2 parents da46c5d + 4040cd5 commit cc15b48
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 0 deletions.
13 changes: 13 additions & 0 deletions Magento2/Sniffs/Namespaces/ImportsFromTestNamespaceSniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Rule: Do not import from `Test` namespaces
## Background
Sometimes IDE imports the namespace with `Test` automatically for return data type like string, float etc or any other means.

## Reasoning
Time to time we're getting issue with running tests on PRs in magento/magento2 repository because someone imported `\Magento\Tests\NamingConvention\true\string` by mistake. As result - we have "No build reports available" for "Database Compare build", "Functional Tests build", "Sample Data Tests build" while Static tests are shown as "failing" but in results - we don't really have reason

## How it works
Any occurrence starts with `Magento\Tests` in import from the namespace will raise the warning.

## How to fix

Remove `Magento\Tests` from the imported namespaces
69 changes: 69 additions & 0 deletions Magento2/Sniffs/Namespaces/ImportsFromTestNamespaceSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php
/**
* Copyright © Magento. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento2\Sniffs\Namespaces;

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

/**
* Detects static test namespace.
*/
class ImportsFromTestNamespaceSniff implements Sniff
{
/**
* @var string
*/
private $prohibitNamespace = 'Magento\Tests';

/**
* @var string
*/
protected $warningMessage = 'Application modules should not use classed from test modules.';

/**
* @var string
*/
protected $warningCode = 'WrongImport';

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

/**
* @inheritdoc
*/
public function process(File $phpcsFile, $stackPtr)
{
$next = $phpcsFile->findNext([T_COMMA, T_SEMICOLON, T_OPEN_USE_GROUP, T_CLOSE_TAG], ($stackPtr + 1));
$tokens = $phpcsFile->getTokens();
$getTokenAsContent = $phpcsFile->getTokensAsString($stackPtr, ($next - $stackPtr));
if (strpos($getTokenAsContent, $this->prohibitNamespace) !== false) {
$phpcsFile->addWarning($this->warningMessage, $stackPtr, $this->warningCode);
}
if ($next !== false
&& $tokens[$next]['code'] !== T_SEMICOLON
&& $tokens[$next]['code'] !== T_CLOSE_TAG
) {
$baseUse = rtrim($phpcsFile->getTokensAsString($stackPtr, ($next - $stackPtr)));
$baseUse = str_replace('use \\', '', $baseUse);
$closingCurly = $phpcsFile->findNext(T_CLOSE_USE_GROUP, ($next + 1));
do {
$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), $closingCurly, true);
$groupedAsContent = $baseUse. $tokens[$next]['content'];
$next = $phpcsFile->findNext(T_COMMA, ($next + 1), $closingCurly);
if (strpos($groupedAsContent, $this->prohibitNamespace) !== false) {
$phpcsFile->addWarning($this->warningMessage, $stackPtr, $this->warningCode);
return;
}
} while ($next !== false);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php
use Magento\Tests;
use Magento\Foo\Tests as FakeTest;
use Magento\Real\Classes;
use \Magento\{Tests\String, Tests\Int};
use \Magento\{Foo\string, Bar\float};
33 changes: 33 additions & 0 deletions Magento2/Tests/Namespaces/ImportsFromTestNamespaceUnitTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php
/**
* Copyright © Magento. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento2\Tests\Namespaces;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

/**
* Class InterfaceNameUnitTest
*/
class ImportsFromTestNamespaceUnitTest extends AbstractSniffUnitTest
{
/**
* @inheritdoc
*/
public function getErrorList()
{
return [];
}

/**
* @inheritdoc
*/
public function getWarningList()
{
return [
2 => 1,
5 => 1
];
}
}
9 changes: 9 additions & 0 deletions Magento2/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@
<exclude-pattern>*Test.php</exclude-pattern>
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>
<rule ref="Magento2.Namespaces.ImportsFromTestNamespace">
<severity>8</severity>
<type>warning</type>
<exclude-pattern>*/_files/*</exclude-pattern>
<exclude-pattern>*/Fixtures/*</exclude-pattern>
<exclude-pattern>*/Test/*</exclude-pattern>
<exclude-pattern>*Test.php</exclude-pattern>
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>
<rule ref="Magento2.NamingConvention.InterfaceName">
<severity>8</severity>
<type>warning</type>
Expand Down

0 comments on commit cc15b48

Please sign in to comment.