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

Attributes with arguments lead to false positives #4843

Closed
1 of 2 tasks
rist opened this issue Mar 28, 2023 · 6 comments · Fixed by #4855
Closed
1 of 2 tasks

Attributes with arguments lead to false positives #4843

rist opened this issue Mar 28, 2023 · 6 comments · Fixed by #4855
Assignees
Labels

Comments

@rist
Copy link

rist commented Mar 28, 2023

New Issue Checklist

Describe the bug

lines like @Persisted(primaryKey: true) var id: Int are marked with Attributes Violation: Attributes should be on their own lines in functions and types, but on the same line as variables and imports (attributes), but lines like @Persisted var reelId: Int are (correctly) recognised as fine

Environment

  • SwiftLint version 0.51.0
  • Installation method used: Homebrew
  • Paste your configuration file:
disabled_rules: # rule identifiers to exclude from running
  - conditional_returns_on_newline # lots of warnings 
  - file_length
  - force_cast
  - force_try
  - identifier_name
  - line_length
  - todo
  - trailing_whitespace

opt_in_rules:
  - anyobject_protocol
  - array_init
  - attributes
  - closure_end_indentation
  - closure_spacing
  - collection_alignment
  - comma_inheritance
  - computed_accessors_order
  - conditional_returns_on_newline
  - contains_over_filter_count
  - contains_over_filter_is_empty
  - contains_over_first_not_nil
  - contains_over_range_nil_comparison
  - control_statement
  - convenience_type
  - deployment_target
  - discarded_notification_center_observer
  - discouraged_assert
  - discouraged_direct_init
  - discouraged_none_name
  - discouraged_object_literal
  - empty_collection_literal
  - empty_count
  - empty_string
  - empty_xctest_method
  - enum_case_associated_values_count
  - explicit_init
  - fallthrough
  - fatal_error_message
  - file_header
  - file_name_no_space
  - first_where
  - flatmap_over_map_reduce
  - identical_operands
  - implicit_return
  - joined_default_parameter
  - last_where
  - legacy_multiple
  - legacy_random
  - let_var_whitespace
  - lower_acl_than_parent
  - modifier_order
  - nimble_operator
  - operator_usage_whitespace
  - optional_enum_case_matching
  - prefer_self_in_static_references
  - prefer_self_type_over_type_of_self
  - prefer_zero_over_explicit_init
  - private_action
  - private_outlet
  - private_subject
  - prohibited_super_call
  - raw_value_for_camel_cased_codable_enum
  - redundant_nil_coalescing
  - redundant_type_annotation
  - required_enum_case
  - return_value_from_void_function
  - self_binding
  - single_test_class
  - sorted_first_last
  - sorted_imports
  - static_operator
  - switch_case_on_newline
  - toggle_bool
  - unavailable_condition
  - unneeded_parentheses_in_closure_argument
  - untyped_error_in_catch
  - vertical_parameter_alignment
  - vertical_parameter_alignment_on_call
  - vertical_whitespace_closing_braces
  - weak_delegate
  - yoda_condition
  - shorthand_optional_binding

analyzer_rules:
  - capture_variable
  - typesafe_array_init
  - unused_declaration
  - unused_import

identifier_name:
  excluded:
    - id

cyclomatic_complexity:
  warning: 13

# parameterized rules are first parameterized as a warning level, then error level.
type_body_length:
  - 300 # warning
  - 400 # error

nesting:
  type_level: 3
  • Are you using nested configurations?
    If so, paste their relative paths and respective contents.
  • Which Xcode version are you using: Xcode 14.3 Build version 14E222b
// This triggers a violation:
@Persisted(primaryKey: true) var id: Int
@SimplyDanny SimplyDanny self-assigned this Mar 28, 2023
@SimplyDanny SimplyDanny added help and removed bug labels Mar 28, 2023
@SimplyDanny
Copy link
Collaborator

The current implementation is like, when an attribute has arguments it should be on a separate line. You can configure that with the option always_on_same_line, however.

@sdc-78
Copy link

sdc-78 commented Mar 29, 2023

This is a breaking change from 0.50.3.
Also, it is incompatible with SwiftFormat which does not allow to depend on whether the attribute has arguments or not.

@SimplyDanny
Copy link
Collaborator

Not really. It has always been intended like that. However, the rule contained a bug that prevented custom (not built in) attributes to be ignored. @Persisted is such an attribute that is now recognized as well.

@geertbleyen
Copy link

geertbleyen commented Mar 31, 2023

In first instance maybe better then to update the message given, indicating that this is expected behavior for attributes with arguments.
Later, it would be nice if this behavior can be configured via the .swiftlint.yml config file

@edorphy
Copy link

edorphy commented Mar 31, 2023

These main concepts in SwiftUI apps are flagging too. Seems like this will have to be a rule I temporary turn off until customization is allowed in detail.

  • @UIApplicationDelegateAdaptor(ApplicationDelegate.self) var applicationDelegate: ApplicationDelegate
  • @SceneStorage("selectedTabView") var selectedTabView: String?
  • @FetchRequest(....) & @SectionedFetchRequest
  @FetchRequest(
      entity: EntityType.entity(),
      sortDescriptors: [
          NSSortDescriptor(keyPath: \EntityType.name, ascending: true)
      ],
      predicate: .truePredicate(),
      animation: nil
  ) var entities: FetchedResults<EntityType>

@SimplyDanny
Copy link
Collaborator

#4855 adds a new option attributes_with_arguments_always_on_line_above (true by default) which can be set to false to allow attributes with arguments on the same line like the variable declaration.

Besides that, the existing options always_on_same_line and always_on_line_above remain usable for more fine grained configuration of specific attributes.

@edorphy: The rule considers your attribute to be on the same line with the variable declaration because the closing parenthesis is. With the new option set to false it wouldn't violate anymore. Another option is to write it like

@FetchRequest(
      // ...
)
var entities: FetchedResults<EntityType>

if your style guide allows.

Are people happy with this new option? Opinions?

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