Skip to content
Open
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
5 changes: 5 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
Ensures that strings use single quotes when possible. Options are:
- `skipStringContainingSingleQuote`: ignore double-quoted strings that contains single-quotes (default true).

- **StrictComparisonOperatorRule**:

Ensures that strict comparison operators `===` and `!==` are used instead of `same as` and `not same as`.

- **TrailingCommaMultiLineRule** (Configurable):

Ensures that multi-line arrays, objects and argument lists have a trailing comma. Options are:
Expand Down Expand Up @@ -193,6 +197,7 @@ new TwigCsFixer\Rules\Whitespace\IndentRule(3);
- IncludeFunctionRule
- IndentRule
- SingleQuoteRule
- StrictComparisonOperatorRule
- TrailingCommaMultiLineRule
- TrailingCommaSingleLineRule
- TrailingSpaceRule
Expand Down
108 changes: 108 additions & 0 deletions src/Rules/Operator/StrictComparisonOperatorRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php

declare(strict_types=1);

namespace TwigCsFixer\Rules\Operator;

use TwigCsFixer\Rules\AbstractFixableRule;
use TwigCsFixer\Token\Token;
use TwigCsFixer\Token\Tokens;

/**
* Ensures that strict comparison operators are used instead of "same as" and "not same as".
*/
final class StrictComparisonOperatorRule extends AbstractFixableRule
{
protected function process(int $tokenIndex, Tokens $tokens): void
{
$token = $tokens->get($tokenIndex);

if (!$token->isMatching(Token::OPERATOR_TYPE, ['is', 'is not'])) {
return;
}

$isNegated = $token->isMatching(Token::OPERATOR_TYPE, 'is not');

$sameAsIndex = $tokens->findNext(Token::WHITESPACE_TOKENS, $tokenIndex + 1, exclude: true);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Exclude EMPTY_TOKENS rather than WHITESPACE.

Cause here you don't support someone with an indent in the middle of the code or a new line. (And add test with this case)

if (false === $sameAsIndex) {
return;
}

$sameAsToken = $tokens->get($sameAsIndex);
$targetIndex = $sameAsIndex;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't understand the need of this new variable.


if (!$sameAsToken->isMatching(Token::TEST_NAME_TYPE, 'same')) {
return;
}

$asIndex = $tokens->findNext(Token::WHITESPACE_TOKENS, $targetIndex + 1, exclude: true);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Exclude EMPTY_TOKENS rather than WHITESPACE.

Cause here you don't support someone with an indent in the middle of the code or a new line. (And add test with this case)

if (false === $asIndex || !$tokens->get($asIndex)->isMatching(Token::TEST_NAME_TYPE, 'as')) {
return;
}

$openParenthesisIndex = $tokens->findNext(Token::WHITESPACE_TOKENS, $asIndex + 1, exclude: true);
if (false === $openParenthesisIndex || !$tokens->get($openParenthesisIndex)->isMatching(Token::PUNCTUATION_TYPE, '(')) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You cannot return if there is no punctuation since

{% if foo is same as bar %}

is a valid syntax similar to

{% if foo is same as(bar) %}

return;
}

$closeParenthesisIndex = $this->findMatchingParenthesis($tokens, $openParenthesisIndex);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You shouldn't have to do this, there is $token->getRelatedToken() to get the related parenthesis

if (false === $closeParenthesisIndex) {
return;
}

$fixer = $this->addFixableError(
'Use strict comparison operators === / !== instead of same as / not same as.',
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I prefer a specific message

'Use strict comparison operators === instead of same as.',

or

'Use strict comparison operators !== instead of not same as.',

based on the situation

$token
);

if (null === $fixer) {
return;
}

$fixer->beginChangeSet();

$replacement = $isNegated ? '!==' : '===';
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This one time var is useless


$fixer->replaceToken($tokenIndex, $replacement);

for ($i = $tokenIndex + 1; $i <= $asIndex; ++$i) {
$fixer->replaceToken($i, '');
}

// We want only one whitespace between the operator and the parenthesis content.
// If there is already a whitespace between "as" and "(", we can remove the "(".
// Otherwise, we replace "(" by a whitespace.
$hasWhitespaceBeforeParen = $tokens->get($openParenthesisIndex - 1)->isMatching(Token::WHITESPACE_TOKENS);
$fixer->replaceToken($openParenthesisIndex, $hasWhitespaceBeforeParen ? '' : ' ');
Comment on lines +72 to +76
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I wouldn't do any of this here and rely on other fixers.

Once same as(foo) is fixed into ===foo the whitespace fixer will change it into === foo.

$fixer->replaceToken($closeParenthesisIndex, '');

$fixer->endChangeSet();
}

