Skip to content

Commit

Permalink
fix(lsp): properly handle static assets (denoland#9476)
Browse files Browse the repository at this point in the history
  • Loading branch information
kitsonk authored Feb 12, 2021
1 parent 54e53cc commit 7991622
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 84 deletions.
10 changes: 5 additions & 5 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand All @@ -52,9 +52,9 @@ lazy_static! {
("spelling", (2, false)),
("addMissingAwait", (1, false)),
("fixImport", (0, true)),
]
])
.iter()
.copied()
.cloned()
.collect();
}

Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl DocumentCache {
version: i32,
content_changes: Vec<TextDocumentContentChangeEvent>,
) -> Result<Option<String>, AnyError> {
if !self.contains(specifier) {
if !self.contains_key(specifier) {
return Err(custom_error(
"NotFound",
format!(
Expand All @@ -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)
}

Expand Down Expand Up @@ -213,8 +213,8 @@ mod tests {
let missing_specifier =
ModuleSpecifier::resolve_url("file:https:///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]
Expand Down
15 changes: 7 additions & 8 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -63,7 +64,7 @@ pub struct LanguageServer(Arc<tokio::sync::Mutex<Inner>>);

#[derive(Debug, Clone, Default)]
pub struct StateSnapshot {
pub assets: HashMap<ModuleSpecifier, Option<AssetDocument>>,
pub assets: Assets,
pub documents: DocumentCache,
pub performance: Performance,
pub sources: Sources,
Expand All @@ -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<ModuleSpecifier, Option<AssetDocument>>,
assets: Assets,
/// The LSP client that this LSP server is connected to.
client: Client,
/// Configuration information.
Expand All @@ -86,7 +87,7 @@ pub(crate) struct Inner {
/// file which will be used by the Deno LSP.
maybe_config_uri: Option<Url>,
/// An optional import map which is used to resolve modules.
pub maybe_import_map: Option<ImportMap>,
pub(crate) maybe_import_map: Option<ImportMap>,
/// The URL for the import map which is used to determine relative imports.
maybe_import_map_uri: Option<Url>,
/// A map of all the cached navigation trees.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions cli/lsp/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize> {
Expand Down Expand Up @@ -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;
Expand Down
149 changes: 112 additions & 37 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: AsRef<str>>(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<ModuleSpecifier, Option<AssetDocument>>);

impl Default for Assets {
fn default() -> Self {
let assets = tsc::STATIC_ASSETS
.iter()
.map(|(k, v)| {
let url_str = format!("asset:https:///{}", 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<AssetDocument>> {
self.0.get(k)
}

pub fn insert(
&mut self,
k: ModuleSpecifier,
v: Option<AssetDocument>,
) -> Option<Option<AssetDocument>> {
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<Option<AssetDocument>, AnyError> {
let specifier_str = specifier.to_string().replace("asset:https:///", "");
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<String> = 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)
}
}
Expand Down Expand Up @@ -1043,7 +1076,9 @@ fn get_length(state: &mut State, args: Value) -> Result<Value, AnyError> {
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
Expand Down Expand Up @@ -1071,17 +1106,20 @@ fn get_text(state: &mut State, args: Value) -> Result<Value, AnyError> {
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)))
}
Expand All @@ -1093,7 +1131,7 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
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)
{
Expand All @@ -1115,13 +1153,17 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
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)
{
Expand All @@ -1142,7 +1184,7 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
}
}
}
} 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)
Expand Down Expand Up @@ -1190,7 +1232,15 @@ fn script_version(state: &mut State, args: Value) -> Result<Value, AnyError> {
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;
Expand All @@ -1200,7 +1250,7 @@ fn script_version(state: &mut State, args: Value) -> Result<Value, AnyError> {
}

state.state_snapshot.performance.measure(mark);
Ok(json!(None::<String>))
Ok(json!(null))
}

#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -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:https:///a.ts", r#"console.log(document.location);"#, 1)],
);
let specifier = ModuleSpecifier::resolve_url("file:https:///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:https:///a.ts": [] }));
}

#[test]
fn test_module_resolution() {
let (mut runtime, state_snapshot) = setup(
Expand Down
Loading

0 comments on commit 7991622

Please sign in to comment.