From fc6ec5bccac1e74dbffe5be6d78a00394b177dd8 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Mon, 15 Jul 2024 11:52:53 -0700 Subject: [PATCH] Use `PackageType::Lib` by default (#1737) In most places, we already used the `PackageType::Lib` by default, but not in the language service. With more library authoring expected in the future, this tweaks the user experience to avoid errors for missing entry point until the Q# program as run. As part of these changes, the behavior of the entry point check is updated to always provide feedback for other entry point related errors (like duplicates) but only return an error on missing entry point if the compilation is for an executable. Of note, this does NOT remove the configuration option for `packageType` in the qsharp.json, which we can do as a follow up if needed. This does update the error string that pops up when failing to run/debug the Q#. If the program is missing an entry point, a more specific error message is returned: ![image](https://github.com/user-attachments/assets/1e667ac3-dd3d-4c04-b393-49c8ceb1097d) and the generic compilation failure message is also updated: ![image](https://github.com/user-attachments/assets/78fa1b8c-5ad9-4806-b501-28e01a546049) --- compiler/qsc/src/interpret.rs | 5 +- compiler/qsc_frontend/src/incremental.rs | 6 ++ compiler/qsc_passes/src/entry_point.rs | 13 ++++- compiler/qsc_passes/src/entry_point/tests.rs | 4 +- compiler/qsc_passes/src/lib.rs | 9 +-- language_service/src/state.rs | 2 +- language_service/src/tests.rs | 60 ++------------------ npm/qsharp/test/basics.js | 4 ++ vscode/src/debugger/session.ts | 8 ++- 9 files changed, 40 insertions(+), 71 deletions(-) diff --git a/compiler/qsc/src/interpret.rs b/compiler/qsc/src/interpret.rs index 8ce79fb793..bdad76328f 100644 --- a/compiler/qsc/src/interpret.rs +++ b/compiler/qsc/src/interpret.rs @@ -359,10 +359,13 @@ impl Interpreter { ) -> InterpretResult { let label = self.next_line_label(); - let increment = self + let mut increment = self .compiler .compile_fragments_fail_fast(&label, fragments) .map_err(into_errors)?; + // Clear the entry expression, as we are evaluating fragments and a fragment with a `@EntryPoint` attribute + // should not change what gets executed. + increment.clear_entry(); self.eval_increment(receiver, increment) } diff --git a/compiler/qsc_frontend/src/incremental.rs b/compiler/qsc_frontend/src/incremental.rs index ea680a30eb..ffef95ad4f 100644 --- a/compiler/qsc_frontend/src/incremental.rs +++ b/compiler/qsc_frontend/src/incremental.rs @@ -52,6 +52,12 @@ pub struct Increment { pub hir: hir::Package, } +impl Increment { + pub fn clear_entry(&mut self) { + self.hir.entry = None; + } +} + impl Compiler { /// Creates a new compiler. pub fn new( diff --git a/compiler/qsc_passes/src/entry_point.rs b/compiler/qsc_passes/src/entry_point.rs index 28c0df81d7..c2b745711c 100644 --- a/compiler/qsc_passes/src/entry_point.rs +++ b/compiler/qsc_passes/src/entry_point.rs @@ -4,6 +4,8 @@ #[cfg(test)] mod tests; +use crate::PackageType; + use super::Error as PassErr; use miette::Diagnostic; use qsc_data_structures::span::Span; @@ -44,13 +46,14 @@ pub enum Error { pub(super) fn generate_entry_expr( package: &mut Package, assigner: &mut Assigner, + package_type: PackageType, ) -> Vec { if package.entry.is_some() { return vec![]; } let callables = get_callables(package); - match create_entry_from_callables(assigner, callables) { + match create_entry_from_callables(assigner, callables, package_type) { Ok(expr) => { package.entry = Some(expr); vec![] @@ -62,6 +65,7 @@ pub(super) fn generate_entry_expr( fn create_entry_from_callables( assigner: &mut Assigner, callables: Vec<(&CallableDecl, LocalItemId)>, + package_type: PackageType, ) -> Result> { if callables.len() == 1 { let ep = callables[0].0; @@ -110,7 +114,12 @@ fn create_entry_from_callables( Err(vec![PassErr::EntryPoint(Error::Args(ep.input.span))]) } } else if callables.is_empty() { - Err(vec![PassErr::EntryPoint(Error::NotFound)]) + if package_type == PackageType::Exe { + Err(vec![PassErr::EntryPoint(Error::NotFound)]) + } else { + // For libraries, no entry point is required. Leave the entry expression empty and return no errors. + Err(Vec::new()) + } } else { Err(callables .into_iter() diff --git a/compiler/qsc_passes/src/entry_point/tests.rs b/compiler/qsc_passes/src/entry_point/tests.rs index c40c5e3878..41d8b9ec9b 100644 --- a/compiler/qsc_passes/src/entry_point/tests.rs +++ b/compiler/qsc_passes/src/entry_point/tests.rs @@ -3,7 +3,7 @@ #![allow(clippy::needless_raw_string_hashes)] -use crate::entry_point::generate_entry_expr; +use crate::{entry_point::generate_entry_expr, PackageType}; use expect_test::{expect, Expect}; use indoc::indoc; use qsc_data_structures::{language_features::LanguageFeatures, target::TargetCapabilityFlags}; @@ -20,7 +20,7 @@ fn check(file: &str, expr: &str, expect: &Expect) { ); assert!(unit.errors.is_empty(), "{:?}", unit.errors); - let errors = generate_entry_expr(&mut unit.package, &mut unit.assigner); + let errors = generate_entry_expr(&mut unit.package, &mut unit.assigner, PackageType::Exe); if errors.is_empty() { expect.assert_eq( &unit diff --git a/compiler/qsc_passes/src/lib.rs b/compiler/qsc_passes/src/lib.rs index f142605c9c..a9d58e9c5d 100644 --- a/compiler/qsc_passes/src/lib.rs +++ b/compiler/qsc_passes/src/lib.rs @@ -105,13 +105,8 @@ impl PassContext { let conjugate_errors = conjugate_invert::invert_conjugate_exprs(core, package, assigner); Validator::default().visit_package(package); - let entry_point_errors = if package_type == PackageType::Exe { - let entry_point_errors = generate_entry_expr(package, assigner); - Validator::default().visit_package(package); - entry_point_errors - } else { - Vec::new() - }; + let entry_point_errors = generate_entry_expr(package, assigner, package_type); + Validator::default().visit_package(package); LoopUni { core, assigner }.visit_package(package); Validator::default().visit_package(package); diff --git a/language_service/src/state.rs b/language_service/src/state.rs index 6afd2737a2..4a3aaa49aa 100644 --- a/language_service/src/state.rs +++ b/language_service/src/state.rs @@ -69,7 +69,7 @@ impl Default for Configuration { fn default() -> Self { Self { target_profile: Profile::Unrestricted, - package_type: PackageType::Exe, + package_type: PackageType::Lib, language_features: LanguageFeatures::default(), lints_config: Vec::default(), } diff --git a/language_service/src/tests.rs b/language_service/src/tests.rs index 9b633c5971..fde96a5f39 100644 --- a/language_service/src/tests.rs +++ b/language_service/src/tests.rs @@ -29,22 +29,7 @@ async fn single_document() { &mut received_errors.borrow_mut(), "foo.qs", &(expect![[r#" - [ - ( - "foo.qs", - Some( - 1, - ), - [ - Pass( - EntryPoint( - NotFound, - ), - ), - ], - [], - ), - ] + [] "#]]), &(expect![[r#" SourceMap { @@ -78,22 +63,7 @@ async fn single_document_update() { &mut received_errors.borrow_mut(), "foo.qs", &(expect![[r#" - [ - ( - "foo.qs", - Some( - 1, - ), - [ - Pass( - EntryPoint( - NotFound, - ), - ), - ], - [], - ), - ] + [] "#]]), &(expect![[r#" SourceMap { @@ -124,16 +94,7 @@ async fn single_document_update() { &mut received_errors.borrow_mut(), "foo.qs", &(expect![[r#" - [ - ( - "foo.qs", - Some( - 1, - ), - [], - [], - ), - ] + [] "#]]), &(expect![[r#" SourceMap { @@ -177,20 +138,7 @@ async fn document_in_project() { &mut received_errors.borrow_mut(), "project/src/this_file.qs", &expect![[r#" - [ - ( - "project/qsharp.json", - None, - [ - Pass( - EntryPoint( - NotFound, - ), - ), - ], - [], - ), - ] + [] "#]], &expect![[r#" SourceMap { diff --git a/npm/qsharp/test/basics.js b/npm/qsharp/test/basics.js index 9eee53277c..4e5178e7f0 100644 --- a/npm/qsharp/test/basics.js +++ b/npm/qsharp/test/basics.js @@ -597,6 +597,10 @@ test("language service diagnostics - web worker", async () => { test("language service configuration update", async () => { const languageService = getLanguageServiceWorker(); + + // Set the configuration to expect an entry point. + await languageService.updateConfiguration({ packageType: "exe" }); + let actualMessages = []; languageService.addEventListener("diagnostics", (event) => { actualMessages.push({ diff --git a/vscode/src/debugger/session.ts b/vscode/src/debugger/session.ts index 95ab26253f..26a85a12cf 100644 --- a/vscode/src/debugger/session.ts +++ b/vscode/src/debugger/session.ts @@ -45,7 +45,9 @@ import { isPanelOpen } from "../webviewPanel"; import { FullProgramConfig } from "../programConfig"; const ErrorProgramHasErrors = - "program contains compile errors(s): cannot run. See debug console for more details."; + "The Q# program contains one or more compile errors and cannot run. See debug console for more details."; +const ErrorProgramMissingEntry = + "The Q# program does not contain an entry point and cannot run. See debug console for more details."; const SimulationCompleted = "Q# simulation completed."; const ConfigurationDelayMS = 1000; @@ -259,7 +261,9 @@ export class QscDebugSession extends LoggingDebugSession { this.writeToDebugConsole(this.failureMessage); this.sendErrorResponse(response, { id: -1, - format: ErrorProgramHasErrors, + format: this.failureMessage.includes("Qsc.EntryPoint.NotFound") + ? ErrorProgramMissingEntry + : ErrorProgramHasErrors, showUser: true, }); return;