Fix crash in pylsp_definitions on definitions without a source position#715
Open
metheoryt wants to merge 1 commit into
Open
Fix crash in pylsp_definitions on definitions without a source position#715metheoryt wants to merge 1 commit into
metheoryt wants to merge 1 commit into
Conversation
When follow_builtin_definitions is True (the default), the return-list comprehension filter short-circuits `follow_builtin_defns or _not_internal_definition(d)`, so the None-guards inside _not_internal_definition never run. A jedi definition that resolves into a compiled/builtin module has line/column == None, so `d.line - 1` raises `TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'`. Exclude definitions with no line/column unconditionally: they cannot form a valid LSP range regardless of the follow_builtin_definitions setting. Fixes python-lsp#623 Fixes python-lsp#624 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
metheoryt
added a commit
to metheoryt/machines
that referenced
this pull request
Jul 8, 2026
Now that the pylsp_definitions None-guard fix is committed upstream as python-lsp/python-lsp-server#715, build python-lsp-server from that fork commit (metheoryt/python-lsp-server @ e4ee218) rather than carrying the change as a local .patch. Overrides src/version only and keeps nixpkgs' own patches (jedi-compat). Verified: builds and the full test suite — including the new in-tree regression test — passes. Revert to plain nixpkgs once #715 merges and ships in a release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
pylsp_definitionsraises on sometextDocument/definitionrequests:Reported in #623 and #624.
Root cause
The default
follow_builtin_definitionsisTrue, and the return-list filter is:_not_internal_definition(d)is the only placed.line/d.columnare checked forNone, but thefollow_builtin_defns or ...short-circuit means it is neverevaluated when the setting is left at its default. When jedi resolves a definition
into a compiled/builtin module,
d.lineandd.columnareNone, sod.line - 1crashes.
Fix
Exclude definitions that have no
line/columnunconditionally — such a definitioncannot be expressed as a valid LSP range regardless of the
follow_builtin_definitionssetting. This is a one-clause addition to the existingfilter and leaves the behaviour of builtins that do carry a position (e.g. the
builtins.pyistub covered bytest_builtin_definition) unchanged.Test
Adds
test_definition_without_source_position, which patchesjedi'sgototoreturn a definition with
line=None/column=Noneand assertspylsp_definitionsreturns
[]instead of raising. It fails ondevelop(with the exactTypeErrorabove) and passes with this change.
Design note
I went with dropping positionless definitions since they can't form a usable LSP
location. An alternative would be to keep them and clamp to line 0 (
(d.line or 1) - 1)so navigation still lands at the top of
module_pathwhen one exists — happy toswitch to that if maintainers prefer preserving the result over dropping it.
Fixes #623
Fixes #624