SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

Add Rules to Support Ray Wenderlich's Swift Style Guide

Open glennrfisher opened this issue 9 years ago • 32 comments

It would be great if SwiftLint supported Ray Wenderlich's Swift Style Guide:

  • [x] Variable Names: Use camel case.
  • [x] Function Names: Use camel case.
  • [x] Class Names: Use upper camel case.
  • [ ] Function Arguments: Prefer named parameters, unless the context is very clear.
  • [x] Enumerations: Use upper camel case.
  • [x] Spacing: Indent using 2 spaces. Should be parametrizable, and probably 4 by default.
  • [x] Opening Braces: Always open on the same line as the statement.
  • [ ] Closing Braces: Close on a new line.
  • [ ] Spacing Between Functions: Leave exactly one blank line between functions. This should probably also apply between structs, classes, etc. Should be parametrizable.
  • [ ] #5828
  • [ ] Access Modifiers: Don't repeat the access modifier when overriding a method.
  • [ ] Protocol Conformance: When adding protocol conformance to a class, prefer adding a separate class extension for the protocol methods.
  • [x] Computed Properties: If a computed property is read-only, omit the get clause. The get clause is only required when a set clause is provided.
  • [x] Closure Expressions: Use trailing closure syntax only if there's a single closure expression parameter at the end of the argument list.
  • [x] Types: Prefer Swift's native types over Objective-C types (e.g. use Double instead of NSNumber).
  • [x] Constants: Always use let instead of var if the value of the variable will not change. Supported by compiler.
  • [ ] Naming Optionals: Avoid names like optionalString or maybeView, since optionalness is already captured in the variable's type. (Prefer using the same name: if let subview = subview { ... }.)
  • [x] Initializing CGGeometry: Use native Swift struct initializers (e.g. let bounds = CGRect(...) instead of let bounds = CGRectMake(...)).
  • [x] CGGeometry: Prefer struct-scope constants CGRect.infinite, CGRect.null, etc. over global constants CGRectInfinite, CGRectNull, etc.
  • [x] Syntactic Sugar: Prefer shortcut versions (var deviceModels: [String]) instead of the full generics syntax (var deviceModels: Array<String>).
  • [x] For Loops: Prefer the for-in style instead of the for-condition-increment style.
  • [x] Semicolons: Do not use semicolons after each statement (i.e. no trailing semicolons).
  • [ ] #5829

Probably Not Worth It

  • Type Inference: Let the compiler infer the type for a constant or variable, unless you need a specific type other than the default (e.g. avoid let message: String = "Click the button."). -> There are probably many valid reasons to break this rule.
  • Self: Avoid using self except when required by the compiler or for disambiguating property names from arguments. -> This seems very difficult to verify.
  • Comments: Avoid block comments inline with code, as the code should be as self-documenting as possible. -> There are many excellent reasons to use block comments inline with code.
  • Spacing Within Functions: Too many sections means you should refactor. -> This is effectively satisfied by the function_body_length rule.
  • Prose (Comments): When referring to functions, include the required parameter names or _ for unnamed parameters. -> Not worthing trying to style-check comments.
  • Class Prefixes: Types are namespaced by the module that contains them, so class prefixes should not be added to member variables or functions. -> How could SwiftLint identify a class prefix?

glennrfisher avatar Jan 06 '16 04:01 glennrfisher

I'd love to try and help with as many of these as I can. Hopefully I'll be able to submit a pull request soon!

Any recommendations on a simple rule to start with? Would any of these be a particularly good first rule for me to try and implement?

glennrfisher avatar Jan 06 '16 04:01 glennrfisher

For Initializing CGGeometry, I believe this is already done as part of the Legacy Constructor rule.

ghost avatar Jan 11 '16 14:01 ghost

It may not be worth implementing "For Loops: Prefer the for-in style instead of the for-condition-increment style."

There is an accepted proposal for Swift 3 that eliminates this style of for loops. See:

https://github.com/apple/swift-evolution/blob/master/proposals/0007-remove-c-style-for-loops.md

ghost avatar Jan 12 '16 08:01 ghost

It may not be worth implementing "For Loops: Prefer the for-in style instead of the for-condition-increment style."

True, but on the other hand, if people writing Swift got a diagnostic for that with Swift 2.1.x, they might be more likely to adapt their code before 2.2 when the Swift compiler itself will have a deprecation diagnostic.

jpsim avatar Jan 12 '16 15:01 jpsim

Is this still in progress? Let me know if I can contribute.

mattgabor avatar Jul 05 '16 23:07 mattgabor

@mattgabor still lots of low hanging fruit here! Feel free to implement any of the non-checked items in the initial comment here.

jpsim avatar Jul 06 '16 00:07 jpsim

@glennrfisher could you update the ones already done here? For findability we should probably split them up into each separate rule (making sure there are not duplicates out there already)

masters3d avatar Aug 26 '16 06:08 masters3d

related https://github.com/realm/SwiftLint/issues/57

masters3d avatar Aug 26 '16 07:08 masters3d

@glennrfisher

"Function Names: Use camel case" and "Enumerations: Use upper camel case." is completed in pull request #954

kimdv avatar Mar 25 '17 10:03 kimdv

I'd love to have this one:

Protocol Conformance: When adding protocol conformance to a class, prefer adding a separate class extension for the protocol methods.

Is there a way to easily distinguish between class inheritance and protocol conformance in a class declaration? If so, I assume this rule would simply check for the latter?

