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

Report CodeActions for Fix-Its produced by swift-syntax #909

Closed
ahoppen opened this issue Oct 18, 2023 · 4 comments
Closed

Report CodeActions for Fix-Its produced by swift-syntax #909

ahoppen opened this issue Oct 18, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@ahoppen
Copy link
Member

ahoppen commented Oct 18, 2023

Address this FIXME https://github.com/apple/sourcekit-lsp/pull/902/files#diff-6147b806788b196876d46885643488f44c48b14791d7adc3ef5d2d91b361bd11R74-R76 and report CodeActions for the Fix-Its produced by the integrated Swift parser if we don’t have compiler arguments.

@ahoppen
Copy link
Member Author

ahoppen commented Oct 19, 2023

Tracked in Apple’s issue tracker as rdar://117176551

@joehsieh
Copy link
Contributor

joehsieh commented Oct 25, 2023

Just to clarify the requirements

// FIXME: Once swiftlang/swift-syntax#2226 is merged and
// FixItApplier is public, use it to compute the edits that should be
// applied to the source.
https://github.com/apple/sourcekit-lsp/pull/902/files#diff-6147b806788b196876d46885643488f44c48b14791d7adc3ef5d2d91b361bd11R74-R76

  1. FixItApplier is under _SwiftSyntaxTestSupport to support testing, so FixItApplier won't be used in Diagnostic.swift
    Instead, what we need is a func in Diagnostic.swift to convert FixIt.Change to TextEdit .
    pseudo code
extension FixIt.Change {
  func edit(snapshot: DocumentSnapshot) -> TextEdit? {
    switch self {
      case .replace(let oldNode, let newNode):
        return TextEdit(...)
      case .replaceLeadingTrivia(let token, let newTrivia):
        return TextEdit(...)
      case .replaceTrailingTrivia(let token, let newTrivia):
        return TextEdit(...)
    }
  }
}

so that we can convert the fixIt to an array of TextEdit here.

  init?(_ fixIt: FixIt, in snapshot: DocumentSnapshot) {
    // FIXME: Once https://github.com/apple/swift-syntax/pull/2226 is merged and
    // FixItApplier is public, use it to compute the edits that should be
    // applied to the source.
    var edits = [TextEdit]()
    for change in fixIt.changes {
      if let edit = change.edit(snapshot: snapshot) {
        edits.append(edit)
      }
    }
...
}

is my understanding correct?

report CodeActions for the Fix-Its produced by the integrated Swift parser if we don’t have compiler arguments.

  1. what are these compiler arguments?

Let me know if it's a urgent issue we need to fix in near future. i'll work on other ticket if needed. Thanks!

@ahoppen
Copy link
Member Author

ahoppen commented Oct 25, 2023

Instead, what we need is a func in Diagnostic.swift to convert FixIt.Change to TextEdit

Yes, FixItApplier should have that code now and we need to figure out where to put the conversion of FixIt.Change to TextEdit and how to structure its API. That shouldn’t be too hard but needs to be done.

what are these compiler arguments?

To get semantic functionality you need to know the module search paths (-I), SDK (-sdk) etc. For example without the module search paths we can’t find the modules you are importing. We refer to these as compiler arguments or build settings more or less interchangeably. BuildSystemManager is what provides the compiler arguments.

Let me know if it's a urgent issue we need to fix in near future. i'll work on other ticket if needed. Thanks!

This is not a super urgent issue to fix but would be nice to get done. I think the main work for this issue is to figure out the API that swift-syntax should provide to get text edits for the FixIt.Change.

@ahoppen
Copy link
Member Author

ahoppen commented Nov 23, 2023

Fixed by swiftlang/swift-syntax#2314

@ahoppen ahoppen closed this as completed Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants