Skip to content

Commit d93f75e

Browse files
SomeoneToIgnoreSomeoneToIgnore
committed
Apply the api design suggestions
1 parent 324e817 commit d93f75e

File tree

10 files changed

+95
-59
lines changed

10 files changed

+95
-59
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/ra_assists/src/assist_ctx.rs‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! This module defines `AssistCtx` -- the API surface that is exposed to assists.
22
use hir::{db::HirDatabase, InFile, SourceAnalyzer};
3+
use itertools::Either;
34
use ra_db::FileRange;
45
use ra_fmt::{leading_indent, reindent};
56
use ra_syntax::{
@@ -9,12 +10,12 @@ use ra_syntax::{
910
};
1011
use ra_text_edit::TextEditBuilder;
1112

12-
use crate::{AssistAction, AssistId, AssistLabel};
13+
use crate::{AssistAction, AssistId, AssistLabel, ResolvedAssist};
1314

1415
#[derive(Clone, Debug)]
1516
pub(crate) enum Assist {
1617
Unresolved { label: AssistLabel },
17-
Resolved { label: AssistLabel, action: AssistAction, alternative_actions: Vec<AssistAction> },
18+
Resolved { assist: ResolvedAssist },
1819
}
1920

2021
/// `AssistCtx` allows to apply an assist or check if it could be applied.
@@ -90,7 +91,7 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
9091
f(&mut edit);
9192
edit.build()
9293
};
93-
Assist::Resolved { label, action, alternative_actions: Vec::default() }
94+
Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } }
9495
} else {
9596
Assist::Unresolved { label }
9697
};
@@ -103,18 +104,20 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
103104
self,
104105
id: AssistId,
105106
label: impl Into<String>,
106-
f: impl FnOnce() -> (ActionBuilder, Vec<ActionBuilder>),
107+
f: impl FnOnce() -> Vec<ActionBuilder>,
107108
) -> Option<Assist> {
108109
let label = AssistLabel { label: label.into(), id };
109110
let assist = if self.should_compute_edit {
110-
let (action, alternative_actions) = f();
111+
let actions = f();
112+
assert!(!actions.is_empty(), "Assist cannot have no");
113+
111114
Assist::Resolved {
112-
label,
113-
action: action.build(),
114-
alternative_actions: alternative_actions
115-
.into_iter()
116-
.map(ActionBuilder::build)
117-
.collect(),
115+
assist: ResolvedAssist {
116+
label,
117+
action_data: Either::Right(
118+
actions.into_iter().map(ActionBuilder::build).collect(),
119+
),
120+
},
118121
}
119122
} else {
120123
Assist::Unresolved { label }

‎crates/ra_assists/src/doc_tests.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,21 @@ fn check(assist_id: &str, before: &str, after: &str) {
1515
let (db, file_id) = TestDB::with_single_file(&before);
1616
let frange = FileRange { file_id, range: selection.into() };
1717

18-
let (_assist_id, action, _) = crate::assists(&db, frange)
18+
let assist = crate::assists(&db, frange)
1919
.into_iter()
20-
.find(|(id, _, _)| id.id.0 == assist_id)
20+
.find(|assist| assist.label.id.0 == assist_id)
2121
.unwrap_or_else(|| {
2222
panic!(
2323
"\n\nAssist is not applicable: {}\nAvailable assists: {}",
2424
assist_id,
2525
crate::assists(&db, frange)
2626
.into_iter()
27-
.map(|(id, _, _)| id.id.0)
27+
.map(|assist| assist.label.id.0)
2828
.collect::<Vec<_>>()
2929
.join(", ")
3030
)
3131
});
3232

33-
let actual = action.edit.apply(&before);
33+
let actual = assist.get_first_action().edit.apply(&before);
3434
assert_eq_text!(after, &actual);
3535
}

‎crates/ra_assists/src/lib.rs‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod test_db;
1414
pub mod ast_transform;
1515

1616
use hir::db::HirDatabase;
17+
use itertools::Either;
1718
use ra_db::FileRange;
1819
use ra_syntax::{TextRange, TextUnit};
1920
use ra_text_edit::TextEdit;
@@ -41,6 +42,21 @@ pub struct AssistAction {
4142
pub target: Option<TextRange>,
4243
}
4344

45+
#[derive(Debug, Clone)]
46+
pub struct ResolvedAssist {
47+
pub label: AssistLabel,
48+
pub action_data: Either<AssistAction, Vec<AssistAction>>,
49+
}
50+
51+
impl ResolvedAssist {
52+
pub fn get_first_action(&self) -> AssistAction {
53+
match &self.action_data {
54+
Either::Left(action) => action.clone(),
55+
Either::Right(actions) => actions[0].clone(),
56+
}
57+
}
58+
}
59+
4460
/// Return all the assists applicable at the given position.
4561
///
4662
/// Assists are returned in the "unresolved" state, that is only labels are
@@ -65,7 +81,7 @@ where
6581
///
6682
/// Assists are returned in the "resolved" state, that is with edit fully
6783
/// computed.
68-
pub fn assists<H>(db: &H, range: FileRange) -> Vec<(AssistLabel, AssistAction, Vec<AssistAction>)>
84+
pub fn assists<H>(db: &H, range: FileRange) -> Vec<ResolvedAssist>
6985
where
7086
H: HirDatabase + 'static,
7187
{
@@ -76,13 +92,11 @@ where
7692
.iter()
7793
.filter_map(|f| f(ctx.clone()))
7894
.map(|a| match a {
79-
Assist::Resolved { label, action, alternative_actions } => {
80-
(label, action, alternative_actions)
81-
}
95+
Assist::Resolved { assist } => assist,
8296
Assist::Unresolved { .. } => unreachable!(),
8397
})
8498
.collect::<Vec<_>>();
85-
a.sort_by(|a, b| match (a.1.target, b.1.target) {
99+
a.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) {
86100
(Some(a), Some(b)) => a.len().cmp(&b.len()),
87101
(Some(_), None) => Ordering::Less,
88102
(None, Some(_)) => Ordering::Greater,
@@ -177,7 +191,7 @@ mod helpers {
177191
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
178192
let action = match assist {
179193
Assist::Unresolved { .. } => unreachable!(),
180-
Assist::Resolved { action, .. } => action,
194+
Assist::Resolved { assist } => assist.get_first_action(),
181195
};
182196

183197
let actual = action.edit.apply(&before);
@@ -204,7 +218,7 @@ mod helpers {
204218
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
205219
let action = match assist {
206220
Assist::Unresolved { .. } => unreachable!(),
207-
Assist::Resolved { action, .. } => action,
221+
Assist::Resolved { assist } => assist.get_first_action(),
208222
};
209223

210224
let mut actual = action.edit.apply(&before);
@@ -227,7 +241,7 @@ mod helpers {
227241
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
228242
let action = match assist {
229243
Assist::Unresolved { .. } => unreachable!(),
230-
Assist::Resolved { action, .. } => action,
244+
Assist::Resolved { assist } => assist.get_first_action(),
231245
};
232246

233247
let range = action.target.expect("expected target on action");
@@ -246,7 +260,7 @@ mod helpers {
246260
AssistCtx::with_ctx(&db, frange, true, assist).expect("code action is not applicable");
247261
let action = match assist {
248262
Assist::Unresolved { .. } => unreachable!(),
249-
Assist::Resolved { action, .. } => action,
263+
Assist::Resolved { assist } => assist.get_first_action(),
250264
};
251265

252266
let range = action.target.expect("expected target on action");
@@ -295,8 +309,8 @@ mod tests {
295309
let assists = super::assists(&db, frange);
296310
let mut assists = assists.iter();
297311

298-
assert_eq!(assists.next().expect("expected assist").0.label, "make pub(crate)");
299-
assert_eq!(assists.next().expect("expected assist").0.label, "add `#[derive]`");
312+
assert_eq!(assists.next().expect("expected assist").label.label, "make pub(crate)");
313+
assert_eq!(assists.next().expect("expected assist").label.label, "add `#[derive]`");
300314
}
301315

302316
#[test]
@@ -315,7 +329,7 @@ mod tests {
315329
let assists = super::assists(&db, frange);
316330
let mut assists = assists.iter();
317331

318-
assert_eq!(assists.next().expect("expected assist").0.label, "introduce variable");
319-
assert_eq!(assists.next().expect("expected assist").0.label, "replace with match");
332+
assert_eq!(assists.next().expect("expected assist").label.label, "introduce variable");
333+
assert_eq!(assists.next().expect("expected assist").label.label, "replace with match");
320334
}
321335
}

‎crates/ra_ide/src/assists.rs‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,37 @@ use ra_db::{FilePosition, FileRange};
44

55
use crate::{db::RootDatabase, FileId, SourceChange, SourceFileEdit};
66

7+
use itertools::Either;
78
pub use ra_assists::AssistId;
89
use ra_assists::{AssistAction, AssistLabel};
910

1011
#[derive(Debug)]
1112
pub struct Assist {
1213
pub id: AssistId,
13-
pub change: SourceChange,
1414
pub label: String,
15-
pub alternative_changes: Vec<SourceChange>,
15+
pub change_data: Either<SourceChange, Vec<SourceChange>>,
1616
}
1717

1818
pub(crate) fn assists(db: &RootDatabase, frange: FileRange) -> Vec<Assist> {
1919
ra_assists::assists(db, frange)
2020
.into_iter()
21-
.map(|(assist_label, action, alternative_actions)| {
21+
.map(|assist| {
2222
let file_id = frange.file_id;
23+
let assist_label = &assist.label;
2324
Assist {
2425
id: assist_label.id,
2526
label: assist_label.label.clone(),
26-
change: action_to_edit(action, file_id, &assist_label),
27-
alternative_changes: alternative_actions
28-
.into_iter()
29-
.map(|action| action_to_edit(action, file_id, &assist_label))
30-
.collect(),
27+
change_data: match assist.action_data {
28+
Either::Left(action) => {
29+
Either::Left(action_to_edit(action, file_id, assist_label))
30+
}
31+
Either::Right(actions) => Either::Right(
32+
actions
33+
.into_iter()
34+
.map(|action| action_to_edit(action, file_id, assist_label))
35+
.collect(),
36+
),
37+
},
3138
}
3239
})
3340
.collect()

‎crates/ra_lsp_server/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ ra_prof = { path = "../ra_prof" }
2828
ra_vfs_glob = { path = "../ra_vfs_glob" }
2929
env_logger = { version = "0.7.1", default-features = false, features = ["humantime"] }
3030
ra_cargo_watch = { path = "../ra_cargo_watch" }
31+
itertools = "0.8"
3132

3233
[dev-dependencies]
3334
tempfile = "3"

‎crates/ra_lsp_server/src/main_loop/handlers.rs‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
44
use std::{fmt::Write as _, io::Write as _};
55

6+
use itertools::Either;
67
use lsp_server::ErrorCode;
78
use lsp_types::{
89
CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem,
@@ -698,18 +699,25 @@ pub fn handle_code_action(
698699

699700
for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() {
700701
let title = assist.label.clone();
701-
let edit = assist.change.try_conv_with(&world)?;
702-
let alternative_edits = assist
703-
.alternative_changes
704-
.into_iter()
705-
.map(|change| change.try_conv_with(&world))
706-
.collect::<Result<Vec<_>>>()?;
707702

708-
let command = Command {
709-
title,
710-
command: "rust-analyzer.applySourceChange".to_string(),
711-
arguments: Some(vec![to_value(edit).unwrap(), to_value(alternative_edits).unwrap()]),
703+
let command = match assist.change_data {
704+
Either::Left(change) => Command {
705+
title,
706+
command: "rust-analyzer.applySourceChange".to_string(),
707+
arguments: Some(vec![to_value(change.try_conv_with(&world)?)?]),
708+
},
709+
Either::Right(changes) => Command {
710+
title,
711+
command: "rust-analyzer.selectAndApplySourceChange".to_string(),
712+
arguments: Some(vec![to_value(
713+
changes
714+
.into_iter()
715+
.map(|change| change.try_conv_with(&world))
716+
.collect::<Result<Vec<_>>>()?,
717+
)?]),
718+
},
712719
};
720+
713721
let action = CodeAction {
714722
title: command.title.clone(),
715723
kind: match assist.id {

‎editors/code/src/commands/index.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,18 @@ function showReferences(ctx: Ctx): Cmd {
3434
}
3535

3636
function applySourceChange(ctx: Ctx): Cmd {
37-
return async (change: sourceChange.SourceChange, alternativeChanges: sourceChange.SourceChange[] | undefined) => {
38-
sourceChange.applySourceChange(ctx, change, alternativeChanges);
37+
return async (change: sourceChange.SourceChange) => {
38+
sourceChange.applySourceChange(ctx, change);
39+
};
40+
}
41+
42+
function selectAndApplySourceChange(ctx: Ctx): Cmd {
43+
return async (changes: sourceChange.SourceChange[]) => {
44+
if (changes.length > 0) {
45+
const selectedChange = await vscode.window.showQuickPick(changes);
46+
if (!selectedChange) return;
47+
await sourceChange.applySourceChange(ctx, selectedChange);
48+
}
3949
};
4050
}
4151

@@ -59,5 +69,6 @@ export {
5969
runSingle,
6070
showReferences,
6171
applySourceChange,
72+
selectAndApplySourceChange,
6273
reload
6374
};

‎editors/code/src/main.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export async function activate(context: vscode.ExtensionContext) {
2626
ctx.registerCommand('runSingle', commands.runSingle);
2727
ctx.registerCommand('showReferences', commands.showReferences);
2828
ctx.registerCommand('applySourceChange', commands.applySourceChange);
29+
ctx.registerCommand('selectAndApplySourceChange', commands.selectAndApplySourceChange);
2930

3031
if (ctx.config.enableEnhancedTyping) {
3132
ctx.overrideCommand('type', commands.onEnter);

‎editors/code/src/source_change.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export interface SourceChange {
99
cursorPosition?: lc.TextDocumentPositionParams;
1010
}
1111

12-
async function applySelectedSourceChange(ctx: Ctx, change: SourceChange) {
12+
export async function applySourceChange(ctx: Ctx, change: SourceChange) {
1313
const client = ctx.client;
1414
if (!client) return;
1515

@@ -55,13 +55,3 @@ async function applySelectedSourceChange(ctx: Ctx, change: SourceChange) {
5555
);
5656
}
5757
}
58-
59-
export async function applySourceChange(ctx: Ctx, change: SourceChange, alternativeChanges: SourceChange[] | undefined) {
60-
if (alternativeChanges !== undefined && alternativeChanges.length > 0) {
61-
const selectedChange = await vscode.window.showQuickPick([change, ...alternativeChanges]);
62-
if (!selectedChange) return;
63-
await applySelectedSourceChange(ctx, selectedChange);
64-
} else {
65-
await applySelectedSourceChange(ctx, change);
66-
}
67-
}

0 commit comments

Comments
 (0)