Skip to content

Commit

Permalink
Format Action
Browse files Browse the repository at this point in the history
  • Loading branch information
T-256 committed Jun 8, 2024
1 parent 32ca704 commit 56e3832
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ benchmark = "bench -p ruff_benchmark --bench linter --bench formatter --"
# that shared/dynamic library.
#
# See: https://github.com/astral-sh/ruff/issues/11503
[target.'cfg(all(target_env="msvc", target_os = "windows"))']
[target.'cfg(all(target_env = "msvc", target_os = "windows"))']
rustflags = ["-C", "target-feature=+crt-static"]
47 changes: 44 additions & 3 deletions crates/ruff_server/src/fix.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::borrow::Cow;
use std::{borrow::Cow, sync::Arc};

use rustc_hash::FxHashMap;

Expand All @@ -8,13 +8,13 @@ use ruff_linter::{
settings::{flags, types::UnsafeFixes, LinterSettings},
};
use ruff_notebook::SourceValue;
use ruff_source_file::LineIndex;
use ruff_source_file::{LineIndex, OneIndexed};
use ruff_workspace::resolver::match_any_exclusion;

use crate::{
edit::{Replacement, ToRangeExt},
session::DocumentQuery,
PositionEncoding,
PositionEncoding, TextDocument,
};

/// A simultaneous fix made across a single text document or among an arbitrary
Expand Down Expand Up @@ -165,3 +165,44 @@ pub(crate) fn fix_all(
.collect())
}
}

pub(crate) fn parse_all(query: &DocumentQuery, fixes: Fixes) -> DocumentQuery {
match query {
DocumentQuery::Text {
file_url,
document,
settings,
} => {
let mut contents = document.contents().to_owned();
for (url, edits) in fixes {
if url == *file_url {
let text_edit = edits.first().unwrap();
let source_index = LineIndex::from_source_text(&contents);
let start_offset = source_index
.line_start(
OneIndexed::from_zero_indexed(text_edit.range.start.line as usize),
&contents,
)
.to_usize();
let end_offset = source_index
.line_start(
OneIndexed::from_zero_indexed(text_edit.range.end.line as usize),
&contents,
)
.to_usize();
let mut new_contents = String::new();
new_contents.push_str(&contents[..start_offset]);
new_contents.push_str(&text_edit.new_text);
new_contents.push_str(&contents[end_offset..]);
contents = new_contents;
}
}
DocumentQuery::Text {
file_url: file_url.clone(),
document: Arc::new(TextDocument::new(contents, document.version())),
settings: settings.clone(),
}
}
DocumentQuery::Notebook { .. } => query.clone(), // TODO
}
}
46 changes: 40 additions & 6 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req};
use types::TextEdit;

Expand All @@ -11,6 +12,8 @@ use crate::server::{client::Notifier, Result};
use crate::session::{DocumentQuery, DocumentSnapshot};
use crate::{PositionEncoding, TextDocument};

use super::code_action_resolve::{fix_all_edit, organize_imports_edit};

pub(crate) struct Format;

impl super::RequestHandler for Format {
Expand All @@ -24,7 +27,37 @@ impl super::BackgroundDocumentRequestHandler for Format {
_notifier: Notifier,
_params: types::DocumentFormattingParams,
) -> Result<super::FormatResponse> {
format_document(&snapshot)
match snapshot.query() {
DocumentQuery::Text { .. } => {
let mut query = snapshot.query().to_owned();
let mut response = None;

if snapshot.client_settings().organize_imports()
&& snapshot.client_settings().format_action().isort
{
let fixes = organize_imports_edit(&query, snapshot.encoding())
.with_failure_code(ErrorCode::InternalError)?;
query = crate::fix::parse_all(&query, fixes.clone());
response = fixes.values().next().map(std::borrow::ToOwned::to_owned);
}
if snapshot.client_settings().fix_all()
&& snapshot.client_settings().format_action().fix
{
let fixes = fix_all_edit(&query, snapshot.encoding())
.with_failure_code(ErrorCode::InternalError)?;
query = crate::fix::parse_all(&query, fixes.clone());
response = fixes.values().next().map(std::borrow::ToOwned::to_owned);
}
if snapshot.client_settings().format_action().style {
response = format_document(&query, snapshot.encoding())?;
}
Ok(response)
}
DocumentQuery::Notebook { .. } => {
format_document(snapshot.query(), snapshot.encoding())
} // TODO
}
// if snapshot.client_settings().format_action().style {}
}
}

Expand Down Expand Up @@ -60,16 +93,17 @@ pub(super) fn format_full_document(snapshot: &DocumentSnapshot) -> Result<Fixes>

