Skip to content

Commit

Permalink
WIP / review - TODO: rest of sniff + tests reviewen
Browse files Browse the repository at this point in the history
  • Loading branch information
@jrfnl
jrfnl committed Dec 9, 2022
1 parent a8d53eb commit 5834c64
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 55 deletions.
16 changes: 10 additions & 6 deletions Universal/Docs/DeclareStatements/BlockModeStandard.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ declare(strict_types=1);
declare(encoding='utf-8');
declare(ticks=10):
// Code.
// Code.
enddeclare;
]]>
</code>
Expand All @@ -36,20 +36,24 @@ enddeclare;
</code_comparison>
<standard>
<![CDATA[
strict_types declaration directive mustn't be written using curly braces, or with alternative syntax.
A declare statement for the `strict_types` directive is not allowed to be written using curly braces or with alternative syntax.
]]>
</standard>
<code_comparison>
<code title="Valid: strict_types written without curly braces.">
<code title="Valid: strict_types statement not using block mode.">
<![CDATA[
declare(strict_types=1);
]]>
</code>
<code title="Invalid: strict_types written using alternative syntax/curly braces.">
<code title="Invalid: strict_types statement using alternative syntax/curly braces.">
<![CDATA[
declare(strict_types=1, ticks=1) <em>{</em>
// Code.
declare(strict_types=1) <em>{</em>
// Code.
<em>}</em>
declare(strict_types=1) <em>:</em>
// Code.
<em>enddeclare</em>;
]]>
</code>
</code_comparison>
Expand Down
97 changes: 58 additions & 39 deletions Universal/Sniffs/DeclareStatements/BlockModeSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
/**
* Checks the style of the declare statement.
*
* `declare` directives can be written in two different styles:
* Declare statements can be written in two different styles:
* 1. Applied to the rest of the file, usually written at the top of a file like `declare(strict_types=1);`.
* 2. Applied to a limited scope using curly braces or using alternative control structure syntax
* (the exception to this rule is the `strict_types` directive). This is known as a block mode.
* (the exception to this rule is the `strict_types` directive). This is known as block mode.
*
* You can also have multiple directives written inside the `declare` directive.
* This sniff will check the preferred mode of the `declare` directive.
* There can be multiple directives inside a `declare` statement.
* This sniff checks the preferred mode for the `declare` statements.
*
* You can modify the sniff by changing the whether the block mode of the encoding and the ticks directives
* is allowed, disallowed or required. By default, the ticks directive, if written in
Expand All @@ -45,7 +45,7 @@ class BlockModeSniff implements Sniff
*
* @var string
*/
const DECLARE_SCOPE_METRIC = 'Declare directive scope';
const DECLARE_SCOPE_METRIC = 'Declare statement scope';

/**
* Name of the metric.
Expand All @@ -57,10 +57,10 @@ class BlockModeSniff implements Sniff
const DECLARE_TYPE_METRIC = 'Declare directive type';

/**
* The option for the encoding directive.
* Whether block mode is allowed for `encoding` directives.
*
* Can be one of: 'disallow', 'allow' (no preference), 'require'.
* By default it's disallowed.
* Can be one of: 'disallow', 'allow' (no preference), or 'require'.
* Defaults to: 'disallow'.
*
* @since 1.0.0
*
Expand All @@ -69,10 +69,10 @@ class BlockModeSniff implements Sniff
public $encodingBlockMode = 'disallow';

/**
* The option for the ticks directive.
* Whether block mode is allowed for `ticks` directives.
*
* Can be one of: 'disallow', 'allow' (no preference), 'require'.
* By default it's allowed.
* Can be one of: 'disallow', 'allow' (no preference), or 'require'.
* Defaults to: 'allow'.
*
* @since 1.0.0
*
Expand All @@ -83,7 +83,7 @@ class BlockModeSniff implements Sniff
/**
* The default option for the strict_types directive.
*
* Only directive that cannot be written in block mode is strict_types.
* Block mode is not allowed for the `strict_types` directive.
* Using it in block mode will throw a PHP fatal error.
*
* @since 1.0.0
Expand Down Expand Up @@ -114,9 +114,7 @@ class BlockModeSniff implements Sniff
*/
public function register()
{
return [
T_DECLARE
];
return [T_DECLARE];
}

/**
Expand Down Expand Up @@ -146,10 +144,12 @@ public function process(File $phpcsFile, $stackPtr)
// Get the next string and check if it's an allowed directive.
// Find all the directive strings inside the declare statement.
for ($i = $openParenPtr; $i <= $closeParenPtr; $i++) {
if ($tokens[$i]['code'] === \T_STRING
&& isset($this->allowedDirectives[\strtolower($tokens[$i]['content'])])
) {
$directiveStrings[$tokens[$i]['content']] = true;
if ($tokens[$i]['code'] === \T_STRING) {
$contentsLC = \strtolower($tokens[$i]['content']);
if (isset($this->allowedDirectives[$contentsLC])) {
$phpcsFile->recordMetric($i, self::DECLARE_TYPE_METRIC, $contentsLC);
$directiveStrings[$contentsLC] = true;
}
}
}

Expand All @@ -163,40 +163,59 @@ public function process(File $phpcsFile, $stackPtr)
$usesBlockMode = isset($tokens[$stackPtr]['scope_opener']);

if ($usesBlockMode) {
$phpcsFile->recordMetric($stackPtr, self::DECLARE_SCOPE_METRIC, 'Block');
$phpcsFile->recordMetric($stackPtr, self::DECLARE_SCOPE_METRIC, 'Block mode');
} else {
$phpcsFile->recordMetric($stackPtr, self::DECLARE_SCOPE_METRIC, 'Global');
$phpcsFile->recordMetric($stackPtr, self::DECLARE_SCOPE_METRIC, 'File mode');
}

// If strict types is defined using block mode, throw error.
if ($usesBlockMode && isset($directiveStrings['strict_types'])) {
$phpcsFile->addError(
'strict_types declaration must not use block mode.',
$stackPtr,
'Forbidden'
);
$error = 'strict_types declaration must not use block mode.';
$code = 'Forbidden';

if (isset($tokens[$stackPtr]['scope_closer'])) {
// If there is no scope closer, we cannot auto-fix.
$phpcsFile->addError($error, $stackPtr, $code);
return;
}

$fix = $phpcsFile->addFixableError($error, $stackPtr, $code);

if ($fix === true) {
$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent($closeParenPtr, ';');
$phpcsFile->fixer->replaceToken($tokens[$stackPtr]['scope_opener'], '');

// Remove potential whitespace between parenthesis closer and the brace.
for ($i = ($tokens[$stackPtr]['scope_opener'] - 1); $i > 0; $i--) {
if ($tokens[$i]['code'] !== \T_WHITESPACE) {
break;
}

$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->replaceToken($tokens[$stackPtr]['scope_closer'], '');
$phpcsFile->fixer->endChangeset();
}
return;
}

// Check if there is a code between the declare statement and opening brace/alternative syntax.
$nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($closeParenPtr + 1), null, true);
$directiveCloserTokens = [\T_SEMICOLON, \T_CLOSE_TAG, \T_OPEN_CURLY_BRACKET, \T_COLON];

if (!in_array($tokens[$nextNonEmpty]['code'], $directiveCloserTokens, true)) {
// Check if there is code between the declare statement and opening brace/alternative syntax.
$nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($closeParenPtr + 1), null, true);
if ($tokens[$nextNonEmpty]['code'] !== \T_SEMICOLON
&& $tokens[$nextNonEmpty]['code'] !== \T_CLOSE_TAG
&& $tokens[$nextNonEmpty]['code'] !== \T_OPEN_CURLY_BRACKET
&& $tokens[$nextNonEmpty]['code'] !== \T_COLON
) {
$phpcsFile->addError(
'Unexpected code found after opening the declare statement without closing it.',
'Unexpected code found after the declare statement.',
$stackPtr,
'UnexpectedCodeFound'
);
return;
}

foreach (\array_keys($directiveStrings) as $directiveString) {
if (isset($this->allowedDirectives[$directiveString])) {
$phpcsFile->recordMetric($stackPtr, self::DECLARE_TYPE_METRIC, $directiveString);
}
}

// Multiple directives - if one requires block mode usage, other has to as well.
if (count($directiveStrings) > 1
&& (($this->encodingBlockMode === 'disallow' && $this->ticksBlockMode !== 'disallow')
Expand All @@ -205,7 +224,7 @@ public function process(File $phpcsFile, $stackPtr)
$phpcsFile->addError(
'Multiple directives found, but one of them is disallowing the use of block mode.',
$stackPtr,
'Forbidden'
'Forbidden' // <= Duplicate error code for different message (line 175)
);
return;
}
Expand Down
4 changes: 0 additions & 4 deletions Universal/Tests/DeclareStatements/BlockModeUnitTest.1.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,3 @@ echo 'hi!';
}

declare ticks=1; // Intentional live coding error. Ignore.

/* Rest to the default directives for the next run */
// phpcs:set Universal.DeclareStatements.BlockMode encodingBlockMode disallow
// phpcs:set Universal.DeclareStatements.BlockMode ticksBlockMode allow
4 changes: 1 addition & 3 deletions Universal/Tests/DeclareStatements/BlockModeUnitTest.2.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@ declare(enCodINg='1');
declare(STRICT_TYPES=1);
declare(random_directive=false);

/* Rest to the default directives for the next run */
// phpcs:set Universal.DeclareStatements.BlockMode encodingBlockMode disallow
// phpcs:set Universal.DeclareStatements.BlockMode ticksBlockMode allow
// What's invalid about these ? (safe for line 4 and 8) Directives are case-INsensitive.
2 changes: 1 addition & 1 deletion Universal/Tests/DeclareStatements/BlockModeUnitTest.3.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ declare(strict_types=1, encoding='ISO-8859-1'): // Error.
enddeclare;
enddeclare;

/* Rest to the default directives for the next run */
/* Reset to the default directives for the next run */
// phpcs:set Universal.DeclareStatements.BlockMode encodingBlockMode disallow
// phpcs:set Universal.DeclareStatements.BlockMode ticksBlockMode allow
2 changes: 1 addition & 1 deletion Universal/Tests/DeclareStatements/BlockModeUnitTest.4.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,6 @@ declare(ticks=1): // Ok.
enddeclare;
enddeclare;

/* Rest to the default directives for the next run */
/* Reset to the default directives for the next run */
// phpcs:set Universal.DeclareStatements.BlockMode encodingBlockMode disallow
// phpcs:set Universal.DeclareStatements.BlockMode ticksBlockMode allow
2 changes: 1 addition & 1 deletion Universal/Tests/DeclareStatements/BlockModeUnitTest.5.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,6 @@ declare(ticks=1) { // Ok.
// Code.
}

/* Rest to the default directives for the next run */
/* Reset to the default directives for the next run */
// phpcs:set Universal.DeclareStatements.BlockMode encodingBlockMode disallow
// phpcs:set Universal.DeclareStatements.BlockMode ticksBlockMode allow

0 comments on commit 5834c64

Please sign in to comment.