Apply the api design suggestions

This commit is contained in:
Kirill Bulatov 2020-01-12 00:40:36 +02:00
parent 73dc8b6f06
commit 78a21253b4
10 changed files with 97 additions and 59 deletions

1
Cargo.lock generated
View file

@ -1067,6 +1067,7 @@ version = "0.1.0"
dependencies = [
"crossbeam-channel 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
"itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
"jod-thread 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
"lsp-server 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",

View file

@ -1,5 +1,6 @@
//! This module defines `AssistCtx` -- the API surface that is exposed to assists.
use hir::{db::HirDatabase, InFile, SourceAnalyzer};
use itertools::Either;
use ra_db::FileRange;
use ra_fmt::{leading_indent, reindent};
use ra_syntax::{
@ -9,12 +10,12 @@ use ra_syntax::{
};
use ra_text_edit::TextEditBuilder;
use crate::{AssistAction, AssistId, AssistLabel};
use crate::{AssistAction, AssistId, AssistLabel, ResolvedAssist};
#[derive(Clone, Debug)]
pub(crate) enum Assist {
Unresolved { label: AssistLabel },
Resolved { label: AssistLabel, action: AssistAction, alternative_actions: Vec<AssistAction> },
Resolved { assist: ResolvedAssist },
}
/// `AssistCtx` allows to apply an assist or check if it could be applied.
@ -92,7 +93,7 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
f(&mut edit);
edit.build()
};
Assist::Resolved { label, action, alternative_actions: Vec::default() }
Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } }
} else {
Assist::Unresolved { label }
};
@ -105,18 +106,20 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
self,
id: AssistId,
label: impl Into<String>,
f: impl FnOnce() -> (ActionBuilder, Vec<ActionBuilder>),
f: impl FnOnce() -> Vec<ActionBuilder>,
) -> Option<Assist> {
let label = AssistLabel { label: label.into(), id };
let assist = if self.should_compute_edit {
let (action, alternative_actions) = f();
let actions = f();
assert!(!actions.is_empty(), "Assist cannot have no");
Assist::Resolved {
label,
action: action.build(),
alternative_actions: alternative_actions
.into_iter()
.map(ActionBuilder::build)
.collect(),
assist: ResolvedAssist {
label,
action_data: Either::Right(
actions.into_iter().map(ActionBuilder::build).collect(),
),
},
}
} else {
Assist::Unresolved { label }

View file

@ -15,21 +15,21 @@ fn check(assist_id: &str, before: &str, after: &str) {
let (db, file_id) = TestDB::with_single_file(&before);
let frange = FileRange { file_id, range: selection.into() };
let (_assist_id, action, _) = crate::assists(&db, frange)
let assist = crate::assists(&db, frange)
.into_iter()
.find(|(id, _, _)| id.id.0 == assist_id)
.find(|assist| assist.label.id.0 == assist_id)
.unwrap_or_else(|| {
panic!(
"\n\nAssist is not applicable: {}\nAvailable assists: {}",
assist_id,
crate::assists(&db, frange)
.into_iter()
.map(|(id, _, _)| id.id.0)
.map(|assist| assist.label.id.0)
.collect::<Vec<_>>()
.join(", ")
)
});
let actual = action.edit.apply(&before);
let actual = assist.get_first_action().edit.apply(&before);
assert_eq_text!(after, &actual);
}

View file

@ -14,6 +14,7 @@ mod test_db;
pub mod ast_transform;
use hir::db::HirDatabase;
use itertools::Either;
use ra_db::FileRange;
use ra_syntax::{TextRange, TextUnit};
use ra_text_edit::TextEdit;
@ -41,6 +42,21 @@ pub struct AssistAction {
pub target: Option<TextRange>,
}
#[derive(Debug, Clone)]
pub struct ResolvedAssist {
pub label: AssistLabel,
pub action_data: Either<AssistAction, Vec<AssistAction>>,
}
impl ResolvedAssist {
pub fn get_first_action(&self) -> AssistAction {
match &self.action_data {
Either::Left(action) => action.clone(),
Either::Right(actions) => actions[0].clone(),
}
}
}
/// Return all the assists applicable at the given position.
///
/// Assists are returned in the "unresolved" state, that is only labels are
@ -65,7 +81,7 @@ where
///
/// Assists are returned in the "resolved" state, that is with edit fully
/// computed.
pub fn assists<H>(db: &H, range: FileRange) -> Vec<(AssistLabel, AssistAction, Vec<AssistAction>)>
pub fn assists<H>(db: &H, range: FileRange) -> Vec<ResolvedAssist>
where
H: HirDatabase + 'static,
{
@ -76,13 +92,11 @@ where
.iter()
.filter_map(|f| f(ctx.clone()))
.map(|a| match a {
Assist::Resolved { label, action, alternative_actions } => {
(label, action, alternative_actions)
}
Assist::Resolved { assist } => assist,
Assist::Unresolved { .. } => unreachable!(),
})
.collect::<Vec<_>>();
a.sort_by(|a, b| match (a.1.target, b.1.target) {
a.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) {
(Some(a), Some(b)) => a.len().cmp(&b.len()),
(Some(_), None) => Ordering::Less,
(None, Some(_)) => Ordering::Greater,
@ -177,7 +191,7 @@ mod helpers {
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
let action = match assist {
Assist::Unresolved { .. } => unreachable!(),
Assist::Resolved { action, .. } => action,
Assist::Resolved { assist } => assist.get_first_action(),
};
let actual = action.edit.apply(&before);
@ -204,7 +218,7 @@ mod helpers {
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
let action = match assist {
Assist::Unresolved { .. } => unreachable!(),
Assist::Resolved { action, .. } => action,
Assist::Resolved { assist } => assist.get_first_action(),
};
let mut actual = action.edit.apply(&before);
@ -227,7 +241,7 @@ mod helpers {
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
let action = match assist {
Assist::Unresolved { .. } => unreachable!(),
Assist::Resolved { action, .. } => action,
Assist::Resolved { assist } => assist.get_first_action(),
};
let range = action.target.expect("expected target on action");
@ -246,7 +260,7 @@ mod helpers {
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
let action = match assist {
Assist::Unresolved { .. } => unreachable!(),
Assist::Resolved { action, .. } => action,
Assist::Resolved { assist } => assist.get_first_action(),
};
let range = action.target.expect("expected target on action");
@ -296,10 +310,10 @@ mod tests {
let mut assists = assists.iter();
assert_eq!(
assists.next().expect("expected assist").0.label,
assists.next().expect("expected assist").label.label,
"Change visibility to pub(crate)"
);
assert_eq!(assists.next().expect("expected assist").0.label, "Add `#[derive]`");
assert_eq!(assists.next().expect("expected assist").label.label, "Add `#[derive]`");
}
#[test]
@ -318,7 +332,7 @@ mod tests {
let assists = super::assists(&db, frange);
let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").0.label, "Extract into variable");
assert_eq!(assists.next().expect("expected assist").0.label, "Replace with match");
assert_eq!(assists.next().expect("expected assist").label.label, "Extract into variable");
assert_eq!(assists.next().expect("expected assist").label.label, "Replace with match");
}
}

View file

@ -4,30 +4,37 @@ use ra_db::{FilePosition, FileRange};
use crate::{db::RootDatabase, FileId, SourceChange, SourceFileEdit};
use itertools::Either;
pub use ra_assists::AssistId;
use ra_assists::{AssistAction, AssistLabel};
#[derive(Debug)]
pub struct Assist {
pub id: AssistId,
pub change: SourceChange,
pub label: String,
pub alternative_changes: Vec<SourceChange>,
pub change_data: Either<SourceChange, Vec<SourceChange>>,
}
pub(crate) fn assists(db: &RootDatabase, frange: FileRange) -> Vec<Assist> {
ra_assists::assists(db, frange)
.into_iter()
.map(|(assist_label, action, alternative_actions)| {
.map(|assist| {
let file_id = frange.file_id;
let assist_label = &assist.label;
Assist {
id: assist_label.id,
label: assist_label.label.clone(),
change: action_to_edit(action, file_id, &assist_label),
alternative_changes: alternative_actions
.into_iter()
.map(|action| action_to_edit(action, file_id, &assist_label))
.collect(),
change_data: match assist.action_data {
Either::Left(action) => {
Either::Left(action_to_edit(action, file_id, assist_label))
}
Either::Right(actions) => Either::Right(
actions
.into_iter()
.map(|action| action_to_edit(action, file_id, assist_label))
.collect(),
),
},
}
})
.collect()

View file

@ -28,6 +28,7 @@ ra_prof = { path = "../ra_prof" }
ra_vfs_glob = { path = "../ra_vfs_glob" }
env_logger = { version = "0.7.1", default-features = false, features = ["humantime"] }
ra_cargo_watch = { path = "../ra_cargo_watch" }
itertools = "0.8"
[dev-dependencies]
tempfile = "3"

View file

@ -3,6 +3,7 @@
use std::{fmt::Write as _, io::Write as _};
use itertools::Either;
use lsp_server::ErrorCode;
use lsp_types::{
CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem,
@ -698,18 +699,25 @@ pub fn handle_code_action(
for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() {
let title = assist.label.clone();
let edit = assist.change.try_conv_with(&world)?;
let alternative_edits = assist
.alternative_changes
.into_iter()
.map(|change| change.try_conv_with(&world))
.collect::<Result<Vec<_>>>()?;
let command = Command {
title,
command: "rust-analyzer.applySourceChange".to_string(),
arguments: Some(vec![to_value(edit).unwrap(), to_value(alternative_edits).unwrap()]),
let command = match assist.change_data {
Either::Left(change) => Command {
title,
command: "rust-analyzer.applySourceChange".to_string(),
arguments: Some(vec![to_value(change.try_conv_with(&world)?)?]),
},
Either::Right(changes) => Command {
title,
command: "rust-analyzer.selectAndApplySourceChange".to_string(),
arguments: Some(vec![to_value(
changes
.into_iter()
.map(|change| change.try_conv_with(&world))
.collect::<Result<Vec<_>>>()?,
)?]),
},
};
let action = CodeAction {
title: command.title.clone(),
kind: match assist.id {

View file

@ -34,8 +34,20 @@ function showReferences(ctx: Ctx): Cmd {
}
function applySourceChange(ctx: Ctx): Cmd {
return async (change: sourceChange.SourceChange, alternativeChanges: sourceChange.SourceChange[] | undefined) => {
sourceChange.applySourceChange(ctx, change, alternativeChanges);
return async (change: sourceChange.SourceChange) => {
sourceChange.applySourceChange(ctx, change);
};
}
function selectAndApplySourceChange(ctx: Ctx): Cmd {
return async (changes: sourceChange.SourceChange[]) => {
if (changes.length === 1) {
await sourceChange.applySourceChange(ctx, changes[0]);
} else if (changes.length > 0) {
const selectedChange = await vscode.window.showQuickPick(changes);
if (!selectedChange) return;
await sourceChange.applySourceChange(ctx, selectedChange);
}
};
}
@ -59,5 +71,6 @@ export {
runSingle,
showReferences,
applySourceChange,
selectAndApplySourceChange,
reload
};

View file

@ -26,6 +26,7 @@ export async function activate(context: vscode.ExtensionContext) {
ctx.registerCommand('runSingle', commands.runSingle);
ctx.registerCommand('showReferences', commands.showReferences);
ctx.registerCommand('applySourceChange', commands.applySourceChange);
ctx.registerCommand('selectAndApplySourceChange', commands.selectAndApplySourceChange);
if (ctx.config.enableEnhancedTyping) {
ctx.overrideCommand('type', commands.onEnter);

View file

@ -9,7 +9,7 @@ export interface SourceChange {
cursorPosition?: lc.TextDocumentPositionParams;
}
async function applySelectedSourceChange(ctx: Ctx, change: SourceChange) {
export async function applySourceChange(ctx: Ctx, change: SourceChange) {
const client = ctx.client;
if (!client) return;
@ -55,13 +55,3 @@ async function applySelectedSourceChange(ctx: Ctx, change: SourceChange) {
);
}
}
export async function applySourceChange(ctx: Ctx, change: SourceChange, alternativeChanges: SourceChange[] | undefined) {
if (alternativeChanges !== undefined && alternativeChanges.length > 0) {
const selectedChange = await vscode.window.showQuickPick([change, ...alternativeChanges]);
if (!selectedChange) return;
await applySelectedSourceChange(ctx, selectedChange);
} else {
await applySelectedSourceChange(ctx, change);
}
}