From 5d946fbe5d25ad992655dbd8e36d5d2cab3287db Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:45:06 +0000 Subject: [PATCH 1/2] fix: report the missing `lsp` feature instead of panicking --- prqlc/prqlc/src/cli/mod.rs | 75 ++++++++++++++++++++++--------------- prqlc/prqlc/src/cli/test.rs | 17 +++++++++ 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/prqlc/prqlc/src/cli/mod.rs b/prqlc/prqlc/src/cli/mod.rs index d847e3d3ab6d..af0488aeea22 100644 --- a/prqlc/prqlc/src/cli/mod.rs +++ b/prqlc/prqlc/src/cli/mod.rs @@ -169,6 +169,12 @@ enum Command { ListTargets, /// Language Server Protocol + // + // Only does anything when built with the `lsp` feature, which is off by + // default. The variant stays present without it so that the help text and + // the shell completions don't vary with the feature set. (Hence a `//` + // comment — a second doc-comment paragraph would render as clap long help, + // which the completion snapshots pick up.) #[command(hide = true)] Lsp, @@ -344,6 +350,13 @@ impl Command { Ok(_) => Ok(()), Err(err) => Err(anyhow!(err)), }, + // Without the feature there's no server to start. This has to be + // handled here rather than falling through to `run_io_command`, + // which has no `IoArgs` to work with and would panic. + #[cfg(not(feature = "lsp"))] + Command::Lsp => Err(anyhow!( + "`lsp` requires `prqlc` to be built with the `lsp` feature" + )), _ => self.run_io_command(), } } @@ -357,7 +370,27 @@ impl Command { let (mut file_tree, main_path) = self.read_input()?; self.execute(&mut file_tree, &main_path) - .and_then(|buf| Ok(self.write_output(&buf)?)) + .and_then(|buf| self.write_output(&buf)) + } + + /// The [`IoArgs`] of the commands that read input and write output. + /// + /// `None` for the commands that handle their own IO, which [`Self::run`] + /// dispatches before falling through to [`Self::run_io_command`]. + fn io_args(&mut self) -> Option<&mut IoArgs> { + use Command::*; + match self { + Parse { io_args, .. } + | Lex { io_args, .. } + | Collect(io_args) + | Compile { io_args, .. } + | Debug(DebugCommand::Annotate(io_args) | DebugCommand::Lineage { io_args, .. }) + | Experimental( + ExperimentalCommand::GenerateDocs { io_args, .. } + | ExperimentalCommand::Highlight(io_args), + ) => Some(io_args), + _ => None, + } } fn execute<'a>(&self, sources: &'a mut SourceTree, main_path: &'a str) -> Result> { @@ -497,19 +530,9 @@ impl Command { // `input`, rather than matching on them and grabbing `input` from // `self`? But possibly if everything moves to `io_args`, then this is // quite reasonable? - use Command::*; - let io_args = match self { - Parse { io_args, .. } - | Lex { io_args, .. } - | Collect(io_args) - | Compile { io_args, .. } - | Debug(DebugCommand::Annotate(io_args) | DebugCommand::Lineage { io_args, .. }) => { - io_args - } - Experimental(ExperimentalCommand::GenerateDocs { io_args, .. }) => io_args, - Experimental(ExperimentalCommand::Highlight(io_args)) => io_args, - _ => unreachable!(), - }; + let io_args = self + .io_args() + .ok_or_else(|| anyhow!("internal error: command does not take input & output"))?; let input = &mut io_args.input; // Don't wait without a prompt when running `prqlc compile` — @@ -532,23 +555,13 @@ impl Command { Ok((sources, main_path)) } - fn write_output(&mut self, data: &[u8]) -> std::io::Result<()> { - use Command::{Collect, Compile, Debug, Experimental, Lex, Parse}; - let mut output = match self { - Parse { io_args, .. } - | Lex { io_args, .. } - | Collect(io_args) - | Compile { io_args, .. } - | Debug(DebugCommand::Annotate(io_args) | DebugCommand::Lineage { io_args, .. }) => { - io_args.output.clone() - } - Experimental(ExperimentalCommand::GenerateDocs { io_args, .. }) => { - io_args.output.clone() - } - Experimental(ExperimentalCommand::Highlight(io_args)) => io_args.output.clone(), - _ => unreachable!(), - }; - output.write_all(data) + fn write_output(&mut self, data: &[u8]) -> Result<()> { + let mut output = self + .io_args() + .ok_or_else(|| anyhow!("internal error: command does not take input & output"))? + .output + .clone(); + Ok(output.write_all(data)?) } } diff --git a/prqlc/prqlc/src/cli/test.rs b/prqlc/prqlc/src/cli/test.rs index 0dc64075aebf..0393b2a1ea9c 100644 --- a/prqlc/prqlc/src/cli/test.rs +++ b/prqlc/prqlc/src/cli/test.rs @@ -670,6 +670,23 @@ fn lex() { "#); } +/// `lsp` is off by default, and the subcommand exists either way — so before +/// the fix a default build dispatched `prqlc lsp` into the IO commands, which +/// have no `IoArgs` for it, and panicked on `unreachable!()`. It should report +/// the missing feature instead. +#[cfg(not(feature = "lsp"))] +#[test] +fn lsp_without_feature() { + assert_cmd_snapshot!(prqlc_command().args(["lsp"]), @" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + `lsp` requires `prqlc` to be built with the `lsp` feature + "); +} + #[cfg(feature = "lsp")] #[test] fn lsp() { From 4de693fa2eb3f099e402b4b1781c3b7439fac252 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:53:05 +0000 Subject: [PATCH 2/2] docs: add changelog entry for the `lsp` feature fix --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e60f4b6ac53c..0aae6781cb9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ **Fixes**: +- `prqlc lsp` now reports that `prqlc` was built without the `lsp` feature, + rather than panicking with `internal error: entered unreachable code`. The + subcommand is registered in every build, but the feature is off by default, so + the released binaries were affected. (@prql-bot, #6221) + - Deserializing an ident with an empty path now reports an error rather than panicking. A document containing `{"Ident":[]}` passed to `json::to_pl` or `json::to_rq` — reachable from the Python and JS bindings — hit