mirror of
https://github.com/tweag/nickel.git
synced 2024-10-05 15:47:33 +03:00
Make the LSP configurable (#1974)
* Make the LSP options configurable Take into account the configuration sent by the client through [`InitializeParams.initializationOptions`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#initializeParams). This is currently used to set the limits for the background evaluation, although it also creates the infrastructure for setting more things. * Fix the doc comments for the config * Remove a useless explicit Default instance Replace it by the derived one (thanks clippy) * Comment fixes Co-Authored-By: Yann Hamdaoui <yann.hamdaoui@tweag.io> * Simplify the nls config definition `#[serde(default)]` applied to a whole struct is enough for it to do the right thing with the `Default` trait. Makes the module significantly nicer to read. --------- Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
This commit is contained in:
parent
af5d1b557b
commit
32a4b675a4
@ -90,11 +90,11 @@ pub struct Response {
|
||||
}
|
||||
|
||||
impl Server {
|
||||
/// Launch a language server by running the given command.
|
||||
///
|
||||
/// The command's stdin and stdout will be overridden to "piped" (because
|
||||
/// that's what LSes do).
|
||||
pub fn new(mut cmd: std::process::Command) -> Result<Server> {
|
||||
/// Similar to `new`, but allows passing custom stuff
|
||||
pub fn new_with_options(
|
||||
mut cmd: std::process::Command,
|
||||
initialization_options: Option<serde_json::Value>,
|
||||
) -> Result<Server> {
|
||||
let lsp = cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).spawn()?;
|
||||
|
||||
let mut lsp = Server {
|
||||
@ -104,11 +104,19 @@ impl Server {
|
||||
id: 0,
|
||||
};
|
||||
|
||||
lsp.initialize()?;
|
||||
lsp.initialize(initialization_options)?;
|
||||
|
||||
Ok(lsp)
|
||||
}
|
||||
|
||||
/// Launch a language server by running the given command.
|
||||
///
|
||||
/// The command's stdin and stdout will be overridden to "piped" (because
|
||||
/// that's what LSes do).
|
||||
pub fn new(cmd: std::process::Command) -> Result<Server> {
|
||||
Server::new_with_options(cmd, None)
|
||||
}
|
||||
|
||||
/// Make the language server aware of a file.
|
||||
pub fn send_file(&mut self, uri: Url, contents: &str) -> Result<()> {
|
||||
self.send_notification::<DidOpenTextDocument>(DidOpenTextDocumentParams {
|
||||
@ -151,7 +159,7 @@ impl Server {
|
||||
self.send_notification::<Exit>(())
|
||||
}
|
||||
|
||||
fn initialize(&mut self) -> Result<()> {
|
||||
fn initialize(&mut self, initialization_options: Option<serde_json::Value>) -> Result<()> {
|
||||
// `root_path` is deprecated, but we need ot initialize the struct
|
||||
// somehow. There is no `Default` implementation for `InitilizeParams`
|
||||
// in versions of `lsp-types` compatible with `codespan-lsp`
|
||||
@ -160,7 +168,7 @@ impl Server {
|
||||
process_id: None,
|
||||
root_path: None,
|
||||
root_uri: None,
|
||||
initialization_options: None,
|
||||
initialization_options,
|
||||
capabilities: ClientCapabilities::default(),
|
||||
trace: None,
|
||||
workspace_folders: None,
|
||||
|
@ -126,14 +126,17 @@ impl Default for TestHarness {
|
||||
}
|
||||
|
||||
impl TestHarness {
|
||||
pub fn new() -> Self {
|
||||
pub fn new_with_options(initialization_options: Option<serde_json::Value>) -> Self {
|
||||
let cmd = std::process::Command::cargo_bin("nls").unwrap();
|
||||
let srv = Server::new(cmd).unwrap();
|
||||
let srv = Server::new_with_options(cmd, initialization_options).unwrap();
|
||||
Self {
|
||||
srv,
|
||||
out: Vec::new(),
|
||||
}
|
||||
}
|
||||
pub fn new() -> Self {
|
||||
Self::new_with_options(None)
|
||||
}
|
||||
|
||||
pub fn request<T: LspRequest>(&mut self, params: T::Params)
|
||||
where
|
||||
|
@ -16,14 +16,12 @@ use nickel_lang_core::{
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::{
|
||||
cache::CacheExt as _, diagnostic::SerializableDiagnostic, files::uri_to_path, world::World,
|
||||
cache::CacheExt as _, config, diagnostic::SerializableDiagnostic, files::uri_to_path,
|
||||
world::World,
|
||||
};
|
||||
|
||||
const EVAL_TIMEOUT: Duration = Duration::from_secs(1);
|
||||
const RECURSION_LIMIT: usize = 128;
|
||||
// The duration during which a file causing the evaluator to timeout will be blacklisted from further
|
||||
// evaluations
|
||||
const BLACKLIST_DURATION: Duration = Duration::from_secs(30);
|
||||
// Environment variable used to pass the recursion limit value to the child worker
|
||||
const RECURSION_LIMIT_ENV_VAR_NAME: &str = "NICKEL_NLS_RECURSION_LIMIT";
|
||||
|
||||
#[derive(Debug, Serialize, Deserialize)]
|
||||
enum Command {
|
||||
@ -106,7 +104,8 @@ pub fn worker_main() -> anyhow::Result<()> {
|
||||
// We've already checked that parsing and typechecking are successful, so we
|
||||
// don't expect further errors.
|
||||
let rt = vm.prepare_eval(file_id).unwrap();
|
||||
let errors = vm.eval_permissive(rt, RECURSION_LIMIT);
|
||||
let recursion_limit = std::env::var(RECURSION_LIMIT_ENV_VAR_NAME)?.parse::<usize>()?;
|
||||
let errors = vm.eval_permissive(rt, recursion_limit);
|
||||
diagnostics.extend(
|
||||
errors
|
||||
.into_iter()
|
||||
@ -142,12 +141,18 @@ struct SupervisorState {
|
||||
eval_stack: Vec<Url>,
|
||||
|
||||
// If evaluating a file causes the worker to time out or crash, we blacklist that file
|
||||
// and refuse to evaluate it for `BLACKLIST_DURATION`
|
||||
// and refuse to evaluate it for `self.config.blacklist_duration`
|
||||
banned_files: HashMap<Url, Instant>,
|
||||
|
||||
config: config::LspEvalConfig,
|
||||
}
|
||||
|
||||
impl SupervisorState {
|
||||
fn new(cmd_rx: Receiver<Command>, response_tx: Sender<Diagnostics>) -> anyhow::Result<Self> {
|
||||
fn new(
|
||||
cmd_rx: Receiver<Command>,
|
||||
response_tx: Sender<Diagnostics>,
|
||||
config: config::LspEvalConfig,
|
||||
) -> anyhow::Result<Self> {
|
||||
Ok(Self {
|
||||
cmd_rx,
|
||||
response_tx,
|
||||
@ -155,6 +160,7 @@ impl SupervisorState {
|
||||
deps: HashMap::new(),
|
||||
banned_files: HashMap::new(),
|
||||
eval_stack: Vec::new(),
|
||||
config,
|
||||
})
|
||||
}
|
||||
|
||||
@ -182,6 +188,10 @@ impl SupervisorState {
|
||||
fn eval(&self, uri: &Url) -> anyhow::Result<Diagnostics> {
|
||||
let path = std::env::current_exe()?;
|
||||
let mut child = std::process::Command::new(path)
|
||||
.env(
|
||||
RECURSION_LIMIT_ENV_VAR_NAME,
|
||||
self.config.eval_limits.recursion_limit.to_string(),
|
||||
)
|
||||
.arg("--background-eval")
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stdin(std::process::Stdio::piped())
|
||||
@ -214,7 +224,10 @@ impl SupervisorState {
|
||||
};
|
||||
bincode::serialize_into(&mut tx, &eval)?;
|
||||
|
||||
let result = run_with_timeout(move || bincode::deserialize_from(rx), EVAL_TIMEOUT);
|
||||
let result = run_with_timeout(
|
||||
move || bincode::deserialize_from(rx),
|
||||
self.config.eval_limits.timeout,
|
||||
);
|
||||
|
||||
Ok(result??)
|
||||
}
|
||||
@ -230,7 +243,8 @@ impl SupervisorState {
|
||||
}
|
||||
Command::EvalFile { uri } => {
|
||||
match self.banned_files.get(&uri) {
|
||||
Some(blacklist_time) if blacklist_time.elapsed() < BLACKLIST_DURATION => {}
|
||||
Some(blacklist_time)
|
||||
if blacklist_time.elapsed() < self.config.blacklist_duration => {}
|
||||
_ => {
|
||||
// If we re-request an evaluation, remove the old one. (This is quadratic in the
|
||||
// size of the eval stack, but it only contains unique entries so we don't expect it
|
||||
@ -285,10 +299,10 @@ impl SupervisorState {
|
||||
}
|
||||
|
||||
impl BackgroundJobs {
|
||||
pub fn new() -> Self {
|
||||
pub fn new(config: config::LspEvalConfig) -> Self {
|
||||
let (cmd_tx, cmd_rx) = crossbeam::channel::unbounded();
|
||||
let (diag_tx, diag_rx) = crossbeam::channel::unbounded();
|
||||
match SupervisorState::new(cmd_rx, diag_tx) {
|
||||
match SupervisorState::new(cmd_rx, diag_tx, config) {
|
||||
Ok(mut sup) => {
|
||||
std::thread::spawn(move || {
|
||||
sup.run();
|
||||
|
52
lsp/nls/src/config.rs
Normal file
52
lsp/nls/src/config.rs
Normal file
@ -0,0 +1,52 @@
|
||||
//! Configuration for the Nickel Language Server
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use std::time::Duration;
|
||||
|
||||
/**
|
||||
Limits to appy to the LSP background evaluator.
|
||||
If an evaluation reaches one of these limits, it will be canceled and the offending file will be
|
||||
temporarily blacklisted.
|
||||
*/
|
||||
#[derive(Debug, Deserialize, Serialize)]
|
||||
#[serde(default)]
|
||||
pub struct LspEvalLimits {
|
||||
/// Time out at which to cancel the background evaluation
|
||||
pub timeout: Duration,
|
||||
/// The maximum recursion level to allow in the background evaluator
|
||||
pub recursion_limit: usize,
|
||||
}
|
||||
impl Default for LspEvalLimits {
|
||||
fn default() -> Self {
|
||||
LspEvalLimits {
|
||||
timeout: Duration::from_secs(1),
|
||||
recursion_limit: 128,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The configuration of the LSP evaluator
|
||||
#[derive(Debug, Deserialize, Serialize)]
|
||||
#[serde(default)]
|
||||
pub struct LspEvalConfig {
|
||||
pub eval_limits: LspEvalLimits,
|
||||
/// The duration during which a file that broke the background evaluator will be blacklisted
|
||||
/// from it
|
||||
pub blacklist_duration: Duration,
|
||||
}
|
||||
|
||||
impl Default for LspEvalConfig {
|
||||
fn default() -> Self {
|
||||
LspEvalConfig {
|
||||
eval_limits: Default::default(),
|
||||
blacklist_duration: Duration::from_secs(30),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, Serialize, Default)]
|
||||
#[serde(default)]
|
||||
pub struct LspConfig {
|
||||
/// Configuration for the background evaluator in the LSP
|
||||
pub eval_config: LspEvalConfig,
|
||||
}
|
@ -12,6 +12,7 @@ mod background;
|
||||
mod cache;
|
||||
mod codespan_lsp;
|
||||
mod command;
|
||||
mod config;
|
||||
mod diagnostic;
|
||||
mod error;
|
||||
mod field_walker;
|
||||
@ -29,7 +30,7 @@ mod usage;
|
||||
mod utils;
|
||||
mod world;
|
||||
|
||||
use crate::trace::Trace;
|
||||
use crate::{config::LspConfig, trace::Trace};
|
||||
|
||||
#[derive(clap::Parser, Debug)]
|
||||
/// The language server of the Nickel language.
|
||||
@ -83,9 +84,17 @@ fn main() -> Result<()> {
|
||||
|
||||
let capabilities = Server::capabilities();
|
||||
|
||||
connection.initialize(serde_json::to_value(capabilities)?)?;
|
||||
let initialize_params = connection.initialize(serde_json::to_value(capabilities)?)?;
|
||||
|
||||
let _server = Server::new(connection).run();
|
||||
debug!("Raw InitializeParams: {:?}", initialize_params);
|
||||
let config = match initialize_params.get("initializationOptions") {
|
||||
Some(opts) => serde_json::from_value::<LspConfig>(opts.clone())?,
|
||||
None => LspConfig::default(),
|
||||
};
|
||||
|
||||
debug!("Parsed InitializeParams: {:?}", config);
|
||||
|
||||
let _server = Server::new(connection, config).run();
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
@ -19,6 +19,7 @@ use crate::{
|
||||
actions,
|
||||
background::BackgroundJobs,
|
||||
command,
|
||||
config::LspConfig,
|
||||
requests::{completion, formatting, goto, hover, rename, symbols},
|
||||
trace::Trace,
|
||||
world::World,
|
||||
@ -73,11 +74,11 @@ impl Server {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn new(connection: Connection) -> Server {
|
||||
pub fn new(connection: Connection, config: LspConfig) -> Server {
|
||||
Server {
|
||||
connection,
|
||||
world: World::default(),
|
||||
background_jobs: BackgroundJobs::new(),
|
||||
background_jobs: BackgroundJobs::new(config.eval_config),
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1,4 +1,6 @@
|
||||
use nickel_lang_utils::project_root::project_root;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use test_generator::test_resources;
|
||||
|
||||
use lsp_harness::{TestFixture, TestHarness};
|
||||
@ -111,3 +113,32 @@ fn reload_broken_imports() {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_client_options() {
|
||||
let _ = env_logger::try_init();
|
||||
let lsp_options = json!({
|
||||
"eval_config": {
|
||||
"eval_limits": {
|
||||
"recursion_limit": 1
|
||||
}
|
||||
}
|
||||
});
|
||||
let mut harness = TestHarness::new_with_options(Some(lsp_options));
|
||||
let url = |s: &str| lsp_types::Url::from_file_path(s).unwrap();
|
||||
harness.send_file(
|
||||
url("/test.ncl"),
|
||||
"{ C = fun n => if n == 0 then String else C (n - 1), res = 2 | C 5 }",
|
||||
);
|
||||
|
||||
// Typecheck diagnostics. Empty because there's nothing to error on
|
||||
let diags = harness.wait_for_diagnostics();
|
||||
assert!(diags.diagnostics.is_empty());
|
||||
|
||||
// Evaluator diagnostics.
|
||||
// These shouldn't be empty (because `C 5 == String` so `2 | C 5` is a contract
|
||||
// violation), but they are because `recursion_limit` is too low for the evaluator to be able
|
||||
// to compute that.
|
||||
let diags = harness.wait_for_diagnostics();
|
||||
assert!(diags.diagnostics.is_empty());
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user