1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-01-03 04:48:52 -05:00

fix(lsp): correct positions in some scenarios (#14359)

This commit is contained in:
David Sherret 2022-05-15 14:41:37 -04:00 committed by GitHub
parent 8744ee883e
commit eb5ffab1cb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 96 additions and 533 deletions

View file

@ -197,6 +197,14 @@ fn create_compiler_snapshot(
false
}
#[op]
fn op_script_version(
_state: &mut OpState,
_args: Value,
) -> Result<Option<String>, AnyError> {
Ok(Some("1".to_string()))
}
#[op]
// using the same op that is used in `tsc.rs` for loading modules and reading
// files, but a slightly different implementation at build time.
@ -211,7 +219,7 @@ fn create_compiler_snapshot(
if args.specifier == build_specifier {
Ok(json!({
"data": r#"console.log("hello deno!");"#,
"hash": "1",
"version": "1",
// this corresponds to `ts.ScriptKind.TypeScript`
"scriptKind": 3
}))
@ -230,7 +238,7 @@ fn create_compiler_snapshot(
let data = std::fs::read_to_string(path)?;
Ok(json!({
"data": data,
"hash": "1",
"version": "1",
// this corresponds to `ts.ScriptKind.TypeScript`
"scriptKind": 3
}))
@ -255,6 +263,7 @@ fn create_compiler_snapshot(
op_cwd::decl(),
op_exists::decl(),
op_load::decl(),
op_script_version::decl(),
])
.state(move |state| {
state.put(op_crate_libs.clone());

View file

@ -200,6 +200,13 @@ impl AssetOrDocument {
}
}
pub fn media_type(&self) -> MediaType {
match self {
AssetOrDocument::Asset(_) => MediaType::TypeScript, // assets are always TypeScript
AssetOrDocument::Document(d) => d.media_type(),
}
}
pub fn get_maybe_dependency(
&self,
position: &lsp::Position,

View file

@ -2,13 +2,9 @@
use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::serde_json::json;
use deno_core::serde_json::Value;
use dissimilar::diff;
use dissimilar::Chunk;
use std::collections::HashMap;
use std::ops::Bound;
use std::ops::RangeBounds;
use text_size::TextRange;
use text_size::TextSize;
use tower_lsp::jsonrpc;
@ -261,112 +257,6 @@ pub fn get_edits(a: &str, b: &str, line_index: &LineIndex) -> Vec<TextEdit> {
text_edits
}
/// Convert a difference between two strings into a change range used by the
/// TypeScript Language Service.
pub fn get_range_change(a: &str, b: &str) -> Value {
if a == b {
return json!(null);
}
let chunks = diff(a, b);
let mut iter = chunks.iter().peekable();
let mut started = false;
let mut start = 0;
let mut end = 0;
let mut new_length = 0;
let mut equal = 0;
let mut a_pos = 0;
loop {
let diff = iter.next();
match diff {
None => break,
Some(Chunk::Equal(e)) => {
a_pos += e.encode_utf16().count();
equal += e.encode_utf16().count();
}
Some(Chunk::Delete(d)) => {
if !started {
start = a_pos;
started = true;
equal = 0;
}
a_pos += d.encode_utf16().count();
if started {
end = a_pos;
new_length += equal;
equal = 0;
}
}
Some(Chunk::Insert(i)) => {
if !started {
start = a_pos;
end = a_pos;
started = true;
equal = 0;
} else {
end += equal;
}
new_length += i.encode_utf16().count() + equal;
equal = 0;
}
}
}
json!({
"span": {
"start": start,
"length": end - start,
},
"newLength": new_length,
})
}
/// Provide a slice of a string based on a character range.
pub fn slice(s: &str, range: impl RangeBounds<usize>) -> &str {
let start = match range.start_bound() {
Bound::Included(bound) | Bound::Excluded(bound) => *bound,
Bound::Unbounded => 0,
};
let len = match range.end_bound() {
Bound::Included(bound) => *bound + 1,
Bound::Excluded(bound) => *bound,
Bound::Unbounded => s.encode_utf16().count(),
} - start;
substring(s, start, start + len)
}
/// Provide a substring based on the start and end character index positions.
pub fn substring(s: &str, start: usize, end: usize) -> &str {
let len = end - start;
let mut char_pos = 0;
let mut byte_start = 0;
let mut it = s.chars();
loop {
if char_pos == start {
break;
}
if let Some(c) = it.next() {
char_pos += c.len_utf16();
byte_start += c.len_utf8();
} else {
break;
}
}
char_pos = 0;
let mut byte_end = byte_start;
loop {
if char_pos == len {
break;
}
if let Some(c) = it.next() {
char_pos += c.len_utf16();
byte_end += c.len_utf8();
} else {
break;
}
}
&s[byte_start..byte_end]
}
#[cfg(test)]
mod tests {
use super::*;
@ -637,155 +527,4 @@ const C: char = \"メ メ\";
]
)
}
#[test]
fn test_get_range_change() {
let a = "abcdefg";
let b = "abcdefg";
let actual = get_range_change(a, b);
assert_eq!(actual, json!(null));
let a = "abcdefg";
let b = "abedcfg";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 2,
"length": 3,
},
"newLength": 3
})
);
let a = "abfg";
let b = "abcdefg";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 2,
"length": 0,
},
"newLength": 3
})
);
let a = "abcdefg";
let b = "abfg";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 2,
"length": 3,
},
"newLength": 0
})
);
let a = "abcdefg";
let b = "abfghij";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 2,
"length": 5,
},
"newLength": 5
})
);
let a = "abcdefghijk";
let b = "axcxexfxixk";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 1,
"length": 9,
},
"newLength": 9
})
);
let a = "abcde";
let b = "ab(c)de";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span" : {
"start": 2,
"length": 1,
},
"newLength": 3
})
);
let a = "hello 🦕!";
let b = "hello deno!";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 6,
"length": 2,
},
"newLength": 4
})
);
let a = "hello deno!";
let b = "hello deno🦕!";
let actual = get_range_change(a, b);
assert_eq!(
actual,
json!({
"span": {
"start": 10,
"length": 0,
},
"newLength": 2
})
);
// TODO(@kitsonk): https://github.com/dtolnay/dissimilar/issues/5
// let a = r#" 🦕🇺🇸👍 "#;
// let b = r#" 🇺🇸👍 "#;
// let actual = get_range_change(a, b);
// assert_eq!(
// actual,
// json!({
// "span": {
// "start": 1,
// "length": 2,
// },
// "newLength": 0
// })
// );
}
#[test]
fn test_substring() {
assert_eq!(substring("Deno", 1, 3), "en");
assert_eq!(substring("y̆y̆", 2, 4), "");
assert_eq!(substring("🦕🦕", 2, 4), "🦕");
}
#[test]
fn test_slice() {
assert_eq!(slice("Deno", 1..3), "en");
assert_eq!(slice("Deno", 1..=3), "eno");
assert_eq!(slice("Deno Land", 1..), "eno Land");
assert_eq!(slice("Deno", ..3), "Den");
assert_eq!(slice("Hello 🦕", 6..8), "🦕");
}
}

View file

@ -13,7 +13,6 @@ use super::refactor::EXTRACT_INTERFACE;
use super::refactor::EXTRACT_TYPE;
use super::semantic_tokens;
use super::semantic_tokens::SemanticTokensBuilder;
use super::text;
use super::text::LineIndex;
use super::urls::LspUrlMap;
use super::urls::INVALID_SPECIFIER;
@ -47,7 +46,6 @@ use log::warn;
use once_cell::sync::Lazy;
use regex::Captures;
use regex::Regex;
use std::borrow::Cow;
use std::cmp;
use std::collections::HashMap;
use std::collections::HashSet;
@ -157,7 +155,6 @@ impl TsServer {
struct AssetDocumentInner {
specifier: ModuleSpecifier,
text: Arc<String>,
length: usize,
line_index: Arc<LineIndex>,
maybe_navigation_tree: Option<Arc<NavigationTree>>,
}
@ -173,7 +170,6 @@ impl AssetDocument {
Self(Arc::new(AssetDocumentInner {
specifier,
text: Arc::new(text.to_string()),
length: text.encode_utf16().count(),
line_index: Arc::new(LineIndex::new(text)),
maybe_navigation_tree: None,
}))
@ -197,14 +193,6 @@ impl AssetDocument {
self.0.text.clone()
}
pub fn text_str(&self) -> &str {
self.0.text.as_str()
}
pub fn length(&self) -> usize {
self.0.length
}
pub fn line_index(&self) -> Arc<LineIndex> {
self.0.line_index.clone()
}
@ -2374,17 +2362,16 @@ struct Response {
data: Value,
}
struct State<'a> {
struct State {
last_id: usize,
performance: Arc<Performance>,
response: Option<Response>,
state_snapshot: Arc<StateSnapshot>,
snapshots: HashMap<(ModuleSpecifier, Cow<'a, str>), String>,
specifiers: HashMap<String, String>,
token: CancellationToken,
}
impl<'a> State<'a> {
impl State {
fn new(
state_snapshot: Arc<StateSnapshot>,
performance: Arc<Performance>,
@ -2394,7 +2381,6 @@ impl<'a> State<'a> {
performance,
response: None,
state_snapshot,
snapshots: HashMap::default(),
specifiers: HashMap::default(),
token: Default::default(),
}
@ -2426,31 +2412,37 @@ impl<'a> State<'a> {
}
ModuleSpecifier::parse(&specifier_str).map_err(|err| err.into())
}
fn get_asset_or_document(
&self,
specifier: &ModuleSpecifier,
) -> Option<AssetOrDocument> {
let snapshot = &self.state_snapshot;
if specifier.scheme() == "asset" {
snapshot.assets.get(specifier).map(AssetOrDocument::Asset)
} else {
snapshot
.documents
.get(specifier)
.map(AssetOrDocument::Document)
}
}
/// If a snapshot is missing from the state cache, add it.
fn cache_snapshot(
state: &mut State,
specifier: &ModuleSpecifier,
version: String,
) -> Result<(), AnyError> {
if !state
.snapshots
.contains_key(&(specifier.clone(), version.clone().into()))
{
let content = state
fn script_version(&self, specifier: &ModuleSpecifier) -> Option<String> {
if specifier.scheme() == "asset" {
if self.state_snapshot.assets.contains_key(specifier) {
Some("1".to_string())
} else {
None
}
} else {
self
.state_snapshot
.documents
.get(specifier)
.ok_or_else(|| {
anyhow!("Specifier unexpectedly doesn't exist: {}", specifier)
})?
.content();
state
.snapshots
.insert((specifier.clone(), version.into()), content.to_string());
.map(|d| d.script_version())
}
}
Ok(())
}
fn normalize_specifier<S: AsRef<str>>(
@ -2460,28 +2452,6 @@ fn normalize_specifier<S: AsRef<str>>(
.map_err(|err| err.into())
}
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
struct SourceSnapshotArgs {
specifier: String,
version: String,
}
/// The language service is dropping a reference to a source file snapshot, and
/// we can drop our version of that document.
#[op]
fn op_dispose(
state: &mut OpState,
args: SourceSnapshotArgs,
) -> Result<bool, AnyError> {
let state = state.borrow_mut::<State>();
let mark = state.performance.mark("op_dispose", Some(&args));
let specifier = state.normalize_specifier(&args.specifier)?;
state.snapshots.remove(&(specifier, args.version.into()));
state.performance.measure(mark);
Ok(true)
}
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
struct SpecifierArgs {
@ -2506,116 +2476,6 @@ fn op_exists(state: &mut OpState, args: SpecifierArgs) -> bool {
state.state_snapshot.documents.exists(&specifier)
}
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
struct GetChangeRangeArgs {
specifier: String,
old_length: u32,
old_version: String,
version: String,
}
/// The language service wants to compare an old snapshot with a new snapshot to
/// determine what source has changed.
#[op]
fn op_get_change_range(
state: &mut OpState,
args: GetChangeRangeArgs,
) -> Result<Value, AnyError> {
let state = state.borrow_mut::<State>();
let mark = state.performance.mark("op_get_change_range", Some(&args));
let specifier = state.normalize_specifier(&args.specifier)?;
cache_snapshot(state, &specifier, args.version.clone())?;
let r = if let Some(current) = state
.snapshots
.get(&(specifier.clone(), args.version.clone().into()))
{
if let Some(prev) = state
.snapshots
.get(&(specifier, args.old_version.clone().into()))
{
Ok(text::get_range_change(prev, current))
} else {
let new_length = current.encode_utf16().count();
// when a local file is opened up in the editor, the compiler might
// already have a snapshot of it in memory, and will request it, but we
// now are working off in memory versions of the document, and so need
// to tell tsc to reset the whole document
Ok(json!({
"span": {
"start": 0,
"length": args.old_length,
},
"newLength": new_length,
}))
}
} else {
Err(custom_error(
"MissingSnapshot",
format!(
"The current snapshot version is missing.\n Args: \"{:?}\"",
args
),
))
};
state.performance.measure(mark);
r
}
#[op]
fn op_get_length(
state: &mut OpState,
args: SourceSnapshotArgs,
) -> Result<usize, AnyError> {
let state = state.borrow_mut::<State>();
let mark = state.performance.mark("op_get_length", Some(&args));
let specifier = state.normalize_specifier(args.specifier)?;
let r = if let Some(asset) = state.state_snapshot.assets.get(&specifier) {
Ok(asset.length())
} else {
cache_snapshot(state, &specifier, args.version.clone())?;
let content = state
.snapshots
.get(&(specifier, args.version.into()))
.unwrap();
Ok(content.encode_utf16().count())
};
state.performance.measure(mark);
r
}
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
struct GetTextArgs {
specifier: String,
version: String,
start: usize,
end: usize,
}
#[op]
fn op_get_text(
state: &mut OpState,
args: GetTextArgs,
) -> Result<String, AnyError> {
let state = state.borrow_mut::<State>();
let mark = state.performance.mark("op_get_text", Some(&args));
let specifier = state.normalize_specifier(args.specifier)?;
let maybe_asset = state.state_snapshot.assets.get(&specifier);
let content = if let Some(content) = &maybe_asset {
content.text_str()
} else {
cache_snapshot(state, &specifier, args.version.clone())?;
state
.snapshots
.get(&(specifier, args.version.into()))
.unwrap()
};
state.performance.measure(mark);
Ok(text::slice(content, args.start..args.end).to_string())
}
#[op]
fn op_is_cancelled(state: &mut OpState) -> bool {
let state = state.borrow_mut::<State>();
@ -2626,13 +2486,22 @@ fn op_is_cancelled(state: &mut OpState) -> bool {
fn op_load(
state: &mut OpState,
args: SpecifierArgs,
) -> Result<Option<String>, AnyError> {
) -> Result<Value, AnyError> {
let state = state.borrow_mut::<State>();
let mark = state.performance.mark("op_load", Some(&args));
let specifier = state.normalize_specifier(args.specifier)?;
let document = state.state_snapshot.documents.get(&specifier);
let asset_or_document = state.get_asset_or_document(&specifier);
state.performance.measure(mark);
Ok(document.map(|d| d.content().to_string()))
Ok(match asset_or_document {
Some(doc) => {
json!({
"data": doc.text(),
"scriptKind": crate::tsc::as_ts_script_kind(&doc.media_type()),
"version": state.script_version(&specifier),
})
}
None => Value::Null,
})
}
#[op]
@ -2705,20 +2574,7 @@ fn op_script_version(
// this op is very "noisy" and measuring its performance is not useful, so we
// don't measure it uniquely anymore.
let specifier = state.normalize_specifier(args.specifier)?;
if specifier.scheme() == "asset" {
if state.state_snapshot.assets.contains_key(&specifier) {
Ok(Some("1".to_string()))
} else {
Ok(None)
}
} else {
let script_version = state
.state_snapshot
.documents
.get(&specifier)
.map(|d| d.script_version());
Ok(script_version)
}
Ok(state.script_version(&specifier))
}
/// Create and setup a JsRuntime based on a snapshot. It is expected that the
@ -2735,11 +2591,7 @@ fn js_runtime(performance: Arc<Performance>) -> JsRuntime {
fn init_extension(performance: Arc<Performance>) -> Extension {
Extension::builder()
.ops(vec![
op_dispose::decl(),
op_exists::decl(),
op_get_change_range::decl(),
op_get_length::decl(),
op_get_text::decl(),
op_is_cancelled::decl(),
op_load::decl(),
op_resolve::decl(),

View file

@ -1401,7 +1401,9 @@ fn lsp_hover_change_mbc() {
},
"end": {
"line": 1,
"character": 13
// the LSP uses utf16 encoded characters indexes, so
// after the deno emoiji is character index 15
"character": 15
}
},
"text": ""
@ -1425,7 +1427,7 @@ fn lsp_hover_change_mbc() {
},
"position": {
"line": 2,
"character": 14
"character": 15
}
}),
)
@ -1444,11 +1446,11 @@ fn lsp_hover_change_mbc() {
"range": {
"start": {
"line": 2,
"character": 13,
"character": 15,
},
"end": {
"line": 2,
"character": 14,
"character": 16,
},
}
}))
@ -4284,7 +4286,7 @@ fn lsp_performance() {
.unwrap();
assert!(maybe_err.is_none());
if let Some(res) = maybe_res {
assert_eq!(res.averages.len(), 14);
assert_eq!(res.averages.len(), 13);
} else {
panic!("unexpected result");
}

View file

@ -422,7 +422,7 @@ struct LoadArgs {
specifier: String,
}
fn as_ts_script_kind(media_type: &MediaType) -> i32 {
pub fn as_ts_script_kind(media_type: &MediaType) -> i32 {
match media_type {
MediaType::JavaScript => 1,
MediaType::Jsx => 2,
@ -492,7 +492,7 @@ fn op_load(state: &mut OpState, args: Value) -> Result<Value, AnyError> {
};
Ok(
json!({ "data": data, "hash": hash, "scriptKind": as_ts_script_kind(&media_type) }),
json!({ "data": data, "version": hash, "scriptKind": as_ts_script_kind(&media_type) }),
)
}
@ -1002,7 +1002,7 @@ mod tests {
actual,
json!({
"data": "console.log(\"hello deno\");\n",
"hash": "149c777056afcc973d5fcbe11421b6d5ddc57b81786765302030d7fc893bf729",
"version": "149c777056afcc973d5fcbe11421b6d5ddc57b81786765302030d7fc893bf729",
"scriptKind": 3,
})
);
@ -1012,7 +1012,7 @@ mod tests {
#[serde(rename_all = "camelCase")]
struct LoadResponse {
data: String,
hash: Option<String>,
version: Option<String>,
script_kind: i64,
}
@ -1033,7 +1033,7 @@ mod tests {
serde_json::from_value(value).expect("failed to deserialize");
let expected = get_asset("lib.dom.d.ts").unwrap();
assert_eq!(actual.data, expected);
assert!(actual.hash.is_some());
assert!(actual.version.is_some());
assert_eq!(actual.script_kind, 3);
}
@ -1052,7 +1052,7 @@ mod tests {
actual,
json!({
"data": "some content",
"hash": null,
"version": null,
"scriptKind": 0,
})
);
@ -1070,7 +1070,7 @@ mod tests {
actual,
json!({
"data": null,
"hash": null,
"version": null,
"scriptKind": 0,
})
)

View file

@ -72,6 +72,8 @@ delete Object.prototype.__proto__;
}
}
// In the case of the LSP, this is initialized with the assets
// when snapshotting and never added to or removed after that.
/** @type {Map<string, ts.SourceFile>} */
const sourceFileCache = new Map();
@ -186,62 +188,6 @@ delete Object.prototype.__proto__;
target: ts.ScriptTarget.ESNext,
};
class ScriptSnapshot {
/** @type {string} */
specifier;
/** @type {string} */
version;
/**
* @param {string} specifier
* @param {string} version
*/
constructor(specifier, version) {
this.specifier = specifier;
this.version = version;
}
/**
* @param {number} start
* @param {number} end
* @returns {string}
*/
getText(start, end) {
const { specifier, version } = this;
debug(
`snapshot.getText(${start}, ${end}) specifier: ${specifier} version: ${version}`,
);
return core.opSync("op_get_text", { specifier, version, start, end });
}
/**
* @returns {number}
*/
getLength() {
const { specifier, version } = this;
debug(`snapshot.getLength() specifier: ${specifier} version: ${version}`);
return core.opSync("op_get_length", { specifier, version });
}
/**
* @param {ScriptSnapshot} oldSnapshot
* @returns {ts.TextChangeRange | undefined}
*/
getChangeRange(oldSnapshot) {
const { specifier, version } = this;
const { version: oldVersion } = oldSnapshot;
const oldLength = oldSnapshot.getLength();
debug(
`snapshot.getLength() specifier: ${specifier} oldVersion: ${oldVersion} version: ${version}`,
);
return core.opSync(
"op_get_change_range",
{ specifier, oldLength, oldVersion, version },
);
}
dispose() {
const { specifier, version } = this;
debug(`snapshot.dispose() specifier: ${specifier} version: ${version}`);
core.opSync("op_dispose", { specifier, version });
}
}
/** Error thrown on cancellation. */
class OperationCanceledError extends Error {
}
@ -310,16 +256,17 @@ delete Object.prototype.__proto__;
ts.ScriptTarget[languageVersion]
})`,
);
// Needs the original specifier
specifier = normalizedToOriginalMap.get(specifier) ?? specifier;
let sourceFile = sourceFileCache.get(specifier);
if (sourceFile) {
return sourceFile;
}
// Needs the original specifier
specifier = normalizedToOriginalMap.get(specifier) ?? specifier;
/** @type {{ data: string; hash?: string; scriptKind: ts.ScriptKind }} */
const { data, hash, scriptKind } = core.opSync(
/** @type {{ data: string; scriptKind: ts.ScriptKind }} */
const { data, scriptKind, version } = core.opSync(
"op_load",
{ specifier },
);
@ -335,8 +282,9 @@ delete Object.prototype.__proto__;
scriptKind,
);
sourceFile.moduleName = specifier;
sourceFile.version = hash;
sourceFile.version = version;
sourceFileCache.set(specifier, sourceFile);
scriptVersionCache.set(specifier, version);
return sourceFile;
},
getDefaultLibFileName() {
@ -445,11 +393,17 @@ delete Object.prototype.__proto__;
},
};
}
const version = host.getScriptVersion(specifier);
if (version != null) {
return new ScriptSnapshot(specifier, version);
}
const fileInfo = core.opSync(
"op_load",
{ specifier },
);
if (fileInfo) {
scriptVersionCache.set(specifier, fileInfo.version);
return ts.ScriptSnapshot.fromString(fileInfo.data);
} else {
return undefined;
}
},
};