Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Replace class method parameters changed with class parameters added/removed #50

Open
tomzx opened this issue Feb 25, 2015 · 3 comments
Open

Comments

@tomzx
Copy link
Owner

tomzx commented Feb 25, 2015

I propose to remove the Class (public|protected|private) method parameter changed rules with a more expressive set of [Final] Class (public|protected|private) method parameter (added|removed). It has the advantage of better discerning what is happening to a method signature beside having "changed". The current implementation would for example consider that adding a new parameter with a default value to be of no significance (not even a PATCH triggered).

Consider the following signature changes:

function f(A $a); => function f(A $a, B $b); // B added

function f(A $a); => function f(A $a, B $b = null); // B added with a default

function f(A $a, B $b); => function f(A $a); // B removed

function f(A $a); => function f(A $a = null); // A optionalized (same as adding with default)

function f(A $a = null); => function f(A $a); // A removed, A added

function f(A $a, B $b); => function f(B $b, A $a); // A removed, B removed, B added, A added

function f(A $a = null, B $b = null); => function f(B $b = null, A $a = null); // A removed, B removed, B added with a default, A added with a default

This changes comes with the following new rules.

Visibility (public, protected, private) applies to classes/trait methods.
Final applies to whether the method or class is marked as final.

Functions (+3)

Code Level Rule
VXXX MAJOR - Added
VXXX MINOR - Added (with default)
VXXX MAJOR - Removed

Classes (+1827)

Final class has priority over final method.

Code Level Rule
---- ----- - Added
---- ----- -- Public
VXXX MAJOR --- Final class
VXXX MAJOR --- Final method
VXXX MAJOR --- Not final
---- ----- -- Protected
VXXX PATCH --- Final class
VXXX MAJOR --- Final method
VXXX MAJOR --- Not final
---- ----- -- Private
VXXX PATCH --- Final class
VXXX PATCH --- Final method
VXXX PATCH --- Not final
---- ----- - Added (with default)/Optionalized
---- ----- -- Public
VXXX MINOR --- Final class
VXXX MINOR --- Final method
VXXX MINOR --- Not final
---- ----- -- Protected
VXXX PATCH --- Final class
VXXX MINOR --- Final method
VXXX MINOR --- Not final
---- ----- -- Private
VXXX PATCH --- Final class
VXXX PATCH --- Final method
VXXX PATCH --- Not final
---- ----- - Removed
---- ----- -- Public
VXXX MAJOR --- Final class
VXXX MAJOR --- Final method
VXXX MAJOR --- Not final
---- ----- -- Protected
VXXX PATCH --- Final class
VXXX MAJOR --- Final method
VXXX MAJOR --- Not final
---- ----- -- Private
VXXX PATCH --- Final class
VXXX PATCH --- Final method
VXXX PATCH --- Not final

Interface (+2)

Code Level Rule
VXXX MAJOR - Added
VXXX MAJOR - Removed

Trait (+18)

Code Level Rule
---- ----- - Added
---- ----- -- Public
VXXX MAJOR --- Final
VXXX MAJOR --- Not final
---- ----- -- Protected
VXXX MAJOR --- Final
VXXX MAJOR --- Not final
---- ----- -- Private
VXXX MAJOR --- Final
VXXX MAJOR --- Not final
---- ----- - Added (with default)/Optionalized
---- ----- -- Public
VXXX MINOR --- Final
VXXX MINOR --- Not final
---- ----- -- Protected
VXXX MINOR --- Final
VXXX MINOR --- Not final
---- ----- -- Private
VXXX MINOR --- Final
VXXX MINOR --- Not final
---- ----- - Removed
---- ----- -- Public
VXXX MAJOR --- Final
VXXX MAJOR --- Not final
---- ----- -- Protected
VXXX MAJOR --- Final
VXXX MAJOR --- Not final
---- ----- -- Private
VXXX MAJOR --- Final
VXXX MAJOR --- Not final

The rule levels are up for discussion.

This creates 4150 new rules and deprecates 15 existing rules.

I would also like to get input on whether the "optionalize", that is, make a parameter optional by giving it a default value, should be considered as separate rules, or together like it is presented here. From my initial analysis, it appears that the generated levels would be the same as considering the "optionalization" to be like adding a parameter with a default.

Edit 1: Updated to take into account final class vs final method.

@tomzx tomzx added this to the Candidate for next Minor milestone Feb 25, 2015
@tomzx
Copy link
Owner Author

tomzx commented May 28, 2015

There's a difference between a final class method and a class final method that does not seem to be taken into account here.

For instance, in the case of a final class, which cannot be inherited, protected methods become similar to private methods, that is, they are not visible to external clients. However, a class final method cannot be overridden, but can still be called by subtypes, thus similar to a public method.

Thus, in a quick analysis, this may mean +9 rules for classes.

@tomzx
Copy link
Owner Author

tomzx commented May 28, 2015

Nothing is said about the following cases either:

function f(A $a); => function f($a); // A typehint removed

function f($a); => function f(A $a); // A typehint added

@tomzx tomzx added the advanced label Aug 24, 2015
@tomzx
Copy link
Owner Author

tomzx commented May 8, 2016

Partially implemented by 4666578. Final was left aside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant