#2737 - Fix unused_imports false positive when only operators from the module are used #2840
Conversation
| import Foundation | ||
| import ObjectiveC | ||
| let 👨👩👧👦 = #selector(NSArray.contains(_:)) | ||
| 👨👩👧👦 == 👨👩👧👦 |
There was a problem hiding this comment.
ObjectiveC module introduces an overload of the == operator:
public static func == (lhs: Selector, rhs: Selector) -> Booland even though Selector is introduced by the ObjectiveC module it can be indirectly created with #selector. This operator overload is the only way I found to reproduce the original issue in the unit tests.
|
Great to see this 😍 Give me some time to take it for a spin and review. |
Generated by 🚫 Danger |
|
I tested the alternative implementation which uses |
| } | ||
| .sorted(by: { $0.1.location < $1.1.location }) | ||
| } | ||
|
|
||
| private func containsAttributesRequiringFoundation() -> Bool { | ||
| // Operators are omitted in the editor.open request and thus have to be looked up by the indexsource request | ||
| func operatorImports(arguments: [String], processedTokenOffsets: Set<Int>) -> Set<String> { |
There was a problem hiding this comment.
Can you mark these as private where possible?
There was a problem hiding this comment.
Since these methods are a part of a private extension, the compiler warns that using private is redundant in this case. Do you still want me to mark them private?
I could also make the extension non-private and the methods private for the sake of consistency.
There was a problem hiding this comment.
Oh I didn’t realize these were in a private extension. All good!
|
Code looks good, I'm validating this now on a large closed source project with a number of files where only operator overloads are used. |
|
One thing that running on a large codebase that this identified was that subscript extensions still aren't detected. Not a blocker to merging this but I'm curious to see if you think this approach can be extended to catch those? For example, if this is defined in ModuleA: extension Collection where Index == Int {
public subscript(safe index: Int) -> Iterator.Element? {
return index < self.count && index >= 0 ? self[index] : nil
}
}and ModuleB has this: import ModuleA
let array = ["a", "b", "c"]
_ = array[safe: 0] // equivalent to array.firstThen ModuleA will still be listed as an unused import. |
|
ok, finally validated this. Looks great, thank you so much for owning this @Biboran ! ❤️ |
As was discussed #2737, I utilized the
indexsourcerequest to look up the operators and fetch the cursorInfo per operator. In the implementation I refrained from usingusrto look up the operator because SourceKit doesn't support fetching cursorInfo byusrwhen it comes from C (but it can still be fetched by offset).I also made an alternative implementation which uses the
indexsourcerequest instead of theeditor.open. It seems to work with the unit tests but I am not 100% sure it doesn't introduce a regression since there is no oss-check for analyzer rules.I also updated the example to better reflect the original issue in case you want to test the changes manually