ruff server now supports the source.organizeImports source action (#10652)

## Summary

This builds on top of the work in
https://github.com/astral-sh/ruff/pull/10597 to support `Ruff: Organize
imports` as an available source action.

To do this, we have to support `Clone`-ing for linter settings, since we
need to modify them in place to select import-related diagnostics
specifically (`I001` and `I002`).

## Test Plan


04282d01-dfda-4ac5-aa8f-6a92d5f85bfd
This commit is contained in:
Jane Lewis 2024-04-04 15:20:50 -07:00 committed by GitHub
parent fd8da66fcb
commit d050d6da2e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
33 changed files with 106 additions and 39 deletions

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
#[allow(clippy::struct_excessive_bools)] #[allow(clippy::struct_excessive_bools)]
pub struct Settings { pub struct Settings {
pub mypy_init_return: bool, pub mypy_init_return: bool,

View file

@ -10,7 +10,7 @@ pub fn default_tmp_dirs() -> Vec<String> {
.to_vec() .to_vec()
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub hardcoded_tmp_directory: Vec<String>, pub hardcoded_tmp_directory: Vec<String>,
pub check_typed_exception: bool, pub check_typed_exception: bool,

View file

@ -6,7 +6,7 @@ use ruff_macros::CacheKey;
use crate::display_settings; use crate::display_settings;
#[derive(Debug, CacheKey, Default)] #[derive(Debug, Clone, CacheKey, Default)]
pub struct Settings { pub struct Settings {
pub extend_allowed_calls: Vec<String>, pub extend_allowed_calls: Vec<String>,
} }

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub extend_immutable_calls: Vec<String>, pub extend_immutable_calls: Vec<String>,
} }

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub builtins_ignorelist: Vec<String>, pub builtins_ignorelist: Vec<String>,
} }

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub allow_dict_calls_with_keyword_arguments: bool, pub allow_dict_calls_with_keyword_arguments: bool,
} }

View file

@ -7,7 +7,7 @@ use std::fmt::{Display, Formatter};
use crate::display_settings; use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub notice_rgx: Regex, pub notice_rgx: Regex,
pub author: Option<String>, pub author: Option<String>,

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub max_string_length: usize, pub max_string_length: usize,
} }

View file

@ -2,7 +2,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub functions_names: Vec<String>, pub functions_names: Vec<String>,
} }

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub allow_multiline: bool, pub allow_multiline: bool,
} }

View file

@ -57,7 +57,7 @@ impl FromIterator<String> for BannedAliases {
} }
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub aliases: FxHashMap<String, String>, pub aliases: FxHashMap<String, String>,
pub banned_aliases: FxHashMap<String, BannedAliases>, pub banned_aliases: FxHashMap<String, BannedAliases>,

View file

@ -24,7 +24,7 @@ pub fn default_broad_exceptions() -> Vec<IdentifierPattern> {
.to_vec() .to_vec()
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub fixture_parentheses: bool, pub fixture_parentheses: bool,
pub parametrize_names_type: types::ParametrizeNameType, pub parametrize_names_type: types::ParametrizeNameType,

View file

@ -31,7 +31,7 @@ impl From<ruff_python_ast::str::Quote> for Quote {
} }
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub inline_quotes: Quote, pub inline_quotes: Quote,
pub multiline_quotes: Quote, pub multiline_quotes: Quote,

View file

@ -17,7 +17,7 @@ pub const IGNORE_NAMES: [&str; 7] = [
"_value_", "_value_",
]; ];
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub ignore_names: Vec<String>, pub ignore_names: Vec<String>,
} }

View file

@ -39,7 +39,7 @@ impl Display for Strictness {
} }
} }
#[derive(Debug, CacheKey, Default)] #[derive(Debug, Clone, CacheKey, Default)]
pub struct Settings { pub struct Settings {
pub ban_relative_imports: Strictness, pub ban_relative_imports: Strictness,
pub banned_api: FxHashMap<String, ApiBan>, pub banned_api: FxHashMap<String, ApiBan>,

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub strict: bool, pub strict: bool,
pub exempt_modules: Vec<String>, pub exempt_modules: Vec<String>,

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub ignore_variadic_names: bool, pub ignore_variadic_names: bool,
} }

View file

@ -270,7 +270,7 @@ pub(crate) fn categorize_imports<'a>(
block_by_type block_by_type
} }
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct KnownModules { pub struct KnownModules {
/// A map of known modules to their section. /// A map of known modules to their section.
known: Vec<(glob::Pattern, ImportSection)>, known: Vec<(glob::Pattern, ImportSection)>,

View file

@ -44,7 +44,7 @@ impl Display for RelativeImportsOrder {
} }
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
#[allow(clippy::struct_excessive_bools)] #[allow(clippy::struct_excessive_bools)]
pub struct Settings { pub struct Settings {
pub required_imports: BTreeSet<String>, pub required_imports: BTreeSet<String>,

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter}; use std::fmt::{Display, Formatter};
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub max_complexity: usize, pub max_complexity: usize,
} }

View file

@ -11,7 +11,7 @@ use ruff_macros::CacheKey;
use crate::display_settings; use crate::display_settings;
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub ignore_names: IgnoreNames, pub ignore_names: IgnoreNames,
pub classmethod_decorators: Vec<String>, pub classmethod_decorators: Vec<String>,
@ -85,7 +85,7 @@ static DEFAULTS: &[&str] = &[
"maxDiff", "maxDiff",
]; ];
#[derive(Debug)] #[derive(Debug, Clone)]
pub enum IgnoreNames { pub enum IgnoreNames {
Default, Default,
UserProvided { UserProvided {

View file

@ -6,7 +6,7 @@ use std::fmt;
use crate::line_width::LineLength; use crate::line_width::LineLength;
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub max_line_length: LineLength, pub max_line_length: LineLength,
pub max_doc_length: Option<LineLength>, pub max_doc_length: Option<LineLength>,

View file

@ -83,7 +83,7 @@ impl fmt::Display for Convention {
} }
} }
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub convention: Option<Convention>, pub convention: Option<Convention>,
pub ignore_decorators: BTreeSet<String>, pub ignore_decorators: BTreeSet<String>,

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt; use std::fmt;
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub extend_generics: Vec<String>, pub extend_generics: Vec<String>,
} }

View file

@ -48,7 +48,7 @@ impl fmt::Display for ConstantType {
} }
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct Settings { pub struct Settings {
pub allow_magic_value_types: Vec<ConstantType>, pub allow_magic_value_types: Vec<ConstantType>,
pub allow_dunder_method_names: FxHashSet<String>, pub allow_dunder_method_names: FxHashSet<String>,

View file

@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey; use ruff_macros::CacheKey;
use std::fmt; use std::fmt;
#[derive(Debug, Default, CacheKey)] #[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings { pub struct Settings {
pub keep_runtime_typing: bool, pub keep_runtime_typing: bool,
} }

View file

@ -14,7 +14,7 @@ use crate::{
/// A table to keep track of which rules fixes should have /// A table to keep track of which rules fixes should have
/// their safety overridden. /// their safety overridden.
#[derive(Debug, CacheKey, Default)] #[derive(Debug, Clone, CacheKey, Default)]
pub struct FixSafetyTable { pub struct FixSafetyTable {
forced_safe: RuleSet, forced_safe: RuleSet,
forced_unsafe: RuleSet, forced_unsafe: RuleSet,

View file

@ -206,7 +206,7 @@ macro_rules! display_settings {
}; };
} }
#[derive(Debug, CacheKey)] #[derive(Debug, Clone, CacheKey)]
pub struct LinterSettings { pub struct LinterSettings {
pub exclude: FilePatternSet, pub exclude: FilePatternSet,
pub extension: ExtensionMapping, pub extension: ExtensionMapping,

View file

@ -6,7 +6,7 @@ use ruff_macros::CacheKey;
use crate::registry::{Rule, RuleSet, RuleSetIterator}; use crate::registry::{Rule, RuleSet, RuleSetIterator};
/// A table to keep track of which rules are enabled and whether they should be fixed. /// A table to keep track of which rules are enabled and whether they should be fixed.
#[derive(Debug, CacheKey, Default)] #[derive(Debug, Clone, CacheKey, Default)]
pub struct RuleTable { pub struct RuleTable {
/// Maps rule codes to a boolean indicating if the rule should be fixed. /// Maps rule codes to a boolean indicating if the rule should be fixed.
enabled: RuleSet, enabled: RuleSet,

View file

@ -590,7 +590,7 @@ impl Display for RequiredVersion {
/// pattern matching. /// pattern matching.
pub type IdentifierPattern = glob::Pattern; pub type IdentifierPattern = glob::Pattern;
#[derive(Debug, CacheKey, Default)] #[derive(Debug, Clone, CacheKey, Default)]
pub struct PerFileIgnores { pub struct PerFileIgnores {
// Ordered as (absolute path matcher, basename matcher, rules) // Ordered as (absolute path matcher, basename matcher, rules)
ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>, ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>,

View file

@ -271,7 +271,7 @@ impl SupportedCodeAction {
[ [
Self::QuickFix, Self::QuickFix,
Self::SourceFixAll, Self::SourceFixAll,
// Self::SourceOrganizeImports, Self::SourceOrganizeImports,
] ]
.into_iter() .into_iter()
} }

View file

@ -9,7 +9,7 @@ use lsp_types::{self as types, request as req};
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
use types::{CodeActionKind, CodeActionOrCommand}; use types::{CodeActionKind, CodeActionOrCommand};
use super::code_action_resolve::resolve_edit_for_fix_all; use super::code_action_resolve::{resolve_edit_for_fix_all, resolve_edit_for_organize_imports};
pub(crate) struct CodeActions; pub(crate) struct CodeActions;
@ -26,11 +26,11 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
) -> Result<Option<types::CodeActionResponse>> { ) -> Result<Option<types::CodeActionResponse>> {
let mut response: types::CodeActionResponse = types::CodeActionResponse::default(); let mut response: types::CodeActionResponse = types::CodeActionResponse::default();
let supported_code_actions = supported_code_actions(params.context.only); let supported_code_actions = supported_code_actions(params.context.only.clone());
if supported_code_actions.contains(&SupportedCodeAction::QuickFix) { if supported_code_actions.contains(&SupportedCodeAction::QuickFix) {
response.extend( response.extend(
quick_fix(&snapshot, params.context.diagnostics) quick_fix(&snapshot, params.context.diagnostics.clone())
.with_failure_code(ErrorCode::InternalError)?, .with_failure_code(ErrorCode::InternalError)?,
); );
} }
@ -40,7 +40,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
} }
if supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) { if supported_code_actions.contains(&SupportedCodeAction::SourceOrganizeImports) {
todo!("Implement the `source.organizeImports` code action"); response.push(organize_imports(&snapshot).with_failure_code(ErrorCode::InternalError)?);
} }
Ok(Some(response)) Ok(Some(response))
@ -109,6 +109,39 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
Ok(types::CodeActionOrCommand::CodeAction(action)) Ok(types::CodeActionOrCommand::CodeAction(action))
} }
fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
let document = snapshot.document();
let (edit, data) = if snapshot
.resolved_client_capabilities()
.code_action_deferred_edit_resolution
{
// The edit will be resolved later in the `CodeActionsResolve` request
(
None,
Some(serde_json::to_value(snapshot.url()).expect("document url to serialize")),
)
} else {
(
Some(resolve_edit_for_organize_imports(
document,
snapshot.url(),
&snapshot.configuration().linter,
snapshot.encoding(),
)?),
None,
)
};
let action = types::CodeAction {
title: format!("{DIAGNOSTIC_NAME}: Organize imports"),
kind: Some(types::CodeActionKind::SOURCE_ORGANIZE_IMPORTS),
edit,
data,
..Default::default()
};
Ok(types::CodeActionOrCommand::CodeAction(action))
}
/// If `action_filter` is `None`, this returns [`SupportedCodeActionKind::all()`]. Otherwise, /// If `action_filter` is `None`, this returns [`SupportedCodeActionKind::all()`]. Otherwise,
/// the list is filtered. /// the list is filtered.
fn supported_code_actions( fn supported_code_actions(

View file

@ -7,6 +7,7 @@ use crate::session::DocumentSnapshot;
use crate::PositionEncoding; use crate::PositionEncoding;
use lsp_server::ErrorCode; use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req}; use lsp_types::{self as types, request as req};
use ruff_linter::codes::Rule;
use ruff_linter::settings::LinterSettings; use ruff_linter::settings::LinterSettings;
pub(crate) struct CodeActionResolve; pub(crate) struct CodeActionResolve;
@ -47,9 +48,15 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
) )
.with_failure_code(ErrorCode::InternalError)?, .with_failure_code(ErrorCode::InternalError)?,
), ),
SupportedCodeAction::SourceOrganizeImports => { SupportedCodeAction::SourceOrganizeImports => Some(
todo!("Support `source.organizeImports`") resolve_edit_for_organize_imports(
} document,
snapshot.url(),
&snapshot.configuration().linter,
snapshot.encoding(),
)
.with_failure_code(ErrorCode::InternalError)?,
),
SupportedCodeAction::QuickFix => { SupportedCodeAction::QuickFix => {
return Err(anyhow::anyhow!( return Err(anyhow::anyhow!(
"Got a code action that should not need additional resolution: {action_kind:?}" "Got a code action that should not need additional resolution: {action_kind:?}"
@ -80,3 +87,30 @@ pub(super) fn resolve_edit_for_fix_all(
..Default::default() ..Default::default()
}) })
} }
pub(super) fn resolve_edit_for_organize_imports(
document: &crate::edit::Document,
url: &types::Url,
linter_settings: &ruff_linter::settings::LinterSettings,
encoding: PositionEncoding,
) -> crate::Result<types::WorkspaceEdit> {
let mut linter_settings = linter_settings.clone();
linter_settings.rules = [
Rule::UnsortedImports, // I001
Rule::MissingRequiredImport, // I002
]
.into_iter()
.collect();
Ok(types::WorkspaceEdit {
changes: Some(
[(
url.clone(),
crate::fix::fix_all(document, &linter_settings, encoding)?,
)]
.into_iter()
.collect(),
),
..Default::default()
})
}