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

SwiftLint is reporting less violations with Xcode 13.3 than with Xcode 13.2 #3975

Closed
2 tasks done
rd-david-wahlandt opened this issue May 12, 2022 · 4 comments · Fixed by #3977
Closed
2 tasks done
Labels
bug Unexpected and reproducible misbehavior.

Comments

@rd-david-wahlandt
Copy link

rd-david-wahlandt commented May 12, 2022

New Issue Checklist

Describe the bug

Our development team recently moved to MacBooks with M1 chip. Although SwiftLint runs and reports most errors, the multiline_arguments_brackets rule doesn't seem to work the same way as on Intel MacBooks.

Fine on M1, reporting warning on Intel:

views.append(ViewModel(title: "MacBook", subtitle: "M1", action: { [weak self] in
    print("action tapped")
}))

Warning on both M1 and Intel:

views.append(ViewModel(
    title: "MacBook", subtitle: "M1", action: { [weak self] in
    print("action tapped")
}))
Complete output when running SwiftLint, including the stack trace and command used
swiftlint
Linting Swift files in current working directory
[...]
Done linting! Found 0 violations, 0 serious in 1343 files.

Environment

  • SwiftLint version (run swiftlint version to be sure): 0.47.1
  • Installation method used (Homebrew, CocoaPods, building from source, etc): Homebrew
  • Paste your configuration file:
disabled_rules: # rule identifiers turned on by default to exclude from running
  - cyclomatic_complexity
  - file_length
  - force_cast
  - function_body_length
  - function_parameter_count
  - identifier_name
  - inclusive_language
  - large_tuple
  - line_length
  - nesting
  - todo
  - type_body_length
  - type_name
  - xctfail_message

opt_in_rules:
  - anyobject_protocol
  - array_init
  - capture_variable
  - closure_end_indentation
  - closure_spacing
  - collection_alignment
  - contains_over_filter_count
  - contains_over_filter_is_empty
  - contains_over_first_not_nil
  - contains_over_range_nil_comparison
  - discouraged_assert
  - discouraged_object_literal
  - empty_collection_literal
  - empty_count
  - empty_string
  - empty_xctest_method
  - expiring_todo
  - explicit_init
  - fallthrough
  - fatal_error_message
  - file_name_no_space
  - first_where
  - flatmap_over_map_reduce
  - ibinspectable_in_extension
  - identical_operands
  - implicit_return
  - implicitly_unwrapped_optional
  - joined_default_parameter
  - last_where
  - legacy_multiple
  - legacy_random
  - literal_expression_end_indentation
  - lower_acl_than_parent
  - modifier_order
  - multiline_arguments_brackets
  - multiline_literal_brackets
  - multiline_parameters
  - multiline_parameters_brackets
  - operator_usage_whitespace
  - optional_enum_case_matching
  - overridden_super_call
  - prefer_self_in_static_references
  - prefer_self_type_over_type_of_self
  - prefer_zero_over_explicit_init
  - private_action
  - private_subject
  - prohibited_super_call
  - raw_value_for_camel_cased_codable_enum
  - redundant_nil_coalescing
  - redundant_type_annotation
  - required_enum_case
  - single_test_class
  - sorted_first_last
  - static_operator
  - strong_iboutlet
  - toggle_bool
  - unneeded_parentheses_in_closure_argument
  - untyped_error_in_catch
  - unused_declaration
  - unused_import
  - vertical_whitespace_closing_braces
  - vertical_whitespace_opening_braces
  - weak_delegate
  - yoda_condition

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

deployment_target:
  iOS_deployment_target: 14.0

trailing_comma:
  mandatory_comma: true

empty_count:
  only_after_dot: true

custom_rules:
  unnecessary_type:
    name: "Unnecessary Type"
    regex: "[ a-zA-Z0-9]*(?:let|var) [ a-zA-Z0-9]*: ([a-zA-Z0-9]*)[\\? ]*= \\1"
    message: "Type Definition Not Needed"
    severity: error
  • Are you using nested configurations: No
    If so, paste their relative paths and respective contents.
  • Which Xcode version are you using (check xcodebuild -version):
    Xcode 13.3.1
    Build version 13E500a
// This should trigger a multiline_arguments_brackets violation:
views.append(ViewModel(title: "MacBook", subtitle: "M1", action: { [weak self] in
    print("action tapped")
}))
@SimplyDanny
Copy link
Collaborator

I can confirm that the examples behave indeed differently on Intel and Arm. While both of them report the issue on Intel, only the second one causes a warning on my local M1. In the second example, Intel even reports two warnings. In other words, the test for the examples added in #3977 are successful on my local M1, while on Intel they are apparently not.

The question is what the correct behavior shall actually be. In my opinion, the first example shouldn't report any issue while the second example should report one for the arguments of ViewModel only - like committed in #3977.

I'm quite lost, however, about potential reasons for the different behavior on Intel and Arm. It doesn't make any sense that the architecture has something to do with these kinds of issues. Any idea on how this can happen, @jpsim, @marcelofabri? Do you know of past similar issues?

@SimplyDanny SimplyDanny added the bug Unexpected and reproducible misbehavior. label May 14, 2022
@marcelofabri
Copy link
Collaborator

@SimplyDanny thanks for looking at this! I’d be surprised if this is really due to different architectures. Can you double check you’re using the same Xcode version as CI? I’ve seen sourcekit returning different data in different Swift toolchains before

@rd-david-wahlandt
Copy link
Author

rd-david-wahlandt commented May 15, 2022

Hey @SimplyDanny, @marcelofabri thank you for looking into this.
Good call on the Xcode version. It seems that SwiftLint with Xcode 13.3 is reporting less warnings as with Xcode 13.2.
Rules affected are as mentioned multiline_arguments_brackets and for_where, where the example below triggers in Xcode 13.2, but not in Xcode 13.3:

for subview in subviews {
    if !(subview is UIStackView) {
        subview.removeConstraints(subview.constraints)
        subview.removeFromSuperview()
    }
}

@SimplyDanny for the question of what should be correct I agree, that the first example should not trigger a warning, so the current (Xcode 13.3) behavior is fine with me. But in general the advantages of this rule would me accept both ways.

So it just was a coincidence that we switched to M1 while simultaneously updating to the latest Xcode version. My apologies for giving the wrong reason for this behavior. Xcode 13.4RC seems to be the same as Xcode 13.3. It's a bit furstrating to know that each Xcode version might change the outcome of SwiftLint, but i guess there is nothing that can be done.

@SimplyDanny
Copy link
Collaborator

You are right. The differences are due to the Xcode versions. We also see this in the builds. Xcode 13.3.1 reports success and only 13.2.1 fails. I missed that there are tests for different versions.

The changed behavior in the for_where example should be addressed in #3979.

The initially reported differences in the multiline_arguments_brackets rule can potentially be fixed by changing its implementation in a similar manner. One might need Xcode 13.2.1, though, to see what changed in the data structures.

@SimplyDanny SimplyDanny changed the title SwiftLint on M1 reporting less violations compared to Intel MacBooks SwiftLint is reporting less violations with Xcode 13.3 than with Xcode 13.2 May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected and reproducible misbehavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants