From 79916226b74bf9593afacbcdfe9d58263bed132f Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Fri, 12 Feb 2021 22:49:42 +1100 Subject: [PATCH] fix(lsp): properly handle static assets (#9476) --- cli/lsp/analysis.rs | 10 +-- cli/lsp/diagnostics.rs | 2 +- cli/lsp/documents.rs | 8 +- cli/lsp/language_server.rs | 15 ++-- cli/lsp/sources.rs | 6 +- cli/lsp/tsc.rs | 149 ++++++++++++++++++++++++++++--------- cli/tsc.rs | 58 ++++++++------- 7 files changed, 164 insertions(+), 84 deletions(-) diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 8600301a5ce1f5..1824e529889af1 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -32,14 +32,14 @@ lazy_static! { /// Diagnostic error codes which actually are the same, and so when grouping /// fixes we treat them the same. static ref FIX_ALL_ERROR_CODES: HashMap<&'static str, &'static str> = - [("2339", "2339"), ("2345", "2339"),] + (&[("2339", "2339"), ("2345", "2339"),]) .iter() - .copied() + .cloned() .collect(); /// Fixes which help determine if there is a preferred fix when there are /// multiple fixes available. - static ref PREFERRED_FIXES: HashMap<&'static str, (u32, bool)> = [ + static ref PREFERRED_FIXES: HashMap<&'static str, (u32, bool)> = (&[ ("annotateWithTypeFromJSDoc", (1, false)), ("constructorForDerivedNeedSuperCall", (1, false)), ("extendsInterfaceBecomesImplements", (1, false)), @@ -52,9 +52,9 @@ lazy_static! { ("spelling", (2, false)), ("addMissingAwait", (1, false)), ("fixImport", (0, true)), - ] + ]) .iter() - .copied() + .cloned() .collect(); } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 1ea2cce1461e0d..1237ff7a340854 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -294,7 +294,7 @@ pub async fn generate_dependency_diagnostics( }) } ResolvedDependency::Resolved(specifier) => { - if !(state_snapshot.documents.contains(&specifier) || sources.contains(&specifier)) { + if !(state_snapshot.documents.contains_key(&specifier) || sources.contains_key(&specifier)) { let is_local = specifier.as_url().scheme() == "file"; let (code, message) = if is_local { (Some(lsp::NumberOrString::String("no-local".to_string())), format!("Unable to load a local module: \"{}\".\n Please check the file path.", specifier)) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index e8ea0e2cf0dd30..2a99069586fa81 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -96,7 +96,7 @@ impl DocumentCache { version: i32, content_changes: Vec, ) -> Result, AnyError> { - if !self.contains(specifier) { + if !self.contains_key(specifier) { return Err(custom_error( "NotFound", format!( @@ -119,7 +119,7 @@ impl DocumentCache { } } - pub fn contains(&self, specifier: &ModuleSpecifier) -> bool { + pub fn contains_key(&self, specifier: &ModuleSpecifier) -> bool { self.docs.contains_key(specifier) } @@ -213,8 +213,8 @@ mod tests { let missing_specifier = ModuleSpecifier::resolve_url("file:///a/c.ts").unwrap(); document_cache.open(specifier.clone(), 1, "console.log(\"Hello Deno\");\n"); - assert!(document_cache.contains(&specifier)); - assert!(!document_cache.contains(&missing_specifier)); + assert!(document_cache.contains_key(&specifier)); + assert!(!document_cache.contains_key(&missing_specifier)); } #[test] diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 5b4ab0f999fd74..7f7b693f9283ca 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -50,6 +50,7 @@ use super::text; use super::text::LineIndex; use super::tsc; use super::tsc::AssetDocument; +use super::tsc::Assets; use super::tsc::TsServer; use super::utils; @@ -63,7 +64,7 @@ pub struct LanguageServer(Arc>); #[derive(Debug, Clone, Default)] pub struct StateSnapshot { - pub assets: HashMap>, + pub assets: Assets, pub documents: DocumentCache, pub performance: Performance, pub sources: Sources, @@ -73,7 +74,7 @@ pub struct StateSnapshot { pub(crate) struct Inner { /// Cached versions of "fixed" assets that can either be inlined in Rust or /// are part of the TypeScript snapshot and have to be fetched out. - assets: HashMap>, + assets: Assets, /// The LSP client that this LSP server is connected to. client: Client, /// Configuration information. @@ -86,7 +87,7 @@ pub(crate) struct Inner { /// file which will be used by the Deno LSP. maybe_config_uri: Option, /// An optional import map which is used to resolve modules. - pub maybe_import_map: Option, + pub(crate) maybe_import_map: Option, /// The URL for the import map which is used to determine relative imports. maybe_import_map_uri: Option, /// A map of all the cached navigation trees. @@ -203,7 +204,7 @@ impl Inner { } } else { let documents = &self.documents; - if documents.contains(specifier) { + if documents.contains_key(specifier) { documents.line_index(specifier) } else { self.sources.get_line_index(specifier) @@ -529,10 +530,8 @@ impl Inner { if let Some(maybe_asset) = self.assets.get(specifier) { return Ok(maybe_asset.clone()); } else { - let mut state_snapshot = self.snapshot(); let maybe_asset = - tsc::get_asset(&specifier, &self.ts_server, &mut state_snapshot) - .await?; + tsc::get_asset(&specifier, &self.ts_server, self.snapshot()).await?; self.assets.insert(specifier.clone(), maybe_asset.clone()); Ok(maybe_asset) } @@ -1887,7 +1886,7 @@ impl Inner { } // now that we have dependencies loaded, we need to re-analyze them and // invalidate some diagnostics - if self.documents.contains(&referrer) { + if self.documents.contains_key(&referrer) { if let Some(source) = self.documents.content(&referrer).unwrap() { self.analyze_dependencies(&referrer, &source); } diff --git a/cli/lsp/sources.rs b/cli/lsp/sources.rs index 3298b847bba0c7..62316e4a26e82f 100644 --- a/cli/lsp/sources.rs +++ b/cli/lsp/sources.rs @@ -67,8 +67,8 @@ impl Sources { Self(Arc::new(Mutex::new(Inner::new(location)))) } - pub fn contains(&self, specifier: &ModuleSpecifier) -> bool { - self.0.lock().unwrap().contains(specifier) + pub fn contains_key(&self, specifier: &ModuleSpecifier) -> bool { + self.0.lock().unwrap().contains_key(specifier) } pub fn get_length_utf16(&self, specifier: &ModuleSpecifier) -> Option { @@ -132,7 +132,7 @@ impl Inner { } } - fn contains(&mut self, specifier: &ModuleSpecifier) -> bool { + fn contains_key(&mut self, specifier: &ModuleSpecifier) -> bool { if let Some(specifier) = self.resolve_specifier(specifier) { if self.get_metadata(&specifier).is_some() { return true; diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 9a5b94f85b432a..f3a83d544ceb16 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -89,46 +89,79 @@ impl TsServer { #[derive(Debug, Clone)] pub struct AssetDocument { pub text: String, + pub length: usize, pub line_index: LineIndex, } +impl AssetDocument { + pub fn new>(text: T) -> Self { + let text = text.as_ref(); + Self { + text: text.to_string(), + length: text.encode_utf16().count(), + line_index: LineIndex::new(text), + } + } +} + +#[derive(Debug, Clone)] +pub struct Assets(HashMap>); + +impl Default for Assets { + fn default() -> Self { + let assets = tsc::STATIC_ASSETS + .iter() + .map(|(k, v)| { + let url_str = format!("asset:///{}", k); + let specifier = ModuleSpecifier::resolve_url(&url_str).unwrap(); + let asset = AssetDocument::new(v); + (specifier, Some(asset)) + }) + .collect(); + Self(assets) + } +} + +impl Assets { + pub fn contains_key(&self, k: &ModuleSpecifier) -> bool { + self.0.contains_key(k) + } + + pub fn get(&self, k: &ModuleSpecifier) -> Option<&Option> { + self.0.get(k) + } + + pub fn insert( + &mut self, + k: ModuleSpecifier, + v: Option, + ) -> Option> { + self.0.insert(k, v) + } +} + /// Optionally returns an internal asset, first checking for any static assets /// in Rust, then checking any previously retrieved static assets from the /// isolate, and then finally, the tsc isolate itself. pub async fn get_asset( specifier: &ModuleSpecifier, ts_server: &TsServer, - state_snapshot: &mut StateSnapshot, + state_snapshot: StateSnapshot, ) -> Result, AnyError> { let specifier_str = specifier.to_string().replace("asset:///", ""); if let Some(text) = tsc::get_asset(&specifier_str) { - let maybe_asset = Some(AssetDocument { - line_index: LineIndex::new(text), - text: text.to_string(), - }); - state_snapshot - .assets - .insert(specifier.clone(), maybe_asset.clone()); + let maybe_asset = Some(AssetDocument::new(text)); Ok(maybe_asset) } else { let res = ts_server - .request( - state_snapshot.clone(), - RequestMethod::GetAsset(specifier.clone()), - ) + .request(state_snapshot, RequestMethod::GetAsset(specifier.clone())) .await?; let maybe_text: Option = serde_json::from_value(res)?; let maybe_asset = if let Some(text) = maybe_text { - Some(AssetDocument { - line_index: LineIndex::new(&text), - text, - }) + Some(AssetDocument::new(text)) } else { None }; - state_snapshot - .assets - .insert(specifier.clone(), maybe_asset.clone()); Ok(maybe_asset) } } @@ -1043,7 +1076,9 @@ fn get_length(state: &mut State, args: Value) -> Result { let mark = state.state_snapshot.performance.mark("op_get_length"); let v: SourceSnapshotArgs = serde_json::from_value(args)?; let specifier = ModuleSpecifier::resolve_url(&v.specifier)?; - if state.state_snapshot.documents.contains(&specifier) { + if let Some(Some(asset)) = state.state_snapshot.assets.get(&specifier) { + Ok(json!(asset.length)) + } else if state.state_snapshot.documents.contains_key(&specifier) { cache_snapshot(state, v.specifier.clone(), v.version.clone())?; let content = state .snapshots @@ -1071,17 +1106,20 @@ fn get_text(state: &mut State, args: Value) -> Result { let mark = state.state_snapshot.performance.mark("op_get_text"); let v: GetTextArgs = serde_json::from_value(args)?; let specifier = ModuleSpecifier::resolve_url(&v.specifier)?; - let content = if state.state_snapshot.documents.contains(&specifier) { - cache_snapshot(state, v.specifier.clone(), v.version.clone())?; - state - .snapshots - .get(&(v.specifier.into(), v.version.into())) - .unwrap() - .clone() - } else { - let sources = &mut state.state_snapshot.sources; - sources.get_text(&specifier).unwrap() - }; + let content = + if let Some(Some(content)) = state.state_snapshot.assets.get(&specifier) { + content.text.clone() + } else if state.state_snapshot.documents.contains_key(&specifier) { + cache_snapshot(state, v.specifier.clone(), v.version.clone())?; + state + .snapshots + .get(&(v.specifier.into(), v.version.into())) + .unwrap() + .clone() + } else { + let sources = &mut state.state_snapshot.sources; + sources.get_text(&specifier).unwrap() + }; state.state_snapshot.performance.measure(mark); Ok(json!(text::slice(&content, v.start..v.end))) } @@ -1093,7 +1131,7 @@ fn resolve(state: &mut State, args: Value) -> Result { let referrer = ModuleSpecifier::resolve_url(&v.base)?; let sources = &mut state.state_snapshot.sources; - if state.state_snapshot.documents.contains(&referrer) { + if state.state_snapshot.documents.contains_key(&referrer) { if let Some(dependencies) = state.state_snapshot.documents.dependencies(&referrer) { @@ -1115,13 +1153,17 @@ fn resolve(state: &mut State, args: Value) -> Result { if let ResolvedDependency::Resolved(resolved_specifier) = resolved_import { - if state.state_snapshot.documents.contains(&resolved_specifier) { + if state + .state_snapshot + .documents + .contains_key(&resolved_specifier) + { let media_type = MediaType::from(&resolved_specifier); resolved.push(Some(( resolved_specifier.to_string(), media_type.as_ts_extension(), ))); - } else if sources.contains(&resolved_specifier) { + } else if sources.contains_key(&resolved_specifier) { let media_type = if let Some(media_type) = sources.get_media_type(&resolved_specifier) { @@ -1142,7 +1184,7 @@ fn resolve(state: &mut State, args: Value) -> Result { } } } - } else if sources.contains(&referrer) { + } else if sources.contains_key(&referrer) { for specifier in &v.specifiers { if let Some((resolved_specifier, media_type)) = sources.resolve_import(specifier, &referrer) @@ -1190,7 +1232,15 @@ fn script_version(state: &mut State, args: Value) -> Result { let mark = state.state_snapshot.performance.mark("op_script_version"); let v: ScriptVersionArgs = serde_json::from_value(args)?; let specifier = ModuleSpecifier::resolve_url(&v.specifier)?; - if let Some(version) = state.state_snapshot.documents.version(&specifier) { + if specifier.as_url().scheme() == "asset" { + return if state.state_snapshot.assets.contains_key(&specifier) { + Ok(json!("1")) + } else { + Ok(json!(null)) + }; + } else if let Some(version) = + state.state_snapshot.documents.version(&specifier) + { return Ok(json!(version.to_string())); } else { let sources = &mut state.state_snapshot.sources; @@ -1200,7 +1250,7 @@ fn script_version(state: &mut State, args: Value) -> Result { } state.state_snapshot.performance.measure(mark); - Ok(json!(None::)) + Ok(json!(null)) } #[derive(Debug, Deserialize)] @@ -1626,6 +1676,31 @@ mod tests { ); } + #[test] + fn test_get_diagnostics_lib() { + let (mut runtime, state_snapshot) = setup( + false, + json!({ + "target": "esnext", + "module": "esnext", + "jsx": "react", + "lib": ["esnext", "dom", "deno.ns"], + "noEmit": true, + }), + vec![("file:///a.ts", r#"console.log(document.location);"#, 1)], + ); + let specifier = ModuleSpecifier::resolve_url("file:///a.ts") + .expect("could not resolve url"); + let result = request( + &mut runtime, + state_snapshot, + RequestMethod::GetDiagnostics(vec![specifier]), + ); + assert!(result.is_ok()); + let response = result.unwrap(); + assert_eq!(response, json!({ "file:///a.ts": [] })); + } + #[test] fn test_module_resolution() { let (mut runtime, state_snapshot) = setup( diff --git a/cli/tsc.rs b/cli/tsc.rs index 9e32c19083d856..a8a33ca116ce4a 100644 --- a/cli/tsc.rs +++ b/cli/tsc.rs @@ -44,32 +44,38 @@ pub fn compiler_snapshot() -> Snapshot { Snapshot::Static(COMPILER_SNAPSHOT) } -/// Provide static assets that are not preloaded in the compiler snapshot. +macro_rules! inc { + ($e:expr) => { + include_str!(concat!("dts/", $e)) + }; +} + +lazy_static! { + /// Contains static assets that are not preloaded in the compiler snapshot. + pub(crate) static ref STATIC_ASSETS: HashMap<&'static str, &'static str> = (&[ + ("lib.dom.asynciterable.d.ts", inc!("lib.dom.asynciterable.d.ts")), + ("lib.dom.d.ts", inc!("lib.dom.d.ts")), + ("lib.dom.iterable.d.ts", inc!("lib.dom.iterable.d.ts")), + ("lib.es6.d.ts", inc!("lib.es6.d.ts")), + ("lib.es2016.full.d.ts", inc!("lib.es2016.full.d.ts")), + ("lib.es2017.full.d.ts", inc!("lib.es2017.full.d.ts")), + ("lib.es2018.full.d.ts", inc!("lib.es2018.full.d.ts")), + ("lib.es2019.full.d.ts", inc!("lib.es2019.full.d.ts")), + ("lib.es2020.full.d.ts", inc!("lib.es2020.full.d.ts")), + ("lib.esnext.full.d.ts", inc!("lib.esnext.full.d.ts")), + ("lib.scripthost.d.ts", inc!("lib.scripthost.d.ts")), + ("lib.webworker.d.ts", inc!("lib.webworker.d.ts")), + ("lib.webworker.importscripts.d.ts", inc!("lib.webworker.importscripts.d.ts")), + ("lib.webworker.iterable.d.ts", inc!("lib.webworker.iterable.d.ts")), + ]) + .iter() + .cloned() + .collect(); +} + +/// Retrieve a static asset that are included in the binary. pub fn get_asset(asset: &str) -> Option<&'static str> { - macro_rules! inc { - ($e:expr) => { - Some(include_str!(concat!("dts/", $e))) - }; - } - match asset { - "lib.dom.asynciterable.d.ts" => inc!("lib.dom.asynciterable.d.ts"), - "lib.dom.d.ts" => inc!("lib.dom.d.ts"), - "lib.dom.iterable.d.ts" => inc!("lib.dom.iterable.d.ts"), - "lib.es6.d.ts" => inc!("lib.es6.d.ts"), - "lib.es2016.full.d.ts" => inc!("lib.es2016.full.d.ts"), - "lib.es2017.full.d.ts" => inc!("lib.es2017.full.d.ts"), - "lib.es2018.full.d.ts" => inc!("lib.es2018.full.d.ts"), - "lib.es2019.full.d.ts" => inc!("lib.es2019.full.d.ts"), - "lib.es2020.full.d.ts" => inc!("lib.es2020.full.d.ts"), - "lib.esnext.full.d.ts" => inc!("lib.esnext.full.d.ts"), - "lib.scripthost.d.ts" => inc!("lib.scripthost.d.ts"), - "lib.webworker.d.ts" => inc!("lib.webworker.d.ts"), - "lib.webworker.importscripts.d.ts" => { - inc!("lib.webworker.importscripts.d.ts") - } - "lib.webworker.iterable.d.ts" => inc!("lib.webworker.iterable.d.ts"), - _ => None, - } + STATIC_ASSETS.get(asset).map(|s| s.to_owned()) } fn get_maybe_hash( @@ -294,7 +300,7 @@ fn load(state: &mut State, args: Value) -> Result { Some("declare const __: any;\nexport = __;\n".to_string()) } else if v.specifier.starts_with("asset:///") { let name = v.specifier.replace("asset:///", ""); - let maybe_source = get_asset(&name).map(|s| s.to_string()); + let maybe_source = get_asset(&name).map(String::from); hash = get_maybe_hash(&maybe_source, &state.hash_data); media_type = MediaType::from(&v.specifier); maybe_source