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

Unused import rule transitive dependencies #5622

Merged
merged 4 commits into from
Oct 25, 2024

Conversation

PaulTaykalo
Copy link
Collaborator

@PaulTaykalo PaulTaykalo commented Jun 12, 2024

This PR tries to resolve an issue, when unused import resolved incorrectly, since swiflint doesn't check transitive imports

For example, if we have

import Cocoa

let viewController = NSViewController()

Cocoa module is actually

import AppKit
import Foundation
import CoreData

So this PR actually tries to account transitive_modules configuration to prevent incorrect import removals

Alternative

As an alternative we can try to generate interfaces of all imported modules (once per module)
And parse their imports on the top level

So , for example, if we found some missing modules, we'll check if those can be found in the generated interface.

For example,

import SwiftUI

Will allow to use any of the underlying modules

/// Generated SwifTUI interface
import Accessibility
import AppKit
import Combine
import CoreData
import CoreFoundation
import CoreGraphics
import CoreTransferable
import Darwin
import DeveloperToolsSupport
import Foundation
import OSLog
import Observation
import Spatial
import SwiftUICore
import Symbols
import TargetConditionals
import UniformTypeIdentifiers
import _Concurrency
import _StringProcessing
import _SwiftConcurrencyShims
import os
import os.log
import simd

Related Issues:
#5167

@SimplyDanny SimplyDanny force-pushed the experiment/update-unused-import-rule branch 2 times, most recently from dbec2d1 to 824f5df Compare September 10, 2024 21:51
@SwiftLintBot
Copy link

SwiftLintBot commented Sep 10, 2024

1 Warning
⚠️ This PR may need tests.
17 Messages
📖 Linting Aerial with this PR took 0.92s vs 0.94s on main (2% faster)
📖 Linting Alamofire with this PR took 1.26s vs 1.27s on main (0% faster)
📖 Linting Brave with this PR took 7.17s vs 7.18s on main (0% faster)
📖 Linting DuckDuckGo with this PR took 5.06s vs 5.07s on main (0% faster)
📖 Linting Firefox with this PR took 10.58s vs 10.61s on main (0% faster)
📖 Linting Kickstarter with this PR took 9.84s vs 9.79s on main (0% slower)
📖 Linting Moya with this PR took 0.53s vs 0.53s on main (0% slower)
📖 Linting NetNewsWire with this PR took 2.62s vs 2.62s on main (0% slower)
📖 Linting Nimble with this PR took 0.77s vs 0.77s on main (0% slower)
📖 Linting PocketCasts with this PR took 8.42s vs 8.37s on main (0% slower)
📖 Linting Quick with this PR took 0.44s vs 0.46s on main (4% faster)
📖 Linting Realm with this PR took 4.51s vs 4.48s on main (0% slower)
📖 Linting Sourcery with this PR took 2.3s vs 2.29s on main (0% slower)
📖 Linting Swift with this PR took 4.48s vs 4.48s on main (0% slower)
📖 Linting VLC with this PR took 1.25s vs 1.25s on main (0% slower)
📖 Linting Wire with this PR took 17.5s vs 17.42s on main (0% slower)
📖 Linting WordPress with this PR took 11.52s vs 11.47s on main (0% slower)

Generated by 🚫 Danger

@SimplyDanny SimplyDanny force-pushed the experiment/update-unused-import-rule branch from 824f5df to 023ab9a Compare October 24, 2024 17:40
@SimplyDanny SimplyDanny marked this pull request as ready for review October 24, 2024 17:40
Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

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

Thanks @PaulTaykalo for the initial implementation!

I think this is good to be merged now.

@SimplyDanny SimplyDanny merged commit 8328212 into main Oct 25, 2024
14 checks passed
@SimplyDanny SimplyDanny deleted the experiment/update-unused-import-rule branch October 25, 2024 17:43
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