Skip to content

Commit

Permalink
ruff server now supports the source.organizeImports source action (
Browse files Browse the repository at this point in the history
…#10652)

## Summary

This builds on top of the work in
#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


https://github.com/astral-sh/ruff/assets/19577865/04282d01-dfda-4ac5-aa8f-6a92d5f85bfd
  • Loading branch information
snowsignal committed Apr 4, 2024
1 parent fd8da66 commit d050d6d
Show file tree
Hide file tree
Showing 33 changed files with 106 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
#[allow(clippy::struct_excessive_bools)]
pub struct Settings {
pub mypy_init_return: bool,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_bandit/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub fn default_tmp_dirs() -> Vec<String> {
.to_vec()
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub hardcoded_tmp_directory: Vec<String>,
pub check_typed_exception: bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_macros::CacheKey;

use crate::display_settings;

#[derive(Debug, CacheKey, Default)]
#[derive(Debug, Clone, CacheKey, Default)]
pub struct Settings {
pub extend_allowed_calls: Vec<String>,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_bugbear/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub extend_immutable_calls: Vec<String>,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_builtins/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

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

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub allow_dict_calls_with_keyword_arguments: bool,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_copyright/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::fmt::{Display, Formatter};
use crate::display_settings;
use ruff_macros::CacheKey;

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub notice_rgx: Regex,
pub author: Option<String>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_errmsg/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub max_string_length: usize,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_gettext/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub functions_names: Vec<String>,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub allow_multiline: bool,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl FromIterator<String> for BannedAliases {
}
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub aliases: FxHashMap<String, String>,
pub banned_aliases: FxHashMap<String, BannedAliases>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn default_broad_exceptions() -> Vec<IdentifierPattern> {
.to_vec()
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub fixture_parentheses: bool,
pub parametrize_names_type: types::ParametrizeNameType,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_quotes/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl From<ruff_python_ast::str::Quote> for Quote {
}
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub inline_quotes: Quote,
pub multiline_quotes: Quote,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_self/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub const IGNORE_NAMES: [&str; 7] = [
"_value_",
];

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub ignore_names: Vec<String>,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl Display for Strictness {
}
}

#[derive(Debug, CacheKey, Default)]
#[derive(Debug, Clone, CacheKey, Default)]
pub struct Settings {
pub ban_relative_imports: Strictness,
pub banned_api: FxHashMap<String, ApiBan>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

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

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub ignore_variadic_names: bool,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/isort/categorize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ pub(crate) fn categorize_imports<'a>(
block_by_type
}

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct KnownModules {
/// A map of known modules to their section.
known: Vec<(glob::Pattern, ImportSection)>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/isort/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl Display for RelativeImportsOrder {
}
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
#[allow(clippy::struct_excessive_bools)]
pub struct Settings {
pub required_imports: BTreeSet<String>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/mccabe/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub max_complexity: usize,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pep8_naming/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use ruff_macros::CacheKey;

use crate::display_settings;

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub ignore_names: IgnoreNames,
pub classmethod_decorators: Vec<String>,
Expand Down Expand Up @@ -85,7 +85,7 @@ static DEFAULTS: &[&str] = &[
"maxDiff",
];

#[derive(Debug)]
#[derive(Debug, Clone)]
pub enum IgnoreNames {
Default,
UserProvided {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pycodestyle/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::fmt;

use crate::line_width::LineLength;

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub max_line_length: LineLength,
pub max_doc_length: Option<LineLength>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pydocstyle/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl fmt::Display for Convention {
}
}

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub convention: Option<Convention>,
pub ignore_decorators: BTreeSet<String>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyflakes/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt;

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub extend_generics: Vec<String>,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pylint/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl fmt::Display for ConstantType {
}
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct Settings {
pub allow_magic_value_types: Vec<ConstantType>,
pub allow_dunder_method_names: FxHashSet<String>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyupgrade/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt;

#[derive(Debug, Default, CacheKey)]
#[derive(Debug, Clone, Default, CacheKey)]
pub struct Settings {
pub keep_runtime_typing: bool,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/settings/fix_safety_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{

/// A table to keep track of which rules fixes should have
/// their safety overridden.
#[derive(Debug, CacheKey, Default)]
#[derive(Debug, Clone, CacheKey, Default)]
pub struct FixSafetyTable {
forced_safe: RuleSet,
forced_unsafe: RuleSet,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ macro_rules! display_settings {
};
}

#[derive(Debug, CacheKey)]
#[derive(Debug, Clone, CacheKey)]
pub struct LinterSettings {
pub exclude: FilePatternSet,
pub extension: ExtensionMapping,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/settings/rule_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_macros::CacheKey;
use crate::registry::{Rule, RuleSet, RuleSetIterator};

/// 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 {
/// Maps rule codes to a boolean indicating if the rule should be fixed.
enabled: RuleSet,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/settings/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ impl Display for RequiredVersion {
/// pattern matching.
pub type IdentifierPattern = glob::Pattern;

#[derive(Debug, CacheKey, Default)]
#[derive(Debug, Clone, CacheKey, Default)]
pub struct PerFileIgnores {
// Ordered as (absolute path matcher, basename matcher, rules)
ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl SupportedCodeAction {
[
Self::QuickFix,
Self::SourceFixAll,
// Self::SourceOrganizeImports,
Self::SourceOrganizeImports,
]
.into_iter()
}
Expand Down
41 changes: 37 additions & 4 deletions crates/ruff_server/src/server/api/requests/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use lsp_types::{self as types, request as req};
use rustc_hash::FxHashSet;
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;

Expand All @@ -26,11 +26,11 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
) -> Result<Option<types::CodeActionResponse>> {
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) {
response.extend(
quick_fix(&snapshot, params.context.diagnostics)
quick_fix(&snapshot, params.context.diagnostics.clone())
.with_failure_code(ErrorCode::InternalError)?,
);
}
Expand All @@ -40,7 +40,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
}

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))
Expand Down Expand Up @@ -109,6 +109,39 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
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,
/// the list is filtered.
fn supported_code_actions(
Expand Down

0 comments on commit d050d6d

Please sign in to comment.