Skip to content

textDocument/documentSymbol returns empty result for non-existing files #301

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

Closed
rear1019 opened this issue Nov 18, 2022 · 3 comments · Fixed by #302
Closed

textDocument/documentSymbol returns empty result for non-existing files #301

rear1019 opened this issue Nov 18, 2022 · 3 comments · Fixed by #302
Assignees
Milestone

Comments

@rear1019
Copy link
Contributor

rear1019 commented Nov 18, 2022

textDocument/documentSymbol returns an empty result for non-existing files even though Jedi does provide symbol information. The culprit us the following code:

try:
docismodule = os.path.samefile(document.path, d.module_path)
except (TypeError, FileNotFoundError):
# Python 2 on Windows has no .samefile, but then these are
# strings for sure
docismodule = document.path == d.module_path

This can reproduced by creating a new file in Spyder: No symbol information is available in the Symbols Switcher and Outline plugin as long as the file is not saved¹. Code folding and linting does work.

¹ Note: Saving the new file is not sufficient to fix the issue in Spyder <= 5.4.0 due an another unrelated bug. See spyder/spyder-ide#20047.

EDIT: Formatting

@ccordoba12 ccordoba12 added this to the v1.7.0 milestone Nov 20, 2022
@ccordoba12
Copy link
Member

Hey @rear1019, thanks for reporting. What do you propose to fix the issue? Removing the try/except you referenced?

@rear1019
Copy link
Contributor Author

I am not quite sure, but I think that docismodule should be determined via pathlib.Path(document.path) == d.module_path¹ – that is, check the file paths for equivalence. os.path.samefile()² actually accesses the filesystem to check if the two paths point to the same file (think of hard links on UNIX). I don’t think that’s intended here.

I can make a PR if the proposed fix is deemed correct.

¹ Or a variant thereof, depending on the required Python/Jedi compatibility.
² https://docs.python.org/3.10/library/os.path.html#os.path.samefile

@ccordoba12
Copy link
Member

os.path.samefile()² actually accesses the filesystem to check if the two paths point to the same file (think of hard links on UNIX). I don’t think that’s intended here.

Right, then I agree with your proposed solution. Please submit a PR for it.

rear1019 added a commit to procitec/python-lsp-server that referenced this issue Nov 23, 2022
Add a test for `textDocument/documentSymbols` of a non-existing
(unsaved) file. The test fails as of this commit and is a regression
test for issue python-lsp#301 [1].

[1] python-lsp#301
rear1019 added a commit to procitec/python-lsp-server that referenced this issue Nov 23, 2022
Fix `textDocument/documentSymbols` returning an empty result for
non-existing (unsaved) files: Do not use `os.path.samefile()` which
accesses the file system to check if two file paths point to the same
file. Just compare the file paths. (This basically reverts commit
40afab3.)

This fixes issue python-lsp#301 [1].

[1] python-lsp#301
rear1019 added a commit to procitec/python-lsp-server that referenced this issue Nov 23, 2022
Fix `textDocument/documentSymbols` returning an empty result for
non-existing (unsaved) files: Do not use `os.path.samefile()` which
accesses the file system to check if two file paths point to the same
file. Just compare the file paths. (This basically reverts commit
40afab3.)

This fixes issue python-lsp#301 [1].

[1] python-lsp#301
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 a pull request may close this issue.

2 participants