From 0ff8e2cdb6e3054eedb694ce61f4ef12d51a1882 Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Mon, 9 Oct 2023 10:33:41 +0200 Subject: [PATCH] compiler: Rework path handling Add some code to do platform-independent path processing. This is necessary aas WASM does e.g. not have any absolute paths and such and the compiler tended to produce wrong results in that case. Side-effect: We no longer need to depend on `dunce` --- internal/compiler/Cargo.toml | 1 - internal/compiler/fileaccess.rs | 10 +- internal/compiler/lib.rs | 1 + internal/compiler/parser.rs | 5 +- internal/compiler/passes/resolving.rs | 2 +- internal/compiler/pathutils.rs | 544 ++++++++++++++++++++++++ internal/compiler/tests/syntax_tests.rs | 4 +- internal/compiler/typeloader.rs | 118 +++-- tools/lsp/Cargo.toml | 1 - tools/lsp/language.rs | 5 +- tools/lsp/language/completion.rs | 2 +- tools/lsp/language/goto.rs | 3 +- 12 files changed, 611 insertions(+), 85 deletions(-) create mode 100644 internal/compiler/pathutils.rs diff --git a/internal/compiler/Cargo.toml b/internal/compiler/Cargo.toml index af0a25659..cace2bc7e 100644 --- a/internal/compiler/Cargo.toml +++ b/internal/compiler/Cargo.toml @@ -52,7 +52,6 @@ css-color-parser2 = "1.0.1" itertools = "0.11" once_cell = "1" url = "2.2.1" -dunce = "1.0.1" linked_hash_set = "0.1.4" # for processing and embedding the rendered image (texture) diff --git a/internal/compiler/fileaccess.rs b/internal/compiler/fileaccess.rs index 47e511e6f..4393e888b 100644 --- a/internal/compiler/fileaccess.rs +++ b/internal/compiler/fileaccess.rs @@ -30,7 +30,7 @@ pub fn load_file(path: &std::path::Path) -> Option { match path.strip_prefix("builtin:/") { Ok(builtin_path) => builtin_library::load_builtin_file(builtin_path), Err(_) => path.exists().then(|| VirtualFile { - canon_path: dunce::canonicalize(path).unwrap_or_else(|_| path.into()), + canon_path: crate::pathutils::clean_path(path), builtin_contents: None, }), } @@ -75,9 +75,11 @@ mod builtin_library { library.iter().find_map(|builtin_file| { if builtin_file.path == file { Some(VirtualFile { - canon_path: ["builtin:/", folder.to_str().unwrap(), builtin_file.path] - .iter() - .collect::(), + canon_path: std::path::PathBuf::from(format!( + "builtin:/{}/{}", + folder.to_str().unwrap(), + builtin_file.path + )), builtin_contents: Some(builtin_file.contents), }) } else { diff --git a/internal/compiler/lib.rs b/internal/compiler/lib.rs index 6668f3eb0..f9590833a 100644 --- a/internal/compiler/lib.rs +++ b/internal/compiler/lib.rs @@ -30,6 +30,7 @@ pub mod lookup; pub mod namedreference; pub mod object_tree; pub mod parser; +pub mod pathutils; pub mod typeloader; pub mod typeregister; diff --git a/internal/compiler/parser.rs b/internal/compiler/parser.rs index 030b50669..a5eaffb79 100644 --- a/internal/compiler/parser.rs +++ b/internal/compiler/parser.rs @@ -989,7 +989,7 @@ pub fn parse( ) -> SyntaxNode { let mut p = DefaultParser::new(&source, build_diagnostics); p.source_file = std::rc::Rc::new(crate::diagnostics::SourceFileInner::new( - path.map(|p| p.to_path_buf()).unwrap_or_default(), + path.map(|p| crate::pathutils::clean_path(p)).unwrap_or_default(), source, )); document::parse_document(&mut p); @@ -1003,7 +1003,8 @@ pub fn parse_file>( path: P, build_diagnostics: &mut BuildDiagnostics, ) -> Option { - let source = crate::diagnostics::load_from_path(path.as_ref()) + let path = crate::pathutils::clean_path(path.as_ref()); + let source = crate::diagnostics::load_from_path(&path) .map_err(|d| build_diagnostics.push_internal_error(d)) .ok()?; Some(parse(source, Some(path.as_ref()), build_diagnostics)) diff --git a/internal/compiler/passes/resolving.rs b/internal/compiler/passes/resolving.rs index 7cff37c13..c157ae5ce 100644 --- a/internal/compiler/passes/resolving.rs +++ b/internal/compiler/passes/resolving.rs @@ -337,7 +337,7 @@ impl Expression { let absolute_source_path = { let path = std::path::Path::new(&s); - if path.is_absolute() || s.starts_with("http://") || s.starts_with("https://") { + if crate::pathutils::is_absolute(path) { s } else { ctx.type_loader diff --git a/internal/compiler/pathutils.rs b/internal/compiler/pathutils.rs new file mode 100644 index 000000000..f4b0460c1 --- /dev/null +++ b/internal/compiler/pathutils.rs @@ -0,0 +1,544 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial + +//! Reimplement some Path handling code: The one in `std` is not available +//! when running in WASM! +//! +//! This is not helped by us using URLs in place of paths *sometimes*. + +use std::path::{Path, PathBuf}; + +/// Convert a `Path` to an `url::Url` if possible +fn to_url(path: &str) -> Option { + let Ok(url) = url::Url::parse(path) else { + return None; + }; + if url.scheme().len() == 1 { + // "c:/" is a path, not a URL + None + } else { + Some(url) + } +} + +#[test] +fn test_to_url() { + #[track_caller] + fn th(input: &str, expected: bool) { + assert_eq!(to_url(&input).is_some(), expected); + } + + th("https://foo.bar/", true); + th("builtin:/foo/bar/", true); + th("user://foo/bar.rs", true); + th("/foo/bar/", false); + th("../foo/bar", false); + th("foo/bar", false); + th("./foo/bar", false); + // Windows style paths: + th("C:\\Documents\\Newsletters\\Summer2018.pdf", false); + th("\\Program Files\\Custom Utilities\\StringFinder.exe", false); + th("2018\\January.xlsx", false); + th("..\\Publications\\TravelBrochure.pdf", false); + th("C:\\Projects\\library\\library.sln", false); + th("C:Projects\\library\\library.sln", false); + th("\\\\system07\\C$\\", false); + th("\\\\Server2\\Share\\Test\\Foo.txt", false); + th("\\\\.\\C:\\Test\\Foo.txt", false); + th("\\\\?\\C:\\Test\\Foo.txt", false); + th("\\\\.\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\Test\\Foo.txt", false); + th("\\\\?\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\Test\\Foo.txt", false); + // Windows style paths - some programs will helpfully convert the backslashes:-(: + th("C:/Documents/Newsletters/Summer2018.pdf", false); + th("/Program Files/Custom Utilities/StringFinder.exe", false); + th("2018/January.xlsx", false); + th("../Publications/TravelBrochure.pdf", false); + th("C:/Projects/library/library.sln", false); + th("C:Projects/library/library.sln", false); + th("//system07/C$/", false); + th("//Server2/Share/Test/Foo.txt", false); + th("//./C:/Test/Foo.txt", false); + th("//?/C:/Test/Foo.txt", false); + th("//./Volume{b75e2c83-0000-0000-0000-602f00000000}/Test/Foo.txt", false); + th("//?/Volume{b75e2c83-0000-0000-0000-602f00000000}/Test/Foo.txt", false); + // Some corner case: + th("C:///Documents/Newsletters/Summer2018.pdf", false); + th("/http://foo/bar/", false); + th("../http://foo/bar", false); + th("foo/http://foo/bar", false); + th("./http://foo/bar", false); + th("", false); +} + +// Check whether a path is absolute. +// +// This returns true is the Path contains a `url::Url` or starts with anything +// that is a root prefix (e.g. `/` on Unix or `C:\` on Windows). +pub fn is_absolute(path: &Path) -> bool { + let Some(path) = path.to_str() else { + // URLs can always convert to string in Rust + return false; + }; + + if to_url(path).is_some() { + return true; + } + + matches!(components(path, 0, &None), Some((PathComponent::RootComponent(_), _, _))) +} + +#[test] +fn test_is_absolute() { + #[track_caller] + fn th(input: &str, expected: bool) { + let path = PathBuf::from(input); + assert_eq!(is_absolute(&path), expected); + } + + th("https://foo.bar/", true); + th("builtin:/foo/bar/", true); + th("user://foo/bar.rs", true); + th("/foo/bar/", true); + th("../foo/bar", false); + th("foo/bar", false); + th("./foo/bar", false); + // Windows style paths: + th("C:\\Documents\\Newsletters\\Summer2018.pdf", true); + // Windows actually considers this to be a relative path: + th("\\Program Files\\Custom Utilities\\StringFinder.exe", true); + th("2018\\January.xlsx", false); + th("..\\Publications\\TravelBrochure.pdf", false); + th("C:\\Projects\\library\\library.sln", true); + th("C:Projects\\library\\library.sln", false); + th("\\\\system07\\C$\\", true); + th("\\\\Server2\\Share\\Test\\Foo.txt", true); + th("\\\\.\\C:\\Test\\Foo.txt", true); + th("\\\\?\\C:\\Test\\Foo.txt", true); + th("\\\\.\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\Test\\Foo.txt", true); + th("\\\\?\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\Test\\Foo.txt", true); + // Windows style paths - some programs will helpfully convert the backslashes:-(: + th("C:/Documents/Newsletters/Summer2018.pdf", true); + // Windows actually considers this to be a relative path: + th("/Program Files/Custom Utilities/StringFinder.exe", true); + th("2018/January.xlsx", false); + th("../Publications/TravelBrochure.pdf", false); + th("C:/Projects/library/library.sln", true); + th("C:Projects/library/library.sln", false); + // These are true, but only because '/' is root on Unix! + th("//system07/C$/", true); + th("//Server2/Share/Test/Foo.txt", true); + th("//./C:/Test/Foo.txt", true); + th("//?/C:/Test/Foo.txt", true); + th("//./Volume{b75e2c83-0000-0000-0000-602f00000000}/Test/Foo.txt", true); + th("//?/Volume{b75e2c83-0000-0000-0000-602f00000000}/Test/Foo.txt", true); + // Some corner case: + th("C:///Documents/Newsletters/Summer2018.pdf", true); + th("C:", false); + th("C:\\", true); + th("C:/", true); + th("", false); +} + +#[derive(Debug, PartialEq)] +enum PathComponent<'a> { + RootComponent(&'a str), + EmptyComponent, + SameDirectoryComponent(&'a str), + ParentDirectoryComponent(&'a str), + DirectoryComponent(&'a str), + FileComponent(&'a str), +} + +/// Find which kind of path separator is used in the `str` +fn find_path_separator(path: &str) -> char { + for c in path.chars() { + if c == '/' || c == '\\' { + return c; + } + } + '/' +} + +/// Look at the individual parts of a `path`. +/// +/// This will not work well with URLs, check whether something is an URL first! +fn components<'a>( + path: &'a str, + offset: usize, + separator: &Option, +) -> Option<(PathComponent<'a>, usize, char)> { + use PathComponent as PC; + + if offset >= path.len() { + return None; + } + + let b = path.as_bytes(); + + if offset == 0 { + if b.len() >= 3 { + if ((b'a'..=b'z').contains(&b[0]) || (b'A'..=b'Z').contains(&b[0])) + && b[1] == b':' + && (b[2] == b'\\' || b[2] == b'/') + { + return Some((PC::RootComponent(&path[0..3]), 3, b[2] as char)); + } + } + if b.len() >= 2 { + if b[0] == b'\\' && b[1] == b'\\' { + let second_bs = path[2..] + .find('\\') + .map(|pos| pos + 2 + 1) + .and_then(|pos1| path[pos1..].find('\\').map(|pos2| pos2 + pos1)); + if let Some(end_offset) = second_bs { + return Some((PC::RootComponent(&path[0..=end_offset]), end_offset + 1, '\\')); + } + } + } + if b[0] == b'/' || b[0] == b'\\' { + return Some((PC::RootComponent(&path[0..1]), 1, b[0] as char)); + } + } + + let separator = separator.unwrap_or_else(|| find_path_separator(path)); + + let next_component = path[offset..].find(separator).map(|p| p + offset).unwrap_or(path.len()); + if &path[offset..next_component] == "." { + return Some(( + PC::SameDirectoryComponent(&path[offset..next_component]), + next_component + 1, + separator, + )); + } + if &path[offset..next_component] == ".." { + return Some(( + PC::ParentDirectoryComponent(&path[offset..next_component]), + next_component + 1, + separator, + )); + } + + if next_component == path.len() { + Some((PC::FileComponent(&path[offset..next_component]), next_component, separator)) + } else { + if next_component == offset { + Some((PC::EmptyComponent, next_component + 1, separator)) + } else { + Some(( + PC::DirectoryComponent(&path[offset..next_component]), + next_component + 1, + separator, + )) + } + } +} + +#[test] +fn test_components() { + use PathComponent as PC; + + #[track_caller] + fn th(input: &str, expected: Option<(PathComponent, usize, char)>) { + assert_eq!(components(input, 0, &None), expected); + } + + th("/foo/bar/", Some((PC::RootComponent("/"), 1, '/'))); + th("../foo/bar", Some((PC::ParentDirectoryComponent(".."), 3, '/'))); + th("foo/bar", Some((PC::DirectoryComponent("foo"), 4, '/'))); + th("./foo/bar", Some((PC::SameDirectoryComponent("."), 2, '/'))); + // Windows style paths: + th("C:\\Documents\\Newsletters\\Summer2018.pdf", Some((PC::RootComponent("C:\\"), 3, '\\'))); + // Windows actually considers this to be a relative path: + th( + "\\Program Files\\Custom Utilities\\StringFinder.exe", + Some((PC::RootComponent("\\"), 1, '\\')), + ); + th("2018\\January.xlsx", Some((PC::DirectoryComponent("2018"), 5, '\\'))); + th("..\\Publications\\TravelBrochure.pdf", Some((PC::ParentDirectoryComponent(".."), 3, '\\'))); + // TODO: This is wrong, but we are unlikely to need it:-) + th("C:Projects\\library\\library.sln", Some((PC::DirectoryComponent("C:Projects"), 11, '\\'))); + th("\\\\system07\\C$\\", Some((PC::RootComponent("\\\\system07\\C$\\"), 14, '\\'))); + th( + "\\\\Server2\\Share\\Test\\Foo.txt", + Some((PC::RootComponent("\\\\Server2\\Share\\"), 16, '\\')), + ); + th("\\\\.\\C:\\Test\\Foo.txt", Some((PC::RootComponent("\\\\.\\C:\\"), 7, '\\'))); + th("\\\\?\\C:\\Test\\Foo.txt", Some((PC::RootComponent("\\\\?\\C:\\"), 7, '\\'))); + th( + "\\\\.\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\Test\\Foo.txt", + Some(( + PC::RootComponent("\\\\.\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\"), + 49, + '\\', + )), + ); + th( + "\\\\?\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\Test\\Foo.txt", + Some(( + PC::RootComponent("\\\\?\\Volume{b75e2c83-0000-0000-0000-602f00000000}\\"), + 49, + '\\', + )), + ); + // Windows style paths - some programs will helpfully convert the backslashes:-(: + th("C:/Documents/Newsletters/Summer2018.pdf", Some((PC::RootComponent("C:/"), 3, '/'))); + // TODO: All the following are wrong, but unlikely to bother us! + th("/Program Files/Custom Utilities/StringFinder.exe", Some((PC::RootComponent("/"), 1, '/'))); + th("//system07/C$/", Some((PC::RootComponent("/"), 1, '/'))); + th("//Server2/Share/Test/Foo.txt", Some((PC::RootComponent("/"), 1, '/'))); + th("//./C:/Test/Foo.txt", Some((PC::RootComponent("/"), 1, '/'))); + th("//?/C:/Test/Foo.txt", Some((PC::RootComponent("/"), 1, '/'))); + th( + "//./Volume{b75e2c83-0000-0000-0000-602f00000000}/Test/Foo.txt", + Some((PC::RootComponent("/"), 1, '/')), + ); + th( + "//?/Volume{b75e2c83-0000-0000-0000-602f00000000}/Test/Foo.txt", + Some((PC::RootComponent("/"), 1, '/')), + ); + // // Some corner case: + // th("C:///Documents/Newsletters/Summer2018.pdf", true); + // TODO: This is wrong, but unlikely to be needed + th("C:", Some((PC::FileComponent("C:"), 2, '/'))); + th("foo", Some((PC::FileComponent("foo"), 3, '/'))); + th("foo/", Some((PC::DirectoryComponent("foo"), 4, '/'))); + th("foo\\", Some((PC::DirectoryComponent("foo"), 4, '\\'))); + th("", None); +} + +struct Components<'a> { + path: &'a str, + offset: usize, + separator: Option, +} + +fn component_iter<'a>(path: &'a str) -> Components<'a> { + Components { path, offset: 0, separator: None } +} + +impl<'a> Iterator for Components<'a> { + type Item = PathComponent<'a>; + + fn next(&mut self) -> Option { + let Some((result, new_offset, separator)) = + components(&self.path, self.offset, &self.separator) + else { + return None; + }; + self.offset = new_offset; + self.separator = Some(separator); + + Some(result) + } +} + +fn clean_path_string(path: &str) -> String { + use PathComponent as PC; + + let separator = find_path_separator(path); + let path = if separator == '\\' { + path.replace('/', &format!("{separator}")) + } else { + path.replace('\\', "/") + }; + + let mut clean_components = Vec::new(); + let mut iter = component_iter(&path); + + while let Some(component) = iter.next() { + match component { + PC::RootComponent(v) => { + clean_components = vec![PC::RootComponent(v)]; + } + PC::EmptyComponent | PC::SameDirectoryComponent(_) => { /* nothing to do */ } + PC::ParentDirectoryComponent(v) => { + match clean_components.last() { + Some(PC::DirectoryComponent(_)) => { + clean_components.pop(); + } + Some(PC::FileComponent(_)) => unreachable!("Must be the last component"), + Some(PC::SameDirectoryComponent(_) | PC::EmptyComponent) => { + unreachable!("Will never be in a the vector") + } + Some(PC::ParentDirectoryComponent(_)) => { + clean_components.push(PC::ParentDirectoryComponent(v)); + } + Some(PC::RootComponent(_)) => { /* do nothing */ } + None => { + clean_components.push(PC::ParentDirectoryComponent(v)); + } + }; + } + PC::DirectoryComponent(v) => clean_components.push(PC::DirectoryComponent(v)), + PC::FileComponent(v) => clean_components.push(PC::FileComponent(v)), + } + } + if clean_components.is_empty() { + ".".to_string() + } else { + let mut result = String::new(); + for c in clean_components { + match c { + PC::RootComponent(v) => { + result = v.to_string(); + } + PC::EmptyComponent | PC::SameDirectoryComponent(_) => { + unreachable!("Never in the vector!") + } + PC::ParentDirectoryComponent(v) => { + result += &format!("{v}{separator}"); + } + PC::DirectoryComponent(v) => result += &format!("{v}{separator}"), + PC::FileComponent(v) => { + result += v; + } + } + } + result + } +} + +#[test] +fn test_clean_path_string() { + #[track_caller] + fn th(input: &str, expected: &str) { + let result = clean_path_string(input); + assert_eq!(result, expected); + } + + th("../../ab/.././hello.txt", "../../hello.txt"); + th("/../../ab/.././hello.txt", "/hello.txt"); + th("ab/.././cb/././///./..", "."); + th("ab/.././cb/.\\.\\\\\\\\./..", "."); + th("ab\\..\\.\\cb\\././///./..", "."); +} + +/// Return a clean up path without unnecessary `.` and `..` directories in it. +/// +/// This will *not* look at the file system, so symlinks will not get resolved. +pub fn clean_path(path: &Path) -> PathBuf { + let Some(path_str) = path.to_str() else { + return path.to_owned(); + }; + + if let Some(url) = to_url(path_str) { + // URL is cleaned up while parsing! + PathBuf::from(url.to_string()) + } else { + PathBuf::from(clean_path_string(path_str)) + } +} + +fn dirname_string(path: &str) -> String { + let separator = find_path_separator(path); + let mut iter = component_iter(path); + let mut result = String::new(); + + while let Some(component) = iter.next() { + match component { + PathComponent::RootComponent(v) => result = v.to_string(), + PathComponent::EmptyComponent => result.push(separator), + PathComponent::SameDirectoryComponent(v) + | PathComponent::ParentDirectoryComponent(v) + | PathComponent::DirectoryComponent(v) => result += &format!("{v}{separator}"), + PathComponent::FileComponent(_) => { /* nothing to do */ } + }; + } + + if result.is_empty() { + String::from(".") + } else { + result + } +} + +#[test] +fn test_dirname() { + #[track_caller] + fn th(input: &str, expected: &str) { + let result = dirname_string(&input); + assert_eq!(result, expected); + } + + th("/../../ab/.././", "/../../ab/.././"); + th("ab/.././cb/./././..", "ab/.././cb/./././../"); + th("hello.txt", "."); + th("../hello.txt", "../"); + th("/hello.txt", "/"); +} + +/// Return the part of `Path` before the last path separator. +pub fn dirname(path: &Path) -> PathBuf { + let Some(path_str) = path.to_str() else { + return path.to_owned(); + }; + + PathBuf::from(dirname_string(path_str)) +} + +/// Join a `path` to a `base_path`, handling URLs in both, matching up +/// path separators, etc. +/// +/// The result will be a `clean_path(...)`. +pub fn join(base: &Path, path: &Path) -> Option { + if is_absolute(path) { + return Some(path.to_owned()); + } + + let Some(base_str) = base.to_str() else { + return Some(path.to_owned()); + }; + let Some(path_str) = path.to_str() else { + return Some(path.to_owned()); + }; + + let path_separator = find_path_separator(path_str); + + if let Some(mut base_url) = to_url(base_str) { + let path_str = if path_separator != '/' { + path_str.replace(path_separator, "/") + } else { + path_str.to_string() + }; + + let base_path = base_url.path(); + if !base_path.is_empty() && !base_path.ends_with('/') { + base_url.set_path(&format!("{base_path}/")); + } + + Some(PathBuf::from(base_url.join(&path_str).ok()?.to_string())) + } else { + let base_separator = find_path_separator(base_str); + let path_str = if path_separator != base_separator { + path_str.replace(path_separator, &base_separator.to_string()) + } else { + path_str.to_string() + }; + let joined = clean_path_string(&format!("{base_str}{base_separator}{path_str}")); + Some(PathBuf::from(joined)) + } +} + +#[test] +fn test_join() { + #[track_caller] + fn th(base: &str, path: &str, expected: Option<&str>) { + let base = PathBuf::from(base); + let path = PathBuf::from(path); + let expected = expected.map(|e| PathBuf::from(e)); + + let result = join(&base, &path); + assert_eq!(result, expected); + } + + th("https://slint.dev/", "/hello.txt", Some("/hello.txt")); + th("https://slint.dev/", "../../hello.txt", Some("https://slint.dev/hello.txt")); + th("/../../ab/.././", "hello.txt", Some("/hello.txt")); + th("ab/.././cb/./././..", "../.././hello.txt", Some("../../hello.txt")); + th("builtin:/foo", "..\\bar.slint", Some("builtin:/bar.slint")); + th("builtin:/", "..\\bar.slint", Some("builtin:/bar.slint")); + th("builtin:/foo/baz", "..\\bar.slint", Some("builtin:/foo/bar.slint")); + th("builtin:/foo", "bar.slint", Some("builtin:/foo/bar.slint")); + th("builtin:/foo/", "bar.slint", Some("builtin:/foo/bar.slint")); + th("builtin:/", "..\\bar.slint", Some("builtin:/bar.slint")); +} diff --git a/internal/compiler/tests/syntax_tests.rs b/internal/compiler/tests/syntax_tests.rs index 6bca630f7..fc257c340 100644 --- a/internal/compiler/tests/syntax_tests.rs +++ b/internal/compiler/tests/syntax_tests.rs @@ -78,7 +78,7 @@ fn process_diagnostics( compile_diagnostics: &i_slint_compiler::diagnostics::BuildDiagnostics, path: &Path, source: &str, - silent: bool, + _silent: bool, ) -> std::io::Result { let mut success = true; @@ -159,7 +159,7 @@ fn process_diagnostics( println!("{:?}: Unexpected errors/warnings: {:#?}", path, diags); #[cfg(feature = "display-diagnostics")] - if !silent { + if !_silent { let mut to_report = i_slint_compiler::diagnostics::BuildDiagnostics::default(); for d in diags { to_report.push_compiler_error(d.clone()); diff --git a/internal/compiler/typeloader.rs b/internal/compiler/typeloader.rs index 571fa2d76..b1d1f4b72 100644 --- a/internal/compiler/typeloader.rs +++ b/internal/compiler/typeloader.rs @@ -265,20 +265,12 @@ impl TypeLoader { || { referencing_file_or_url .and_then(|base_path_or_url| { - let base_path_or_url_str = base_path_or_url.to_string_lossy(); - if base_path_or_url_str.contains("://") { - url::Url::parse(&base_path_or_url_str).ok().and_then(|base_url| { - base_url - .join(maybe_relative_path_or_url) - .ok() - .map(|url| url.to_string().into()) - }) - } else { - base_path_or_url.parent().and_then(|base_dir| { - dunce::canonicalize(base_dir.join(maybe_relative_path_or_url)).ok() - }) - } + crate::pathutils::join( + &crate::pathutils::dirname(base_path_or_url), + &PathBuf::from(maybe_relative_path_or_url), + ) }) + .filter(|p| p.exists()) .map(|p| (p, None)) }, ) @@ -297,8 +289,9 @@ impl TypeLoader { .resolve_import_path(import_token.as_ref(), file_to_import) { Some(x) => x, - None => match dunce::canonicalize(file_to_import) { - Ok(path) => { + None => { + let import_path = crate::pathutils::clean_path(&Path::new(file_to_import)); + if import_path.exists() { if import_token.as_ref().and_then(|x| x.source_file()).is_some() { borrowed_state.diag.push_warning( format!( @@ -307,42 +300,23 @@ impl TypeLoader { &import_token, ); } - (path, None) - } - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + (import_path, None) + } else { // We will load using the `open_import_fallback` // Simplify the path to remove the ".." - let mut path = import_token + let base_path = import_token .as_ref() .and_then(|tok| tok.source_file().map(|s| s.path())) - .and_then(|p| p.parent()) .map_or(PathBuf::new(), |p| p.into()); - for c in Path::new(file_to_import).components() { - use std::path::Component::*; - match c { - RootDir => path = PathBuf::new(), - CurDir => {} - ParentDir - if matches!( - path.components().last(), - Some(Normal(_) | RootDir) - ) => - { - path.pop(); - } - Prefix(_) | ParentDir | Normal(_) => path.push(c), - } - } + let Some(path) = crate::pathutils::join( + &crate::pathutils::dirname(&base_path), + Path::new(file_to_import), + ) else { + return None; + }; (path, None) } - Err(err) => { - borrowed_state.diag.push_error( - format!("Error reading requested import \"{file_to_import}\": {err}",), - &import_token, - ); - return None; - } - }, + } }; if !import_stack.insert(path_canon.clone()) { @@ -560,26 +534,23 @@ impl TypeLoader { file_to_import: &str, ) -> Option<(PathBuf, Option<&'static [u8]>)> { // The directory of the current file is the first in the list of include directories. - let maybe_current_directory = referencing_file.and_then(base_directory); - maybe_current_directory - .clone() + referencing_file + .map(base_directory) .into_iter() - .chain(self.compiler_config.include_paths.iter().map(PathBuf::as_path).map({ + .chain(self.compiler_config.include_paths.iter().map(PathBuf::as_path).map( |include_path| { - if include_path.is_relative() && maybe_current_directory.as_ref().is_some() { - maybe_current_directory.as_ref().unwrap().join(include_path) - } else { - include_path.to_path_buf() - } - } - })) + let base = referencing_file.map(Path::to_path_buf).unwrap_or_default(); + crate::pathutils::join(&crate::pathutils::dirname(&base), include_path) + .unwrap_or_else(|| include_path.to_path_buf()) + }, + )) .chain( (file_to_import == "std-widgets.slint" || referencing_file.map_or(false, |x| x.starts_with("builtin:/"))) .then(|| format!("builtin:/{}", self.style).into()), ) .find_map(|include_dir| { - let candidate = include_dir.join(file_to_import); + let candidate = crate::pathutils::join(&include_dir, &Path::new(file_to_import))?; crate::fileaccess::load_file(&candidate) .map(|virtual_file| (virtual_file.canon_path, virtual_file.builtin_contents)) }) @@ -630,10 +601,8 @@ impl TypeLoader { /// Return a document if it was already loaded pub fn get_document<'b>(&'b self, path: &Path) -> Option<&'b object_tree::Document> { - dunce::canonicalize(path).map_or_else( - |_| self.all_documents.docs.get(path), - |path| self.all_documents.docs.get(&path), - ) + let path = crate::pathutils::clean_path(path); + self.all_documents.docs.get(&path) } /// Return an iterator over all the loaded file path @@ -654,7 +623,7 @@ fn get_native_style(all_loaded_files: &mut Vec) -> String { let target_path = std::env::var_os("OUT_DIR") .and_then(|path| { // Same logic as in i-slint-backend-selector's build script to get the path - Path::new(&path).parent()?.parent()?.join("SLINT_DEFAULT_STYLE.txt").into() + crate::pathutils::join(&Path::new(&path), &Path::new("../../SLINT_DEFAULT_STYLE.txt")) }) .or_else(|| { // When we are called from a slint!, OUT_DIR is only defined when the crate having the macro has a build.rs script. @@ -668,7 +637,12 @@ fn get_native_style(all_loaded_files: &mut Vec) -> String { break; } } - Path::new(&out_dir?).parent()?.join("build/SLINT_DEFAULT_STYLE.txt").into() + out_dir.and_then(|od| { + crate::pathutils::join( + &Path::new(&od), + &Path::new("../build/SLINT_DEFAULT_STYLE.txt"), + ) + }) }); if let Some(style) = target_path.and_then(|target_path| { all_loaded_files.push(target_path.clone()); @@ -687,21 +661,25 @@ fn get_native_style(all_loaded_files: &mut Vec) -> String { /// Note: this function is only called for .rs path as part of the LSP or viewer. /// Because from a proc_macro, we don't actually know the path of the current file, and this /// is why we must be relative to CARGO_MANIFEST_DIR. -pub fn base_directory(referencing_file: &Path) -> Option { +pub fn base_directory(referencing_file: &Path) -> PathBuf { if referencing_file.extension().map_or(false, |e| e == "rs") { // For .rs file, this is a rust macro, and rust macro locates the file relative to the CARGO_MANIFEST_DIR which is the directory that has a Cargo.toml file. - let mut candidate = referencing_file; + let mut candidate = crate::pathutils::dirname(referencing_file); + let mut previous_candidate = referencing_file.to_path_buf(); loop { - candidate = - if let Some(c) = candidate.parent() { c } else { break referencing_file.parent() }; + if candidate == previous_candidate { + return crate::pathutils::dirname(referencing_file); + } + previous_candidate = candidate; + candidate = crate::pathutils::dirname(&previous_candidate); + if candidate.join("Cargo.toml").exists() { - break Some(candidate); + return candidate.to_path_buf(); } } } else { - referencing_file.parent() + crate::pathutils::dirname(referencing_file) } - .map(|p| p.to_path_buf()) } #[test] @@ -862,7 +840,7 @@ component Foo { XX {} } diags, &["HELLO:3: Cannot find requested import \"error.slint\" in the include search path"] ); - // Try loading another time with the same registery + // Try loading another time with the same registry let mut build_diagnostics = BuildDiagnostics::default(); spin_on::spin_on(loader.load_dependencies_recursively( &doc_node, diff --git a/tools/lsp/Cargo.toml b/tools/lsp/Cargo.toml index 786dc00c0..b3831def1 100644 --- a/tools/lsp/Cargo.toml +++ b/tools/lsp/Cargo.toml @@ -70,7 +70,6 @@ default = ["backend-qt", "backend-winit", "renderer-femtovg", "preview"] [dependencies] i-slint-compiler = { workspace = true, features = ["default"] } -dunce = "1.0.1" euclid = "0.22" lsp-types = { version = "0.94.0", features = ["proposed"] } serde = "1.0.118" diff --git a/tools/lsp/language.rs b/tools/lsp/language.rs index 24385412b..97c0a05eb 100644 --- a/tools/lsp/language.rs +++ b/tools/lsp/language.rs @@ -19,6 +19,7 @@ use crate::wasm_prelude::*; use i_slint_compiler::object_tree::ElementRc; use i_slint_compiler::parser::{syntax_nodes, NodeOrToken, SyntaxKind, SyntaxNode, SyntaxToken}; +use i_slint_compiler::pathutils::clean_path; use i_slint_compiler::CompilerConfiguration; use i_slint_compiler::{diagnostics::BuildDiagnostics, langtype::Type}; use i_slint_compiler::{typeloader::TypeLoader, typeregister::TypeRegister}; @@ -51,8 +52,8 @@ const TOGGLE_DESIGN_MODE_COMMAND: &str = "slint/toggleDesignMode"; pub fn uri_to_file(uri: &lsp_types::Url) -> Option { let Ok(path) = uri.to_file_path() else { return None }; - let path_canon = dunce::canonicalize(&path).unwrap_or_else(|_| path.to_owned()); - Some(path_canon) + let cleaned_path = clean_path(&path); + Some(cleaned_path) } fn command_list() -> Vec { diff --git a/tools/lsp/language/completion.rs b/tools/lsp/language/completion.rs index f76a80d46..840c326cb 100644 --- a/tools/lsp/language/completion.rs +++ b/tools/lsp/language/completion.rs @@ -531,7 +531,7 @@ fn complete_path_in_string(base: &Path, text: &str, offset: u32) -> Option