Part of #2571965: [meta] Fix PHP coding standards in core

Remove all unused use statements and enable the rule.

To test the patches run phpcs -p --sniffs=Drupal.Classes.UnusedUseStatement from the core directory.

CommentFileSizeAuthor
#28 2674408.28.patch576 bytesalexpott
#22 2674408.22.patch561 bytesalexpott
#19 2674408-19.patch76.82 KBalexpott
#19 17-19-interdiff.txt14.37 KBalexpott
#17 2674408-17.patch62.45 KBalexpott
#17 15-17-interdiff.txt508 bytesalexpott
#15 2674408-15.patch62.41 KBalexpott
#10 2674408-10.patch65.42 KBalexpott
#8 2674408-8.patch59.6 KBalexpott
#8 7-8-pseudo-interdiff.txt499 bytesalexpott
#7 2674408-7.patch59.64 KBalexpott
#7 5-7-interdiff.txt1.41 KBalexpott
#5 2674408-5.patch58.83 KBalexpott
#5 2-5-interdiff.txt16.92 KBalexpott
#2 2674408-2.patch51.29 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Active » Needs review
FileSize
2674408-2.patch51.29 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2674408-2.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Hmm... so the "Drupal.Classes.UnusedUseStatement" has some problems...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Needs review
FileSize
2-5-interdiff.txt16.92 KB
2674408-5.patch58.83 KB

Created #2674598: UnusedUseStatement sniff must not be case sensitive for the false positives because of case sensitivity.

Status: Needs review » Needs work

The last submitted patch, 5: 2674408-5.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Needs review
FileSize
5-7-interdiff.txt1.41 KB
2674408-7.patch59.64 KB

Rebased and found a couple more after.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
7-8-pseudo-interdiff.txt499 bytes
2674408-8.patch59.6 KB

Rerolled due to #2666172: PHPCS configuration should include rules and not exclude them

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
2674408-10.patch65.42 KB

Rerolled and re-ran phpcs to fix the rule - plenty of new instances being added to core all the time :(

Status: Needs review » Needs work

The last submitted patch, 10: 2674408-10.patch, failed testing.

The last submitted patch, 10: 2674408-10.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Issue tags: +needs backport to 8.1.x

This will need to be rerolled for 8.1.x...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Issue summary: View changes
Status: Needs work » Needs review

So #10 passes on 8.2.x - to test this run phpcs -p --sniffs=Drupal.Classes.UnusedUseStatement from the core directory.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
2674408-15.patch62.41 KB

Due to a revert against 8.2.x this patch needs a reroll - patch might apply to both 8.2.x and 8.1.x now :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Went through all classes.

+++ b/core/modules/field/tests/src/Kernel/WidgetPluginManagerTest.php
@@ -7,7 +7,6 @@
 use Drupal\Tests\field\Kernel\FieldKernelTestBase;

This class is unused

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
FileSize
15-17-interdiff.txt508 bytes
2674408-17.patch62.45 KB

Great spot PHPStorm ;) Need to file a bug against coder.

@dawehner++

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community

There we go!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs review
FileSize
17-19-interdiff.txt14.37 KB
2674408-19.patch76.82 KB

I fixed the PHPCS rule in #2685877: Drupal.Classes.UnusedUseStatement misses some and ran it again...

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Oh wow, yeah all of those instances have been caused by myself quite recently.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Fixed » Needs review
FileSize
2674408.22.patch561 bytes

A very small followup cause one got introduced and I don't think we should have a yet-another-unused-use-removal issue.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Reviewed & tested by the community

Boom.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

  • catch committed 4d185fa on 8.2.x 
    Issue #2674408 by alexpott, dawehner: Fix Drupal.Classes....

  • catch committed 00ebd9a on 8.2.x 
    Issue #2674408 by alexpott, dawehner: Fix "Drupal.Classes....

  • catch committed ff9aa91 on 8.1.x 
    Issue #2674408 by alexpott, dawehner: Fix "Drupal.Classes....
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Fixed » Reviewed & tested by the community
FileSize
2674408.28.patch576 bytes

One more crept in...

I'm not opening a new issue because I don;t want additional review credit for this.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

+1

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 93f9d84 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed 783fead on 8.2.x 
    Issue #2674408 by alexpott, dawehner: Fix "Drupal.Classes....

  • alexpott committed 93f9d84 on 8.1.x 
    Issue #2674408 by alexpott, dawehner: Fix "Drupal.Classes....
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Fixed » Needs review
FileSize
2674408.33.patch10.45 KB

#2686207: Convert simpletest kernel tests in modules A-I to phpunit introduced a load more and because #2685877: Drupal.Classes.UnusedUseStatement misses some has not yet been released I missed them when I reviewed...

Status: Needs review » Needs work

The last submitted patch, 33: 2674408.33.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Needs review

DrupalCI had a hiccup.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Version: 8.2.x-dev » 8.1.x-dev
Issue tags: -needs backport to 8.1.x
FileSize
2674408-2-36.patch10.47 KB

Rerolled and now it applies to both 8.1.x and 8.2.x.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community

So many real life usecases!

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Looks good.

It's a bit annoying that you end up with those left-overs as phpstorm adds them when you move a class around but doesn't remove it when you add them back into the same namespac.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed f33830d and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed f3f3800 on 8.2.x 
    Issue #2674408 by alexpott: Fix "Drupal.Classes.UnusedUseStatement"...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

JimmyAx’s picture

JimmyAx CreditAttribution: JimmyAx commented

It looks like this commit is somehow breaking my site.
#2716827: OptionsProviderInterface::class is causing a fatal error