Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Sniff to Evaluate Usages of imports #1

Closed
mikeselander opened this issue Jan 11, 2020 · 11 comments · Fixed by #25
Closed

Sniff to Evaluate Usages of imports #1

mikeselander opened this issue Jan 11, 2020 · 11 comments · Fixed by #25

Comments

@mikeselander
Copy link

I would very much like a sniff that allows you to specifically allow/disallow usage of types of imports and usage of said imports. This sniff could detect types of imports and aliasing or not of imports within files.

Scenario 1:
I want to limit the usage of use function, but still allow the usage of use {namespace} or use const in our ruleset.

Scenario 2:
I want to limit the usage of use function, but allow use function {name} as {moreSpecificName} aliasing.

The most important (for my needs) and, I assume, easiest implementation is just to flag types of importing that are disallowed in a codebases. Do ping me if any of this is confusing or ambiguous!

@jrfnl
Copy link
Member

jrfnl commented Jan 11, 2020

@mikeselander Just to make sure we're talking about the same thing:

use function MyNamespace\myFunction; // Error.
use function Vendor\YourNamespace\yourFunction as FunctionAlias; // OK.
use function foo\math\sin, foo\math\cos as FooCos, foo\math\cosh;  // Error, OK, Error.

use function bar\math\{
    Msin, // Error.
    level\Mcos as BarCos, // OK.
    Mcosh, // Error.
};

Is that what you mean ?

@mikeselander
Copy link
Author

@jrfnl yes, that's exactly what I meant ❤️

@jrfnl
Copy link
Member

jrfnl commented Jan 13, 2020

@mikeselander In that case: done ;-) It will take a few more weeks before I can put it online as I need PHPCSUtils to be completely online first (and tagged), but you can expect the sniff to be available soonish (via this repo).

@jrfnl
Copy link
Member

jrfnl commented Jan 13, 2020

@mikeselander FYI: the implementation I've chosen to go with is as follows:

  • Three sniffs DisallowUseClass, DisallowUseFunction, DisallowUseConst
  • Each sniff has two error codes FoundWithoutAlias and FoundWithAlias.

You can either include the complete sniff or just include a single error code like
<rule ref="Stnd.Cat.SniffName.ErrorCode"/>.

The sniffs will generate metrics which can be viewed via the --report=info option to show how many import statements there are of each type with/without alias. Example:
image

The sniff does not currently contain a fixer.

@mikeselander
Copy link
Author

@jrfnl I don't quite understand how you got that done so fast, but I'm properly impressed 👏 That all looks well-structured and I wouldn't have expected a fixer for this myself. Thank you so much!

@jrfnl
Copy link
Member

jrfnl commented Jan 16, 2020

@mikeselander The secret is in the PHPCSUtils utility methods ;-) Basically all the logic needed was already in place, all I needed to do in the sniff was call the appropriate utility method 😀

@jrfnl
Copy link
Member

jrfnl commented Jan 23, 2020

@mikeselander The PRs I just pulled should sort this issue out. Testing appreciated. #7, #8, #9

@mikeselander
Copy link
Author

Amazing, thank you! I'll try them out on our ruleset as time allows in the next few weeks.

@jrfnl
Copy link
Member

jrfnl commented Jan 24, 2020

@mikeselander Great, let me know how you get on.

I was thinking of maybe adding a third error code to the sniffs for when non-namespaced entities are being imported. That way you could use that error code to just forbid non-aliased imports from the global namespace, but allow aliased and namespaced imports.

What do you think ?

@mikeselander
Copy link
Author

@jrfnl that makes a lot of sense to me 👍

@jrfnl
Copy link
Member

jrfnl commented Feb 17, 2020

@mikeselander I've implemented my suggestion above and ended up making the sniffs even more modular. See PR #25, which I've just merged.

I've added four new error codes to each of the sniffs:

  • FoundSameNamespace, FoundSameNamespaceWithAlias for use statements importing from the same namespace;
  • FoundGlobalNamespace, FoundGlobalNamespaceWithAlias for use statements importing from the global namespace, like import statements for PHP native classes, functions and constants.

In all other circumstances, the pre-existing error codes FoundWithAlias and FoundWithoutAlias will continue to be used.

I've also added this information to the metric which can be viewed using --report=info. The mtreics will now look something like this:

PHP CODE SNIFFER INFORMATION REPORT                                                
----------------------------------------------------------------------             
Use import statement for class/interface/trait:                                    
        without alias => 42 ( 72.41%)                                              
        with alias    => 16 ( 27.59%)                                              
        ------------------------------                                             
        total         => 58 (100.00%)                                              
                                                                                   
Use import statement source for class/interface/trait:                             
        different namespace => 43 ( 74.14%)                                        
        same namespace      =>  8 ( 13.79%)                                        
        global namespace    =>  7 ( 12.07%)                                        
        ------------------------------------                                       
        total               => 58 (100.00%)                                        

@jrfnl jrfnl added this to the 1.0.0-alpha1 milestone Apr 29, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants