Repository: python-lsp/python-lsp-server
Pull request #322 (2022-12-28 15:26:26)

Change Pylint run to set cwd correctly
Since `__init__.py` are not necessary using the pylint plugins will miss configuration files is such files are not present.

### Explanation

In `pylsp.plugins/pylint_lint.py` the use `py_run` (from `pylint.epylint`) will not use the proper working directory.

If `__init__.py` are not present as `py_run` will call `lint` from the same file. The `lint` function traverse parent folder as long as a `__init__.py` is present (see https://github.com/PyCQA/pylint/blob/main/pylint/epylint.py#L65).

Directly calling `Run` from `pylint.lint` can enable proper usage of `cwd` exactly as the flake8 plugins does (set to `document._workspace.root_path`, see https://github.com/python-lsp/python-lsp-server/blob/develop/pylsp/plugins/flake8_lint.py#L114).

### Additional Points

`py_run` is deprecated and to be removed in pylint 3.0 : https://github.com/PyCQA/pylint/blob/main/pylint/epylint.py#L171

If anything can be improved in this PR I will gladly make any change suggested.


Patch:
diff --git a/CONFIGURATION.md b/CONFIGURATION.md
index aed5627..f2626e4 100644
--- a/CONFIGURATION.md
+++ b/CONFIGURATION.md
@@ -32,6 +32,7 @@ This server can be configured using the `workspace/didChangeConfiguration` metho
 | `pylsp.plugins.jedi_definition.enabled` | `boolean` | Enable or disable the plugin. | `true` |
 | `pylsp.plugins.jedi_definition.follow_imports` | `boolean` | The goto call will follow imports. | `true` |
 | `pylsp.plugins.jedi_definition.follow_builtin_imports` | `boolean` | If follow_imports is True will decide if it follow builtin imports. | `true` |
+| `pylsp.plugins.jedi_definition.follow_builtin_definitions` | `boolean` | Follow builtin and extension definitions to stubs. | `true` |
 | `pylsp.plugins.jedi_hover.enabled` | `boolean` | Enable or disable the plugin. | `true` |
 | `pylsp.plugins.jedi_references.enabled` | `boolean` | Enable or disable the plugin. | `true` |
 | `pylsp.plugins.jedi_signature_help.enabled` | `boolean` | Enable or disable the plugin. | `true` |
diff --git a/pylsp/config/schema.json b/pylsp/config/schema.json
index 7e23500..4ac085d 100644
--- a/pylsp/config/schema.json
+++ b/pylsp/config/schema.json
@@ -176,6 +176,11 @@
       "default": true,
       "description": "If follow_imports is True will decide if it follow builtin imports."
     },
+    "pylsp.plugins.jedi_definition.follow_builtin_definitions": {
+      "type": "boolean",
+      "default": true,
+      "description": "Follow builtin and extension definitions to stubs."
+    },
     "pylsp.plugins.jedi_hover.enabled": {
       "type": "boolean",
       "default": true,
diff --git a/pylsp/plugins/definition.py b/pylsp/plugins/definition.py
index bf707b7..b2110af 100644
--- a/pylsp/plugins/definition.py
+++ b/pylsp/plugins/definition.py
@@ -17,6 +17,7 @@ def pylsp_definitions(config, workspace, document, position):
             follow_builtin_imports=settings.get('follow_builtin_imports', True),
             **code_position)
 
+        follow_builtin_defns = settings.get("follow_builtin_definitions", True)
         return [
             {
                 'uri': uris.uri_with(document.uri, path=str(d.module_path)),
@@ -25,7 +26,7 @@ def pylsp_definitions(config, workspace, document, position):
                     'end': {'line': d.line - 1, 'character': d.column + len(d.name)},
                 }
             }
-            for d in definitions if d.is_definition() and _not_internal_definition(d)
+            for d in definitions if d.is_definition() and (follow_builtin_defns or _not_internal_definition(d))
         ]
 
 
diff --git a/pylsp/plugins/pylint_lint.py b/pylsp/plugins/pylint_lint.py
index 459afbe..452f45b 100644
--- a/pylsp/plugins/pylint_lint.py
+++ b/pylsp/plugins/pylint_lint.py
@@ -10,7 +10,6 @@ import re
 from subprocess import Popen, PIPE
 import os
 
-from pylint.epylint import py_run
 from pylsp import hookimpl, lsp
 
 try:
@@ -85,20 +84,21 @@ class PylintLinter:
             # save.
             return cls.last_diags[document.path]
 
-        # py_run will call shlex.split on its arguments, and shlex.split does
-        # not handle Windows paths (it will try to perform escaping). Turn
-        # backslashes into forward slashes first to avoid this issue.
-        path = document.path
-        if sys.platform.startswith('win'):
-            path = path.replace('\\', '/')
-
-        pylint_call = '{} -f json {}'.format(path, flags)
-        log.debug("Calling pylint with '%s'", pylint_call)
-        json_out, err = py_run(pylint_call, return_std=True)
-
-        # Get strings
-        json_out = json_out.getvalue()
-        err = err.getvalue()
+        cmd = [
+            'python',
+            '-c',
+            'import sys; from pylint.lint import Run; Run(sys.argv[1:])',
+            '-f',
+            'json',
+            document.path
+        ] + (str(flags).split(' ') if flags else [])
+        log.debug("Calling pylint with '%s'", ' '.join(cmd))
+
+        with Popen(cmd, stdout=PIPE, stderr=PIPE,
+                   cwd=document._workspace.root_path, universal_newlines=True) as process:
+            process.wait()
+            json_out = process.stdout.read()
+            err = process.stderr.read()
 
         if err != '':
             log.error("Error calling pylint: '%s'", err)
diff --git a/pylsp/plugins/symbols.py b/pylsp/plugins/symbols.py
index 939dcda..d5925db 100644
--- a/pylsp/plugins/symbols.py
+++ b/pylsp/plugins/symbols.py
@@ -91,7 +91,7 @@ def pylsp_document_symbols(config, document):
                 else:
                     continue
 
-        if _include_def(d) and Path(document.path) == d.module_path:
+        if _include_def(d) and Path(document.path) == Path(d.module_path):
             tuple_range = _tuple_range(d)
             if tuple_range in exclude:
                 continue


Test patch:
diff --git a/test/plugins/test_definitions.py b/test/plugins/test_definitions.py
index bcc7648..a8972cd 100644
--- a/test/plugins/test_definitions.py
+++ b/test/plugins/test_definitions.py
@@ -42,9 +42,24 @@ def test_builtin_definition(config, workspace):
     # Over 'i' in dict
     cursor_pos = {'line': 8, 'character': 24}
 
-    # No go-to def for builtins
     doc = Document(DOC_URI, workspace, DOC)
-    assert not pylsp_definitions(config, workspace, doc, cursor_pos)
+    orig_settings = config.settings()
+
+    # Check definition for `dict` goes to `builtins.pyi::dict`
+    follow_defns_setting = {'follow_builtin_definitions': True}
+    settings = {'plugins': {'jedi_definition': follow_defns_setting}}
+    config.update(settings)
+    defns = pylsp_definitions(config, workspace, doc, cursor_pos)
+    assert len(defns) == 1
+    assert defns[0]["uri"].endswith("builtins.pyi")
+
+    # Check no definitions for `dict`
+    follow_defns_setting['follow_builtin_definitions'] = False
+    config.update(settings)
+    defns = pylsp_definitions(config, workspace, doc, cursor_pos)
+    assert not defns
+
+    config.update(orig_settings)
 
 
 def test_assignment(config, workspace):
diff --git a/test/plugins/test_pylint_lint.py b/test/plugins/test_pylint_lint.py
index 4e63781..3eae891 100644
--- a/test/plugins/test_pylint_lint.py
+++ b/test/plugins/test_pylint_lint.py
@@ -3,11 +3,12 @@
 # Copyright 2021- Python Language Server Contributors.
 
 import contextlib
+from pathlib import Path
 import os
 import tempfile
 
 from pylsp import lsp, uris
-from pylsp.workspace import Document
+from pylsp.workspace import Document, Workspace
 from pylsp.plugins import pylint_lint
 
 DOC_URI = uris.from_fs_path(__file__)
@@ -90,8 +91,9 @@ def test_lint_free_pylint(config, workspace):
     # Can't use temp_document because it might give us a file that doesn't
     # match pylint's naming requirements. We should be keeping this file clean
     # though, so it works for a test of an empty lint.
+    ws = Workspace(str(Path(__file__).absolute().parents[2]), workspace._endpoint)
     assert not pylint_lint.pylsp_lint(
-        config, workspace, Document(uris.from_fs_path(__file__), workspace), True)
+        config, ws, Document(uris.from_fs_path(__file__), ws), True)
 
 
 def test_lint_caching(workspace):