Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Add two sniffs to find and remove unused "use" clauses #11

Merged
merged 1 commit into from
Oct 11, 2017

Conversation

thiemowmde
Copy link
Collaborator

@thiemowmde thiemowmde commented Aug 31, 2017

A run on the Wikibase.git code base currently shows 9 violations that are all fine and can be fixed automatically.

Note that the Wikibase.Namespaces.UnusedUse sniff does have a series of known as well as potential flaws:

  • It considers class names aliased with as. The code to find the alias might be a bit naive and miss edge cases. However, I can not think of a test case that fails. Can you find one?
  • It only considers a list of PHPDoc tags that are known to contain class names: @expectedException, @param, @return, @throw, and @var (as well as @type for compatibility, even if deprecated, see Replace ReturnsTag with DisallowedDocTags sniff #8). Did I missed one?

Copy link
Member

@lucaswerkmeister lucaswerkmeister left a comment

Choose a reason for hiding this comment

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

I tried this out in the WikibaseQualityConstraints extension, where it removes 92 lines (all correct, as far as I can tell – I’ve just been sloppy with the imports). Seems to work fine.

Copy link
Member

@lucaswerkmeister lucaswerkmeister left a comment

Choose a reason for hiding this comment

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

Found a bug:

<?php
use A\B as /*_*/C;
$b = new C();

turns into

<?php
C;
$b = new C();

which is invalid.

@addshore
Copy link
Contributor

Can this make its way into the mediawiki codesniffer?

wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-WikibaseQualityConstraints that referenced this pull request Oct 10, 2017
All found and fixed with wikibase/wikibase-codesniffer#11 [1].

[1]: wmde/WikibaseCodeSniffer#11

Change-Id: I0f1d9a4cc79e75314772b672386ec842bd86d463
@thiemowmde
Copy link
Collaborator Author

@lucaswerkmeister: Good catch. That was easy to fix.

@addshore: See https://gerrit.wikimedia.org/r/375547. Both teams started to work on the same idea independently. Naturally, I like my version more. ;-)

@lucaswerkmeister
Copy link
Member

Sorry, there’s another broken case:

<?php
use A\B as /** @var */ C;

$b = new C();

I tried expanding your fix, but it doesn’t work:

diff --git a/Wikibase/Sniffs/Namespaces/UnusedUseSniff.php b/Wikibase/Sniffs/Namespaces/UnusedUseSniff.php
index f2e94c4737..09dc891d09 100644
--- a/Wikibase/Sniffs/Namespaces/UnusedUseSniff.php
+++ b/Wikibase/Sniffs/Namespaces/UnusedUseSniff.php
@@ -24,6 +24,10 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
 		$useEndPtr = $phpcsFile->findNext( [
 			T_AS,
 			T_COMMENT,
+			T_DOC_COMMENT_OPEN_TAG,
+			T_DOC_COMMENT_CLOSE_TAG,
+			T_DOC_COMMENT_STRING,
+			T_DOC_COMMENT_TAG,
 			T_NS_SEPARATOR,
 			T_STRING,
 			T_WHITESPACE,

But this is a seriously pathological case… if the bug is too hard to fix, I think we could merge this without the bugfix.

@thiemowmde
Copy link
Collaborator Author

Solved it. Thanks for poking this stuff!

@lucaswerkmeister
Copy link
Member

Great, thanks! That fix also looks more robust ;)

@lucaswerkmeister lucaswerkmeister merged commit 73458be into master Oct 11, 2017
@lucaswerkmeister lucaswerkmeister deleted the unusedUsesSniff branch October 11, 2017 12:34
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-Wikibase that referenced this pull request Oct 11, 2017
As reported by the custom PHPCS sniff introduced via
wmde/WikibaseCodeSniffer#11

Change-Id: I011d95d8eda9ff47ebf23a2a8e04389ea401c332
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants