Stop treating non-file document URIs as filesystem paths

CLAUDE.md

@@ -0,0 +1,67 @@
+# CLAUDE.md
+
+Guidance for [Claude Code](https://claude.com/claude-code) when working in this repository.
+
+## What this repository is
+
+`pyls` is an implementation of the Language Server Protocol for Python. An editor speaks JSON-RPC to it over stdio or TCP; the server parses each open document and answers completion, definition, hover, reference, symbol and formatting requests, mostly by delegating to Jedi and to linters loaded as pluggy plugins.
+
+Two things shape almost every decision here:
+
+- **It still targets Python 2.7.** `setup.py` carries `python_version<"3"` markers and CI runs a 2.7 job. That rules out f-strings, and it is why source literals in tests carry `u''` prefixes.
+- **Jedi is pinned below 0.18** (`jedi>=0.17.2,<0.18.0`). The 0.18 release changed the `Script` API, so the pin is load-bearing, not incidental.
+
+Development here has been quiet since December 2020 — the most recent commit is an automated config change — and the actively maintained community fork is `python-lsp-server`. Worth knowing before promising a change will ship, but this repository is not archived and still accepts issues and PRs.
+
+## Setting up an environment that works
+
+**A modern interpreter will not work.** CI runs 2.7, 3.6, 3.7 and 3.8, and the pinned Jedi does not install on recent Pythons. Use 3.8:
+
+```
+uv python install 3.8
+uv venv --python 3.8 /tmp/pls
+uv pip install --python /tmp/pls/bin/python -e ".[all,test]"
+uv pip install --python /tmp/pls/bin/python "setuptools<70" "pylint<3"
+```
+
+The last line is not optional:
+
+- `pyls/config/config.py` imports `pkg_resources`, which setuptools 70+ no longer ships.
+- `test/plugins/test_pylint_lint.py` imports `pylint.epylint`, removed in pylint 3.
+
+## Commands
+
+These are the four steps CI runs, in order:
+
+```
+/tmp/pls/bin/python -m pytest test/ -q
+/tmp/pls/bin/python -m pylint pyls test
+/tmp/pls/bin/python -m pycodestyle pyls test
+/tmp/pls/bin/python -m pyflakes pyls test
+```
+
+**A clean checkout does not produce a clean run.** Record a baseline before changing anything and compare against it, rather than assuming a failure is yours. On Python 3.8 with current dependency versions a clean checkout gives roughly **12 failed, 99 passed, 8 skipped**, concentrated in the flake8, pydocstyle, pylint and numpy-hover tests — these track linter versions that have moved on since 2020. `pylint pyls test` reports around 84 messages and `pyflakes` reports an undefined `unicode` in `pyls/_utils.py`, both pre-existing.
+
+## URIs are not always files
+
+A document is identified by a URI, and **not every URI has a file behind it**. Editors such as Monaco send `inmemory://dummy.py`, and VS Code sends `untitled:` and `vscode-notebook-cell:`; the contents of those documents only ever arrive over `textDocument/didOpen` and `didChange`.
+
+- `uris.to_fs_path()` returns a path-shaped **identifier**. For a non-file URI it is not a location on disk and must not be opened, walked, or used to derive a directory.
+- `uris.is_file_uri()` is the check to use before doing anything filesystem-flavoured with `Document.path`, and `Document.is_file_backed` caches it.
+- Note that `inmemory://dummy.py` puts `dummy.py` in the URI *authority*, not the path, so `urlparse` yields an empty path for it. That is a normal shape, not a malformed URI.
+
+Anything that computes `os.path.dirname(document.path)` and feeds it to Jedi, rope or a linter needs to be guarded by `is_file_backed`. An unguarded `dirname` of a non-file document used to normalize to `'.'`, which silently put the server's working directory on the module search path.
+
+## Conventions
+
+- **Every source file starts with `# Copyright 2017 Palantir Technologies, Inc.`** Copy it from a neighbouring file.
+- **Write Python 2/3 compatible code.** No f-strings; use `.format()`. `pylint` will suggest `consider-using-f-string` anyway — that suggestion does not apply here.
+- **Plugins are pluggy hooks** registered as entry points in `setup.py`. Adding a plugin means adding both the module and its `pyls` entry point.
+- **Tests use plain pytest with fixtures in `test/fixtures.py`** (`workspace`, `doc`, `config`, `pyls`), re-exported through `test/conftest.py`. `from test.fixtures import ...` is deliberately the first import in a test module: `test` shadows a stdlib name, so pylint sorts it as a standard import and reports `wrong-import-order` if anything precedes it.
+
+## Things to be careful about
+
+- **Do not "modernize" the codebase.** Removing `u''` prefixes, `object` base classes or `.format()` calls breaks the Python 2.7 job.
+- **Do not bump Jedi past 0.18** without reworking `Document.jedi_script`; the `Script` constructor signature changed.
+- **`pyls/uris.py` mirrors VS Code's `vscode-uri`** and says so. If you diverge from it, say why in a comment, because the next reader will check the two against each other.
+- **Changing `to_fs_path` affects everything.** It feeds `Document.path`, the workspace root, config discovery and most plugins. Verify plugin behaviour, not just the URI unit tests.

pyls/uris.py

@@ -43,12 +43,26 @@ def urlunparse(parts):
     ))
 
 
