Automattic / phpcs-neutron-standard

A set of phpcs sniffs for PHP >7 development
MIT License
94 stars 7 forks source link

Disallow unused imports #3

Closed sirbrillig closed 6 years ago

sirbrillig commented 6 years ago

Add a sniff that prevents use statements that import classes, functions, or constants which are not used.

For example, if I have the line use My\Classname; but Classname is not used anywhere in the file, this would trigger an error.

This adds the rule:

New code MUST NOT import a class, function, or constant without using that import.

janm6k commented 6 years ago

Is this sniff from other coding standard or written from scratch?

I think this one should do the same: https://github.com/slevomat/coding-standard/blob/master/SlevomatCodingStandard/Sniffs/Namespaces/UnusedUsesSniff.php

sirbrillig commented 6 years ago

Is this sniff from other coding standard or written from scratch?

It's written from scratch. I figured one would exist somewhere but I couldn't find it. Clearly you are better at looking than I am. 😁 Theirs looks much more... complete? But mine is much shorter! 😛 Looking through their version I'm trying to come up with a case where mine would fail, but I'm having trouble doing that. I'm inclined to keep my version unless I can come up with a failing test.

janm6k commented 6 years ago

If I understand the difference correctly, this is the case where theirs sniff would detect it, but the shorter one would not detect it:

<?php

use First;
use Second;

new \Third\Second\First();
Hywan commented 6 years ago

We have 4 kind of names in PHP:

  1. Unqualified, like C,
  2. Qualified, like A\B\C,
  3. Relative qualified, like namespace\C,
  4. Fully-qualified, like \A\B\C.

I would rename the rule as:

New code MUST NOT use fully-qualified names.

(with a note to remind the kind of names in PHP maybe?)

But the main point of my comment is: It's easier to detect fully-qualified names, than comparing aliases (use) with names. See my comment https://github.com/Automattic/phpcs-neutron-standard/pull/3#discussion_r156016787.

janm6k commented 6 years ago

New code MUST NOT use fully-qualified names.

Situations when I think fully qualified names are better (all fall into category of using a library/wp classes):

Hywan commented 6 years ago

Oh sorry, I guess I misinterprete the whole goal of the rule… I thought it was to forbidden fully-qualified names, but it's not :-p.

sirbrillig commented 6 years ago

If we create a separate repo which is a configuration of different standards (see #13), we could just include the sniff that Jan found there, rather than trying to duplicate it here.

sirbrillig commented 6 years ago

https://github.com/Automattic/phpcs-neutron-ruleset/issues/1 will take care of this.