Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

helix-lsp-types: Replace url::Url type with String wrapper #11889

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ tree-sitter = { version = "0.22" }
nucleo = "0.5.0"
slotmap = "1.0.7"
thiserror = "1.0"
percent-encoding = "2.3"

[workspace.package]
version = "24.7.0"
Expand Down
2 changes: 1 addition & 1 deletion helix-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ bitflags = "2.6"
ahash = "0.8.11"
hashbrown = { version = "0.14.5", features = ["raw"] }
dunce = "1.0"
url = "2.5.0"
percent-encoding.workspace = true

log = "0.4"
serde = { version = "1.0", features = ["derive"] }
Expand Down
114 changes: 59 additions & 55 deletions helix-core/src/uri.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{
fmt,
path::{Path, PathBuf},
str::FromStr,
sync::Arc,
};

Expand All @@ -16,14 +17,6 @@ pub enum Uri {
}

impl Uri {
// This clippy allow mirrors url::Url::from_file_path
#[allow(clippy::result_unit_err)]
pub fn to_url(&self) -> Result<url::Url, ()> {
match self {
Uri::File(path) => url::Url::from_file_path(path),
}
}

pub fn as_path(&self) -> Option<&Path> {
match self {
Self::File(path) => Some(path),
Expand All @@ -45,81 +38,92 @@ impl fmt::Display for Uri {
}
}

#[derive(Debug)]
pub struct UrlConversionError {
source: url::Url,
kind: UrlConversionErrorKind,
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct UriParseError {
source: String,
kind: UriParseErrorKind,
}

#[derive(Debug)]
pub enum UrlConversionErrorKind {
UnsupportedScheme,
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum UriParseErrorKind {
UnsupportedScheme(String),
UnableToConvert,
}

impl fmt::Display for UrlConversionError {
impl fmt::Display for UriParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.kind {
UrlConversionErrorKind::UnsupportedScheme => {
write!(
f,
"unsupported scheme '{}' in URL {}",
self.source.scheme(),
self.source
)
match &self.kind {
UriParseErrorKind::UnsupportedScheme(scheme) => {
write!(f, "unsupported scheme '{scheme}' in URL {}", self.source)
}
UrlConversionErrorKind::UnableToConvert => {
UriParseErrorKind::UnableToConvert => {
write!(f, "unable to convert URL to file path: {}", self.source)
}
}
}
}

impl std::error::Error for UrlConversionError {}

fn convert_url_to_uri(url: &url::Url) -> Result<Uri, UrlConversionErrorKind> {
if url.scheme() == "file" {
url.to_file_path()
.map(|path| Uri::File(helix_stdx::path::normalize(path).into()))
.map_err(|_| UrlConversionErrorKind::UnableToConvert)
} else {
Err(UrlConversionErrorKind::UnsupportedScheme)
}
}
impl std::error::Error for UriParseError {}

impl FromStr for Uri {
type Err = UriParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
use std::ffi::OsStr;
#[cfg(any(unix, target_os = "redox"))]
use std::os::unix::prelude::OsStrExt;
#[cfg(target_os = "wasi")]
use std::os::wasi::prelude::OsStrExt;

let Some((scheme, rest)) = s.split_once("://") else {
return Err(Self::Err {
source: s.to_string(),
kind: UriParseErrorKind::UnableToConvert,
});
};

if scheme != "file" {
return Err(Self::Err {
source: s.to_string(),
kind: UriParseErrorKind::UnsupportedScheme(scheme.to_string()),
});
}

impl TryFrom<url::Url> for Uri {
type Error = UrlConversionError;
// Assert there is no query or fragment in the URI.
if s.find(['?', '#']).is_some() {
return Err(Self::Err {
source: s.to_string(),
kind: UriParseErrorKind::UnableToConvert,
});
}

fn try_from(url: url::Url) -> Result<Self, Self::Error> {
convert_url_to_uri(&url).map_err(|kind| Self::Error { source: url, kind })
let mut bytes = Vec::new();
bytes.extend(percent_encoding::percent_decode(rest.as_bytes()));
Ok(PathBuf::from(OsStr::from_bytes(&bytes)).into())
}
}

impl TryFrom<&url::Url> for Uri {
type Error = UrlConversionError;
impl TryFrom<&str> for Uri {
type Error = UriParseError;

fn try_from(url: &url::Url) -> Result<Self, Self::Error> {
convert_url_to_uri(url).map_err(|kind| Self::Error {
source: url.clone(),
kind,
})
fn try_from(s: &str) -> Result<Self, Self::Error> {
s.parse()
}
}

#[cfg(test)]
mod test {
use super::*;
use url::Url;

#[test]
fn unknown_scheme() {
let url = Url::parse("csharp:/metadata/foo/bar/Baz.cs").unwrap();
assert!(matches!(
Uri::try_from(url),
Err(UrlConversionError {
kind: UrlConversionErrorKind::UnsupportedScheme,
..
let uri = "csharp://metadata/foo/barBaz.cs";
assert_eq!(
uri.parse::<Uri>(),
Err(UriParseError {
source: uri.to_string(),
kind: UriParseErrorKind::UnsupportedScheme("csharp".to_string()),
})
));
);
}
}
2 changes: 1 addition & 1 deletion helix-lsp-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ bitflags = "2.6.0"
serde = { version = "1.0.209", features = ["derive"] }
serde_json = "1.0.127"
serde_repr = "0.1"
url = {version = "2.0.0", features = ["serde"]}
percent-encoding.workspace = true

[features]
default = []
Expand Down
4 changes: 3 additions & 1 deletion helix-lsp-types/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Helix's `lsp-types`

This is a fork of the [`lsp-types`](https://crates.io/crates/lsp-types) crate ([`gluon-lang/lsp-types`](https://github.com/gluon-lang/lsp-types)) taken at version v0.95.1 (commit [3e6daee](https://github.com/gluon-lang/lsp-types/commit/3e6daee771d14db4094a554b8d03e29c310dfcbe)). This fork focuses usability improvements that make the types easier to work with for the Helix codebase. For example the URL type - the `uri` crate at this version of `lsp-types` - will be replaced with a wrapper around a string.
This is a fork of the [`lsp-types`](https://crates.io/crates/lsp-types) crate ([`gluon-lang/lsp-types`](https://github.com/gluon-lang/lsp-types)) taken at version v0.95.1 (commit [3e6daee](https://github.com/gluon-lang/lsp-types/commit/3e6daee771d14db4094a554b8d03e29c310dfcbe)). This fork focuses usability improvements that make the types easier to work with for the Helix codebase.

The URL type has been replaced with a newtype wrapper of a `String`. The `lsp-types` crate at the forked version used [`url::Url`](https://docs.rs/url/2.5.0/url/struct.Url.html) which provides conveniences for using URLs according to [the WHATWG URL spec](https://url.spec.whatwg.org). Helix supports a subset of valid URLs, namely the `file://` scheme, so a wrapper around a normal `String` is sufficient. Plus the LSP spec requires URLs to be in [RFC3986](https://tools.ietf.org/html/rfc3986) format instead.
3 changes: 1 addition & 2 deletions helix-lsp-types/src/call_hierarchy.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use serde::{Deserialize, Serialize};
use serde_json::Value;
use url::Url;

use crate::{
DynamicRegistrationClientCapabilities, PartialResultParams, Range, SymbolKind, SymbolTag,
TextDocumentPositionParams, WorkDoneProgressOptions, WorkDoneProgressParams,
TextDocumentPositionParams, Url, WorkDoneProgressOptions, WorkDoneProgressParams,
};

pub type CallHierarchyClientCapabilities = DynamicRegistrationClientCapabilities;
Expand Down
3 changes: 1 addition & 2 deletions helix-lsp-types/src/document_diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use std::collections::HashMap;

use serde::{Deserialize, Serialize};
use url::Url;

use crate::{
Diagnostic, PartialResultParams, StaticRegistrationOptions, TextDocumentIdentifier,
TextDocumentRegistrationOptions, WorkDoneProgressOptions, WorkDoneProgressParams,
TextDocumentRegistrationOptions, Url, WorkDoneProgressOptions, WorkDoneProgressParams,
};

/// Client capabilities specific to diagnostic pull requests.
Expand Down
3 changes: 1 addition & 2 deletions helix-lsp-types/src/document_link.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::{
PartialResultParams, Range, TextDocumentIdentifier, WorkDoneProgressOptions,
PartialResultParams, Range, TextDocumentIdentifier, Url, WorkDoneProgressOptions,
WorkDoneProgressParams,
};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use url::Url;

#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
Expand Down
95 changes: 79 additions & 16 deletions helix-lsp-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,90 @@
Language Server Protocol types for Rust.

Based on: <https://microsoft.github.io/language-server-protocol/specification>

This library uses the URL crate for parsing URIs. Note that there is
some confusion on the meaning of URLs vs URIs:
<http://stackoverflow.com/a/28865728/393898>. According to that
information, on the classical sense of "URLs", "URLs" are a subset of
URIs, But on the modern/new meaning of URLs, they are the same as
URIs. The important take-away aspect is that the URL crate should be
able to parse any URI, such as `urn:isbn:0451450523`.


*/
#![allow(non_upper_case_globals)]
#![forbid(unsafe_code)]
#[macro_use]
extern crate bitflags;

use std::{collections::HashMap, fmt::Debug};
use bitflags::bitflags;

use std::{collections::HashMap, fmt::Debug, path::Path};

use serde::{de, de::Error as Error_, Deserialize, Serialize};
use serde_json::Value;
pub use url::Url;

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize, Serialize)]
pub struct Url(String);

impl Url {
pub fn from_file_path<P: AsRef<Path>>(path: P) -> Self {
use percent_encoding::{percent_encode, AsciiSet, CONTROLS};
#[cfg(any(unix, target_os = "redox"))]
use std::os::unix::prelude::OsStrExt;
#[cfg(target_os = "wasi")]
use std::os::wasi::prelude::OsStrExt;

// <https://datatracker.ietf.org/doc/html/rfc3986#section-2.2>, also see
// <https://github.com/microsoft/vscode-uri/blob/6dec22d7dcc6c63c30343d3a8d56050d0078cb6a/src/uri.ts#L454-L477>
const RESERVED: &AsciiSet = &CONTROLS
// GEN_DELIMS
.add(b':')
.add(b'/')
.add(b'?')
.add(b'#')
.add(b'[')
.add(b']')
.add(b'@')
// SUB_DELIMS
.add(b'!')
.add(b'$')
.add(b'&')
.add(b'\'')
.add(b'(')
.add(b')')
.add(b'*')
.add(b'+')
.add(b',')
.add(b';')
.add(b'=');

let mut serialization = String::from("file://");
// skip the root component
for component in path.as_ref().components().skip(1) {
serialization.push('/');
serialization.extend(percent_encode(component.as_os_str().as_bytes(), RESERVED));
}
if &serialization == "file://" {
// An URL's path must not be empty.
serialization.push('/');
}
Self(serialization)
}

pub fn from_directory_path<P: AsRef<Path>>(path: P) -> Self {
let Self(mut serialization) = Self::from_file_path(path);
if !serialization.ends_with('/') {
serialization.push('/');
}
Self(serialization)
}

/// Returns the serialized representation of the URL as a `&str`
pub fn as_str(&self) -> &str {
&self.0
}

/// Consumes the URL, converting into a `String`.
/// Note that the string is the serialized representation of the URL.
pub fn into_string(self) -> String {
self.0
}
}

impl From<&str> for Url {
fn from(value: &str) -> Self {
Self(value.to_string())
}
}

// Large enough to contain any enumeration name defined in this crate
type PascalCaseBuf = [u8; 32];
Expand Down Expand Up @@ -2843,14 +2906,14 @@ mod tests {
test_serialization(
&WorkspaceEdit {
changes: Some(
vec![(Url::parse("file://test").unwrap(), vec![])]
vec![(Url::from("file://test"), vec![])]
.into_iter()
.collect(),
),
document_changes: None,
..Default::default()
},
r#"{"changes":{"file://test/":[]}}"#,
r#"{"changes":{"file://test":[]}}"#,
);
}

Expand Down
Loading
Loading