/// Formats either a full text document or an specific notebook cell. If the query within the snapshot is a notebook document
/// with no selected cell, this will throw an error.
pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::FormatResponse> {
let text_document = snapshot
.query()
pub(super) fn format_document(
query: &DocumentQuery,
encoding: PositionEncoding,
) -> Result<super::FormatResponse> {
let text_document = query
.as_single_document()
.expect("format should only be called on text documents or notebook cells");
let query = snapshot.query();
format_text_document(
text_document,
query,
snapshot.encoding(),
encoding,
query.as_notebook().is_some(),
)
}
Expand Down
88 changes: 84 additions & 4 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub(crate) type WorkspaceSettingsMap = FxHashMap<Url, ClientSettings>;
#[cfg_attr(test, derive(PartialEq, Eq))]
#[allow(clippy::struct_excessive_bools)]
pub(crate) struct ResolvedClientSettings {
format_action: ResolvedFormatAction,
fix_all: bool,
organize_imports: bool,
lint_enable: bool,
Expand All @@ -24,6 +25,14 @@ pub(crate) struct ResolvedClientSettings {
editor_settings: ResolvedEditorSettings,
}

#[derive(Clone, Debug)]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub(crate) struct ResolvedFormatAction {
pub(crate) fix: bool,
pub(crate) isort: bool,
pub(crate) style: bool,
}

/// Contains the resolved values of 'editor settings' - Ruff configuration for the linter/formatter that was passed in via
/// LSP client settings. These fields are optional because we don't want to override file-based linter/formatting settings
/// if these were un-set.
Expand Down Expand Up @@ -62,11 +71,10 @@ pub(crate) enum ConfigurationPreference {
#[serde(rename_all = "camelCase")]
pub(crate) struct ClientSettings {
configuration: Option<String>,
fix_all: Option<bool>,
organize_imports: Option<bool>,
lint: Option<LintOptions>,
format: Option<FormatOptions>,
code_action: Option<CodeActionOptions>,
source_action: Option<SourceActionOptions>,
exclude: Option<Vec<String>>,
line_length: Option<LineLength>,
configuration_preference: Option<ConfigurationPreference>,
Expand Down Expand Up @@ -100,6 +108,7 @@ struct LintOptions {
#[serde(rename_all = "camelCase")]
struct FormatOptions {
preview: Option<bool>,
action: Option<FormatActionParameters>,
}

#[derive(Debug, Default, Deserialize)]
Expand All @@ -117,6 +126,30 @@ struct CodeActionParameters {
enable: Option<bool>,
}

#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct SourceActionOptions {
fix_all: Option<SourceActionParameters>,
organize_imports: Option<SourceActionParameters>,
}

#[derive(Debug, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct SourceActionParameters {
enable: Option<bool>,
}

#[derive(Debug, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct FormatActionParameters {
fix: Option<bool>,
isort: Option<bool>,
style: Option<bool>,
}

/// This is the exact schema for initialization options sent in by the client
/// during initialization.
#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -194,10 +227,38 @@ impl ResolvedClientSettings {

fn new_impl(all_settings: &[&ClientSettings]) -> Self {
Self {
fix_all: Self::resolve_or(all_settings, |settings| settings.fix_all, true),
format_action: ResolvedFormatAction {
fix: Self::resolve_or(
all_settings,
|settings| settings.format.as_ref()?.action.as_ref()?.fix,
false,
),
isort: Self::resolve_or(
all_settings,
|settings| settings.format.as_ref()?.action.as_ref()?.isort,
false,
),
style: Self::resolve_or(
all_settings,
|settings| settings.format.as_ref()?.action.as_ref()?.style,
true,
),
},
fix_all: Self::resolve_or(
all_settings,
|settings| settings.source_action.as_ref()?.fix_all.as_ref()?.enable,
true,
),
organize_imports: Self::resolve_or(
all_settings,
|settings| settings.organize_imports,
|settings| {
settings
.source_action
.as_ref()?
.organize_imports
.as_ref()?
.enable
},
true,
),
lint_enable: Self::resolve_or(
Expand Down Expand Up @@ -310,6 +371,10 @@ impl ResolvedClientSettings {
}

impl ResolvedClientSettings {
pub(crate) fn format_action(&self) -> &ResolvedFormatAction {
&self.format_action
}

pub(crate) fn fix_all(&self) -> bool {
self.fix_all
}
Expand Down Expand Up @@ -570,6 +635,11 @@ mod tests {
&global_settings
),
ResolvedClientSettings {
format_action: ResolvedFormatAction {
fix: false,
isort: false,
style: true,
},
fix_all: true,
organize_imports: true,
lint_enable: true,
Expand Down Expand Up @@ -601,6 +671,11 @@ mod tests {
&global_settings
),
ResolvedClientSettings {
format_action: ResolvedFormatAction {
fix: false,
isort: false,
style: true,
},
fix_all: true,
organize_imports: true,
lint_enable: true,
Expand Down Expand Up @@ -688,6 +763,11 @@ mod tests {
assert_eq!(
ResolvedClientSettings::global(&global_settings),
ResolvedClientSettings {
format_action: ResolvedFormatAction {
fix: false,
isort: false,
style: true,
},
fix_all: false,
organize_imports: true,
lint_enable: true,
Expand Down

0 comments on commit 56e3832

Please sign in to comment.