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

Rule Request: Prohibit subclassing #2079

Closed
2 tasks done
driver733 opened this issue Mar 2, 2018 · 13 comments
Closed
2 tasks done

Rule Request: Prohibit subclassing #2079

driver733 opened this issue Mar 2, 2018 · 13 comments
Labels
rule-request Requests for a new rules.

Comments

@driver733
Copy link

New Issue Checklist

  • Updated SwiftLint to the latest version
  • I searched for existing GitHub issues

Rule Request

Description

Implementation inheritance was created as a mechanism for code reuse, and it doesn't fit into OOP at all. Yes, it may look convenient in the beginning, but it is absolutely wrong in terms of object thinking. Just like getters and setters, implementation inheritance turns objects into containers with data and procedures. Of course, it's convenient to copy some of those data and procedures to a new object in order to avoid code duplication. But this is not what objects are about.

Read this for more info.

What would trigger?

class MyType: SuperType {
                public init() { super() }" 
            }

What would not trigger?

class MyType: SuperType {
                public init() {  }" 
            }

Configurable?

The rule should not be configurable.

Opt-in or enabled?

This rule could be a personal preference or coding style from various individuals, so it could be kept as opt-in.

driver733 pushed a commit to driver733/SwiftLint that referenced this issue Mar 2, 2018
driver733 pushed a commit to driver733/SwiftLint that referenced this issue Mar 2, 2018
driver733 pushed a commit to driver733/SwiftLint that referenced this issue Mar 2, 2018
@marcelofabri
Copy link
Collaborator

I don't get it. You should call super on inits in general. The "problem" is not calling super.init(), but declaring inheritance.

@marcelofabri marcelofabri added the rule-request Requests for a new rules. label Mar 9, 2018
@driver733
Copy link
Author

@marcelofabri If a class is not a subclass, then it does not need to call super.
If a class calls super, we assume, by this rule, that it is a subclass and highlight it.

@marcelofabri
Copy link
Collaborator

But this is the wrong assumption. You can have subclasses that don't call super (if they don't override any methods) but you can't have classes that call super and are not subclasses (the compiler doesn't let you do that).

@driver733
Copy link
Author

driver733 commented Mar 9, 2018

@marcelofabri

You can have subclasses that don't call super (if they don't override any methods)

If a class is a subclass it will have super.init() as a first statement is at least one of the constructors and that rule will detect that.

you can't have classes that call super and are not subclasses (the compiler doesn't let you do that).

Although I agree, I do not understand how this statement is applicable to this rule.

Again, if a class is not a subclass, then it will not have super anywhere. If a class is a subclass, it will have super at least somewhere, for instance, in one of the constructors.

@marcelofabri
Copy link
Collaborator

If a class is a subclass it will have super.init() as a first statement is at least one of the constructors and that rule will detect that.

You don't need to override a constructor in a subclass.

@driver733
Copy link
Author

driver733 commented Mar 9, 2018

@marcelofabri If you introduce new properties or create a new constructor in a subclass, you will inevitably end up having a call to the superclass in one of the constructors. (You might not have the override keyword, but you will have a call to super) See example here.
By convention, I believe, you have to call super in subclasses.
An example can be seen here.

Also, see this.

@marcelofabri
Copy link
Collaborator

If you introduce new properties or create a new constructor in a subclass, you will inevitably end up having a call to the superclass in one of the constructors.

That's simply not true. You can have properties with default values. And you can have subclasses without adding new properties. This is common when using UIKit.

@SDGGiesbrecht
Copy link
Contributor

SDGGiesbrecht commented Mar 9, 2018

@driver733,

If a class is a subclass, it will have super at least somewhere, for instance, in one of the constructors.

@marcelofabri is correct.

This compiles:

class BaseClass {}
class Subclass : BaseClass {}

So does this:

