Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### ✨ New features and improvements

- Implement initial PythonTA LSP server

### πŸ› Bug fixes

### πŸ”§ Internal changes
202 changes: 57 additions & 145 deletions bundled/tool/lsp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,10 @@ def update_sys_path(path_to_add: str, strategy: str) -> None:
# Black: https://github.com/microsoft/vscode-black-formatter/blob/main/bundled/tool
# isort: https://github.com/microsoft/vscode-isort/blob/main/bundled/tool

TOOL_MODULE = "python-ta"
TOOL_MODULE = "python_ta"

TOOL_DISPLAY = "PythonTA"

TOOL_ARGS = [] # default arguments always passed to your tool.

TOOL_ARGS = ["--output-format", "pyta-lsp", "--exit-zero"] # default arguments always passed to your tool

# TODO: If your tool is a linter then update this section.
# Delete "Linting features" section if your tool is NOT a linter.
Expand Down Expand Up @@ -232,147 +230,57 @@ def _get_document_path(document: workspace.Document) -> str:
return uris.to_fs_path(file_uri)
return uris.to_fs_path(document.uri)


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Revert this change

def _linting_helper(document: workspace.Document) -> list[lsp.Diagnostic]:
# TODO: Determine if your tool supports passing file content via stdin.
# If you want to support linting on change then your tool will need to
# support linting over stdin to be effective. Read, and update
# _run_tool_on_document and _run_tool functions as needed for your project.
result = _run_tool_on_document(document)
return _parse_output_using_regex(result.stdout) if result.stdout else []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Revert this change (keep the blank line)

if result and result.stdout:
return _parse_json_output(result.stdout, document.uri)
return []

# TODO: If your linter outputs in a known format like JSON, then parse
# accordingly. But incase you need to parse the output using RegEx here
# is a helper you can work with.
# flake8 example:
# If you use following format argument with flake8 you can use the regex below to parse it.
# TOOL_ARGS += ["--format='%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s'"]
# DIAGNOSTIC_RE =
# r"(?P<line>\d+),(?P<column>-?\d+),(?P<type>\w+),(?P<code>\w+\d+):(?P<message>[^\r\n]*)"
DIAGNOSTIC_RE = re.compile(r"")


def _parse_output_using_regex(content: str) -> list[lsp.Diagnostic]:
lines: list[str] = content.splitlines()
def _parse_json_output(content: str, doc_uri: str) -> list[lsp.Diagnostic]:
"""Parses PythonTA's JSON output and maps it to LSP Diagnostics."""
diagnostics: list[lsp.Diagnostic] = []

# TODO: Determine if your linter reports line numbers starting at 1 (True) or 0 (False).
line_at_1 = True
# TODO: Determine if your linter reports column numbers starting at 1 (True) or 0 (False).
column_at_1 = True

line_offset = 1 if line_at_1 else 0
col_offset = 1 if column_at_1 else 0
for line in lines:
if line.startswith("'") and line.endswith("'"):
line = line[1:-1]
match = DIAGNOSTIC_RE.match(line)
if match:
data = match.groupdict()
position = lsp.Position(
line=max([int(data["line"]) - line_offset, 0]),
character=int(data["column"]) - col_offset,
)
diagnostic = lsp.Diagnostic(
range=lsp.Range(
start=position,
end=position,
),
message=data.get("message"),
severity=_get_severity(data["code"], data["type"]),
code=data["code"],
source=TOOL_MODULE,
)
diagnostics.append(diagnostic)

return diagnostics


# TODO: if you want to handle setting specific severity for your linter
# in a user configurable way, then look at look at how it is implemented
# for `pylint` extension from our team.
# Pylint: https://github.com/microsoft/vscode-pylint
# Follow the flow of severity from the settings in package.json to the server.
def _get_severity(*_codes: list[str]) -> lsp.DiagnosticSeverity:
# TODO: All reported issues from linter are treated as warning.
# change it as appropriate for your linter.
return lsp.DiagnosticSeverity.Warning


# **********************************************************
# Linting features end here
# **********************************************************

# TODO: If your tool is a formatter then update this section.
# Delete "Formatting features" section if your tool is NOT a
# formatter.
# **********************************************************
# Formatting features start here
# **********************************************************
# Sample implementations:
# Black: https://github.com/microsoft/vscode-black-formatter/blob/main/bundled/tool


@LSP_SERVER.feature(lsp.TEXT_DOCUMENT_FORMATTING)
def formatting(params: lsp.DocumentFormattingParams) -> list[lsp.TextEdit] | None:
"""LSP handler for textDocument/formatting request."""
# If your tool is a formatter you can use this handler to provide
# formatting support on save. You have to return an array of lsp.TextEdit
# objects, to provide your formatted results.

document = LSP_SERVER.workspace.get_text_document(params.text_document.uri)
edits = _formatting_helper(document)
if edits:
return edits

# NOTE: If you provide [] array, VS Code will clear the file of all contents.
# To indicate no changes to file return None.
return None


def _formatting_helper(document: workspace.TextDocument) -> list[lsp.TextEdit] | None:
# TODO: For formatting on save support the formatter you use must support
# formatting via stdin.
# Read, and update_run_tool_on_document and _run_tool functions as needed
# for your formatter.
result = _run_tool_on_document(document, use_stdin=True)
if result.stdout:
new_source = _match_line_endings(document, result.stdout)
return [
lsp.TextEdit(
range=lsp.Range(
start=lsp.Position(line=0, character=0),
end=lsp.Position(line=len(document.lines), character=0),
),
new_text=new_source,
)
]
return None


def _get_line_endings(lines: list[str]) -> str:
"""Returns line endings used in the text."""
try:
if lines[0][-2:] == "\r\n":
return "\r\n"
return "\n"
except Exception: # pylint: disable=broad-except
return None


def _match_line_endings(document: workspace.TextDocument, text: str) -> str:
"""Ensures that the edited text line endings matches the document line endings."""
expected = _get_line_endings(document.source.splitlines(keepends=True))
actual = _get_line_endings(text.splitlines(keepends=True))
if actual == expected or actual is None or expected is None:
return text
return text.replace(actual, expected)


# **********************************************************
# Formatting features ends here
# **********************************************************
# Strip initial output
json_start = content.find("[")
if json_start != -1:
content = content[json_start:]

results = json.loads(content)
for file_result in results:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall I think this code can be simplified using lsp.converters.get_converter(). This returns a cattrs converter that should be usable to parse the JSON into the relevant lsp classes, since the PythonTA reporter should be set up using them already.

if file_result.get("uri") == doc_uri:
for d in file_result.get("diagnostics", []):
start = lsp.Position(
line=d["range"]["start"]["line"],
character=d["range"]["start"]["character"]
)
end = lsp.Position(
line=d["range"]["end"]["line"],
character=d["range"]["end"]["character"]
)
raw_severity = d.get("severity", 3)
severity_enum = lsp.DiagnosticSeverity(raw_severity)

diagnostic = lsp.Diagnostic(
range=lsp.Range(start=start, end=end),
message=d.get("message", ""),
severity=severity_enum,
code=d.get("code"),
source=d.get("source", TOOL_DISPLAY)
)
diagnostics.append(diagnostic)
except json.JSONDecodeError:
if "[INFO] Your PythonTA report is being opened in your web browser." in content:
log_always("PythonTA generated a web report instead of LSP data.")
else:
log_error(f"Failed to parse JSON output from PythonTA. Raw output: {content}")
except Exception as ex:
log_error(f"Error mapping diagnostics: {ex}")

return diagnostics


# **********************************************************
Expand Down Expand Up @@ -573,23 +481,27 @@ def _run_tool_on_document(
# Pass document so get_cwd can resolve file-related variables for this document.
cwd = get_cwd(settings, document)

if settings["interpreter"] and len(settings["interpreter"]) > 0:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure why you changed the logic in this part of the code from what the template already provides. Let's just stick with the template here, we can always extend it later if we want.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I completely agree we should stick with what the template provides, but I believe for PythonTA to function correctly, adding this block of code might be necessary? For reference, here is the fatal error from PythonTA that we get when we run the extension without these lines:
Image

And the file is also missing all the PythonTA-specific errors that we would see if we were to run PythonTA on the code normally. These are the only errors we see when we run the extension without this block of code:
Image

And these are all the errors we should be seeing (matches what we would see in an HTML output) when PythonTA is run with this block of code included:
Image

I could be wrong, but when I ran into this issue before, it was astroid using the environment of the extension (language server specifically, in this case) itself, instead of the user's workspace, so we prepend the proper virtual environment directory to the PATH to use that instead. If there's a better way, feel free to let me know!

python_exe = settings["interpreter"][0]
if python_exe not in ("python", "python3"):
python_dir = os.path.dirname(python_exe)
venv_dir = os.path.dirname(python_dir)

os.environ["PATH"] = f"{python_dir}{os.pathsep}{os.environ.get('PATH', '')}"
os.environ["VIRTUAL_ENV"] = venv_dir
if "PYTHONHOME" in os.environ:
del os.environ["PYTHONHOME"]

use_path = False
use_rpc = False
if settings["path"]:
# 'path' setting takes priority over everything.
use_path = True
argv = settings["path"]
elif settings["interpreter"] and not utils.is_current_interpreter(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see you deleted some of the code you added, but it also seems like we should then restore this block here.

settings["interpreter"][0]
):
# If there is a different interpreter set use JSON-RPC to the subprocess
# running under that interpreter.
argv = [TOOL_MODULE]
use_rpc = True
else:
# if the interpreter is same as the interpreter running this
# process then run as module.
# Run under subprocess since python_ta calls sys.exit
argv = [TOOL_MODULE]
use_rpc = True

argv += TOOL_ARGS + settings["args"] + extra_args

Expand Down