Skip to content

Fix unused_import when using a constructor defined transitively#5246

Merged
SimplyDanny merged 1 commit into
realm:mainfrom
jszumski:jszumski/fix-unused-imports-constructor
Oct 1, 2023
Merged

Fix unused_import when using a constructor defined transitively#5246
SimplyDanny merged 1 commit into
realm:mainfrom
jszumski:jszumski/fix-unused-imports-constructor

Conversation

@jszumski

@jszumski jszumski commented Sep 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

SwiftLintBot commented Sep 28, 2023 •
edited
Loading

Copy link
Copy Markdown
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

jszumski commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

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

@SimplyDanny

SimplyDanny commented Sep 30, 2023

Copy link
Copy Markdown
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

jszumski commented Sep 30, 2023

Copy link
Copy Markdown
Contributor Author

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

@SimplyDanny

SimplyDanny commented Sep 30, 2023

Copy link
Copy Markdown
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
KS1019 pushed a commit to KS1019/SwiftLint that referenced this pull request Oct 2, 2023
@jszumski @KS1019
Fix false positives in unused_import rule for constructors defined …
e22cf43 
…in transitive modules (realm#5246)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SimplyDanny SimplyDanny SimplyDanny approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jszumski @SwiftLintBot @SimplyDanny