Enforce explicit self

[czechboy0]

Due to the Swift evolution proposal 009 being rejected, but still having a pretty big following, I think it'd be very valuable for certain teams to be able to enforce explicit self (like in Objective-C). Here's the reasoning for not changing the language from the core team and here's the proposal itself, where you can find reasons why the proposed change makes sense for some teams.

The gist of the decision was that explicit self makes sense for a certain group of people and those should use a lint tool (suggested by the core Swift team) instead of the language being changed. Thus I think SwiftLint supporting this rule would help it to be used by all those devs.

[NachoSoto]

I'd love this, even if it's not enabled by default.

[czechboy0]

Yeah I'm assuming people would cherry-pick it into their SwiftLint config file only if they wanted it enforced in their project. Unfortunately I'm not yet familiar enough with SwiftLint myself so I'm hoping for someone experienced to at least chime in about how difficult it'd be to add. (Also I'm studying for exams atm, so this time right here officially counts as procrastination.)

[keith]

We also have this in our style guide and would love to see this in swiftlint. I'll look into an approach for this today.

[jpsim]

I'd love to see this as an opt-in rule.

[czechboy0]

Are opt-in rules already supported by SwiftLint?

[keith]

No they're not. That's something that's come up multiple times before and
We have been meaning to add. Pull requests welcome!

On Wed, Jan 6, 2016 at 08:26 Honza Dvorsky notifications@github.com wrote:

Are opt-in rules already supported by SwiftLint?

—
Reply to this email directly or view it on GitHub
#321 (comment).

[czechboy0]

Understood. Does that mean that this issue will be blocked until opt-in rules are implemented? I'd be happy to use the rule from a feature branch in the meantime if so.

[keith]

It definitely would be. We wouldn't be able to ship this on by default.

[jpsim]

It would be trivial to introduce opt-in rules.

1. Introduce an OptInType protocol to tag opt-in rules
2. Make opt-in rules conform to OptInType
3. In Configuration's initializer, filter out opt-in rules that haven't been explicitly added to enabled_rules in the YAML file

[AliSoftware]

Just to throw in some ideas for the configurable elements:

• users might choose to activate either the "enforce self" or "avoid self" rule (so rule either Off, Enforce mode, or Avoid mode)
• could make sense to have different rules to differentiate properties and methods (like enforce self on properties but not on method calls). Some might even want to have a different setting between public and private funds but I'm not sure that's worth going that far

I'd personally enforce self on properties but not on method calls, but I know others have different conventions so this way each team could configure to their liking.

EDIT: Maybe the OptInType protocol (only On/Off) is not the best way to go, maybe we should instead introduce the concept of "modes" (Off/Enforce/Avoid) — and make those rule have the Off mode by default (so one add to change the mode in the config file explicitly)?

[scottrhoyt]

@AliSoftware, what advantages do you see with the rule modes vs. the current approach of disabled_rules, opt_in_rules, and whitelist_rules?

FWIW, I don't like the idea of separating properties vs. method calls in their enforcement of self. It seems too inconsistent. But to be fair, I'm also against explicit self for the same reasons that the Swift team stated. extension already has trained me to use IDE features to discover the scope of a particular definition when I'm uncertain. But I recognize that a this is a stylistic choice and will differ from team to team, so I'd like to see a self supporter add this as an opt-in rule.

[AliSoftware]

@scottrhoyt i don't think opt_in_rules cover all the cases.

If you don't list the enforce_self rule in the opt-in rules list, no check will be performed at all and the code could contain both self and not self without SwiftLint warning the user.

If you do list the enforce_self rule, a missing self in the code will generate a warning of a self is missing.

But what if the convention on your team is to NOT use self, and so you want SwiftLint to generate a warning if there is a self whereas it wasn't needed? With opt_in_rules there is no way to configure SwiftLint for that.

[czechboy0]

Isn't that a completely different rule? I understand they both touch self,
but I think you want something like enforce_minimal_self, which should be a
completely separate opt in rule IMO. That shouldn't change how this rule
works.
On Thu, Feb 4, 2016 at 9:14 AM AliSoftware notifications@github.com wrote:

@scottrhoyt https://github.com/scottrhoyt i don't think opt_in_rules
cover all the cases.

If you don't list the enforce_self rule in the opt-in rules list, no
check will be performed at all and the code could contain both self and not
self without SwiftLint warning the user.

If you do list the enforce_self rule, a missing self in the code will
generate a warning of a self is missing.

But what if the convention on your team is to NOT use self, and so you
want SwiftLint to generate a warning if there is a self whereas it wasn't
needed? With opt_in_rules there is no way to configure SwiftLint for
that.

—
Reply to this email directly or view it on GitHub
#321 (comment).

[AliSoftware]

@czechboy0 But then we'll have two conflicting rules, which seems strange especially whereas they treat the exact same problem. So their implementation will probably be the same, just a self_is_present == true vs. a self_is_present == false…

And also with 2 conflicting rules instead of one configurable unified rule, what if the user opt in for both rules, enforce_self and enforce_minimal_self?!

[czechboy0]

Yes they'd conflict if you put them both in, but I don't really see that as
a problem, since they're opt in.

Maybe we're just over engineering this. I think a completely simple, naive
version of enforce self should be implemented first and we'll work from
there. That's what all I and many people need. I'm absolutely for adding
functionality, but I think that'd be better to do once we have something
working already, especially since the discussion/implementation ratio is
pretty high already for this rule :)
On Thu, Feb 4, 2016 at 9:30 AM AliSoftware notifications@github.com wrote:

