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: Blank Lines #1926

Open
mrcljx opened this issue Oct 26, 2017 · 2 comments
Open

Rule Request: Blank Lines #1926

mrcljx opened this issue Oct 26, 2017 · 2 comments
Labels
rule-request Requests for a new rules.

Comments

@mrcljx
Copy link
Contributor

mrcljx commented Oct 26, 2017

  1. Why should this rule be added? Share links to existing discussion about what the community thinks about this.

There are a few rule requests (#640 et al) to checking blank lines after curly braces,
and discussion often dries up because we can't get to common grounds.

I wonder whether that is because we are focusing too much on the curly braces.

Eclipse and IntelliJ have been focusing on the blank lines based on context (there's a generic max lines before closing curly brace though). While that change removes a bit of flexibility, configuration seems more manageable, and also would allow for incremental extension of this rule.

  1. Provide several examples of what would and wouldn't trigger violations.

Hard to provide examples since it needs to be quite flexible.

People seem to have different preferences on, whether the body of a class should start with an empty line or not.

I believe other things are less contested though.

  • File headers should have an empty line after them (however that can already be checked using the FileHeader rule)
  • Import statements should have blank likes around them.
  • Struct/Classes/... should have 1-2 blank lines around them.
  • Documentation comments should have a blank line before them.
  1. Should the rule be configurable, if so what parameters should be configurable?

A complex default configuration could look like this (not suggesting we need all of them right now):

blank_lines:
    before_file_body: 0! # ! indicating precedence, see comments below
    after_file_header: 1
    around_imports: 1 # note plural, referring to "all imports" (not each)
    around_import: 0
    before_comment: 1 # exception for swiftlint:disable:previous
    before_documentation: 1 # no after/around
    around_declaration_with_members: # shorthand for below:
    # around_class: 1
    # around_enum: 1
    # around_extension: 1
    # around_protocol: 1
    # around_struct: 1
    around_members: 1 # note plural, referring to "all members" in struct/class/protocol/extension/enum (not each)
    around_enum_case: 1
    around_protocol_subscript: 0
    around_subscript: 1
    around_protocol_method: 0
    around_function: 1 # also init/deinit
    before_function_body: 0
    before_multiline_lambda_body: 0
    around_protocol_property: 0
    around_property: 1 # shorthand for below:
    # around_simple_property: 1
    # around_property_with_body: 1 # shorthand for below:
    ## around_computed_property: 1
    ## around_property_with_accessors: 1
    before_closing_curly_brace: 0...1

Note: around is always a shorthand for before/after.

Things like around_protocol_property consider documentation being part of them,
so space before here means space before the documentation section. A conflict
with the documentation rule would be resolved by max(before_documentation, around_...).

Conflicts with previous elements are solved by precedence (indicated by !) and otherwise max.

Here's a valid example for the conflicting configuration around_members: 1, around_enum_case: 0, before_documentation: 2:

enum Foo {

    case bar


    /// The BAZ!
    case baz

}

Right now it does not handle things like if statement format checking,
but that should be easy to add later on.

  1. Should the rule be opt-in or enabled by default? Why?

Opt-In. While I believe we can find a default configuration that roughly follows the Swift code that Apple puts out in documentation/examples, the community hasn't arrived at a consistent style yet.

@iliaskarim
Copy link

Thanks for opening the issue, @sirlantis! What did you think of #1659?

We use a custom rule called trailing_newline_before_bracket that detects newlines before closing brackets. Because there is marginally more consensus on omitting a newline before a closing bracket than on omitting a newline after an opening bracket, it would be helpful to provide an opt-in rule that could autocorrect the newline before bracket. This would essentially be half of what @Dschee proposed in #1518.

One reason I suspect this feature request may be unfilled yet is conversation stalling around finding the perfect specification to meet everyone's needs, when a less-exhaustive set of options that fits the most common use case may be sufficient.

@marcelofabri marcelofabri added the rule-request Requests for a new rules. label Mar 27, 2018
@acecilia
Copy link
Contributor

Meanwhile, I managed to avoid blank lines after brackets with this, maybe somebody finds it useful. The trick is that (as far as I experienced) Swiftlint considers the . as any character, included new line. Thus, to make it work .* should be written as [^\n]*.

  avoid_empty_lines_after_opening_bracket:
    name: "Avoid empty lines after opening brackets"
    message: "No empty lines are allowed after opening brackets"
    regex: '\{[^\n\{\}]*\n\n'

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

4 participants