Skip to content

Commit

Permalink
Conside include, extend-include for the native server (#12252)
Browse files Browse the repository at this point in the history
## 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.

https://github.com/astral-sh/ruff/assets/67177269/e8969128-c175-4f98-8114-0d692b906cc8
  • Loading branch information
dhruvmanila authored Jul 10, 2024
1 parent 855d62c commit 0bb2fc6
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 47 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

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

1 change: 0 additions & 1 deletion crates/ruff_server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ ruff_workspace = { workspace = true }

anyhow = { workspace = true }
crossbeam = { workspace = true }
globset = { workspace = true }
jod-thread = { workspace = true }
lsp-server = { workspace = true }
lsp-types = { workspace = true }
Expand Down
14 changes: 4 additions & 10 deletions crates/ruff_server/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use ruff_linter::{
};
use ruff_notebook::SourceValue;
use ruff_source_file::LineIndex;
use ruff_workspace::resolver::match_any_exclusion;

use crate::{
edit::{Replacement, ToRangeExt},
resolve::is_document_excluded,
session::DocumentQuery,
PositionEncoding,
};
Expand All @@ -33,18 +33,12 @@ 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 let Some(exclusion) = match_any_exclusion(
if is_document_excluded(
document_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
Some(&linter_settings.exclude),
file_resolver_settings,
Some(linter_settings),
None,
) {
tracing::debug!(
"Ignored path via `{}`: {}",
exclusion,
document_path.display()
);
return Ok(Fixes::default());
}

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod edit;
mod fix;
mod format;
mod lint;
mod resolve;
mod server;
mod session;
mod trace;
Expand Down
14 changes: 4 additions & 10 deletions crates/ruff_server/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_source_file::{LineIndex, Locator};
use ruff_text_size::{Ranged, TextRange};
use ruff_workspace::resolver::match_any_exclusion;

use crate::{
edit::{NotebookRange, ToRangeExt},
resolve::is_document_excluded,
session::DocumentQuery,
PositionEncoding, DIAGNOSTIC_NAME,
};
Expand Down Expand Up @@ -72,18 +72,12 @@ 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 let Some(exclusion) = match_any_exclusion(
if is_document_excluded(
document_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
Some(&linter_settings.exclude),
file_resolver_settings,
Some(linter_settings),
None,
) {
tracing::debug!(
"Ignored path via `{}`: {}",
exclusion,
document_path.display()
);
return DiagnosticsMap::default();
}

Expand Down
45 changes: 45 additions & 0 deletions crates/ruff_server/src/resolve.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use std::path::Path;

use ruff_linter::settings::LinterSettings;
use ruff_workspace::resolver::{match_any_exclusion, match_any_inclusion};
use ruff_workspace::{FileResolverSettings, FormatterSettings};

/// 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
/// that tool. For example, a diagnostics request should provide the linter settings while the
/// formatting request should provide the formatter settings.
///
/// The logic for the resolution considers both inclusion and exclusion and is as follows:
/// 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(
path: &Path,
resolver_settings: &FileResolverSettings,
linter_settings: Option<&LinterSettings>,
formatter_settings: Option<&FormatterSettings>,
) -> bool {
if let Some(exclusion) = match_any_exclusion(
path,
&resolver_settings.exclude,
&resolver_settings.extend_exclude,
linter_settings.map(|s| &*s.exclude),
formatter_settings.map(|s| &*s.exclude),
) {
tracing::debug!("Ignored path via `{}`: {}", exclusion, path.display());
return true;
}

if let Some(inclusion) = match_any_inclusion(
path,
&resolver_settings.include,
&resolver_settings.extend_include,
) {
tracing::debug!("Included path via `{}`: {}", inclusion, path.display());
false
} else {
// Path is excluded by not being in the inclusion set.
true
}
}
10 changes: 4 additions & 6 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use lsp_types::{self as types, request as req};
use types::TextEdit;

use ruff_source_file::LineIndex;
use ruff_workspace::resolver::match_any_exclusion;

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

// If the document is excluded, return early.
if let Some(file_path) = query.file_path() {
if let Some(exclusion) = match_any_exclusion(
if is_document_excluded(
&file_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
file_resolver_settings,
None,
Some(&formatter_settings.exclude),
Some(formatter_settings),
) {
tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display());
return Ok(None);
}
}
Expand Down
11 changes: 4 additions & 7 deletions crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use lsp_types::{self as types, request as req, Range};

use ruff_workspace::resolver::match_any_exclusion;

use crate::edit::{RangeExt, ToRangeExt};
use crate::resolve::is_document_excluded;
use crate::server::api::LSPResult;
use crate::server::{client::Notifier, Result};
use crate::session::{DocumentQuery, DocumentSnapshot};
Expand Down Expand Up @@ -50,14 +49,12 @@ fn format_text_document_range(

// If the document is excluded, return early.
if let Some(file_path) = query.file_path() {
if let Some(exclusion) = match_any_exclusion(
if is_document_excluded(
&file_path,
&file_resolver_settings.exclude,
&file_resolver_settings.extend_exclude,
file_resolver_settings,
None,
Some(&formatter_settings.exclude),
Some(formatter_settings),
) {
tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display());
return Ok(None);
}
}
Expand Down
18 changes: 6 additions & 12 deletions crates/ruff_server/src/session/index/ruff_settings.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use globset::Candidate;
use ruff_linter::{
display_settings, fs::normalize_path_to, settings::types::FilePattern,
settings::types::PreviewMode,
};
use ruff_workspace::resolver::match_candidate_exclusion;
use ruff_workspace::resolver::match_exclusion;
use ruff_workspace::{
configuration::{Configuration, FormatConfiguration, LintConfiguration, RuleSelection},
pyproject::{find_user_settings_toml, settings_toml},
Expand Down Expand Up @@ -41,6 +40,7 @@ impl std::fmt::Display for RuffSettings {
display_settings! {
formatter = f,
fields = [
self.file_resolver,
self.linter,
self.formatter
]
Expand Down Expand Up @@ -146,20 +146,14 @@ impl RuffSettingsIndex {
.range(..directory.clone())
.rfind(|(path, _)| directory.starts_with(path))
{
let candidate = Candidate::new(&directory);
let basename = Candidate::new(file_name);
if match_candidate_exclusion(
&candidate,
&basename,
&settings.file_resolver.exclude,
) {
if match_exclusion(&directory, file_name, &settings.file_resolver.exclude) {
tracing::debug!("Ignored path via `exclude`: {}", directory.display());

walker.skip_current_dir();
continue;
} else if match_candidate_exclusion(
&candidate,
&basename,
} else if match_exclusion(
&directory,
file_name,
&settings.file_resolver.extend_exclude,
) {
tracing::debug!(
Expand Down
33 changes: 33 additions & 0 deletions crates/ruff_workspace/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,39 @@ pub fn match_any_exclusion(
None
}

#[derive(Debug, Copy, Clone)]
pub enum InclusionKind {
/// The inclusion came from the `include` setting.
Include,
/// The inclusion came from the `extend-include` setting.
ExtendInclude,
}

impl std::fmt::Display for InclusionKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
InclusionKind::Include => write!(f, "include"),
InclusionKind::ExtendInclude => write!(f, "extend-include"),
}
}
}

/// Return the [`InclusionKind`] for a given [`Path`], if the path match any of the inclusion
/// criteria.
pub fn match_any_inclusion(
path: &Path,
include: &GlobSet,
extend_include: &GlobSet,
) -> Option<InclusionKind> {
if include.is_match(path) {
Some(InclusionKind::Include)
} else if extend_include.is_match(path) {
Some(InclusionKind::ExtendInclude)
} else {
None
}
}

#[cfg(test)]
mod tests {
use std::fs::{create_dir, File};
Expand Down

0 comments on commit 0bb2fc6

Please sign in to comment.