SwiftFormat icon indicating copy to clipboard operation
SwiftFormat copied to clipboard

Allow user to customize additional variable names to be replaced by self with the strongifiedSelf rule

Open krunk4ever opened this issue 6 years ago • 7 comments

Related: https://github.com/nicklockwood/SwiftFormat/issues/521

For the strongifiedSelf rule, it would be great if the user can customize an additional list of variable names to be auto-corrected to self (e.g. ss or strongSelf).

This PR introduces a new option for the strongifiedSelf rule which is a comma-delimited list of identifiers we want to replace with self.

krunk4ever avatar Jan 03 '20 12:01 krunk4ever

Coverage Status

Coverage increased (+0.003%) to 92.973% when pulling eb771ee6154670c92b481aae222cf435ed5023d8 on krunk4ever:update-strongified-self into f50fb3200d6dccdad8e0ea47458931bbc8bd0a70 on nicklockwood:master.

coveralls avatar Jan 03 '20 12:01 coveralls

This is a great idea, but I fear the implementation may be over-simplistic, because the user might have defined another variable with the same name that shadows the first, and this would end up replacing both. I'll add some tests and see if that's a problem in practice.

nicklockwood avatar Jan 03 '20 12:01 nicklockwood

@nicklockwood I added the option to pass in a comma delimited list of identifiers. e.g. my *.swiftformat file now includes the following line:

--strongselfids s,ss,sself,sSelf,strongself,strongSelf

krunk4ever avatar Jan 03 '20 12:01 krunk4ever

@nicklockwood I added the option to pass in a comma delimited list of identifiers. e.g. my *.swiftformat file now includes the following line:

--strongselfids s,ss,sself,sSelf,strongself,strongSelf

Is it common to use the guard let foo = self else { ... } pattern and not want to substitute "foo" for "self"? I think it might be unnecessary to specify a list of ids at all, and just always do the replacement.

In the rare case that it is needed, you could use // swiftformat:disable:next strongifiedSelf on the line above to preserve it. And if it turns out that there is a common use-case, perhaps it could be handled with a blacklist instead of a whitelist?

nicklockwood avatar Jan 04 '20 00:01 nicklockwood

So maybe you have a better way of detecting this, but we were having some trouble using regex to correctly identify some of the following edge cases, hence why we opted to start with a list of known variable names.

guard let foo = self else { return }

guard var foo = self else { return }

guard let x = y, let foo = self else { return }

guard var x = y, var foo = self else { return } 

guard let x = y, foo = self else { return } 

guard var x = y, foo = self else { return } 

And the following should be untouched

let foo = self

var foo = self

let x = y, foo = self

var x = y, foo = self

krunk4ever avatar Jan 04 '20 00:01 krunk4ever

@krunk4ever I don't think that should be a problem. The SwiftFormat parsing functions can handle recursive or iterative structures, so they're more flexible than regex.

nicklockwood avatar Jan 07 '20 08:01 nicklockwood

@krunk4ever I'm not sure this is actually valid Swift syntax, unless you're talking about support for Swift 2?

guard let x = y, foo = self else { return } 
guard var x = y, foo = self else { return }

nicklockwood avatar Jan 07 '20 08:01 nicklockwood