@czechboy0 https://github.com/czechboy0 But then we'll have two
conflicting rules, which seems strange especially whereas they treat the
exact same problem. So their implementation will probably be the same, just
a self_is_present == true vs. a self_is_present == false…

And also with 2 conflicting rules instead of one configurable unified
rule, what if the user opt in for both rules, enforce_self and
enforce_minimal_self?!

—
Reply to this email directly or view it on GitHub
#321 (comment).

[AliSoftware]

WFM! :)

[charlieMonroe]

Is there any update on this? I have tried looking into the source code, but can't figure out how to implement this rule. Any ideas?

[IanKeen]

Let me know if I've missed any test cases 👍

[charlieMonroe]

@IanKeen - looks amazing! BTW would it be possible to add an option not to apply this rule on variables with a _ prefix?

I don't think I'm alone who prefixes private instance variables with underscore, which lets you easily identify them in the code - for those I don't mind omitting the explicit self since local scope variables are not prefixed with an underscore so any bugs resulting from shadowing are unlikely.

Example:

class MyClass {
    var _foo: Int = 0
    var bar: Int = 0

    func baz() {
        _foo = 3 // OK
        bar = 4 // Not OK
    }
}

[3lvis]

@charlieMonroe Interesting. I would use self._foo = 3 as well to keep things consistent.

[IanKeen]

🤔 I'm inclined to agree with @3lvis - this seems a counter intuitive practise to what addingself. represents in terms of avoiding this class of bug.

But this wouldn't be hard to add if people want it 😄

[charlieMonroe]

The whole rationale behind the explicit self is to prevent accidental use of instance members. You are, however, unlikely (I never do it) to use an underscore-prefixed variable in a local scope - which is my justification for allowing access to underscored variables without explicit self - you always know that you are accessing a private instance variable.

This may be my old habits from ObjC, though:

@interface MyClass: NSObject
@property NSInteger foo;
@end

@implementation MyClass {
    NSInteger _bar;
}

-(void)baz {
    _bar = 3;
    self.foo = 4;
}
@end

You can potentially address _bar as self->_bar, but in practice you simply use _bar = 3, but for foo, you need to explicitly state self.

As they say - old habits die hard... Sure, I could fork this project and from what I've looked at the code, it's a one-line change, but I thought perhaps more people would benefit from this.

[IanKeen]

Yeah it is a simple addition. I've got a few cases to cater for which I'm
gonna look into this weekend hopefully. I'll take stab at that too :)

On Friday, October 7, 2016, charlieMonroe notifications@github.com wrote:

The whole rationale behind the explicit self is to accidental use of
instance members. You are, however, unlikely (I never do it) to use an
underscore-prefixed variable in a local scope - which is my justification
for allowing access to underscored variables without explicit self - you
always know that you are accessing a private instance variable.

This may be my old habits from ObjC, though:

@interface MyClass: NSObject
@Property NSInteger foo;
@EnD

@implementation MyClass {
NSInteger _bar;
}

-(void)baz {
_bar = 3;
self.foo = 4;
}
@EnD

You can potentially address _bar as self->_bar, but in practice you
simply use _bar = 3, but for foo, you need to explicitly state self.

As they say - old habits die hard... Sure, I could fork this project and
from what I've looked at the code, it's a one-line change, but I thought
perhaps more people would benefit from this.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#321 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABEPPagT9bXG0cjSowd3OE6a0PLBEVZCks5qxzZsgaJpZM4G_qRR
.

[charlieMonroe]

Thanks, Ian!

[NachoSoto]

I would use self._foo = 3 as well to keep things consistent.

Yeah, what some of you have described is not general enough to make it part of the rule for everyone in my opinion 👎

[jpsim]

If anything, making an exception for underscored members should be either a rule configuration option or not supported at all.

