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

Skip module import if cursor info is missing module info #2746

Merged
merged 1 commit into from May 6, 2019

Conversation

alvarhansen
Copy link
Contributor

I believe setting nextIsModuleImport = false in current code is accidental bug, as it makes no sense to set it false if it already is false. It makes more sense if it is in inner if statements else block.

At the moment it will lead to crash if you import module that SourceKit is unable to find info for.

With following code:

import UnknownModule
func foo(error: Swift.Error) {}

The execution goes like this:

  1. import token is found and nextIsModuleImport is set to true
  2. identifier token is found but if SourceKit returns empty dictionary the loop just continues
  3. Swift token in foo(error: Swift.Error) is visited and if it has key.modulename source.lang.swift.ref.module it is falsely considered as an import statement.
  4. Its name is added to unusedImports
  5. contentsNSString.range(of: "import \(module)\n") returns NSRange with location NSNotFound
    ...
  6. func correct(file: File, compilerArguments: [String]) -> [Correction] will throw NSException when attempting to replace this range.

@SwiftLintBot
Copy link

SwiftLintBot commented May 6, 2019

12 Messages
📖 Linting Aerial with this PR took 3.48s vs 3.22s on master (8% slower)
📖 Linting Alamofire with this PR took 5.82s vs 5.39s on master (7% slower)
📖 Linting Firefox with this PR took 17.29s vs 13.95s on master (23% slower)
📖 Linting Kickstarter with this PR took 32.03s vs 27.74s on master (15% slower)
📖 Linting Moya with this PR took 2.85s vs 2.54s on master (12% slower)
📖 Linting Nimble with this PR took 2.29s vs 2.29s on master (0% slower)
📖 Linting Quick with this PR took 0.83s vs 0.82s on master (1% slower)
📖 Linting Realm with this PR took 3.87s vs 3.86s on master (0% slower)
📖 Linting SourceKitten with this PR took 1.52s vs 1.52s on master (0% slower)
📖 Linting Sourcery with this PR took 5.07s vs 5.02s on master (0% slower)
📖 Linting Swift with this PR took 40.02s vs 39.64s on master (0% slower)
📖 Linting WordPress with this PR took 30.54s vs 30.44s on master (0% slower)

Generated by 🚫 Danger

@jpsim
Copy link
Collaborator

jpsim commented May 6, 2019

Makes sense. Thanks!

@jpsim jpsim merged commit af72c06 into realm:master May 6, 2019
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.

None yet

3 participants