Skip to content
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
135 changes: 129 additions & 6 deletions MO4/Sniffs/Formatting/AlphabeticalUseStatementsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,55 @@
/**
* Alphabetical Use Statements sniff.
*
* Use statements must be in alphabetical order, grouped by empty lines
* Use statements must be in alphabetical order, grouped by empty lines.
*
* @category PHP
* @package PHP_CodeSniffer-MO4
* @author Xaver Loppenstedt <xaver@loppenstedt.de>
* @author Steffen Ritter <steffenritter1@gmail.com>
* @author Christian Albrecht <christian.albrecht@mayflower.de>
* @copyright 2013-2014 Xaver Loppenstedt, some rights reserved.
* @copyright 2013-2017 Xaver Loppenstedt, some rights reserved.
* @license http://spdx.org/licenses/MIT MIT License
* @link https://github.com/Mayflower/mo4-coding-standard
*/

namespace MO4\Sniffs\Formatting;

use PHP_CodeSniffer\Exceptions\RuntimeException;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Standards\PSR2\Sniffs\Namespaces\UseDeclarationSniff;
use PHP_CodeSniffer\Util\Common;
use PHP_CodeSniffer\Util\Tokens as PHP_CodeSniffer_Tokens;

class AlphabeticalUseStatementsSniff extends UseDeclarationSniff
{

const NAMESPACE_SEPRATOR_STRING = '\\';

Choose a reason for hiding this comment

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

typo: "SEPRATOR" ->"SEPARATOR"


/**
* Sorting order, can be one of:
* 'dictionary', 'string', 'string-locale' or 'string-case-insensitive'
*
* Unknown types will be mapped to 'string'.
*
* @var string
*/
public $order = 'dictionary';


/**
* Supported ordering methods
*
* @var array
*/
private $supportedOrderingMethods = [
'dictionary',
'string',
'string',
'string-locale',
'string-case-insensitive',
];

/**
* Last import seen in group
*
Expand All @@ -58,6 +87,30 @@ class AlphabeticalUseStatementsSniff extends UseDeclarationSniff
private $currentFile = null;


/**
* Returns an array of tokens this test wants to listen for.
*
* @return array
* @throws \PHP_CodeSniffer\Exceptions\RuntimeException
*/
public function register()
{
if (in_array($this->order, $this->supportedOrderingMethods) === false) {
$error = sprintf(
"'%s' is not a valid order function for %s! Pick one of: %s",
$this->order,
Common::getSniffCode(__CLASS__),
implode(', ', $this->supportedOrderingMethods)
);

throw new RuntimeException($error);
}

return parent::register();

}//end register()


/**
* Processes this test, when one of its tokens is encountered.
*
Expand Down Expand Up @@ -99,11 +152,11 @@ public function process(File $phpcsFile, $stackPtr)

$fixable = false;
if ($this->lastImport !== ''
&& strcmp($this->lastImport, $currentImport) > 0
&& $this->compareString($this->lastImport, $currentImport) > 0
) {
$msg = 'USE statements must be sorted alphabetically';
$msg = 'USE statements must be sorted alphabetically, order %s';
$code = 'MustBeSortedAlphabetically';
$fixable = $phpcsFile->addFixableError($msg, $currentPtr, $code);
$fixable = $phpcsFile->addFixableError($msg, $currentPtr, $code, [$this->order]);
}

if (true === $fixable) {
Expand Down Expand Up @@ -280,12 +333,82 @@ private function findNewDestination(
$prevLine = $tokens[$prevPtr]['line'];
$prevImportArr = $this->getUseImport($phpcsFile, $prevPtr);
} while ($prevLine === ($line - 1)
&& (strcmp($prevImportArr['content'], $import) > 0)
&& ($this->compareString($prevImportArr['content'], $import) > 0)
);

return $ptr;

}//end findNewDestination()


/**
* Compare namespace strings according defined order function.
*
* @param string $a first namespace string
* @param string $b second namespace string
*
* @return int
*/
private function compareString($a, $b)
{
if ('dictionary' === $this->order) {
return $this->dictionaryCompare($a, $b);
} else if ('string' === $this->order) {
return strcmp($a, $b);
} else if ('string-locale' === $this->order) {
return strcoll($a, $b);
} else if ('string-case-insensitive' === $this->order) {
return strcasecmp($a, $b);
} else {
return $this->dictionaryCompare($a, $b);
Copy link
Contributor

Choose a reason for hiding this comment

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

This method falls back to dictionary as validation approach by default. I've seen that PHPCS loves to mutate public properties using an XML, however I wonder if some kind of validation (e.g. throw an exception in here if order has an invalid value) makes sense to avoid unexpected behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the check to register()

}

}//end compareString()


/**
* Lexicographical namespace string compare.
*
* Example:
*
* use Doctrine\ORM\Query;
* use Doctrine\ORM\Query\Expr;
* use Doctrine\ORM\QueryBuilder;
*
* @param string $a first namespace string
* @param string $b second namespace string
*
* @return int
*/
private function dictionaryCompare($a, $b)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aoldemeier could you check the correctness of this function?

Copy link

@aoldemeier aoldemeier Nov 23, 2017

Choose a reason for hiding this comment

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

After having realized I was confused about the signature of substr I am now convinced that the function is correct (making common sense assumptions about how dictionaryCompare should work)...

{
$min = min(strlen($a), strlen($b));

for ($i = 0; $i < $min; $i++) {
if ($a[$i] === $b[$i]) {
continue;
}

if ($a[$i] === self::NAMESPACE_SEPRATOR_STRING) {
return -1;
}

if ($b[$i] === self::NAMESPACE_SEPRATOR_STRING) {
return 1;
}

if ($a[$i] < $b[$i]) {
return -1;
}

if ($a[$i] > $b[$i]) {
return 1;
}
}//end for

return strcmp(substr($a, $min), substr($b, $min));

}//end dictionaryCompare()


}//end class
26 changes: 26 additions & 0 deletions MO4/Tests/Formatting/AlphabeticalUseStatementsUnitTest.fail.4.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

use A1\B\C;
use A1\B;
use A1\BD;

use A2\BD;
use A2\B;
use A2\B\C;

use A3\B;
use A3\BD;
use A3\B\C;

use A4\B\C;
use A4\BD;
use A4\B;

use A5\BD;
use A5\B\C;
use A5\B;

class Foo
{

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

use A1\B;
use A1\B\C;
use A1\BD;

use A2\B;
use A2\B\C;
use A2\BD;

use A3\B;
use A3\B\C;
use A3\BD;

use A4\B;
use A4\B\C;
use A4\BD;

use A5\B;
use A5\B\C;
use A5\BD;

class Foo
{

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ use Z;

use X;

use A\B;
use A\B\C;
use A\BD;

class Foo {
$bar = 'foo';

Expand All @@ -25,4 +29,4 @@ class Foo {

// test run away tokens
use () {};
}
}
9 changes: 9 additions & 0 deletions MO4/Tests/Formatting/AlphabeticalUseStatementsUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,15 @@ protected function getErrorList($testFile='')
10 => 1,
15 => 1,
);
case 'AlphabeticalUseStatementsUnitTest.fail.4.inc':
return array(
4 => 1,
8 => 1,
13 => 1,
17 => 1,
20 => 1,
21 => 1,
);
}//end switch

return null;
Expand Down
43 changes: 41 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,52 @@ The MO4 Coding Standard is an extension of the [Symfony Coding Standard](http://
* in associative arrays, the `=>` operators must be aligned
* in arrays, the key and `=>` operator must be on the same line
* each consecutive variable assignment must align at the assignment operator
* use statements must be sorted lexicographically
* use statements must be sorted lexicographically, grouped by empty lines. The order function can be configured.
* you should use the imported class name when it was imported with a use statement
* interpolated variables in double quoted strings must be surrounded by `{ }`, e.g. `{$VAR}` instead of `$VAR`
* `sprintf` or `"{$VAR1} {$VAR2}"` must be used instead of the dot operator; concat operators are only allowed to concatenate constants and multi line strings,
* `sprintf` or `"{$VAR1} {$VAR2}"` must be used instead of the dot operator; concat operators are only allowed to concatenate constants and multi line strings
* a whitespace is required after each typecast, e.g. `(int) $value` instead of `(int)$value`
* doc blocks of class properties must be multiline and have exactly one `@var` annotation

## Configuration

### MO4.Formatting.AlphabeticalUseStatements

The `order` property of the `MO4.Formatting.AlphabeticalUseStatements` sniff defines
which function is used for ordering.

Possible values for order:
* `dictionary` (default): based on [strcmp](http://php.net/strcmp), the namespace separator
precedes any other character
```php
use Doctrine\ORM\Query;
use Doctrine\ORM\Query\Expr;
use Doctrine\ORM\QueryBuilder;
```
* `string`: binary safe string comparison using [strcmp](http://php.net/strcmp)
```php
use Doctrine\ORM\Query;
use Doctrine\ORM\QueryBuilder;
use Doctrine\ORM\Query\Expr;

use ExampleSub;
use Examples;
```
* `string-locale`: locale based string comparison using [strcoll](http://php.net/strcoll)
* `string-case-insenstive`: binary safe case-insensitive string comparison [strcasecmp](http://php.net/strcasecmp)
```php
use Examples;
use ExampleSub;
```

```
<rule ref="MO4.Formatting.AlphabeticalUseStatements">
<properties>
<property name="order" value="string-locale"/>
</properties>
</rule>
```

## Installation

### Composer
Expand Down