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

Add final modifier support #49

Open
tomzx opened this issue Feb 24, 2015 · 4 comments
Open

Add final modifier support #49

tomzx opened this issue Feb 24, 2015 · 4 comments

Comments

@tomzx
Copy link
Owner

tomzx commented Feb 24, 2015

It is clear that having the final keyword applied either to a class in its entirety or on specific methods has an impact on the type of semantic version a change should generate.

I suggest adding the following rules in order to cover this new requirement.

Visibility (public, protected, private) applies to classes/trait methods.

Classes

Classes with final keyword on the class
The case is for a class that already has the final modifier applied to it, but where we're adding/removing methods.

Code Level Rule
---- ----- - Added
V017 MINOR -- Public
V018 PATCH -- Protected
V030 PATCH -- Private
---- ----- - Removed
VXXX MAJOR -- Public
V022 PATCH -- Protected
VXXX PATCH -- Private

Classes without final keyword on the class but with final keyword on the methods
The case is for a class that does not have the final modifier applied to it and where we're adding/removing methods with the final keyword.

Code Level Rule
---- ----- - Added
VXXX MAJOR -- Public
VXXX MAJOR -- Protected
VXXX PATCH -- Private
---- ----- - Removed
VXXX MAJOR -- Public
VXXX MAJOR -- Protected
VXXX PATCH -- Private

Trait

The case is for a trait but where we're adding/removing methods with the final keyword.

Code Level Rule
---- ----- - Added
VXXX MAJOR -- Public
VXXX MAJOR -- Protected
VXXX MAJOR -- Private
---- ----- - Removed
VXXX MAJOR -- Public
VXXX MAJOR -- Protected
VXXX MAJOR -- Private

The rule levels are up for discussion.

This creates 14 new rules and use 4 existing rules (not implemented).

Changelog:
2015-02-25: Add separation between final on class and final on methods based on @mikeSimonson feedback.

@mikeSimonson
Copy link
Contributor

Classes with final keyword on the class

Code Level Rule comment
---- ----- - Added
V017 MINOR -- Public ok
V018 PATCH -- Protected ok
V030 PATCH -- Private ok
---- ----- - Removed
VXXX MAJOR -- Public ok
V022 PATCH -- Protected ok
VXXX PATCH -- Private ok

Classes without final keyword on the class but with final keyword on the mehods

Code Level Rule
---- ----- - Added
VXXX MAJOR -- Public
VXXX MAJOR -- Protected
VXXX PATCH -- Private
---- ----- - Removed
VXXX MAJOR -- Public
VXXX MAJOR -- Protected
VXXX PATCH -- Private

As for the trait

A trait in itself can't be final so the rules makes sense to me.

@mikeSimonson
Copy link
Contributor

http://3v4l.org/OjLfF for the trait behavior

@tomzx
Copy link
Owner Author

tomzx commented Feb 25, 2015

@mikeSimonson Not sure that demo proves anything. See http://3v4l.org/klGki. Final or not final, it doesn't change a thing.

Final can be applied on trait methods, but not the whole trait itself.

My understanding of trait is basically that you do a copy/paste of what is inside of it and apply it to a class. Final modifier on methods are said to work according to the rfc.

See http://3v4l.org/uhsP9 for an example of a trait being used and its final modifier constraining subclasses from overriding it.

I'm not sure I see why you'd have different levels triggered if the final modifier is applied on a class or a method. To me, having the final keyword on the class means that it is the same as having the final keyword applied to all of the methods. But it appears I am wrong in that regard. Final on class => cannot extend the class, final on methods => cannot override the method in subclasses.

Thus, I do agree there should be a difference between final being on the class or the method. 👍 on the suggested levels.

@mikeSimonson
Copy link
Contributor

Ok, I see what I missed about the the final in the trait, but still it's a bit confusing :-p.

Thanks

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

2 participants