Skip to content
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

feat: Introduce NumericLiteralSeparatorFixer #6761

Merged
merged 22 commits into from
Jan 10, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 86 additions & 0 deletions doc/rules/basic/numeric_literal_separator.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
==================================
Rule ``numeric_literal_separator``
Wirone marked this conversation as resolved.
Show resolved Hide resolved
==================================

Adds separators to numeric literals of any kind.

Configuration
-------------

``override_existing``
Wirone marked this conversation as resolved.
Show resolved Hide resolved
~~~~~~~~~~~~~~~~~~~~~

Whether literals already containing underscores should be reformatted.

Allowed types: ``bool``

Default value: ``false``

``strategy``
~~~~~~~~~~~~

Whether numeric literal should be separated by underscores or not.

Allowed values: ``'no_separator'`` and ``'use_separator'``

Default value: ``'no_separator'``
Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure of the default.

Copy link
Member

Choose a reason for hiding this comment

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

It was suggested by me here and agreed by @kubawerlos so I implemented it like this. All the years I've been programming I have never worked with codebase that used literal separators, that's why I believe it's sane default.

Copy link
Member

@keradus keradus Jan 10, 2024

Choose a reason for hiding this comment

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

well, it was not possible for long in PHP, no doubts there is not adoption.

I aim to use everywhere personally, basically a must have for me for TS or python.
if both of you think this set of defaults is good, let's keep the defaults as-is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Must personally agree with @keradus, but I am OK with it as long as I have the option to set it for myself.


Examples
--------

Example #1
~~~~~~~~~~

*Default* configuration.

.. code-block:: diff

--- Original
+++ New
<?php
-$integer = 1234_5678;
-$octal = 01_234_56;
-$binary = 0b00_10_01_00;
-$hexadecimal = 0x3D45_8F4F;
+$integer = 12345678;
+$octal = 0123456;
+$binary = 0b00100100;
+$hexadecimal = 0x3D458F4F;

Example #2
~~~~~~~~~~

With configuration: ``['strategy' => 'use_separator']``.

.. code-block:: diff

--- Original
+++ New
<?php
-$integer = 12345678;
-$octal = 0123456;
-$binary = 0b0010010011011010;
-$hexadecimal = 0x3D458F4F;
+$integer = 12_345_678;
+$octal = 0123_456;
+$binary = 0b00100100_11011010;
+$hexadecimal = 0x3D_45_8F_4F;

Example #3
~~~~~~~~~~

With configuration: ``['override_existing' => true]``.

.. code-block:: diff

--- Original
+++ New
-<?php $var = 24_40_21;
+<?php $var = 244021;
References
----------

- Fixer class: `PhpCsFixer\\Fixer\\Basic\\NumericLiteralSeparatorFixer <./../../../src/Fixer/Basic/NumericLiteralSeparatorFixer.php>`_
- Test class: `PhpCsFixer\\Tests\\Fixer\\Basic\\NumericLiteralSeparatorFixerTest <./../../../tests/Fixer/Basic/NumericLiteralSeparatorFixerTest.php>`_

The test class defines officially supported behaviour. Each test case is a part of our backward compatibility promise.
3 changes: 3 additions & 0 deletions doc/rules/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ Basic
- `non_printable_character <./basic/non_printable_character.rst>`_ *(risky)*

Remove Zero-width space (ZWSP), Non-breaking space (NBSP) and other invisible unicode symbols.
- `numeric_literal_separator <./basic/numeric_literal_separator.rst>`_

Adds separators to numeric literals of any kind.
- `octal_notation <./basic/octal_notation.rst>`_

Literal octal must be in ``0o`` notation.
Expand Down
207 changes: 207 additions & 0 deletions src/Fixer/Basic/NumericLiteralSeparatorFixer.php
Wirone marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
<?php

declare(strict_types=1);

/*
* This file is part of PHP CS Fixer.
*
* (c) Fabien Potencier <fabien@symfony.com>
* Dariusz Rumiński <dariusz.ruminski@gmail.com>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/

namespace PhpCsFixer\Fixer\Basic;

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\Fixer\ConfigurableFixerInterface;
use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
use PhpCsFixer\FixerConfiguration\FixerConfigurationResolverInterface;
use PhpCsFixer\FixerConfiguration\FixerOptionBuilder;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
use PhpCsFixer\Preg;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;

/**
* Let's you add underscores to numeric literals.
*
* Inspired by:
* - {@link https://github.com/kubawerlos/php-cs-fixer-custom-fixers/blob/main/src/Fixer/NumericLiteralSeparatorFixer.php}
* - {@link https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/rules/numeric-separators-style.js}
*
* @author Marvin Heilemann <marvin.heilemann+github@googlemail.com>
* @author Greg Korba <greg@codito.dev>
*/
final class NumericLiteralSeparatorFixer extends AbstractFixer implements ConfigurableFixerInterface
{
public const STRATEGY_USE_SEPARATOR = 'use_separator';
public const STRATEGY_NO_SEPARATOR = 'no_separator';

public function getDefinition(): FixerDefinitionInterface
{
return new FixerDefinition(
'Adds separators to numeric literals of any kind.',
muuvmuuv marked this conversation as resolved.
Show resolved Hide resolved
[
new CodeSample(
<<<'PHP'
<?php
$integer = 1234_5678;
$octal = 01_234_56;
$binary = 0b00_10_01_00;
$hexadecimal = 0x3D45_8F4F;

PHP
),
new CodeSample(
<<<'PHP'
<?php
$integer = 12345678;
$octal = 0123456;
$binary = 0b0010010011011010;
$hexadecimal = 0x3D458F4F;

PHP
,
['strategy' => self::STRATEGY_USE_SEPARATOR],
),
new CodeSample(
"<?php \$var = 24_40_21;\n",
['override_existing' => true]
),
]
);
}

public function isCandidate(Tokens $tokens): bool
{
return $tokens->isAnyTokenKindsFound([T_DNUMBER, T_LNUMBER]);
}

protected function createConfigurationDefinition(): FixerConfigurationResolverInterface
{
return new FixerConfigurationResolver([
(new FixerOptionBuilder(
'override_existing',
'Whether literals already containing underscores should be reformatted.'
))
->setAllowedTypes(['bool'])
->setDefault(false)
->getOption(),
(new FixerOptionBuilder(
'strategy',
'Whether numeric literal should be separated by underscores or not.'
))
->setAllowedValues([self::STRATEGY_USE_SEPARATOR, self::STRATEGY_NO_SEPARATOR])
->setDefault(self::STRATEGY_NO_SEPARATOR)
->getOption(),
]);
}

protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
{
foreach ($tokens as $index => $token) {
if (!$token->isGivenKind([T_DNUMBER, T_LNUMBER])) {
continue;
}

$content = $token->getContent();

$newContent = $this->formatValue($content);

if ($content === $newContent) {
// Skip Token override if its the same content, like when it
// already got a valid literal separator structure.
continue;
}

$tokens[$index] = new Token([$token->getId(), $newContent]);
}
}

private function formatValue(string $value): string
muuvmuuv marked this conversation as resolved.
Show resolved Hide resolved
{
if (self::STRATEGY_NO_SEPARATOR === $this->configuration['strategy']) {
return str_contains($value, '_') ? str_replace('_', '', $value) : $value;
}

if (true === $this->configuration['override_existing']) {
$value = str_replace('_', '', $value);
} elseif (str_contains($value, '_')) {
// Keep already underscored literals untouched.
return $value;
}

$lowerValue = strtolower($value);

if (str_starts_with($lowerValue, '0b')) {
// Binary
return $this->insertEveryRight($value, 8, 2);
}

if (str_starts_with($lowerValue, '0x')) {
// Hexadecimal
return $this->insertEveryRight($value, 2, 2);
}

if (str_starts_with($lowerValue, '0o')) {
// Octal
return $this->insertEveryRight($value, 3, 2);
}
if (str_starts_with($lowerValue, '0')) {
// Octal prior PHP 8.1
return $this->insertEveryRight($value, 3, 1);
}

// All other types

/** If its a negative value we need an offset */
$negativeOffset = static fn ($v) => str_contains($v, '-') ? 1 : 0;

Preg::matchAll('/([0-9-_]+)((\.)([0-9_]+))?((e)([0-9-_]+))?/i', $value, $result);

$integer = $result[1][0];
$joinedValue = $this->insertEveryRight($integer, 3, $negativeOffset($integer));

$dot = $result[3][0];
if ('' !== $dot) {
$integer = $result[4][0];
$decimal = $this->insertEveryLeft($integer, 3, $negativeOffset($integer));
$joinedValue = $joinedValue.$dot.$decimal;
}

$tim = $result[6][0];
if ('' !== $tim) {
$integer = $result[7][0];
$times = $this->insertEveryRight($integer, 3, $negativeOffset($integer));
$joinedValue = $joinedValue.$tim.$times;
}

return $joinedValue;
}

private function insertEveryRight(string $value, int $length, int $offset = 0): string
{
$position = $length * -1;
while ($position > -(\strlen($value) - $offset)) {
$value = substr_replace($value, '_', $position, 0);
$position -= $length + 1;
}

return $value;
}

private function insertEveryLeft(string $value, int $length, int $offset = 0): string
{
$position = $length;
while ($position < \strlen($value)) {
$value = substr_replace($value, '_', $position, $offset);
$position += $length + 1;
}

return $value;
}
}