Skip to content

Conversation

@jrfnl
Copy link
Member

@jrfnl jrfnl commented Oct 18, 2022

QA: make all classes final

The "class vs sniff name" matcher in the PHPCS autoloader does not handle sniffs classes extending other sniffs well.

Now, while this "standard" doesn't actually contain any sniffs, the utilities in this library are not intended to be extended anyway, aside from the explicitly abstract base sniffs.

So, with that in mind, I'm making all classes in this library final, with the exception of some select test classes (the BCFile ones as those need to be extended to safeguard that the PHPCSUtils native functionality mirrors the PHPCS native functionality).

If anyone has issues with any particular class now being final, please report this ASAP and please include a good use-case of why that particular class should not be final.

QA: don't use static for LSB in final class

... as there can never be a (grand-)child class.

jrfnl added 2 commits October 18, 2022 06:39
The "class vs sniff name" matcher in the PHPCS autoloader does not handle sniffs classes extending other sniffs well.

Now, while this "standard" doesn't actually contain any sniffs, the utilities in this library are not intended to be extended anyway, aside from the explicitly `abstract` base sniffs.

So, with that in mind, I'm making all classes in this library `final`, with the exception of some select test classes (the `BCFile` ones as those need to be extended to safeguard that the PHPCSUtils native functionality mirrors the PHPCS native functionality).

If anyone has issues with any particular class now being `final`, please report this ASAP and please include a good use-case of why that particular class should not be `final`.
... as there can never be a (grand-)child class.
@jrfnl jrfnl added this to the 1.0.0-alpha4 milestone Oct 18, 2022
@jrfnl jrfnl merged commit be2cdce into develop Oct 18, 2022
@jrfnl jrfnl deleted the feature/qa-make-all-classes-final branch October 18, 2022 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants