Skip to content

#58 : Implement sniff for class properties PHPDoc formatting #140

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
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
4 changes: 0 additions & 4 deletions Magento2/Helpers/Tokenizer/AbstractTokenizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@
abstract class AbstractTokenizer
{
/**
* Current index in string
*
* @var int
*/
protected $_currentIndex;

/**
* String for tokenize
*
* @var string
*/
protected $_string;
Expand Down
2 changes: 0 additions & 2 deletions Magento2/Helpers/Tokenizer/Variable.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
class Variable extends AbstractTokenizer
{
/**
* Internal counter used to keep track of how deep in array parsing we are
*
* @var int
*/
protected $arrayDepth = 0;
Expand Down
6 changes: 0 additions & 6 deletions Magento2/Sniffs/Classes/DiscouragedDependenciesSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,16 @@ class DiscouragedDependenciesSniff implements Sniff
protected $warningCode = 'ConstructorProxyInterceptor';

/**
* Aliases of proxies or plugins from use statements
*
* @var string[]
*/
private $aliases = [];

/**
* The current file - used for clearing USE aliases when file changes
*
* @var null|string
*/
private $currentFile = null;

/**
* Terms to search for in variables and namespaces
*
* @var string[]
*/
public $incorrectClassNames = ['proxy','interceptor'];
Expand Down
158 changes: 158 additions & 0 deletions Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
<?php
namespace Magento2\Sniffs\Commenting;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\AbstractVariableSniff;
use PHP_CodeSniffer\Util\Tokens;
use Magento2\Helpers\Commenting\PHPDocFormattingValidator;

/**
* Class ClassPropertyPHPDocFormattingSniff
*/
class ClassPropertyPHPDocFormattingSniff extends AbstractVariableSniff
{

/**
* @var array
*/
private $ignoreTokens = [
T_PUBLIC,
T_PRIVATE,
T_PROTECTED,
T_VAR,
T_STATIC,
T_WHITESPACE,
];

/**
* @var PHPDocFormattingValidator
*/
private $PHPDocFormattingValidator;

/**
* Constructs an ClassPropertyPHPDocFormattingSniff.
*/
public function __construct()
{
$scopes = Tokens::$ooScopeTokens;
$this->PHPDocFormattingValidator = new PHPDocFormattingValidator();
$listen = [
T_VARIABLE,
T_DOUBLE_QUOTED_STRING,
T_HEREDOC,
];

parent::__construct($scopes, $listen, true);
}

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

$commentEnd = $phpcsFile->findPrevious($this->ignoreTokens, ($stackPtr - 1), null, true);
if ($commentEnd === false
|| ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG
&& $tokens[$commentEnd]['code'] !== T_COMMENT)
) {
$phpcsFile->addWarning('Missing PHP DocBlock for class property.', $stackPtr, 'Missing');
return;
}
$commentStart = $tokens[$commentEnd]['comment_opener'];
$foundVar = null;
foreach ($tokens[$commentStart]['comment_tags'] as $tag) {
if ($tokens[$tag]['content'] === '@var') {
if ($foundVar !== null) {
$error = 'Only one @var tag is allowed for class property declaration.';
$phpcsFile->addWarning($error, $tag, 'DuplicateVar');
} else {
$foundVar = $tag;
}
}
}

if ($foundVar === null) {
$error = 'Class properties must have type declaration using @var tag.';
$phpcsFile->addWarning($error, $stackPtr, 'MissingVar');
return;
}

$string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar, $commentEnd);
if ($string === false || $tokens[$string]['line'] !== $tokens[$foundVar]['line']) {
$error = 'Content missing for @var tag in class property declaration.';
$phpcsFile->addWarning($error, $foundVar, 'EmptyVar');
return;
}

// Check if class has already have meaningful description after @var tag
$isShortDescriptionAfterVar = $phpcsFile->findNext(
T_DOC_COMMENT_STRING,
$foundVar + 4,
$commentEnd,
false,
null,
false
);
if ($this->PHPDocFormattingValidator->providesMeaning(
Copy link
Contributor

Choose a reason for hiding this comment

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

This if block can be extracted into a separate method, as it basically repeats is the same as the check done for checking description before @var tag. This way, we reduce repetition and also function size, as processMemeberSize() is quite big, helping readability along the way

$isShortDescriptionAfterVar,
$commentStart,
$tokens
) !== true) {
preg_match(
'`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i',
$tokens[($foundVar + 2)]['content'],
$varParts
);
if ($varParts[1]) {
return;
}
$error = 'Short description duplicates class property name.';
$phpcsFile->addWarning($error, $isShortDescriptionAfterVar, 'AlreadyHaveMeaningFulNameVar');
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct camel casing: AlreadyHaveMeaningfulNameVar

return;
}
// Check if class has already have meaningful description before @var tag
$isShortDescriptionPreviousVar = $phpcsFile->findPrevious(
T_DOC_COMMENT_STRING,
$foundVar,
$commentStart,
false,
null,
false
);
if ($this->PHPDocFormattingValidator->providesMeaning(
$isShortDescriptionPreviousVar,
$commentStart,
$tokens
) !== true) {
preg_match(
'`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i',
$tokens[($foundVar + 2)]['content'],
$varParts
);
if ($varParts[1]) {
return;
}
$error = 'Short description duplicates class property name.';
$phpcsFile->addWarning($error, $isShortDescriptionPreviousVar, 'AlreadyHaveMeaningFulNameVar');
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct camel casing: AlreadyHaveMeaningfulNameVar

return;
}
}

