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

Regex for custom rule not matching (works in xcode search, not from linting) #2040

Closed
2 tasks done
bryantv opened this issue Feb 5, 2018 · 14 comments
Closed
2 tasks done

Comments

@bryantv
Copy link

bryantv commented Feb 5, 2018

New Issue Checklist

Bug Report

Complete output when running SwiftLint, including the stack trace and command used

I've created regex to determine if a class has an empty line following it. When searching in xcode, this works perfectly. However, swiftlint does not seem to work properly.

Environment

  • SwiftLint version 0.24.2
  • Installation method used CocoaPods
  • Paste your configuration file:
#disabled_rules: # rule identifiers to exclude from running
#opt_in_rules: # some rules are only opt-in
whitelist_rules:
  - class_delegate_protocol
  - closing_brace
  - closure_end_indentation
  - closure_parameter_position #debatable
  - comma
  - colon
  - control_statement
  - custom_rules
  - empty_count #error by default
  - empty_parameters
  #- empty_parentheses_with_trailing_closure # debatable
  - explicit_init # debatable
  - file_length
  - identifier_name # can't use because of _variable standard.  https://stackoverflow.com/questions/32192828/name-convention-for-unwrapped-value-in-swift
  - line_length
  - large_tuple
  - leading_whitespace
  - legacy_constant
  - literal_expression_end_indentation
  - mark
  - multiline_arguments
  - multiline_parameters
  # - multiple_closures_with_trailing_closure
  - opening_brace #debatable
  - operator_usage_whitespace
  - protocol_property_accessors_order
  - redundant_discardable_let
  - redundant_optional_initialization
  - redundant_string_enum_value
  - redundant_void_return
  - return_arrow_whitespace
  - shorthand_operator
  - statement_position
  - superfluous_disable_command	# not sure
  - switch_case_alignment
  - syntactic_sugar
  - trailing_newline
  - trailing_semicolon
  - type_body_length
  - type_name
  - unneeded_break_in_switch
  - vertical_parameter_alignment # dateable
  - vertical_parameter_alignment_on_call # debateble
  - vertical_whitespace
  - weak_delegate

  # Find all the available rules by running:
  # swiftlint rules
included: # paths to include during linting. `--path` is ignored if present.
  - //omitted for privacy
excluded: # paths to ignore during linting. Takes precedence over `included`.
  - Carthage
  - Pods
  - Source/ExcludedFolder
  - Source/ExcludedFile.swift

# forced errors
class_delegate_protocol:
  severity: error
closure_parameter_position:
  severity: error
closure_end_indentation:
  severity: error
comma:
  severity: error
colon:
  severity: error
control_statement:
  severity: error
empty_parameters:
  severity: error
empty_parentheses_with_trailing_closure:
  severity: error
large_tuple:
  severity: error
leading_whitespace:
  severity: error
legacy_constant:
  severity: error
legacy_constructor:
  severity: error
literal_expression_end_indentation:
  severity: error
mark:
  severity: error
multiline_arguments:
  severity: error
multiline_parameters:
  severity: error
multiple_closures_with_trailing_closure:
  severity: error
opening_brace:
  severity: error
weak_delegate:
  severity: error
operator_usage_whitespace:
  severity: error
protocol_property_accessors_order:
  severity: error
redundant_discardable_let:
  severity: error
redundant_optional_initialization:
  severity: error
redundant_string_enum_value:
  severity: error
redundant_void_return:
  severity: error
return_arrow_whitespace:
  severity: error
shorthand_operator:
  severity: error
statement_position:
  severity: error
switch_case_alignment:
  severity: error
syntactic_sugar:
  severity: error
trailing_newline:
  severity: error
trailing_semicolon:
  severity: error
unneeded_break_in_switch:
  severity: error
vertical_parameter_alignment:
  severity: error
vertical_parameter_alignment_on_call:
  severity: error
vertical_whitespace:
  severity: error


# configurable rules can be customized from this configuration file
# binary rules can set their severity level
force_cast: warning # implicitly
force_try:
  severity: warning # explicitly
# rules that have both warning and error levels, can set just the warning level
# implicitly
line_length:
  warning: 500
  error: 500

# they can set both implicitly with an array
type_body_length:
  - 600 # warning
  - 600 # error
# or they can set both explicitly
file_length:
  warning: 500
  error: 1200
large_tuple:
  warning: 3
  error: 4
# naming rules can set warnings/errors for min_length and max_length
# additionally they can set excluded names
type_name:
  min_length: 4 # only warning
  max_length: # warning and error
    warning: 40
    error: 50
  excluded: iPhone # excluded via string
identifier_name:
  min_length: # only min_length
    error: 3 # only error
  excluded: # excluded via string array
    - id
    - URL
    - lhs
    - rhs
reporter: "xcode" # reporter type (xcode, json, csv, checkstyle, junit, html, emoji)


custom_rules:
  space_after_class:
    name: "No Space After Class"
    message: "Empty line required after class declarations"
    regex: '^(open|internal|private|public)*class\s(?!func).*\{$\n(?!^\s*$)'
    severity: error

  • Which Xcode version are you using (check xcode-select -p)? 9.2
  • Do you have a sample that shows the issue?

Example of search in xcode:
screen shot 2018-02-02 at 4 55 31 pm

Example of linting (notice the blank line- this is not found by the same regex using ctrl+f):
screen shot 2018-02-02 at 4 58 01 pm

