SolaWing / xcode-build-server

a build server protocol implementation for integrate xcode with sourcekit-lsp
MIT License
424 stars 19 forks source link

False positives (for functions) and duplicates (for properties) for vim.lsp.buf.definition() #56

Closed hboon closed 5 months ago

hboon commented 5 months ago

Hi, I'm getting false positives for vim.lsp.buf.definition() of functions.

For example, when I am at the call site of the function fetchXML() at https://github.com/AlphaWallet/alpha-wallet-ios/blob/e870048dfde93923ce88d798d22c3bff8d1116f8/modules/AlphaWalletTokenScript/AlphaWalletTokenScript/Models/AssetDefinitionStore.swift#L130 and do vim.lsp.buf.definition(), I get these 2:

AssetDefinitionStore.swift|136 col 17| /// IMPLEMENTATION NOTE: Current implementation will fetch the same XML multiple times if this function is called again before the previous attempt has completed. A check (which requires tracking completion handlers) hasn't been implemented because this doesn't usually happen in practice
AssetDefinitionStore.swift|137 col 17| public func fetchXML(forContract contract: AlphaWallet.Address, server: RPCServer?, useCacheAndFetch: Bool = false, completionHandler: ((Result) -> Void)? = nil) {

The 1st entry is just the comment for the function 1 line above the function definition and the 2nd entry is the correct match.


I'm also getting duplicates for some type properties with vim.lsp.buf.definition(). eg.

When cursor is at https://github.com/AlphaWallet/alpha-wallet-ios/blob/e870048dfde93923ce88d798d22c3bff8d1116f8/AlphaWallet/Tokens/Collectibles/ViewModels/NFTAssetViewModel.swift#L35

I get these correct, but 2 duplicated matches:

NFTAssetViewModel.swift|35 col 17| private let service: TokensProcessingPipeline
NFTAssetViewModel.swift|35 col 17| private let service: TokensProcessingPipeline

Have tried cleaning and rebuilding in Xcode. Any idea what might be wrong? Or if you could suggest where to look, I might dig into the code a bit.

Thanks. xcode-build-server is a great tool.

SolaWing commented 5 months ago

This is indeed a long-standing problem, which may be related to the implementation of the symbol cache within LSP. To make the jump position accurate, you can try the following operations:

  1. Compile the app to update the index.
  2. Restart LSP.
  3. Delete ~/Library/cache/xcode-build-server to clear the database cache of LSP, and then restart LSP.
hboon commented 5 months ago

Same thing after steps 1-3. (I cleaned too and verified LSP and xcode-build-server were restarted/killed with ps)

Away for a bit. Will see if I can check out the code. Thanks.

SolaWing commented 5 months ago
  1. ensure you don't modify any code after compile. (modify code line will make indexstore outdated.)
  2. ensure clear xcode-build-server cache dir when editor is close and LSP is not alive, so when reopen editor, a new LSP will have to recompute the symbols from indexstore.

Do this still has problem?

hboon commented 5 months ago

Still observe the problem. I do this:

  1. Compile in Xcode. Wait till done
  2. Close Neovim
  3. Check no xcode-build-server instances running
  4. Delete the xcode-build-server cache for that project
  5. Start Neovim
  6. Look up definition with :lua vim.lsp.buf.definition()
SolaWing commented 5 months ago

Weird.. the symbol location info origin from the index store... It should no way to has duplicate locations...

Did you still get the problem if you use vscode with Swift Extensions?

hboon commented 5 months ago

I could check (I don’t use Code). Does extensions refer to the language feature or a Code extension? :)

I forgot to mention. I have at least 3 other, but smaller projects. They don’t have this problem. This is the only one that use CocoaPods.

SolaWing commented 5 months ago

https://www.swift.org/documentation/articles/getting-started-with-vscode-swift.html

just install it, and it should work

hboon commented 5 months ago

Same results in VS Code for the function definition (it's not duplicated like properties, in case I didn't communicate clearly)

But in VS Code, looking up references for that property crashed SourceKitService.

Interestingly, if I remove the function definition's comments, looking up references in both Neovim and VS Code gets this:

Screenshot 2024-06-09 at 10 55 06 AM

Now, the first line is the correct match and the second is a false positive.

I then tried this, adding a line of comment before the false positive:

    public func fetchXML(forContract contract: AlphaWallet.Address, server: RPCServer?, useCacheAndFetch: Bool = false, completionHandler: ((Result) -> Void)? = nil) {
        if useCacheAndFetch && backingStore.getXml(byContract: contract) != nil {
            //
            completionHandler?(.cached)
        }

And now lua vim.lsp.buf.definition() works as expected with exactly 1 result. Maybe it's a sourcekit LSP bug?

SolaWing commented 5 months ago

maybe. what your xcode and sourcekit version? if it is not the newest, does upgrade and clean build fix the issue? if you can upload a reproducible sample, may help debug the issue

hboon commented 5 months ago

I was using Xcode 15.2, but just tried with Xcode 15.4 and restarted things to make sure sourcekit-ls is the newer version:

$ ps ax | grep sourcekit-ls

22405   ??  Ss     0:00.68 /Applications/Xcode-15.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/sourcekit-lsp
22603 s010  S+     0:00.00 grep --color=auto sourcekit-ls

Same results.


The code link in the 1st comment is open source if you have the chance to try. I think this should get ready the project/workspace for building in Xcode: make bootstrap (installs xcodegen, gems/pods and creates workspace). Then just have to build the workspace and reproduce with those 2 instances in the same comment.

On my side, I'll do a little workaround for now and then swing back to explore further. But thanks for the help so far!

SolaWing commented 5 months ago

In my project, I seems to also reproduce this issue, multiple location and not fixed when delete xcode-build-server cache. so the duplicated location may come from index-store?

after I try to delete ~/Library/Developer/Xcode/DerivedData/ to clear all build cache, it seems fixed.

hboon commented 5 months ago

Huh interesting. I deleted the sub-directories in ~/Library/Developer/Xcode/DerivedData/ that might match and yeah those 2 examples worked. I notice looking up definitions for the fetchXML(forContract:server:useCacheAndFetch:completionHandler:) only go to the other reference now. It's also telling that you could reproduce it as a fresh clone and it work after a rebuild…

It doesn't look like it's something we can fix though. Sorry that it's taken so much of your time. Hopefully it's at least worth knowing about.