Skip to content

Commit ca4ee39

Browse files
Natalie Jamesonfacebook-github-bot
authored andcommitted
Add a URL with typed schemes
Summary: We're going to be adding support for a "starlark:" uri scheme for native types. Previously all code erroneously assumed a file:// scheme. Add this type so that all supported cases are covered by the LSPContext trait and clients can make sure they cover everything that the LSP client might require. Reviewed By: ndmitchell Differential Revision: D38307726 fbshipit-source-id: d3195491c0111a9f6905ef3c444869097283f2f4
1 parent 6ed422c commit ca4ee39

File tree

4 files changed

+281
-111
lines changed

4 files changed

+281
-111
lines changed

starlark/bin/eval.rs

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use starlark::eval::Evaluator;
3333
use starlark::lsp::server::LoadContentsError;
3434
use starlark::lsp::server::LspContext;
3535
use starlark::lsp::server::LspEvalResult;
36+
use starlark::lsp::server::LspUrl;
3637
use starlark::lsp::server::ResolveLoadError;
3738
use starlark::lsp::server::StringLiteralResult;
3839
use starlark::syntax::AstModule;
@@ -126,7 +127,7 @@ impl Context {
126127
) -> EvalResult<impl Iterator<Item = EvalMessage>> {
127128
match result {
128129
Err(e) => EvalResult {
129-
messages: Either::Left(iter::once(EvalMessage::from_anyhow(file, &e))),
130+
messages: Either::Left(iter::once(EvalMessage::from_anyhow(Path::new(file), &e))),
130131
ast: None,
131132
},
132133
Ok(res) => EvalResult {
@@ -210,29 +211,42 @@ impl Context {
210211
}
211212

212213
impl LspContext for Context {
213-
fn parse_file_with_contents(&self, uri: &Url, content: String) -> LspEvalResult {
214-
let EvalResult { messages, ast } = self.file_with_contents(uri.path(), content);
215-
LspEvalResult {
216-
diagnostics: messages.map(Diagnostic::from).collect(),
217-
ast,
214+
fn parse_file_with_contents(&self, uri: &LspUrl, content: String) -> LspEvalResult {
215+
match uri {
216+
LspUrl::File(uri) => {
217+
let EvalResult { messages, ast } =
218+
self.file_with_contents(&uri.to_string_lossy(), content);
219+
LspEvalResult {
220+
diagnostics: messages.map(Diagnostic::from).collect(),
221+
ast,
222+
}
223+
}
224+
_ => LspEvalResult::default(),
218225
}
219226
}
220227

221-
fn resolve_load(&self, path: &str, current_file: &Path) -> anyhow::Result<Url> {
228+
fn resolve_load(&self, path: &str, current_file: &LspUrl) -> anyhow::Result<LspUrl> {
222229
let path = PathBuf::from(path);
223-
let current_file_dir = current_file.parent();
224-
let absolute_path = match (current_file_dir, path.is_absolute()) {
225-
(_, true) => Ok(path),
226-
(Some(current_file_dir), false) => Ok(current_file_dir.join(&path)),
227-
(None, false) => Err(ResolveLoadError::MissingCurrentFilePath(path)),
228-
}?;
229-
Ok(Url::from_file_path(absolute_path).unwrap())
230+
match current_file {
231+
LspUrl::File(current_file_path) => {
232+
let current_file_dir = current_file_path.parent();
233+
let absolute_path = match (current_file_dir, path.is_absolute()) {
234+
(_, true) => Ok(path),
235+
(Some(current_file_dir), false) => Ok(current_file_dir.join(&path)),
236+
(None, false) => Err(ResolveLoadError::MissingCurrentFilePath(path)),
237+
}?;
238+
Ok(Url::from_file_path(absolute_path).unwrap().try_into()?)
239+
}
240+
_ => Err(
241+
ResolveLoadError::WrongScheme("file://".to_owned(), current_file.clone()).into(),
242+
),
243+
}
230244
}
231245

232246
fn resolve_string_literal(
233247
&self,
234248
literal: &str,
235-
current_file: &Path,
249+
current_file: &LspUrl,
236250
) -> anyhow::Result<Option<StringLiteralResult>> {
237251
self.resolve_load(literal, current_file).map(|url| {
238252
Some(StringLiteralResult {
@@ -242,20 +256,22 @@ impl LspContext for Context {
242256
})
243257
}
244258

245-
fn get_load_contents(&self, uri: &Url) -> anyhow::Result<Option<String>> {
246-
let path = PathBuf::from(uri.path());
247-
match path.is_absolute() {
248-
true => match fs::read_to_string(&path) {
249-
Ok(contents) => Ok(Some(contents)),
250-
Err(e)
251-
if e.kind() == io::ErrorKind::NotFound
252-
|| e.kind() == io::ErrorKind::NotADirectory =>
253-
{
254-
Ok(None)
255-
}
256-
Err(e) => Err(e.into()),
259+
fn get_load_contents(&self, uri: &LspUrl) -> anyhow::Result<Option<String>> {
260+
match uri {
261+
LspUrl::File(path) => match path.is_absolute() {
262+
true => match fs::read_to_string(&path) {
263+
Ok(contents) => Ok(Some(contents)),
264+
Err(e)
265+
if e.kind() == io::ErrorKind::NotFound
266+
|| e.kind() == io::ErrorKind::NotADirectory =>
267+
{
268+
Ok(None)
269+
}
270+
Err(e) => Err(e.into()),
271+
},
272+
false => Err(LoadContentsError::NotAbsolute(uri.clone()).into()),
257273
},
258-
false => Err(LoadContentsError::NotAbsolute(uri.clone()).into()),
274+
_ => Err(LoadContentsError::WrongScheme("file://".to_owned(), uri.clone()).into()),
259275
}
260276
}
261277
}

starlark/src/analysis/types.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
use std::fmt;
1919
use std::fmt::Display;
20+
use std::path::Path;
2021

2122
use gazebo::prelude::*;
2223
use gazebo::variants::VariantName;
@@ -160,7 +161,7 @@ impl Display for EvalMessage {
160161

161162
impl EvalMessage {
162163
/// Convert from an `anyhow::Error`, including some type checking, to an `EvalMessage`
163-
pub fn from_anyhow(file: &str, x: &anyhow::Error) -> Self {
164+
pub fn from_anyhow(file: &Path, x: &anyhow::Error) -> Self {
164165
match x.downcast_ref::<StarlarkDiagnostic>() {
165166
Some(
166167
d @ StarlarkDiagnostic {
@@ -182,7 +183,7 @@ impl EvalMessage {
182183
}
183184
}
184185
_ => Self {
185-
path: file.to_owned(),
186+
path: file.display().to_string(),
186187
span: None,
187188
severity: EvalSeverity::Error,
188189
name: "error".to_owned(),

0 commit comments

Comments
 (0)