[eneko]

One thing to keep in mind is that Swift and SwiftLint can be used on Server Side Swift and non Cocoa environments. Following Cocoa standards, like underscore prefixes (which was short of a hack to hide private properties and methods) should be discouraged by default.

[charlieMonroe]

@eneko - I honestly do not see why. When I come to someone else's code and see an identifier starting with an underscore, I immediately know that it's a private instance member. Otherwise you need to look it up, mainly when one doesn't use explicit self. E.g. in git diffs, I find this very useful. I personally think that it improves readability of the code and it visibly separates private members from public.

But I fully understand that you have a different opinion on this matter - code style is often something very personal and can be a source of endless fights.

Nevertheless, I don't think that "should be discouraged by default" is absolutely true - is there any particular reason on your mind? If such a coding style makes sense to one and helps him more easily understand the code, why not? Since the identifiers are not public, the API doesn't get exported, so I don't see any harm - I personally only see benefits.

[eneko]

Languages and frameworks over the years develop habits. Some are good and some are bad. Just because we were used to do things like that in Objective-C does not mean we should follow the same practices in Swift (even if the platform we are developing on is the same).

Using prefixes or suffixes to indicate the visibility of methods or properties is a bad idea because:

1. It is not checked by the compiler (having an underscore prefix does not guarantee the property is private).
2. It adds technical debt
3. You can easily Option+Click or Command+Click any property or method to learn their signature. The name should not imply neither the type nor the visibility.
4. Apple used underscores in ObjC as a hack to hide instance variables in favor of using properties in ObjC classes. It was a way to prevent confusion and naming collisions between the ivar and the property.

In summary, I does not belong to SwiftLint to encourage or support bad programming practices, even if those rules were optional.

[woolsweater]

@eneko Using underscore prefixes on names to denote privacy was not invented by Apple and is not even remotely exclusive to Objective-C or Cocoa. It's a not-uncommon practice in Javascript, Python, Ruby, C, C++, C#, Perl, Java...

It's also used regularly by the Swift stdlib itself. See, e.g. https://github.com/apple/swift/blob/master/stdlib/public/core/ArrayBody.swift#L21

[charlieMonroe]

@eneko

1. It is not checked by the compiler (having an underscore prefix does not guarantee the property is private).

A lot of stuff isn't. A lot of bugs could have been prevented by explicit self in my codebase. Just because it's not checked by the compiler doesn't make it a wrong practice.

When you forget to add the (file)private keyword to the declaration, you can easily spot it in the exported APIs. Otherwise, it's easy to miss you've provided access to a private member.

2. It adds technical debt

I'm sorry, could you elaborate?

3. You can easily Option+Click or Command+Click any property or method to learn their signature.

In one post, you mention that "Swift and SwiftLint can be used on Server Side Swift and non Cocoa environments" and then you mention an Xcode feature (that works about 20% of the time in my experience). IDE is not (and shouldn't be) part of the language nor code style. Not everyone is using Xcode (Linux) and in many cases, you look into diffs on Github/elsewhere for code review.

The name should not imply neither the type nor the visibility.

So you'd prefer var name: NSTextField!? Over nameTextField? That is confusing!

4. Apple used underscores in ObjC as a hack to hide instance variables in favor of using properties in ObjC classes. It was a way to prevent confusion and naming collisions between the ivar and the property.

This dates waaaay before properties. I've started with ObjC a few years before ObjC 2.0.

[jpsim]

Let's please keep it civil in here.

SwiftLint's role is to help make projects a bit more stylistically consistent, rather than enforce one philosophy over another.

Since there's clearly some differences of opinion, we should strive to make SwiftLint accommodate both these cases and move on. I think this can reasonably be done by making an "explicit self" rule configurable on whether or not underscore-prefixed members should be exempt.

[DevAndArtist]

Any progress on this? I'd love to force self. in my codebase whenever possible. :)

[Uncommon]

Is it possible to do this in a class extension that's in a different file? Does SwiftLint work file-by-file, or can it handle things across files?

[micnguyen]

How's this looking? My team has agreed to omit the implied self, having settings that accomodate both this and the explicit self requirement would be great.

[sindresorhus]

I would also like to see an option to only enforce self in initializers. I generally prefer using implicit self, but in initializers, it makes more sense to use self for consistency, as some properties might need self to differentiate them from the initializer arguments.

To illustrate, I would like to enforce:

class Unicorn {
	init(foo: String, goat: String) {
		self.foo = foo
		bar = goat
	}
}

To be:

class Unicorn {
	init(foo: String, goat: String) {
		self.foo = foo
		self.bar = goat
	}
}

---

Continued in #2436.

[jpsim]

This was built in #2379.