Skip to content

Commit

Permalink
ruff server: Resolve configuration for each document individually (#…
Browse files Browse the repository at this point in the history
…10950)

## Summary

Configuration is no longer the property of a workspace but rather of
individual documents. Just like the Ruff CLI, each document is
configured based on the 'nearest' project configuration. See [the Ruff
documentation](https://docs.astral.sh/ruff/configuration/#config-file-discovery)
for more details.

To reduce the amount of times we resolve configuration for a file, we
have an index for each workspace that stores a reference-counted pointer
to a configuration for a given folder. If another file in the same
folder is opened, the configuration is simply re-used rather than us
re-resolving it.

## Guide for reviewing

The first commit is just the restructuring work, which adds some noise
to the diff. If you want to quickly understand what's actually changed,
I recommend looking at the two commits that come after it.
f7c073d makes configuration a property
of `DocumentController`/`DocumentRef`, moving it out of `Workspace`, and
it also sets up the `ConfigurationIndex`, though it doesn't implement
its key function, `get_or_insert`. In the commit after it,
fc35618, we implement `get_or_insert`.

## Test Plan

The best way to test this would be to ensure that the behavior matches
the Ruff CLI. Open a project with multiple configuration files (or add
them yourself), and then introduce problems in certain files that won't
show due to their configuration. Add those same problems to a section of
the project where those rules are run. Confirm that the lint rules are
run as expected with `ruff check`. Then, open your editor and confirm
that the diagnostics shown match the CLI output.

As an example - I have a workspace with two separate folders, `pandas`
and `scipy`. I created a `pyproject.toml` file in `pandas/pandas/io` and
a `ruff.toml` file in `pandas/pandas/api`. I changed the `select` and
`preview` settings in the sub-folder configuration files and confirmed
that these were reflected in the diagnostics. I also confirmed that this
did not change the diagnostics for the `scipy` folder whatsoever.
  • Loading branch information
snowsignal committed Apr 16, 2024
1 parent 4284e07 commit cffc555
Show file tree
Hide file tree
Showing 12 changed files with 377 additions and 288 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/ruff_server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ rustc-hash = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
tracing = { workspace = true }
walkdir = { workspace = true }

[dev-dependencies]
insta = { workspace = true }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles {
) -> Result<()> {
for change in params.changes {
session
.reload_configuration(&change.uri)
.reload_settings(&change.uri)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
}
Ok(())
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_server/src/server/api/requests/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
document.version(),
)?),
Expand Down Expand Up @@ -140,7 +140,7 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCo
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
document.version(),
)?),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
document.version(),
)
Expand All @@ -56,7 +56,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
document,
snapshot.resolved_client_capabilities(),
snapshot.url(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
document.version(),
)
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server/api/requests/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl super::BackgroundDocumentRequestHandler for DocumentDiagnostic {
let diagnostics = if snapshot.client_settings().lint() {
crate::lint::check(
snapshot.document(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
)
} else {
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_server/src/server/api/requests/execute_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
Command::FixAll => {
let edits = super::code_action_resolve::fix_all_edit(
snapshot.document(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
)
.with_failure_code(ErrorCode::InternalError)?;
Expand All @@ -83,7 +83,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
Command::OrganizeImports => {
let edits = super::code_action_resolve::organize_imports_edit(
snapshot.document(),
&snapshot.configuration().linter,
&snapshot.settings().linter,
snapshot.encoding(),
)
.with_failure_code(ErrorCode::InternalError)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl super::BackgroundDocumentRequestHandler for Format {
pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::FormatResponse> {
let doc = snapshot.document();
let source = doc.contents();
let formatted = crate::format::format(doc, &snapshot.configuration().formatter)
let formatted = crate::format::format(doc, &snapshot.settings().formatter)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
// fast path - if the code is the same, return early
if formatted == source {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl super::BackgroundDocumentRequestHandler for FormatRange {
let index = document.index();
let range = params.range.to_text_range(text, index, snapshot.encoding());
let formatted_range =
crate::format::format_range(document, &snapshot.configuration().formatter, range)
crate::format::format_range(document, &snapshot.settings().formatter, range)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
Ok(Some(vec![types::TextEdit {
range: formatted_range
Expand Down

0 comments on commit cffc555

Please sign in to comment.