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 produces false positive when only operators from a module are used #2737

Closed
biboran opened this issue Apr 30, 2019 · 6 comments

Comments

@biboran
Copy link
Contributor

commented Apr 30, 2019

New Issue Checklist

Describe the bug

UnusedImportRule produces a false positive result when a file that imports a certain module uses only the operators defined by the imported module.

It seems that the File.synthaxMap property doesn't contain the operators and thus the cursorInfo for the operator is never queried.

Environment

I made an example repo which reproduces the bug. Here is how to run it.
Build the library and invoke swiftlint:

xcodebuild -project ./synthax-bug.xcodeproj -scheme synthax-bug-Package > xcodebuild.log
swiftlint analyze ./Sources/MainLib/MainLib.swift --compiler-log-path ./xcodebuild.log

A warning of unused import should appear even though the MainLib uses an overload of a + operator. Using any other function from the Extensions module dismisses the violation.

@jpsim

This comment has been minimized.

Copy link
Collaborator

commented Apr 30, 2019

Yeah, we're aware of this but haven't been able to come up with any workarounds. Can you think of a way to identify when operators are used from a given module?

@biboran

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

What's interesting is that by querying a source.request.cursorinfo with an offset of 160 in MainLib.swift (where the + operator is) I am able to get the following structure:

{
  "key.annotated_decl": "<Declaration>static func + (lhs: <Type usr=\"s:Sq\">Optional</Type>&lt;<Type usr=\"s:s4Voida\">Void</Type>&gt;, rhs: <Type usr=\"s:Sq\">Optional</Type>&lt;<Type usr=\"s:s4Voida\">Void</Type>&gt;)</Declaration>",
  "key.containertypeusr": "$Ss4VoidaSgmD",
  "key.fully_annotated_decl": "<decl.function.operator.infix><syntaxtype.keyword>static</syntaxtype.keyword> <syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>+ </decl.name>(<decl.var.parameter><decl.var.parameter.name>lhs</decl.var.parameter.name>: <decl.var.parameter.type><ref.enum usr=\"s:Sq\">Optional</ref.enum>&lt;<ref.typealias usr=\"s:s4Voida\">Void</ref.typealias>&gt;</decl.var.parameter.type></decl.var.parameter>, <decl.var.parameter><decl.var.parameter.name>rhs</decl.var.parameter.name>: <decl.var.parameter.type><ref.enum usr=\"s:Sq\">Optional</ref.enum>&lt;<ref.typealias usr=\"s:s4Voida\">Void</ref.typealias>&gt;</decl.var.parameter.type></decl.var.parameter>)</decl.function.operator.infix>",
  "key.groupname": "Optional",
  "key.kind": "source.lang.swift.ref.function.operator.infix",
  "key.modulename": "Extensions",
  "key.name": "+(_:_:)",
  "key.related_decls": [
    {
      "key.annotated_decl": "<RelatedName usr=\"s:SF1poiyxx_xtFZ\">+ (_: Self, _: Self) -&gt; Self</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Sf1poiyS2f_SftFZ\">+ (_: Float, _: Float) -&gt; Float</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Sd1poiyS2d_SdtFZ\">+ (_: Double, _: Double) -&gt; Double</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s7Float80V1poiyA2B_ABtFZ\">+ (_: Float80, _: Float80) -&gt; Float80</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Sj1poiyxx_xtFZ\">+ (_: Self, _: Self) -&gt; Self</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:SjsE1popyxxFZ\">+(_:)</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Sz1poiyxx_xtFZ\">+ (_: Self, _: Self) -&gt; Self</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s5UInt8V1poiyA2B_ABtFZ\">+ (_: UInt8, _: UInt8) -&gt; UInt8</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s4Int8V1poiyA2B_ABtFZ\">+ (_: Int8, _: Int8) -&gt; Int8</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s6UInt16V1poiyA2B_ABtFZ\">+ (_: UInt16, _: UInt16) -&gt; UInt16</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s5Int16V1poiyA2B_ABtFZ\">+ (_: Int16, _: Int16) -&gt; Int16</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s6UInt32V1poiyA2B_ABtFZ\">+ (_: UInt32, _: UInt32) -&gt; UInt32</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s5Int32V1poiyA2B_ABtFZ\">+ (_: Int32, _: Int32) -&gt; Int32</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s6UInt64V1poiyA2B_ABtFZ\">+ (_: UInt64, _: UInt64) -&gt; UInt64</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:s5Int64V1poiyA2B_ABtFZ\">+ (_: Int64, _: Int64) -&gt; Int64</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Su1poiyS2u_SutFZ\">+ (_: UInt, _: UInt) -&gt; UInt</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Si1poiyS2i_SitFZ\">+ (_: Int, _: Int) -&gt; Int</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:SmsE1poiyxx_qd__tSTRd__7ElementQyd__ABRtzlFZ\">+ &lt;Other&gt;(_: Self, _: Other) -&gt; Self where Other : Sequence, Self.Element == Other.Element</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:SmsE1poiyxqd___xtSTRd__7ElementQyd__ABRtzlFZ\">+ &lt;Other&gt;(_: Other, _: Self) -&gt; Self where Other : Sequence, Self.Element == Other.Element</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:SmsE1poiyxx_qd__tSmRd__7ElementQyd__ABRtzlFZ\">+ &lt;Other&gt;(_: Self, _: Other) -&gt; Self where Other : RangeReplaceableCollection, Self.Element == Other.Element</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Sxss8_PointerRzrlE1poiyxx_6StrideSxQztFZ\">+ (_: Self, _: Self.Stride) -&gt; Self</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:Sxss8_PointerRzrlE1poiyx6StrideSxQz_xtFZ\">+ (_: Self.Stride, _: Self) -&gt; Self</RelatedName>"
    },
    {
      "key.annotated_decl": "<RelatedName usr=\"s:SS1poiyS2S_SStFZ\">+ (_: String, _: String) -&gt; String</RelatedName>"
    }
  ],
  "key.typename": "<Wrapped> (Optional<Wrapped>.Type) -> (Optional<Wrapped>, Optional<Wrapped>) -> ()",
  "key.typeusr": "$SyyxSg_AAtcD",
  "key.usr": "s:Sq10ExtensionsE1poiyyxSg_ACtFZ"
}

