Skip to content

Commit

Permalink
Use tool specific function to perform exclude checks (#15486)
Browse files Browse the repository at this point in the history
## Summary

This PR creates separate functions to check whether the document path is
excluded for linting or formatting. The main motivation is to avoid the
double `Option` for the call sites and makes passing the correct
settings simpler.
  • Loading branch information
dhruvmanila authored Jan 15, 2025
1 parent aefb607 commit bec8441
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 17 deletions.
7 changes: 3 additions & 4 deletions crates/ruff_server/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_hash::FxHashMap;

use crate::{
edit::{Replacement, ToRangeExt},
resolve::is_document_excluded,
resolve::is_document_excluded_for_linting,
session::DocumentQuery,
PositionEncoding,
};
Expand Down Expand Up @@ -33,11 +33,10 @@ pub(crate) fn fix_all(

// If the document is excluded, return an empty list of fixes.
let package = if let Some(document_path) = document_path.as_ref() {
if is_document_excluded(
if is_document_excluded_for_linting(
document_path,
file_resolver_settings,
Some(linter_settings),
None,
linter_settings,
query.text_document_language_id(),
) {
return Ok(Fixes::default());
Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_server/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};

use crate::{
edit::{NotebookRange, ToRangeExt},
resolve::is_document_excluded,
resolve::is_document_excluded_for_linting,
session::DocumentQuery,
PositionEncoding, DIAGNOSTIC_NAME,
};
Expand Down Expand Up @@ -73,11 +73,10 @@ pub(crate) fn check(

// If the document is excluded, return an empty list of diagnostics.
let package = if let Some(document_path) = document_path.as_ref() {
if is_document_excluded(
if is_document_excluded_for_linting(
document_path,
file_resolver_settings,
Some(linter_settings),
None,
linter_settings,
query.text_document_language_id(),
) {
return DiagnosticsMap::default();
Expand Down
36 changes: 35 additions & 1 deletion crates/ruff_server/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,38 @@ use ruff_workspace::{FileResolverSettings, FormatterSettings};

use crate::edit::LanguageId;

/// Return `true` if the document at the given [`Path`] should be excluded from linting.
pub(crate) fn is_document_excluded_for_linting(
path: &Path,
resolver_settings: &FileResolverSettings,
linter_settings: &LinterSettings,
language_id: Option<LanguageId>,
) -> bool {
is_document_excluded(
path,
resolver_settings,
Some(linter_settings),
None,
language_id,
)
}

/// Return `true` if the document at the given [`Path`] should be excluded from formatting.
pub(crate) fn is_document_excluded_for_formatting(
path: &Path,
resolver_settings: &FileResolverSettings,
formatter_settings: &FormatterSettings,
language_id: Option<LanguageId>,
) -> bool {
is_document_excluded(
path,
resolver_settings,
None,
Some(formatter_settings),
language_id,
)
}

/// Return `true` if the document at the given [`Path`] should be excluded.
///
/// The tool-specific settings should be provided if the request for the document is specific to
Expand All @@ -16,7 +48,9 @@ use crate::edit::LanguageId;
/// 1. Check for global `exclude` and `extend-exclude` options along with tool specific `exclude`
/// option (`lint.exclude`, `format.exclude`).
/// 2. Check for global `include` and `extend-include` options.
pub(crate) fn is_document_excluded(
/// 3. Check if the language ID is Python, in which case the document is included.
/// 4. If none of the above conditions are met, the document is excluded.
fn is_document_excluded(
path: &Path,
resolver_settings: &FileResolverSettings,
linter_settings: Option<&LinterSettings>,
Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_source_file::LineIndex;

use crate::edit::{Replacement, ToRangeExt};
use crate::fix::Fixes;
use crate::resolve::is_document_excluded;
use crate::resolve::is_document_excluded_for_formatting;
use crate::server::api::LSPResult;
use crate::server::{client::Notifier, Result};
use crate::session::{DocumentQuery, DocumentSnapshot};
Expand Down Expand Up @@ -87,11 +87,10 @@ fn format_text_document(

// If the document is excluded, return early.
if let Some(file_path) = query.file_path() {
if is_document_excluded(
if is_document_excluded_for_formatting(
&file_path,
file_resolver_settings,
None,
Some(formatter_settings),
formatter_settings,
text_document.language_id(),
) {
return Ok(None);
Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Context;
use lsp_types::{self as types, request as req, Range};

use crate::edit::{RangeExt, ToRangeExt};
use crate::resolve::is_document_excluded;
use crate::resolve::is_document_excluded_for_formatting;
use crate::server::api::LSPResult;
use crate::server::{client::Notifier, Result};
use crate::session::{DocumentQuery, DocumentSnapshot};
Expand Down Expand Up @@ -51,11 +51,10 @@ fn format_text_document_range(

// If the document is excluded, return early.
if let Some(file_path) = query.file_path() {
if is_document_excluded(
if is_document_excluded_for_formatting(
&file_path,
file_resolver_settings,
None,
Some(formatter_settings),
formatter_settings,
text_document.language_id(),
) {
return Ok(None);
Expand Down

0 comments on commit bec8441

Please sign in to comment.