Automattic / phpcs-neutron-standard

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

Add RequireImportsSniff (take 2) #59

Closed sirbrillig closed 6 years ago

sirbrillig commented 6 years ago

This adds a sniff which shows warnings if a symbol (function, constant, class) is used and is not defined directly, imported explicitly, nor has its namespace imported.

When code is moved around, it can be problematic if classes which are used in a relative or global context get moved to a different namespace. In those cases it's better if the classes use their fully-qualified namespace, or if they are imported explicitly using use (in which case they can be detected by a linter like this one). These warnings should help when refactoring code to avoid bugs.

It also detects imports which are not being used.

For example:

namespace Vehicles;
use Registry;
use function Vehicles\startCar;
use Chocolate; // this will be a warning because `Chocolate` is never used
class Car {
  public function drive() {
    startCar(); // this is fine because `startCar` is imported
    Registry\registerCar($this); // this is fine because `Registry` is imported
    goFaster(); // this will be a warning because `goFaster` was not imported
  }
}

You can use the config option ignoreUnimportedSymbols to ignore as many symbols as you like. It is a regular expression. Here's an example:

    <rule ref="NeutronStandard.Imports.RequireImports">
        <properties>
            <property name="ignoreUnimportedSymbols" value="/^(wp_parse_args|OBJECT\S*|ARRAY_\S+|is_wp_error|__|esc_html__|get_blog_\S+)$/"/>
        </properties>
    </rule>

This is a revised version of #57.

Fixes #45 in a way which is less risky than #57 by allowing absolute imports and namespace imports.

Things still to do:

Hywan commented 6 years ago

Does it work attribute too? It should be great to detect undeclared attributes.

sirbrillig commented 6 years ago

Does it work attribute too? It should be great to detect undeclared attributes.

That's a very good idea, but I think it should be a different sniff.

Hywan commented 6 years ago

Correct!

sirbrillig commented 6 years ago

I moved this to its own standard, to avoid any controversy and to make it more portable: ImportDetection

I will add it to NeutronRuleset soon with a custom ignore line.