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

Add a file cache #186

Merged
merged 5 commits into from
Jan 28, 2024
Merged
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
114 changes: 37 additions & 77 deletions ante-ls/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
use std::{
collections::HashMap,
env::{current_dir, set_current_dir},
env::set_current_dir,
path::{Path, PathBuf},
};

use ante::{
cache::ModuleCache,
cache::{cached_read, ModuleCache},
error::{location::Locatable, ErrorType},
lexer::Lexer,
nameresolution::NameResolver,
parser::parse,
types::typechecker,
frontend,
};

use dashmap::DashMap;
Expand Down Expand Up @@ -73,9 +70,6 @@ impl LanguageServer for Backend {
self.update_diagnostics(params.text_document.uri, &rope).await;
}

// The diagnostics can't be updated on change, because the content of the file in the file system
// is not guaranteed to be the same as the content of the file in the editor. This will result in
// a panic when running Diagnostic::format, and the column are lengths different than expected.
async fn did_change(&self, params: DidChangeTextDocumentParams) {
self.client.log_message(MessageType::LOG, format!("ante_ls did_change: {:?}", params)).await;
self.document_map.alter(&params.text_document.uri, |_, mut rope| {
Expand All @@ -90,25 +84,9 @@ impl LanguageServer for Backend {
}
rope
});
}
}

fn relative_path<P1: AsRef<Path>, P2: AsRef<Path>>(root: P1, path: P2) -> Option<PathBuf> {
let path = path.as_ref();
if let Ok(path) = path.strip_prefix(&root) {
return Some(path.to_path_buf());
}

let mut acc = PathBuf::new();
let mut root = root.as_ref();
loop {
if let Ok(path) = path.strip_prefix(root) {
acc.push(path);
return Some(acc);
} else {
acc.push("..");
root = root.parent()?;
}
if let Some(rope) = self.document_map.get(&params.text_document.uri) {
self.update_diagnostics(params.text_document.uri, &rope).await;
};
}
}

Expand Down Expand Up @@ -147,32 +125,20 @@ fn rope_range_to_lsp_range(range: std::ops::Range<usize>, rope: &Rope) -> Range

impl Backend {
async fn update_diagnostics(&self, uri: Url, rope: &Rope) {
let root = match current_dir() {
Ok(root) => root,
Err(_) => {
self.client.log_message(MessageType::ERROR, "Failed to get current directory".to_string()).await;
return;
},
};
// We want the filename to be relative to the root for nicer error messages.
// This could fail on windows when the root is on a different drive than the file.
// Urls always contain ablsoute canonical paths, so there's no need to canonicalize them.
let filename = Path::new(uri.path());
let filename = relative_path(&root, filename).unwrap();

let mut cache = ModuleCache::new(filename.parent().unwrap());
let tokens = Lexer::new(&filename, &rope.to_string()).collect::<Vec<_>>();
match parse(&tokens) {
Ok(ast) => {
NameResolver::start(ast, &mut cache);
if cache.error_count() == 0 {
let ast = cache.parse_trees.get_mut(0).unwrap();
typechecker::infer_ast(ast, &mut cache);
}
},
Err(err) => {
cache.push_full_diagnostic(err.into_diagnostic());
},
};
let cache_root = filename.parent().unwrap();

let (paths, contents) =
self.document_map.iter().fold((Vec::new(), Vec::new()), |(mut paths, mut contents), item| {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
paths.push(PathBuf::from(item.key().path()));
contents.push(item.value().to_string());
(paths, contents)
});
let file_cache = paths.iter().zip(contents.into_iter()).map(|(p, c)| (p.as_path(), c)).collect();
let mut cache = ModuleCache::new(cache_root, file_cache);

let _ = frontend::check(filename, rope.to_string(), &mut cache, frontend::FrontendPhase::TypeCheck, false);

// Diagnostics for a document get cleared only when an empty list is sent for it's Uri.
// This presents an issue, as when we have files A and B, where file A imports the file B,
Expand All @@ -186,7 +152,7 @@ impl Backend {
// to generate a list of files in one, we could run the compiler on the root of the project.
// That should provide an exhaustive list of diagnostics, and allow us to clear all diagnostics
// for files that had none in the new list.
let mut diagnostics = HashMap::from([(uri, Vec::new())]);
let mut diagnostics = HashMap::from([(uri.clone(), Vec::new())]);

for diagnostic in cache.get_diagnostics() {
let severity = Some(match diagnostic.error_type() {
Expand All @@ -196,40 +162,26 @@ impl Backend {
});

let loc = diagnostic.locate();
let filename = root.join(loc.filename);
let filename = match filename.canonicalize() {
Ok(filename) => filename,
Err(_) => {
self.client
.log_message(
MessageType::ERROR,
format!("Diagnostics for file {filename:?}, but its path could not be canonicalized"),
)
.await;
continue;
},
};
let uri = Url::from_file_path(filename).unwrap();
let uri = Url::from_file_path(loc.filename).unwrap();

let rope = match self.document_map.get(&uri) {
Some(rope) => rope,
None => {
// Can we somehow retrieve the file from the compiler rather than reading it again?
// Or have the compiler go through the lsp server file buffer instead of reading it from the file system?
let rope = Rope::from_str(&std::fs::read_to_string(uri.path()).unwrap());
self.document_map.insert(uri.clone(), rope.clone());
let contents = cached_read(&cache.file_cache, loc.filename).unwrap();
let rope = Rope::from_str(&contents);
self.document_map.insert(uri.clone(), rope);
self.document_map.get(&uri).unwrap()
},
};

// TODO: This range seems to be off
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly was off about it? Not sure if it's just fixed now because they seem fine to me:

image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an example I'm working on currently where the location is off:
image

Running the compiler on the file reports:
image

Where the line and column info are correct.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can ignore the contents of the file. I've found a good way of making Ante thread-safe while also allowing aliased mutability without lifetime annotations!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've determined this is because of larger unicode characters present in the file causing the internal byte index to be different than the character index.

let range = rope_range_to_lsp_range(loc.start.index..loc.end.index, &rope);
let message = format!("{}", diagnostic.display(&cache));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it looks like this change on master was accidentally reverted.


let diagnostic = Diagnostic {
code: None,
code_description: None,
data: None,
message: diagnostic.msg().to_string(),
message,
range,
related_information: None,
severity,
Expand All @@ -245,10 +197,18 @@ impl Backend {
};
}

join_all(
let handle = join_all(
diagnostics.into_iter().map(|(uri, diagnostics)| self.client.publish_diagnostics(uri, diagnostics, None)),
)
.await;
);

for (path, content) in cache.file_cache {
let uri = Url::from_file_path(path).unwrap();
if self.document_map.get(&uri).is_none() {
self.document_map.insert(uri.clone(), Rope::from_str(&content));
}
}

handle.await;
}
}

Expand Down
4 changes: 2 additions & 2 deletions examples/nameresolution/Trait.an
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ bar : Unit

// args: --check
// expected stderr:
// examples/nameresolution/Trait.an:7:5 error: baz is not required by Foo
// Trait.an:7:5 error: baz is not required by Foo
// baz = 2 // error: baz not in foo
//
// examples/nameresolution/Trait.an:5:1 error: impl is missing a definition for bar
// Trait.an:5:1 error: impl is missing a definition for bar
// impl Foo I32 String with
8 changes: 4 additions & 4 deletions examples/nameresolution/conflictingimport.an
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ library_fn _ = 3

// args: --check
// expected stderr:
// examples/nameresolution/conflictingimport.an:1:1 error: import shadows previous definition of library_fn
// conflictingimport.an:1:1 error: import shadows previous definition of library_fn
// import Library
//
// examples/nameresolution/conflictingimport.an:5:1 note: library_fn was previously defined here
// conflictingimport.an:5:1 note: library_fn was previously defined here
// library_fn _ = 3
//
// examples/nameresolution/conflictingimport.an:1:1 error: import shadows previous definition of library_int
// conflictingimport.an:1:1 error: import shadows previous definition of library_int
// import Library
//
// examples/nameresolution/conflictingimport.an:3:1 note: library_int was previously defined here
// conflictingimport.an:3:1 note: library_int was previously defined here
// library_int = 1
4 changes: 2 additions & 2 deletions examples/nameresolution/effects.an
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ handle ()

// args: --check
// expected stderr:
// examples/nameresolution/effects.an:12:1 error: Handler is missing 2 cases: two, three
// effects.an:12:1 error: Handler is missing 2 cases: two, three
// handle ()
//
// examples/nameresolution/effects.an:21:1 error: Handler is missing 3 cases: one, two, get
// effects.an:21:1 error: Handler is missing 3 cases: one, two, get
// handle ()
16 changes: 8 additions & 8 deletions examples/nameresolution/errors.an
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,26 @@ a = 3 // already declared

// args: --check
// expected stderr:
// examples/nameresolution/errors.an:15:1 error: a is already in scope
// errors.an:15:1 error: a is already in scope
// a = 3 // already declared
//
// examples/nameresolution/errors.an:14:1 note: a was previously defined here
// errors.an:14:1 note: a was previously defined here
// a = 2
//
// examples/nameresolution/errors.an:4:14 error: No declaration for `is_an_error` was found in scope
// errors.an:4:14 error: No declaration for `is_an_error` was found in scope
// not_an_error is_an_error
//
// examples/nameresolution/errors.an:6:15 error: No declaration for `c` was found in scope
// errors.an:6:15 error: No declaration for `c` was found in scope
// fn a b -> a + c + b
//
// examples/nameresolution/errors.an:9:9 warning: c is unused (prefix name with _ to silence this warning)
// errors.an:9:9 warning: c is unused (prefix name with _ to silence this warning)
// bar c d =
//
// examples/nameresolution/errors.an:9:11 warning: d is unused (prefix name with _ to silence this warning)
// errors.an:9:11 warning: d is unused (prefix name with _ to silence this warning)
// bar c d =
//
// examples/nameresolution/errors.an:8:5 warning: a is unused (prefix name with _ to silence this warning)
// errors.an:8:5 warning: a is unused (prefix name with _ to silence this warning)
// foo a b =
//
// examples/nameresolution/errors.an:9:5 warning: bar is unused (prefix name with _ to silence this warning)
// errors.an:9:5 warning: bar is unused (prefix name with _ to silence this warning)
// bar c d =
14 changes: 7 additions & 7 deletions examples/nameresolution/redeclare.an
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,23 @@ foo _ =

// args: --check
// expected stderr:
// examples/nameresolution/redeclare.an:2:1 error: a is already in scope
// redeclare.an:2:1 error: a is already in scope
// a = 2
//
// examples/nameresolution/redeclare.an:1:1 note: a was previously defined here
// redeclare.an:1:1 note: a was previously defined here
// a = 1
//
// examples/nameresolution/redeclare.an:3:1 error: a is already in scope
// redeclare.an:3:1 error: a is already in scope
// a = 3
//
// examples/nameresolution/redeclare.an:2:1 note: a was previously defined here
// redeclare.an:2:1 note: a was previously defined here
// a = 2
//
// examples/nameresolution/redeclare.an:6:5 warning: a is unused (prefix name with _ to silence this warning)
// redeclare.an:6:5 warning: a is unused (prefix name with _ to silence this warning)
// a = 4
//
// examples/nameresolution/redeclare.an:5:1 warning: foo is unused (prefix name with _ to silence this warning)
// redeclare.an:5:1 warning: foo is unused (prefix name with _ to silence this warning)
// foo _ =
//
// examples/nameresolution/redeclare.an:7:5 warning: a is unused (prefix name with _ to silence this warning)
// redeclare.an:7:5 warning: a is unused (prefix name with _ to silence this warning)
// a = 5
8 changes: 4 additions & 4 deletions examples/nameresolution/unused_warning.an
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ id x = error

// args: --check
// expected stderr:
// examples/nameresolution/unused_warning.an:4:1 error: id is already in scope
// unused_warning.an:4:1 error: id is already in scope
// id x = error
//
// examples/nameresolution/unused_warning.an:2:1 note: id was previously defined here
// unused_warning.an:2:1 note: id was previously defined here
// id x = x
//
// examples/nameresolution/unused_warning.an:4:8 error: No declaration for `error` was found in scope
// unused_warning.an:4:8 error: No declaration for `error` was found in scope
// id x = error
//
// examples/nameresolution/unused_warning.an:4:4 warning: x is unused (prefix name with _ to silence this warning)
// unused_warning.an:4:4 warning: x is unused (prefix name with _ to silence this warning)
// id x = error
2 changes: 1 addition & 1 deletion examples/parsing/invalid_integer_literal_suffix.an
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

// args: --check
// expected stderr:
// examples/parsing/invalid_integer_literal_suffix.an:1:1 error: Invalid suffix after integer literal. Expected an integer type like i32 or a space to separate the two tokens
// invalid_integer_literal_suffix.an:1:1 error: Invalid suffix after integer literal. Expected an integer type like i32 or a space to separate the two tokens
// 3_2_fdsa
2 changes: 1 addition & 1 deletion examples/parsing/parse_error.an
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ bar a b = if then else

// args: --parse --no-color
// expected stderr:
// examples/parsing/parse_error.an:2:9 error: Parser expected 'then' here
// parse_error.an:2:9 error: Parser expected 'then' here
// if true else
// ^^^^
2 changes: 1 addition & 1 deletion examples/regressions/146_invalid_int_type.an
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ a: Int String = 3

// args: --check
// expected stderr:
// examples/regressions/146_invalid_int_type.an:1:4 error: Type String is not an integer type
// 146_invalid_int_type.an:1:4 error: Type String is not an integer type
// a: Int String = 3
14 changes: 7 additions & 7 deletions examples/typechecking/completeness_checking.an
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,23 @@ match (1, 2, 3, 4)
// args: --check
// TODO: First error can be improved. Should be "Missing case Some _"
// expected stderr:
// examples/typechecking/completeness_checking.an:2:1 error: Missing case _
// completeness_checking.an:2:1 error: Missing case _
// match None
//
// examples/typechecking/completeness_checking.an:5:1 error: Missing case (_, None)
// completeness_checking.an:5:1 error: Missing case (_, None)
// match (2, None)
//
// examples/typechecking/completeness_checking.an:18:4 warning: Unreachable pattern
// completeness_checking.an:18:4 warning: Unreachable pattern
// | (1, 2) -> 1
//
// examples/typechecking/completeness_checking.an:16:1 error: Missing case (_ : Int, _)
// completeness_checking.an:16:1 error: Missing case (_ : Int, _)
// match (1, 2)
//
// examples/typechecking/completeness_checking.an:20:1 error: Missing case (true, true)
// completeness_checking.an:20:1 error: Missing case (true, true)
// match (true, true)
//
// examples/typechecking/completeness_checking.an:20:1 error: Missing case (false, false)
// completeness_checking.an:20:1 error: Missing case (false, false)
// match (true, true)
//
// examples/typechecking/completeness_checking.an:25:4 error: This pattern of type ((Int a), (Int b)) does not match the type ((Int a), ((Int b), ((Int c), (Int d)))) that is being matched on
// completeness_checking.an:25:4 error: This pattern of type ((Int a), (Int b)) does not match the type ((Int a), ((Int b), ((Int c), (Int d)))) that is being matched on
// | (1, 2) -> 1
2 changes: 1 addition & 1 deletion examples/typechecking/given_constraint_error.an
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ impl Print a given DoesNotExist a with

// args: --check
// expected stderr:
// examples/typechecking/given_constraint_error.an:2:20 error: Trait DoesNotExist was not found in scope
// given_constraint_error.an:2:20 error: Trait DoesNotExist was not found in scope
// impl Print a given DoesNotExist a with
2 changes: 1 addition & 1 deletion examples/typechecking/impl.an
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ c = foo "one" "two"

// args: --check --show-types
// expected stderr:
// examples/typechecking/impl.an:14:5 error: No impl found for Foo String
// impl.an:14:5 error: No impl found for Foo String
// c = foo "one" "two"
//

Expand Down
2 changes: 1 addition & 1 deletion examples/typechecking/member_access.an
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ foo_and_bar foo bar

// args: --check --show-types
// expected stderr:
// examples/typechecking/member_access.an:16:17 error: Expected argument of type { bar: String, .. }, but found Bar
// member_access.an:16:17 error: Expected argument of type { bar: String, .. }, but found Bar
// foo_and_bar foo bar
//

Expand Down
Loading
Loading