495: Fix on type handlers r=matklad a=matklad

Looks like our on type handlers didn't actually worked, this shoud fix that!

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2019-01-11 12:49:09 +00:00
commit 0f4bc7589c
4 changed files with 176 additions and 180 deletions

View File

@ -15,10 +15,11 @@ use ra_text_edit::{TextEdit, TextEditBuilder};
use ra_syntax::{
Direction, SyntaxNode, TextUnit, TextRange, SourceFile, AstNode,
algo::{find_leaf_at_offset, find_node_at_offset, find_covering_node, LeafAtOffset},
ast::{self, AstToken},
};
use itertools::Itertools;
use crate::formatting::leading_indent;
pub use self::{
flip_comma::flip_comma,
add_derive::add_derive,
@ -165,7 +166,7 @@ impl AssistBuilder {
}
fn replace_node_and_indent(&mut self, node: &SyntaxNode, replace_with: impl Into<String>) {
let mut replace_with = replace_with.into();
if let Some(indent) = calc_indent(node) {
if let Some(indent) = leading_indent(node) {
replace_with = reindent(&replace_with, indent)
}
self.replace(node.range(), replace_with)
@ -182,12 +183,6 @@ impl AssistBuilder {
}
}
fn calc_indent(node: &SyntaxNode) -> Option<&str> {
let prev = node.prev_sibling()?;
let ws_text = ast::Whitespace::cast(prev)?.text();
ws_text.rfind('\n').map(|pos| &ws_text[pos + 1..])
}
fn reindent(text: &str, indent: &str) -> String {
let indent = format!("\n{}", indent);
text.lines().intersperse(&indent).collect()

View File

@ -1,8 +1,16 @@
use ra_syntax::{
ast, AstNode,
AstNode,
SyntaxNode, SyntaxKind::*,
ast::{self, AstToken},
};
/// If the node is on the begining of the line, calculate indent.
pub(crate) fn leading_indent(node: &SyntaxNode) -> Option<&str> {
let prev = node.prev_sibling()?;
let ws_text = ast::Whitespace::cast(prev)?.text();
ws_text.rfind('\n').map(|pos| &ws_text[pos + 1..])
}
pub(crate) fn extract_trivial_expression(block: &ast::Block) -> Option<&ast::Expr> {
let expr = block.expr()?;
if expr.syntax().text().contains('\n') {

View File

@ -1,11 +1,11 @@
use ra_syntax::{
AstNode, SourceFile, SyntaxKind::*,
SyntaxNode, TextUnit, TextRange,
algo::{find_node_at_offset, find_leaf_at_offset, LeafAtOffset},
ast,
AstNode, Direction, SourceFile, SyntaxKind::*,
SyntaxNode, TextUnit,
ast::{self, AstToken},
};
use crate::{LocalEdit, TextEditBuilder};
use crate::{LocalEdit, TextEditBuilder, formatting::leading_indent};
pub fn on_enter(file: &SourceFile, offset: TextUnit) -> Option<LocalEdit> {
let comment = find_leaf_at_offset(file.syntax(), offset)
@ -53,20 +53,21 @@ fn node_indent<'a>(file: &'a SourceFile, node: &SyntaxNode) -> Option<&'a str> {
Some(&text[pos..])
}
pub fn on_eq_typed(file: &SourceFile, offset: TextUnit) -> Option<LocalEdit> {
let let_stmt: &ast::LetStmt = find_node_at_offset(file.syntax(), offset)?;
pub fn on_eq_typed(file: &SourceFile, eq_offset: TextUnit) -> Option<LocalEdit> {
assert_eq!(file.syntax().text().char_at(eq_offset), Some('='));
let let_stmt: &ast::LetStmt = find_node_at_offset(file.syntax(), eq_offset)?;
if let_stmt.has_semi() {
return None;
}
if let Some(expr) = let_stmt.initializer() {
let expr_range = expr.syntax().range();
if expr_range.contains(offset) && offset != expr_range.start() {
if expr_range.contains(eq_offset) && eq_offset != expr_range.start() {
return None;
}
if file
.syntax()
.text()
.slice(offset..expr_range.start())
.slice(eq_offset..expr_range.start())
.contains('\n')
{
return None;
@ -84,54 +85,44 @@ pub fn on_eq_typed(file: &SourceFile, offset: TextUnit) -> Option<LocalEdit> {
})
}
pub fn on_dot_typed(file: &SourceFile, offset: TextUnit) -> Option<LocalEdit> {
let before_dot_offset = offset - TextUnit::of_char('.');
pub fn on_dot_typed(file: &SourceFile, dot_offset: TextUnit) -> Option<LocalEdit> {
assert_eq!(file.syntax().text().char_at(dot_offset), Some('.'));
let whitespace = find_leaf_at_offset(file.syntax(), before_dot_offset).left_biased()?;
let whitespace = find_leaf_at_offset(file.syntax(), dot_offset)
.left_biased()
.and_then(ast::Whitespace::cast)?;
// find whitespace just left of the dot
ast::Whitespace::cast(whitespace)?;
let current_indent = {
let text = whitespace.text();
let newline = text.rfind('\n')?;
&text[newline + 1..]
};
let current_indent_len = TextUnit::of_str(current_indent);
// make sure there is a method call
let method_call = whitespace
.siblings(Direction::Prev)
// first is whitespace
.skip(1)
.next()?;
ast::MethodCallExpr::cast(method_call)?;
// find how much the _method call is indented
let method_chain_indent = method_call
.parent()?
.siblings(Direction::Prev)
.skip(1)
.next()?
.leaf_text()
.map(|x| last_line_indent_in_whitespace(x))?;
let current_indent = TextUnit::of_str(last_line_indent_in_whitespace(whitespace.leaf_text()?));
// TODO: indent is always 4 spaces now. A better heuristic could look on the previous line(s)
let target_indent = TextUnit::of_str(method_chain_indent) + TextUnit::from_usize(4);
let diff = target_indent - current_indent;
let indent = "".repeat(diff.to_usize());
let cursor_position = offset + diff;
// Make sure dot is a part of call chain
let field_expr = whitespace
.syntax()
.parent()
.and_then(ast::FieldExpr::cast)?;
let prev_indent = leading_indent(field_expr.syntax())?;
let target_indent = format!(" {}", prev_indent);
let target_indent_len = TextUnit::of_str(&target_indent);
if current_indent_len == target_indent_len {
return None;
}
let mut edit = TextEditBuilder::default();
edit.insert(before_dot_offset, indent);
Some(LocalEdit {
label: "indent dot".to_string(),
edit.replace(
TextRange::from_to(dot_offset - current_indent_len, dot_offset),
target_indent.into(),
);
let res = LocalEdit {
label: "reindent dot".to_string(),
edit: edit.finish(),
cursor_position: Some(cursor_position),
})
}
/// Finds the last line in the whitespace
fn last_line_indent_in_whitespace(ws: &str) -> &str {
ws.split('\n').last().unwrap_or("")
cursor_position: Some(
dot_offset + target_indent_len - current_indent_len + TextUnit::of_char('.'),
),
};
Some(res)
}
#[cfg(test)]
@ -142,12 +133,18 @@ mod tests {
#[test]
fn test_on_eq_typed() {
fn do_check(before: &str, after: &str) {
fn type_eq(before: &str, after: &str) {
let (offset, before) = extract_offset(before);
let mut edit = TextEditBuilder::default();
edit.insert(offset, "=".to_string());
let before = edit.finish().apply(&before);
let file = SourceFile::parse(&before);
let result = on_eq_typed(&file, offset).unwrap();
let actual = result.edit.apply(&before);
assert_eq_text!(after, &actual);
if let Some(result) = on_eq_typed(&file, offset) {
let actual = result.edit.apply(&before);
assert_eq_text!(after, &actual);
} else {
assert_eq_text!(&before, after)
};
}
// do_check(r"
@ -159,10 +156,10 @@ mod tests {
// let foo =;
// }
// ");
do_check(
type_eq(
r"
fn foo() {
let foo =<|> 1 + 1
let foo <|> 1 + 1
}
",
r"
@ -184,112 +181,113 @@ fn foo() {
// ");
}
fn type_dot(before: &str, after: &str) {
let (offset, before) = extract_offset(before);
let mut edit = TextEditBuilder::default();
edit.insert(offset, ".".to_string());
let before = edit.finish().apply(&before);
let file = SourceFile::parse(&before);
if let Some(result) = on_dot_typed(&file, offset) {
let actual = result.edit.apply(&before);
assert_eq_text!(after, &actual);
} else {
assert_eq_text!(&before, after)
};
}
#[test]
fn test_on_dot_typed() {
fn do_check(before: &str, after: &str) {
let (offset, before) = extract_offset(before);
let file = SourceFile::parse(&before);
if let Some(result) = on_eq_typed(&file, offset) {
let actual = result.edit.apply(&before);
assert_eq_text!(after, &actual);
};
}
// indent if continuing chain call
do_check(
fn indents_new_chain_call() {
type_dot(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.<|>
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.
}
",
);
type_dot(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.
}
",
)
}
// do not indent if already indented
do_check(
#[test]
fn indents_continued_chain_call() {
type_dot(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.<|>
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
.
}
",
);
// indent if the previous line is already indented
do_check(
type_dot(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
.<|>
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
.
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
.
}
",
);
}
// don't indent if indent matches previous line
do_check(
#[test]
fn dont_indent_freestanding_dot() {
type_dot(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
.<|>
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.first()
.
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
.
}
",
);
// don't indent if there is no method call on previous line
do_check(
type_dot(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
.<|>
}
",
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
.
}
",
);
// indent to match previous expr
do_check(
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
.<|>
}
",
r"
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
self.child_impl(db, name)
pub fn child(&self, db: &impl HirDatabase, name: &Name) -> Cancelable<Option<Module>> {
.
}
",
}
",
);
}

View File

@ -9,7 +9,7 @@ use languageserver_types::{
SignatureInformation, SymbolInformation, TextDocumentIdentifier, TextEdit, WorkspaceEdit,
};
use ra_ide_api::{
FileId, FilePosition, FileRange, FoldKind, Query, RunnableKind, Severity, SourceChange,
FileId, FilePosition, FileRange, FoldKind, Query, RunnableKind, Severity,
};
use ra_syntax::{TextUnit, AstNode};
use rustc_hash::FxHashMap;
@ -92,35 +92,30 @@ pub fn handle_on_type_formatting(
world: ServerWorld,
params: req::DocumentOnTypeFormattingParams,
) -> Result<Option<Vec<TextEdit>>> {
let analysis: Option<Box<Fn(FilePosition) -> Option<SourceChange>>> = match params.ch.as_str() {
"=" => Some(Box::new(|pos| world.analysis().on_eq_typed(pos))),
"." => Some(Box::new(|pos| world.analysis().on_dot_typed(pos))),
_ => None,
let file_id = params.text_document.try_conv_with(&world)?;
let line_index = world.analysis().file_line_index(file_id);
let position = FilePosition {
file_id,
/// in `ra_ide_api`, the `on_type` invariant is that
/// `text.char_at(position) == typed_char`.
offset: params.position.conv_with(&line_index) - TextUnit::of_char('.'),
};
if let Some(ana) = analysis {
let file_id = params.text_document.try_conv_with(&world)?;
let line_index = world.analysis().file_line_index(file_id);
let position = FilePosition {
file_id,
offset: params.position.conv_with(&line_index),
};
let edit = match params.ch.as_str() {
"=" => world.analysis().on_eq_typed(position),
"." => world.analysis().on_dot_typed(position),
_ => return Ok(None),
};
let mut edit = match edit {
Some(it) => it,
None => return Ok(None),
};
if let Some(mut action) = ana(position) {
let change: Vec<TextEdit> = action
.source_file_edits
.pop()
.unwrap()
.edit
.as_atoms()
.iter()
.map_conv_with(&line_index)
.collect();
return Ok(Some(change));
}
}
// This should be a single-file edit
let edit = edit.source_file_edits.pop().unwrap();
return Ok(None);
let change: Vec<TextEdit> = edit.edit.conv_with(&line_index);
return Ok(Some(change));
}
pub fn handle_document_symbol(