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

Jump to definition requires a reload after build #620

Closed
adam-fowler opened this issue Aug 29, 2022 · 8 comments · Fixed by #1180
Closed

Jump to definition requires a reload after build #620

adam-fowler opened this issue Aug 29, 2022 · 8 comments · Fixed by #1180

Comments

@adam-fowler
Copy link
Contributor

If I load a project with dependencies into VS Code which has not been built when I right click on a symbol to go to definition (run a textDocument/definition request) it returns an empty result. This is a known issue.

If I then build the project and repeat I still get an empty result. I would assume once the project is built SourceKit-LSP should be able to return a position for the symbol.

I have to restart VS Code (ie restart the SourceKit-LSP server) for SourceKit-LSP to return a valid position for the symbol.

@ahoppen
Copy link
Member

ahoppen commented Aug 30, 2022

rdar://99329579

@adam-fowler
Copy link
Contributor Author

This look like it is resolved, will re-open if I see it again

@adam-fowler
Copy link
Contributor Author

Re-opening saw it again

@adam-fowler adam-fowler reopened this Jul 7, 2023
@adam-fowler
Copy link
Contributor Author

Actually it is just slow to respond. Is there a way we can indicate the LSP server is loading the index after a build, or whatever it is doing post build that means it doesn't have the built symbols available.

ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Jul 12, 2023
… packages

I noticed that the initial package loading can take ~5s. It’s good behavior to inform the client that sourcekit-lsp is busy reloading the package, showing the user that semantic functionality might not be ready yet.

swiftlang#620
rdar://111917300
@ahoppen
Copy link
Member

ahoppen commented Jul 12, 2023

I’m adding a progress indicator for package reloading in #767. That’s all I can think of that might take a non-trivial amount of time and happens on the SourceKit-LSP side after the project has been modified.

@ahoppen
Copy link
Member

ahoppen commented Jul 12, 2023

What’s happening is that after building, you need to make an edit to the file to get jump-to-definition working. Could that match what you’ve been seeing @adam-fowler?

On a technical level

  • When we open the file, we build an AST for it. That AST has errors because the dependent modules haven’t been built yet.
  • Now we build the dependent modules and the dependent module files exist
  • If you now do a jump to definition request again, we perform a cursor info request to get the USR of the symbol we do jump-to-definition for but since the file hasn’t changed, cursor info re-uses the previously built AST and is thus unable to resolve the symbol (because at the point that the AST was created the module that contains the symbol didn’t exist yet)
  • If you make an edit to the file (that’s not an edit inside a comment), we stop re-using the previously built AST. At that point, you see semantic highlighting appearing for the symbol in the other module and jump-to-definition starts working.

The way I would like to fix this, is to migrate cursor info to solver-based, which removes the need for this AST reuse altogether. This is a long-term effort though. I don’t have a good idea for a short-term workaround and since things recover after the first edit, I believe there’s nothing immediate we need to or should do here.

@adam-fowler
Copy link
Contributor Author

What’s happening is that after building, you need to make an edit to the file to get jump-to-definition working. Could that match what you’ve been seeing @adam-fowler?

I'll have to check that. It could be.

Given you are saying the correct solution is a long-term effort is there anything temporary I could do to resolve this issue. I know when a build finishes could I get VSCode to send some request, notification that would trigger an AST rebuild

ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Jul 12, 2023
… packages

I noticed that the initial package loading can take ~5s. It’s good behavior to inform the client that sourcekit-lsp is busy reloading the package, showing the user that semantic functionality might not be ready yet.

swiftlang#620
rdar://111917300
@ahoppen
Copy link
Member

ahoppen commented Jul 13, 2023

Other than making an edit to the file, unfortunately I don’t think there’s anything you could do here to work around the issue. I’m sorry.

ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Jul 17, 2023
… packages

I noticed that the initial package loading can take ~5s. It’s good behavior to inform the client that sourcekit-lsp is busy reloading the package, showing the user that semantic functionality might not be ready yet.

swiftlang#620
rdar://111917300
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Jul 17, 2023
… packages

I noticed that the initial package loading can take ~5s. It’s good behavior to inform the client that sourcekit-lsp is busy reloading the package, showing the user that semantic functionality might not be ready yet.

swiftlang#620
rdar://111917300
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Aug 4, 2023
… packages

I noticed that the initial package loading can take ~5s. It’s good behavior to inform the client that sourcekit-lsp is busy reloading the package, showing the user that semantic functionality might not be ready yet.

swiftlang#620
rdar://112498447
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Apr 17, 2024
…ule` file has been changed

When the client sends us `workspace/didChangeWatchedFiles` notification of an updated `.swift` file, we should refresh the other open files in that module since they might be referencing functions from that updated file.

If a `.swiftmodule` file has been updated, we refresh all the files within the package since they might import that module. Technically, we would only need to refresh files that are in module that are downstream of the updated module but we don’t currently have that information easily available from SwiftPM. Also, usually, if the client has a file from a low-level module open, he’ll be working on that module which means that such an optimization won’t help. The real solution here is to wait for us to finish preparation (which we would exactly know when it finishes since sourcekit-lsp would schedule it) but for that we need to implement background preparation.

Fixes swiftlang#620
Fixes swiftlang#1116
rdar://99329579
rdar://123971779
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Apr 18, 2024
…ule` file has been changed

When the client sends us `workspace/didChangeWatchedFiles` notification of an updated `.swift` file, we should refresh the other open files in that module since they might be referencing functions from that updated file.

If a `.swiftmodule` file has been updated, we refresh all the files within the package since they might import that module. Technically, we would only need to refresh files that are in module that are downstream of the updated module but we don’t currently have that information easily available from SwiftPM. Also, usually, if the client has a file from a low-level module open, he’ll be working on that module which means that such an optimization won’t help. The real solution here is to wait for us to finish preparation (which we would exactly know when it finishes since sourcekit-lsp would schedule it) but for that we need to implement background preparation.

Fixes swiftlang#620
Fixes swiftlang#1116
rdar://99329579
rdar://123971779
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this issue Apr 23, 2024
…ule` file has been changed

When the client sends us `workspace/didChangeWatchedFiles` notification of an updated `.swift` file, we should refresh the other open files in that module since they might be referencing functions from that updated file.

If a `.swiftmodule` file has been updated, we refresh all the files within the package since they might import that module. Technically, we would only need to refresh files that are in module that are downstream of the updated module but we don’t currently have that information easily available from SwiftPM. Also, usually, if the client has a file from a low-level module open, he’ll be working on that module which means that such an optimization won’t help. The real solution here is to wait for us to finish preparation (which we would exactly know when it finishes since sourcekit-lsp would schedule it) but for that we need to implement background preparation.

Fixes swiftlang#620
Fixes swiftlang#1116
rdar://99329579
rdar://123971779
plemarquand pushed a commit to plemarquand/sourcekit-lsp that referenced this issue Apr 24, 2024
…ule` file has been changed

When the client sends us `workspace/didChangeWatchedFiles` notification of an updated `.swift` file, we should refresh the other open files in that module since they might be referencing functions from that updated file.

If a `.swiftmodule` file has been updated, we refresh all the files within the package since they might import that module. Technically, we would only need to refresh files that are in module that are downstream of the updated module but we don’t currently have that information easily available from SwiftPM. Also, usually, if the client has a file from a low-level module open, he’ll be working on that module which means that such an optimization won’t help. The real solution here is to wait for us to finish preparation (which we would exactly know when it finishes since sourcekit-lsp would schedule it) but for that we need to implement background preparation.

Fixes swiftlang#620
Fixes swiftlang#1116
rdar://99329579
rdar://123971779
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants