Skip to content

Ecgm2 added extdn rules #63

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 8 commits into from
Jan 24, 2022
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
40 changes: 33 additions & 7 deletions Ecg/Sniffs/PHP/NamespaceSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,42 @@

class NamespaceSniff implements Sniff
{
const NAME_T_NS_NAMESPACE = 'T_NS_SEPARATOR';

private $tokens = [];

/**
* @return array|int[]|mixed[]
*/
public function register()
{
return [
T_CATCH
];
}

/**
* @param File $phpcsFile
* @param $stackPtr
* @return int|void
*/
public function process(File $phpcsFile, $stackPtr)
{
if ($phpcsFile->findNext(T_NAMESPACE, 0) === false) {
return;
}

$tokens = $phpcsFile->getTokens();
$this->tokens = $phpcsFile->getTokens();

$endOfTryStatement = $phpcsFile->findEndOfStatement($stackPtr);

$posOfCatchVariable = $phpcsFile->findNext(T_VARIABLE, $stackPtr, $endOfTryStatement);

$posOfExceptionClassName = $phpcsFile->findNext(T_STRING, $stackPtr, $posOfCatchVariable);

$posOfNsSeparator = $phpcsFile->findNext(T_NS_SEPARATOR, $stackPtr, $posOfExceptionClassName);

if ($posOfNsSeparator === false) {
$exceptionClassName = trim($tokens[$posOfExceptionClassName]['content']);
$posOfClassInUse = $phpcsFile->findNext(T_STRING, 0, $stackPtr, false, $exceptionClassName);
if ($posOfClassInUse === false || $tokens[$posOfClassInUse]['level'] != 0) {
$exceptionClassName = trim($this->tokens[$posOfExceptionClassName]['content']);
$posOfClassInUse = $this->findNextClassName($phpcsFile, 0, $stackPtr, $exceptionClassName);
if ($posOfClassInUse === false || $this->tokens[$posOfClassInUse]['level'] !== 0) {
$phpcsFile->addError(
'Namespace for "' . $exceptionClassName . '" class is not specified.',
$posOfExceptionClassName,
Expand All @@ -44,4 +53,21 @@ public function process(File $phpcsFile, $stackPtr)
}
}
}

/**
* @param File $phpcsFile
* @param int $startPtr
* @param int $endPtr
* @param string $exceptionClassName
* @return bool|int
*/
private function findNextClassName(File $phpcsFile, $startPtr, $endPtr, $exceptionClassName)
{
$position = $phpcsFile->findNext(T_STRING, $startPtr, $endPtr, false, $exceptionClassName);
if ($this->tokens[$position + 1]['type'] === self::NAME_T_NS_NAMESPACE) {
$position = $this->findNextClassName($phpcsFile, $position + 1, $endPtr, $exceptionClassName);
}

return $position;
}
}
11 changes: 9 additions & 2 deletions Ecg/Sniffs/Performance/LoopSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

class LoopSniff implements Sniff
{
private const GET_BY_METHOD_LSD_PATTERN = 'getBy';

protected $countFunctions = [
'sizeof',
'count'
Expand All @@ -14,7 +16,9 @@ class LoopSniff implements Sniff
protected $modelLsdMethods = [
'load',
'save',
'delete'
'delete',
'get',
'getList'
];

protected $dataLoadMethods = [
Expand Down Expand Up @@ -77,7 +81,10 @@ public function process(File $phpcsFile, $stackPtr)
if (in_array($content, $this->countFunctions)) {
$error = 'Array size calculation function %s detected in loop';
$code = 'ArraySize';
} elseif (in_array($content, $this->modelLsdMethods)) {
} elseif (
in_array($content, $this->modelLsdMethods)
|| 0 === strpos($content, self::GET_BY_METHOD_LSD_PATTERN)
) {
$error = 'Model LSD method %s detected in loop';
$code = 'ModelLSD';
} elseif (in_array($content, $this->dataLoadMethods)) {
Expand Down
26 changes: 26 additions & 0 deletions Ecg/Tests/PHP/NamespaceUnitTest1.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace N1;

use Magento\Framework\Exception\LocalizedException;

class NSException extends LocalizedException
{
public function checkFalsePositiveNamespaceIssue()
{
$checked = true;
try {
throw new NSException(__('Check false positive NS sniffer'));
} catch (Exception $e) {
$checked = false;
}

return $checked;
}
}

try {
throw new \N1\NSException('Check falsepositive');
} catch (\N1\NSException $e) {
$error = $e->getMessage();
}
13 changes: 7 additions & 6 deletions Ecg/Tests/Performance/LoopUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,26 @@ class TestLoop
$collection = Mage::getModel('catalog/product')->getCollection();
do {
$product = Mage::getModel('catalog/product')->load($id);
$c = count($data) = sizeof($data);
$c = count($data) == sizeof($data);
$product2 = $collection->getFirstItem();
$product2->save();
Mage::getModel('catalog/product')->setId($id)->delete();

$id--;
} while ($id);

for ($i = 1; $i <= 100; $i++) {
$array = [];
for ($i = 1; $i <= count($array); $i++) {
$product = Mage::getModel('catalog/product')->load($id);
$c = count($data) = sizeof($data);
$c = count($data) == sizeof($data);
$product2 = $collection->getFirstItem();
$product2->save();
Mage::getModel('catalog/product')->setId($id)->delete();
}

foreach($collection as $product) {
$product = Mage::getModel('catalog/product')->load($id);
$c = count($data) = sizeof($data);
$c = count($data) == sizeof($data);
$product2 = $collection->getFirstItem();
$product2->save();
Mage::getModel('catalog/product')->setId($id)->delete();
Expand All @@ -37,7 +38,7 @@ class TestLoop

while ($id) {
$product = Mage::getModel('catalog/product')->load($id);
$c = count($data) = sizeof($data);
$c = count($data) == sizeof($data);
$product2 = $collection->getFirstItem();
$product2->save();
Mage::getModel('catalog/product')->setId($id)->delete();
Expand All @@ -51,4 +52,4 @@ foreach ($collection as $item) {
foreach ($item->getData() as $product) {
$product = Mage::getModel('catalog/product')->load($id);
}
}
}
4 changes: 3 additions & 1 deletion Ecg/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

<rule ref="Squiz.Functions.GlobalFunction"/>
<rule ref="Squiz.Operators.IncrementDecrementUsage"/>
<rule ref="Squiz.PHP.DiscouragedFunctions"/>
<rule ref="Squiz.PHP.Eval"/>
<rule ref="Squiz.PHP.GlobalKeyword"/>
<rule ref="Squiz.PHP.NonExecutableCode"/>
Expand All @@ -39,6 +38,9 @@
<exclude-pattern>*.phtml</exclude-pattern>
</rule>

<rule ref="Ecg.Security.DiscouragedFunction"/>
<rule ref="Ecg.Security.ForbiddenFunction"/>

<rule ref="Ecg.Security.LanguageConstruct.DirectOutput">
<exclude-pattern>*.phtml</exclude-pattern>
</rule>
Expand Down
58 changes: 58 additions & 0 deletions EcgM2/Sniffs/Deprecated/InheritDocSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php
declare(strict_types=1);

namespace EcgM2\Sniffs\Deprecated;

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

class InheritDocSniff implements Sniff
{
private const ERROR_MESSAGE = 'This DocBlock has been deprecated and should not be used. For more info see ' .
'https://devdocs.magento.com/guides/v2.4/coding-standards/docblock-standard-general.html#inheritdoc';

private const INHERITDOC_COMMENT_TAG = '@inheritdoc';

private const T_DOC_COMMENT_TYPES = [
'T_DOC_COMMENT_TAG',
'T_DOC_COMMENT_STRING'
];

/**
* @return array|int[]|mixed[]
*/
public function register()
{
return [
T_DOC_COMMENT_TAG
];
}

/**
* @param File $phpcsFile
* @param $stackPtr
* @return int|void
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();
foreach ($tokens as $line => $token) {
if (!$this->hasTokenMatch($token)) {
continue;
}

$error = self::ERROR_MESSAGE . ' Found: %s';
$phpcsFile->addError($error, $line, 'Found', $token['content']);
}
}

/**
* @param array $token
* @return bool
*/
private function hasTokenMatch(array $token): bool
{
return false !== stripos($token['content'], self::INHERITDOC_COMMENT_TAG)
&& in_array($token['type'], self::T_DOC_COMMENT_TYPES, true);
}
}
62 changes: 62 additions & 0 deletions EcgM2/Sniffs/Deprecated/SpaceDocBlockSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php
declare(strict_types=1);

namespace EcgM2\Sniffs\Deprecated;

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

class SpaceDocBlockSniff implements Sniff
{
private const ERROR_MESSAGE = 'This DocBlock has been deprecated and should not be used. For more info see ' .
'https://devdocs.magento.com/guides/v2.4/coding-standards/docblock-standard-general.html#documentation-space';

private const INHERITDOC_COMMENT_TAG = [
'@author',
'@category',
'@package',
'@subpackage'
];

/**
* @return array|int[]|mixed[]
*/
public function register()
{
return [
T_DOC_COMMENT_TAG
];
}

/**
* @param File $phpcsFile
* @param $stackPtr
* @return int|void
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();
foreach ($tokens as $line => $token) {
if (!$this->hasTokenMatch($token)) {
continue;
}

$error = self::ERROR_MESSAGE . ' Found: %s';
$phpcsFile->addError($error, $line, 'Found', $token['content']);
}
}

private function hasTokenMatch($token): bool
{
foreach (self::INHERITDOC_COMMENT_TAG as $deprecatedDocBlock) {
if (
$token['type'] === 'T_DOC_COMMENT_TAG'
&& false !== stripos($token['content'], $deprecatedDocBlock)
) {
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Rule: Do not use the object manager in templates
# Rule: Do not use the object manager in templates or Classes
## Background
The Object Manager is responsible to create concrete objects for a required interface, and to generate code e.g. for adding plugins, factories, proxies.
It is used transparently by requesting a class/interface name as constructor dependency or in layout XML.
Expand All @@ -16,7 +16,7 @@ Bypassing this system leads to hidden dependencies and potentially to too many o
Besides, for separation of appearance and logic, a template should contain as little and uncomplex PHP code as possible.

## How it works
For all PHTML files, the sniff looks for string tokens "ObjectManager".
For all PHTML and PHP files, the sniff looks for string tokens "ObjectManager".

## How to fix
Given, your template contains code like this:
Expand Down
51 changes: 51 additions & 0 deletions EcgM2/Sniffs/Performance/ObjectManagerUsageSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

namespace EcgM2\Sniffs\Performance;

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

class ObjectManagerUsageSniff implements Sniff
{
/**
* @var string
*/
protected $message = 'Define dependencies in Class constructor or via DI instead of using ObjectManager manually.';

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

/**
* {@inheritdoc}
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();
foreach ($tokens as $line => $token) {
if (!$this->hasTokenMatch($token)) {
continue;
}

$error = $this->message . ' Found: %s';
$phpcsFile->addError($error, $line, 'Found', $token['content']);
}
}

/**
* Check if the content related to direct ObjectManager usage.
*
* @param $token
* @return bool
*/
public function hasTokenMatch($token)
{
return $token['content'] === 'ObjectManager';
}
}
Loading