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 Rules to Support Ray Wenderlich's Swift Style Guide #319

Open
glennrfisher opened this Issue Jan 6, 2016 · 30 comments

Comments

Projects
None yet
@glennrfisher

glennrfisher commented Jan 6, 2016

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

  • Variable Names: Use camel case.
  • Function Names: Use camel case.
  • Class Names: Use upper camel case.
  • Function Arguments: Prefer named parameters, unless the context is very clear.
  • Enumerations: Use upper camel case.
  • Spacing: Indent using 2 spaces. Should be parametrizable, and probably 4 by default.
  • 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.
  • Access Modifiers: Don't add modifiers such as internal when they're already the default.
  • 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.
  • 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.
  • Closure Expressions: Use trailing closure syntax only if there's a single closure expression parameter at the end of the argument list.
  • Types: Prefer Swift's native types over Objective-C types (e.g. use Double instead of NSNumber).
  • 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 { ... }.)
  • Initializing CGGeometry: Use native Swift struct initializers (e.g. let bounds = CGRect(...) instead of let bounds = CGRectMake(...)).
  • CGGeometry: Prefer struct-scope constants CGRect.infinite, CGRect.null, etc. over global constants CGRectInfinite, CGRectNull, etc.
  • Syntactic Sugar: Prefer shortcut versions (var deviceModels: [String]) instead of the full generics syntax (var deviceModels: Array<String>).
  • For Loops: Prefer the for-in style instead of the for-condition-increment style.
  • Semicolons: Do not use semicolons after each statement (i.e. no trailing semicolons).
  • Semicolons: Avoid writing multiple statements on a single line separated with semicolons.

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

This comment has been minimized.

Show comment
Hide comment
@glennrfisher

glennrfisher Jan 6, 2016

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 commented Jan 6, 2016

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?

@aamctustwo

This comment has been minimized.

Show comment
Hide comment
@aamctustwo

aamctustwo Jan 11, 2016

Contributor

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

Contributor

aamctustwo commented Jan 11, 2016

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

@aamctustwo

This comment has been minimized.

Show comment
Hide comment
@aamctustwo

aamctustwo Jan 12, 2016

Contributor

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

Contributor

aamctustwo commented Jan 12, 2016

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

@jpsim

This comment has been minimized.

Show comment
Hide comment
@jpsim

jpsim Jan 12, 2016

Collaborator

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.

Collaborator

jpsim commented Jan 12, 2016

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 added a commit that referenced this issue Jan 14, 2016

Implement LegacyConstantRule
From #319

CGGeometry: Prefer struct-scope constants CGRect.infinite, CGRect.null,
etc. over global constants CGRectInfinite, CGRectNull, etc.

@jpsim jpsim added the rule-request label Jan 29, 2016

@mattgabor

This comment has been minimized.

Show comment
Hide comment
@mattgabor

mattgabor Jul 5, 2016

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

mattgabor commented Jul 5, 2016

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

@jpsim

This comment has been minimized.

Show comment
Hide comment
@jpsim

jpsim Jul 6, 2016

Collaborator

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

Collaborator

jpsim commented Jul 6, 2016

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

@masters3d

This comment has been minimized.

Show comment
Hide comment
@masters3d

masters3d Aug 26, 2016

Contributor

@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)

Contributor

masters3d commented Aug 26, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@masters3d

masters3d Aug 26, 2016

Contributor

related #57

Contributor

masters3d commented Aug 26, 2016

related #57

@kimdv

This comment has been minimized.

Show comment
Hide comment
@kimdv

kimdv Mar 25, 2017

@glennrfisher

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

kimdv commented Mar 25, 2017

@glennrfisher

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

@samrayner

This comment has been minimized.

Show comment
Hide comment
@samrayner

samrayner May 7, 2017

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 commented May 7, 2017

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?

@myworkout-jenkins

This comment has been minimized.

Show comment
Hide comment
@myworkout-jenkins

myworkout-jenkins Jun 21, 2017

@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 commented Jun 21, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@myworkout-jenkins

myworkout-jenkins Jun 21, 2017

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 commented Jun 21, 2017

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().

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Jun 21, 2017

Collaborator

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

Collaborator

marcelofabri commented Jun 21, 2017

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

@myworkout-jenkins

This comment has been minimized.

Show comment
Hide comment
@myworkout-jenkins

myworkout-jenkins Jun 21, 2017

@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 commented Jun 21, 2017

@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

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Jun 21, 2017

Collaborator

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

Collaborator

marcelofabri commented Jun 21, 2017

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

@atlassian-gaustin

This comment has been minimized.

Show comment
Hide comment
@atlassian-gaustin

atlassian-gaustin Aug 23, 2017

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 commented Aug 23, 2017

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).

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 23, 2017

Collaborator

@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.

Collaborator

marcelofabri commented Aug 23, 2017

@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.

@atlassian-gaustin

This comment has been minimized.

Show comment
Hide comment
@atlassian-gaustin

atlassian-gaustin Aug 23, 2017

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 commented Aug 23, 2017

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.

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 24, 2017

Collaborator

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

Collaborator

marcelofabri commented Aug 24, 2017

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

@atlassian-gaustin

This comment has been minimized.

Show comment
Hide comment
@atlassian-gaustin

atlassian-gaustin Aug 25, 2017

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 commented Aug 25, 2017

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.

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 25, 2017

Collaborator

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.

Collaborator

marcelofabri commented Aug 25, 2017

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.

@atlassian-gaustin

This comment has been minimized.

Show comment
Hide comment
@atlassian-gaustin

atlassian-gaustin Aug 28, 2017

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 commented Aug 28, 2017

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.

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 28, 2017

Collaborator

What do you mean by idiomatic rule?

Collaborator

marcelofabri commented Aug 28, 2017

What do you mean by idiomatic rule?

@atlassian-gaustin

This comment has been minimized.

Show comment
Hide comment
@atlassian-gaustin

atlassian-gaustin Aug 28, 2017

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 commented Aug 28, 2017

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.
@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 28, 2017

Collaborator

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

Collaborator

marcelofabri commented Aug 28, 2017

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

@atlassian-gaustin

This comment has been minimized.

Show comment
Hide comment
@atlassian-gaustin

atlassian-gaustin Aug 29, 2017

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

atlassian-gaustin commented Aug 29, 2017

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

@SPopenko

This comment has been minimized.

Show comment
Hide comment
@SPopenko

SPopenko Oct 23, 2017

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 commented Oct 23, 2017

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?

@jpsim

This comment has been minimized.

Show comment
Hide comment
@jpsim

jpsim Oct 23, 2017

Collaborator

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

Collaborator

jpsim commented Oct 23, 2017

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

@yeradis

This comment has been minimized.

Show comment
Hide comment
@yeradis

yeradis Jan 9, 2018

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 commented Jan 9, 2018

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?

@jpsim

This comment has been minimized.

Show comment
Hide comment
@jpsim

jpsim Jan 10, 2018

Collaborator

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.!

Collaborator

jpsim commented Jan 10, 2018

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.!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment