search

realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.67k stars 2.22k forks source link

`identifier_name` rule doesn't respect `allowed_symbols` configuration option for functions #1852

Closed fbartho closed 7 years ago

fbartho commented 7 years ago

Description

I created a method prefixed with double underscores. I had identifier_name rule enabled, and had the option of allowed_symbols: "_" enabled

Expectation: method would pass the identifier name check. Reality: method triggered a rule violation.

A casual reading of IdentifierNameRule shows that there is an if-block that handles name rules for functions, and this if-block does not reference the allowed_symbols list.

Reference: Line 77

New Issue Checklist

Bug Report

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint --path "./MyFile.swift"
Loading configuration from '.swiftlint.yml'
configuration error: 'comments_space' is not a valid rule identifier
configuration error: 'comments_capitalized_ignore_possible_code' is not a valid rule identifier
configuration error: 'comments_capitalized_find_possible_code' is not a valid rule identifier
configuration error: 'explicit_failure_calls' is not a valid rule identifier
configuration error: 'missing_docs' is not a valid rule identifier
configuration error: 'multiple_empty_lines' is not a valid rule identifier
Valid rule identifiers:
empty_count
trailing_semicolon
no_grouping_extension
statement_position
type_name
legacy_constant
multiple_closures_with_trailing_closure
protocol_property_accessors_order
nimble_operator
comma
legacy_cggeometry_functions
leading_whitespace
cyclomatic_complexity
control_statement
empty_parentheses_with_trailing_closure
dynamic_inline
no_extension_access_modifier
unused_optional_binding
closure_spacing
prohibited_super_call
explicit_top_level_acl
redundant_void_return
trailing_whitespace
mark
empty_parameters
shorthand_operator
colon
class_delegate_protocol
closure_parameter_position
nesting
switch_case_on_newline
file_header
for_where
compiler_protocol_init
superfluous_disable_command
operator_usage_whitespace
vertical_parameter_alignment
private_over_fileprivate
is_disjoint
private_outlet
discouraged_direct_init
generic_type_name
legacy_constructor
identifier_name
custom_rules
opening_brace
redundant_discardable_let
weak_delegate
redundant_string_enum_value
xctfail_message
redundant_optional_initialization
unused_enumerated
todo
explicit_type_interface
quick_discouraged_call
force_cast
unused_closure_parameter
number_separator
sorted_imports
trailing_closure
implicit_getter
unneeded_parentheses_in_closure_argument
force_unwrapping
function_body_length
type_body_length
operator_whitespace
object_literal
vertical_whitespace
large_tuple
explicit_enum_raw_value
implicitly_unwrapped_optional
closing_brace
implicit_return
legacy_nsgeometry_functions
notification_center_detachment
single_test_class
empty_enum_arguments
trailing_newline
conditional_returns_on_newline
joined_default_parameter
vertical_parameter_alignment_on_call
redundant_nil_coalescing
private_unit_test
pattern_matching_keywords
return_arrow_whitespace
attributes
overridden_super_call
block_based_kvo
fatal_error_message
first_where
strict_fileprivate
explicit_init
discarded_notification_center_observer
closure_end_indentation
function_parameter_count
syntactic_sugar
trailing_comma
let_var_whitespace
void_return
valid_ibinspectable
multiline_parameters
line_length
extension_access_modifier
force_try
file_length
Linting Swift files at path ./MyFile.swift
Linting 'MyFile.swift' (1/1)
./MyFile.swift:77:129: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
./MyFile.swift:212:60: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
./MyFile.swift:186:74: warning: Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)
./MyFile.swift:187:76: warning: Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)
./MyFile.swift:188:30: warning: Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)
./MyFile.swift:11:1: warning: Private over fileprivate Violation: Prefer `private` over `fileprivate` declarations. (private_over_fileprivate)
./MyFile.swift:12:1: warning: Private over fileprivate Violation: Prefer `private` over `fileprivate` declarations. (private_over_fileprivate)
./MyFile.swift:302:12: error: Identifier Name Violation: Function name should start with a lowercase character: '__presentLocationPermissionRequest(onDismiss:icon:title:message:primaryCallToAction:primaryCallback:secondaryCallToAction:secondaryCallback:postActivationCallback:)' (identifier_name)
./MyFile.swift:198:12: warning: Function Body Length Violation: Function body should span 40 lines or less excluding comments and whitespace: currently spans 62 lines (function_body_length)
./MyFile.swift:302:12: error: Function Parameter Count Violation: Function should have 5 parameters or less: it currently has 9 (function_parameter_count)
./MyFile.swift:301: warning: Line Length Violation: Line should be 180 characters or less: currently 183 characters (line_length)
Done linting! Found 11 violations, 2 serious in 1 file.

Environment

reporter: "xcode" # reporter type (xcode, json, csv, checkstyle, junit)

excluded: # paths to ignore during linting. Takes precedence over `included`.
  - Pods

# If you need help with rules run $ swiftlint 

opt_in_rules:
  - closure_end_indentation
  - closure_spacing
  - comments_space
  - comments_capitalized_ignore_possible_code
  - comments_capitalized_find_possible_code
  - empty_count
  - explicit_init
  - explicit_failure_calls
  - extension_access_modifier
  - fatal_error_message
  - first_where
  - force_cast
  - force_unwrapping
  - implicitly_unwrapped_optional
  - missing_docs
  - multiline_parameters
  - multiple_empty_lines
  - nesting
  - number_separator
  - operator_usage_whitespace
  - overridden_super_call
  - private_outlet
  - prohibited_super_call
  - protocol_property_accessors_order
  - redundant_nil_coalescing
  - sorted_imports
  - syntactic_sugar
  - vertical_parameter_alignment_on_call
  - vertical_whitespace

disabled_rules: # rule identifiers to exclude from running
  # - force_try
  # - class_delegate_protocol
  # - function_parameter_count
  # - function_body_length
  # - todo
  # - discarded_notification_center_observer
  # - conditional_returns_on_newline

cyclomatic_complexity:
  ignores_case_statements: true
  warning: 20

file_length:
  warning: 1000
  error: 1500

line_length: 
  warning: 180 
  error: 350
  ignores_comments: true
  ignores_urls: true

nesting:
  type_level:
    warning: 3
    error: 6
  statement_level:
    warning: 5
    error: 10

identifier_name:
  allowed_symbols: "_"
  min_length: 2
  max_length: 
    warning: 90
    error: 1000
  excluded:
    - id

implicitly_unwrapped_optional:
  mode: all_except_iboutlets

large_tuple:
  warning: 4
  error: 5

type_body_length:
  warning: 1000
  error: 1000

trailing_comma:
  mandatory_comma: true

number_separator:
  minimum_length: 8

private_outlet:
  allow_private_set: true

colon: 
  severity: error
comma: error
empty_count: error
legacy_constant: error
legacy_constructor: error
opening_brace: error
trailing_newline: error
trailing_semicolon: error

custom_rules:
  comments_space: # From https://github.com/brandenr/swiftlintconfig
    name: "Space After Comment"
    regex: '(^ *//\w+)'
    message: "There should be a space after //"
    severity: warning
  explicit_failure_calls:
      name: “Avoid asserting ‘false’”
      regex: ‘((assert|precondition)\(false)’
      message: “Use assertionFailure() or preconditionFailure() instead.”
      severity: warning
  multiple_empty_lines:
     name: "Multiple Empty Lines"
     regex: '((?:\s*\n){3,})'
     message: "There are too many line breaks"
     severity: error
marcelofabri commented 7 years ago

This is a bit tricky, because allowed_symbols was added to add exceptions for validating that identifiers only have alphanumeric characters.

I wonder what's the most predictable thing to do here. Just checking if the first character is in allowed_symbols might surprise someone too.

On a first look, I guess we should change the rule to validate that the first character is not an uppercase one (instead of enforcing it to be a lowercase).

marcelofabri commented 7 years ago

In fact, that's what the other rules that use NameConfiguration do, so I guess this is a bug.

marcelofabri commented 7 years ago

Turns out this is a duplicate of #1762.

lhunath commented 3 years ago

Note that this issue exists for type_name as well.