Skip to content

Commit

Permalink
Directly include Settings struct for the server (#16042)
Browse files Browse the repository at this point in the history
## Summary

This PR refactors the `RuffSettings` struct to directly include the
resolved `Settings` instead of including the specific fields from it.
The server utilizes a lot of it already, so it makes sense to just
include the entire struct for simplicity.

### `Deref`

I implemented `Deref` on `RuffSettings` to return the `Settings` because
`RuffSettings` is now basically a wrapper around it with the config path
as the other field. This path field is only used for debugging
("printDebugInformation" command).
  • Loading branch information
dhruvmanila authored Feb 10, 2025
1 parent b54e390 commit cc0a5dd
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 76 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 @@ -27,15 +27,14 @@ pub(crate) fn fix_all(
encoding: PositionEncoding,
) -> crate::Result<Fixes> {
let source_kind = query.make_source_kind();

let file_resolver_settings = query.settings().file_resolver();
let settings = query.settings();
let document_path = query.file_path();

// 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_for_linting(
document_path,
file_resolver_settings,
&settings.file_resolver,
linter_settings,
query.text_document_language_id(),
) {
Expand Down Expand Up @@ -71,7 +70,7 @@ pub(crate) fn fix_all(
&query.virtual_file_path(),
package,
flags::Noqa::Enabled,
query.settings().unsafe_fixes(),
settings.unsafe_fixes,
linter_settings,
&source_kind,
source_type,
Expand Down
13 changes: 6 additions & 7 deletions crates/ruff_server/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,15 @@ pub(crate) fn check(
show_syntax_errors: bool,
) -> DiagnosticsMap {
let source_kind = query.make_source_kind();
let file_resolver_settings = query.settings().file_resolver();
let linter_settings = query.settings().linter();
let settings = query.settings();
let document_path = query.file_path();

// 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_for_linting(
document_path,
file_resolver_settings,
linter_settings,
&settings.file_resolver,
&settings.linter,
query.text_document_language_id(),
) {
return DiagnosticsMap::default();
Expand All @@ -86,7 +85,7 @@ pub(crate) fn check(
document_path
.parent()
.expect("a path to a document should have a parent path"),
&linter_settings.namespace_packages,
&settings.linter.namespace_packages,
)
.map(PackageRoot::root)
} else {
Expand Down Expand Up @@ -118,7 +117,7 @@ pub(crate) fn check(
&stylist,
&indexer,
&directives,
linter_settings,
&settings.linter,
flags::Noqa::Enabled,
&source_kind,
source_type,
Expand All @@ -130,7 +129,7 @@ pub(crate) fn check(
&diagnostics,
&locator,
indexer.comment_ranges(),
&linter_settings.external,
&settings.linter.external,
&directives.noqa_line_for,
stylist.line_ending(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub(super) fn fix_all_edit(
query: &DocumentQuery,
encoding: PositionEncoding,
) -> crate::Result<Fixes> {
crate::fix::fix_all(query, query.settings().linter(), encoding)
crate::fix::fix_all(query, &query.settings().linter, encoding)
}

pub(super) fn resolve_edit_for_organize_imports(
Expand All @@ -110,7 +110,7 @@ pub(super) fn organize_imports_edit(
query: &DocumentQuery,
encoding: PositionEncoding,
) -> crate::Result<Fixes> {
let mut linter_settings = query.settings().linter().clone();
let mut linter_settings = query.settings().linter.clone();
linter_settings.rules = [
Rule::UnsortedImports, // I001
Rule::MissingRequiredImport, // I002
Expand Down
9 changes: 4 additions & 5 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,22 @@ fn format_text_document(
encoding: PositionEncoding,
is_notebook: bool,
) -> Result<super::FormatResponse> {
let file_resolver_settings = query.settings().file_resolver();
let formatter_settings = query.settings().formatter();
let settings = query.settings();

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

let source = text_document.contents();
let formatted = crate::format::format(text_document, query.source_type(), formatter_settings)
let formatted = crate::format::format(text_document, query.source_type(), &settings.formatter)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
let Some(mut formatted) = formatted else {
return Ok(None);
Expand Down
9 changes: 4 additions & 5 deletions crates/ruff_server/src/server/api/requests/format_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,14 @@ fn format_text_document_range(
query: &DocumentQuery,
encoding: PositionEncoding,
) -> Result<super::FormatResponse> {
let file_resolver_settings = query.settings().file_resolver();
let formatter_settings = query.settings().formatter();
let settings = query.settings();

// If the document is excluded, return early.
if let Some(file_path) = query.file_path() {
if is_document_excluded_for_formatting(
&file_path,
file_resolver_settings,
formatter_settings,
&settings.file_resolver,
&settings.formatter,
text_document.language_id(),
) {
return Ok(None);
Expand All @@ -67,7 +66,7 @@ fn format_text_document_range(
let formatted_range = crate::format::format_range(
text_document,
query.source_type(),
formatter_settings,
&settings.formatter,
range,
)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
Expand Down
71 changes: 18 additions & 53 deletions crates/ruff_server/src/session/index/ruff_settings.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
use std::collections::BTreeMap;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;

use anyhow::Context;
use ignore::{WalkBuilder, WalkState};

use ruff_linter::settings::types::UnsafeFixes;
use ruff_linter::{
display_settings, fs::normalize_path_to, settings::types::FilePattern,
settings::types::PreviewMode,
fs::normalize_path_to, settings::types::FilePattern, settings::types::PreviewMode,
};
use ruff_workspace::resolver::match_exclusion;
use ruff_workspace::Settings;
use ruff_workspace::{
configuration::{Configuration, FormatConfiguration, LintConfiguration, RuleSelection},
pyproject::{find_user_settings_toml, settings_toml},
Expand All @@ -24,14 +24,16 @@ pub struct RuffSettings {
/// The path to this configuration file, used for debugging.
/// The default fallback configuration does not have a file path.
path: Option<PathBuf>,
/// Toggle for unsafe fixes.
unsafe_fixes: UnsafeFixes,
/// Settings used to manage file inclusion and exclusion.
file_resolver: ruff_workspace::FileResolverSettings,
/// Settings to pass into the Ruff linter.
linter: ruff_linter::settings::LinterSettings,
/// Settings to pass into the Ruff formatter.
formatter: ruff_workspace::FormatterSettings,
/// The resolved settings.
settings: Settings,
}

impl Deref for RuffSettings {
type Target = Settings;

fn deref(&self) -> &Settings {
&self.settings
}
}

pub(super) struct RuffSettingsIndex {
Expand All @@ -42,15 +44,7 @@ pub(super) struct RuffSettingsIndex {

impl std::fmt::Display for RuffSettings {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
display_settings! {
formatter = f,
fields = [
self.file_resolver,
self.linter,
self.formatter
]
}
Ok(())
std::fmt::Display::fmt(&self.settings, f)
}
}

Expand Down Expand Up @@ -80,32 +74,9 @@ impl RuffSettings {

RuffSettings {
path,
unsafe_fixes: fallback.unsafe_fixes,
file_resolver: fallback.file_resolver,
formatter: fallback.formatter,
linter: fallback.linter,
settings: fallback,
}
}

/// Return the [`ruff_workspace::FileResolverSettings`] for this [`RuffSettings`].
pub(crate) fn file_resolver(&self) -> &ruff_workspace::FileResolverSettings {
&self.file_resolver
}

/// Return the [`ruff_linter::settings::LinterSettings`] for this [`RuffSettings`].
pub(crate) fn linter(&self) -> &ruff_linter::settings::LinterSettings {
&self.linter
}

/// Return the [`ruff_workspace::FormatterSettings`] for this [`RuffSettings`].
pub(crate) fn formatter(&self) -> &ruff_workspace::FormatterSettings {
&self.formatter
}

/// Return the [`UnsafeFixes`] for this [`RuffSettings`].
pub(crate) fn unsafe_fixes(&self) -> UnsafeFixes {
self.unsafe_fixes
}
}

impl RuffSettingsIndex {
Expand Down Expand Up @@ -152,10 +123,7 @@ impl RuffSettingsIndex {
directory.to_path_buf(),
Arc::new(RuffSettings {
path: Some(pyproject),
unsafe_fixes: settings.unsafe_fixes,
file_resolver: settings.file_resolver,
linter: settings.linter,
formatter: settings.formatter,
settings,
}),
);
break;
Expand Down Expand Up @@ -210,7 +178,7 @@ impl RuffSettingsIndex {
// Add any settings within the workspace itself
let mut builder = WalkBuilder::new(root);
builder.standard_filters(
respect_gitignore.unwrap_or_else(|| fallback.file_resolver().respect_gitignore),
respect_gitignore.unwrap_or_else(|| fallback.file_resolver.respect_gitignore),
);
builder.hidden(false);
builder.threads(
Expand Down Expand Up @@ -277,10 +245,7 @@ impl RuffSettingsIndex {
directory,
Arc::new(RuffSettings {
path: Some(pyproject),
unsafe_fixes: settings.unsafe_fixes,
file_resolver: settings.file_resolver,
linter: settings.linter,
formatter: settings.formatter,
settings,
}),
);
}
Expand Down

0 comments on commit cc0a5dd

Please sign in to comment.