Skip to content

Commit

Permalink
fix(lsp): handle import specifier not having a trailing quote (#13074)
Browse files Browse the repository at this point in the history
* fix(lsp): handle import specifier not having a trailing quote

* clean up

* Add test.
  • Loading branch information
dsherret authored Dec 13, 2021
1 parent a54fc7a commit c9d32e0
Showing 1 changed file with 71 additions and 4 deletions.
75 changes: 71 additions & 4 deletions cli/lsp/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ use super::tsc;
use crate::fs_util::is_supported_ext;
use crate::fs_util::specifier_to_file_path;

use deno_ast::swc::common::BytePos;
use deno_ast::LineAndColumnIndex;
use deno_ast::SourceTextInfo;
use deno_core::normalize_path;
use deno_core::resolve_path;
use deno_core::resolve_url;
Expand Down Expand Up @@ -93,17 +96,32 @@ async fn check_auto_config_registry(
}
}

/// Ranges from the graph for specifiers include the leading and trailing quote,
/// Ranges from the graph for specifiers include the leading and maybe trailing quote,
/// which we want to ignore when replacing text.
fn to_narrow_lsp_range(range: &deno_graph::Range) -> lsp::Range {
fn to_narrow_lsp_range(
text_info: &SourceTextInfo,
range: &deno_graph::Range,
) -> lsp::Range {
let end_byte_index = text_info.byte_index(LineAndColumnIndex {
line_index: range.end.line,
column_index: range.end.character,
});
let text_bytes = text_info.text_str().as_bytes();
let has_trailing_quote =
matches!(text_bytes[end_byte_index.0 as usize - 1], (b'"' | b'\''));
lsp::Range {
start: lsp::Position {
line: range.start.line as u32,
// skip the leading quote
character: (range.start.character + 1) as u32,
},
end: lsp::Position {
line: range.end.line as u32,
character: (range.end.character - 1) as u32,
character: if has_trailing_quote {
range.end.character - 1 // do not include it
} else {
range.end.character
} as u32,
},
}
}
Expand All @@ -119,7 +137,7 @@ pub(crate) async fn get_import_completions(
) -> Option<lsp::CompletionResponse> {
let document = state_snapshot.documents.get(specifier)?;
let (text, _, range) = document.get_maybe_dependency(position)?;
let range = to_narrow_lsp_range(&range);
let range = to_narrow_lsp_range(&document.text_info(), &range);
// completions for local relative modules
if text.starts_with("./") || text.starts_with("../") {
Some(lsp::CompletionResponse::List(lsp::CompletionList {
Expand Down Expand Up @@ -437,6 +455,7 @@ mod tests {
use crate::lsp::documents::Documents;
use crate::lsp::documents::LanguageId;
use deno_core::resolve_url;
use deno_graph::Range;
use std::collections::HashMap;
use std::path::Path;
use std::sync::Arc;
Expand Down Expand Up @@ -679,4 +698,52 @@ mod tests {
}]
);
}

#[test]
fn test_to_narrow_lsp_range() {
let text_info = SourceTextInfo::from_string(r#""te""#.to_string());
let range = to_narrow_lsp_range(
&text_info,
&Range {
specifier: ModuleSpecifier::parse("https://deno.land").unwrap(),
start: deno_graph::Position {
line: 0,
character: 0,
},
end: deno_graph::Position {
line: 0,
character: text_info.text_str().chars().count(),
},
},
);
assert_eq!(range.start.character, 1);
assert_eq!(
range.end.character,
(text_info.text_str().chars().count() - 1) as u32
);
}

#[test]
fn test_to_narrow_lsp_range_no_trailing_quote() {
let text_info = SourceTextInfo::from_string(r#""te"#.to_string());
let range = to_narrow_lsp_range(
&text_info,
&Range {
specifier: ModuleSpecifier::parse("https://deno.land").unwrap(),
start: deno_graph::Position {
line: 0,
character: 0,
},
end: deno_graph::Position {
line: 0,
character: text_info.text_str().chars().count(),
},
},
);
assert_eq!(range.start.character, 1);
assert_eq!(
range.end.character,
text_info.text_str().chars().count() as u32
);
}
}

0 comments on commit c9d32e0

Please sign in to comment.