Skip to content

Commit

Permalink
Move DocumentError to use thiserror (FuelLabs#2968)
Browse files Browse the repository at this point in the history 
Related FuelLabs#2286

- adds `thiserror` to sway LSP and moves the existing `DocumentError`
types over
- adds `path` or `dir` to the document errors
- logs the parsing errors using the LSP logging interface
- adds 2 new error types, distinguishing between BuildPlan, Parse, and
Compile errors
- adds `ManifestFileNotFound` error
- refactors functions to use `map_err` pattern instead of `match`
- adds tests for the DocumentErrors

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
@sdankel @JoshuaBatty
sdankel and JoshuaBatty authored Oct 12, 2022
1 parent 800ac2f commit 205073d
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 119 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock
View file

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

2 changes: 2 additions & 0 deletions sway-lsp/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ repository = "https://github.com/FuelLabs/sway"
description = "LSP server for Sway."

[dependencies]
anyhow = "1.0.41"
dashmap = "5.4"
forc-pkg = { version = "0.25.2", path = "../forc-pkg" }
forc-util = { version = "0.25.2", path = "../forc-util" }
Expand All @@ -20,6 +21,7 @@ sway-error = { version = "0.25.2", path = "../sway-error" }
sway-types = { version = "0.25.2", path = "../sway-types" }
sway-utils = { version = "0.25.2", path = "../sway-utils" }
swayfmt = { version = "0.25.2", path = "../swayfmt" }
thiserror = "1.0.30"
tokio = { version = "1.3", features = ["io-std", "io-util", "macros", "net", "rt-multi-thread", "sync", "time"] }
tower-lsp = "0.17"
tracing = "0.1"
Expand Down
37 changes: 26 additions & 11 deletions sway-lsp/src/core/document.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(dead_code)]
use ropey::Rope;
use tower_lsp::lsp_types::{Diagnostic, Position, Range, TextDocumentContentChangeEvent};
use tower_lsp::lsp_types::{Position, Range, TextDocumentContentChangeEvent};

use crate::error::DocumentError;

