diff --git a/Cargo.lock b/Cargo.lock index a1e0199218..7dc78a1516 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1418,9 +1418,9 @@ dependencies = [ [[package]] name = "lsp-server" -version = "0.6.0" +version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f70570c1c29cf6654029b8fe201a5507c153f0d85be6f234d471d756bc36775a" +checksum = "248f65b78f6db5d8e1b1604b4098a28b43d21a8eb1deeca22b1c421b276c7095" dependencies = [ "crossbeam-channel", "log", @@ -1430,9 +1430,9 @@ dependencies = [ [[package]] name = "lsp-types" -version = "0.88.0" +version = "0.95.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8e8e042772e4e10b3785822f63c82399d0dd233825de44d2596f7fa86e023e0" +checksum = "158c1911354ef73e8fe42da6b10c0484cb65c7f1007f28022e847706c1ab6984" dependencies = [ "bitflags 1.3.2", "serde", diff --git a/Cargo.toml b/Cargo.toml index 1595e852a3..222e83d639 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -59,8 +59,8 @@ lalrpop-util = "0.19.9" lazy_static = "1" log = "0.4" logos = "0.12" -lsp-server = "0.6" -lsp-types = "0.88" +lsp-server = "0.7" +lsp-types = "0.95" malachite = "0.4" malachite-q = "0.4" md-5 = "0.10.5" diff --git a/lsp/lsp-harness/src/jsonrpc.rs b/lsp/lsp-harness/src/jsonrpc.rs index 7b200268d8..a2a68d06db 100644 --- a/lsp/lsp-harness/src/jsonrpc.rs +++ b/lsp/lsp-harness/src/jsonrpc.rs @@ -14,7 +14,7 @@ use lsp_types::{ ClientCapabilities, DidChangeTextDocumentParams, DidOpenTextDocumentParams, GotoDefinitionParams, GotoDefinitionResponse, InitializeParams, InitializedParams, Position, TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentPositionParams, Url, - VersionedTextDocumentIdentifier, + VersionedTextDocumentIdentifier, WorkDoneProgressParams, }; use std::{ io::{BufRead, BufReader, Read, Write}, @@ -166,6 +166,7 @@ impl Server { workspace_folders: None, client_info: None, locale: None, + work_done_progress_params: WorkDoneProgressParams::default(), })?; self.send_notification::(InitializedParams {}) } diff --git a/lsp/nls/src/diagnostic.rs b/lsp/nls/src/diagnostic.rs index ac863b89eb..6895bfb6f8 100644 --- a/lsp/nls/src/diagnostic.rs +++ b/lsp/nls/src/diagnostic.rs @@ -57,11 +57,11 @@ impl LocationCompat for lsp_types::Location { impl DiagnosticCompat for lsp_types::Diagnostic { fn from_codespan(diagnostic: Diagnostic, files: &mut Files) -> Vec { let severity = Some(match diagnostic.severity { - diagnostic::Severity::Bug => lsp_types::DiagnosticSeverity::Warning, - diagnostic::Severity::Error => lsp_types::DiagnosticSeverity::Error, - diagnostic::Severity::Warning => lsp_types::DiagnosticSeverity::Warning, - diagnostic::Severity::Note => lsp_types::DiagnosticSeverity::Information, - diagnostic::Severity::Help => lsp_types::DiagnosticSeverity::Hint, + diagnostic::Severity::Bug => lsp_types::DiagnosticSeverity::WARNING, + diagnostic::Severity::Error => lsp_types::DiagnosticSeverity::ERROR, + diagnostic::Severity::Warning => lsp_types::DiagnosticSeverity::WARNING, + diagnostic::Severity::Note => lsp_types::DiagnosticSeverity::INFORMATION, + diagnostic::Severity::Help => lsp_types::DiagnosticSeverity::HINT, }); diagnostic diff --git a/lsp/nls/src/field_walker.rs b/lsp/nls/src/field_walker.rs index 5f90b15f7a..46af775397 100644 --- a/lsp/nls/src/field_walker.rs +++ b/lsp/nls/src/field_walker.rs @@ -48,7 +48,7 @@ impl Record { .map(|(id, val)| CompletionItem { label: ident_quoted(id), detail: metadata_detail(&val.metadata), - kind: Some(CompletionItemKind::Property), + kind: Some(CompletionItemKind::PROPERTY), documentation: metadata_doc(&val.metadata), ident: Some((*id).into()), }) @@ -60,7 +60,7 @@ impl Record { RecordRowsIteratorItem::TailVar(_) => None, RecordRowsIteratorItem::Row(r) => Some(CompletionItem { label: ident_quoted(&r.id), - kind: Some(CompletionItemKind::Property), + kind: Some(CompletionItemKind::PROPERTY), detail: Some(r.typ.to_string()), ..Default::default() }), @@ -236,7 +236,7 @@ impl Def { CompletionItem { label: ident_quoted(&self.ident().into()), detail: self.metadata().and_then(metadata_detail), - kind: Some(CompletionItemKind::Property), + kind: Some(CompletionItemKind::PROPERTY), documentation: self.metadata().and_then(metadata_doc), ..Default::default() } diff --git a/lsp/nls/src/files.rs b/lsp/nls/src/files.rs index 64b8ee2ad9..c868c613e7 100644 --- a/lsp/nls/src/files.rs +++ b/lsp/nls/src/files.rs @@ -11,7 +11,7 @@ use lsp_types::{ }; use nickel_lang_core::{ cache::{CacheError, CacheOp, SourcePath}, - error::IntoDiagnostics, + error::{ImportError, IntoDiagnostics}, }; use crate::{ @@ -46,12 +46,32 @@ pub fn handle_open(server: &mut Server, params: DidOpenTextDocumentParams) -> Re }, ); let path = uri_to_path(¶ms.text_document.uri)?; + + // Invalidate the cache of every file that tried, but failed, to import a file + // with a name like this. + let invalid = path + .file_name() + .and_then(|name| server.failed_imports.remove(name)) + .unwrap_or_default(); + for rev_dep in &invalid { + server.analysis.remove(*rev_dep); + // Reset the cached state (Parsed is the earliest one) so that it will + // re-resolve its imports. + server + .cache + .update_state(*rev_dep, nickel_lang_core::cache::EntryState::Parsed); + } + let file_id = server .cache .add_string(SourcePath::Path(path), params.text_document.text); server.file_uris.insert(file_id, params.text_document.uri); parse_and_typecheck(server, file_id)?; + + for rev_dep in &invalid { + parse_and_typecheck(server, *rev_dep)?; + } Trace::reply(id); Ok(()) } @@ -97,6 +117,19 @@ pub fn handle_save(server: &mut Server, params: DidChangeTextDocumentParams) -> Ok(()) } +// Make a record of I/O errors in imports so that we can retry them when appropriate. +fn associate_failed_import(server: &mut Server, err: &nickel_lang_core::error::Error) { + if let nickel_lang_core::error::Error::ImportError(ImportError::IOError(name, _, pos)) = &err { + if let Some((filename, pos)) = PathBuf::from(name).file_name().zip(pos.into_opt()) { + server + .failed_imports + .entry(filename.to_owned()) + .or_default() + .insert(pos.src_id); + } + } +} + pub(crate) fn typecheck( server: &mut Server, file_id: FileId, @@ -112,7 +145,10 @@ pub(crate) fn typecheck( .map_err(|error| match error { CacheError::Error(tc_error) => tc_error .into_iter() - .flat_map(|err| err.into_diagnostics(server.cache.files_mut(), None)) + .flat_map(|err| { + associate_failed_import(server, &err); + err.into_diagnostics(server.cache.files_mut(), None) + }) .collect(), CacheError::NotParsed => unreachable!(), }) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index 970f429443..fea5aeb5d2 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -254,9 +254,9 @@ fn handle_import_completion( }) .map(|entry| { let kind = if entry.file { - CompletionItemKind::File + CompletionItemKind::FILE } else { - CompletionItemKind::Folder + CompletionItemKind::FOLDER }; lsp_types::CompletionItem { label: entry diff --git a/lsp/nls/src/requests/symbols.rs b/lsp/nls/src/requests/symbols.rs index b8b8e4c606..5c4d22be7d 100644 --- a/lsp/nls/src/requests/symbols.rs +++ b/lsp/nls/src/requests/symbols.rs @@ -33,7 +33,7 @@ pub fn handle_document_symbols( Some(DocumentSymbol { name: ident.ident.to_string(), detail: ty.map(Type::to_string), - kind: SymbolKind::Variable, + kind: SymbolKind::VARIABLE, tags: None, range, selection_range: range, diff --git a/lsp/nls/src/server.rs b/lsp/nls/src/server.rs index 2bf9304148..07fef4dab6 100644 --- a/lsp/nls/src/server.rs +++ b/lsp/nls/src/server.rs @@ -1,4 +1,7 @@ -use std::collections::HashMap; +use std::{ + collections::{HashMap, HashSet}, + ffi::OsString, +}; use anyhow::Result; use codespan::FileId; @@ -48,6 +51,14 @@ pub struct Server { pub analysis: AnalysisRegistry, pub initial_ctxt: Context, pub initial_term_env: crate::usage::Environment, + + /// A map associating imported files with failed imports. This allows us to + /// invalidate the cached version of a file when one of its imports becomes available. + /// + /// The keys in this map are the filenames (just the basename; no directory) of the + /// files that failed to import, and the values in this map are the file ids that tried + /// to import it. + pub failed_imports: HashMap>, } impl Server { @@ -56,7 +67,7 @@ impl Server { text_document_sync: Some(TextDocumentSyncCapability::Options( TextDocumentSyncOptions { open_close: Some(true), - change: Some(TextDocumentSyncKind::Full), + change: Some(TextDocumentSyncKind::FULL), ..TextDocumentSyncOptions::default() }, )), @@ -86,6 +97,11 @@ impl Server { pub fn new(connection: Connection) -> Server { let mut cache = Cache::new(ErrorTolerance::Tolerant); + + if let Ok(nickel_path) = std::env::var("NICKEL_IMPORT_PATH") { + cache.add_import_paths(nickel_path.split(':')); + } + // We don't recover from failing to load the stdlib for now. cache.load_stdlib().unwrap(); let initial_ctxt = cache.mk_type_ctxt().unwrap(); @@ -96,6 +112,7 @@ impl Server { analysis: AnalysisRegistry::default(), initial_ctxt, initial_term_env: crate::usage::Environment::new(), + failed_imports: HashMap::new(), } } diff --git a/lsp/nls/tests/main.rs b/lsp/nls/tests/main.rs index c2b6424a7c..3620f22665 100644 --- a/lsp/nls/tests/main.rs +++ b/lsp/nls/tests/main.rs @@ -22,3 +22,28 @@ fn check_snapshots(path: &str) { insta::assert_snapshot!(path, output); } + +#[test] +fn refresh_missing_imports() { + let _ = env_logger::try_init(); + let mut harness = TestHarness::new(); + + let url = |s: &str| lsp_types::Url::from_file_path(s).unwrap(); + harness.send_file(url("/test.ncl"), "import \"dep.ncl\""); + let diags = harness.wait_for_diagnostics().diagnostics; + assert_eq!(1, diags.len()); + assert!(diags[0].message.contains("import of dep.ncl failed")); + + // Now provide the import. + harness.send_file(url("/dep.ncl"), "42"); + + // Check that we get back clean diagnostics for both files. + // (LSP doesn't define the order, but we happen to know it) + let diags = harness.wait_for_diagnostics(); + assert_eq!(diags.uri.path(), "/dep.ncl"); + assert!(diags.diagnostics.is_empty()); + + let diags = harness.wait_for_diagnostics(); + assert_eq!(diags.uri.path(), "/test.ncl"); + assert!(diags.diagnostics.is_empty()); +}