-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add Codesniffer to detect unused use-statements #1349
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
Closed
Closed
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e2acd05
Add code sniffer
nickvergessen e13b9b1
Do not allow unused use statements
nickvergessen 1a0e02c
Check some more rules
nickvergessen f5dd7d3
Add support for closures
nickvergessen a2b642f
Allow inline controls in templates
nickvergessen d568e16
Add the unused sniffer
nickvergessen 3826d8f
Fix single line comments
nickvergessen 30e349d
Skip breaking comments
nickvergessen 7714cb1
Exclude 3rdparty
nickvergessen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "require-dev": { | ||
| "squizlabs/php_codesniffer": "2.*" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| <?xml version="1.0"?> | ||
| <ruleset name="Nextcloud PHP Default Standard"> | ||
|
|
||
| <description>Nextcloud coding standard for PHP files</description> | ||
|
|
||
| <!-- There MUST NOT be unused use statements. --> | ||
| <rule ref="nextcloud-unused.xml" /> | ||
|
|
||
| <!-- "for (; bar; )" should be "while (bar)" instead --> | ||
| <rule ref="Generic.CodeAnalysis.ForLoopShouldBeWhileLoop" /> | ||
|
|
||
| <!-- A method MUST not only call its parent --> | ||
| <rule ref="Generic.CodeAnalysis.UselessOverridingMethod" /> | ||
|
|
||
| <!-- When present, all use declarations MUST go after the namespace declaration. | ||
| There MUST be one use keyword per declaration. | ||
| There MUST be one blank line after the use block. --> | ||
| <rule ref="PSR2.Namespaces.UseDeclaration" /> | ||
|
|
||
| <!-- Constructors MUST be called __construct() instead of after the class. --> | ||
| <rule ref="Generic.NamingConventions.ConstructorName" /> | ||
|
|
||
| <!-- Class constants MUST be declared in all upper case with underscore separators. --> | ||
| <rule ref="Generic.NamingConventions.UpperCaseConstantName" /> | ||
|
|
||
| <!-- Each file MUST end with exactly one newline character --> | ||
| <rule ref="PSR2.Files.EndFileNewline" /> | ||
|
|
||
| <!-- The eval() function MUST NOT be used. --> | ||
| <rule ref="Squiz.PHP.Eval" /> | ||
|
|
||
| <!-- There MUST NOT be trailing whitespace at the end of lines. --> | ||
| <rule ref="Squiz.WhiteSpace.SuperfluousWhitespace" /> | ||
|
|
||
| <!-- There MUST NOT be whitespace before the first content of a file --> | ||
| <rule ref="Squiz.WhiteSpace.SuperfluousWhitespace.StartFile" /> | ||
|
|
||
| <!-- There MUST NOT be whitespace after the last content of a file --> | ||
| <rule ref="Squiz.WhiteSpace.SuperfluousWhitespace.EndFile" /> | ||
|
|
||
| <!-- The ?> closing tag MUST be omitted from files containing only PHP. --> | ||
| <rule ref="Zend.Files.ClosingTag" /> | ||
| </ruleset> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <?xml version="1.0"?> | ||
| <ruleset name="Nextcloud PHP Strict Standard"> | ||
|
|
||
| <description>Nextcloud coding standard for PHP files</description> | ||
|
|
||
| <rule ref="nextcloud-default.xml" /> | ||
|
|
||
| <!-- The PHP constants true, false, and null MUST be in lower case. --> | ||
| <rule ref="Generic.PHP.LowerCaseConstant" /> | ||
|
|
||
| <!-- PHP keywords MUST be in lower case. --> | ||
| <rule ref="Generic.PHP.LowerCaseKeyword" /> | ||
|
|
||
| <!-- The body of each structure MUST be enclosed by braces. --> | ||
| <rule ref="Generic.ControlStructures.InlineControlStructure"> | ||
| <exclude-pattern>*/templates/*</exclude-pattern> | ||
| </rule> | ||
|
|
||
| <!-- There MUST not be more than one statement per line. --> | ||
| <rule ref="Generic.Formatting.DisallowMultipleStatements" /> | ||
|
|
||
| <!-- Only <?php, no short tags. --> | ||
| <rule ref="Generic.PHP.DisallowShortOpenTag.EchoFound" /> | ||
| </ruleset> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <?xml version="1.0"?> | ||
| <ruleset name="Nextcloud PHP Strict Standard"> | ||
|
|
||
| <description>Nextcloud coding standard for PHP files</description> | ||
|
|
||
| <!-- There MUST NOT be unused use statements. --> | ||
| <rule ref="./phpbb/Sniffs/Namespaces/UnusedUseSniff.php"> | ||
| <exclude-pattern>*/3rdparty/*</exclude-pattern> | ||
| </rule> | ||
| </ruleset> |
293 changes: 293 additions & 0 deletions
293
build/codesniffer/phpbb/Sniffs/Namespaces/UnusedUseSniff.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,293 @@ | ||
| <?php | ||
| /** | ||
| * | ||
| * This file is part of the phpBB Forum Software package. | ||
| * | ||
| * @copyright (c) phpBB Limited <https://www.phpbb.com> | ||
| * @license GNU General Public License, version 2 (GPL-2.0) | ||
| * | ||
| * For full copyright and license information, please see | ||
| * the docs/CREDITS.txt file. | ||
| * | ||
| */ | ||
|
|
||
| /** | ||
| * Checks that each use statement is used. | ||
| */ | ||
| class phpbb_Sniffs_Namespaces_UnusedUseSniff implements PHP_CodeSniffer_Sniff | ||
| { | ||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function register() | ||
| { | ||
| return array(T_USE); | ||
| } | ||
|
|
||
| protected function check(PHP_CodeSniffer_File $phpcsFile, $found_name, $full_name, $short_name, $line) | ||
| { | ||
|
|
||
| if ($found_name === $full_name) | ||
| { | ||
| $error = 'Either use statement or full name must be used.'; | ||
| $phpcsFile->addError($error, $line, 'FullName'); | ||
| } | ||
|
|
||
| if ($found_name === $short_name) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr) | ||
| { | ||
| if ($this->should_ignore_use($phpcsFile, $stackPtr) === true) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| $tokens = $phpcsFile->getTokens(); | ||
|
|
||
| $class_name_start = $phpcsFile->findNext(array(T_NS_SEPARATOR, T_STRING), ($stackPtr + 1)); | ||
|
|
||
| $find = array( | ||
| T_NS_SEPARATOR, | ||
| T_STRING, | ||
| T_WHITESPACE, | ||
| ); | ||
|
|
||
| $class_name_end = $phpcsFile->findNext($find, ($stackPtr + 1), null, true); | ||
|
|
||
| $aliasing_as_position = $phpcsFile->findNext(T_AS, $class_name_end, null, false, null, true); | ||
| if ($aliasing_as_position !== false) | ||
| { | ||
| $alias_position = $phpcsFile->findNext(T_STRING, $aliasing_as_position, null, false, null, true); | ||
| $class_name_short = $tokens[$alias_position]['content']; | ||
| $class_name_full = $phpcsFile->getTokensAsString($class_name_start, ($class_name_end - $class_name_start - 1)); | ||
| } | ||
| else | ||
| { | ||
| $class_name_full = $phpcsFile->getTokensAsString($class_name_start, ($class_name_end - $class_name_start)); | ||
| $class_name_short = $tokens[$class_name_end - 1]['content']; | ||
| } | ||
|
|
||
| $ok = false; | ||
|
|
||
| // Checks in simple statements (new, instanceof and extends) | ||
| foreach (array(T_INSTANCEOF, T_NEW, T_EXTENDS) as $keyword) | ||
| { | ||
| $old_simple_statement = $stackPtr; | ||
| while (($simple_statement = $phpcsFile->findNext($keyword, ($old_simple_statement + 1))) !== false) | ||
| { | ||
| $old_simple_statement = $simple_statement; | ||
|
|
||
| $simple_class_name_start = $phpcsFile->findNext(array(T_NS_SEPARATOR, T_STRING), ($simple_statement + 1)); | ||
|
|
||
| if ($simple_class_name_start === false) { | ||
| continue; | ||
| } | ||
|
|
||
| $simple_class_name_end = $phpcsFile->findNext($find, ($simple_statement + 1), null, true); | ||
|
|
||
| $simple_class_name = trim($phpcsFile->getTokensAsString($simple_class_name_start, ($simple_class_name_end - $simple_class_name_start))); | ||
|
|
||
| $ok = $this->check($phpcsFile, $simple_class_name, $class_name_full, $class_name_short, $simple_statement) ? true : $ok; | ||
| } | ||
| } | ||
|
|
||
| // Checks paamayim nekudotayim | ||
| $old_paamayim_nekudotayim = $stackPtr; | ||
| while (($paamayim_nekudotayim = $phpcsFile->findNext(T_PAAMAYIM_NEKUDOTAYIM, ($old_paamayim_nekudotayim + 1))) !== false) | ||
| { | ||
| $old_paamayim_nekudotayim = $paamayim_nekudotayim; | ||
|
|
||
| $paamayim_nekudotayim_class_name_start = $phpcsFile->findPrevious($find, $paamayim_nekudotayim - 1, null, true); | ||
| $paamayim_nekudotayim_class_name_end = $paamayim_nekudotayim - 1; | ||
|
|
||
| $paamayim_nekudotayim_class_name = trim($phpcsFile->getTokensAsString($paamayim_nekudotayim_class_name_start + 1, ($paamayim_nekudotayim_class_name_end - $paamayim_nekudotayim_class_name_start))); | ||
|
|
||
| $ok = $this->check($phpcsFile, $paamayim_nekudotayim_class_name, $class_name_full, $class_name_short, $paamayim_nekudotayim) ? true : $ok; | ||
| } | ||
|
|
||
| // Checks in implements | ||
| $old_implements = $stackPtr; | ||
| while (($implements = $phpcsFile->findNext(T_IMPLEMENTS, ($old_implements + 1))) !== false) | ||
| { | ||
| $old_implements = $implements; | ||
|
|
||
| $old_implemented_class = $implements; | ||
| while (($implemented_class = $phpcsFile->findNext(T_STRING, ($old_implemented_class + 1), null, false, null, true)) !== false) | ||
| { | ||
| $old_implemented_class = $implemented_class; | ||
|
|
||
| $implements_class_name_start = $phpcsFile->findNext(array(T_NS_SEPARATOR, T_STRING), ($implemented_class - 1)); | ||
| $implements_class_name_end = $phpcsFile->findNext($find, ($implemented_class - 1), null, true); | ||
|
|
||
| $implements_class_name = trim($phpcsFile->getTokensAsString($implements_class_name_start, ($implements_class_name_end - $implements_class_name_start))); | ||
|
|
||
| $ok = $this->check($phpcsFile, $implements_class_name, $class_name_full, $class_name_short, $implements) ? true : $ok; | ||
| } | ||
| } | ||
|
|
||
| $old_docblock = $stackPtr; | ||
| while (($docblock = $phpcsFile->findNext(T_DOC_COMMENT_CLOSE_TAG, ($old_docblock + 1))) !== false) | ||
| { | ||
| $old_docblock = $docblock; | ||
| $ok = $this->checkDocblock($phpcsFile, $docblock, $tokens, $class_name_full, $class_name_short) ? true : $ok; | ||
| } | ||
|
|
||
| $old_docline = $stackPtr; | ||
| while (($docline = $phpcsFile->findNext(T_COMMENT, ($old_docline + 1))) !== false) | ||
| { | ||
| $old_docline = $docline; | ||
| $content = $phpcsFile->getTokensAsString($docline, 1); | ||
| $content = str_replace("\t", ' ', $content); | ||
| $content = explode(' ', $content); | ||
|
|
||
| // E.g. /* @var Class $var */ | ||
| if (isset($content[1]) && isset($content[2]) && $content[1] === '@var') { | ||
| $classes = explode('|', str_replace('[]', '', $content[2])); | ||
| foreach ($classes as $class) | ||
| { | ||
| $ok = $this->check($phpcsFile, $class, $class_name_full, $class_name_short, $docline) ? true : $ok; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Checks in type hinting | ||
| $old_function_declaration = $stackPtr; | ||
| while (($function_declaration = $phpcsFile->findNext(T_FUNCTION, ($old_function_declaration + 1))) !== false) | ||
| { | ||
| $old_function_declaration = $function_declaration; | ||
|
|
||
| // Check type hint | ||
| $params = $phpcsFile->getMethodParameters($function_declaration); | ||
| foreach ($params as $param) | ||
| { | ||
| $ok = $this->check($phpcsFile, $param['type_hint'], $class_name_full, $class_name_short, $function_declaration) ? true : $ok; | ||
| } | ||
| } | ||
|
|
||
| $old_closure_declaration = $stackPtr; | ||
| while (($closure_declaration = $phpcsFile->findNext(T_CLOSURE, ($old_closure_declaration + 1))) !== false) { | ||
| $old_closure_declaration = $closure_declaration; | ||
|
|
||
| $parameter_start = $phpcsFile->findNext(array(T_OPEN_PARENTHESIS), ($closure_declaration + 1)); | ||
|
|
||
| if ($parameter_start === false) { | ||
| continue; | ||
| } | ||
|
|
||
| $parameter_end = $phpcsFile->findNext(array(T_CLOSE_PARENTHESIS), ($parameter_start + 1)); | ||
|
|
||
| if ($parameter_end === false) { | ||
| continue; | ||
| } | ||
|
|
||
| $type_hint_start = $parameter_start; | ||
| while ($type_hint_start = $phpcsFile->findNext(array(T_STRING), ($type_hint_start + 1), $parameter_end)) { | ||
| $type_hint_class_name = trim($phpcsFile->getTokensAsString($type_hint_start, 1)); | ||
| $ok = $this->check($phpcsFile, $type_hint_class_name, $class_name_full, $class_name_short, $parameter_start) ? true : $ok; | ||
| } | ||
| } | ||
|
|
||
| // Checks in catch blocks | ||
| $old_catch = $stackPtr; | ||
| while (($catch = $phpcsFile->findNext(T_CATCH, ($old_catch + 1))) !== false) | ||
| { | ||
| $old_catch = $catch; | ||
|
|
||
| $caught_class_name_start = $phpcsFile->findNext(array(T_NS_SEPARATOR, T_STRING), $catch + 1); | ||
| $caught_class_name_end = $phpcsFile->findNext($find, $caught_class_name_start + 1, null, true); | ||
|
|
||
| $caught_class_name = trim($phpcsFile->getTokensAsString($caught_class_name_start, ($caught_class_name_end - $caught_class_name_start))); | ||
|
|
||
| $ok = $this->check($phpcsFile, $caught_class_name, $class_name_full, $class_name_short, $catch) ? true : $ok; | ||
| } | ||
|
|
||
| if (!$ok) | ||
| { | ||
| $error = 'There must not be unused USE statements.'; | ||
| $phpcsFile->addError($error, $stackPtr, 'Unused'); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check if this use statement is part of the namespace block. | ||
| * | ||
| * @param PHP_CodeSniffer_File $phpcsFile The file being scanned. | ||
| * @param int $stackPtr The position of the current token in | ||
| * the stack passed in $tokens. | ||
| * | ||
| * @return bool | ||
| */ | ||
| private function should_ignore_use(PHP_CodeSniffer_File $phpcsFile, $stackPtr) | ||
| { | ||
| $tokens = $phpcsFile->getTokens(); | ||
|
|
||
| // Ignore USE keywords inside closures. | ||
| $next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true); | ||
| if ($tokens[$next]['code'] === T_OPEN_PARENTHESIS) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Ignore USE keywords for traits. | ||
| if ($phpcsFile->hasCondition($stackPtr, array(T_CLASS, T_TRAIT)) === true) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
|
|
||
| } | ||
|
|
||
| /** | ||
| * @param PHP_CodeSniffer_File $phpcsFile | ||
| * @param int $comment_end | ||
| * @param array $tokens | ||
| * @param string $class_name_full | ||
| * @param string $class_name_short | ||
| * | ||
| * @return bool | ||
| */ | ||
| private function checkDocblock(PHP_CodeSniffer_File $phpcsFile, $comment_end, $tokens, $class_name_full, $class_name_short) | ||
| { | ||
| $ok = false; | ||
|
|
||
| $comment_start = $tokens[$comment_end]['comment_opener']; | ||
| foreach ($tokens[$comment_start]['comment_tags'] as $tag) | ||
| { | ||
| if (!in_array($tokens[$tag]['content'], array('@param', '@var', '@return', '@throws'), true)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| $classes = $tokens[($tag + 2)]['content']; | ||
| $space = strpos($classes, ' '); | ||
| if ($space !== false) | ||
| { | ||
| $classes = substr($classes, 0, $space); | ||
| } | ||
|
|
||
| $tab = strpos($classes, "\t"); | ||
| if ($tab !== false) | ||
| { | ||
| $classes = substr($classes, 0, $tab); | ||
| } | ||
|
|
||
| $classes = explode('|', str_replace('[]', '', $classes)); | ||
| foreach ($classes as $class) | ||
| { | ||
| $ok = $this->check($phpcsFile, $class, $class_name_full, $class_name_short, $tokens[$tag + 2]['line']) ? true : $ok; | ||
| } | ||
| } | ||
|
|
||
| return $ok; | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we adjust this license header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I just c+p the file. But we are thinking about putting it into another project, then we could composer it. Until then I think we should just keep the copright+license, if that is okay @schiessle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least this line should be changed. Currently it's leading to a non-existing file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes we need the content of that file here then ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's delay until it is split by phpBB into a decent package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.