Added AbstractPrefixRequiredForAbstractClass, InterfaceSuffixRequired…

[annechko]

Added 3 new sniffs to Generic standard to be able to check these rules:
PSR Naming Conventions https://www.php-fig.org/bylaws/psr-naming-conventions/

1. Interfaces MUST be suffixed by Interface: e.g. Psr\Foo\BarInterface.
2. Abstract classes MUST be prefixed by Abstract: e.g. Psr\Foo\AbstractBar.
3. Traits MUST be suffixed by Trait: e.g. Psr\Foo\BarTrait

[annechko]

Related to this issue
#3054

[jrfnl]

Hi @annechko, Thanks for these sniffs. Looking good!

I've left some suggestions and comments in-line for you to consider. Most apply to all three sniffs, but I've only left a comment where I encountered something the first time.

[jrfnl]

@annechko 👍

I just noticed that I overlooked the docs files, so two more small remarks there.

Only other thing I could imagine, would be to make the desired prefix/suffix configurable as well as making it configurable whether it should be a prefix or a suffix, but that's probably functionality which would be used by only a small % of users of the sniffs.

[annechko]

@jrfnl Thanks for your feedback! I've tried to fix all places.

Only other thing I could imagine, would be to make the desired prefix/suffix configurable as well as making it configurable whether it should be a prefix or a suffix, but that's probably functionality which would be used by only a small % of users of the sniffs.

I changed InterfaceSuffixRequiredForInterface to AffixRequiredForInterface
because I think it might be a real use-case for some projects, sometimes developers use names like IName, IProperty etc.

And I'm not sure about other sniffs, whether to wait for an issue for Abstract Classes and Traits sniffs
or maybe - if new implementation of AffixRequiredForInterface is ok - I can do something alike with the other two sniffs)

[gsherwood]

Sorry, have only just gotten to this. Great work by the way.

Only other thing I could imagine, would be to make the desired prefix/suffix configurable as well as making it configurable whether it should be a prefix or a suffix, but that's probably functionality which would be used by only a small % of users of the sniffs.

I actually think the sniff would be better reverted to before this change was made, especially given that it's only been made in the interface naming sniff. We could add complexity to it if there is a strong desire for it by a larger number of users, but I like the idea of committing 3 sniffs that just implement those PSR naming conventions for anyone who wants to use them.

We could do the other way and implement the same options in the other 2 sniffs, but I think this would require a rethink of how the error messages are created as the interface one is getting a bit complex.

Thoughts?

Either way, I've marked this for inclusion in the 3.6.0 release. I'm sure we'll get to a good place before then.

[annechko]

@gsherwood I think that maybe it would be better to cover only those rules from PSR naming first, and then wait if there will be a request from users to extend the behavior, at least these sniffs are better then nothing, as it was before them :)

Also all these 3 sniffs are very similar, except for error messages and register() method (different tokens), so if we ever want to expand behavior in all 3 sniffs it'd be better to use some common base class and inheret all sniffs from it, and it takes time, and maybe this behavior is not really needed for now.

So - revert code and wait for an issue - is ok?

[gsherwood]

So - revert code and wait for an issue - is ok?

I'd be happy with that. I think the sniffs provide real value even without these config options.

[gsherwood]

I've merged in these new sniffs, although I did decide to rename them so they fit a bit better with the other sniffs in that category. So PHPCS now has Generic.NamingConventions.AbstractClassNamePrefix, Generic.NamingConventions.InterfaceNameSuffix, and Generic.NamingConventions.TraitNameSuffix sniffs from version 3.6.0.

Thanks again.