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

Fix unused_import when using a constructor defined transitively #5246

Merged

Conversation

jszumski
Copy link
Contributor

@jszumski jszumski commented Sep 28, 2023

Fixes a false positive in unused_import when using a constructor defined in a transitive module.

swiftlang/swift@3ea9bed added additional cursor info for the constructor in key.secondary_symbols.

Example

// Module A
class Thing {}

// Module B
import ModuleA
extension Thing {
   init(model: [String: Any]) {}
}

// Module C
import ModuleA
import ModuleB // this gets marked as unused

let foo = Thing(model: ...)

@SwiftLintBot
Copy link

SwiftLintBot commented Sep 28, 2023

17 Messages
📖 Linting Aerial with this PR took 1.23s vs 1.18s on main (4% slower)
📖 Linting Alamofire with this PR took 1.58s vs 1.56s on main (1% slower)
📖 Linting Brave with this PR took 8.78s vs 8.76s on main (0% slower)
📖 Linting DuckDuckGo with this PR took 4.4s vs 4.4s on main (0% slower)
📖 Linting Firefox with this PR took 10.3s vs 10.26s on main (0% slower)
📖 Linting Kickstarter with this PR took 11.39s vs 11.37s on main (0% slower)
📖 Linting Moya with this PR took 0.61s vs 0.62s on main (1% faster)
📖 Linting NetNewsWire with this PR took 3.3s vs 3.28s on main (0% slower)
📖 Linting Nimble with this PR took 0.82s vs 0.81s on main (1% slower)
📖 Linting PocketCasts with this PR took 8.71s vs 8.68s on main (0% slower)
📖 Linting Quick with this PR took 0.4s vs 0.4s on main (0% slower)
📖 Linting Realm with this PR took 11.42s vs 11.39s on main (0% slower)
📖 Linting Sourcery with this PR took 2.8s vs 2.8s on main (0% slower)
📖 Linting Swift with this PR took 5.22s vs 5.21s on main (0% slower)
📖 Linting VLC with this PR took 1.53s vs 1.53s on main (0% slower)
📖 Linting Wire with this PR took 19.36s vs 19.32s on main (0% slower)
📖 Linting WordPress with this PR took 13.15s vs 13.07s on main (0% slower)

Generated by 🚫 Danger

@jszumski
Copy link
Contributor Author

What is the best way to model this fix in a test?

@SimplyDanny
Copy link
Collaborator

What is the best way to model this fix in a test?

I can only think of a separate test that runs the rule on a dummy project portraying exactly this setup of dependencies, that is an Xcode project with three modules like you've used in the description. But it's also fine for me to accept this PR without a dedicated test as all existing examples still seem to work.

@jszumski
Copy link
Contributor Author

I'm good to merge as is then, thanks!

@SimplyDanny
Copy link
Collaborator

Please rebase. Danger doesn't like merge commits. 😉

@jszumski jszumski force-pushed the jszumski/fix-unused-imports-constructor branch from bfb4101 to bc98118 Compare October 1, 2023 00:44
@SimplyDanny SimplyDanny merged commit 696e19d into realm:main Oct 1, 2023
4 checks passed
KS1019 pushed a commit to KS1019/SwiftLint that referenced this pull request Oct 2, 2023
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