Skip to content
Snippets Groups Projects
Unverified Commit f9fb9a17 authored by Jonathan Smith's avatar Jonathan Smith Committed by GitHub
Browse files

feat(Deprecated): Add fixer for Drupal core deprecated tag formatting (#3057988 by jonathan1055)

parent e53e75b4
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
coder_sniffer/Drupal/Sniffs/Commenting/DeprecatedSniff.php
+ 117
− 24
View file @ f9fb9a17
......@@ -11,9 +11,10 @@ namespace Drupal\Sniffs\Commenting;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Config;
/**
* Ensures standard format of a @deprecated text.
* Ensures standard format of @ deprecated tag text in docblock.
*
* @category PHP
* @package PHP_CodeSniffer
......@@ -22,6 +23,15 @@ use PHP_CodeSniffer\Sniffs\Sniff;
class DeprecatedSniff implements Sniff
{
/**
* Show debug output for this sniff.
*
* Use phpcs --runtime-set deprecated_debug true
*
* @var boolean
*/
private $debug = false;
/**
* Returns an array of tokens this test wants to listen for.
......@@ -30,6 +40,10 @@ class DeprecatedSniff implements Sniff
*/
public function register()
{
if (defined('PHP_CODESNIFFER_IN_TESTS') === true) {
$this->debug = false;
}
return [T_DOC_COMMENT_TAG];
}//end register()
......@@ -46,6 +60,11 @@ class DeprecatedSniff implements Sniff
*/
public function process(File $phpcsFile, $stackPtr)
{
$debug = Config::getConfigData('deprecated_debug');
if ($debug !== null) {
$this->debug = (bool) $debug;
}
$tokens = $phpcsFile->getTokens();
// Only process @deprecated tags.
......@@ -57,13 +76,13 @@ class DeprecatedSniff implements Sniff
$commentEnd = $phpcsFile->findNext(T_DOC_COMMENT_CLOSE_TAG, ($stackPtr + 1));
// Get the full @deprecated text which may cover multiple lines.
$depText = '';
$depEnd = ($stackPtr + 1);
$textItems = [];
$lastLine = $tokens[($stackPtr + 1)]['line'];
for ($i = ($stackPtr + 1); $i < $commentEnd; $i++) {
if ($tokens[$i]['code'] === T_DOC_COMMENT_STRING) {
if ($tokens[$i]['line'] <= ($tokens[$depEnd]['line'] + 1)) {
$depText .= ' '.$tokens[$i]['content'];
$depEnd = $i;
if ($tokens[$i]['line'] <= ($lastLine + 1)) {
$textItems[$i] = $tokens[$i]['content'];
$lastLine = $tokens[$i]['line'];
} else {
break;
}
......@@ -75,20 +94,79 @@ class DeprecatedSniff implements Sniff
}
}
$depText = trim($depText);
// The standard format for the deprecation text is:
// @deprecated in %in-version% and will be removed from %removal-version%. %extra-info%.
// Use (?U) 'ungreedy' before the version so that only the text up to
// the first '. ' is matched, as there may be more than one sentence in
// the extra-info part.
$matches = [];
preg_match('/in (.+) and is removed from (?U)(.+)\. (.+)$/', $depText, $matches);
// @deprecated in %in-version% and is removed from %removal-version%. %extra-info%.
$standardFormat = "@deprecated in %%deprecation-version%% and is removed from %%removal-version%%. %%extra-info%%.";
// Use (?U) 'ungreedy' before the removal-version so that only the text
// up to the first dot+space is matched, as there may be more than one
// sentence in the extra-info part.
$fullText = trim(implode(' ', $textItems));
$matches = [];
preg_match('/^in (.+) and is removed from (?U)(.+)(?:\. | |\.$|$)(.*)$/', $fullText, $matches);
// There should be 4 items in $matches: 0 is full text, 1 = in-version,
// 2 = removal-version, 3 = extra-info.
// 2 = removal-version, 3 = extra-info (can be blank at this stage).
if (count($matches) !== 4) {
$error = "The text '@deprecated %s' does not match the standard format: @deprecated in %%deprecation-version%% and is removed from %%removal-version%%. %%extra-info%%.";
$phpcsFile->addError($error, $stackPtr, 'IncorrectTextLayout', [$depText]);
// The full text does not match the standard. Try to find fixes by
// testing with a relaxed set of criteria, based on common
// formatting variations. This is designed for Core fixes only.
$error = "The text '@deprecated %s' does not match the standard format: ".$standardFormat;
// All of the standard text should be on the first comment line, so
// try to match with common formatting errors to allow an automatic
// fix. If not possible then report a normal error.
$matchesFix = [];
$fix = null;
if (count($textItems) > 0) {
// Get just the first line of the text.
$key = array_keys($textItems)[0];
$text1 = $textItems[$key];
// Matching on (drupal|) here says that we are only attempting to provide
// automatic fixes for Drupal core, and if the project is missing we are
// assuming it is Drupal core. Deprecations for contrib projects are much
// less frequent and faults can be corrected manually.
preg_match('/^(.*)(as of|in) (drupal|)( |:|)+([\d\.\-xdev\?]+)(,| |. |)(.*)(removed|removal)([ |from|before|in|the]*) (drupal|)( |:|)([\d\-\.xdev]+)( |,|$)+(?:release|)(?:[\.,])*(.*)$/i', $text1, $matchesFix);
if (count($matchesFix) >= 12) {
// It is a Drupal core deprecation and is fixable.
if (empty($matchesFix[1]) === false && $this->debug === true) {
// For info, to check it is acceptable to remove the text in [1].
echo('DEBUG: File: '.$phpcsFile->path.', line '.$tokens[($stackPtr)]['line'].PHP_EOL);
echo('DEBUG: "@deprecated '.$text1.'"'.PHP_EOL);
echo('DEBUG: Fix will remove: "'.$matchesFix[1].'"'.PHP_EOL);
}
$ver1 = str_Replace(['-dev', 'x'], ['', '0'], trim($matchesFix[5], '.'));
$ver2 = str_Replace(['-dev', 'x'], ['', '0'], trim($matchesFix[12], '.'));
// If the version is short, add enough '.0' to correct it.
while (substr_count($ver1, '.') < 2) {
$ver1 .= '.0';
}
while (substr_count($ver2, '.') < 2) {
$ver2 .= '.0';
}
$correctedText = trim('in drupal:'.$ver1.' and is removed from drupal:'.$ver2.'. '.trim($matchesFix[14]));
// If $correctedText is longer than 65 this will make the whole line
// exceed 80 so give a warning if running with debug.
if (strlen($correctedText) > 65 && $this->debug === true) {
echo('WARNING: File '.$phpcsFile->path.', line '.$tokens[($stackPtr)]['line'].PHP_EOL);
echo('WARNING: Original = * @deprecated '.$text1.PHP_EOL);
echo('WARNING: Corrected = * @deprecated '.$correctedText.PHP_EOL);
echo('WARNING: New line length '.(strlen($correctedText) + 15).' exceeds standard 80 character limit'.PHP_EOL);
}
$fix = $phpcsFile->addFixableError($error, $key, 'IncorrectTextLayout', [$fullText]);
if ($fix === true) {
$phpcsFile->fixer->replaceToken($key, $correctedText);
}
}//end if
}//end if
if ($fix === null) {
// There was no automatic fix, so give a normal error.
$phpcsFile->addError($error, $stackPtr, 'IncorrectTextLayout', [$fullText]);
}
} else {
// The text follows the basic layout. Now check that the versions
// match drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-version[n].
......@@ -102,7 +180,15 @@ class DeprecatedSniff implements Sniff
$phpcsFile->addWarning($error, $stackPtr, 'DeprecatedVersionFormat', [$name, $version]);
}
}
}
// The 'IncorrectTextLayout' above is designed to pass if all is ok
// except for missing extra info. This is a common fault so provide
// a separate check and message for this.
if ($matches[3] === '') {
$error = 'The @deprecated tag must have %extra-info%. The standard format is: '.str_replace('%%', '%', $standardFormat);
$phpcsFile->addError($error, $stackPtr, 'MissingExtraInfo', []);
}
}//end if
// The next tag in this comment block after @deprecated must be @see.
$seeTag = $phpcsFile->findNext(T_DOC_COMMENT_TAG, ($stackPtr + 1), $commentEnd, false, '@see');
......@@ -114,15 +200,22 @@ class DeprecatedSniff implements Sniff
// Check the format of the @see url.
$string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, ($seeTag + 1), $commentEnd);
$crLink = $tokens[$string]['content'];
// If the @see tag exists but has no content then $string will be empty
// and $tokens[$string]['content'] will return '<?php' which makes the
// standards message confusing. Better to set crLink to blank here.
if ($string === false) {
$crLink = ' ';
} else {
$crLink = $tokens[$string]['content'];
}
// Allow for the alternative 'node' or 'project/aaa/issues' format.
preg_match('[^http(s*)://www.drupal.org/(node|project/\w+/issues)/(\d+)(\.*)$]', $crLink, $matches);
// If matches[4] is not blank it means that the url is correct but it
// ends with a period. As this can be a common mistake give a specific
// message to assist in fixing.
if (isset($matches[4]) === true && empty($matches[4]) === false) {
$error = "The @see url '%s' should not end with a period.";
$phpcsFile->addWarning($error, $seeTag, 'DeprecatedPeriodAfterSeeUrl', [$crLink]);
// If matches[4] is not blank it means that the url is correct but
// it ends with a period. This is covered by the generic fixable
// sniff FunctionCommentSniff.SeePunctuation so allow that sniff to
// report it (and fix it).
} else if (empty($matches) === true) {
$error = "The @see url '%s' does not match the standard: http(s)://www.drupal.org/node/n or http(s)://www.drupal.org/project/aaa/issues/n";
$phpcsFile->addWarning($error, $seeTag, 'DeprecatedWrongSeeUrlFormat', [$crLink]);
......
coder_sniffer/Drupal/Test/Commenting/DeprecatedUnitTest.inc
+ 50
− 18
View file @ f9fb9a17
......@@ -2,9 +2,9 @@
/**
* @file
* This file contains test cases for DeprecatedSniff.
*
* @deprecated in drupal:8.4.0 and is removed from drupal:8.6.0.
* Use something else instead.
* This file doc block passes the 'Deprecated' standards checks.
*
* @see https://www.drupal.org/node/1234
......@@ -13,46 +13,46 @@
/**
* This doc block will also pass the 'Deprecated' standards checks.
*
* @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0.
* This function block is OK.
* @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0. This function
* block is OK.
* @see https://www.drupal.org/project/drupal/issues/5678
*/
function x() {
}
/**
* This block will fail with two errors.
*
* @deprecated in Drupal 8.4.
* This will fail basic layout.
* It also has no 'see' url.
* The basic layout is wrong and there is no 'see' url.
*/
/**
* This is here to ensure that the 'see' url has to be in the same block.
* @see https://www.drupal.org/node/5678
* This shows that a 'see' url without a 'deprecated' tag does not get reported.
*
* @see https://www.example.com/incorrect-for-a-deprecated-see-url
*/
/**
* This block will produce three warnings.
* This block will produce three warnings (but the basic layout is correct).
*
* @Deprecated in drupal 8.4.0 and is removed from drupal:9.0.
* @Deprecated in Drupal 8.4.0 and is removed from 9.x
* Both version formats are incorrect, and the see url is wrong.
* @see http:drupal.org
* @see http:drupal.org/node/1234
*/
function y() {
}
/**
* This block will produce three warnings.
* This contrib block will produce three warnings.
*
* @deprecated in My_Proj:8.x-1.8 and is removed from my_Proj:8.x-2.
* Both version formats are incorrect, and the see url ends with a period.
* @see http://www.drupal.org/node/7890.
* @deprecated in My_Proj:8.x-1.8 and is removed from my_Proj-8.x-2
* Both version formats are incorrect and see url is incorrect.
* @see http://www.drupal.org/project/issues/1234
*/
function z() {
}
/**
* This doc block will pass the 'Deprecated' standards checks.
* This contrib block will pass the 'Deprecated' standards checks.
*
* @deprecated in myproj:8.x-1.8 and is removed from myproj:8.x-2.0.
* This function block is OK.
......@@ -60,7 +60,7 @@ function z() {
*/
/**
* This doc block will pass the 'Deprecated' standards checks.
* This contrib doc block will pass the 'Deprecated' standards checks.
*
* @deprecated in project:8.x-1.0-beta3 and is removed from project:8.x-1.0-rc1.
* Not stable versions rc, alpha and etc should be correctly accepted.
......@@ -68,3 +68,35 @@ function z() {
*/
function w() {
}
/**
* This will produce two errors (no text at all, hence bad layout and no see)
*
* @deprecated
*/
/**
* Will give one error for no extra info and one warning for version format.
*
* @deprecated in drupal:8.6.0 and is removed from 9.x
*
* @see http://www.drupal.org/node/7890
*/
/**
* This block has incorrect basic layout but is fixable.
*
* @deprecated deprecated in Drupal 8.7, removed in 8.9.x There is extra
* text before the 'in' version, wrong case, missing :, missing middle text,
* missing 'drupal' and incorrect 'removal' version, no period after version.
* @see http://www.drupal.org/node/123
*/
/**
* This block also has incorrect basic layout and is fixable.
*
* @deprecated as of 8.8.3-dev, scheduled for removal in DRUPAL 9
* It has 'as of' not 'in', missing word 'drupal', has -dev in first version,
* has 'removal', upper-case 'drupal' and short second version.
* @see http://www.drupal.org/node/123
*/
coder_sniffer/Drupal/Test/Commenting/DeprecatedUnitTest.inc.fixed 0 → 100644
+ 102
− 0
View file @ f9fb9a17
<?php
/**
* @file
* This file contains test cases for DeprecatedSniff.
*
* @deprecated in drupal:8.4.0 and is removed from drupal:8.6.0.
* This file doc block passes the 'Deprecated' standards checks.
*
* @see https://www.drupal.org/node/1234
*/
/**
* This doc block will also pass the 'Deprecated' standards checks.
*
* @deprecated in drupal:8.6.0 and is removed from drupal:9.0.0. This function
* block is OK.
* @see https://www.drupal.org/project/drupal/issues/5678
*/
/**
* This block will fail with two errors.
*
* @deprecated in Drupal 8.4.
* The basic layout is wrong and there is no 'see' url.
*/
/**
* This shows that a 'see' url without a 'deprecated' tag does not get reported.
*
* @see https://www.example.com/incorrect-for-a-deprecated-see-url
*/
/**
* This block will produce three warnings (but the basic layout is correct).
*
* @Deprecated in Drupal 8.4.0 and is removed from 9.x
* Both version formats are incorrect, and the see url is wrong.
* @see http:drupal.org/node/1234
*/
function y() {
}
/**
* This contrib block will produce three warnings.
*
* @deprecated in My_Proj:8.x-1.8 and is removed from my_Proj-8.x-2
* Both version formats are incorrect and see url is incorrect.
* @see http://www.drupal.org/project/issues/1234
*/
function z() {
}
/**
* This contrib block will pass the 'Deprecated' standards checks.
*
* @deprecated in myproj:8.x-1.8 and is removed from myproj:8.x-2.0.
* This function block is OK.
* @see http://www.drupal.org/node/7890
*/
/**
* This contrib doc block will pass the 'Deprecated' standards checks.
*
* @deprecated in project:8.x-1.0-beta3 and is removed from project:8.x-1.0-rc1.
* Not stable versions rc, alpha and etc should be correctly accepted.
* @see http://www.drupal.org/node/7890
*/
function w() {
}
/**
* This will produce two errors (no text at all, hence bad layout and no see)
*
* @deprecated
*/
/**
* Will give one error for no extra info and one warning for version format.
*
* @deprecated in drupal:8.6.0 and is removed from 9.x
*
* @see http://www.drupal.org/node/7890
*/
/**
* This block has incorrect basic layout but is fixable.
*
* @deprecated in drupal:8.7.0 and is removed from drupal:8.9.0. There is extra
* text before the 'in' version, wrong case, missing :, missing middle text,
* missing 'drupal' and incorrect 'removal' version, no period after version.
* @see http://www.drupal.org/node/123
*/
/**
* This block also has incorrect basic layout and is fixable.
*
* @deprecated in drupal:8.8.3 and is removed from drupal:9.0.0.
* It has 'as of' not 'in', missing word 'drupal', has -dev in first version,
* has 'removal', upper-case 'drupal' and short second version.
* @see http://www.drupal.org/node/123
*/
coder_sniffer/Drupal/Test/Commenting/DeprecatedUnitTest.php
+ 27
− 6
View file @ f9fb9a17
......@@ -14,14 +14,29 @@ class DeprecatedUnitTest extends CoderSniffUnitTest
* The key of the array should represent the line number and the value
* should represent the number of errors that should occur on that line.
*
* There are three deprecated sniffs which produce an error:
* 'IncorrectTextLayout - for the basic deprecation text, sometimes fixable.
* 'MissingExtraInfo' - when there is no extra info after the main text.
* 'DeprecatedMissingSeeTag' - when there is no @see tag.
*
* @param string $testFile The name of the file being tested.
*
* @return array<int, int>
*/
protected function getErrorList(string $testFile): array
{
// Basic layout is wrong.
return [24 => 2];
return [
// Basic layout is wrong. Missing see url.
24 => 2,
// No details given, check that the test gives two errors.
75 => 2,
// Layout OK but missing the extra info.
81 => 1,
// Text layout is wrong but fixable.
89 => 1,
// Text layout is wrong but fixable.
98 => 1,
];
}//end getErrorList()
......@@ -32,6 +47,10 @@ class DeprecatedUnitTest extends CoderSniffUnitTest
* The key of the array should represent the line number and the value
* should represent the number of warnings that should occur on that line.
*
* There are two deprecated sniffs which produce a warning:
* 'DeprecatedVersionFormat' - where the version is written incorrectly.
* 'DeprecatedWrongSeeUrlFormat' - the url is not to the standard format.
*
* @param string $testFile The name of the file being tested.
*
* @return array<int, int>
......@@ -39,14 +58,16 @@ class DeprecatedUnitTest extends CoderSniffUnitTest
protected function getWarningList(string $testFile): array
{
return [
// Has version x 2.
// Both core versions incorrectly formatted.
37 => 2,
// The see url.
// The see url is wrong.
39 => 1,
// Has version x 2.
// Both contrib versions incorrectly formatted.
47 => 2,
// The see url.
// The see url is wrong.
49 => 1,
// Core version incorrectly formatted.
81 => 1,
];
}//end getWarningList()
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment