foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-24 15:19:26 -05:00
Code Issues Packages 1 Wiki Activity

fix(lsp): decoding percent-encoding(non-ASCII) file path correctly (#22582)

Browse source
This commit is contained in:
Hajime-san 2024-03-28 00:58:18 +09:00 committed by GitHub
parent 3462248571
commit feb744cebd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 162 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
cli/lsp/diagnostics.rs
View file

@ -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]

14
cli/lsp/language_server.rs
View file

@ -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!("{}​{}", scheme, rest).replace('@', "​@")
};
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!(
"{}​{}",
let mut result = format(
&specifier[..url::Position::AfterScheme],
&specifier[url::Position::AfterScheme..],
)
.replace('@', "​@");
);
if let Ok(jsr_req_ref) =
JsrPackageReqReference::from_specifier(specifier)
{

45
cli/lsp/tsc.rs
View file

@ -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,
}]

29
cli/util/path.rs
View file

@ -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, "🦕");
}
}

42
tests/integration/lsp_tests.rs
View file

@ -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​:///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"
}
]
})