#[derive(Debug)]
pub struct TextDocument {
Expand All @@ -14,15 +16,14 @@ pub struct TextDocument {

impl TextDocument {
pub fn build_from_path(path: &str) -> Result<Self, DocumentError> {
match std::fs::read_to_string(&path) {
Ok(content) => Ok(Self {
std::fs::read_to_string(&path)
.map(|content| Self {
language_id: "sway".into(),
version: 1,
uri: path.into(),
content: Rope::from_str(&content),
}),
Err(_) => Err(DocumentError::DocumentNotFound),
}
})
.map_err(|_| DocumentError::DocumentNotFound { path: path.into() })
}

pub fn get_uri(&self) -> &str {
Expand Down Expand Up @@ -106,9 +107,23 @@ struct EditText<'text> {
change_text: &'text str,
}

#[derive(Debug)]
pub enum DocumentError {
FailedToParse(Vec<Diagnostic>),
DocumentNotFound,
DocumentAlreadyStored,
#[cfg(test)]
mod tests {
use crate::test_utils::get_absolute_path;

use super::*;

#[test]
fn build_from_path_returns_text_document() {
let path = get_absolute_path("sway-lsp/test/fixtures/cats.txt");
let result = TextDocument::build_from_path(&path);
assert!(result.is_ok(), "result = {:?}", result);
}

#[test]
fn build_from_path_returns_document_not_found_error() {
let path = get_absolute_path("not/a/real/file/path");
let result = TextDocument::build_from_path(&path).expect_err("expected DocumentNotFound");
assert_eq!(result, DocumentError::DocumentNotFound { path });
}
}
243 changes: 145 additions & 98 deletions sway-lsp/src/core/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ use crate::{
runnable::{Runnable, RunnableType},
},
core::{
document::{DocumentError, TextDocument},
document::TextDocument,
token::{Token, TokenMap, TypeDefinition},
{traverse_parse_tree, traverse_typed_tree},
},
error::{DocumentError, LanguageServerError},
utils,
};
use dashmap::DashMap;
Expand Down Expand Up @@ -122,132 +123,133 @@ impl Session {

// Document
pub fn store_document(&self, text_document: TextDocument) -> Result<(), DocumentError> {
match self
.documents
.insert(text_document.get_uri().into(), text_document)
{
None => Ok(()),
_ => Err(DocumentError::DocumentAlreadyStored),
}
let uri = text_document.get_uri().to_string();
self.documents
.insert(uri.clone(), text_document)
.map_or(Ok(()), |_| {
Err(DocumentError::DocumentAlreadyStored { path: uri })
})
}

pub fn remove_document(&self, url: &Url) -> Result<TextDocument, DocumentError> {
match self.documents.remove(url.path()) {
Some((_, text_document)) => Ok(text_document),
None => Err(DocumentError::DocumentNotFound),
}
self.documents
.remove(url.path())
.ok_or(DocumentError::DocumentNotFound {
path: url.path().to_string(),
})
.map(|(_, text_document)| text_document)
}

pub fn parse_project(&self, uri: &Url) -> Result<Vec<Diagnostic>, DocumentError> {
pub fn parse_project(&self, uri: &Url) -> Result<Vec<Diagnostic>, LanguageServerError> {
self.token_map.clear();
self.runnables.clear();

let manifest_dir = PathBuf::from(uri.path());
let locked = false;
let offline = false;

// TODO: match on any errors and report them back to the user in a future PR
if let Ok(manifest) = pkg::PackageManifestFile::from_dir(&manifest_dir) {
if let Ok(plan) = pkg::BuildPlan::from_lock_and_manifest(&manifest, locked, offline) {
//we can then use them directly to convert them to a Vec<Diagnostic>
if let Ok(CompileResult {
value,
warnings,
errors,
}) = pkg::check(&plan, true)
{
// FIXME(Centril): Refactor parse_ast_to_tokens + parse_ast_to_typed_tokens
// due to the new API.
let (parsed, typed) = match value {
None => (None, None),
Some((pp, tp)) => (Some(pp), tp),
};
// First, populate our token_map with un-typed ast nodes.
let parsed_res = CompileResult::new(parsed, warnings.clone(), errors.clone());
let _ = self.parse_ast_to_tokens(parsed_res);
// Next, populate our token_map with typed ast nodes.
let ast_res = CompileResult::new(typed, warnings, errors);
return self.parse_ast_to_typed_tokens(ast_res);
}
let manifest = pkg::PackageManifestFile::from_dir(&manifest_dir).map_err(|_| {
DocumentError::ManifestFileNotFound {
dir: uri.path().into(),
}
}
Err(DocumentError::FailedToParse(vec![]))
})?;

let plan = pkg::BuildPlan::from_lock_and_manifest(&manifest, locked, offline)
.map_err(LanguageServerError::BuildPlanFailed)?;

// We can convert these destructured elements to a Vec<Diagnostic> later on.
let CompileResult {
value,
warnings,
errors,
} = pkg::check(&plan, true).map_err(LanguageServerError::FailedToCompile)?;

// FIXME(Centril): Refactor parse_ast_to_tokens + parse_ast_to_typed_tokens
// due to the new API.g
let (parsed, typed) = match value {
None => (None, None),
Some((pp, tp)) => (Some(pp), tp),
};

// First, populate our token_map with un-typed ast nodes.
let parsed_res = CompileResult::new(parsed, warnings.clone(), errors.clone());
let _ = self.parse_ast_to_tokens(parsed_res);
// Next, populate our token_map with typed ast nodes.
let ast_res = CompileResult::new(typed, warnings, errors);

self.parse_ast_to_typed_tokens(ast_res)
}

fn parse_ast_to_tokens(
&self,
parsed_result: CompileResult<ParseProgram>,
) -> Result<Vec<Diagnostic>, DocumentError> {
match parsed_result.value {
None => {
let diagnostics = capabilities::diagnostic::get_diagnostics(
parsed_result.warnings,
parsed_result.errors,
);
Err(DocumentError::FailedToParse(diagnostics))
}
Some(parse_program) => {
for node in &parse_program.root.tree.root_nodes {
traverse_parse_tree::traverse_node(node, &self.token_map);
}

for (_, submodule) in &parse_program.root.submodules {
for node in &submodule.module.tree.root_nodes {
traverse_parse_tree::traverse_node(node, &self.token_map);
}
}

if let LockResult::Ok(mut program) = self.compiled_program.write() {
program.parsed = Some(parse_program);
}
) -> Result<Vec<Diagnostic>, LanguageServerError> {
let parse_program = parsed_result.value.ok_or_else(|| {
let diagnostics = capabilities::diagnostic::get_diagnostics(
parsed_result.warnings.clone(),
parsed_result.errors.clone(),
);
LanguageServerError::FailedToParse { diagnostics }
})?;

for node in &parse_program.root.tree.root_nodes {
traverse_parse_tree::traverse_node(node, &self.token_map);
}

Ok(capabilities::diagnostic::get_diagnostics(
parsed_result.warnings,
parsed_result.errors,
))
for (_, submodule) in &parse_program.root.submodules {
for node in &submodule.module.tree.root_nodes {
traverse_parse_tree::traverse_node(node, &self.token_map);
}
}

if let LockResult::Ok(mut program) = self.compiled_program.write() {
program.parsed = Some(parse_program);
}

Ok(capabilities::diagnostic::get_diagnostics(
parsed_result.warnings,
parsed_result.errors,
))
}

fn parse_ast_to_typed_tokens(
&self,
ast_res: CompileResult<TyProgram>,
) -> Result<Vec<Diagnostic>, DocumentError> {
match ast_res.value {
None => Err(DocumentError::FailedToParse(
capabilities::diagnostic::get_diagnostics(ast_res.warnings, ast_res.errors),
)),
Some(typed_program) => {
if let TyProgramKind::Script {
ref main_function, ..
} = typed_program.kind
{
let main_fn_location =
utils::common::get_range_from_span(&main_function.name.span());
let runnable = Runnable::new(main_fn_location, typed_program.kind.tree_type());
self.runnables.insert(RunnableType::MainFn, runnable);
}
) -> Result<Vec<Diagnostic>, LanguageServerError> {
let typed_program = ast_res.value.ok_or(LanguageServerError::FailedToParse {
diagnostics: capabilities::diagnostic::get_diagnostics(
ast_res.warnings.clone(),
ast_res.errors.clone(),
),
})?;

if let TyProgramKind::Script {
ref main_function, ..
} = typed_program.kind
{
let main_fn_location = utils::common::get_range_from_span(&main_function.name.span());
let runnable = Runnable::new(main_fn_location, typed_program.kind.tree_type());
self.runnables.insert(RunnableType::MainFn, runnable);
}

let root_nodes = typed_program.root.all_nodes.iter();
let sub_nodes = typed_program
.root
.submodules
.iter()
.flat_map(|(_, submodule)| &submodule.module.all_nodes);
root_nodes
.chain(sub_nodes)
.for_each(|node| traverse_typed_tree::traverse_node(node, &self.token_map));

if let LockResult::Ok(mut program) = self.compiled_program.write() {
program.typed = Some(typed_program);
}
let root_nodes = typed_program.root.all_nodes.iter();
let sub_nodes = typed_program
.root
.submodules
.iter()
.flat_map(|(_, submodule)| &submodule.module.all_nodes);
root_nodes
.chain(sub_nodes)
.for_each(|node| traverse_typed_tree::traverse_node(node, &self.token_map));

Ok(capabilities::diagnostic::get_diagnostics(
ast_res.warnings,
ast_res.errors,
))
}
if let LockResult::Ok(mut program) = self.compiled_program.write() {
program.typed = Some(typed_program);
}

Ok(capabilities::diagnostic::get_diagnostics(
ast_res.warnings,
ast_res.errors,
))
}

pub fn contains_sway_file(&self, url: &Url) -> bool {
Expand Down Expand Up @@ -328,3 +330,48 @@ impl Session {
}
}
}

#[cfg(test)]
mod tests {

use crate::test_utils::{get_absolute_path, get_url};

use super::*;

#[test]
fn store_document_returns_empty_tuple() {
let session = Session::new();
let path = get_absolute_path("sway-lsp/test/fixtures/cats.txt");
let document = TextDocument::build_from_path(&path).unwrap();
let result = Session::store_document(&session, document);
assert!(result.is_ok());
}

#[test]
fn store_document_returns_document_already_stored_error() {
let session = Session::new();
let path = get_absolute_path("sway-lsp/test/fixtures/cats.txt");
let document = TextDocument::build_from_path(&path).unwrap();
Session::store_document(&session, document).expect("expected successfully stored");
let document = TextDocument::build_from_path(&path).unwrap();
let result = Session::store_document(&session, document)
.expect_err("expected DocumentAlreadyStored");
assert_eq!(result, DocumentError::DocumentAlreadyStored { path });
}

#[test]
fn parse_project_returns_manifest_file_not_found() {
let session = Session::new();
let dir = get_absolute_path("sway-lsp/test/fixtures");
let uri = get_url(&dir);
let result =
Session::parse_project(&session, &uri).expect_err("expected ManifestFileNotFound");
assert!(matches!(
result,
LanguageServerError::DocumentError(
DocumentError::ManifestFileNotFound { dir: test_dir }
)
if test_dir == dir
));
}
}
Loading

0 comments on commit 205073d

Please sign in to comment.