Skip to content

Commit

Permalink
Respect file exclusions in ruff server (#11590)
Browse files Browse the repository at this point in the history
## Summary

Closes #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.
  • Loading branch information
charliermarsh authored May 29, 2024
1 parent 531ae52 commit 204c59e
Show file tree
Hide file tree
Showing 11 changed files with 235 additions and 43 deletions.
25 changes: 23 additions & 2 deletions crates/ruff_server/src/fix.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
use std::borrow::Cow;

use rustc_hash::FxHashMap;

use ruff_linter::{
linter::{FixerResult, LinterResult},
packaging::detect_package_root,
settings::{flags, types::UnsafeFixes, LinterSettings},
};
use ruff_notebook::SourceValue;
use ruff_source_file::LineIndex;
use rustc_hash::FxHashMap;
use std::borrow::Cow;
use ruff_workspace::resolver::match_any_exclusion;
use ruff_workspace::FileResolverSettings;

use crate::{
edit::{Replacement, ToRangeExt},
Expand All @@ -20,12 +24,29 @@ pub(crate) type Fixes = FxHashMap<lsp_types::Url, Vec<lsp_types::TextEdit>>;

pub(crate) fn fix_all(
query: &DocumentQuery,
file_resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
encoding: PositionEncoding,
) -> crate::Result<Fixes> {
let document_path = query.file_path();
let source_kind = query.make_source_kind();

// If the document is excluded, return an empty list of fixes.
if let Some(exclusion) = match_any_exclusion(
document_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
Some(&linter_settings.exclude),
None,
) {
tracing::debug!(
"Ignored path via `{}`: {}",
exclusion,
document_path.display()
);
return Ok(Fixes::default());
}

let package = detect_package_root(
document_path
.parent()
Expand Down
19 changes: 19 additions & 0 deletions crates/ruff_server/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use ruff_python_index::Indexer;
use ruff_python_parser::AsMode;
use ruff_source_file::{LineIndex, Locator};
use ruff_text_size::{Ranged, TextRange};
use ruff_workspace::resolver::match_any_exclusion;
use ruff_workspace::FileResolverSettings;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -60,12 +62,29 @@ pub(crate) type Diagnostics = FxHashMap<lsp_types::Url, Vec<lsp_types::Diagnosti

pub(crate) fn check(
query: &DocumentQuery,
file_resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
encoding: PositionEncoding,
) -> Diagnostics {
let document_path = query.file_path();
let source_kind = query.make_source_kind();

// If the document is excluded, return an empty list of diagnostics.
if let Some(exclusion) = match_any_exclusion(
document_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
Some(&linter_settings.exclude),
None,
) {
tracing::debug!(
"Ignored path via `{}`: {}",
exclusion,
document_path.display()
);
return Diagnostics::default();
}

let package = detect_package_root(
document_path
.parent()
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_server/src/server/api/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> Diagnostics {
if snapshot.client_settings().lint() {
crate::lint::check(
snapshot.query(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)
Expand Down
13 changes: 9 additions & 4 deletions crates/ruff_server/src/server/api/requests/code_action.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req};
use rustc_hash::FxHashSet;
use types::{CodeActionKind, CodeActionOrCommand};

use crate::edit::WorkspaceEditTracker;
use crate::lint::{fixes_for_diagnostics, DiagnosticFix};
use crate::server::api::LSPResult;
use crate::server::SupportedCodeAction;
use crate::server::{client::Notifier, Result};
use crate::session::DocumentSnapshot;
use crate::DIAGNOSTIC_NAME;
use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req};
use rustc_hash::FxHashSet;
use types::{CodeActionKind, CodeActionOrCommand};

use super::code_action_resolve::{resolve_edit_for_fix_all, resolve_edit_for_organize_imports};

Expand Down Expand Up @@ -156,6 +157,7 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
Some(resolve_edit_for_fix_all(
document,
snapshot.resolved_client_capabilities(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)?),
Expand Down Expand Up @@ -192,6 +194,7 @@ fn notebook_fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCo
Some(resolve_edit_for_fix_all(
document,
snapshot.resolved_client_capabilities(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)?),
Expand Down Expand Up @@ -228,6 +231,7 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCo
Some(resolve_edit_for_organize_imports(
document,
snapshot.resolved_client_capabilities(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)?),
Expand Down Expand Up @@ -264,6 +268,7 @@ fn notebook_organize_imports(snapshot: &DocumentSnapshot) -> crate::Result<CodeA
Some(resolve_edit_for_organize_imports(
document,
snapshot.resolved_client_capabilities(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)?),
Expand Down
29 changes: 19 additions & 10 deletions crates/ruff_server/src/server/api/requests/code_action_resolve.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
use std::borrow::Cow;

use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req};

use ruff_linter::codes::Rule;
use ruff_linter::settings::LinterSettings;
use ruff_workspace::FileResolverSettings;

use crate::edit::WorkspaceEditTracker;
use crate::fix::Fixes;
use crate::server::api::LSPResult;
use crate::server::SupportedCodeAction;
use crate::server::{client::Notifier, Result};
use crate::session::{DocumentQuery, DocumentSnapshot, ResolvedClientCapabilities};
use crate::PositionEncoding;
use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req};
use ruff_linter::codes::Rule;
use ruff_linter::settings::LinterSettings;

pub(crate) struct CodeActionResolve;

Expand All @@ -22,7 +25,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
fn document_url(params: &types::CodeAction) -> Cow<types::Url> {
let uri: lsp_types::Url = serde_json::from_value(params.data.clone().unwrap_or_default())
.expect("code actions should have a URI in their data fields");
std::borrow::Cow::Owned(uri)
Cow::Owned(uri)
}
fn run_with_snapshot(
snapshot: DocumentSnapshot,
Expand Down Expand Up @@ -54,6 +57,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
resolve_edit_for_fix_all(
query,
snapshot.resolved_client_capabilities(),
query.settings().file_resolver(),
query.settings().linter(),
snapshot.encoding(),
)
Expand All @@ -64,6 +68,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
resolve_edit_for_organize_imports(
query,
snapshot.resolved_client_capabilities(),
query.settings().file_resolver(),
query.settings().linter(),
snapshot.encoding(),
)
Expand All @@ -84,41 +89,45 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
pub(super) fn resolve_edit_for_fix_all(
query: &DocumentQuery,
client_capabilities: &ResolvedClientCapabilities,
file_resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
encoding: PositionEncoding,
) -> crate::Result<types::WorkspaceEdit> {
let mut tracker = WorkspaceEditTracker::new(client_capabilities);
tracker.set_fixes_for_document(
fix_all_edit(query, linter_settings, encoding)?,
fix_all_edit(query, file_resolver_settings, linter_settings, encoding)?,
query.version(),
)?;
Ok(tracker.into_workspace_edit())
}

pub(super) fn fix_all_edit(
query: &DocumentQuery,
file_resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
encoding: PositionEncoding,
) -> crate::Result<Fixes> {
crate::fix::fix_all(query, linter_settings, encoding)
crate::fix::fix_all(query, file_resolver_settings, linter_settings, encoding)
}

pub(super) fn resolve_edit_for_organize_imports(
query: &DocumentQuery,
client_capabilities: &ResolvedClientCapabilities,
linter_settings: &ruff_linter::settings::LinterSettings,
file_resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
encoding: PositionEncoding,
) -> crate::Result<types::WorkspaceEdit> {
let mut tracker = WorkspaceEditTracker::new(client_capabilities);
tracker.set_fixes_for_document(
organize_imports_edit(query, linter_settings, encoding)?,
organize_imports_edit(query, file_resolver_settings, linter_settings, encoding)?,
query.version(),
)?;
Ok(tracker.into_workspace_edit())
}

pub(super) fn organize_imports_edit(
query: &DocumentQuery,
file_resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
encoding: PositionEncoding,
) -> crate::Result<Fixes> {
Expand All @@ -130,5 +139,5 @@ pub(super) fn organize_imports_edit(
.into_iter()
.collect();

crate::fix::fix_all(query, &linter_settings, encoding)
crate::fix::fix_all(query, file_resolver_settings, &linter_settings, encoding)
}
2 changes: 2 additions & 0 deletions crates/ruff_server/src/server/api/requests/execute_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
Command::FixAll => {
let fixes = super::code_action_resolve::fix_all_edit(
snapshot.query(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)
Expand All @@ -81,6 +82,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
Command::OrganizeImports => {
let fixes = super::code_action_resolve::organize_imports_edit(
snapshot.query(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().linter(),
snapshot.encoding(),
)
Expand Down
35 changes: 30 additions & 5 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
use std::path::Path;

use lsp_types::{self as types, request as req};
use types::TextEdit;

use ruff_python_ast::PySourceType;
use ruff_source_file::LineIndex;
use ruff_workspace::resolver::match_any_exclusion;
use ruff_workspace::{FileResolverSettings, FormatterSettings};

use crate::edit::{Replacement, ToRangeExt};
use crate::fix::Fixes;
use crate::server::api::LSPResult;
use crate::server::{client::Notifier, Result};
use crate::session::DocumentSnapshot;
use crate::{PositionEncoding, TextDocument};
use lsp_types::{self as types, request as req};
use ruff_python_ast::PySourceType;
use ruff_source_file::LineIndex;
use ruff_workspace::FormatterSettings;
use types::TextEdit;

pub(crate) struct Format;

Expand Down Expand Up @@ -39,6 +44,8 @@ pub(super) fn format_full_document(snapshot: &DocumentSnapshot) -> Result<Fixes>
if let Some(changes) = format_text_document(
text_document,
snapshot.query().source_type(),
snapshot.query().file_path(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().formatter(),
snapshot.encoding(),
true,
Expand All @@ -50,6 +57,8 @@ pub(super) fn format_full_document(snapshot: &DocumentSnapshot) -> Result<Fixes>
if let Some(changes) = format_text_document(
snapshot.query().as_single_document().unwrap(),
snapshot.query().source_type(),
snapshot.query().file_path(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().formatter(),
snapshot.encoding(),
false,
Expand All @@ -71,6 +80,8 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::Form
format_text_document(
text_document,
snapshot.query().source_type(),
snapshot.query().file_path(),
snapshot.query().settings().file_resolver(),
snapshot.query().settings().formatter(),
snapshot.encoding(),
snapshot.query().as_notebook().is_some(),
Expand All @@ -80,10 +91,24 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::Form
fn format_text_document(
text_document: &TextDocument,
source_type: PySourceType,
file_path: &Path,
file_resolver_settings: &FileResolverSettings,
formatter_settings: &FormatterSettings,
encoding: PositionEncoding,
is_notebook: bool,
) -> Result<super::FormatResponse> {
// If the document is excluded, return early.
if let Some(exclusion) = match_any_exclusion(
file_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
None,
Some(&formatter_settings.exclude),
) {
tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display());
return Ok(None);
}

let source = text_document.contents();
let mut formatted = crate::format::format(text_document, source_type, formatter_settings)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
Expand Down
Loading

0 comments on commit 204c59e

Please sign in to comment.