Skip to content

[UnusedDeclarationRule] Speed up and detect more dead code#3340

Merged
jpsim merged 4 commits into
masterfrom
unused-declaration-index
Sep 15, 2020
Merged

[UnusedDeclarationRule] Speed up and detect more dead code#3340
jpsim merged 4 commits into
masterfrom
unused-declaration-index

Conversation

@jpsim

@jpsim jpsim commented Sep 14, 2020

Copy link
Copy Markdown
Collaborator

By using SourceKit's index request to index the entire source file, we can avoid having to make cursor-info requests for every candidate token in the file, which scales linearly with the number of candidate tokens.

For the Yams project, this approach improved the total SwiftLint run time by 4.6x: 7.9 down from 36.8s.

image

The SourceKit index response doesn't have everything we need to identify declarations, so we still need to make some cursor-info requests, mostly to detect overrides: protocol conformances and parent class overrides.

This approach ends up finding more unused declarations because the index contains more declared USRs than can be found by calling cursor-info on candidate tokens in a file.

By using SourceKit's `index` request to index the entire source file,
we can avoid having to make `cursor-info` requests for every candidate
token in the file, which scales linearly with the number of candiate
tokens.

For the Yams project, this approach improved the total SwiftLint run
time by 4.6x: 7.9 down from 36.8s.

The SourceKit index response doesn't have everything we need to identify
declarations, so we still need to make some `cursor-info` requests,
mostly to detect overrides: protocol conformances and parent class
overrides.

This approach ends up finding more unused declarations because the index
contains more declared USRs than can be found by calling `cursor-info`
on candidate tokens in a file.
@SwiftLintBot

SwiftLintBot commented Sep 14, 2020

Copy link
Copy Markdown
1 Warning
⚠️ This PR may need tests.
12 Messages
📖 Linting Aerial with this PR took 1.86s vs 1.83s on master (1% slower)
📖 Linting Alamofire with this PR took 2.72s vs 2.7s on master (0% slower)
📖 Linting Firefox with this PR took 9.23s vs 9.18s on master (0% slower)
📖 Linting Kickstarter with this PR took 15.33s vs 15.32s on master (0% slower)
📖 Linting Moya with this PR took 1.3s vs 1.31s on master (0% faster)
📖 Linting Nimble with this PR took 1.36s vs 1.4s on master (2% faster)
📖 Linting Quick with this PR took 0.63s vs 0.64s on master (1% faster)
📖 Linting Realm with this PR took 2.69s vs 2.68s on master (0% slower)
📖 Linting SourceKitten with this PR took 1.04s vs 1.03s on master (0% slower)
📖 Linting Sourcery with this PR took 7.1s vs 7.12s on master (0% faster)
📖 Linting Swift with this PR took 10.79s vs 10.76s on master (0% slower)
📖 Linting WordPress with this PR took 16.88s vs 16.83s on master (0% slower)

Generated by 🚫 Danger

@codecov-commenter

codecov-commenter commented Sep 14, 2020

Copy link
Copy Markdown

Codecov Report

Merging #3340 into master will decrease coverage by 0.03%.
The diff coverage is 82.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3340      +/-   ##
==========================================
- Coverage   90.58%   90.55%   -0.04%     
==========================================
  Files         416      416              
  Lines       20406    20415       +9     
==========================================
+ Hits        18484    18486       +2     
- Misses       1922     1929       +7     
Impacted Files Coverage Δ
Source/SwiftLintFramework/Models/RuleStorage.swift 83.33% <0.00%> (-16.67%) ⬇️
.../SwiftLintFramework/Rules/Lint/ArrayInitRule.swift 97.43% <ø> (-0.05%) ⬇️
...ntFramework/Rules/Lint/UnusedDeclarationRule.swift 84.17% <83.96%> (-2.76%) ⬇️
...tFramework/Rules/Lint/ValidIBInspectableRule.swift 98.64% <0.00%> (-1.36%) ⬇️
...LintFramework/Extensions/SwiftLintFile+Regex.swift 97.20% <0.00%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9ce579...33c2c23. Read the comment docs.

@jpsim jpsim marked this pull request as ready for review September 14, 2020 13:32
@jpsim jpsim merged commit ea311ba into master Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants