Skip to content

Commit

Permalink
ReferenceUsedNamesOnlySniff: functions and constants support is hopef…
Browse files Browse the repository at this point in the history
…ully complete
  • Loading branch information
kukulich committed Jan 7, 2018
1 parent e7ae6cb commit b742ecb
Show file tree
Hide file tree
Showing 21 changed files with 411 additions and 24 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,13 @@ new Foo\Bar();

* `namespacesRequiredToUse`: if not set, all namespaces are required to be used. When set, only mentioned namespaces are required to be used. Useful in tandem with UseOnlyWhitelistedNamespaces sniff.
* `allowFullyQualifiedNameForCollidingClasses`: allow fully qualified name for a class with a colliding use statement.
* `allowFullyQualifiedNameForCollidingFunctions`: allow fully qualified name for a function with a colliding use statement.
* `allowFullyQualifiedNameForCollidingConstants`: allow fully qualified name for a constant with a colliding use statement.
* `allowFullyQualifiedGlobalClasses`: allows using fully qualified classes from global space (i.e. `\DateTimeImmutable`).
* `allowFullyQualifiedGlobalFunctions`: allows using fully qualified functions from global space (i.e. `\phpversion()`).
* `allowFullyQualifiedGlobalConstants`: allows using fully qualified constants from global space (i.e. `\PHP_VERSION`).
* `allowFallbackGlobalFunctions`: allows using global functions via fallback name without `use` (i.e. `phpversion()`).
* `allowFallbackGlobalConstants`: allows using global constants via fallback name without `use` (i.e. `PHP_VERSION`).

#### SlevomatCodingStandard.Namespaces.UseOnlyWhitelistedNamespaces

Expand Down
60 changes: 60 additions & 0 deletions SlevomatCodingStandard/Helpers/ConstantHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php declare(strict_types = 1);

namespace SlevomatCodingStandard\Helpers;

class ConstantHelper
{

public static function getName(\PHP_CodeSniffer\Files\File $codeSnifferFile, int $constantPointer): string
{
$tokens = $codeSnifferFile->getTokens();
return $tokens[TokenHelper::findNext($codeSnifferFile, T_STRING, $constantPointer + 1)]['content'];
}

public static function getFullyQualifiedName(\PHP_CodeSniffer\Files\File $codeSnifferFile, int $constantPointer): string
{
$name = self::getName($codeSnifferFile, $constantPointer);
$namespace = NamespaceHelper::findCurrentNamespaceName($codeSnifferFile, $constantPointer);

return $namespace !== null ? sprintf('%s%s%s%s', NamespaceHelper::NAMESPACE_SEPARATOR, $namespace, NamespaceHelper::NAMESPACE_SEPARATOR, $name) : $name;
}

/**
* @param \PHP_CodeSniffer\Files\File $codeSnifferFile
* @return string[]
*/
public static function getAllNames(\PHP_CodeSniffer\Files\File $codeSnifferFile): array
{
$previousConstantPointer = 0;

return array_map(
function (int $constantPointer) use ($codeSnifferFile): string {
return self::getName($codeSnifferFile, $constantPointer);
},
array_filter(
iterator_to_array(self::getAllConstantPointers($codeSnifferFile, $previousConstantPointer)),
function (int $constantPointer) use ($codeSnifferFile): bool {
foreach (array_reverse($codeSnifferFile->getTokens()[$constantPointer]['conditions']) as $conditionTokenCode) {
if (in_array($conditionTokenCode, [T_CLASS, T_INTERFACE, T_TRAIT, T_ANON_CLASS], true)) {
return false;
}
}

return true;
}
)
);
}

private static function getAllConstantPointers(\PHP_CodeSniffer\Files\File $codeSnifferFile, int &$previousConstantPointer): \Generator
{
do {
$nextConstantPointer = TokenHelper::findNext($codeSnifferFile, T_CONST, $previousConstantPointer + 1);
if ($nextConstantPointer !== null) {
$previousConstantPointer = $nextConstantPointer;
yield $nextConstantPointer;
}
} while ($nextConstantPointer !== null);
}

}
32 changes: 32 additions & 0 deletions SlevomatCodingStandard/Helpers/FunctionHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,36 @@ public static function findReturnAnnotation(\PHP_CodeSniffer\Files\File $codeSni
return $returnAnnotations[0];
}

/**
* @param \PHP_CodeSniffer\Files\File $codeSnifferFile
* @return string[]
*/
public static function getAllFunctionNames(\PHP_CodeSniffer\Files\File $codeSnifferFile): array
{
$previousFunctionPointer = 0;

return array_map(
function (int $functionPointer) use ($codeSnifferFile): string {
return self::getName($codeSnifferFile, $functionPointer);
},
array_filter(
iterator_to_array(self::getAllFunctionOrMethodPointers($codeSnifferFile, $previousFunctionPointer)),
function (int $functionOrMethodPointer) use ($codeSnifferFile): bool {
return !self::isMethod($codeSnifferFile, $functionOrMethodPointer);
}
)
);
}

private static function getAllFunctionOrMethodPointers(\PHP_CodeSniffer\Files\File $codeSnifferFile, int &$previousFunctionPointer): \Generator
{
do {
$nextFunctionPointer = TokenHelper::findNext($codeSnifferFile, T_FUNCTION, $previousFunctionPointer + 1);
if ($nextFunctionPointer !== null) {
$previousFunctionPointer = $nextFunctionPointer;
yield $nextFunctionPointer;
}
} while ($nextFunctionPointer !== null);
}

}
5 changes: 5 additions & 0 deletions SlevomatCodingStandard/Helpers/ReferencedName.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public function getEndPointer(): int
return $this->endPointer;
}

public function isClass(): bool
{
return $this->type === self::TYPE_DEFAULT;
}

public function isConstant(): bool
{
return $this->type === self::TYPE_CONSTANT;
Expand Down
128 changes: 106 additions & 22 deletions SlevomatCodingStandard/Sniffs/Namespaces/ReferenceUsedNamesOnlySniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace SlevomatCodingStandard\Sniffs\Namespaces;

use SlevomatCodingStandard\Helpers\ClassHelper;
use SlevomatCodingStandard\Helpers\ConstantHelper;
use SlevomatCodingStandard\Helpers\FunctionHelper;
use SlevomatCodingStandard\Helpers\NamespaceHelper;
use SlevomatCodingStandard\Helpers\ReferencedName;
use SlevomatCodingStandard\Helpers\ReferencedNameHelper;
Expand All @@ -19,6 +21,8 @@ class ReferenceUsedNamesOnlySniff implements \PHP_CodeSniffer\Sniffs\Sniff

public const CODE_REFERENCE_VIA_FULLY_QUALIFIED_NAME_WITHOUT_NAMESPACE = 'ReferenceViaFullyQualifiedNameWithoutNamespace';

public const CODE_REFERENCE_VIA_FALLBACK_GLOBAL_NAME = 'ReferenceViaFallbackGlobalName';

public const CODE_PARTIAL_USE = 'PartialUse';

/** @var string[] */
Expand All @@ -36,9 +40,15 @@ class ReferenceUsedNamesOnlySniff implements \PHP_CodeSniffer\Sniffs\Sniff
/** @var bool */
public $allowFullyQualifiedGlobalFunctions = false;

/** @var bool */
public $allowFallbackGlobalFunctions = true;

/** @var bool */
public $allowFullyQualifiedGlobalConstants = false;

/** @var bool */
public $allowFallbackGlobalConstants = true;

/** @var string[] */
public $specialExceptionNames = [];

Expand Down Expand Up @@ -67,6 +77,12 @@ class ReferenceUsedNamesOnlySniff implements \PHP_CodeSniffer\Sniffs\Sniff
/** @var bool */
public $allowFullyQualifiedNameForCollidingClasses = false;

/** @var bool */
public $allowFullyQualifiedNameForCollidingFunctions = false;

/** @var bool */
public $allowFullyQualifiedNameForCollidingConstants = false;

/**
* @return mixed[]
*/
Expand Down Expand Up @@ -140,17 +156,51 @@ public function process(\PHP_CodeSniffer\Files\File $phpcsFile, $openTagPointer)
$tokens = $phpcsFile->getTokens();

$referencedNames = ReferencedNameHelper::getAllReferencedNames($phpcsFile, $openTagPointer);
$useStatements = UseStatementHelper::getUseStatements($phpcsFile, $openTagPointer);

$definedClassesIndex = array_flip(array_map(function (string $className): string {
return strtolower($className);
}, ClassHelper::getAllNames($phpcsFile)));
$definedFunctionsIndex = array_flip(array_map(function (string $functionName): string {
return strtolower($functionName);
}, FunctionHelper::getAllFunctionNames($phpcsFile)));
$definedConstantsIndex = array_flip(ConstantHelper::getAllNames($phpcsFile));

if ($this->allowFullyQualifiedNameForCollidingClasses) {
$referencesIndex = array_flip(
$classReferencesIndex = array_flip(
array_map(
function (ReferencedName $referencedName): string {
return strtolower($referencedName->getNameAsReferencedInFile());
},
array_filter($referencedNames, function (ReferencedName $referencedName): bool {
return $referencedName->isClass();
})
)
);
}

if ($this->allowFullyQualifiedNameForCollidingFunctions) {
$functionReferencesIndex = array_flip(
array_map(
function (ReferencedName $referencedName): string {
return strtolower($referencedName->getNameAsReferencedInFile());
},
$referencedNames
array_filter($referencedNames, function (ReferencedName $referencedName): bool {
return $referencedName->isFunction();
})
)
);
}

if ($this->allowFullyQualifiedNameForCollidingConstants) {
$constantReferencesIndex = array_flip(
array_map(
function (ReferencedName $referencedName): string {
return $referencedName->getNameAsReferencedInFile();
},
array_filter($referencedNames, function (ReferencedName $referencedName): bool {
return $referencedName->isConstant();
})
)
);
}
Expand All @@ -159,16 +209,33 @@ function (ReferencedName $referencedName): string {
$name = $referencedName->getNameAsReferencedInFile();
$nameStartPointer = $referencedName->getStartPointer();
$canonicalName = NamespaceHelper::normalizeToCanonicalName($name);

if ($this->allowFullyQualifiedNameForCollidingClasses) {
$unqualifiedClassName = strtolower(NamespaceHelper::getUnqualifiedNameFromFullyQualifiedName($name));
if (isset($referencesIndex[$unqualifiedClassName]) || array_key_exists($unqualifiedClassName, $definedClassesIndex ?? [])) {
$unqualifiedName = NamespaceHelper::getUnqualifiedNameFromFullyQualifiedName($name);

$isFullyQualified = NamespaceHelper::isFullyQualifiedName($name);
$isGlobalFallback = !$isFullyQualified
&& !NamespaceHelper::hasNamespace($name)
&& !array_key_exists(UseStatement::getUniqueId($referencedName->getType(), $name), $useStatements);
$isGlobalFunctionFallback = $referencedName->isFunction() && $isGlobalFallback;
$isGlobalConstantFallback = $referencedName->isConstant() && $isGlobalFallback;

if ($referencedName->isClass() && $this->allowFullyQualifiedNameForCollidingClasses) {
$lowerCasedUnqualifiedClassName = strtolower($unqualifiedName);
if (isset($classReferencesIndex[$lowerCasedUnqualifiedClassName]) || array_key_exists($lowerCasedUnqualifiedClassName, $definedClassesIndex)) {
continue;
}
} elseif ($referencedName->isFunction() && $this->allowFullyQualifiedNameForCollidingFunctions) {
$lowerCasedUnqualifiedFunctionName = strtolower($unqualifiedName);
if (isset($functionReferencesIndex[$lowerCasedUnqualifiedFunctionName]) || array_key_exists($lowerCasedUnqualifiedFunctionName, $definedFunctionsIndex)) {
continue;
}
} elseif ($referencedName->isConstant() && $this->allowFullyQualifiedNameForCollidingConstants) {
if (isset($constantReferencesIndex[$unqualifiedName]) || array_key_exists($unqualifiedName, $definedConstantsIndex)) {
continue;
}
}

if (NamespaceHelper::isFullyQualifiedName($name)) {
if (!$this->isClassRequiredToBeUsed($name)) {
if ($isFullyQualified || $isGlobalFunctionFallback || $isGlobalConstantFallback) {
if ($isFullyQualified && !$this->isRequiredToBeUsed($name)) {
continue;
}

Expand All @@ -185,12 +252,15 @@ function (ReferencedName $referencedName): string {
$previousKeywordPointer = TokenHelper::findPreviousExcluding($phpcsFile, array_merge(TokenHelper::$nameTokenCodes, [T_WHITESPACE, T_COMMA]), $nameStartPointer - 1);
if (!in_array($tokens[$previousKeywordPointer]['code'], $this->getFullyQualifiedKeywords(), true)) {
if (
!NamespaceHelper::hasNamespace($name)
$isFullyQualified
&& !NamespaceHelper::hasNamespace($name)
&& NamespaceHelper::findCurrentNamespaceName($phpcsFile, $nameStartPointer) === null
) {
$label = sprintf($referencedName->isConstant() ? 'Constant %s' : ($referencedName->isFunction() ? 'Function %s()' : 'Class %s'), $name);

$fix = $phpcsFile->addFixableError(sprintf(
'Type %s should not be referenced via a fully qualified name, but via an unqualified name without the leading \\, because the file does not have a namespace and the type cannot be put in a use statement.',
$name
'%s should not be referenced via a fully qualified name, but via an unqualified name without the leading \\, because the file does not have a namespace and the type cannot be put in a use statement.',
$label
), $nameStartPointer, self::CODE_REFERENCE_VIA_FULLY_QUALIFIED_NAME_WITHOUT_NAMESPACE);
if ($fix) {
$phpcsFile->fixer->beginChangeset();
Expand All @@ -201,9 +271,9 @@ function (ReferencedName $referencedName): string {
$shouldBeUsed = NamespaceHelper::hasNamespace($name);
if (!$shouldBeUsed) {
if ($referencedName->isFunction()) {
$shouldBeUsed = !$this->allowFullyQualifiedGlobalFunctions;
$shouldBeUsed = $isFullyQualified ? !$this->allowFullyQualifiedGlobalFunctions : !$this->allowFallbackGlobalFunctions;
} elseif ($referencedName->isConstant()) {
$shouldBeUsed = !$this->allowFullyQualifiedGlobalConstants;
$shouldBeUsed = $isFullyQualified ? !$this->allowFullyQualifiedGlobalConstants : !$this->allowFallbackGlobalConstants;
} else {
$shouldBeUsed = !$this->allowFullyQualifiedGlobalClasses;
}
Expand All @@ -213,27 +283,41 @@ function (ReferencedName $referencedName): string {
continue;
}

$useStatements = UseStatementHelper::getUseStatements($phpcsFile, $openTagPointer);
$nameToReference = NamespaceHelper::getUnqualifiedNameFromFullyQualifiedName($name);
$canonicalNameToReference = strtolower($nameToReference);
$canonicalNameToReference = $referencedName->isConstant() ? $nameToReference : strtolower($nameToReference);

$canBeFixed = true;
foreach ($useStatements as $useStatement) {
if ($useStatement->getType() !== $referencedName->getType()) {
continue;
}

if ($useStatement->getFullyQualifiedTypeName() === $canonicalName) {
continue;
}

if (
$useStatement->getType() === $referencedName->getType()
&& $useStatement->getFullyQualifiedTypeName() !== $canonicalName
&& ($useStatement->getCanonicalNameAsReferencedInFile() === $canonicalNameToReference || array_key_exists($canonicalNameToReference, $definedClassesIndex))
$useStatement->getCanonicalNameAsReferencedInFile() === $canonicalNameToReference
|| ($referencedName->isClass() && array_key_exists($canonicalNameToReference, $definedClassesIndex))
|| ($referencedName->isFunction() && array_key_exists($canonicalNameToReference, $definedFunctionsIndex))
|| ($referencedName->isConstant() && array_key_exists($canonicalNameToReference, $definedConstantsIndex))
) {
$canBeFixed = false;
break;
}
}

$errorMessage = sprintf('Type %s should not be referenced via a fully qualified name, but via a use statement.', $name);
$label = sprintf($referencedName->isConstant() ? 'Constant %s' : ($referencedName->isFunction() ? 'Function %s()' : 'Class %s'), $name);
$errorCode = $isGlobalConstantFallback || $isGlobalFunctionFallback
? self::CODE_REFERENCE_VIA_FALLBACK_GLOBAL_NAME
: self::CODE_REFERENCE_VIA_FULLY_QUALIFIED_NAME;
$errorMessage = $isGlobalConstantFallback || $isGlobalFunctionFallback
? sprintf('%s should not be referenced via a fallback global name, but via a use statement.', $label)
: sprintf('%s should not be referenced via a fully qualified name, but via a use statement.', $label);
if ($canBeFixed) {
$fix = $phpcsFile->addFixableError($errorMessage, $nameStartPointer, self::CODE_REFERENCE_VIA_FULLY_QUALIFIED_NAME);
$fix = $phpcsFile->addFixableError($errorMessage, $nameStartPointer, $errorCode);
} else {
$phpcsFile->addError($errorMessage, $nameStartPointer, self::CODE_REFERENCE_VIA_FULLY_QUALIFIED_NAME);
$phpcsFile->addError($errorMessage, $nameStartPointer, $errorCode);
$fix = false;
}

Expand Down Expand Up @@ -286,7 +370,7 @@ function (ReferencedName $referencedName): string {
}
}

private function isClassRequiredToBeUsed(string $name): bool
private function isRequiredToBeUsed(string $name): bool
{
if (count($this->namespacesRequiredToUse) === 0) {
return true;
Expand Down
2 changes: 2 additions & 0 deletions build/phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
<property name="namespacesRequiredToUse" type="array" value="SlevomatCodingStandard"/>
<property name="fullyQualifiedKeywords" type="array" value="T_EXTENDS,T_IMPLEMENTS"/>
<property name="allowFullyQualifiedExceptions" value="true"/>
<property name="allowFullyQualifiedGlobalFunctions" value="true"/>
<property name="allowFullyQualifiedGlobalConstants" value="true"/>
<property name="allowPartialUses" value="false"/>
</properties>
</rule>
Expand Down
32 changes: 32 additions & 0 deletions tests/Helpers/ConstantHelperTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php declare(strict_types = 1);

namespace SlevomatCodingStandard\Helpers;

class ConstantHelperTest extends \SlevomatCodingStandard\Helpers\TestCase
{

public function testNameWithNamespace(): void
{
$codeSnifferFile = $this->getCodeSnifferFile(__DIR__ . '/data/constantWithNamespace.php');

$constantPointer = $this->findConstantPointerByName($codeSnifferFile, 'FOO');
$this->assertSame('\FooNamespace\FOO', ConstantHelper::getFullyQualifiedName($codeSnifferFile, $constantPointer));
$this->assertSame('FOO', ConstantHelper::getName($codeSnifferFile, $constantPointer));
}

public function testNameWithoutNamespace(): void
{
$codeSnifferFile = $this->getCodeSnifferFile(__DIR__ . '/data/constantWithoutNamespace.php');

$constantPointer = $this->findConstantPointerByName($codeSnifferFile, 'FOO');
$this->assertSame('FOO', ConstantHelper::getFullyQualifiedName($codeSnifferFile, $constantPointer));
$this->assertSame('FOO', ConstantHelper::getName($codeSnifferFile, $constantPointer));
}

public function testGetAllNames(): void
{
$codeSnifferFile = $this->getCodeSnifferFile(__DIR__ . '/data/constantNames.php');
$this->assertSame(['FOO', 'BOO'], ConstantHelper::getAllNames($codeSnifferFile));
}

}
Loading

5 comments on commit b742ecb

@Majkl578
Copy link
Contributor

Choose a reason for hiding this comment

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

allowFullyQualifiedGlobalFunctions=true doesn't seem to play well with allowFallbackGlobalFunctions=false + allowFullyQualifiedNameForCollidingFunctions=true - doesn't report any errors for fallback references (no \ and use). With allowFullyQualifiedGlobalFunctions=false it works.

@Majkl578
Copy link
Contributor

Choose a reason for hiding this comment

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

Also found one bug: it added a method name into uses: Majkl578/doctrine-orm@2b69bd9#diff-6e8c1c1e78b054ba05e20ea09d877865R45, this method returns reference if that could be the source.

@kukulich
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also found one bug: it added a method name into uses: Majkl578/doctrine-orm@2b69bd9#diff-6e8c1c1e78b054ba05e20ea09d877865R45, this method returns reference if that could be the source.

Fixed in 5309879
It's possible there will be some more mistakes like this.

@kukulich
Copy link
Contributor Author

Choose a reason for hiding this comment

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

allowFullyQualifiedGlobalFunctions=true doesn't seem to play well with allowFallbackGlobalFunctions=false + allowFullyQualifiedNameForCollidingFunctions=true - doesn't report any errors for fallback references (no \ and use). With allowFullyQualifiedGlobalFunctions=false it works.

I fixed bug via 52f3c79 but it was caused by allowFullyQualifiedNameForCollidingFunctions=true not allowFullyQualifiedGlobalFunctions=true. So if there is still a bug caused by allowFullyQualifiedGlobalFunctions=true could you please send me the file?

@Majkl578
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks ok now. 👍

Please sign in to comment.