private function findMatchingParenthesis(Tokens $tokens, int $openParenthesisIndex): int|false
{
$level = 0;
$tokensArray = $tokens->toArray();
$count = \count($tokensArray);

for ($i = $openParenthesisIndex; $i < $count; ++$i) {
$token = $tokensArray[$i];

if ($token->isMatching(Token::PUNCTUATION_TYPE, '(')) {
++$level;

continue;
}

if ($token->isMatching(Token::PUNCTUATION_TYPE, ')')) {
--$level;

if (0 === $level) {
return $i;
}
}
}

return false;
}
}
2 changes: 2 additions & 0 deletions src/Standard/TwigCsFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use TwigCsFixer\Rules\Literal\CompactHashRule;
use TwigCsFixer\Rules\Literal\HashQuoteRule;
use TwigCsFixer\Rules\Literal\SingleQuoteRule;
use TwigCsFixer\Rules\Operator\StrictComparisonOperatorRule;
use TwigCsFixer\Rules\Punctuation\TrailingCommaMultiLineRule;
use TwigCsFixer\Rules\Punctuation\TrailingCommaSingleLineRule;
use TwigCsFixer\Rules\Whitespace\BlankEOFRule;
Expand All @@ -33,6 +34,7 @@ public function getRules(): array
new IncludeFunctionRule(),
new IndentRule(),
new SingleQuoteRule(),
new StrictComparisonOperatorRule(),
new TrailingCommaMultiLineRule(),
new TrailingCommaSingleLineRule(),
new TrailingSpaceRule(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{{ a === b }}
{{ a !== b }}
{{ a === b }}
{{ a !== b }}
{{ a === b }}
{{ a !== b }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

declare(strict_types=1);

namespace TwigCsFixer\Tests\Rules\Operator\StrictComparisonOperator;

use TwigCsFixer\Rules\Operator\StrictComparisonOperatorRule;
use TwigCsFixer\Test\AbstractRuleTestCase;

final class StrictComparisonOperatorRuleTest extends AbstractRuleTestCase
{
public function testRule(): void
{
$this->checkRule(new StrictComparisonOperatorRule(), [
'StrictComparisonOperator.Error:1:6' => 'Use strict comparison operators === / !== instead of same as / not same as.',
'StrictComparisonOperator.Error:2:6' => 'Use strict comparison operators === / !== instead of same as / not same as.',
'StrictComparisonOperator.Error:3:6' => 'Use strict comparison operators === / !== instead of same as / not same as.',
'StrictComparisonOperator.Error:4:6' => 'Use strict comparison operators === / !== instead of same as / not same as.',
]);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{{ a is same as(b) }}
{{ a is not same as(b) }}
{{ a is same as (b) }}
{{ a is not same as (b) }}
{{ a === b }}
{{ a !== b }}
2 changes: 2 additions & 0 deletions tests/Standard/TwigCsFixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use TwigCsFixer\Rules\Literal\SingleQuoteRule;
use TwigCsFixer\Rules\Operator\OperatorNameSpacingRule;
use TwigCsFixer\Rules\Operator\OperatorSpacingRule;
use TwigCsFixer\Rules\Operator\StrictComparisonOperatorRule;
use TwigCsFixer\Rules\Punctuation\PunctuationSpacingRule;
use TwigCsFixer\Rules\Punctuation\TrailingCommaMultiLineRule;
use TwigCsFixer\Rules\Punctuation\TrailingCommaSingleLineRule;
Expand Down Expand Up @@ -51,6 +52,7 @@ public function testGetRules(): void
new IncludeFunctionRule(),
new IndentRule(),
new SingleQuoteRule(),
new StrictComparisonOperatorRule(),
new TrailingCommaMultiLineRule(),
new TrailingCommaSingleLineRule(),
new TrailingSpaceRule(),
Expand Down
Loading