samrayner avatar May 07 '17 08:05 samrayner

@glennrfisher It would be cool to have a map between the Ray Wenderlich rule and the actual rule name for swiftlint (possibly also with config options where applicable)

myworkout-jenkins avatar Jun 21 '17 19:06 myworkout-jenkins

Maybe this is a bug, but this function passed without any validation error (should be caught be identifier rule because of the underscore?): func some_ImProperNAmeedMethodToTriggerWarning().

myworkout-jenkins avatar Jun 21 '17 19:06 myworkout-jenkins

@myworkout-jenkins please open a new issue with more details (sample code, configuration file, SwiftLint version, etc)

marcelofabri avatar Jun 21 '17 19:06 marcelofabri

@marcelofabri I will, just wanted to get it confirmed wether it was a bug or not. I can provide a sample project setup etc. and post an issue if it indeed is a bug

myworkout-jenkins avatar Jun 21 '17 20:06 myworkout-jenkins

there's no way to say if it's a bug or not without a proper issue 😉

marcelofabri avatar Jun 21 '17 20:06 marcelofabri

Color me on the side of using the Swift language documentation rules for containers, Array and Dictionary<keyType, valueType> -- the syntactic sugar version of [type] and [keyType: valueType] are nice shortcuts, but there are times that they cannot be used (some closures).

atlassian-gaustin avatar Aug 23 '17 16:08 atlassian-gaustin

@atlassian-gaustin I'm not aware of any cases where that rule is giving false positives. The only case that it's not allowed to use the syntactic sugar version (AFAIK) is using something like Array<Int>.Index or Optional<T>.self, but the rule doesn't trigger.

marcelofabri avatar Aug 23 '17 19:08 marcelofabri

It's the compiler that occasionally barfs without using the "proper" definitions in those cases. The problem is that using the syntactic sugar everywhere BUT in those locations causes an inconsistency in the code between the definition and later usage. e.g. seeing [Int] everywhere EXCEPT where you must use Array<Int> because the sugar is invalid.

atlassian-gaustin avatar Aug 23 '17 22:08 atlassian-gaustin

Can give me an example where the compiler rejects the syntactic sugar version?

marcelofabri avatar Aug 24 '17 09:08 marcelofabri

Trying to dig up places in my own code where the syntactic sugar version would fail, but I tend to use the full description in my initializers (e.g.var arr = Array<Int>() instead of var arr: [Int] = []). I also use a lot of Sets as well in my code, and there is NO sugar version of those (because there is no Swift-native Set type). However, since you pointed out that you can't use syntactic sugar everywhere, that also points out why you should be re-think the rule about syntactic sugar being the default.

atlassian-gaustin avatar Aug 25 '17 21:08 atlassian-gaustin

I don't agree: you can use them almost in every case (and the rule won't trigger false positives). You can always disable a rule if you don't want to enforce it.

marcelofabri avatar Aug 25 '17 22:08 marcelofabri

The problem is that you either have the rule disabled, or use syntactic sugar when it can be used but you can never have just the idiomatic rule enabled -- because there is no option for just the idiomatic rule.

atlassian-gaustin avatar Aug 28 '17 16:08 atlassian-gaustin

What do you mean by idiomatic rule?

marcelofabri avatar Aug 28 '17 16:08 marcelofabri

The non-syntactic-sugar version. With only "on" or "off" for syntactic-sugar, you either have "use syntactic sugar everywhere you can" or "mix syntactic sugar with idiomatic, we don't care". You can never have "only use idiomatic form". e.g.

  • syntactic sugar on: [Int] and [String: Any] are valid but Array<Int> and Dictionary<String, Any> are not.
  • syntactic sugar off: [Int], [String: Any], Array<Int>, and Dictionary<String, Any> are all valid types.

There needs to be a "third state" for container types. As I envision it:

  • container_type off: [Int], [String: Any], Array<Int>, and Dictionary<String, Any> are all valid types.
  • container_type sugar: [Int] and [String: Any] are valid but Array<Int> and Dictionary<String, Any> are not.
  • container_type strict: Array<Int> and Dictionary<String, Any>are valid but [Int] and [String: Any] are not.

atlassian-gaustin avatar Aug 28 '17 17:08 atlassian-gaustin

@atlassian-gaustin I'd rather make that another (opt-in) rule. Can you open another issue to discuss it?

marcelofabri avatar Aug 28 '17 20:08 marcelofabri

They are linked -- what would the result be if both were set to on?

atlassian-gaustin avatar Aug 29 '17 16:08 atlassian-gaustin

Guys, is it work in progress. I see that some list items were marked as completed [x] but know associated commits in this Issue. How are things looks like on this day?

SPopenko avatar Oct 23 '17 12:10 SPopenko

@SPopenko If you're looking for something specific, you can see all implemented rules in Rules.md.

jpsim avatar Oct 23 '17 15:10 jpsim

sorry, but how is the indentation configuration supposed to work?

release 0.24.1: Dented Tumbler says: Indentation can now be specified via a configuration file.

But where is the doc that says how to use it?

yeradis avatar Jan 09 '18 08:01 yeradis

You can set an indentation key in the configuration file:

indentation: 4 # 4 spaces
indentation: 2 # 2 spaces
indentation: tabs # tabs

It'd be a great starter task to add this to the sample configuration in the readme.!

jpsim avatar Jan 10 '18 18:01 jpsim