Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_analyze): extend ActionCategory to allow expressing more co…
Browse files Browse the repository at this point in the history
…de action kinds (#3627)

* feat(rome_analyze): extent the ActionCategory enum to allow expressing more code action categories

* run codegen

* add documentation
  • Loading branch information
leops authored Nov 15, 2022
1 parent 0287c05 commit 33b5ce6
Show file tree
Hide file tree
Showing 12 changed files with 479 additions and 111 deletions.
135 changes: 133 additions & 2 deletions crates/rome_analyze/src/categories.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use bitflags::bitflags;

#[derive(Copy, Clone, Debug)]
Expand All @@ -18,16 +20,145 @@ pub enum RuleCategory {
Action,
}

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
/// The category of a code action, this type maps directly to the
/// [CodeActionKind] type in the Language Server Protocol specification
///
/// [CodeActionKind]: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionKind
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
pub enum ActionCategory {
/// Base kind for quickfix actions: 'quickfix'.
///
/// This action provides a fix to the diagnostic emitted by the same signal
QuickFix,
/// Base kind for refactoring actions: 'refactor'.
///
/// This action provides an optional refactor opportunity
Refactor,
Refactor(RefactorKind),
/// Base kind for source actions: `source`.
///
/// Source code actions apply to the entire file.
Source(SourceActionKind),
/// This action is using a base kind not covered by any of the previous
/// variants
Other(Cow<'static, str>),
}

impl ActionCategory {
/// Returns true if this category matches the provided filter
///
/// ## Examples
///
/// ```
/// use rome_analyze::{ActionCategory, RefactorKind};
///
/// assert!(ActionCategory::QuickFix.matches("quickfix"));
/// assert!(!ActionCategory::QuickFix.matches("refactor"));
///
/// assert!(ActionCategory::Refactor(RefactorKind::None).matches("refactor"));
/// assert!(!ActionCategory::Refactor(RefactorKind::None).matches("refactor.extract"));
///
/// assert!(ActionCategory::Refactor(RefactorKind::Extract).matches("refactor"));
/// assert!(ActionCategory::Refactor(RefactorKind::Extract).matches("refactor.extract"));
/// ```
pub fn matches(&self, filter: &str) -> bool {
self.to_str().starts_with(filter)
}

/// Returns the representation of this [ActionCategory] as a `CodeActionKind` string
pub fn to_str(&self) -> Cow<'static, str> {
match self {
ActionCategory::QuickFix => Cow::Borrowed("quickfix.rome"),

ActionCategory::Refactor(RefactorKind::None) => Cow::Borrowed("refactor.rome"),
ActionCategory::Refactor(RefactorKind::Extract) => {
Cow::Borrowed("refactor.extract.rome")
}
ActionCategory::Refactor(RefactorKind::Inline) => Cow::Borrowed("refactor.inline.rome"),
ActionCategory::Refactor(RefactorKind::Rewrite) => {
Cow::Borrowed("refactor.rewrite.rome")
}
ActionCategory::Refactor(RefactorKind::Other(tag)) => {
Cow::Owned(format!("refactor.{tag}.rome"))
}

ActionCategory::Source(SourceActionKind::None) => Cow::Borrowed("source.rome"),
ActionCategory::Source(SourceActionKind::FixAll) => Cow::Borrowed("source.fixAll.rome"),
ActionCategory::Source(SourceActionKind::OrganizeImports) => {
Cow::Borrowed("source.organizeImports.rome")
}
ActionCategory::Source(SourceActionKind::Other(tag)) => {
Cow::Owned(format!("source.{tag}.rome"))
}

ActionCategory::Other(tag) => Cow::Owned(format!("{tag}.rome")),
}
}
}

/// The sub-category of a refactor code action
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
pub enum RefactorKind {
/// This action describes a refactor with no particular sub-category
None,
/// Base kind for refactoring extraction actions: 'refactor.extract'.
///
/// Example extract actions:
/// - Extract method
/// - Extract function
/// - Extract variable
/// - Extract interface from class
Extract,
/// Base kind for refactoring inline actions: 'refactor.inline'.
///
/// Example inline actions:
/// - Inline function
/// - Inline variable
/// - Inline constant
/// - ...
Inline,
/// Base kind for refactoring rewrite actions: 'refactor.rewrite'.
///
/// Example rewrite actions:
/// - Convert JavaScript function to class
/// - Add or remove parameter
/// - Encapsulate field
/// - Make method static
/// - Move method to base class
/// - ...
Rewrite,
/// This action is using a refactor kind not covered by any of the previous
/// variants
Other(Cow<'static, str>),
}

/// The sub-category of a source code action
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema)
)]
pub enum SourceActionKind {
/// This action describes a source action with no particular sub-category
None,
// Base kind for a 'fix all' source action: `source.fixAll`.
//
// 'Fix all' actions automatically fix errors that have a clear fix that
// do not require user input. They should not suppress errors or perform
// unsafe fixes such as generating new types or classes.
FixAll,
/// Base kind for an organize imports source action: `source.organizeImports`.
OrganizeImports,
/// This action is using a source action kind not covered by any of the
/// previous variants
Other(Cow<'static, str>),
}

bitflags! {
Expand Down
4 changes: 3 additions & 1 deletion crates/rome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ mod visitor;
// Re-exported for use in the `declare_group` macro
pub use rome_diagnostics::v2::category_concat;

pub use crate::categories::{ActionCategory, RuleCategories, RuleCategory};
pub use crate::categories::{
ActionCategory, RefactorKind, RuleCategories, RuleCategory, SourceActionKind,
};
pub use crate::matcher::{InspectMatcher, MatchQueryParams, QueryMatcher, RuleKey, SignalEntry};
pub use crate::options::{AnalyzerConfiguration, AnalyzerOptions, AnalyzerRules};
pub use crate::query::{Ast, QueryKey, QueryMatch, Queryable};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule};
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, RefactorKind, Rule};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
Expand Down Expand Up @@ -63,7 +63,7 @@ impl Rule for FlipBinExp {
mutation.replace_node(prev_right, new_right);

Some(JsRuleAction {
category: ActionCategory::Refactor,
category: ActionCategory::Refactor(RefactorKind::None),
applicability: Applicability::Always,
message: markup! { "Flip Binary Expression" }.to_owned(),
mutation,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule};
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, RefactorKind, Rule};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_semantic::{AllReferencesExtensions, Reference};
Expand Down Expand Up @@ -94,7 +94,7 @@ impl Rule for InlineVariable {
}

Some(JsRuleAction {
category: ActionCategory::Refactor,
category: ActionCategory::Refactor(RefactorKind::Inline),
applicability: Applicability::Always,
message: markup! { "Inline variable" }.to_owned(),
mutation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl Rule for UseCamelCase {
batch.rename_node_declaration_with_retry(model, binding.clone(), candidates);
if renamed {
Some(JsRuleAction {
category: ActionCategory::Refactor,
category: ActionCategory::QuickFix,
applicability: Applicability::Always,
message: markup! { "Rename this symbol to camel case" }.to_owned(),
mutation: batch,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl Rule for NoShoutyConstants {
}

Some(JsRuleAction {
category: ActionCategory::Refactor,
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Use the constant value directly" }.to_owned(),
mutation: batch,
Expand Down
45 changes: 28 additions & 17 deletions crates/rome_lsp/src/handlers/analysis.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::borrow::Cow;
use std::collections::HashMap;

use anyhow::{Context, Result};
use rome_analyze::ActionCategory;
use rome_analyze::{ActionCategory, SourceActionKind};
use rome_fs::RomePath;
use rome_service::workspace::{
FeatureName, FixFileMode, FixFileParams, PullActionsParams, SupportsFeatureParams,
Expand All @@ -15,7 +16,14 @@ use crate::line_index::LineIndex;
use crate::session::Session;
use crate::utils;

const FIX_ALL: CodeActionKind = CodeActionKind::new("source.fixAll");
const FIX_ALL_CATEGORY: ActionCategory = ActionCategory::Source(SourceActionKind::FixAll);

fn fix_all_kind() -> CodeActionKind {
match FIX_ALL_CATEGORY.to_str() {
Cow::Borrowed(kind) => CodeActionKind::from(kind),
Cow::Owned(kind) => CodeActionKind::from(kind),
}
}

/// Queries the [`AnalysisServer`] for code actions of the file matching [FileId]
///
Expand All @@ -39,17 +47,14 @@ pub(crate) fn code_actions(
let mut has_fix_all = false;
let mut filters = Vec::new();

if let Some(filter) = params.context.only {
if let Some(filter) = &params.context.only {
for kind in filter {
if kind == CodeActionKind::QUICKFIX {
filters.push(ActionCategory::QuickFix);
} else if kind == CodeActionKind::REFACTOR {
filters.push(ActionCategory::Refactor);
} else if kind == FIX_ALL {
let kind = kind.as_str();
if FIX_ALL_CATEGORY.matches(kind) {
has_fix_all = true;
} else {
tracing::warn!("unknown code action kind {kind:?} requested");
}

filters.push(kind);
}
}

Expand Down Expand Up @@ -86,7 +91,8 @@ pub(crate) fn code_actions(
.filter_map(|action| {
// Remove actions that do not match the categories requested by the
// language client
if !filters.is_empty() && !filters.contains(&action.category) {
let matches_filters = filters.iter().any(|filter| action.category.matches(filter));
if !filters.is_empty() && !matches_filters {
return None;
}

Expand All @@ -103,7 +109,7 @@ pub(crate) fn code_actions(
if has_fixes {
actions.retain(|action| {
if let CodeActionOrCommand::CodeAction(action) = action {
action.kind == Some(FIX_ALL) || action.diagnostics.is_some()
action.kind.as_ref() == Some(&fix_all_kind()) || action.diagnostics.is_some()
} else {
true
}
Expand Down Expand Up @@ -134,13 +140,18 @@ fn fix_all(
let diagnostics = diagnostics
.iter()
.filter_map(|d| {
let code = d.code.as_ref().and_then(|code| match code {
lsp::NumberOrString::String(code) => Some(code.as_str()),
lsp::NumberOrString::Number(_) => None,
})?;
let code = d.code.as_ref()?;
let code = match code {
lsp::NumberOrString::String(code) => code.as_str(),
lsp::NumberOrString::Number(_) => return None,
};

let code = code.strip_prefix("lint/")?;

let diag_range = utils::text_range(line_index, d.range).ok()?;
let has_matching_rule = fixed.actions.iter().any(|action| {
let Some(code) = code.strip_prefix(action.group_name.as_ref()) else { return false };
let Some(code) = code.strip_prefix('/') else { return false };
code == action.rule_name && action.range.intersect(diag_range).is_some()
});

Expand Down Expand Up @@ -175,7 +186,7 @@ fn fix_all(

Ok(Some(CodeActionOrCommand::CodeAction(lsp::CodeAction {
title: String::from("Fix all auto-fixable issues"),
kind: Some(FIX_ALL),
kind: Some(fix_all_kind()),
diagnostics: Some(diagnostics),
edit: Some(edit),
command: None,
Expand Down
26 changes: 17 additions & 9 deletions crates/rome_lsp/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,15 @@ pub(crate) fn code_fix_to_lsp(
diagnostics
.iter()
.filter_map(|d| {
let code = d.code.as_ref().and_then(|code| match code {
lsp::NumberOrString::String(code) => Some(code.as_str()),
lsp::NumberOrString::Number(_) => None,
})?;
let code = d.code.as_ref()?;
let code = match code {
lsp::NumberOrString::String(code) => code.as_str(),
lsp::NumberOrString::Number(_) => return None,
};

let code = code.strip_prefix("lint/")?;
let code = code.strip_prefix(action.group_name.as_ref())?;
let code = code.strip_prefix('/')?;

if code == action.rule_name {
Some(d.clone())
Expand All @@ -132,10 +137,13 @@ pub(crate) fn code_fix_to_lsp(
Vec::new()
};

let kind = match action.category {
ActionCategory::QuickFix => Some(lsp::CodeActionKind::QUICKFIX),
ActionCategory::Refactor => Some(lsp::CodeActionKind::REFACTOR),
};
let kind = action.category.to_str();
let mut kind = kind.into_owned();

kind.push('.');
kind.push_str(action.group_name.as_ref());
kind.push('.');
kind.push_str(action.rule_name.as_ref());

let suggestion = action.suggestion;

Expand All @@ -152,7 +160,7 @@ pub(crate) fn code_fix_to_lsp(

lsp::CodeAction {
title: print_markup(&suggestion.msg),
kind,
kind: Some(lsp::CodeActionKind::from(kind)),
diagnostics: if !diagnostics.is_empty() {
Some(diagnostics)
} else {
Expand Down
Loading

0 comments on commit 33b5ce6

Please sign in to comment.