but the token for the operator is missing in the source.request.editor.open request.
Can it be a SourceKit bug?

@jpsim

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

Nice find! 👏

I'm not sure if I'd consider this a SourceKit bug, since it seems like it may be omitting this by design. However, maybe we can find a way to get the operators used in a given file, maybe using Request.index request?

@biboran

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

This seems like it might be a viable solution! indexsource provides a usr that can be used to look up the cursorinfo and it includes the operator.
I'll try rewriting the rule using an indexsource request instead of the editor.open and open a PR if I succeed.

@jpsim

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

Thanks so much for looking into this @biboran!

biboran added a commit to biboran/SwiftLint that referenced this issue Aug 19, 2019
biboran added a commit to biboran/SwiftLint that referenced this issue Aug 19, 2019
biboran added a commit to biboran/SwiftLint that referenced this issue Aug 19, 2019
biboran added a commit to biboran/SwiftLint that referenced this issue Aug 19, 2019
jpsim added a commit that referenced this issue Aug 23, 2019
#2737 - Fix unused_imports false positive when only operators from th…
…e module are used (#2840)

As was discussed #2737, I utilized the `indexsource` request to look up the operators and fetch the cursorInfo per operator. In the implementation I refrained from using `usr` to look up the operator because SourceKit [doesn't support](https://github.com/apple/swift/blob/5add16804272b4df917da15c46eb6f28d826d656/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp#L1799) fetching cursorInfo by `usr` when it comes from C (but it can still be fetched by offset).

I also made [an alternative implementation](master...biboran:fix-unused-imports-for-operators-alternative) which uses the `indexsource` request instead of the `editor.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](https://github.com/biboran/synthax-bug-example) to better reflect the original issue in case you want to test the changes manually
@marcelofabri

This comment has been minimized.

Copy link
Collaborator

commented Aug 25, 2019

Fixed in #2840 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.