+def is_file_uri(uri):
+    """Return whether the URI refers to a path on the local filesystem.
+
+    A URI with no scheme is treated as a file URI, matching how editors send bare
+    paths. Everything else (inmemory:, untitled:, vscode-notebook-cell:, http:, ...)
+    has no file behind it, so its contents only exist in memory.
+    """
+    return urlparse(uri)[0] in ('file', '')
+
+
 def to_fs_path(uri):
     """Returns the filesystem path of the given URI.
 
     Will handle UNC paths and normalize windows drive letters to lower-case. Also
     uses the platform specific path separator. Will *not* validate the path for
     invalid characters and semantics. Will *not* look at the scheme of this URI.
+
+    For a non-file URI the result is a path-shaped identifier rather than a real
+    location on disk, and callers must not assume it can be opened. Use is_file_uri
+    to tell the two apart.
     """
     # scheme://netloc/path;parameters?query#fragment
     scheme, netloc, path, _params, _query, _fragment = urlparse(uri)
@@ -57,6 +71,14 @@ def to_fs_path(uri):
         # unc path: file://shares/c$/far/boo
         value = "//{}{}".format(netloc, path)
 
+    elif netloc and scheme not in ('file', ''):
+        # Non-file URI with an authority, such as Monaco's inmemory://dummy.py or
+        # vscode-notebook-cell://notebook/cell.py. vscode-uri drops the authority here
+        # and returns just the path, which is empty for inmemory://dummy.py. That left
+        # the document with no name at all, so the authority is kept instead: the
+        # document still needs a stable identity for module naming and diagnostics.
+        value = "/{}{}".format(netloc, path)
+
     elif RE_DRIVE_LETTER_PATH.match(path):
         # windows drive letter: file:///C:/far/boo
         value = path[1].lower() + path[2:]

pyls/workspace.py

@@ -135,6 +135,10 @@ def __init__(self, uri, workspace, source=None, version=None, local=True, extra_
         self.path = uris.to_fs_path(uri)
         self.dot_path = _utils.path_to_dot_name(self.path)
         self.filename = os.path.basename(self.path)
+        # Documents opened under a scheme such as inmemory: or untitled: have no file
+        # behind them. self.path is a stable identifier for those, not a location that
+        # can be opened or walked.
+        self.is_file_backed = uris.is_file_uri(uri)
 
         self._config = workspace._config
         self._workspace = workspace
@@ -160,6 +164,15 @@ def lines(self):
     @lock
     def source(self):
         if self._source is None:
+            if not self.is_file_backed:
+                # Nothing to fall back to: the contents only ever arrive over
+                # textDocument/didOpen and didChange. Opening self.path would either
+                # raise a confusing error about a path the client never mentioned, or
+                # read an unrelated file that happens to share the name.
+                raise ValueError(
+                    'no source available for {}: a document with a non-file URI must be '
+                    'opened before it can be read'.format(self.uri)
+                )
             with io.open(self.path, 'r', encoding='utf-8') as f:
                 return f.read()
         return self._source
@@ -262,12 +275,15 @@ def jedi_script(self, position=None, use_document_path=False):
         project_path = self._workspace.root_path
 
         # Extend sys_path with document's path if requested
-        if use_document_path:
+        if use_document_path and self.is_file_backed:
+            # Skipped for non-file documents: os.path.dirname of their identifier is not
+            # a real directory. It used to normalize to '.', which silently put the
+            # server's working directory on the module search path.
             sys_path += [os.path.normpath(os.path.dirname(self.path))]
 
         kwargs = {
             'code': self.source,
-            'path': self.path,
+            'path': self.path or None,
             'environment': environment,
             'project': jedi.Project(path=project_path, sys_path=sys_path),
         }

test/test_document.py

@@ -1,5 +1,16 @@
 # Copyright 2017 Palantir Technologies, Inc.
 from test.fixtures import DOC_URI, DOC
+import os
+
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
+import jedi
+import pytest
+
+from pyls import uris
 from pyls.workspace import Document
 
 
@@ -97,3 +108,69 @@ def test_document_end_of_file_edit(workspace):
         "print 'b'\n",
         "o",
     ]
+
+
+INMEMORY_URI = 'inmemory://dummy.py'
+
+
+def test_non_file_document_props(workspace):
+    """A document under a non-file scheme still needs a usable name.
+
+    Monaco sends inmemory://dummy.py, which puts the name in the URI authority. That used
+    to resolve to an empty path, leaving the document with no filename or module name.
+    """
+    document = Document(INMEMORY_URI, workspace, u'import sys')
+    assert document.uri == INMEMORY_URI
+    assert document.path == '/dummy.py'
+    assert document.filename == 'dummy.py'
+    assert document.dot_path == 'dummy'
+    assert document.is_file_backed is False
+
+
+def test_non_file_document_source(workspace):
+    document = Document(INMEMORY_URI, workspace, u'import sys')
+    assert document.source == u'import sys'
+
+
+def test_non_file_document_without_source_raises(workspace):
+    """There is no file to fall back to, so say so rather than opening something else."""
+    document = Document(INMEMORY_URI, workspace)
+    with pytest.raises(ValueError) as excinfo:
+        document.source  # pylint: disable=pointless-statement
+    assert INMEMORY_URI in str(excinfo.value)
+
+
+def test_non_file_document_does_not_extend_sys_path(workspace):
+    """os.path.dirname of a non-file document is not a directory.
+
+    It previously normalized to '.', which put the server's working directory on the
+    module search path for any in-memory document.
+    """
+    document = Document(INMEMORY_URI, workspace, u'import sys')
+    with mock.patch('jedi.Project', wraps=jedi.Project) as project:
+        document.jedi_script(use_document_path=True)
+        sys_path = project.call_args[1]['sys_path']
+    assert os.getcwd() not in sys_path
+    assert '/' not in sys_path
+
+
+def test_file_document_still_extends_sys_path(tmpdir, workspace):
+    """The file-backed behaviour is unchanged."""
+    subdir = tmpdir.mkdir('sub')
+    source_file = subdir.join('real.py')
+    source_file.write('x = 1')
+
+    document = Document(uris.from_fs_path(str(source_file)), workspace, u'x = 1')
+    assert document.is_file_backed is True
+    with mock.patch('jedi.Project', wraps=jedi.Project) as project:
+        document.jedi_script(use_document_path=True)
+        sys_path = project.call_args[1]['sys_path']
+    assert str(subdir) in sys_path
+
+
+def test_non_file_document_completions(workspace):
+    """The case from the issue: completions inside an in-memory document."""
+    document = Document(INMEMORY_URI, workspace, u'import sys\nsys.')
+    script = document.jedi_script(use_document_path=True)
+    completions = [completion.name for completion in script.complete(2, 4)]
+    assert 'argv' in completions

test/test_uris.py

@@ -14,6 +14,31 @@ def test_to_fs_path(uri, path):
     assert uris.to_fs_path(uri) == path
 
 
+@unix_only
+@pytest.mark.parametrize('uri,path', [
+    # Monaco sends inmemory://name.py, where the name lands in the authority rather than
+    # the path. Dropping the authority left these documents with no name at all.
+    ('inmemory://dummy.py', '/dummy.py'),
+    ('inmemory:///dummy.py', '/dummy.py'),
+    ('vscode-notebook-cell://notebook/cell.py', '/notebook/cell.py'),
+    ('untitled:Untitled-1', 'Untitled-1'),
+])
+def test_non_file_uri_to_fs_path(uri, path):
+    assert uris.to_fs_path(uri) == path
+
+
+@pytest.mark.parametrize('uri,is_file', [
+    ('file:///foo/bar', True),
+    ('/foo/bar', True),
+    ('inmemory://dummy.py', False),
+    ('untitled:Untitled-1', False),
+    ('vscode-notebook-cell://notebook/cell.py', False),
+    ('http://example.com/foo.py', False),
+])
+def test_is_file_uri(uri, is_file):
+    assert uris.is_file_uri(uri) is is_file
+
+
 @windows_only
 @pytest.mark.parametrize('uri,path', [
     ('file:///c:/far/boo', 'c:\\far\\boo'),