Skip to content

Commit

Permalink
[move-compiler] Added support for virtual file system (#16330)
Browse files Browse the repository at this point in the history
## Description 

This PR adds support for virtual file system, with the intention of
allowing the compiler to build a package from files kept by the IDE in
memory (this feature is upcoming).

It also removes a certain awkwardness from the compiler implementation
where interface files had to be built in a temporary directory (now they
are built in an in-memory file system).

It also fixes an existing bug
(move-language/move#1082) where source files
used in the symbolicator (obtained from resolution graph) and source
files used by the compiler could be modified between the two uses
resulting in different file hashes which can ultimately lead to crash
when translating diagnostics (generated by the compiler and using
"compiler file hashes") using symbolicator source files (and
"symbolicator file hashes")

## Test Plan 

All existing tests must pass. Also, manually tested the IDE no longer
needs file saves to reflect changes in the file (e.g., compiler
diagnostics appear as the code is being typed).
  • Loading branch information
awelc authored Mar 1, 2024
1 parent 072f5d9 commit a995f9b
Show file tree
Hide file tree
Showing 29 changed files with 625 additions and 245 deletions.
17 changes: 17 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ once_cell = "1.7.2"
ouroboros = "0.17.2"
parking_lot = "0.11.1"
paste = "1.0.5"
pathdiff = "0.2.1"
petgraph = "0.5.1"
phf = { version = "0.11", features = ["macros"] }
plotters = { version = "0.3.0", default_features = false, features = ["evcxr", "line_series", "histogram"]}
Expand Down Expand Up @@ -104,6 +105,7 @@ tui = "0.17.0"
uint = "0.9.4"
url = "2.2.2"
variant_count = "1.1.0"
vfs = "0.10.0"
walkdir = "2.3.1"
whoami = { version = "1.2.1" }
x25519-dalek = { version = "0.1.0", package = "x25519-dalek-fiat", default-features = false, features = ["std", "u64_backend"] }
Expand Down
1 change: 1 addition & 0 deletions crates/move-analyzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ lsp-types.workspace = true
serde_json.workspace = true
tempfile.workspace = true
url.workspace = true
vfs.workspace = true
clap.workspace = true
crossbeam.workspace = true
move-command-line-common.workspace = true
Expand Down
53 changes: 32 additions & 21 deletions crates/move-analyzer/src/bin/move-analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ use std::{
};

use move_analyzer::{
completion::on_completion_request,
context::Context,
symbols,
vfs::{on_text_document_sync_notification, VirtualFileSystem},
completion::on_completion_request, context::Context, symbols,
vfs::on_text_document_sync_notification,
};
use url::Url;
use vfs::{impls::memory::MemoryFS, VfsPath};

#[derive(Parser)]
#[clap(author, version, about)]
Expand All @@ -48,9 +47,9 @@ fn main() {

let (connection, io_threads) = Connection::stdio();
let symbols = Arc::new(Mutex::new(symbols::empty_symbols()));
let mut context = Context {
let ide_files_root: VfsPath = MemoryFS::new().into();
let context = Context {
connection,
files: VirtualFileSystem::default(),
symbols: symbols.clone(),
};

Expand Down Expand Up @@ -125,7 +124,12 @@ fn main() {
.unwrap_or(false);
}

symbolicator_runner = symbols::SymbolicatorRunner::new(symbols.clone(), diag_sender, lint);
symbolicator_runner = symbols::SymbolicatorRunner::new(
ide_files_root.clone(),
symbols.clone(),
diag_sender,
lint,
);

// If initialization information from the client contains a path to the directory being
// opened, try to initialize symbols before sending response to the client. Do not bother
Expand All @@ -134,7 +138,9 @@ fn main() {
// to be available right after the client is initialized.
if let Some(uri) = initialize_params.root_uri {
if let Some(p) = symbols::SymbolicatorRunner::root_dir(&uri.to_file_path().unwrap()) {
if let Ok((Some(new_symbols), _)) = symbols::get_symbols(p.as_path(), lint) {
if let Ok((Some(new_symbols), _)) =
symbols::get_symbols(ide_files_root.clone(), p.as_path(), lint)
{
let mut old_symbols = symbols.lock().unwrap();
(*old_symbols).merge(new_symbols);
}
Expand Down Expand Up @@ -189,7 +195,8 @@ fn main() {
},
}
},
Err(error) => eprintln!("symbolicator message error: {:?}", error),
Err(error) =>
eprintln!("symbolicator message error: {:?}", error),
}
},
recv(context.connection.receiver) -> message => {
Expand All @@ -199,7 +206,7 @@ fn main() {
// a chance of completing pending requests (but should not accept new requests
// either which is handled inside on_requst) - instead it quits after receiving
// the exit notification from the client, which is handled below
shutdown_req_received = on_request(&context, &request, shutdown_req_received);
shutdown_req_received = on_request(&context, &request, ide_files_root.clone(), shutdown_req_received);
}
Ok(Message::Response(response)) => on_response(&context, &response),
Ok(Message::Notification(notification)) => {
Expand All @@ -210,7 +217,7 @@ fn main() {
// It ought to, especially once it begins processing requests that may
// take a long time to respond to.
}
_ => on_notification(&mut context, &symbolicator_runner, &notification),
_ => on_notification(ide_files_root.clone(), &symbolicator_runner, &notification),
}
}
Err(error) => eprintln!("IDE message error: {:?}", error),
Expand All @@ -228,7 +235,12 @@ fn main() {
/// The reason why this information is also passed as an argument is that according to the LSP
/// spec, if any additional requests are received after shutdownd then the LSP implementation
/// should respond with a particular type of error.
fn on_request(context: &Context, request: &Request, shutdown_request_received: bool) -> bool {
fn on_request(
context: &Context,
request: &Request,
ide_files_root: VfsPath,
shutdown_request_received: bool,
) -> bool {
if shutdown_request_received {
let response = lsp_server::Response::new_err(
request.id.clone(),
Expand All @@ -245,9 +257,12 @@ fn on_request(context: &Context, request: &Request, shutdown_request_received: b
return true;
}
match request.method.as_str() {
lsp_types::request::Completion::METHOD => {
on_completion_request(context, request, &context.symbols.lock().unwrap())
}
lsp_types::request::Completion::METHOD => on_completion_request(
context,
request,
ide_files_root.clone(),
&context.symbols.lock().unwrap(),
),
lsp_types::request::GotoDefinition::METHOD => {
symbols::on_go_to_def_request(context, request, &context.symbols.lock().unwrap());
}
Expand Down Expand Up @@ -286,7 +301,7 @@ fn on_response(_context: &Context, _response: &Response) {
}

fn on_notification(
context: &mut Context,
ide_files_root: VfsPath,
symbolicator_runner: &symbols::SymbolicatorRunner,
notification: &Notification,
) {
Expand All @@ -295,11 +310,7 @@ fn on_notification(
| lsp_types::notification::DidChangeTextDocument::METHOD
| lsp_types::notification::DidSaveTextDocument::METHOD
| lsp_types::notification::DidCloseTextDocument::METHOD => {
on_text_document_sync_notification(
&mut context.files,
symbolicator_runner,
notification,
)
on_text_document_sync_notification(ide_files_root, symbolicator_runner, notification)
}
_ => eprintln!("handle notification '{}' from client", notification.method),
}
Expand Down
64 changes: 36 additions & 28 deletions crates/move-analyzer/src/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use move_compiler::{
};
use move_symbol_pool::Symbol;
use std::{collections::HashSet, path::PathBuf};
use vfs::VfsPath;

/// Constructs an `lsp_types::CompletionItem` with the given `label` and `kind`.
fn completion_item(label: &str, kind: CompletionItemKind) -> CompletionItem {
Expand Down Expand Up @@ -149,7 +150,12 @@ fn get_cursor_token(buffer: &str, position: &Position) -> Option<Tok> {
/// Sends the given connection a response to a completion request.
///
/// The completions returned depend upon where the user's cursor is positioned.
pub fn on_completion_request(context: &Context, request: &Request, symbols: &Symbols) {
pub fn on_completion_request(
context: &Context,
request: &Request,
ide_files: VfsPath,
symbols: &Symbols,
) {
eprintln!("handling completion request");
let parameters = serde_json::from_value::<CompletionParams>(request.params.clone())
.expect("could not deserialize completion request");
Expand All @@ -160,38 +166,40 @@ pub fn on_completion_request(context: &Context, request: &Request, symbols: &Sym
.uri
.to_file_path()
.unwrap();
let buffer = context.files.get(&path);
if buffer.is_none() {
eprintln!(
"Could not read '{:?}' when handling completion request",
path
);
let mut buffer = String::new();
if let Ok(mut f) = ide_files.join(path.to_string_lossy()).unwrap().open_file() {
if f.read_to_string(&mut buffer).is_err() {
eprintln!(
"Could not read '{:?}' when handling completion request",
path
);
}
}

// The completion items we provide depend upon where the user's cursor is positioned.
let cursor =
buffer.and_then(|buf| get_cursor_token(buf, &parameters.text_document_position.position));

let mut items = vec![];
match cursor {
Some(Tok::Colon) => {
items.extend_from_slice(&primitive_types());
}
Some(Tok::Period) | Some(Tok::ColonColon) => {
// `.` or `::` must be followed by identifiers, which are added to the completion items
// below.
}
_ => {
// If the user's cursor is positioned anywhere other than following a `.`, `:`, or `::`,
// offer them Move's keywords, operators, and builtins as completion items.
items.extend_from_slice(&keywords());
items.extend_from_slice(&builtins());
if !buffer.is_empty() {
let cursor = get_cursor_token(buffer.as_str(), &parameters.text_document_position.position);
match cursor {
Some(Tok::Colon) => {
items.extend_from_slice(&primitive_types());
}
Some(Tok::Period) | Some(Tok::ColonColon) => {
// `.` or `::` must be followed by identifiers, which are added to the completion items
// below.
}
_ => {
// If the user's cursor is positioned anywhere other than following a `.`, `:`, or `::`,
// offer them Move's keywords, operators, and builtins as completion items.
items.extend_from_slice(&keywords());
items.extend_from_slice(&builtins());
}
}
}

if let Some(buffer) = &buffer {
let identifiers = identifiers(buffer, symbols, &path);
let identifiers = identifiers(buffer.as_str(), symbols, &path);
items.extend_from_slice(&identifiers);
} else {
// no file content
items.extend_from_slice(&keywords());
items.extend_from_slice(&builtins());
}

let result = serde_json::to_value(items).expect("could not serialize completion response");
Expand Down
4 changes: 1 addition & 3 deletions crates/move-analyzer/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use crate::{symbols::Symbols, vfs::VirtualFileSystem};
use crate::symbols::Symbols;
use lsp_server::Connection;
use std::sync::{Arc, Mutex};

/// The context within which the language server is running.
pub struct Context {
/// The connection with the language server's client.
pub connection: Connection,
/// The files that the language server is providing information about.
pub files: VirtualFileSystem,
/// Symbolication information
pub symbols: Arc<Mutex<Symbols>>,
}
Loading

0 comments on commit a995f9b

Please sign in to comment.