/**
* @inheritDoc
* phpcs:disable Magento2.CodeAnalysis.EmptyBlock
*/
protected function processVariable(File $phpcsFile, $stackPtr)
{
}

/**
* @inheritDoc
* phpcs:disable Magento2.CodeAnalysis.EmptyBlock
*/
protected function processVariableInString(File $phpcsFile, $stackPtr)
{
}
}
1 change: 1 addition & 0 deletions Magento2/Sniffs/Exceptions/DirectThrowSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class DirectThrowSniff implements Sniff
/**
* String representation of warning.
* phpcs:disable Generic.Files.LineLength.TooLong
* @var string
*/
protected $warningMessage = 'Direct throw of generic Exception is discouraged. Use context specific instead.';
//phpcs:enable
Expand Down
2 changes: 0 additions & 2 deletions Magento2/Sniffs/Less/AvoidIdSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ class AvoidIdSniff implements Sniff
public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS];

/**
* Tokens that can appear in a selector
*
* @var array
*/
private $selectorTokens = [
Expand Down
3 changes: 1 addition & 2 deletions Magento2/Sniffs/Less/IndentationSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class IndentationSniff implements Sniff
*/
public $maxIndentLevel = 3;

/** Skip codes that can be detected by sniffer incorrectly
*
/**
* @var array
*/
private $styleCodesToSkip = [T_ASPERAND, T_COLON, T_OPEN_PARENTHESIS, T_CLOSE_PARENTHESIS];
Expand Down
4 changes: 0 additions & 4 deletions Magento2/Sniffs/Less/PropertiesSortingSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,11 @@ class PropertiesSortingSniff implements Sniff
public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS];

/**
* List of properties that belong to class
*
* @var array
*/
private $properties = [];

/**
* Skip symbols that can be detected by sniffer incorrectly
*
* @var array
*/
private $styleSymbolsToSkip = [
Expand Down
5 changes: 1 addition & 4 deletions Magento2/Sniffs/Less/SemicolonSpacingSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ class SemicolonSpacingSniff implements Sniff
public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS];

/**
* Skip symbols that can be detected by sniffer incorrectly
*
* @var array
*/
private $styleSymbolsToSkip = [
Expand All @@ -36,8 +34,7 @@ class SemicolonSpacingSniff implements Sniff
TokenizerSymbolsInterface::CLOSE_PARENTHESIS,
];

/** Skip codes that can be detected by sniffer incorrectly
*
/**
* @var array
*/
private $styleCodesToSkip = [T_ASPERAND, T_COLON, T_OPEN_PARENTHESIS, T_CLOSE_PARENTHESIS];
Expand Down
2 changes: 0 additions & 2 deletions Magento2/Sniffs/Less/TypeSelectorsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
class TypeSelectorsSniff implements Sniff
{
/**
* Tags that are not allowed as type selector
*
* @var array
*/
private $tags = [
Expand Down
2 changes: 0 additions & 2 deletions Magento2/Sniffs/Less/ZeroUnitsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ class ZeroUnitsSniff implements Sniff
const CSS_PROPERTY_UNIT_REM = 'rem';

/**
* List of available CSS Property units
*
* @var array
*/
private $units = [
Expand Down
2 changes: 0 additions & 2 deletions Magento2/Sniffs/Methods/DeprecatedModelMethodSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ class DeprecatedModelMethodSniff implements Sniff
'delete'
];

protected $severity = 0;

/**
* @inheritdoc
*/
Expand Down
4 changes: 2 additions & 2 deletions Magento2/Sniffs/Security/SuperglobalSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class SuperglobalSniff implements Sniff
protected $errorCode = 'SuperglobalUsageError';

/**
* @inheritdoc
* @var array
*/
protected $superGlobalErrors = [
'$GLOBALS',
Expand All @@ -55,7 +55,7 @@ class SuperglobalSniff implements Sniff
];

/**
* @inheritdoc
* @var array
*/
protected $superGlobalWarning = [
'$_COOKIE', //sometimes need to get list of all cookies array and there are no methods to do that in M2
Expand Down
2 changes: 0 additions & 2 deletions Magento2/Sniffs/Security/XssTemplateSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ class XssTemplateSniff implements Sniff
];

/**
* Allowed method name - {suffix}Html{postfix}()
*
* @var string
*/
protected $methodNameContains = 'html';
Expand Down
Loading