Skip to content

Commit feb6cc0

Browse files
committed
Enable strict mode, update docs, use internal Settings object
Updates the default pyre configuration to be strict, and updates the documentation to match. Introduces an internal settigs object, because I'm not a fan of dicts and the typing thereof (yes, TypedDict exists). * Introduces the Settings dataclass * Uses it to convert the pylsp settings dict to a typed object * Defaults pyre to strict mode when writing out the initial config file * Updates the documentation to reflect this strict mode * Updates the documentation to explain squelching of Pyre lints * Adds source commentary about pyre's output vs LSP w.r.t. lints per file * Bumps the version
1 parent 36e2220 commit feb6cc0

File tree

4 files changed

+67
-23
lines changed

4 files changed

+67
-23
lines changed

.pyre_configuration

+4-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
],
66
"exclude": [
77
"/setup.py",
8-
".*/build/.*"
9-
]
8+
".*/build/.*",
9+
".*/.pyre/.*"
10+
],
11+
"strict": true
1012
}

README.md

+28-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ This is a plugin for the [Python LSP Server](https://github.com/python-lsp/pytho
44

55
It implements support for calling Meta's [Pyre type checker](https://github.com/facebook/pyre-check) via a subprocess.
66

7+
Pyre does offer a language server of its own, and that may be more useful to you if your editor supports multiple language servers per language.
8+
9+
## Features
10+
11+
This plugin adds the following features to `python-lsp-server`:
12+
13+
- Type linting via Meta's Pyre (pyre-check)
14+
715
## Installation
816

917
To use this plugin, you need to install this plugin in the same virtualenv as `python-lsp-server` itself.
@@ -43,19 +51,33 @@ The configuration written by this plugin is:
4351
"exclude": [
4452
"/setup.py",
4553
".*/build/.*"
46-
]
54+
],
55+
"strict": true
4756
}
4857
```
4958

50-
The noteable difference from `pyre init` is the change to the search strategy (pep561 to all).
59+
The noteable difference from `pyre init` is the change to the search strategy ("pep561" to "all"), and turning on strict mode as the default. You may find strict mode a bit pedantic, but having worked with strict mode for several years, I highly recommend it.
5160

52-
If the file is not present, the LSP error log, LSP output, and your editor's LSP messages will display an ABEND message containing the error from Pyre as it fails to run.
61+
If the `.pyre_configuration` file is not present (or has a syntax error), the LSP error log, LSP output, and your editor's LSP messages will display an ABEND message containing the error from Pyre as it fails to run.
5362

54-
## Features
63+
## Squelching Pyre lint errors
5564

56-
This plugin adds the following features to `python-lsp-server`:
65+
The recommended way of squelching a Pyre warning is to pick one of `# pyre-ignore` or `# pyre-fixme`. More precisely, suppose Pyre is indicating
5766

58-
- Type linting via Meta's Pyre (pyre-check)
67+
```
68+
Missing global annotation [5]: Globally accessible variable `logger` has type `logging.Logger` but no type is specified.
69+
```
70+
71+
at you, and you do not feel like typing your logger right now. On the line before, you can put either one of
72+
73+
* `# pyre-ignore[5] Don't care about logger`
74+
* `# pyre-fixme[5] Resolve this when doing all logger work`
75+
76+
to squelch the lint, and provide a hint to future you (or other readers of the code). This is a trivial example; it's easier to just type the logger.
77+
78+
You do not need to match the number in the brackets, other than for ease of cross-reference with the [type errors documentation](https://pyre-check.org/docs/errors/).
79+
80+
When you address the squelched error, Pyre will indicate that the comment is not suppressing a type error and can be removed.
5981

6082
## Developing
6183

pylsp_pyre/plugin.py

+34-14
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import dataclasses as dc
12
import json
23
import logging
34
import subprocess
@@ -14,7 +15,19 @@
1415

1516
logger: logging.Logger = logging.getLogger(__name__)
1617
# A logging prefix.
17-
PLUGIN: str = "[python-lsp-pyre]"
18+
PLUGIN = "[python-lsp-pyre]"
19+
20+
21+
@dc.dataclass
22+
class Settings:
23+
enabled: bool = dc.field(init=False)
24+
create_pyre_config: bool = dc.field(init=False)
25+
26+
def from_pylsp(self, config: pylsp_conf.Config) -> "Settings":
27+
settings = config.plugin_settings("pyre")
28+
self.enabled = getattr(settings, "enabled", True)
29+
self.create_pyre_config = getattr(settings, "create-pyre-config", False)
30+
return self
1831

1932

2033
@hookimpl
@@ -43,11 +56,13 @@ def pylsp_lint(
4356
Lints files (saved, not in-progress) and returns found problems.
4457
"""
4558
logger.debug(f"Working with {document.path}, {is_saved=}")
46-
maybe_create_pyre_config(config=config, workspace=workspace)
4759
if is_saved:
60+
plugin_settings = Settings().from_pylsp(config=config)
61+
maybe_create_pyre_config(settings=plugin_settings, workspace=workspace)
4862
with workspace.report_progress("lint: pyre check", "running"):
49-
settings = config.plugin_settings("pyre")
50-
diagnostics = run_pyre(workspace=workspace, document=document, settings=settings)
63+
diagnostics = run_pyre(
64+
workspace=workspace, document=document, settings=plugin_settings
65+
)
5166
workspace.show_message(message=f"Pyre reported {len(diagnostics)} issue(s).")
5267
# Deal with location stuff by using unstructure() for now.
5368
return lsp_con.get_converter().unstructure(diagnostics)
@@ -79,14 +94,19 @@ def abend(message: str, workspace: pylsp_ws.Workspace) -> Dict[str, Any]:
7994

8095

8196
def run_pyre(
82-
workspace: pylsp_ws.Workspace, document: pylsp_ws.Document, settings: Dict
97+
workspace: pylsp_ws.Workspace, document: pylsp_ws.Document, settings: Settings
8398
) -> List[Dict[str, Any]]:
8499
"""
85100
Calls Pyre, converts output to internal structs
86101
"""
87102
try:
88-
data = really_run_pyre(root_path=workspace.root_path)
89-
data = json.loads(data.decode("utf-8"))
103+
pyre_out = really_run_pyre(root_path=workspace.root_path)
104+
data = json.loads(pyre_out.decode("utf-8"))
105+
# Pyre checks all files in the project, but at least with Kate, the LSP response is
106+
# collected under the active file, making for misleading reading. Thus, filter on the
107+
# path for now. This means that the Pyre output is less useful, because things like
108+
# type changes on a commonly included attribute in one module will not show as
109+
# problems in other modules unless the command line is used :(
90110
checks = [
91111
{
92112
"source": "pyre",
@@ -99,6 +119,7 @@ def run_pyre(
99119
line=(x["stop_line"] - 1), character=x["stop_column"]
100120
),
101121
),
122+
"path": x["path"]
102123
}
103124
for x in data
104125
if document.path == f"{workspace.root_path}/{x['path']}"
@@ -136,9 +157,7 @@ def really_run_pyre(root_path: str) -> bytes:
136157
raise
137158

138159

139-
def maybe_create_pyre_config(
140-
config: pylsp_conf.Config, workspace: pylsp_ws.Workspace
141-
) -> None:
160+
def maybe_create_pyre_config(settings: Settings, workspace: pylsp_ws.Workspace) -> None:
142161
"""
143162
Initializes a .pyre_configuration file if `create-pyre-config` setting is enabled.
144163
@@ -153,14 +172,15 @@ def maybe_create_pyre_config(
153172
],
154173
"exclude": [
155174
"\/setup.py",
156-
".*\/build\/.*"
157-
]
175+
".*\/build\/.*",
176+
".*\/.pyre\/.*"
177+
],
178+
"strict": true
158179
}
159180
"""
160181
)
161-
settings = config.plugin_settings("pyre")
162182
try:
163-
if settings["create-pyre-config"]:
183+
if settings.create_pyre_config:
164184
docroot = workspace.root_path
165185
path = Path(docroot).joinpath(".pyre_configuration")
166186
if not path.exists():

pyproject.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "python-lsp-pyre"
3-
version = "0.1.2"
3+
version = "0.1.3"
44
description = "Pyre linting plugin for pylsp"
55
authors = ["Duncan Hill <[email protected]>"]
66
license = "MIT"

0 commit comments

Comments
 (0)