@ornithocoder
Copy link
Contributor

I believe the regex is loaded into SwiftLint as a String/NSString. You might have to escape the backslash character; try '^(open|internal|private|public)*class\\s(?!func).*\\{$\\n(?!^\\s*$)' instead.

@marcelofabri
Copy link
Collaborator

Can you provide a sample file? I've tried with this snippet and it triggered the violation:

class Test {
    let x: Int
}

@bryantv
Copy link
Author

bryantv commented Feb 5, 2018

The negative case also triggers the violation, though. If you enter a blank line between the class declaration and the property declaration, it still catches it (also try searching in xcode; works properly there)

// Should error
class Test {
    let x: Int
}

// Should not error
class Test {

    let x: Int
}

@bryantv
Copy link
Author

bryantv commented Feb 5, 2018

@ornithocoder thanks for the suggestion, though the rest of the regex wouldn't work if that was the case; it's extremely close to working, but it seems something is failing in checking for that next line.

@marcelofabri
Copy link
Collaborator

@bryantv I still only get one violation with that example 🤔

Can you try running swiftlint from the command line?

@bryantv
Copy link
Author

bryantv commented Feb 5, 2018

@marcelofabri same results, it finds both the positive and negative case.

@bryantv
Copy link
Author

bryantv commented Feb 5, 2018

@marcelofabri it sounds like it's working correctly for you? same config file/xcode version? I'm wondering if the logic is correct and I have an environmental issue...

@ornithocoder
Copy link
Contributor

ornithocoder commented Feb 6, 2018

@bryantv you're right. I didn't realize it's defined using single quotation marks instead of double quotation marks. No need to escape the backslash character.

The logic seems right:

screen shot 2018-02-06 at 9 45 26 am

Maybe SwiftLint doesn't handle multiline regex?

@marcelofabri
Copy link
Collaborator

@bryantv This is the configuration file I'm using:

whitelist_rules:
  - custom_rules

custom_rules:
  space_after_class:
    name: "No Space After Class"
    message: "Empty line required after class declarations"
    regex: '^(open|internal|private|public)*class\s(?!func).*\{$\n(?!^\s*$)'
    severity: error
$ swiftlint lint --path file.swift
Loading configuration from '.swiftlint.yml'
Linting Swift files at path file.swift
Linting 'file.swift' (1/1)
<redacted>/file.swift:2:1: error: No Space After Class Violation: Empty line required after class declarations (space_after_class)
Done linting! Found 1 violation, 1 serious in 1 file.

I'm using Xcode 9.2.

@bryantv
Copy link
Author

bryantv commented Feb 12, 2018

@marcelofabri That's actually an interesting idea; perhaps one of the other rules in interfering with mine somehow. I'll do some experimenting and report back.

@bryantv
Copy link
Author

bryantv commented Feb 13, 2018

@marcelofabri Afraid using that exact config and linting yielded the same results - both cases fire. Could there be something related to project layout? spaces vs. tabs, anything else you can think of?

@ornithocoder
Copy link
Contributor

Hi :-) I still believe it's given the multiline nature of the rule. For the examples below I created four files with different code combinations:

01_file.swift: Single test that should throw a violation

// Should error
class Test {
    let x: Int
}
Temp →  swiftlint lint --path 01_file.swift 
Loading configuration from '.swiftlint.yml'
Linting Swift files at path 01_file.swift
Linting '01_file.swift' (1/1)
01_file.swift:2:1: warning: No Space After Class Violation: Empty line required after class declarations (space_after_class)
Done linting! Found 1 violation, 0 serious in 1 file.

Results:

  • Expected: Violation
  • Got: Violation

02_file.swift: Single test that should not throw a violation

// Should not error
class Test {

    let x: Int
}
Temp →  swiftlint lint --path 02_file.swift 
Loading configuration from '.swiftlint.yml'
Linting Swift files at path 02_file.swift
Linting '02_file.swift' (1/1)
Done linting! Found 0 violations, 0 serious in 1 file.

Results:

  • Expected: No violation
  • Got: No violation

03_file.swift: Two classes where the first one should throw a violation

// Should error
class Test {
    let x: Int
}

// Should not error
class Test {

    let x: Int
}
Temp →  swiftlint lint --path 03_file.swift 
Loading configuration from '.swiftlint.yml'
Linting Swift files at path 03_file.swift
Linting '03_file.swift' (1/1)
03_file.swift:2:1: warning: No Space After Class Violation: Empty line required after class declarations (space_after_class)
Done linting! Found 1 violation, 0 serious in 1 file.

Results:

  • Expected: Violation on first class
  • Got: Violation on first class

04_file.swift: Two classes where the second one should throw a violation

// Should not error
class Test {

    let x: Int
}

// Should error
class Test {
    let x: Int
}
Temp →  swiftlint lint --path 04_file.swift 
Loading configuration from '.swiftlint.yml'
Linting Swift files at path 04_file.swift
Linting '04_file.swift' (1/1)
04_file.swift:2:1: warning: No Space After Class Violation: Empty line required after class declarations (space_after_class)
Done linting! Found 1 violation, 0 serious in 1 file.

Results:

  • Expected: Violation on second class
  • Got: Violation on first class

screen shot 2018-02-13 at 11 47 57 am

@jpsim
Copy link
Collaborator

jpsim commented Apr 23, 2018

Any reason to keep this open?

@marcelofabri
Copy link
Collaborator

Closing this due to lack of activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants