Commit graph

154 commits

Author SHA1 Message Date
Dhruv Manilawala
ff53db3d99
Consider VS Code cell metadata to determine valid code cells (#12864)
## Summary

This PR adds support for VS Code specific cell metadata to consider when
collecting valid code cells.

For context, Ruff only runs on valid code cells. These are the code
cells that doesn't contain cell magics. Previously, Ruff only used the
notebook's metadata to determine whether it's a Python notebook. But, in
VS Code, a notebook's preferred language might be Python but it could
still contain code cells for other languages. This can be determined
with the `metadata.vscode.languageId` field.

### References:
* https://code.visualstudio.com/docs/languages/identifiers
* e6c009a3d4/extensions/ipynb/src/serializers.ts (L104-L107)
*
e6c009a3d4/extensions/ipynb/src/serializers.ts (L117-L122)

This brings us one step closer to fixing #12281.

## Test Plan

Add test cases for `is_valid_python_code_cell` and an integration test
case which showcase running it end to end. The test notebook contains a
JavaScript code cell and a Python code cell.
2024-08-13 22:09:56 +05:30
Dhruv Manilawala
540023262e
Collect errors while building up the settings index (#12781)
## Summary

Related to https://github.com/astral-sh/ruff-vscode/issues/571, this PR
updates the settings index builder to trace all the errors it
encountered. Without this, there's no way for user to know that
something failed and some of the capability might not work as expected.
For example, in the linked PR, the settings were invalid which means
notebooks weren't included and there were no log messages for it.

## Test Plan

Create an invalid `ruff.toml` file:
```toml
[tool.ruff]
extend-exclude = ["*.ipynb"]
```

Logs:
```
2024-08-12 18:33:09.873 [info] [Trace - 6:33:09 PM]   12.217043000s ERROR ruff:main ruff_server::session::index::ruff_settings: Failed to parse /Users/dhruv/playground/ruff/pyproject.toml
```

Notification Preview:

<img width="483" alt="Screenshot 2024-08-12 at 18 33 20"
src="https://github.com/user-attachments/assets/a4f303e5-f073-454f-bdcd-ba6af511e232">

Another way to trigger is to provide an invalid `cache-dir` value:
```toml
[tool.ruff]
cache-dir = "$UNKNOWN"
```

Same notification preview but different log message:
```
2024-08-12 18:41:37.571 [info] [Trace - 6:41:37 PM]   21.700112208s ERROR ThreadId(30) ruff_server::session::index::ruff_settings: Error while resolving settings from /Users/dhruv/playground/ruff/pyproject.toml: Invalid `cache-dir` value: error looking key 'UNKNOWN' up: environment variable not found
```

With multiple `pyproject.toml` file:
```
2024-08-12 18:41:15.887 [info] [Trace - 6:41:15 PM]    0.016636833s ERROR ThreadId(04) ruff_server::session::index::ruff_settings: Error while resolving settings from /Users/dhruv/playground/ruff/pyproject.toml: Invalid `cache-dir` value: error looking key 'UNKNOWN' up: environment variable not found

2024-08-12 18:41:15.888 [info] [Trace - 6:41:15 PM]    0.017378833s ERROR ThreadId(13) ruff_server::session::index::ruff_settings: Failed to parse /Users/dhruv/playground/ruff/tools/pyproject.toml
```
2024-08-12 15:42:45 +00:00
Dhruv Manilawala
037e817450
Use struct instead of type alias for workspace settings index (#12726)
## Summary

Follow-up from https://github.com/astral-sh/ruff/pull/12725, this is
just a small refactor to use a wrapper struct instead of type alias for
workspace settings index. This avoids the need to have the
`register_workspace_settings` as a static method on `Index` and instead
is a method on the new struct itself.
2024-08-07 09:26:59 +00:00
Dhruv Manilawala
7fcfedd430
Ignore non-file workspace URL (#12725)
## Summary

This PR updates the server to ignore non-file workspace URL.

This is to avoid crashing the server if the URL scheme is not "file".
We'd still raise an error if the URL to file path conversion fails.

Also, as per the docs of
[`to_file_path`](https://docs.rs/url/2.5.2/url/struct.Url.html#method.to_file_path):

> Note: This does not actually check the URL’s scheme, and may give
nonsensical results for other schemes. It is the user’s responsibility
to check the URL’s scheme before calling this.

resolves: #12660

## Test Plan

I'm not sure how to test this locally but the change is small enough to
validate on its own.
2024-08-07 09:15:55 +00:00
Micha Reiser
27edadec29
Make server panic hook more error resilient (#12610) 2024-08-02 12:10:06 +02:00
Dhruv Manilawala
f3c14a4276
Keep track of deleted cell for reorder change request (#12575)
## Summary

This PR fixes a bug where the server wouldn't retain the cell content in
case of a reorder change request.

As mentioned in
https://github.com/astral-sh/ruff/issues/12573#issuecomment-2257819298,
this change request is modeled as (a) remove these cell URIs and (b) add
these cell URIs. The cell content isn't provided. But, the way we've
modeled the `NotebookCell` (it contains the underlying `TextDocument`),
we need to keep track of the deleted cells to get the content.

This is not an ideal solution and a better long term solution would be
to model it as per the spec but that is a big structural change and will
affect multiple parts of the server. Modeling as per the spec would also
avoid bugs like https://github.com/astral-sh/ruff/pull/11864. For
context, that model would add complexity per
https://github.com/astral-sh/ruff/pull/11206#discussion_r1600165481.

fixes: #12573

## Test Plan

This video shows the before and after the bug is fixed:


https://github.com/user-attachments/assets/2fcad4b5-f9af-4776-8640-4cd1fa16e325
2024-07-30 09:51:26 +00:00
Piotr Osiewicz
fb9f566f56
Use $/logTrace for server trace logs in Zed and VS Code (#12564)
## Summary

This pull request adds support for logging via `$/logTrace` RPC
messages. It also enables that code path for when a client is Zed editor
or VS Code (as there's no way for us to generically tell whether a client prefers
`$/logTrace` over stderr.

Related to: #12523

## Test Plan

I've built Ruff from this branch and tested it manually with Zed.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
2024-07-30 08:32:20 +05:30
Piotr Osiewicz
143e172431
Do not bail code action resolution when a quick fix is requested (#12462)
## Summary

When working on improving Ruff integration with Zed I noticed that it
errors out when we try to resolve a code action of a `QUICKFIX` kind;
apparently, per @dhruvmanila we shouldn't need to resolve it, as the
edit is provided in the initial response for the code action. However,
it's possible for the `resolve` call to fill out other fields (such as
`command`).
AFAICT Helix also tries to resolve the code actions unconditionally (as
in, when either `edit` or `command` is absent); so does VSC. They can
still apply the quickfixes though, as they do not error out on a failed
call to resolve code actions - Zed does. Following suit on Zed's side
does not cut it though, as we still get a log request from Ruff for that
failure (which is surfaced in the UI).
There are also other language servers (such as
[rust-analyzer](c1c9e10f72/crates/rust-analyzer/src/handlers/request.rs (L1257)))
that fill out both `command` and `edit` fields as a part of code action
resolution.

This PR makes the resolve calls for quickfix actions return the input
value.

## Test Plan

N/A
2024-07-23 10:30:03 +05:30
Dhruv Manilawala
648cca199b
Add docs for Ruff language server (#12344)
## Summary

This PR adds documentation for the Ruff language server.

It mainly does the following:
1. Combines various READMEs containing instructions for different editor
setup in their respective section on the online docs
2. Provide an enumerated list of server settings. Additionally, it also
provides a section for VS Code specific options.
3. Adds a "Features" section which enumerates all the current
capabilities of the native server

For (2), the settings documentation is done manually but a future
improvement (easier after `ruff-lsp` is deprecated) is to move the docs
in to Rust struct and generate the documentation from the code itself.
And, the VS Code extension specific options can be generated by diffing
against the `package.json` in `ruff-vscode` repository.

### Structure

1. Setup: This section contains the configuration for setting up the
language server for different editors
2. Features: This section contains a list of capabilities provided by
the server along with short GIF to showcase it
3. Settings: This section contains an enumerated list of settings in a
similar format to the one for the linter / formatter
4. Migrating from `ruff-lsp`

> [!NOTE]
>
> The settings page is manually written but could possibly be
auto-generated via a macro similar to `OptionsMetadata` on the
`ClientSettings` struct

resolves: #11217 

## Test Plan

Generate and open the documentation locally using:
1. `python scripts/generate_mkdocs.py`
2. `mkdocs serve -f mkdocs.insiders.yml`
2024-07-18 17:41:43 +05:30
Dhruv Manilawala
2e77b775b0
Consider --preview flag for server subcommand (#12208)
## Summary

This PR removes the requirement of `--preview` flag to run the `ruff
server` and instead considers it to be an indicator to turn on preview
mode for the linter and the formatter.

resolves: #12161 

## Test Plan

Add test cases to assert the `preview` value is updated accordingly.

In an editor context, I used the local `ruff` executable in Neovim with
the `--preview` flag and verified that the preview-only violations are
being highlighted.

Running with:
```lua
require('lspconfig').ruff.setup({
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
})
```
The screenshot shows that `E502` is highlighted with the below config in
`pyproject.toml`:

<img width="877" alt="Screenshot 2024-07-17 at 16 43 09"
src="https://github.com/user-attachments/assets/c7016ef3-55b1-4a14-bbd3-a07b1bcdd323">
2024-07-18 11:05:01 +05:30
Dhruv Manilawala
ebe5b06c95
Use fallback settings when indexing the project (#12362)
## Summary

This PR updates the settings index building logic in the language server
to consider the fallback settings for applying ignore filters in
`WalkBuilder` and the exclusion via `exclude` / `extend-exclude`.

This flow matches the one in the `ruff` CLI where the root settings is
built by (1) finding the workspace setting in the ancestor directory (2)
finding the user configuration if that's missing and (3) fallback to
using the default configuration.

Previously, the index building logic was being executed before (2) and
(3). This PR reverses the logic so that the exclusion /
`respect_gitignore` is being considered from the default settings if
there's no workspace / user settings. This has the benefit that the
server no longer enters the `.git` directory or any other excluded
directory when a user opens a file in the home directory.

Related to #11366

## Test plan

Opened a test file from the home directory and confirmed with the debug
trace (removed in #12360) that the server excludes the `.git` directory
when indexing.
2024-07-18 09:16:45 +05:30
Dhruv Manilawala
6e0cbe0f35
Remove leftover debug log (#12360)
This was a leftover from #12299
2024-07-17 17:52:44 +05:30
Dhruv Manilawala
ecd4b4d943
Build settings index in parallel for the native server (#12299)
## Summary

This PR updates the server to build the settings index in parallel using
similar logic as `python_files_in_path`.

This should help with https://github.com/astral-sh/ruff/issues/11366 but
ideally we would want to build it lazily.

## Test Plan

`cargo insta test`
2024-07-15 09:57:54 +00:00
Dhruv Manilawala
90e9aae3f4
Consider nested configs for settings reloading (#12253)
## Summary

This PR fixes a bug in the settings reloading logic to consider nested
configuration in a workspace.

fixes: #11766

## Test Plan


69704b7b-44b9-4cc7-b5a7-376bf87c6ef4
2024-07-12 05:00:08 +00:00
Dhruv Manilawala
0bb2fc6eec
Conside include, extend-include for the native server (#12252)
## Summary

This PR updates the native server to consider the `include` and
`extend-include` file resolver settings.

fixes: #12242 

## Test Plan

Note: Settings reloading doesn't work for nested configs which is fixed
in #12253 so the preview here only showcases root level config.

e8969128-c175-4f98-8114-0d692b906cc8
2024-07-10 04:12:57 +00:00
Dhruv Manilawala
7910beecc4
Consider the content of the new cells during notebook sync (#12203)
## Summary

This PR fixes the bug where the server was not considering the
`cells.structure.didOpen` field to sync up the new content of the newly
added cells.

The parameters corresponding to this request provides two fields to get
the newly added cells:
1. `cells.structure.array.cells`: This is a list of `NotebookCell` which
doesn't contain any cell content. The only useful information from this
array is the cell kind and the cell document URI which we use to
initialize the new cell in the index.
2. `cells.structure.didOpen`: This is a list of `TextDocumentItem` which
corresponds to the newly added cells. This actually contains the text
content and the version.

This wasn't a problem before because we initialize the cell with an
empty string and this isn't a problem when someone just creates an empty
cell. But, when someone copy-pastes a cell, the cell needs to be
initialized with the content.

fixes: #12201 

## Test Plan

First, let's see the panic in action:

1. Press <kbd>Esc</kbd> to allow using the keyboard to perform cell
actions (move around, copy, paste, etc.)
2. Copy the second cell with <kbd>c</kbd> key
3. Delete the second cell with <kbd>dd</kbd> key
4. Paste the copied cell with <kbd>p</kbd> key

You can see that the content isn't synced up because the `unused-import`
for `sys` is still being highlighted but it's being used in the second
cell. And, the hover isn't working either. Then, as I start editing the
second cell, it panics.


fc58364c-c8fc-4c11-a917-71b6dd90c1ef

Now, here's the preview of the fixed version:


207872dd-dca6-49ee-8b6e-80435c7ef22e
2024-07-05 17:10:00 +05:30
Dhruv Manilawala
e6e09ea93a
Avoid syntax error notification for source code actions (#12148)
## Summary

This PR avoids the error notification if a user selects the source code
actions and there's a syntax error in the source.

Before https://github.com/astral-sh/ruff/pull/12134, the change would've
been different. But that PR disables generating fixes if there's a
syntax error. This means that we can return an empty map instead as
there won't be any fixes in the diagnostics returned by the `lint_fix`
function.

For reference, following are the screenshot as on `main` with the error:

**VS Code:**

<img width="1715" alt="Screenshot 2024-07-02 at 16 39 59"
src="62f3e99b-0b0c-4608-84a2-26aeabcc6933">

**Neovim:**

<img width="1717" alt="Screenshot 2024-07-02 at 16 38 50"
src="5d637c36-d7f8-4a3b-8011-9a89708919a8">

fixes: #11931

## Test Plan

Considering the following code snippet where there are two diagnostics
(syntax error and useless semicolon `E703`):
```py
x;

y =
```

### VS Code


943537fc-ed8d-448d-8a36-1e34536c4f3e

### Neovim


4e6f3372-6e5b-4380-8919-6221066efd5b
2024-07-04 09:37:16 +05:30
Dhruv Manilawala
d870720841
Fix replacement edit range computation (#12171)
## Summary

This PR fixes various bugs for computing the replacement range between
the original and modified source for the language server.

1. When finding the end offset of the source and modified range, we
should apply `zip` on the reversed iterator. The bug was that it was
reversing the already zipped iterator. The problem here is that the
length of both slices aren't going to be the same unless the source
wasn't modified at all. Refer to the [Rust
playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=44f860d31bd26456f3586b6ab530c22f)
where you can see this in action.
2. Skip the first line when computing the start offset because the first
line start value will always be 0 and the default value of the source /
modified range start is also 0. So, comparing 0 and 0 is not useful
which means we can skip the first value.
3. While iterating in the reverse direction, we should only stop if the
line start is strictly less than the source start i.e., we should use
`<` instead of `<=`.

fixes: #12128 

## Test Plan

Add test cases where the text is being inserted, deleted, and replaced
between the original and new source code, validate the replacement
ranges.
2024-07-04 09:24:07 +05:30
Dhruv Manilawala
526efd398a
Remove E999 to find diagnostic severity (#12080)
## Summary

This PR removes the need to check for `E999` code to find the diagnostic
severity in the server.

**Note:** This is just removing a redundant check because all
`ParseErrors` are converted to `Diagnostic` with default `Error`
severity by
63c92586a1/crates/ruff_server/src/lint.rs (L309-L346)

## Test Plan

Verify that syntax errors are still shown with error severity as it did
before:

<img width="1313" alt="Screenshot 2024-06-28 at 09 30 20"
src="75e389a7-01ea-461c-86a2-0dfc244e515d">
2024-06-28 09:31:35 +05:30
Dhruv Manilawala
22cebdf29b Add server config to filter out syntax error diagnostics (#12059)
## Summary

Follow-up from #11901 

This PR adds a new server setting to show / hide syntax errors.

## Test Plan

### VS Code

Using https://github.com/astral-sh/ruff-vscode/pull/504 with the
following config:

```json
{
  "ruff.nativeServer": true,
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  "ruff.showSyntaxErrors": true
}
```

First, set `ruff.showSyntaxErrors` to `true`:
<img width="1177" alt="Screenshot 2024-06-27 at 08 34 58"
src="5d77547a-a908-4a00-8714-7c00784e8679">

And then set it to `false`:
<img width="1185" alt="Screenshot 2024-06-27 at 08 35 19"
src="9720f089-f10c-420b-a2c1-2bbb2245be35">

### Neovim

Using the following Ruff server config:

```lua
require('lspconfig').ruff.setup {
  init_options = {
    settings = {
      showSyntaxErrors = false,
    },
  },
}
```

First, set `showSyntaxErrors` to `true`:
<img width="1279" alt="Screenshot 2024-06-27 at 08 28 03"
src="e694e231-91ba-47f8-8e8a-ad2e82b85a45">

And then set it to `false`:
<img width="1284" alt="Screenshot 2024-06-27 at 08 28 20"
src="25b86a57-02b1-44f7-9f65-cf5fdde93b0c">
2024-06-27 13:44:11 +02:00
Dhruv Manilawala
72b6c26101 Simplify LinterResult, avoid cloning ParseError (#11903)
## Summary

Follow-up to #11902

This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.

This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.

## Test Plan

`cargo insta test`
2024-06-27 13:44:11 +02:00
Dhruv Manilawala
e7b49694a7 Remove E999 as a rule, disallow any disablement methods for syntax error (#11901)
## Summary

This PR updates the way syntax errors are handled throughout the linter.

The main change is that it's now not considered as a rule which involves
the following changes:
* Update `Message` to be an enum with two variants - one for diagnostic
message and the other for syntax error message
* Provide methods on the new message enum to query information required
by downstream usages

This means that the syntax errors cannot be hidden / disabled via any
disablement methods. These are:
1. Configuration via `select`, `ignore`, `per-file-ignores`, and their
`extend-*` variants
	```console
$ cargo run -- check ~/playground/ruff/src/lsp.py --extend-select=E999
--no-preview --no-cache
	    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.10s
Running `target/debug/ruff check /Users/dhruv/playground/ruff/src/lsp.py
--extend-select=E999 --no-preview --no-cache`
warning: Rule `E999` is deprecated and will be removed in a future
release. Syntax errors will always be shown regardless of whether this
rule is selected or not.
/Users/dhruv/playground/ruff/src/lsp.py:1:8: F401 [*] `abc` imported but
unused
	  |
	1 | import abc
	  |        ^^^ F401
	2 | from pathlib import Path
	3 | import os
	  |
	  = help: Remove unused import: `abc`
	```
3. Command-line flags via `--select`, `--ignore`, `--per-file-ignores`,
and their `--extend-*` variants
	```console
$ cargo run -- check ~/playground/ruff/src/lsp.py --no-cache
--config=~/playground/ruff/pyproject.toml
	    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
Running `target/debug/ruff check /Users/dhruv/playground/ruff/src/lsp.py
--no-cache --config=/Users/dhruv/playground/ruff/pyproject.toml`
warning: Rule `E999` is deprecated and will be removed in a future
release. Syntax errors will always be shown regardless of whether this
rule is selected or not.
/Users/dhruv/playground/ruff/src/lsp.py:1:8: F401 [*] `abc` imported but
unused
	  |
	1 | import abc
	  |        ^^^ F401
	2 | from pathlib import Path
	3 | import os
	  |
	  = help: Remove unused import: `abc`
	```

This also means that the **output format** needs to be updated:
1. The `code`, `noqa_row`, `url` fields in the JSON output is optional
(`null` for syntax errors)
2. Other formats are changed accordingly
For each format, a new test case specific to syntax errors have been
added. Please refer to the snapshot output for the exact format for
syntax error message.

The output of the `--statistics` flag will have a blank entry for syntax
errors:
```
315     F821    [ ] undefined-name
119             [ ] syntax-error
103     F811    [ ] redefined-while-unused
```

The **language server** is updated to consider the syntax errors by
convert them into LSP diagnostic format separately.

### Preview

There are no quick fixes provided to disable syntax errors. This will
automatically work for `ruff-lsp` because the `noqa_row` field will be
`null` in that case.
<img width="772" alt="Screenshot 2024-06-26 at 14 57 08"
src="aaac827e-4777-4ac8-8c68-eaf9f2c36774">

Even with `noqa` comment, the syntax error is displayed:
<img width="763" alt="Screenshot 2024-06-26 at 14 59 51"
src="ba1afb68-7eaf-4b44-91af-6d93246475e2">

Rule documentation page:
<img width="1371" alt="Screenshot 2024-06-26 at 16 48 07"
src="524f01df-d91f-4ac0-86cc-40e76b318b24">


## Test Plan

- [x] Disablement methods via config shows a warning
	- [x] `select`, `extend-select`
	- [ ] ~`ignore`~ _doesn't show any message_
- [ ] ~`per-file-ignores`, `extend-per-file-ignores`~ _doesn't show any
message_
- [x] Disablement methods via command-line flag shows a warning
	- [x] `--select`, `--extend-select`
	- [ ] ~`--ignore`~ _doesn't show any message_
- [ ] ~`--per-file-ignores`, `--extend-per-file-ignores`~ _doesn't show
any message_
- [x] File with syntax errors should exit with code 1
- [x] Language server
	- [x] Should show diagnostics for syntax errors
	- [x] Should not recommend a quick fix edit for adding `noqa` comment
	- [x] Same for `ruff-lsp`

resolves: #8447
2024-06-27 13:44:11 +02:00
T-256
d6a2cad9c2 Drop deprecated nursery rule group (#10172)
Co-authored-by: Micha Reiser <micha@reiser.io>
Resolves https://github.com/astral-sh/ruff/issues/7992
2024-06-27 13:44:11 +02:00
renovate[bot]
53a80a5c11
Update Rust crate rustc-hash to v2 (#12001) 2024-06-23 20:46:42 -04:00
Jane Lewis
ad4a88657b
Remove usage of std::path::absolute from snapshot test (#11973) 2024-06-21 20:21:12 +01:00
Jane Lewis
791f6a1820
ruff server: Closing an untitled, unsaved notebook document no longer throws an error (#11942)
## Summary

Fixes #11651.
Fixes #11851.

We were double-closing a notebook document from the index, once in
`textDocument/didClose` and then in the `notebookDocument/didClose`
handler. The second time this happens, taking a snapshot fails.

I've rewritten how we handle snapshots for closing notebooks / notebook
cells so that any failure is simply logged instead of propagating
upwards. This implementation works consistently even if we don't receive
`textDocument/didClose` notifications for each specific cell, since they
get closed (and the diagnostics get cleared) in the notebook document
removal process.

## Test Plan

1. Open an untitled, unsaved notebook with the `Create: New Jupyter
Notebook` command from the VS Code command palette (`Ctrl/Cmd + Shift +
P`)
2. Without saving the document, close it.
3. No error popup should appear.
4. Run the debug command (`Ruff: print debug information`) to confirm
that there are no open documents
2024-06-21 10:53:30 -07:00
Jane Lewis
3ab7a8da73
Add Jupyter Notebook document change snapshot test (#11944)
## Summary

Closes #11914.

This PR introduces a snapshot test that replays the LSP requests made
during a document formatting request, and confirms that the notebook
document is updated in the expected way.
2024-06-21 05:29:27 +00:00
Jane Lewis
c8ff89c73c
ruff server: Support the usage of tildes and environment variables in logFile (#11945)
## Summary

Fixes #11911.

`shellexpand` is now used on `logFile` to expand the file path, allowing
the usage of `~` and environment variables.

## Test Plan

1. Set `logFile` in either Neovim or Helix to a file path that needs
expansion, like `~/.config/helix/ruff_logs.txt`.
2. Ensure that `RUFF_TRACE` is set to `messages` or `verbose`
3. Open a Python file in Neovim/Helix
4. Confirm that a file at the path specified was created, with the
expected logs.
2024-06-20 18:51:46 +00:00
Jane Lewis
ff3bf583b2
ruff server: Add tracing setup guide to Neovim documentation (#11884)
A follow-up to [this
suggestion](https://github.com/astral-sh/ruff/pull/11747#discussion_r1634297757)
on the tracing PR.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
2024-06-18 13:39:41 -07:00
Jane Lewis
c53d55a483
ruff server: Add tracing setup guide to Helix documentation (#11883)
A follow-up to [this
suggestion](https://github.com/astral-sh/ruff/pull/11747#discussion_r1634297757)
on the tracing PR.
2024-06-18 03:41:24 +00:00
Jane Lewis
ffc98522cd
ruff server: Defer notebook cell deletion to avoid an error message (#11864)
## Summary

Fixes https://github.com/astral-sh/ruff-vscode/issues/496.

Cells are no longer removed from the notebook index when a notebook gets
updated, but rather when `textDocument/didClose` is called for them.
This solves an issue where their premature removal from the notebook
cell index would cause their URL to be un-queryable in the
`textDocument/didClose` handler.

## Test Plan

Create and then delete a notebook cell in VS Code. No error should
appear.
2024-06-18 03:37:40 +00:00
Dhruv Manilawala
f8f0053a6c
Trim trailing whitespace in server debug message (#11895) 2024-06-17 05:46:08 +00:00
Dhruv Manilawala
9dc226be97
Add supported commands in server capabilities (#11850)
## Summary

This PR updates the server capabilities to include the commands that
Ruff supports. This is similar to how there's a list of possible code
actions supported by the server.

I noticed this when I was trying to find whether Helix supported
workspace commands or not based on Jane's comment
(https://github.com/astral-sh/ruff/pull/11831#discussion_r1634984921)
and I found the `:lsp-workspace-command` in the editor but it didn't
show up anything in the picker.

So, I looked at the implementation in Helix
(9c479e6d2d/helix-term/src/commands/typed.rs (L1372-L1384))
which made me realize that Ruff doesn't provide this in its
capabilities. Currently, this does require `ruff` to be first in the
list of language servers in the user config but that should be resolved
by https://github.com/helix-editor/helix/pull/10176. So, the following
config should work:

```toml
[[language]]
name = "python"
# Ruff should come first until https://github.com/helix-editor/helix/pull/10176 is released
language-servers = ["ruff", "pyright"]
```

## Test Plan

1. Neovim's server capabilities output should include the supported
commands:

```
  executeCommandProvider = {                                                                                                                          
    commands = { "ruff.applyFormat", "ruff.applyAutofix", "ruff.applyOrganizeImports", "ruff.printDebugInformation" },                                
    workDoneProgress = false                                                                                                                          
  },
```

2. Helix should now display the commands to pick from when
`:lsp-workspace-command` is invoked:

<img width="832" alt="Screenshot 2024-06-13 at 08 47 14"
src="09048ecd-c974-4e09-ab56-9482ff3d780b">
2024-06-13 09:32:43 +05:30
Jane Lewis
7d5cf1811b
ruff server: Improve error message when a command is run on an unavailable document (#11823)
## Summary

Fixes #11744.

We now show a distinct popup message when we fail to get a document
snapshot during command execution. This message more clearly
communicates the issue to the user, instead of a generic "ruff
encountered an error" message.

## Test Plan

Try running `Fix all auto-fixable problems` on an incompatible file (for
example: `settings.json`). You should see the following popup message:
<img width="456" alt="Screenshot 2024-06-11 at 11 47 16 AM"
src="3a28e3d7-3896-4dd0-b117-f87300dd3b68">
2024-06-11 18:50:01 +00:00
Jane Lewis
4e9d771aa0
ruff server: Introduce the ruff.printDebugInformation command (#11831)
## Summary

Closes #11715.

Introduces a new command, `ruff.printDebugInformation`. This will print
useful information about the status of the server to `stderr`.

Right now, the information shown by this command includes:
* The path to the server executable
* The version of the executable
* The text encoding being used
* The number of open documents and workspaces
* A list of registered configuration files
* The capabilities of the client

## Test Plan

First, checkout and use [the corresponding `ruff-vscode`
PR](https://github.com/astral-sh/ruff-vscode/pull/495).

Running the `Print debug information` command in VS Code should show
something like the following in the Output channel:

<img width="991" alt="Screenshot 2024-06-11 at 11 41 46 AM"
src="ab93c009-bb7b-4291-b057-d44fdc6f9f86">
2024-06-11 11:42:46 -07:00
Jane Lewis
507f5c1137
ruff server: Tracing system now respects log level and trace level, with options to log to a file (#11747)
## Summary

Fixes #10968.
Fixes #11545.

The server's tracing system has been rewritten from the ground up. The
server now has trace level and log level settings which restrict the
tracing events and spans that get logged.

* A `logLevel` setting has been added, which lets a user set the log
level. By default, it is set to `"info"`.
* A `logFile` setting has also been added, which lets the user supply an
optional file to send tracing output (it does not have to exist as a
file yet). By default, if this is unset, tracing output will be sent to
`stderr`.
* A `$/setTrace` handler has also been added, and we also set the trace
level from the initialization options. For editors without direct
support for tracing, the environment variable `RUFF_TRACE` can override
the trace level.
* Small changes have been made to how we display tracing output. We no
longer use `tracing-tree`, and instead use
`tracing_subscriber::fmt::Layer` to format output. Thread names are now
included in traces, and I've made some adjustment to thread worker names
to be more useful.

## Test Plan

In VS Code, with `ruff.trace.server` set to its default value, no logs
from Ruff should appear.

After changing `ruff.trace.server` to either `messages` or `verbose`,
you should see log messages at `info` level or higher appear in Ruff's
output:
<img width="1005" alt="Screenshot 2024-06-10 at 10 35 04 AM"
src="6050d107-9815-4bd2-96d0-e86f096a57f5">

In Helix, by default, no logs from Ruff should appear.

To set the trace level in Helix, you'll need to modify your language
configuration as follows:
```toml
[language-server.ruff]
command = "/Users/jane/astral/ruff/target/debug/ruff"
args = ["server", "--preview"]
environment = { "RUFF_TRACE" = "messages" }
```

After doing this, logs of `info` level or higher should be visible in
Helix:
<img width="1216" alt="Screenshot 2024-06-10 at 10 39 26 AM"
src="8ff88692-d3f7-4fd1-941e-86fb338fcdcc">

You can use `:log-open` to quickly open the Helix log file.

In Neovim, by default, no logs from Ruff should appear.

To set the trace level in Neovim, you'll need to modify your
configuration as follows:
```lua
require('lspconfig').ruff.setup {
  cmd = {"/path/to/debug/executable", "server", "--preview"},
  cmd_env = { RUFF_TRACE = "messages" }
}
```

You should see logs appear in `:LspLog` that look like the following:
<img width="1490" alt="Screenshot 2024-06-11 at 11 24 01 AM"
src="576cd5fa-03cf-477a-b879-b29a9a1200ff">

You can adjust `logLevel` and `logFile` in `settings`:
```lua
require('lspconfig').ruff.setup {
  cmd = {"/path/to/debug/executable", "server", "--preview"},
  cmd_env = { RUFF_TRACE = "messages" },
  settings = {
    logLevel = "debug",
    logFile = "your/log/file/path/log.txt"
  }
}
```

The `logLevel` and `logFile` can also be set in Helix like so:
```toml
[language-server.ruff.config.settings]
logLevel = "debug"
logFile = "your/log/file/path/log.txt"
```

Even if this log file does not exist, it should now be created and
written to after running the server:

<img width="1148" alt="Screenshot 2024-06-10 at 10 43 44 AM"
src="ab533cf7-d5ac-4178-97f1-e56da17450dd">
2024-06-11 11:29:47 -07:00
Dhruv Manilawala
549cc1e437
Build CommentRanges outside the parser (#11792)
## Summary

This PR updates the parser to remove building the `CommentRanges` and
instead it'll be built by the linter and the formatter when it's
required.

For the linter, it'll be built and owned by the `Indexer` while for the
formatter it'll be built from the `Tokens` struct and passed as an
argument.

## Test Plan

`cargo insta test`
2024-06-09 09:55:17 +00:00
Charlie Marsh
ee1621b2f9
Use real file path when available in ruff server (#11800)
## Summary

As-is, we're using the URL path for all files, leading us to use paths
like:

```
/c%3A/Users/crmar/workspace/fastapi/tests/main.py
```

This doesn't match against per-file ignores and other patterns in Ruff
configuration.

This PR modifies the LSP to use the real file path if available, and the
virtual file path if not.

Closes https://github.com/astral-sh/ruff/issues/11751.

## Test Plan

Ran the LSP on Windows. In the FastAPI repo, added:

```toml
[tool.ruff.lint.per-file-ignores]
"tests/**/*.py" = ["F401"]
```

And verified that an unused import was ignored in `tests` after this
change, but not before.
2024-06-07 22:48:53 -07:00
Jane Lewis
8338db6c12
ruff server: Formatting a document with syntax problems no longer spams a visible error popup (#11745)
## Summary

Fixes https://github.com/astral-sh/ruff-vscode/issues/482.

I've made adjustments to `format` and `format_range` that handle parsing
errors before they become server errors. We'll still log this as a
problem, but there will no longer be a visible popup.

## Test Plan

Instead of seeing a visible error when formatting a document with syntax
issues, you should see this warning in the LSP logs:

<img width="991" alt="Screenshot 2024-06-04 at 3 38 23 PM"
src="9d68947d-6462-4ca6-ab5a-65e573c91db6">

Similarly, if you try to format a range with syntax issues, you should
see this warning in the LSP logs instead of a visible error popup:

<img width="1010" alt="Screenshot 2024-06-04 at 3 39 10 PM"
src="99fff098-798d-406a-976e-81ead0da0352">

---------

Co-authored-by: Zanie Blue <contact@zanie.dev>
2024-06-04 17:18:21 -07:00
Dhruv Manilawala
bf5b62edac
Maintain synchronicity between the lexer and the parser (#11457)
## Summary

This PR updates the entire parser stack in multiple ways:

### Make the lexer lazy

* https://github.com/astral-sh/ruff/pull/11244
* https://github.com/astral-sh/ruff/pull/11473

Previously, Ruff's lexer would act as an iterator. The parser would
collect all the tokens in a vector first and then process the tokens to
create the syntax tree.

The first task in this project is to update the entire parsing flow to
make the lexer lazy. This includes the `Lexer`, `TokenSource`, and
`Parser`. For context, the `TokenSource` is a wrapper around the `Lexer`
to filter out the trivia tokens[^1]. Now, the parser will ask the token
source to get the next token and only then the lexer will continue and
emit the token. This means that the lexer needs to be aware of the
"current" token. When the `next_token` is called, the current token will
be updated with the newly lexed token.

The main motivation to make the lexer lazy is to allow re-lexing a token
in a different context. This is going to be really useful to make the
parser error resilience. For example, currently the emitted tokens
remains the same even if the parser can recover from an unclosed
parenthesis. This is important because the lexer emits a
`NonLogicalNewline` in parenthesized context while a normal `Newline` in
non-parenthesized context. This different kinds of newline is also used
to emit the indentation tokens which is important for the parser as it's
used to determine the start and end of a block.

Additionally, this allows us to implement the following functionalities:
1. Checkpoint - rewind infrastructure: The idea here is to create a
checkpoint and continue lexing. At a later point, this checkpoint can be
used to rewind the lexer back to the provided checkpoint.
2. Remove the `SoftKeywordTransformer` and instead use lookahead or
speculative parsing to determine whether a soft keyword is a keyword or
an identifier
3. Remove the `Tok` enum. The `Tok` enum represents the tokens emitted
by the lexer but it contains owned data which makes it expensive to
clone. The new `TokenKind` enum just represents the type of token which
is very cheap.

This brings up a question as to how will the parser get the owned value
which was stored on `Tok`. This will be solved by introducing a new
`TokenValue` enum which only contains a subset of token kinds which has
the owned value. This is stored on the lexer and is requested by the
parser when it wants to process the data. For example:
8196720f80/crates/ruff_python_parser/src/parser/expression.rs (L1260-L1262)

[^1]: Trivia tokens are `NonLogicalNewline` and `Comment`

### Remove `SoftKeywordTransformer`

* https://github.com/astral-sh/ruff/pull/11441
* https://github.com/astral-sh/ruff/pull/11459
* https://github.com/astral-sh/ruff/pull/11442
* https://github.com/astral-sh/ruff/pull/11443
* https://github.com/astral-sh/ruff/pull/11474

For context,
https://github.com/RustPython/RustPython/pull/4519/files#diff-5de40045e78e794aa5ab0b8aacf531aa477daf826d31ca129467703855408220
added support for soft keywords in the parser which uses infinite
lookahead to classify a soft keyword as a keyword or an identifier. This
is a brilliant idea as it basically wraps the existing Lexer and works
on top of it which means that the logic for lexing and re-lexing a soft
keyword remains separate. The change here is to remove
`SoftKeywordTransformer` and let the parser determine this based on
context, lookahead and speculative parsing.

* **Context:** The transformer needs to know the position of the lexer
between it being at a statement position or a simple statement position.
This is because a `match` token starts a compound statement while a
`type` token starts a simple statement. **The parser already knows
this.**
* **Lookahead:** Now that the parser knows the context it can perform
lookahead of up to two tokens to classify the soft keyword. The logic
for this is mentioned in the PR implementing it for `type` and `match
soft keyword.
* **Speculative parsing:** This is where the checkpoint - rewind
infrastructure helps. For `match` soft keyword, there are certain cases
for which we can't classify based on lookahead. The idea here is to
create a checkpoint and keep parsing. Based on whether the parsing was
successful and what tokens are ahead we can classify the remaining
cases. Refer to #11443 for more details.

If the soft keyword is being parsed in an identifier context, it'll be
converted to an identifier and the emitted token will be updated as
well. Refer
8196720f80/crates/ruff_python_parser/src/parser/expression.rs (L487-L491).

The `case` soft keyword doesn't require any special handling because
it'll be a keyword only in the context of a match statement.

### Update the parser API

* https://github.com/astral-sh/ruff/pull/11494
* https://github.com/astral-sh/ruff/pull/11505

Now that the lexer is in sync with the parser, and the parser helps to
determine whether a soft keyword is a keyword or an identifier, the
lexer cannot be used on its own. The reason being that it's not
sensitive to the context (which is correct). This means that the parser
API needs to be updated to not allow any access to the lexer.

Previously, there were multiple ways to parse the source code:
1. Passing the source code itself
2. Or, passing the tokens

Now that the lexer and parser are working together, the API
corresponding to (2) cannot exists. The final API is mentioned in this
PR description: https://github.com/astral-sh/ruff/pull/11494.

### Refactor the downstream tools (linter and formatter)

* https://github.com/astral-sh/ruff/pull/11511
* https://github.com/astral-sh/ruff/pull/11515
* https://github.com/astral-sh/ruff/pull/11529
* https://github.com/astral-sh/ruff/pull/11562
* https://github.com/astral-sh/ruff/pull/11592

And, the final set of changes involves updating all references of the
lexer and `Tok` enum. This was done in two-parts:
1. Update all the references in a way that doesn't require any changes
from this PR i.e., it can be done independently
	* https://github.com/astral-sh/ruff/pull/11402
	* https://github.com/astral-sh/ruff/pull/11406
	* https://github.com/astral-sh/ruff/pull/11418
	* https://github.com/astral-sh/ruff/pull/11419
	* https://github.com/astral-sh/ruff/pull/11420
	* https://github.com/astral-sh/ruff/pull/11424
2. Update all the remaining references to use the changes made in this
PR

For (2), there were various strategies used:
1. Introduce a new `Tokens` struct which wraps the token vector and add
methods to query a certain subset of tokens. These includes:
	1. `up_to_first_unknown` which replaces the `tokenize` function
2. `in_range` and `after` which replaces the `lex_starts_at` function
where the former returns the tokens within the given range while the
latter returns all the tokens after the given offset
2. Introduce a new `TokenFlags` which is a set of flags to query certain
information from a token. Currently, this information is only limited to
any string type token but can be expanded to include other information
in the future as needed. https://github.com/astral-sh/ruff/pull/11578
3. Move the `CommentRanges` to the parsed output because this
information is common to both the linter and the formatter. This removes
the need for `tokens_and_ranges` function.

## Test Plan

- [x] Update and verify the test snapshots
- [x] Make sure the entire test suite is passing
- [x] Make sure there are no changes in the ecosystem checks
- [x] Run the fuzzer on the parser
- [x] Run this change on dozens of open-source projects

### Running this change on dozens of open-source projects

Refer to the PR description to get the list of open source projects used
for testing.

Now, the following tests were done between `main` and this branch:
1. Compare the output of `--select=E999` (syntax errors)
2. Compare the output of default rule selection
3. Compare the output of `--select=ALL`

**Conclusion: all output were same**

## What's next?

The next step is to introduce re-lexing logic and update the parser to
feed the recovery information to the lexer so that it can emit the
correct token. This moves us one step closer to having error resilience
in the parser and provides Ruff the possibility to lint even if the
source code contains syntax errors.
2024-06-03 18:23:50 +05:30
JaRoSchm
7ce17b7736
Add Vim and Kate setup guide for ruff server (#11615)
## Summary

In the [roadmap for `ruff
server`](https://github.com/astral-sh/ruff/discussions/10581) support
for vim and kate is listed. Therefore I added setup guides for them
based on the neovim guide. As I don't use pyright I wasn't able to
translate the corresponding part from the neovim guide.

## Test Plan

Doesn't apply.
2024-05-31 19:06:55 +00:00
T-256
5b500fc4dc
ruff server: Add support for documents not exist on disk (#11588)
Co-authored-by: T-256 <Tester@test.com>
Co-authored-by: Micha Reiser <micha@reiser.io>
2024-05-31 08:34:10 +02:00
Micha Reiser
921bc15542
use owned ast and tokens in bench (#11598) 2024-05-29 18:10:32 +02:00
Micha Reiser
163c374242
Reduce extensive use of snapshot.query (#11596) 2024-05-29 10:11:46 +02:00
Charlie Marsh
204c59e353
Respect file exclusions in ruff server (#11590)
## Summary

Closes https://github.com/astral-sh/ruff/issues/11587.

## Test Plan

- Added a lint error to `test_server.py` in `vscode-ruff`.
- Validated that, prior to this change, diagnostics appeared in the
file.
- Validated that, with this change, no diagnostics were shown.
- Validated that, with this change, no diagnostics were fixed on-save.
2024-05-29 02:58:36 +00:00
Micha Reiser
3989cb8b56
Make ruff_notebook a workspace dependency in ruff_server (#11572) 2024-05-28 09:26:39 +02:00
Charlie Marsh
34a5063aa2
Respect excludes in ruff server configuration discovery (#11551)
## Summary

Right now, we're discovering configuration files even within (e.g.)
virtual environments, because we're recursing without respecting the
`exclude` field on parent configuration.

Closes https://github.com/astral-sh/ruff-vscode/issues/478.

## Test Plan

Installed Pandas; verified that I saw no warnings:

![Screenshot 2024-05-26 at 8 09
05 PM](dcf4115c-d7b3-453b-b7c7-afdd4804d6f5)
2024-05-27 16:59:46 +00:00
Micha Reiser
adc0a5d126
Rename document module to text_document (#11571) 2024-05-27 18:32:21 +02:00
Dhruv Manilawala
37ad994318
Use default settings if initialization options is empty or not provided (#11566)
## Summary

This PR fixes the bug to avoid flattening the global-only settings for
the new server.

This was added in https://github.com/astral-sh/ruff/pull/11497, possibly
to correctly de-serialize an empty value (`{}`). But, this lead to a bug
where the configuration under the `settings` key was not being read for
global-only variant.

By using #[serde(default)], we ensure that the settings field in the
`GlobalOnly` variant is optional and that an empty JSON object `{}` is
correctly deserialized into `GlobalOnly` with a default `ClientSettings`
instance.

fixes: #11507 

## Test Plan

Update the snapshot and existing test case. Also, verify the following
settings in Neovim:

1. Nothing

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
}
```

2. Empty dictionary

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
  init_options = vim.empty_dict(),
}
```

3. Empty `settings`

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
  init_options = {
    settings = vim.empty_dict(),
  },
}
```

4. With some configuration:

```lua
ruff = {
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
  init_options = {
    settings = {
      configuration = '/tmp/ruff-repro/pyproject.toml',
    },
  },
}
```
2024-05-27 21:06:34 +05:30
Jane Lewis
627d230688
ruff server searches for configuration in parent directories (#11537)
## Summary

Fixes #11506.

`RuffSettingsIndex::new` now searches for configuration files in parent
directories.

## Test Plan

I confirmed that the original test case described in the issue worked as
expected.
2024-05-26 18:11:08 +00:00