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

baseline compare produces some false alarms #5606

Closed
2 tasks done
mildm8nnered opened this issue May 26, 2024 · 0 comments · Fixed by #5605
Closed
2 tasks done

baseline compare produces some false alarms #5606

mildm8nnered opened this issue May 26, 2024 · 0 comments · Fixed by #5605
Labels

Comments

@mildm8nnered
Copy link
Collaborator

New Issue Checklist

Describe the bug

The baseline compare subcommand is implemented as follows:

    public func compare(_ otherBaseline: Baseline) -> [StyleViolation] {
        otherBaseline.baseline.flatMap {
            filter($1.violationsWithAbsolutePaths)
        }
    }

This re-uses the Baseline filter API, passing in the underlying StyleViolation's, and ignoring the existing text values. During filter, the files referred to in the StyleViolation will be read to determine the text for each StyleViolation. This is fine when linting, as the files will be present, but during baseline compare, there is no guarantee that the source files will be there at all, or that they will be the same as the files that were present when the baseline was constructed.

Where the files are not present, or have changed, the text values will not match, and violations for those files will be considered as "new" when comparing the baselines.

The fix for this will be to not throw away the text information in the Baseline, and to use a lower-level API to compare the baselines.

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint.release  baseline compare --other-baseline 0.54.Baseline.json 0.53.Baseline.json --reporter summary
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
| rule identifier                         | opt-in | correctable | custom | warnings | errors | total violations | number of files |
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
| explicit_acl                            | yes    | no          | no     |      382 |      0 |              382 |             304 |
| explicit_top_level_acl                  | yes    | no          | no     |      288 |      0 |              288 |             286 |
| required_deinit                         | yes    | no          | no     |      228 |      0 |              228 |             176 |
| explicit_type_interface                 | yes    | no          | no     |      183 |      0 |              183 |              80 |
| indentation_width                       | yes    | no          | no     |       62 |      0 |               62 |              41 |
| no_extension_access_modifier            | yes    | no          | no     |        0 |     47 |               47 |              38 |
| file_types_order                        | yes    | no          | no     |       46 |      0 |               46 |              40 |
| type_contents_order                     | yes    | no          | no     |       41 |      0 |               41 |              19 |
| no_grouping_extension                   | yes    | no          | no     |       34 |      0 |               34 |              34 |
| prefer_nimble                           | yes    | no          | no     |       23 |      0 |               23 |               5 |
| one_declaration_per_file                | yes    | no          | no     |       17 |      0 |               17 |              15 |
| multiline_parameters_brackets           | yes    | no          | no     |       16 |      0 |               16 |               7 |
| missing_docs                            | yes    | no          | no     |       12 |      0 |               12 |               8 |
| no_magic_numbers                        | yes    | no          | no     |        9 |      0 |                9 |               8 |
| implicit_return                         | yes    | yes         | no     |        7 |      0 |                7 |               6 |
| superfluous_disable_command             | no     | no          | no     |        6 |      0 |                6 |               6 |
| superfluous_else                        | yes    | yes         | no     |        6 |      0 |                6 |               3 |
| multiline_arguments_brackets            | yes    | no          | no     |        5 |      0 |                5 |               2 |
| prefer_self_in_static_references        | yes    | yes         | no     |        5 |      0 |                5 |               3 |
| prefixed_toplevel_constant              | yes    | no          | no     |        5 |      0 |                5 |               5 |
| discouraged_optional_collection         | yes    | no          | no     |        4 |      0 |                4 |               3 |
| force_unwrapping                        | yes    | no          | no     |        4 |      0 |                4 |               2 |
| multiline_parameters                    | yes    | no          | no     |        4 |      0 |                4 |               4 |
| strict_fileprivate                      | yes    | no          | no     |        4 |      0 |                4 |               4 |
| trailing_closure                        | yes    | yes         | no     |        4 |      0 |                4 |               3 |
| vertical_whitespace_between_cases       | yes    | yes         | no     |        4 |      0 |                4 |               3 |
| anonymous_argument_in_multiline_closure | yes    | no          | no     |        3 |      0 |                3 |               1 |
| multiline_literal_brackets              | yes    | no          | no     |        3 |      0 |                3 |               3 |
| sorted_enum_cases                       | yes    | no          | no     |        2 |      0 |                2 |               1 |
| conditional_returns_on_newline          | yes    | no          | no     |        1 |      0 |                1 |               1 |
| function_default_parameter_at_end       | yes    | no          | no     |        1 |      0 |                1 |               1 |
| implicitly_unwrapped_optional           | yes    | no          | no     |        1 |      0 |                1 |               1 |
| let_var_whitespace                      | yes    | no          | no     |        1 |      0 |                1 |               1 |
| multiline_arguments                     | yes    | no          | no     |        1 |      0 |                1 |               1 |
| opening_brace                           | no     | yes         | no     |        1 |      0 |                1 |               1 |
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
| Total                                   |        |             |        |    1,413 |     47 |            1,460 |             348 |
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+

Environment

  • SwiftLint version (run swiftlint version to be sure)? 0.55.1
  • Installation method used (Homebrew, CocoaPods, building from source, etc)? Cocoapods
  • Paste your configuration file:

N/A

  • Are you using nested configurations?
    If so, paste their relative paths and respective contents. No
  • Which Xcode version are you using (check xcodebuild -version)? Xcode 15.2
  • Do you have a sample that shows the issue? No
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant