Skip to content
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

Fix empty cwd value for pylint #371

Merged
merged 2 commits into from
May 15, 2023
Merged

Fix empty cwd value for pylint #371

merged 2 commits into from
May 15, 2023

Conversation

Ultimator14
Copy link
Contributor

Fixes #369

A shorter way to write this would be to use the walrus operator but that would require Python 3.8+ and the README states this project is Python 3.7+.

if not (cwd := document._workspace.root_path):
    cwd = os.path.dirname(__file__)

@ccordoba12 ccordoba12 added this to the v1.7.3 milestone Apr 25, 2023
@ccordoba12
Copy link
Member

ccordoba12 commented Apr 27, 2023

@Ultimator14, thanks a lot for your contribution! Let me know if you need help to fix the linting issues reported by our CI.

@Ultimator14
Copy link
Contributor Author

Ultimator14 commented Apr 27, 2023

@ccordoba12 thanks, AFAICS pylint complains about too many if's and variables. I don't really know how to rewrite the commit with less of both. If you have an idea feel free to make the changes.

The only thing I can think of is editing some of the existing code to match the tests.
My ideas:
Too many branches

diff --git a/pylsp/plugins/pylint_lint.py b/pylsp/plugins/pylint_lint.py
index 7daaf18..464d3f1 100644
--- a/pylsp/plugins/pylint_lint.py
+++ b/pylsp/plugins/pylint_lint.py
@@ -156,13 +156,9 @@ class PylintLinter:
                 },
             }
 
-            if diag['type'] == 'convention':
+            if diag['type'] in ['convention', 'information']:
                 severity = lsp.DiagnosticSeverity.Information
-            elif diag['type'] == 'information':
-                severity = lsp.DiagnosticSeverity.Information
-            elif diag['type'] == 'error':
-                severity = lsp.DiagnosticSeverity.Error
-            elif diag['type'] == 'fatal':
+            elif diag['type'] in ['error', 'fatal']:
                 severity = lsp.DiagnosticSeverity.Error
             elif diag['type'] == 'refactor':
                 severity = lsp.DiagnosticSeverity.Hint

For 'Too many local variables' I don't find an unneeded variable. Maybe the whole code calling pylint could be moved to a separate function because the variables used there are not used again afterwards.

diff --git a/pylsp/plugins/pylint_lint.py b/pylsp/plugins/pylint_lint.py
index 464d3f1..5ee2f56 100644
--- a/pylsp/plugins/pylint_lint.py
+++ b/pylsp/plugins/pylint_lint.py
@@ -47,6 +47,35 @@ UNNECESSITY_CODES = {
 class PylintLinter:
     last_diags = collections.defaultdict(list)
 
+    @classmethod
+    def call_pylint(cls, document, flags):
+        cmd = [
+            sys.executable,
+            '-c',
+            'import sys; from pylint.lint import Run; Run(sys.argv[1:])',
+            '-f',
+            'json',
+            document.path
+        ] + (shlex.split(str(flags)) if flags else [])
+        log.debug("Calling pylint with '%s'", ' '.join(cmd))
+
+        cwd = document._workspace.root_path
+
+        if not cwd:
+            cwd = os.path.dirname(__file__)
+
+        with Popen(cmd, stdout=PIPE, stderr=PIPE,
+                   cwd=cwd, 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)
+
+        return json_out
+
+
     @classmethod
     def lint(cls, document, is_saved, flags=''):
         """Plugin interface to pylsp linter.
@@ -85,29 +114,7 @@ class PylintLinter:
             # save.
             return cls.last_diags[document.path]
 
-        cmd = [
-            sys.executable,
-            '-c',
-            'import sys; from pylint.lint import Run; Run(sys.argv[1:])',
-            '-f',
-            'json',
-            document.path
-        ] + (shlex.split(str(flags)) if flags else [])
-        log.debug("Calling pylint with '%s'", ' '.join(cmd))
-
-        cwd = document._workspace.root_path
-
-        if not cwd:
-            cwd = os.path.dirname(__file__)
-
-        with Popen(cmd, stdout=PIPE, stderr=PIPE,
-                   cwd=cwd, 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)
+        json_out = cls.call_pylint(document, flags)
 
         # pylint prints nothing rather than [] when there are no diagnostics.
         # json.loads will not parse an empty string, so just return.

@ccordoba12
Copy link
Member

AFAICS pylint complains about too many if's and variables

Yep, but I think the easiest thing to do is to skip those warnings because I don't see the need to significantly change the code due to them.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Ultimator14 for your contribution!

I fixed the linting issues so we can merge your PR and release a new version.

@ccordoba12 ccordoba12 merged commit 784c6a9 into python-lsp:develop May 15, 2023
10 checks passed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 21, 2023
## Version 1.7.3 (2023/05/15)

### Issues Closed

* [Issue 369](python-lsp/python-lsp-server#369) - Failed to load hook pylsp_lint: [Errno 2] No such file or directory: '' ([PR 371](python-lsp/python-lsp-server#371) by [@Ultimator14](https://github.com/Ultimator14))

In this release 1 issue was closed.

### Pull Requests Merged

* [PR 377](python-lsp/python-lsp-server#377) - Update yapf requirement to 0.33+, by [@bnavigator](https://github.com/bnavigator)
* [PR 371](python-lsp/python-lsp-server#371) - Fix empty cwd value for pylint, by [@Ultimator14](https://github.com/Ultimator14) ([369](python-lsp/python-lsp-server#369))
* [PR 364](python-lsp/python-lsp-server#364) - Add Arch Linux installation command to Readme, by [@GNVageesh](https://github.com/GNVageesh)

In this release 3 pull requests were closed.
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 this pull request may close these issues.

Failed to load hook pylsp_lint: [Errno 2] No such file or directory: ''
2 participants