class BaseClass {
    var baseProperty: Any? = nil
    init() {}
}
class IntermediateSubclass : BaseClass {
    var addedProperty: Any
    init(argument: Any) {
        addedProperty = argument
    }
}
class FinalSubclass : IntermediateSubclass {
    var anotherAddedProperty: Any? = 0
}

I do not really see the value in prohibiting subclassing. Yes, protocols are a better solution in many cases, but they cannot do everything a class cluster can. For example, you cannot store an array of SomeProtocol where AssociatedType == SomeType, but you can store an array of BaseClass<SomeType>. The developer really needs to understand for himself when to use which.

Most Swift developers work with Cocoa and CocoaTouch, or on Linux the Foundation subset. All of these contain many types specifically designed to be subclassed.

However, if you really want to discourage the subclassing paradigm, you need to target the bottom of the class tree instead. That means instead of punishing the client developer for using the tools he has been given, it would be better to discourage the vending developer from designing a subclass‐based API in the first place. To that end you could conceivably make an effective rule to prohibit defining a class without marking it as final (but possibly allowing it when the class is already a subclass of something else, in which case subclassing is part of a lower, unalterable design).

@driver733
Copy link
Author

driver733 commented Mar 9, 2018

@SDGGiesbrecht

Please read this, this and this.

I agree with you. I think there could be two rules. One that prohibits subclassing and another one that requires marking new classes as final.

In the environment where certain classes of the libraries and frameworks are not final, the first rule encourages the creation of new classes and interfaces instead of creating subclasses.

The second rule would encourage creating and designing new classes as final ones.

@SDGGiesbrecht
Copy link
Contributor

SDGGiesbrecht commented Mar 9, 2018

@driver733,

Please read this and this.

You can always trust me to read something the first time. 😉

Those articles do a good job of highlighting some misuses of the subclassing paradigm, but there are other cases when it is the best (or only) tool in the box. Both articles are written about Java, and Swift is simply not the same; maybe that is a cause of confusion.

In the environment where certain classes of the libraries and frameworks are not final, the first rule encourages the creation of new classes and interfaces instead of creating subclasses.

If you want to redesign UIViewController as a protocol, no one will stop you. I just do not want to recommend to every single iOS developer that they should go to that amount of work.

Apple writes:

Subclassing Notes

Every app contains at least one custom subclass of UIViewController. More often, apps contain many custom view controllers. Custom view controllers define the overall behaviors of your app, including the app’s appearance and how it responds to user interactions. The following sections provide a brief overview of some of the tasks your custom subclass performs. For detailed information about using and implementing view controllers, see View Controller Programming Guide for iOS.

@driver733
Copy link
Author

driver733 commented Mar 9, 2018

@SDGGiesbrecht
I agree with you that the nature of iOS Development requires the usage of subclasses. However, since this rule is opt-in, I believe that it could be kept. The more rules we have the better. If a rule is controversial then it can just be made opt-in.

By the way, here is an article on this topic in Swift.

@Jeehut
Copy link
Collaborator

Jeehut commented Feb 25, 2019

@driver733 I do agree that we should include even controversial rules as long as they add value to some Swift-based projects and improve the actual purpose of the Swift programming language. But this rule – if I understand it correctly – basically says: Swift has a feature that shouldn't be used and this rule makes sure you don't use it.

In my opinion, while there should be rules that enforce different code styles even if they are controversial like you suggested in #2075 and #2077, I don't think we should add rules that are diametrical to Swift's own design. If anything at all, we should find cases where subclassing isn't the best solution and other solutions should be preferred. But putting subclassing under general suspicion and warning against it by default simply sounds like a rule not helping anybody. Linters shouldn't be used like this, this would lead to way too many false positives to be considered useful.

@jpsim
Copy link
Collaborator

jpsim commented Apr 7, 2019

Very well said @Dschee, you captured my own feelings about this better than I could have 😊.

Closing.

@jpsim jpsim closed this as completed Apr 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

No branches or pull requests

5 participants