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

Add Unused Import analyzer rule #2381

Merged
merged 2 commits into from
Sep 2, 2018
Merged

Add Unused Import analyzer rule #2381

merged 2 commits into from
Sep 2, 2018

Conversation

jpsim
Copy link
Collaborator

@jpsim jpsim commented Sep 2, 2018

No description provided.

@jpsim
Copy link
Collaborator Author

jpsim commented Sep 2, 2018

Removed SwiftLint's own unused imports using this rule in #2382.

@SwiftLintBot
Copy link

SwiftLintBot commented Sep 2, 2018

12 Messages
📖 Linting Aerial with this PR took 0.42s vs 0.38s on master (10% slower)
📖 Linting Alamofire with this PR took 3.38s vs 3.08s on master (9% slower)
📖 Linting Firefox with this PR took 13.69s vs 11.92s on master (14% slower)
📖 Linting Kickstarter with this PR took 21.04s vs 17.43s on master (20% slower)
📖 Linting Moya with this PR took 2.29s vs 1.9s on master (20% slower)
📖 Linting Nimble with this PR took 2.2s vs 1.7s on master (29% slower)
📖 Linting Quick with this PR took 0.58s vs 0.52s on master (11% slower)
📖 Linting Realm with this PR took 3.79s vs 3.13s on master (21% slower)
📖 Linting SourceKitten with this PR took 1.22s vs 1.03s on master (18% slower)
📖 Linting Sourcery with this PR took 5.27s vs 4.48s on master (17% slower)
📖 Linting Swift with this PR took 32.68s vs 28.42s on master (14% slower)
📖 Linting WordPress with this PR took 19.72s vs 16.66s on master (18% slower)

Generated by 🚫 Danger

}
}

private let syntaxKindsToSkip: [SyntaxKind] = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about making this a Set?

@codecov-io
Copy link

Codecov Report

Merging #2381 into master will decrease coverage by 0.01%.
The diff coverage is 85.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2381      +/-   ##
==========================================
- Coverage   90.51%   90.49%   -0.02%     
==========================================
  Files         313      314       +1     
  Lines       15704    15775      +71     
==========================================
+ Hits        14214    14276      +62     
- Misses       1490     1499       +9
Impacted Files Coverage Δ
...tFrameworkTests/AutomaticRuleTests.generated.swift 100% <100%> (ø) ⬆️
...iftLintFramework/Rules/Lint/UnusedImportRule.swift 85.5% <85.5%> (ø)
Source/SwiftLintFramework/Models/Command.swift 98.7% <0%> (+1.29%) ⬆️

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 2bcea4b...8cc5b3d. Read the comment docs.

@jpsim jpsim merged commit e460c5e into master Sep 2, 2018
@jpsim jpsim deleted the unused-import-rule branch September 2, 2018 21:46
@marcelofabri
Copy link
Collaborator

@jpsim Have you tried running this rule against Lyft's iOS app? I've run it against one app and gave some false positives - I think they're all related to importing a framework that imports other ones.

For example, one class was importing Photos to use AVCaptureDevice. That import was removed and the file doesn't compile anymore. Other examples are importing Firebase (instead of the specific Firebase framework).

@jpsim
Copy link
Collaborator Author

jpsim commented Sep 3, 2018

Thats expected. If you import UIKit but only use CGPoint, a more appropriate import would be CoreGraphics for example.

However, there are times when autocorrect will cause the file to no longer compile, because the wider import wasn’t replaced by a more specific one.

@Jeehut Jeehut mentioned this pull request Sep 6, 2018
@marcelofabri marcelofabri mentioned this pull request Sep 10, 2018
2 tasks
sjavora pushed a commit to sjavora/SwiftLint that referenced this pull request Mar 9, 2019
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.

4 participants