summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHajime-san <41257923+Hajime-san@users.noreply.github.com>2024-03-28 00:58:18 +0900
committerGitHub <noreply@github.com>2024-03-27 15:58:18 +0000
commitfeb744cebd37263026893c7e7c4852daa5df24d0 (patch)
tree99f8da702a85406acec872494a760c538025354e
parent3462248571fd3193106a0427b3d8f585f9716c48 (diff)
fix(lsp): decoding percent-encoding(non-ASCII) file path correctly (#22582)
-rw-r--r--cli/lsp/diagnostics.rs51
-rw-r--r--cli/lsp/language_server.rs14
-rw-r--r--cli/lsp/tsc.rs45
-rw-r--r--cli/util/path.rs29
-rw-r--r--tests/integration/lsp_tests.rs42
5 files changed, 162 insertions, 19 deletions
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index e3b922ba8..03d962741 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -21,6 +21,7 @@ use crate::graph_util::enhanced_resolution_error_message;
use crate::lsp::lsp_custom::DiagnosticBatchNotificationParams;
use crate::resolver::SloppyImportsResolution;
use crate::resolver::SloppyImportsResolver;
+use crate::util::path::to_percent_decoded_str;
use deno_ast::MediaType;
use deno_core::anyhow::anyhow;
@@ -1212,8 +1213,10 @@ impl DenoDiagnostic {
specifier: &ModuleSpecifier,
sloppy_resolution: SloppyImportsResolution,
) -> String {
- let mut message =
- format!("Unable to load a local module: {}\n", specifier);
+ let mut message = format!(
+ "Unable to load a local module: {}\n",
+ to_percent_decoded_str(specifier.as_ref())
+ );
if let Some(additional_message) =
sloppy_resolution.as_suggestion_message()
{
@@ -1971,6 +1974,50 @@ let c: number = "a";
);
}
+ #[tokio::test]
+ async fn unable_to_load_a_local_module() {
+ let temp_dir = TempDir::new();
+ let (snapshot, _) = setup(
+ &temp_dir,
+ &[(
+ "file:///a.ts",
+ r#"
+ import { 東京 } from "./πŸ¦•.ts";
+ "#,
+ 1,
+ LanguageId::TypeScript,
+ )],
+ None,
+ )
+ .await;
+ let config = mock_config();
+ let token = CancellationToken::new();
+ let actual = generate_deno_diagnostics(&snapshot, &config, token);
+ assert_eq!(actual.len(), 1);
+ let record = actual.first().unwrap();
+ assert_eq!(
+ json!(record.versioned.diagnostics),
+ json!([
+ {
+ "range": {
+ "start": {
+ "line": 1,
+ "character": 27
+ },
+ "end": {
+ "line": 1,
+ "character": 35
+ }
+ },
+ "severity": 1,
+ "code": "no-local",
+ "source": "deno",
+ "message": "Unable to load a local module: file:///πŸ¦•.ts\nPlease check the file path.",
+ }
+ ])
+ );
+ }
+
#[test]
fn test_specifier_text_for_redirected() {
#[track_caller]
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index fafd9fe4c..e7e48a04c 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -122,6 +122,7 @@ use crate::tools::upgrade::upgrade_check_enabled;
use crate::util::fs::remove_dir_all_if_exists;
use crate::util::path::is_importable_ext;
use crate::util::path::specifier_to_file_path;
+use crate::util::path::to_percent_decoded_str;
use crate::util::progress_bar::ProgressBar;
use crate::util::progress_bar::ProgressBarStyle;
@@ -1738,16 +1739,21 @@ impl Inner {
match resolution {
Resolution::Ok(resolved) => {
let specifier = &resolved.specifier;
+ let format = |scheme: &str, rest: &str| -> String {
+ format!("{}&#8203;{}", scheme, rest).replace('@', "&#8203;@")
+ };
match specifier.scheme() {
"data" => "_(a data url)_".to_string(),
"blob" => "_(a blob url)_".to_string(),
+ "file" => format(
+ &specifier[..url::Position::AfterScheme],
+ &to_percent_decoded_str(&specifier[url::Position::AfterScheme..]),
+ ),
_ => {
- let mut result = format!(
- "{}&#8203;{}",
+ let mut result = format(
&specifier[..url::Position::AfterScheme],
&specifier[url::Position::AfterScheme..],
- )
- .replace('@', "&#8203;@");
+ );
if let Ok(jsr_req_ref) =
JsrPackageReqReference::from_specifier(specifier)
{
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index 4eb425c1f..691f7c91d 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -31,6 +31,7 @@ use crate::tsc;
use crate::tsc::ResolveArgs;
use crate::util::path::relative_specifier;
use crate::util::path::specifier_to_file_path;
+use crate::util::path::to_percent_decoded_str;
use dashmap::DashMap;
use deno_ast::MediaType;
@@ -543,6 +544,10 @@ impl TsServer {
.and_then(|mut changes| {
for changes in &mut changes {
changes.normalize(&self.specifier_map)?;
+ for text_changes in &mut changes.text_changes {
+ text_changes.new_text =
+ to_percent_decoded_str(&text_changes.new_text);
+ }
}
Ok(changes)
})
@@ -1611,7 +1616,41 @@ fn display_parts_to_string(
link.name = Some(part.text.clone());
}
}
- _ => out.push(part.text.clone()),
+ _ => out.push(
+ // should decode percent-encoding string when hovering over the right edge of module specifier like below
+ // module "file:///path/to/πŸ¦•"
+ to_percent_decoded_str(&part.text),
+ // NOTE: The reason why an example above that lacks `.ts` extension is caused by the implementation of tsc itself.
+ // The request `tsc.request.getQuickInfoAtPosition` receives the payload from tsc host as follows.
+ // {
+ // text_span: {
+ // start: 19,
+ // length: 9,
+ // },
+ // displayParts:
+ // [
+ // {
+ // text: "module",
+ // kind: "keyword",
+ // target: null,
+ // },
+ // {
+ // text: " ",
+ // kind: "space",
+ // target: null,
+ // },
+ // {
+ // text: "\"file:///path/to/%F0%9F%A6%95\"",
+ // kind: "stringLiteral",
+ // target: null,
+ // },
+ // ],
+ // documentation: [],
+ // tags: null,
+ // }
+ //
+ // related issue: https://github.com/denoland/deno/issues/16058
+ ),
}
}
@@ -5424,7 +5463,7 @@ mod tests {
.get_edits_for_file_rename(
snapshot,
resolve_url("file:///b.ts").unwrap(),
- resolve_url("file:///c.ts").unwrap(),
+ resolve_url("file:///πŸ¦•.ts").unwrap(),
FormatCodeSettings::default(),
UserPreferences::default(),
)
@@ -5439,7 +5478,7 @@ mod tests {
start: 8,
length: 6,
},
- new_text: "./c.ts".to_string(),
+ new_text: "./πŸ¦•.ts".to_string(),
}],
is_new_file: None,
}]
diff --git a/cli/util/path.rs b/cli/util/path.rs
index b64fde6b9..144676c01 100644
--- a/cli/util/path.rs
+++ b/cli/util/path.rs
@@ -156,11 +156,12 @@ pub fn relative_specifier(
text.push('/');
}
- Some(if text.starts_with("../") || text.starts_with("./") {
+ let text = if text.starts_with("../") || text.starts_with("./") {
text
} else {
format!("./{text}")
- })
+ };
+ Some(to_percent_decoded_str(&text))
}
/// Gets a path with the specified file stem suffix.
@@ -265,6 +266,24 @@ pub fn matches_pattern_or_exact_path(
false
}
+/// For decoding percent-encodeing string
+/// could be used for module specifier string literal of local modules,
+/// or local file path to display `non-ASCII` characters correctly
+/// # Examples
+/// ```
+/// use crate::util::path::to_percent_decoded_str;
+///
+/// let str = to_percent_decoded_str("file:///Users/path/to/%F0%9F%A6%95.ts");
+/// assert_eq!(str, "file:///Users/path/to/πŸ¦•.ts");
+/// ```
+pub fn to_percent_decoded_str(s: &str) -> String {
+ match percent_encoding::percent_decode_str(s).decode_utf8() {
+ Ok(s) => s.to_string(),
+ // when failed to decode, return the original string
+ Err(_) => s.to_string(),
+ }
+}
+
#[cfg(test)]
mod test {
use super::*;
@@ -457,4 +476,10 @@ mod test {
PathBuf::from("/test_2.d.cts")
);
}
+
+ #[test]
+ fn test_to_percent_decoded_str() {
+ let str = to_percent_decoded_str("%F0%9F%A6%95");
+ assert_eq!(str, "πŸ¦•");
+ }
}
diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs
index 852f56e9b..5a81244fd 100644
--- a/tests/integration/lsp_tests.rs
+++ b/tests/integration/lsp_tests.rs
@@ -2266,7 +2266,7 @@ fn lsp_hover_dependency() {
"uri": "file:///a/file.ts",
"languageId": "typescript",
"version": 1,
- "text": "import * as a from \"http://127.0.0.1:4545/xTypeScriptTypes.js\";\n// @deno-types=\"http://127.0.0.1:4545/type_definitions/foo.d.ts\"\nimport * as b from \"http://127.0.0.1:4545/type_definitions/foo.js\";\nimport * as c from \"http://127.0.0.1:4545/subdir/type_reference.js\";\nimport * as d from \"http://127.0.0.1:4545/subdir/mod1.ts\";\nimport * as e from \"data:application/typescript;base64,ZXhwb3J0IGNvbnN0IGEgPSAiYSI7CgpleHBvcnQgZW51bSBBIHsKICBBLAogIEIsCiAgQywKfQo=\";\nimport * as f from \"./file_01.ts\";\nimport * as g from \"http://localhost:4545/x/a/mod.ts\";\n\nconsole.log(a, b, c, d, e, f, g);\n"
+ "text": "import * as a from \"http://127.0.0.1:4545/xTypeScriptTypes.js\";\n// @deno-types=\"http://127.0.0.1:4545/type_definitions/foo.d.ts\"\nimport * as b from \"http://127.0.0.1:4545/type_definitions/foo.js\";\nimport * as c from \"http://127.0.0.1:4545/subdir/type_reference.js\";\nimport * as d from \"http://127.0.0.1:4545/subdir/mod1.ts\";\nimport * as e from \"data:application/typescript;base64,ZXhwb3J0IGNvbnN0IGEgPSAiYSI7CgpleHBvcnQgZW51bSBBIHsKICBBLAogIEIsCiAgQywKfQo=\";\nimport * as f from \"./file_01.ts\";\nimport * as g from \"http://localhost:4545/x/a/mod.ts\";\nimport * as h from \"./modπŸ¦•.ts\";\n\nconsole.log(a, b, c, d, e, f, g, h);\n"
}
}),
);
@@ -2387,6 +2387,28 @@ fn lsp_hover_dependency() {
}
})
);
+ let res = client.write_request(
+ "textDocument/hover",
+ json!({
+ "textDocument": {
+ "uri": "file:///a/file.ts",
+ },
+ "position": { "line": 8, "character": 28 }
+ }),
+ );
+ assert_eq!(
+ res,
+ json!({
+ "contents": {
+ "kind": "markdown",
+ "value": "**Resolved Dependency**\n\n**Code**: file&#8203;:///a/modπŸ¦•.ts\n"
+ },
+ "range": {
+ "start": { "line": 8, "character": 19 },
+ "end":{ "line": 8, "character": 30 }
+ }
+ })
+ );
}
// This tests for a regression covered by denoland/deno#12753 where the lsp was
@@ -6637,7 +6659,7 @@ fn lsp_completions_auto_import() {
client.initialize_default();
client.did_open(json!({
"textDocument": {
- "uri": "file:///a/b.ts",
+ "uri": "file:///a/πŸ¦•.ts",
"languageId": "typescript",
"version": 1,
"text": "export const foo = \"foo\";\n",
@@ -6668,7 +6690,7 @@ fn lsp_completions_auto_import() {
let req = json!({
"label": "foo",
"labelDetails": {
- "description": "./b.ts",
+ "description": "./πŸ¦•.ts",
},
"kind": 6,
"sortText": "οΏΏ16_0",
@@ -6683,12 +6705,16 @@ fn lsp_completions_auto_import() {
"specifier": "file:///a/file.ts",
"position": 12,
"name": "foo",
- "source": "./b.ts",
+ "source": "./%F0%9F%A6%95.ts",
+ "specifierRewrite": [
+ "./%F0%9F%A6%95.ts",
+ "./πŸ¦•.ts",
+ ],
"data": {
"exportName": "foo",
"exportMapKey": "",
- "moduleSpecifier": "./b.ts",
- "fileName": "file:///a/b.ts"
+ "moduleSpecifier": "./%F0%9F%A6%95.ts",
+ "fileName": "file:///a/%F0%9F%A6%95.ts"
},
"useCodeSnippet": false
}
@@ -6702,7 +6728,7 @@ fn lsp_completions_auto_import() {
json!({
"label": "foo",
"labelDetails": {
- "description": "./b.ts",
+ "description": "./πŸ¦•.ts",
},
"kind": 6,
"detail": "const foo: \"foo\"",
@@ -6717,7 +6743,7 @@ fn lsp_completions_auto_import() {
"start": { "line": 0, "character": 0 },
"end": { "line": 0, "character": 0 }
},
- "newText": "import { foo } from \"./b.ts\";\n\n"
+ "newText": "import { foo } from \"./πŸ¦•.ts\";\n\n"
}
]
})