Dogfood Baselines internally
Nobody has claimed this yet.
- Dominant language
- Swift
- Stars
- 19.7k
- Forks
- 2.3k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 11
Description
% swiftlint --enable-all-rules --progress --reporter summary on SwiftLint produces the following violations:
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
| rule identifier | opt-in | correctable | custom | warnings | errors | total violations | number of files |
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
| explicit_type_interface | yes | no | no | 3,878 | 0 | 3,878 | 541 |
| explicit_acl | yes | no | no | 3,065 | 0 | 3,065 | 535 |
| indentation_width | yes | no | no | 1,833 | 0 | 1,833 | 333 |
| prefer_nimble | yes | no | no | 896 | 0 | 896 | 65 |
| explicit_top_level_acl | yes | no | no | 722 | 0 | 722 | 463 |
| multiline_arguments_brackets | yes | no | no | 644 | 0 | 644 | 113 |
| required_deinit | yes | no | no | 625 | 0 | 625 | 318 |
| no_extension_access_modifier | yes | no | no | 0 | 550 | 550 | 274 |
| no_magic_numbers | yes | no | no | 525 | 0 | 525 | 94 |
| type_contents_order | yes | no | no | 452 | 0 | 452 | 282 |
| one_declaration_per_file | yes | no | no | 356 | 0 | 356 | 75 |
| implicit_return | yes | yes | no | 350 | 0 | 350 | 140 |
| no_grouping_extension | yes | no | no | 223 | 0 | 223 | 207 |
| multiline_arguments | yes | no | no | 213 | 0 | 213 | 54 |
| multiline_parameters_brackets | yes | no | no | 209 | 0 | 209 | 56 |
| file_types_order | yes | no | no | 190 | 0 | 190 | 73 |
| anonymous_argument_in_multiline_closure | yes | no | no | 185 | 0 | 185 | 64 |
| missing_docs | yes | no | no | 164 | 0 | 164 | 23 |
| sorted_enum_cases | yes | no | no | 132 | 0 | 132 | 29 |
| multiline_literal_brackets | yes | no | no | 109 | 0 | 109 | 27 |
| vertical_whitespace_between_cases | yes | yes | no | 109 | 0 | 109 | 45 |
| force_unwrapping | yes | no | no | 104 | 0 | 104 | 36 |
| switch_case_on_newline | yes | no | no | 97 | 0 | 97 | 18 |
| explicit_enum_raw_value | yes | no | no | 84 | 0 | 84 | 15 |
| conditional_returns_on_newline | yes | no | no | 76 | 0 | 76 | 50 |
| trailing_closure | yes | yes | no | 72 | 0 | 72 | 43 |
| convenience_type | yes | no | no | 59 | 0 | 59 | 54 |
| prefixed_toplevel_constant | yes | no | no | 53 | 0 | 53 | 33 |
| strict_fileprivate | yes | no | no | 51 | 0 | 51 | 21 |
| prefer_self_in_static_references | yes | yes | no | 49 | 0 | 49 | 24 |
| multiline_function_chains | yes | no | no | 41 | 0 | 41 | 29 |
| multiline_parameters | yes | no | no | 39 | 0 | 39 | 26 |
| discouraged_optional_collection | yes | no | no | 33 | 0 | 33 | 20 |
| function_default_parameter_at_end | yes | no | no | 18 | 0 | 18 | 12 |
| closure_body_length | yes | no | no | 15 | 2 | 17 | 6 |
| todo | no | no | no | 11 | 0 | 11 | 8 |
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
| Total | | | | 15,682 | 552 | 16,234 | 602 |
+-----------------------------------------+--------+-------------+--------+----------+--------+------------------+-----------------+
Some of these rules we are unlikely to ever want on. For example, explicit_type_interface (3,878 violations), but other, such as implicit_return (350 violations) could actually be usefully on.
For the latter cases, we could simply patch all of the existing violations and enable them, but we could also use a baseline to ignore (all of some of) the existing violations for now, but prevent any new violations of those rules being introduced.
Open questions would be:
- Should we just patch all the violations of rules we want anyway.
In the case of some rules (e.g. force_unwrapping) some case may be awkward to avoid in code.
- Assuming we don't just patch everything, which rules should we enable?
See my comment below for some suggestions on which rules we may want to enable.
- How to run and configure SwiftLint?
SwiftLint is a bit weird, in that unlike most deployments of SwiftLint, there is no SwiftLint run during building - this can be a bit annoying, as you may not find out about violations until you run the integration tests, but is understandable - we would need a release build of SwiftLint to lint our local debug builds efficiently, and what version should that be - it can't be HEAD, because that's what we're building.
Instead, during Integration tests, SwiftLint lints itself (slowly) during the debug build. The proposal would be to use a baseline on this run.
As far as configuration goes, if we configure the baselines in .swiftlint.yml (see #5552), then anyone running swiftlint will pick up the baseline configuration automatically. If we relied on command line configuration, then they would see all the violations that we are using the baseline to filter out.
- How to store the baseline
The baseline.json for the above violations is circa 7MB. If we just enable the rules suggested as DO or MIGHT enable, the baseline.json is about 2MB. Disabling the most frequent of those rules - indentation_width reduces the baseline size to about 1.3MB
We could store this uncompressed in the git tree, but over time, if we update it, it will increase the overall size of the repo history.
We could store it compressed, and then uncompress it during the Integration tests (and leave the uncompressed, .gitignore'd version on disk), but then anyone just running swiftlint would get an error, as the uncompressed baseline would be missing.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the Integration tests that run SwiftLint during the debug build and the .swiftlint.yml configuration, then reproduce the reported swiftlint --enable-all-rules --progress --reporter summary output. Resolve which rules and baseline configuration should be adopted, how the baseline is stored, and how the integration run should use it; done means the chosen rules are enforced without introducing new violations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- testing, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100