From a13a2ba91f3a1441a015eafe5226f54133a24c9f Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 31 Jan 2024 16:05:24 -0800 Subject: [PATCH 01/10] Pull Location struct to a more common place --- compiler/qsc/src/lib.rs | 1 + compiler/qsc/src/location.rs | 256 +++++++++++++++++++++++ language_service/src/definition.rs | 2 +- language_service/src/definition/tests.rs | 17 +- language_service/src/hover/tests.rs | 2 +- language_service/src/lib.rs | 7 +- language_service/src/protocol.rs | 6 - language_service/src/qsc_utils.rs | 43 +--- language_service/src/references.rs | 2 +- language_service/src/references/tests.rs | 22 +- language_service/src/rename.rs | 6 +- language_service/src/rename/tests.rs | 6 +- language_service/src/test_utils.rs | 22 +- wasm/src/language_service.rs | 20 +- wasm/src/line_column.rs | 9 + 15 files changed, 323 insertions(+), 98 deletions(-) create mode 100644 compiler/qsc/src/location.rs diff --git a/compiler/qsc/src/lib.rs b/compiler/qsc/src/lib.rs index ef7b8e80f8..d11b577e4b 100644 --- a/compiler/qsc/src/lib.rs +++ b/compiler/qsc/src/lib.rs @@ -8,6 +8,7 @@ pub mod compile; pub mod error; pub mod incremental; pub mod interpret; +pub mod location; pub mod target; pub use qsc_frontend::compile::{ diff --git a/compiler/qsc/src/location.rs b/compiler/qsc/src/location.rs new file mode 100644 index 0000000000..2518ad1a09 --- /dev/null +++ b/compiler/qsc/src/location.rs @@ -0,0 +1,256 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use std::sync::Arc; + +use qsc_data_structures::{ + line_column::{Encoding, Range}, + span::Span, +}; +use qsc_frontend::compile::PackageStore; +use qsc_hir::hir::PackageId; + +pub const QSHARP_LIBRARY_URI_SCHEME: &str = "qsharp-library-source"; + +/// Describes a location in source code in terms of a source name and [`Range`]. +#[derive(Debug, PartialEq, Clone)] +pub struct Location { + pub source: Arc, + pub range: Range, +} + +impl Location { + /// Creates a [`Location`] from a package ID and a SourceMap-relative span. + /// + /// To differentiate user sources from library sources, this function takes + /// a `user_package_id` parameter which denotes the user package. + /// All other packages in the package store are assumed to be library packages. + /// Source names from library packages are prepended with a unique URI scheme. + #[must_use] + pub fn from( + span: Span, + package_id: PackageId, + package_store: &PackageStore, + user_package_id: PackageId, + position_encoding: Encoding, + ) -> Self { + let source = package_store + .get(package_id) + .expect("package id must exist in store") + .sources + .find_by_offset(span.lo) + .expect("source should exist for offset"); + + let source_name = if package_id == user_package_id { + source.name.clone() + } else { + // Currently the only supported external packages are our library packages, + // URI's to which need to include our custom library scheme. + format!("{}:{}", QSHARP_LIBRARY_URI_SCHEME, source.name).into() + }; + + Location { + source: source_name, + range: Range::from_span(position_encoding, &source.contents, &(span - source.offset)), + } + } +} + +#[cfg(test)] +mod tests { + use crate::compile; + use expect_test::expect; + use qsc_data_structures::{line_column::Encoding, span::Span}; + use qsc_frontend::compile::{PackageStore, RuntimeCapabilityFlags, SourceMap}; + use qsc_hir::hir::PackageId; + use qsc_passes::PackageType; + + use super::Location; + + #[test] + fn from_std_span() { + let (store, std_package_id, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 0, hi: 1 }, + std_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + expect![[r#" + Location { + source_name: "qsharp-library-source:arrays.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 1, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_core_span() { + let (store, _, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 0, hi: 1 }, + PackageId::CORE, + &store, + user_package_id, + Encoding::Utf8, + ); + + expect![[r#" + Location { + source_name: "qsharp-library-source:core/core.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 1, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_user_span() { + let (store, _, user_package_id) = compile_package(); + + let bar_start_offset = store + .get(user_package_id) + .expect("expected to find user package") + .sources + .find_by_name("bar.qs") + .expect("expected to find source") + .offset; + + let location = Location::from( + Span { + lo: bar_start_offset, + hi: bar_start_offset + 1, + }, + user_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + expect![[r#" + Location { + source_name: "bar.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 1, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_out_of_bounds_lo() { + let (store, _, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 1000, hi: 2000 }, + user_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + // Per [`Range`] spec, out of bounds positions map to EOF + expect![[r#" + Location { + source_name: "bar.qs", + range: Range { + start: Position { + line: 0, + column: 17, + }, + end: Position { + line: 0, + column: 17, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_out_of_bounds_hi() { + let (store, _, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 0, hi: 2000 }, + user_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + // Per [`Range`] spec, out of bounds positions map to EOF + expect![[r#" + Location { + source_name: "foo.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 17, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + fn compile_package() -> (PackageStore, PackageId, PackageId) { + let mut store = PackageStore::new(compile::core()); + let mut dependencies = Vec::new(); + + let (package_type, capabilities) = (PackageType::Lib, RuntimeCapabilityFlags::all()); + + let std = compile::std(&store, capabilities); + let std_package_id = store.insert(std); + + dependencies.push(std_package_id); + let sources = SourceMap::new( + [ + ("foo.qs".into(), "namespace Foo { }".into()), + ("bar.qs".into(), "namespace Bar { }".into()), + ], + None, + ); + let (unit, _) = + compile::compile(&store, &dependencies, sources, package_type, capabilities); + let user_package_id = store.insert(unit); + + (store, std_package_id, user_package_id) + } +} diff --git a/language_service/src/definition.rs b/language_service/src/definition.rs index ddc1621382..61cf021ccb 100644 --- a/language_service/src/definition.rs +++ b/language_service/src/definition.rs @@ -6,11 +6,11 @@ mod tests; use crate::compilation::Compilation; use crate::name_locator::{Handler, Locator, LocatorContext}; -use crate::protocol::Location; use crate::qsc_utils::into_location; use qsc::ast::visit::Visitor; use qsc::hir::PackageId; use qsc::line_column::{Encoding, Position}; +use qsc::location::Location; use qsc::{ast, hir, Span}; pub(crate) fn get_definition( diff --git a/language_service/src/definition/tests.rs b/language_service/src/definition/tests.rs index a364ac5ee6..8ccee134b7 100644 --- a/language_service/src/definition/tests.rs +++ b/language_service/src/definition/tests.rs @@ -4,10 +4,10 @@ #![allow(clippy::needless_raw_string_hashes)] use expect_test::{expect, Expect}; +use qsc::location::Location; use super::get_definition; use crate::{ - protocol::Location, test_utils::{ compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers, }, @@ -26,8 +26,8 @@ fn assert_definition(source_with_markers: &str) { None } else { Some(Location { - source: "".to_string(), - span: target_spans[0], + source: "".into(), + range: target_spans[0], }) }; assert_eq!(&expected_definition, &actual_definition); @@ -40,10 +40,7 @@ fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) { let expected_definition = if target_spans.is_empty() { None } else { - Some(Location { - source: target_spans[0].0.clone(), - span: target_spans[0].1, - }) + Some(target_spans[0].clone()) }; assert_eq!(&expected_definition, &actual_definition); } @@ -302,7 +299,7 @@ fn std_call() { Some( Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 1, column: 26, @@ -410,7 +407,7 @@ fn std_udt() { Some( Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 24, @@ -442,7 +439,7 @@ fn std_udt_udt_field() { Some( Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 31, diff --git a/language_service/src/hover/tests.rs b/language_service/src/hover/tests.rs index 0f4ec3595f..fef0da84a5 100644 --- a/language_service/src/hover/tests.rs +++ b/language_service/src/hover/tests.rs @@ -37,7 +37,7 @@ fn check_notebook(cells_with_markers: &[(&str, &str)], expect: &Expect) { let actual = get_hover(&compilation, &cell_uri, position, Encoding::Utf8).expect("Expected a hover."); - assert_eq!(&actual.span, &target_spans[0].1); + assert_eq!(&actual.span, &target_spans[0].range); expect.assert_eq(&actual.contents); } diff --git a/language_service/src/lib.rs b/language_service/src/lib.rs index 03b6355284..98bf10b53f 100644 --- a/language_service/src/lib.rs +++ b/language_service/src/lib.rs @@ -27,10 +27,13 @@ use futures::channel::mpsc::{unbounded, UnboundedReceiver, UnboundedSender}; use futures_util::StreamExt; use log::{trace, warn}; use protocol::{ - CompletionList, DiagnosticUpdate, Hover, Location, NotebookMetadata, SignatureHelp, + CompletionList, DiagnosticUpdate, Hover, NotebookMetadata, SignatureHelp, WorkspaceConfigurationUpdate, }; -use qsc::line_column::{Encoding, Position, Range}; +use qsc::{ + line_column::{Encoding, Position, Range}, + location::Location, +}; use qsc_project::JSFileEntry; use state::{CompilationState, CompilationStateUpdater}; use std::{cell::RefCell, fmt::Debug, future::Future, pin::Pin, rc::Rc, sync::Arc}; diff --git a/language_service/src/protocol.rs b/language_service/src/protocol.rs index 190b5f2db2..781432675a 100644 --- a/language_service/src/protocol.rs +++ b/language_service/src/protocol.rs @@ -86,12 +86,6 @@ impl Hash for CompletionItem { } } -#[derive(Debug, PartialEq)] -pub struct Location { - pub source: String, - pub span: Range, -} - #[derive(Debug, PartialEq)] pub struct Hover { pub contents: String, diff --git a/language_service/src/qsc_utils.rs b/language_service/src/qsc_utils.rs index 384b6513ae..940addc944 100644 --- a/language_service/src/qsc_utils.rs +++ b/language_service/src/qsc_utils.rs @@ -1,15 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::{ - compilation::Compilation, - protocol::{self}, -}; +use crate::compilation::Compilation; use qsc::line_column::{Encoding, Range}; +use qsc::location::Location; use qsc::{ast, hir::PackageId, SourceMap, Span}; -pub(crate) const QSHARP_LIBRARY_URI_SCHEME: &str = "qsharp-library-source"; - pub(crate) fn span_contains(span: Span, offset: u32) -> bool { offset >= span.lo && offset < span.hi } @@ -39,33 +35,16 @@ pub(crate) fn into_range(encoding: Encoding, span: Span, source_map: &SourceMap) pub(crate) fn into_location( position_encoding: Encoding, compilation: &Compilation, - location: Span, + span: Span, package_id: PackageId, -) -> protocol::Location { - let source_map = &compilation - .package_store - .get(package_id) - .expect("package id must exist in store") - .sources; - let source = source_map - .find_by_offset(location.lo) - .expect("source should exist for offset"); - let source_name = if package_id == compilation.user_package_id { - source.name.to_string() - } else { - // Currently the only supported external packages are our library packages, - // URI's to which need to include our custom library scheme. - format!("{}:{}", QSHARP_LIBRARY_URI_SCHEME, source.name) - }; - - protocol::Location { - source: source_name, - span: Range::from_span( - position_encoding, - &source.contents, - &(location - source.offset), - ), - } +) -> Location { + Location::from( + span, + package_id, + &compilation.package_store, + compilation.user_package_id, + position_encoding, + ) } pub(crate) fn find_ident<'a>( diff --git a/language_service/src/references.rs b/language_service/src/references.rs index 0a265e64c8..44931bc7c5 100644 --- a/language_service/src/references.rs +++ b/language_service/src/references.rs @@ -8,12 +8,12 @@ use std::rc::Rc; use crate::compilation::{Compilation, Lookup}; use crate::name_locator::{Handler, Locator, LocatorContext}; -use crate::protocol::Location; use crate::qsc_utils::into_location; use qsc::ast::visit::{walk_callable_decl, walk_expr, walk_ty, Visitor}; use qsc::hir::ty::Ty; use qsc::hir::{PackageId, Res}; use qsc::line_column::{Encoding, Position}; +use qsc::location::Location; use qsc::{ast, hir, resolve, Span}; pub(crate) fn get_references( diff --git a/language_service/src/references/tests.rs b/language_service/src/references/tests.rs index 4ebf3a1e48..531a80647f 100644 --- a/language_service/src/references/tests.rs +++ b/language_service/src/references/tests.rs @@ -5,7 +5,6 @@ use super::get_references; use crate::{ - protocol, test_utils::{ compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers, }, @@ -43,7 +42,7 @@ fn check(source_with_markers: &str, include_declaration: bool) { include_declaration, ) .into_iter() - .map(|l| l.span) + .map(|l| l.range) .collect::>(); for target in &target_spans { assert!( @@ -71,10 +70,7 @@ fn check_notebook_exclude_decl(cells_with_markers: &[(&str, &str)]) { .collect::>(); for target in &target_spans { assert!( - actual.contains(&protocol::Location { - source: target.0.clone(), - span: target.1 - }), + actual.contains(target), "expected {actual:?} to contain {target:?}" ); } @@ -98,7 +94,7 @@ fn std_callable_ref() { [ Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 1, column: 26, @@ -111,7 +107,7 @@ fn std_callable_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 3, column: 8, @@ -124,7 +120,7 @@ fn std_callable_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 5, column: 8, @@ -255,7 +251,7 @@ fn std_udt_ref() { [ Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 24, @@ -268,7 +264,7 @@ fn std_udt_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 2, column: 22, @@ -345,7 +341,7 @@ fn std_field_ref() { [ Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 31, @@ -358,7 +354,7 @@ fn std_field_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 4, column: 23, diff --git a/language_service/src/rename.rs b/language_service/src/rename.rs index c2934dd07d..b0bef3e7b6 100644 --- a/language_service/src/rename.rs +++ b/language_service/src/rename.rs @@ -6,11 +6,11 @@ mod tests; use crate::compilation::{Compilation, Lookup}; use crate::name_locator::{Handler, Locator, LocatorContext}; -use crate::protocol::Location; use crate::qsc_utils::into_range; use crate::references::ReferenceFinder; use qsc::ast::visit::Visitor; use qsc::line_column::{Encoding, Position, Range}; +use qsc::location::Location; use qsc::{ast, hir, resolve, Span}; pub(crate) fn prepare_rename( @@ -126,9 +126,9 @@ impl<'a> Rename<'a> { .for_ty_param(param_id, current_callable) .into_iter() .map(|l| { - assert!(!l.span.empty(), "Type parameter name is empty"); + assert!(!l.range.empty(), "Type parameter name is empty"); Location { - span: type_param_ident_range(l.span), + range: type_param_ident_range(l.range), ..l } }) diff --git a/language_service/src/rename/tests.rs b/language_service/src/rename/tests.rs index 85f06b99f6..0e730b6e04 100644 --- a/language_service/src/rename/tests.rs +++ b/language_service/src/rename/tests.rs @@ -20,7 +20,7 @@ fn check(source_with_markers: &str) { compile_with_fake_stdlib_and_markers(source_with_markers); let actual = get_rename(&compilation, "", cursor_position, Encoding::Utf8) .into_iter() - .map(|l| l.span) + .map(|l| l.range) .collect::>(); for target in &target_spans { assert!(actual.contains(target)); @@ -391,7 +391,7 @@ fn notebook_rename_across_cells() { [ Location { source: "cell1", - span: Range { + range: Range { start: Position { line: 0, column: 10, @@ -404,7 +404,7 @@ fn notebook_rename_across_cells() { }, Location { source: "cell2", - span: Range { + range: Range { start: Position { line: 0, column: 0, diff --git a/language_service/src/test_utils.rs b/language_service/src/test_utils.rs index 115a3fe099..611c255a62 100644 --- a/language_service/src/test_utils.rs +++ b/language_service/src/test_utils.rs @@ -9,6 +9,7 @@ use qsc::{ hir::PackageId, incremental::Compiler, line_column::{Encoding, Position, Range}, + location::Location, target::Profile, PackageStore, PackageType, SourceMap, Span, }; @@ -21,13 +22,13 @@ pub(crate) fn compile_with_fake_stdlib_and_markers( ( compilation, cursor_offset, - target_spans.iter().map(|(_, s)| *s).collect(), + target_spans.iter().map(|l| l.range).collect(), ) } pub(crate) fn compile_project_with_fake_stdlib_and_markers( sources_with_markers: &[(&str, &str)], -) -> (Compilation, String, Position, Vec<(String, Range)>) { +) -> (Compilation, String, Position, Vec) { let (sources, cursor_uri, cursor_offset, target_spans) = get_sources_and_markers(sources_with_markers); @@ -58,7 +59,7 @@ pub(crate) fn compile_project_with_fake_stdlib_and_markers( pub(crate) fn compile_notebook_with_fake_stdlib_and_markers( cells_with_markers: &[(&str, &str)], -) -> (Compilation, String, Position, Vec<(String, Range)>) { +) -> (Compilation, String, Position, Vec) { let (cells, cell_uri, offset, target_spans) = get_sources_and_markers(cells_with_markers); let compilation = @@ -160,12 +161,7 @@ fn compile_fake_stdlib() -> (PackageStore, PackageId) { #[allow(clippy::type_complexity)] fn get_sources_and_markers( sources: &[(&str, &str)], -) -> ( - Vec<(Arc, Arc)>, - String, - Position, - Vec<(String, Range)>, -) { +) -> (Vec<(Arc, Arc)>, String, Position, Vec) { let (mut cursor_uri, mut cursor_offset, mut target_spans) = (None, None, Vec::new()); let sources = sources .iter() @@ -189,10 +185,10 @@ fn get_sources_and_markers( } if !targets.is_empty() { for span in target_offsets_to_spans(&targets) { - target_spans.push(( - s.0.to_string(), - Range::from_span(Encoding::Utf8, &source, &span), - )); + target_spans.push(Location { + source: s.0.into(), + range: Range::from_span(Encoding::Utf8, &source, &span), + }); } } (Arc::from(s.0), Arc::from(source.as_ref())) diff --git a/wasm/src/language_service.rs b/wasm/src/language_service.rs index 6313556d95..2e4b8e8190 100644 --- a/wasm/src/language_service.rs +++ b/wasm/src/language_service.rs @@ -239,10 +239,13 @@ impl LanguageService { let mut renames: FxHashMap> = FxHashMap::default(); locations.into_iter().for_each(|l| { - renames.entry(l.source).or_default().push(TextEdit { - range: l.span.into(), - newText: new_name.to_string(), - }) + renames + .entry(l.source.to_string()) + .or_default() + .push(TextEdit { + range: l.range.into(), + newText: new_name.to_string(), + }) }); let workspace_edit = WorkspaceEdit { @@ -265,15 +268,6 @@ impl LanguageService { } } -impl From for Location { - fn from(location: qsls::protocol::Location) -> Self { - Location { - source: location.source, - span: location.span.into(), - } - } -} - serializable_type! { WorkspaceConfiguration, { diff --git a/wasm/src/line_column.rs b/wasm/src/line_column.rs index d1a905076a..7c7a771d25 100644 --- a/wasm/src/line_column.rs +++ b/wasm/src/line_column.rs @@ -70,3 +70,12 @@ impl From for Range { } } } + +impl From for Location { + fn from(location: qsc::location::Location) -> Self { + Location { + source: location.source.to_string(), + span: location.range.into(), + } + } +} From df14c57dd5dd7a7938893077889db466db7ab542 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 31 Jan 2024 16:05:40 -0800 Subject: [PATCH 02/10] Eliminate FileAccessor in the debugger --- compiler/qsc/src/interpret.rs | 27 ++-- vscode/src/common.ts | 8 -- vscode/src/debugger/activate.ts | 104 +++++++-------- vscode/src/debugger/session.ts | 230 +++++++++++++++----------------- wasm/src/debug_service.rs | 13 +- 5 files changed, 176 insertions(+), 206 deletions(-) diff --git a/compiler/qsc/src/interpret.rs b/compiler/qsc/src/interpret.rs index cb01fa9e07..0bd11a24cf 100644 --- a/compiler/qsc/src/interpret.rs +++ b/compiler/qsc/src/interpret.rs @@ -19,6 +19,7 @@ pub use qsc_eval::{ use crate::{ error::{self, WithStack}, incremental::Compiler, + location::Location, }; use debug::format_call_stack; use miette::Diagnostic; @@ -431,25 +432,15 @@ impl Debugger { Global::Udt => "udt".into(), }; - let hir_package = self - .interpreter - .compiler - .package_store() - .get(map_fir_package_to_hir(frame.id.package)) - .expect("package should exist"); - let source = hir_package - .sources - .find_by_offset(frame.span.lo) - .expect("frame should have a source"); - let path = source.name.to_string(); StackFrame { name, functor, - path, - range: Range::from_span( + location: Location::from( + frame.span, + map_fir_package_to_hir(frame.id.package), + self.interpreter.compiler.package_store(), + map_fir_package_to_hir(self.interpreter.source_package), self.position_encoding, - &source.contents, - &(frame.span - source.offset), ), } }) @@ -536,10 +527,8 @@ pub struct StackFrame { pub name: String, /// The functor of the callable. pub functor: String, - /// The path of the source file. - pub path: String, - /// The source range of the call site. - pub range: Range, + /// The source location of the call site. + pub location: Location, } #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] diff --git a/vscode/src/common.ts b/vscode/src/common.ts index 6baf9294b4..29217adc69 100644 --- a/vscode/src/common.ts +++ b/vscode/src/common.ts @@ -21,14 +21,6 @@ export function isQsharpNotebookCell(document: TextDocument): boolean { export const qsharpExtensionId = "qsharp-vscode"; -export interface FileAccessor { - normalizePath(path: string): string; - convertToWindowsPathSeparator(path: string): string; - resolvePathToUri(path: string): Uri; - openPath(path: string): Promise; - openUri(uri: Uri): Promise; -} - export function basename(path: string): string | undefined { return path.replace(/\/+$/, "").split("/").pop(); } diff --git a/vscode/src/debugger/activate.ts b/vscode/src/debugger/activate.ts index 86e3455f19..c0a101ebc8 100644 --- a/vscode/src/debugger/activate.ts +++ b/vscode/src/debugger/activate.ts @@ -3,8 +3,8 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ -import { IDebugServiceWorker, getDebugServiceWorker } from "qsharp-lang"; -import { qsharpExtensionId, isQsharpDocument, FileAccessor } from "../common"; +import { IDebugServiceWorker, getDebugServiceWorker, log } from "qsharp-lang"; +import { qsharpExtensionId, isQsharpDocument } from "../common"; import { QscDebugSession } from "./session"; import { getRandomGuid } from "../utils"; @@ -49,13 +49,18 @@ function registerCommands(context: vscode.ExtensionContext) { } if (targetResource) { + // We'll omit config.program and let the configuration + // resolver fill it in with the currently open editor's URI. + // This will also let us correctly handle untitled files + // where the save prompt pops up before the debugger is launched, + // potentially causing the active editor URI to change if + // the file is saved with a different name. vscode.debug.startDebugging( undefined, { type: "qsharp", name: "Run Q# File", request: "launch", - program: targetResource.toString(), shots: 1, stopOnEntry: false, }, @@ -73,11 +78,16 @@ function registerCommands(context: vscode.ExtensionContext) { } if (targetResource) { + // We'll omit config.program and let the configuration + // resolver fill it in with the currently open editor's URI. + // This will also let us correctly handle untitled files + // where the save prompt pops up before the debugger is launched, + // potentially causing the active editor URI to change if + // the file is saved with a different name. vscode.debug.startDebugging(undefined, { type: "qsharp", name: "Debug Q# File", request: "launch", - program: targetResource.toString(), shots: 1, stopOnEntry: true, noDebug: false, @@ -101,29 +111,51 @@ class QsDebugConfigProvider implements vscode.DebugConfigurationProvider { config.type = "qsharp"; config.name = "Launch"; config.request = "launch"; - config.program = editor.document.uri.toString(); + config.programUri = editor.document.uri.toString(); config.shots = 1; config.noDebug = "noDebug" in config ? config.noDebug : false; config.stopOnEntry = !config.noDebug; } + } else if (config.program && folder) { + // A program is specified in launch.json. + // + // Variable substitution is a bit odd in VS Code. Variables such as + // ${file} and ${workspaceFolder} are expanded to absolute filesystem + // paths with platform-specific separators. To correctly convert them + // back to a URI, we need to use the vscode.Uri.file constructor. + // + // However, this gives us the URI scheme file:// , which is not correct + // when the workspace uses a virtual filesystem such as qsharp-vfs:// + // or vscode-test-web://. So now we also need the workspace folder URI + // to use as the basis for our file URI. + // + // Examples of program paths that can come through variable substitution: + // C:\foo\bar.qs + // \foo\bar.qs + // /foo/bar.qs + const fileUri = vscode.Uri.file(config.program); + config.programUri = folder.uri + .with({ + path: fileUri.path, + }) + .toString(); } else { - // we have a launch config, resolve the program path - - // ensure we have the program uri correctly formatted - // this is a user specified path. - if (config.program) { - const uri = workspaceFileAccessor.resolvePathToUri(config.program); - config.program = uri.toString(); - } else { - // Use the active editor if no program or ${file} is specified. - const editor = vscode.window.activeTextEditor; - if (editor && isQsharpDocument(editor.document)) { - config.program = editor.document.uri.toString(); - } + // Use the active editor if no program is specified. + const editor = vscode.window.activeTextEditor; + if (editor && isQsharpDocument(editor.document)) { + config.programUri = editor.document.uri.toString(); } } - if (!config.program) { + log.trace( + `resolveDebugConfigurationWithSubstitutedVariables config.program=${ + config.program + } folder.uri=${folder?.uri.toString()} config.programUri=${ + config.programUri + }`, + ); + + if (!config.programUri) { // abort launch return vscode.window .showInformationMessage("Cannot find a Q# program to debug") @@ -157,35 +189,6 @@ class QsDebugConfigProvider implements vscode.DebugConfigurationProvider { } } -// The path normalization, fallbacks, and uri resolution are necessary -// due to https://github.com/microsoft/vscode-debugadapter-node/issues/298 -// We can't specify that the debug adapter should use Uri for paths and can't -// use the DebugSession conversion functions because they don't work in the web. -export const workspaceFileAccessor: FileAccessor = { - normalizePath(path: string): string { - return path.replace(/\\/g, "/"); - }, - convertToWindowsPathSeparator(path: string): string { - return path.replace(/\//g, "\\"); - }, - resolvePathToUri(path: string): vscode.Uri { - const normalizedPath = this.normalizePath(path); - return vscode.Uri.parse(normalizedPath, false); - }, - async openPath(path: string): Promise { - const uri: vscode.Uri = this.resolvePathToUri(path); - return this.openUri(uri); - }, - async openUri(uri: vscode.Uri): Promise { - try { - return await vscode.workspace.openTextDocument(uri); - } catch { - const path = this.convertToWindowsPathSeparator(uri.toString()); - return await vscode.workspace.openTextDocument(vscode.Uri.file(path)); - } - }, -}; - class InlineDebugAdapterFactory implements vscode.DebugAdapterDescriptorFactory { @@ -194,12 +197,9 @@ class InlineDebugAdapterFactory _executable: vscode.DebugAdapterExecutable | undefined, ): Promise { const worker = debugServiceWorkerFactory(); - const uri = workspaceFileAccessor.resolvePathToUri( - session.configuration.program, - ); + const uri = vscode.Uri.parse(session.configuration.programUri); const sources = await loadProject(uri); const qscSession = new QscDebugSession( - workspaceFileAccessor, worker, session.configuration, sources, diff --git a/vscode/src/debugger/session.ts b/vscode/src/debugger/session.ts index 28789fbdf3..0d2602d776 100644 --- a/vscode/src/debugger/session.ts +++ b/vscode/src/debugger/session.ts @@ -21,12 +21,7 @@ import { Handles, Scope, } from "@vscode/debugadapter"; -import { - FileAccessor, - basename, - isQsharpDocument, - toVscodeRange, -} from "../common"; +import { basename, isQsharpDocument, toVscodeRange } from "../common"; import { DebugProtocol } from "@vscode/debugprotocol"; import { IDebugServiceWorker, @@ -34,7 +29,6 @@ import { StepResultId, IStructStepResult, QscEventTarget, - qsharpLibraryUriScheme, } from "qsharp-lang"; import { createDebugConsoleEventTarget } from "./output"; import { ILaunchRequestArguments } from "./types"; @@ -68,6 +62,8 @@ interface IBreakpointLocationData { export class QscDebugSession extends LoggingDebugSession { private static threadID = 1; + private readonly knownPaths = new Map(); + private breakpointLocations: Map; private breakpoints: Map; private variableHandles = new Handles<"locals" | "quantum">(); @@ -76,7 +72,6 @@ export class QscDebugSession extends LoggingDebugSession { private supportsVariableType = false; public constructor( - private fileAccessor: FileAccessor, private debugService: IDebugServiceWorker, private config: vscode.DebugConfiguration, private sources: [string, string][], @@ -92,6 +87,19 @@ export class QscDebugSession extends LoggingDebugSession { this.breakpoints = new Map(); this.setDebuggerLinesStartAt1(false); this.setDebuggerColumnsStartAt1(false); + + for (const source of sources) { + const uri = vscode.Uri.parse(source[0], true); + + // In Debug Protocol requests, the VS Code debug adapter client + // will strip file URIs to just the filesystem path. + // Keep track of the filesystem paths we know about so that + // we can resolve them back to the original URI when handling requests. + // See `asUri()` for more details. + if (uri.scheme === "file") { + this.knownPaths.set(uri.fsPath, uri.toString()); + } + } } public async init(associationId: string): Promise { @@ -459,27 +467,15 @@ export class QscDebugSession extends LoggingDebugSession { breakpoints: [], }; - const file = await this.fileAccessor - .openPath(args.source.path ?? "") - .catch((e) => { - log.trace(`Failed to open file: ${e}`); - const fileUri = this.fileAccessor.resolvePathToUri( - args.source.path ?? "", - ); - log.trace( - "breakpointLocationsRequest, target file: " + fileUri.toString(), - ); - return undefined; - }); + const doc = await this.tryLoadSource(args.source); + log.trace( + `breakpointLocationsRequest: path=${args.source.path} resolved to ${doc?.uri}`, + ); - // If we couldn't find the file, or it wasn't a Q# file, or - // the range is longer than the file, just return + // If we couldn't find the document, or if + // the range is longer than the document, just return const targetLineNumber = this.convertClientLineToDebugger(args.line); - if ( - !file || - !isQsharpDocument(file) || - targetLineNumber >= file.lineCount - ) { + if (!doc || targetLineNumber >= doc.lineCount) { log.trace(`setBreakPointsResponse: %O`, response); this.sendResponse(response); return; @@ -489,7 +485,7 @@ export class QscDebugSession extends LoggingDebugSession { // everything from `file` is 0 based, everything from `args` is 1 based // so we have to convert anything from `args` to 0 based - const line = file.lineAt(targetLineNumber); + const line = doc.lineAt(targetLineNumber); const lineRange = line.range; // If the column isn't specified, it is a line breakpoint so that we // use the whole line's range for breakpoint finding. @@ -525,7 +521,7 @@ export class QscDebugSession extends LoggingDebugSession { // column offset, so we need to check if the startOffset is within range. const bps = this.breakpointLocations - .get(file.uri.toString()) + .get(doc.uri.toString()) ?.filter((bp) => isLineBreakpoint ? bp.range.start.line == requestRange.start.line @@ -554,31 +550,25 @@ export class QscDebugSession extends LoggingDebugSession { ): Promise { log.trace(`setBreakPointsRequest: %O`, args); - const file = await this.fileAccessor - .openPath(args.source.path ?? "") - .catch((e) => { - log.trace(`setBreakPointsRequest - Failed to open file: ${e}`); - const fileUri = this.fileAccessor.resolvePathToUri( - args.source.path ?? "", - ); - log.trace("setBreakPointsRequest, target file: " + fileUri.toString()); - return undefined; - }); + const doc = await this.tryLoadSource(args.source); + log.trace( + `setBreakPointsRequest: path=${args.source.path} resolved to ${doc?.uri}`, + ); - // If we couldn't find the file, or it wasn't a Q# file, just return - if (!file || !isQsharpDocument(file)) { + // If we couldn't find the document, just return + if (!doc) { log.trace(`setBreakPointsResponse: %O`, response); this.sendResponse(response); return; } log.trace(`setBreakPointsRequest: looking`); - this.breakpoints.set(file.uri.toString(), []); + this.breakpoints.set(doc.uri.toString(), []); log.trace( `setBreakPointsRequest: files in cache %O`, this.breakpointLocations.keys(), ); - const locations = this.breakpointLocations.get(file.uri.toString()) ?? []; + const locations = this.breakpointLocations.get(doc.uri.toString()) ?? []; log.trace(`setBreakPointsRequest: got locations %O`, locations); const desiredBpOffsets: { range: vscode.Range; @@ -588,12 +578,12 @@ export class QscDebugSession extends LoggingDebugSession { .filter( (sourceBreakpoint) => this.convertClientLineToDebugger(sourceBreakpoint.line) < - file.lineCount, + doc.lineCount, ) .map((sourceBreakpoint) => { const isLineBreakpoint = !sourceBreakpoint.column; const line = this.convertClientLineToDebugger(sourceBreakpoint.line); - const lineRange = file.lineAt(line).range; + const lineRange = doc.lineAt(line).range; const startCol = sourceBreakpoint.column ? this.convertClientColumnToDebugger(sourceBreakpoint.column) : lineRange.start.character; @@ -635,7 +625,7 @@ export class QscDebugSession extends LoggingDebugSession { } // Update our breakpoint list for the given file - this.breakpoints.set(file.uri.toString(), bps); + this.breakpoints.set(doc.uri.toString(), bps); response.body = { breakpoints: bps, @@ -666,79 +656,28 @@ export class QscDebugSession extends LoggingDebugSession { const mappedStackFrames = await Promise.all( debuggerStackFrames .map(async (f, id) => { - log.trace(`frames: path %O`, f.path); - - const file = await this.fileAccessor - .openPath(f.path ?? "") - .catch((e) => { - log.error(`stackTraceRequest - Failed to open file: ${e}`); - const fileUri = this.fileAccessor.resolvePathToUri(f.path ?? ""); - log.trace( - "stackTraceRequest, target file: " + fileUri.toString(), - ); - }); - if (file) { - log.trace(`frames: file %O`, file); - const sf: DebugProtocol.StackFrame = new StackFrame( - id, - f.name, - new Source( - basename(f.path) ?? f.path, - file.uri.toString(true), - undefined, - undefined, - "qsharp-adapter-data", - ), - this.convertDebuggerLineToClient(f.range.start.line), - this.convertDebuggerColumnToClient(f.range.start.character), - ); - sf.endLine = this.convertDebuggerLineToClient(f.range.end.line); - sf.endColumn = this.convertDebuggerColumnToClient( - f.range.end.character, - ); - return sf; - } else { - try { - // This file isn't part of the workspace, so we'll - // create a URI which can try to load it from the core and std lib - // There is a custom content provider subscribed to this scheme. - // Opening the text document by that uri will use the content - // provider to look for the source code. - const uri = vscode.Uri.from({ - scheme: qsharpLibraryUriScheme, - path: f.path, - }); - const source = new Source( - basename(f.path) ?? f.path, - uri.toString(), - 0, - "internal core/std library", - "qsharp-adapter-data", - ) as DebugProtocol.Source; - const sf = new StackFrame( - id, - f.name, - source as Source, - this.convertDebuggerLineToClient(f.range.start.line), - this.convertDebuggerColumnToClient(f.range.start.character), - ); - sf.endLine = this.convertDebuggerLineToClient(f.range.end.line); - sf.endColumn = this.convertDebuggerColumnToClient( - f.range.end.character, - ); - - return sf as DebugProtocol.StackFrame; - } catch (e: any) { - log.warn(e.message); - return new StackFrame( - id, - f.name, - undefined, - undefined, - undefined, - ); - } - } + log.trace(`frames: location %O`, f.location); + + const sf: DebugProtocol.StackFrame = new StackFrame( + id, + f.name, + new Source( + basename(f.location.source) ?? f.location.source, + f.location.source, + undefined, + undefined, + "qsharp-adapter-data", + ), + this.convertDebuggerLineToClient(f.location.span.start.line), + this.convertDebuggerColumnToClient(f.location.span.start.character), + ); + sf.endLine = this.convertDebuggerLineToClient( + f.location.span.end.line, + ); + sf.endColumn = this.convertDebuggerColumnToClient( + f.location.span.end.character, + ); + return sf; }) .filter(filterUndefined), ); @@ -874,4 +813,57 @@ export class QscDebugSession extends LoggingDebugSession { ); this.sendEvent(evt); } + + /** + * Attempts to find the Source in the current session and returns the + * TextDocument if it exists. + * + * This method *may* return a valid result even when the requested + * path does not belong in the current program (e.g. another Q# file + * in the workspace). + */ + async tryLoadSource(source: DebugProtocol.Source) { + if (!source.path) { + return; + } + + const uri = this.asUri(source.path); + if (!uri) { + return; + } + + try { + const doc = await vscode.workspace.openTextDocument(uri); + if (!isQsharpDocument(doc)) { + return; + } + return doc; + } catch (e) { + log.trace(`Failed to open ${uri}: ${e}`); + } + } + + /** + * Attemps to resolve a DebugProtocol.Source.path to a URI. + * + * This method will attempt to undo the lossy conversion + * from URI to path that the VS Code debug adapter client performs. + * See: https://github.com/microsoft/vscode/blob/3246d63177e1e5ae211029e7ab0021c33342a3c7/src/vs/workbench/contrib/debug/common/debugSource.ts#L90 + * + * If the path was originally constructed from a file:// URI, it's a + * filesystem path. We use our knownPaths map to resolve it back to a URI. + * Filesystem paths we don't know about *won't* be resolved, and that's ok. + * + * If the path was originally constructed from a URI, it's a URI, + * use it directly. + */ + asUri(pathOrUri: string): vscode.Uri | undefined { + pathOrUri = this.knownPaths.get(pathOrUri) || pathOrUri; + + try { + return vscode.Uri.parse(pathOrUri); + } catch (e) { + log.trace(`Could not resolve path ${pathOrUri}`); + } + } } diff --git a/wasm/src/debug_service.rs b/wasm/src/debug_service.rs index 849ecef361..b782a4f9fd 100644 --- a/wasm/src/debug_service.rs +++ b/wasm/src/debug_service.rs @@ -8,7 +8,7 @@ use qsc::interpret::{Debugger, Error, StepAction, StepResult}; use qsc::line_column::Encoding; use qsc::{fmt_complex, target::Profile}; -use crate::line_column::Range; +use crate::line_column::{Location, Range}; use crate::{get_source_map, serializable_type, CallbackReceiver}; use serde::{Deserialize, Serialize}; use serde_json::json; @@ -67,11 +67,10 @@ impl DebugService { StackFrameList { frames: frames - .iter() + .into_iter() .map(|s| StackFrame { name: format!("{} {}", s.name, s.functor), - path: s.path.clone(), - range: s.range.into(), + location: s.location.into(), }) .collect(), } @@ -326,13 +325,11 @@ serializable_type! { StackFrame, { pub name: String, - pub path: String, - pub range: Range + pub location: Location }, r#"export interface IStackFrame { name: string; - path: string; - range: IRange; + location: ILocation }"# } From 18ba7c5039f734b47ab6a28d48982af935bafc27 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 1 Feb 2024 16:00:11 -0800 Subject: [PATCH 03/10] Debugger integration tests --- vscode/test/runTests.mjs | 2 +- vscode/test/suites/debugger/debugger.test.ts | 478 ++++++++++++++++++ vscode/test/suites/debugger/extension.test.ts | 15 - vscode/test/suites/debugger/index.ts | 2 +- .../debugger/test-workspace/qsharp.json | 1 + .../suites/debugger/test-workspace/src/bar.qs | 5 + .../suites/debugger/test-workspace/src/foo.qs | 10 + vscode/test/suites/extensionUtils.ts | 63 +++ .../suites/language-service/notebook.test.ts | 38 +- 9 files changed, 569 insertions(+), 45 deletions(-) create mode 100644 vscode/test/suites/debugger/debugger.test.ts delete mode 100644 vscode/test/suites/debugger/extension.test.ts create mode 100644 vscode/test/suites/debugger/test-workspace/qsharp.json create mode 100644 vscode/test/suites/debugger/test-workspace/src/bar.qs create mode 100644 vscode/test/suites/debugger/test-workspace/src/foo.qs diff --git a/vscode/test/runTests.mjs b/vscode/test/runTests.mjs index 210c38527d..51f044ca8d 100644 --- a/vscode/test/runTests.mjs +++ b/vscode/test/runTests.mjs @@ -39,7 +39,7 @@ try { // Debugger tests await runSuite( join(thisDir, "out", "debugger", "index"), - join(thisDir, "..", "..", "samples"), + join(thisDir, "suites", "debugger", "test-workspace"), ); } catch (err) { console.error("Failed to run tests", err); diff --git a/vscode/test/suites/debugger/debugger.test.ts b/vscode/test/suites/debugger/debugger.test.ts new file mode 100644 index 0000000000..072c33132b --- /dev/null +++ b/vscode/test/suites/debugger/debugger.test.ts @@ -0,0 +1,478 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import * as vscode from "vscode"; +import { assert } from "chai"; +import { activateExtension, waitForCondition } from "../extensionUtils"; +import { DebugProtocol } from "@vscode/debugprotocol"; + +suite("Q# Debugger Tests", function suite() { + const workspaceFolder = + vscode.workspace.workspaceFolders && vscode.workspace.workspaceFolders[0]; + assert(workspaceFolder, "Expecting an open folder"); + const fooUri = vscode.Uri.joinPath(workspaceFolder.uri, "src", "foo.qs"); + const barUri = vscode.Uri.joinPath(workspaceFolder.uri, "src", "bar.qs"); + + let tracker: Tracker | undefined; + let disposable; + + this.beforeAll(async () => { + await activateExtension(); + }); + + this.beforeEach(async () => { + tracker = new Tracker(); + disposable = vscode.debug.registerDebugAdapterTrackerFactory("qsharp", { + createDebugAdapterTracker(): vscode.ProviderResult { + return tracker; + }, + }); + }); + + this.afterEach(async () => { + disposable.dispose(); + tracker = undefined; + vscode.debug.removeBreakpoints(vscode.debug.breakpoints); + await terminateSession(); + vscode.commands.executeCommand("workbench.action.closeAllEditors"); + }); + + test("Launch with debugEditorContents command", async () => { + await vscode.window.showTextDocument(fooUri); + + // launch debugger + await vscode.commands.executeCommand("qsharp-vscode.debugEditorContents"); + + await waitUntilStopped([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 9, + name: "Foo ", + endLine: 5, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + }); + + test("Launch with launch.json configuration - workspaceFolder substitution", async () => { + // The DebugConfiguration object is what would go in launch.json, + // pass it in directly here + vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${workspaceFolder}src/foo.qs", + }); + + await waitUntilStopped([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 9, + name: "Foo ", + endLine: 5, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + }); + + test("Launch with launch.json configuration - file substitution", async () => { + await vscode.window.showTextDocument(fooUri); + + // ${file} will expand to the filesystem path of the currently opened file + vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${file}", + }); + + await waitUntilStopped([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 9, + name: "Foo ", + endLine: 5, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + }); + + test("Set breakpoint in main file", async () => { + // Set a breakpoint on line 6 of foo.qs (5 when 0-indexed) + await vscode.debug.addBreakpoints([ + new vscode.SourceBreakpoint( + new vscode.Location(fooUri, new vscode.Position(5, 0)), + ), + ]); + + // launch debugger + vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${workspaceFolder}src/foo.qs", + stopOnEntry: false, + }); + + // should hit the breakpoint we set above + await waitUntilStopped([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 6, + column: 9, + name: "Foo ", + endLine: 6, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + }); + + test("Set breakpoint in other file", async () => { + // Set a breakpoint on line 3 of bar.qs (2 when 0-indexed) + await vscode.debug.addBreakpoints([ + new vscode.SourceBreakpoint( + new vscode.Location(barUri, new vscode.Position(2, 0)), + ), + ]); + + // launch debugger + vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${workspaceFolder}src/foo.qs", + stopOnEntry: false, + }); + + // should hit the breakpoint we set above + await waitUntilStopped([ + { + id: 1, + source: { + name: "bar.qs", + path: "vscode-test-web://mount/src/bar.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 3, + column: 9, + name: "Bar ", + endLine: 3, + endColumn: 26, + }, + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 12, + name: "Foo ", + endLine: 5, + endColumn: 14, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + + // text editor should now be open on bar.qs + await waitForTextEditorOn(barUri); + }); + + test("Step into other file", async () => { + // launch debugger + vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${workspaceFolder}src/foo.qs", + stopOnEntry: true, + }); + + // should break on entry (per debug config above) + await waitUntilStopped([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 9, + name: "Foo ", + endLine: 5, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + + // step into call (will be a call into bar.qs) + await vscode.commands.executeCommand("workbench.action.debug.stepInto"); + + await waitUntilStopped([ + { + id: 1, + source: { + name: "bar.qs", + path: "vscode-test-web://mount/src/bar.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 3, + column: 9, + name: "Bar ", + endLine: 3, + endColumn: 26, + }, + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 12, + name: "Foo ", + endLine: 5, + endColumn: 14, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + + // text editor should now be open on bar.qs + await waitForTextEditorOn(barUri); + }); + + test("Step into standard lib", async () => { + // Set a breakpoint on line 8 of foo.qs (7 when 0-indexed) + // This will be a call into stdlib + await vscode.debug.addBreakpoints([ + new vscode.SourceBreakpoint( + new vscode.Location(fooUri, new vscode.Position(7, 0)), + ), + ]); + + // launch debugger + vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${workspaceFolder}src/foo.qs", + stopOnEntry: false, + }); + + // should hit the breakpoint we set above + await waitUntilStopped([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 8, + column: 9, + name: "Foo ", + endLine: 8, + endColumn: 14, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + + // step into call (will be a call into intrinsic.qs) + await vscode.commands.executeCommand("workbench.action.debug.stepInto"); + + await waitUntilStopped([ + { + id: 1, + source: { + name: "qsharp-library-source:intrinsic.qs", + path: "qsharp-library-source:intrinsic.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 162, + column: 13, + name: "H ", + endLine: 162, + endColumn: 44, + }, + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 8, + column: 11, + name: "Foo ", + endLine: 8, + endColumn: 12, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + + // text editor should now be open on intrinsic.qs + await waitForTextEditorOn( + vscode.Uri.parse("qsharp-library-source:intrinsic.qs"), + ); + }); + + /** + * Wait until the debugger has entered stopped state. + * + * @param expectedStackTrace assert that the stack trace matches this value + */ + function waitUntilStopped(expectedStackTrace: DebugProtocol.StackFrame[]) { + tracker!.waitUntilStopped(expectedStackTrace); + } +}); + +/** + * Terminate the active debug session and wait for it to end. + */ +async function terminateSession() { + vscode.commands.executeCommand("workbench.action.debug.stop"); + await waitForCondition( + () => !vscode.debug.activeDebugSession, + vscode.debug.onDidChangeActiveDebugSession, + 2000, + "timed out waiting for the debugger to be terminated", + ); +} + +/** + * Wait for the active text editor to be open to the given document URI. + */ +async function waitForTextEditorOn(uri: vscode.Uri) { + await waitForCondition( + () => + vscode.window.activeTextEditor?.document.uri.toString() === + uri.toString(), + vscode.window.onDidChangeActiveTextEditor, + 500, + `timed out waiting for the text editor to open to ${uri}.\nactive text editor is ${vscode.window.activeTextEditor?.document.uri}`, + ); +} + +/** + * This class will listen to the communication between VS Code and the debug adapter (our code). + * + * VS Code does not provide an easy way to hook into debugger state for our tests. But there + * is a predictable pattern of Debug Adapter Protocol messages we can listen to, + * to figure out when the debugger has entered the stop state (as a result of a breakpoint, step, breaking on entry, etc.). + * + * 1. a "stopped" event coming from the debug adapter. + * 2. a response to a "stackTrace" request. + * 3. a response to a "variables" request. + * + * The "variables" request is the last thing VS Code sends to the debug adapter, and thus we can + * use that event to reasonably determine we're ready to move on to the next test command. + * + * This pattern is based on the debug tests in the VS Code repo: + * https://github.com/microsoft/vscode/blob/13e49a698cf441f82984b357f09ed095779751b8/extensions/vscode-api-tests/src/singlefolder-tests/debug.test.ts#L52 + */ +class Tracker implements vscode.DebugAdapterTracker { + private stoppedCount = 0; + private stackTrace; + private variables; + private onVariablesResponse: ((e: any) => void) | undefined; + + /** + * Wait until the debugger has entered stopped state by waiting for the + * appropriate sequence of messages in the debug adapter. + * + * @param expectedStackTrace assert that the stack trace matches this value + */ + async waitUntilStopped(expectedStackTrace: DebugProtocol.StackFrame[]) { + const start = performance.now(); + + await waitForCondition( + () => this.stoppedCount === 1 && this.stackTrace && this.variables, + (listener: (e: any) => void) => { + this.onVariablesResponse = listener; + return { + dispose() { + this.variables = undefined; + }, + }; + }, + 2000, + "timed out waiting for the debugger to stop after continue", + ); + + assert.deepEqual( + this.stackTrace, + expectedStackTrace, + // print copy-pastable stack trace + `actual stack trace:\n${JSON.stringify(this.stackTrace)}\n`, + ); + + this.stoppedCount = 0; + this.stackTrace = undefined; + this.variables = undefined; + + const stepMs = performance.now() - start; + if (stepMs > 700) { + // Not much we can control here if the debugger is taking too long, + // but log a warning so that we see it in the test log if we hit test timeouts. + // The default mocha test timeout is 2000ms. + console.log(`qsharp-tests: debugger took ${stepMs}ms to stop`); + } + } + + onDidSendMessage(message: any): void { + if (message.type === "event") { + if (message.event === "stopped") { + this.stoppedCount++; + } + } else if (message.type === "response") { + if (message.command === "variables") { + this.variables = message.body.variables; + this.onVariablesResponse?.(undefined); + } else if (message.command === "stackTrace") { + this.stackTrace = message.body.stackFrames; + } + } + } +} diff --git a/vscode/test/suites/debugger/extension.test.ts b/vscode/test/suites/debugger/extension.test.ts deleted file mode 100644 index e3b28a73ba..0000000000 --- a/vscode/test/suites/debugger/extension.test.ts +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import * as vscode from "vscode"; -import { assert } from "chai"; - -suite("Q# Debugger Tests", () => { - const workspaceFolder = - vscode.workspace.workspaceFolders && vscode.workspace.workspaceFolders[0]; - assert(workspaceFolder, "Expecting an open folder"); - - test("Placeholder - success", async () => { - assert.equal(1, 1); - }); -}); diff --git a/vscode/test/suites/debugger/index.ts b/vscode/test/suites/debugger/index.ts index 82a441be4a..e6ca2e261d 100644 --- a/vscode/test/suites/debugger/index.ts +++ b/vscode/test/suites/debugger/index.ts @@ -8,6 +8,6 @@ export function run(): Promise { // We can't use any wildcards or dynamically discovered // paths here since ESBuild needs these modules to be // real paths on disk at bundling time. - require("./extension.test"); + require("./debugger.test"); }); } diff --git a/vscode/test/suites/debugger/test-workspace/qsharp.json b/vscode/test/suites/debugger/test-workspace/qsharp.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/vscode/test/suites/debugger/test-workspace/qsharp.json @@ -0,0 +1 @@ +{} diff --git a/vscode/test/suites/debugger/test-workspace/src/bar.qs b/vscode/test/suites/debugger/test-workspace/src/bar.qs new file mode 100644 index 0000000000..d1e0eae0d9 --- /dev/null +++ b/vscode/test/suites/debugger/test-workspace/src/bar.qs @@ -0,0 +1,5 @@ +namespace Bar { + operation Bar() : Unit { + Message("hello"); + } +} diff --git a/vscode/test/suites/debugger/test-workspace/src/foo.qs b/vscode/test/suites/debugger/test-workspace/src/foo.qs new file mode 100644 index 0000000000..53c3a63aff --- /dev/null +++ b/vscode/test/suites/debugger/test-workspace/src/foo.qs @@ -0,0 +1,10 @@ +namespace Foo { + open Bar; + @EntryPoint() + operation Foo() : Unit { + Bar(); + Bar(); + use q = Qubit(); + H(q); + } +} diff --git a/vscode/test/suites/extensionUtils.ts b/vscode/test/suites/extensionUtils.ts index 5c8a2b64ca..6cf5d4a7dd 100644 --- a/vscode/test/suites/extensionUtils.ts +++ b/vscode/test/suites/extensionUtils.ts @@ -31,3 +31,66 @@ export async function activateExtension() { `qsharp-tests: activate() completed in ${performance.now() - start}ms`, ); } + +/** + * Waits until a condition is met, with a timeout. + * + * @param condition A function that returns true when the condition is met + * @param wakeUpOn The event that we want to wake up to check the condition + * @param timeoutMs If the condition is not met by this timeout, this function will throw + * @param timeoutErrorMsg The custom error message to throw if the condition is not met + */ +export async function waitForCondition( + condition: (...args: any[]) => boolean, + wakeUpOn: vscode.Event, + timeoutMs: number, + timeoutErrorMsg: string, +) { + let disposable: vscode.Disposable | undefined; + await new Promise((resolve, reject) => { + let done = false; + setTimeout(() => { + if (!done) { + reject(new Error(timeoutErrorMsg)); + } + }, timeoutMs); + + disposable = wakeUpOn(() => { + if (!done && condition()) { + done = true; + resolve(); + } + }); + + // Resolve immediately if condition is already true + if (condition()) { + done = true; + resolve(); + } + }); + disposable?.dispose(); +} + +/** + * Convenience method to add a time delay. + * + * @deprecated While this function is convenient for local development, + * please do NOT use it in actual tests. Instead, use `waitForCondition` + * with an appropriate condition and wake up event. + * + * Adding an unconditional delay to tests is guaranteed to slow tests down + * unnecessarily. It also reduces reliability when we're not clear about + * exactly what we're waiting on. + */ +export async function delay(timeoutMs: number) { + try { + await waitForCondition( + () => false, + () => ({ dispose() {} }), + timeoutMs, + "hit the expected timeout", + ); + } catch (e) { + // expected + } +} diff --git a/vscode/test/suites/language-service/notebook.test.ts b/vscode/test/suites/language-service/notebook.test.ts index 039b61f421..423196a7e5 100644 --- a/vscode/test/suites/language-service/notebook.test.ts +++ b/vscode/test/suites/language-service/notebook.test.ts @@ -3,7 +3,7 @@ import * as vscode from "vscode"; import { assert } from "chai"; -import { activateExtension } from "../extensionUtils"; +import { activateExtension, waitForCondition } from "../extensionUtils"; suite("Q# Notebook Tests", function suite() { const workspaceFolder = @@ -23,40 +23,22 @@ suite("Q# Notebook Tests", function suite() { // Test if the cell with the %%qsharp magic has been detected // and its language switched to qsharp. We can only expect this to happen - // after the notebook has happened, and the document handlers + // after the notebook has been opened, and the document handlers // have been invoked, but of course there is no callback for that. // So we just verify the notebook has been updated with the qsharp // language id within 50ms (If we start exceeding this timeout for some // reason, it's enough of a user-perceptible delay that we're probably // better off disabling this behavior, rather than suddenly change the // cell language from under the user after a delay). - await new Promise((resolve, reject) => { - let done = false; - setTimeout(() => { - if (!done) { - reject(new Error("timed out waiting for a Q# code cell")); - } - }, 50); - - vscode.workspace.onDidChangeNotebookDocument((event) => { - if (!done && hasQSharpCell(event.notebook)) { - done = true; - resolve(); - } - }); - - // in case the notebook updates have already occurred by the time we get here - if (hasQSharpCell(notebook)) { - done = true; - resolve(); - } - - function hasQSharpCell(notebook) { - return notebook + await waitForCondition( + () => + !!notebook .getCells() - .find((cell) => cell.document.languageId === "qsharp"); - } - }); + .find((cell) => cell.document.languageId === "qsharp"), + vscode.workspace.onDidChangeNotebookDocument, + 50, + "timed out waiting for a Q# code cell", + ); }); test("Diagnostics", async () => { From 55702d51a854b4e5381ecc4e52cacc9e7a2f3a3c Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 31 Jan 2024 16:05:24 -0800 Subject: [PATCH 04/10] Pull Location struct to a more common place --- compiler/qsc/src/lib.rs | 1 + compiler/qsc/src/location.rs | 256 +++++++++++++++++++++++ language_service/src/definition.rs | 2 +- language_service/src/definition/tests.rs | 17 +- language_service/src/hover/tests.rs | 2 +- language_service/src/lib.rs | 7 +- language_service/src/protocol.rs | 6 - language_service/src/qsc_utils.rs | 43 +--- language_service/src/references.rs | 2 +- language_service/src/references/tests.rs | 22 +- language_service/src/rename.rs | 6 +- language_service/src/rename/tests.rs | 6 +- language_service/src/test_utils.rs | 22 +- wasm/src/language_service.rs | 20 +- wasm/src/line_column.rs | 9 + 15 files changed, 323 insertions(+), 98 deletions(-) create mode 100644 compiler/qsc/src/location.rs diff --git a/compiler/qsc/src/lib.rs b/compiler/qsc/src/lib.rs index ef7b8e80f8..d11b577e4b 100644 --- a/compiler/qsc/src/lib.rs +++ b/compiler/qsc/src/lib.rs @@ -8,6 +8,7 @@ pub mod compile; pub mod error; pub mod incremental; pub mod interpret; +pub mod location; pub mod target; pub use qsc_frontend::compile::{ diff --git a/compiler/qsc/src/location.rs b/compiler/qsc/src/location.rs new file mode 100644 index 0000000000..1c4a3b3a73 --- /dev/null +++ b/compiler/qsc/src/location.rs @@ -0,0 +1,256 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use std::sync::Arc; + +use qsc_data_structures::{ + line_column::{Encoding, Range}, + span::Span, +}; +use qsc_frontend::compile::PackageStore; +use qsc_hir::hir::PackageId; + +pub const QSHARP_LIBRARY_URI_SCHEME: &str = "qsharp-library-source"; + +/// Describes a location in source code in terms of a source name and [`Range`]. +#[derive(Debug, PartialEq, Clone)] +pub struct Location { + pub source: Arc, + pub range: Range, +} + +impl Location { + /// Creates a [`Location`] from a package ID and a SourceMap-relative span. + /// + /// To differentiate user sources from library sources, this function takes + /// a `user_package_id` parameter which denotes the user package. + /// All other packages in the package store are assumed to be library packages. + /// Source names from library packages are prepended with a unique URI scheme. + #[must_use] + pub fn from( + span: Span, + package_id: PackageId, + package_store: &PackageStore, + user_package_id: PackageId, + position_encoding: Encoding, + ) -> Self { + let source = package_store + .get(package_id) + .expect("package id must exist in store") + .sources + .find_by_offset(span.lo) + .expect("source should exist for offset"); + + let source_name = if package_id == user_package_id { + source.name.clone() + } else { + // Currently the only supported external packages are our library packages, + // URI's to which need to include our custom library scheme. + format!("{}:{}", QSHARP_LIBRARY_URI_SCHEME, source.name).into() + }; + + Location { + source: source_name, + range: Range::from_span(position_encoding, &source.contents, &(span - source.offset)), + } + } +} + +#[cfg(test)] +mod tests { + use crate::compile; + use expect_test::expect; + use qsc_data_structures::{line_column::Encoding, span::Span}; + use qsc_frontend::compile::{PackageStore, RuntimeCapabilityFlags, SourceMap}; + use qsc_hir::hir::PackageId; + use qsc_passes::PackageType; + + use super::Location; + + #[test] + fn from_std_span() { + let (store, std_package_id, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 0, hi: 1 }, + std_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + expect![[r#" + Location { + source: "qsharp-library-source:arrays.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 1, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_core_span() { + let (store, _, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 0, hi: 1 }, + PackageId::CORE, + &store, + user_package_id, + Encoding::Utf8, + ); + + expect![[r#" + Location { + source: "qsharp-library-source:core/core.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 1, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_user_span() { + let (store, _, user_package_id) = compile_package(); + + let bar_start_offset = store + .get(user_package_id) + .expect("expected to find user package") + .sources + .find_by_name("bar.qs") + .expect("expected to find source") + .offset; + + let location = Location::from( + Span { + lo: bar_start_offset, + hi: bar_start_offset + 1, + }, + user_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + expect![[r#" + Location { + source: "bar.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 1, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_out_of_bounds_lo() { + let (store, _, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 1000, hi: 2000 }, + user_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + // Per [`Range`] spec, out of bounds positions map to EOF + expect![[r#" + Location { + source: "bar.qs", + range: Range { + start: Position { + line: 0, + column: 17, + }, + end: Position { + line: 0, + column: 17, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + #[test] + fn from_out_of_bounds_hi() { + let (store, _, user_package_id) = compile_package(); + + let location = Location::from( + Span { lo: 0, hi: 2000 }, + user_package_id, + &store, + user_package_id, + Encoding::Utf8, + ); + + // Per [`Range`] spec, out of bounds positions map to EOF + expect![[r#" + Location { + source: "foo.qs", + range: Range { + start: Position { + line: 0, + column: 0, + }, + end: Position { + line: 0, + column: 17, + }, + }, + } + "#]] + .assert_debug_eq(&location); + } + + fn compile_package() -> (PackageStore, PackageId, PackageId) { + let mut store = PackageStore::new(compile::core()); + let mut dependencies = Vec::new(); + + let (package_type, capabilities) = (PackageType::Lib, RuntimeCapabilityFlags::all()); + + let std = compile::std(&store, capabilities); + let std_package_id = store.insert(std); + + dependencies.push(std_package_id); + let sources = SourceMap::new( + [ + ("foo.qs".into(), "namespace Foo { }".into()), + ("bar.qs".into(), "namespace Bar { }".into()), + ], + None, + ); + let (unit, _) = + compile::compile(&store, &dependencies, sources, package_type, capabilities); + let user_package_id = store.insert(unit); + + (store, std_package_id, user_package_id) + } +} diff --git a/language_service/src/definition.rs b/language_service/src/definition.rs index ddc1621382..61cf021ccb 100644 --- a/language_service/src/definition.rs +++ b/language_service/src/definition.rs @@ -6,11 +6,11 @@ mod tests; use crate::compilation::Compilation; use crate::name_locator::{Handler, Locator, LocatorContext}; -use crate::protocol::Location; use crate::qsc_utils::into_location; use qsc::ast::visit::Visitor; use qsc::hir::PackageId; use qsc::line_column::{Encoding, Position}; +use qsc::location::Location; use qsc::{ast, hir, Span}; pub(crate) fn get_definition( diff --git a/language_service/src/definition/tests.rs b/language_service/src/definition/tests.rs index a364ac5ee6..8ccee134b7 100644 --- a/language_service/src/definition/tests.rs +++ b/language_service/src/definition/tests.rs @@ -4,10 +4,10 @@ #![allow(clippy::needless_raw_string_hashes)] use expect_test::{expect, Expect}; +use qsc::location::Location; use super::get_definition; use crate::{ - protocol::Location, test_utils::{ compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers, }, @@ -26,8 +26,8 @@ fn assert_definition(source_with_markers: &str) { None } else { Some(Location { - source: "".to_string(), - span: target_spans[0], + source: "".into(), + range: target_spans[0], }) }; assert_eq!(&expected_definition, &actual_definition); @@ -40,10 +40,7 @@ fn assert_definition_notebook(cells_with_markers: &[(&str, &str)]) { let expected_definition = if target_spans.is_empty() { None } else { - Some(Location { - source: target_spans[0].0.clone(), - span: target_spans[0].1, - }) + Some(target_spans[0].clone()) }; assert_eq!(&expected_definition, &actual_definition); } @@ -302,7 +299,7 @@ fn std_call() { Some( Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 1, column: 26, @@ -410,7 +407,7 @@ fn std_udt() { Some( Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 24, @@ -442,7 +439,7 @@ fn std_udt_udt_field() { Some( Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 31, diff --git a/language_service/src/hover/tests.rs b/language_service/src/hover/tests.rs index 0f4ec3595f..fef0da84a5 100644 --- a/language_service/src/hover/tests.rs +++ b/language_service/src/hover/tests.rs @@ -37,7 +37,7 @@ fn check_notebook(cells_with_markers: &[(&str, &str)], expect: &Expect) { let actual = get_hover(&compilation, &cell_uri, position, Encoding::Utf8).expect("Expected a hover."); - assert_eq!(&actual.span, &target_spans[0].1); + assert_eq!(&actual.span, &target_spans[0].range); expect.assert_eq(&actual.contents); } diff --git a/language_service/src/lib.rs b/language_service/src/lib.rs index 03b6355284..98bf10b53f 100644 --- a/language_service/src/lib.rs +++ b/language_service/src/lib.rs @@ -27,10 +27,13 @@ use futures::channel::mpsc::{unbounded, UnboundedReceiver, UnboundedSender}; use futures_util::StreamExt; use log::{trace, warn}; use protocol::{ - CompletionList, DiagnosticUpdate, Hover, Location, NotebookMetadata, SignatureHelp, + CompletionList, DiagnosticUpdate, Hover, NotebookMetadata, SignatureHelp, WorkspaceConfigurationUpdate, }; -use qsc::line_column::{Encoding, Position, Range}; +use qsc::{ + line_column::{Encoding, Position, Range}, + location::Location, +}; use qsc_project::JSFileEntry; use state::{CompilationState, CompilationStateUpdater}; use std::{cell::RefCell, fmt::Debug, future::Future, pin::Pin, rc::Rc, sync::Arc}; diff --git a/language_service/src/protocol.rs b/language_service/src/protocol.rs index 190b5f2db2..781432675a 100644 --- a/language_service/src/protocol.rs +++ b/language_service/src/protocol.rs @@ -86,12 +86,6 @@ impl Hash for CompletionItem { } } -#[derive(Debug, PartialEq)] -pub struct Location { - pub source: String, - pub span: Range, -} - #[derive(Debug, PartialEq)] pub struct Hover { pub contents: String, diff --git a/language_service/src/qsc_utils.rs b/language_service/src/qsc_utils.rs index 384b6513ae..940addc944 100644 --- a/language_service/src/qsc_utils.rs +++ b/language_service/src/qsc_utils.rs @@ -1,15 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::{ - compilation::Compilation, - protocol::{self}, -}; +use crate::compilation::Compilation; use qsc::line_column::{Encoding, Range}; +use qsc::location::Location; use qsc::{ast, hir::PackageId, SourceMap, Span}; -pub(crate) const QSHARP_LIBRARY_URI_SCHEME: &str = "qsharp-library-source"; - pub(crate) fn span_contains(span: Span, offset: u32) -> bool { offset >= span.lo && offset < span.hi } @@ -39,33 +35,16 @@ pub(crate) fn into_range(encoding: Encoding, span: Span, source_map: &SourceMap) pub(crate) fn into_location( position_encoding: Encoding, compilation: &Compilation, - location: Span, + span: Span, package_id: PackageId, -) -> protocol::Location { - let source_map = &compilation - .package_store - .get(package_id) - .expect("package id must exist in store") - .sources; - let source = source_map - .find_by_offset(location.lo) - .expect("source should exist for offset"); - let source_name = if package_id == compilation.user_package_id { - source.name.to_string() - } else { - // Currently the only supported external packages are our library packages, - // URI's to which need to include our custom library scheme. - format!("{}:{}", QSHARP_LIBRARY_URI_SCHEME, source.name) - }; - - protocol::Location { - source: source_name, - span: Range::from_span( - position_encoding, - &source.contents, - &(location - source.offset), - ), - } +) -> Location { + Location::from( + span, + package_id, + &compilation.package_store, + compilation.user_package_id, + position_encoding, + ) } pub(crate) fn find_ident<'a>( diff --git a/language_service/src/references.rs b/language_service/src/references.rs index 0a265e64c8..44931bc7c5 100644 --- a/language_service/src/references.rs +++ b/language_service/src/references.rs @@ -8,12 +8,12 @@ use std::rc::Rc; use crate::compilation::{Compilation, Lookup}; use crate::name_locator::{Handler, Locator, LocatorContext}; -use crate::protocol::Location; use crate::qsc_utils::into_location; use qsc::ast::visit::{walk_callable_decl, walk_expr, walk_ty, Visitor}; use qsc::hir::ty::Ty; use qsc::hir::{PackageId, Res}; use qsc::line_column::{Encoding, Position}; +use qsc::location::Location; use qsc::{ast, hir, resolve, Span}; pub(crate) fn get_references( diff --git a/language_service/src/references/tests.rs b/language_service/src/references/tests.rs index 4ebf3a1e48..531a80647f 100644 --- a/language_service/src/references/tests.rs +++ b/language_service/src/references/tests.rs @@ -5,7 +5,6 @@ use super::get_references; use crate::{ - protocol, test_utils::{ compile_notebook_with_fake_stdlib_and_markers, compile_with_fake_stdlib_and_markers, }, @@ -43,7 +42,7 @@ fn check(source_with_markers: &str, include_declaration: bool) { include_declaration, ) .into_iter() - .map(|l| l.span) + .map(|l| l.range) .collect::>(); for target in &target_spans { assert!( @@ -71,10 +70,7 @@ fn check_notebook_exclude_decl(cells_with_markers: &[(&str, &str)]) { .collect::>(); for target in &target_spans { assert!( - actual.contains(&protocol::Location { - source: target.0.clone(), - span: target.1 - }), + actual.contains(target), "expected {actual:?} to contain {target:?}" ); } @@ -98,7 +94,7 @@ fn std_callable_ref() { [ Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 1, column: 26, @@ -111,7 +107,7 @@ fn std_callable_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 3, column: 8, @@ -124,7 +120,7 @@ fn std_callable_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 5, column: 8, @@ -255,7 +251,7 @@ fn std_udt_ref() { [ Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 24, @@ -268,7 +264,7 @@ fn std_udt_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 2, column: 22, @@ -345,7 +341,7 @@ fn std_field_ref() { [ Location { source: "qsharp-library-source:", - span: Range { + range: Range { start: Position { line: 4, column: 31, @@ -358,7 +354,7 @@ fn std_field_ref() { }, Location { source: "", - span: Range { + range: Range { start: Position { line: 4, column: 23, diff --git a/language_service/src/rename.rs b/language_service/src/rename.rs index c2934dd07d..b0bef3e7b6 100644 --- a/language_service/src/rename.rs +++ b/language_service/src/rename.rs @@ -6,11 +6,11 @@ mod tests; use crate::compilation::{Compilation, Lookup}; use crate::name_locator::{Handler, Locator, LocatorContext}; -use crate::protocol::Location; use crate::qsc_utils::into_range; use crate::references::ReferenceFinder; use qsc::ast::visit::Visitor; use qsc::line_column::{Encoding, Position, Range}; +use qsc::location::Location; use qsc::{ast, hir, resolve, Span}; pub(crate) fn prepare_rename( @@ -126,9 +126,9 @@ impl<'a> Rename<'a> { .for_ty_param(param_id, current_callable) .into_iter() .map(|l| { - assert!(!l.span.empty(), "Type parameter name is empty"); + assert!(!l.range.empty(), "Type parameter name is empty"); Location { - span: type_param_ident_range(l.span), + range: type_param_ident_range(l.range), ..l } }) diff --git a/language_service/src/rename/tests.rs b/language_service/src/rename/tests.rs index 85f06b99f6..0e730b6e04 100644 --- a/language_service/src/rename/tests.rs +++ b/language_service/src/rename/tests.rs @@ -20,7 +20,7 @@ fn check(source_with_markers: &str) { compile_with_fake_stdlib_and_markers(source_with_markers); let actual = get_rename(&compilation, "", cursor_position, Encoding::Utf8) .into_iter() - .map(|l| l.span) + .map(|l| l.range) .collect::>(); for target in &target_spans { assert!(actual.contains(target)); @@ -391,7 +391,7 @@ fn notebook_rename_across_cells() { [ Location { source: "cell1", - span: Range { + range: Range { start: Position { line: 0, column: 10, @@ -404,7 +404,7 @@ fn notebook_rename_across_cells() { }, Location { source: "cell2", - span: Range { + range: Range { start: Position { line: 0, column: 0, diff --git a/language_service/src/test_utils.rs b/language_service/src/test_utils.rs index 115a3fe099..611c255a62 100644 --- a/language_service/src/test_utils.rs +++ b/language_service/src/test_utils.rs @@ -9,6 +9,7 @@ use qsc::{ hir::PackageId, incremental::Compiler, line_column::{Encoding, Position, Range}, + location::Location, target::Profile, PackageStore, PackageType, SourceMap, Span, }; @@ -21,13 +22,13 @@ pub(crate) fn compile_with_fake_stdlib_and_markers( ( compilation, cursor_offset, - target_spans.iter().map(|(_, s)| *s).collect(), + target_spans.iter().map(|l| l.range).collect(), ) } pub(crate) fn compile_project_with_fake_stdlib_and_markers( sources_with_markers: &[(&str, &str)], -) -> (Compilation, String, Position, Vec<(String, Range)>) { +) -> (Compilation, String, Position, Vec) { let (sources, cursor_uri, cursor_offset, target_spans) = get_sources_and_markers(sources_with_markers); @@ -58,7 +59,7 @@ pub(crate) fn compile_project_with_fake_stdlib_and_markers( pub(crate) fn compile_notebook_with_fake_stdlib_and_markers( cells_with_markers: &[(&str, &str)], -) -> (Compilation, String, Position, Vec<(String, Range)>) { +) -> (Compilation, String, Position, Vec) { let (cells, cell_uri, offset, target_spans) = get_sources_and_markers(cells_with_markers); let compilation = @@ -160,12 +161,7 @@ fn compile_fake_stdlib() -> (PackageStore, PackageId) { #[allow(clippy::type_complexity)] fn get_sources_and_markers( sources: &[(&str, &str)], -) -> ( - Vec<(Arc, Arc)>, - String, - Position, - Vec<(String, Range)>, -) { +) -> (Vec<(Arc, Arc)>, String, Position, Vec) { let (mut cursor_uri, mut cursor_offset, mut target_spans) = (None, None, Vec::new()); let sources = sources .iter() @@ -189,10 +185,10 @@ fn get_sources_and_markers( } if !targets.is_empty() { for span in target_offsets_to_spans(&targets) { - target_spans.push(( - s.0.to_string(), - Range::from_span(Encoding::Utf8, &source, &span), - )); + target_spans.push(Location { + source: s.0.into(), + range: Range::from_span(Encoding::Utf8, &source, &span), + }); } } (Arc::from(s.0), Arc::from(source.as_ref())) diff --git a/wasm/src/language_service.rs b/wasm/src/language_service.rs index 6313556d95..2e4b8e8190 100644 --- a/wasm/src/language_service.rs +++ b/wasm/src/language_service.rs @@ -239,10 +239,13 @@ impl LanguageService { let mut renames: FxHashMap> = FxHashMap::default(); locations.into_iter().for_each(|l| { - renames.entry(l.source).or_default().push(TextEdit { - range: l.span.into(), - newText: new_name.to_string(), - }) + renames + .entry(l.source.to_string()) + .or_default() + .push(TextEdit { + range: l.range.into(), + newText: new_name.to_string(), + }) }); let workspace_edit = WorkspaceEdit { @@ -265,15 +268,6 @@ impl LanguageService { } } -impl From for Location { - fn from(location: qsls::protocol::Location) -> Self { - Location { - source: location.source, - span: location.span.into(), - } - } -} - serializable_type! { WorkspaceConfiguration, { diff --git a/wasm/src/line_column.rs b/wasm/src/line_column.rs index d1a905076a..7c7a771d25 100644 --- a/wasm/src/line_column.rs +++ b/wasm/src/line_column.rs @@ -70,3 +70,12 @@ impl From for Range { } } } + +impl From for Location { + fn from(location: qsc::location::Location) -> Self { + Location { + source: location.source.to_string(), + span: location.range.into(), + } + } +} From c73ea020c958560818b8e7e3117baf9ae9c89840 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 1 Feb 2024 22:11:12 -0800 Subject: [PATCH 05/10] temp - comment out problematic test --- vscode/test/suites/debugger/debugger.test.ts | 68 ++++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/vscode/test/suites/debugger/debugger.test.ts b/vscode/test/suites/debugger/debugger.test.ts index 072c33132b..d28c6ce5db 100644 --- a/vscode/test/suites/debugger/debugger.test.ts +++ b/vscode/test/suites/debugger/debugger.test.ts @@ -65,7 +65,7 @@ suite("Q# Debugger Tests", function suite() { test("Launch with launch.json configuration - workspaceFolder substitution", async () => { // The DebugConfiguration object is what would go in launch.json, // pass it in directly here - vscode.debug.startDebugging(workspaceFolder, { + await vscode.debug.startDebugging(workspaceFolder, { name: "Launch foo.qs", type: "qsharp", request: "launch", @@ -91,35 +91,35 @@ suite("Q# Debugger Tests", function suite() { ]); }); - test("Launch with launch.json configuration - file substitution", async () => { - await vscode.window.showTextDocument(fooUri); - - // ${file} will expand to the filesystem path of the currently opened file - vscode.debug.startDebugging(workspaceFolder, { - name: "Launch foo.qs", - type: "qsharp", - request: "launch", - program: "${file}", - }); - - await waitUntilStopped([ - { - id: 0, - source: { - name: "foo.qs", - path: "vscode-test-web://mount/src/foo.qs", - sourceReference: 0, - adapterData: "qsharp-adapter-data", - }, - line: 5, - column: 9, - name: "Foo ", - endLine: 5, - endColumn: 15, - }, - { id: 0, line: 0, column: 0, name: "entry", source: undefined }, - ]); - }); + // test("Launch with launch.json configuration - file substitution", async () => { + // await vscode.window.showTextDocument(fooUri); + + // // ${file} will expand to the filesystem path of the currently opened file + // await vscode.debug.startDebugging(workspaceFolder, { + // name: "Launch foo.qs", + // type: "qsharp", + // request: "launch", + // program: "${file}", + // }); + + // await waitUntilStopped([ + // { + // id: 0, + // source: { + // name: "foo.qs", + // path: "vscode-test-web://mount/src/foo.qs", + // sourceReference: 0, + // adapterData: "qsharp-adapter-data", + // }, + // line: 5, + // column: 9, + // name: "Foo ", + // endLine: 5, + // endColumn: 15, + // }, + // { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + // ]); + // }); test("Set breakpoint in main file", async () => { // Set a breakpoint on line 6 of foo.qs (5 when 0-indexed) @@ -130,7 +130,7 @@ suite("Q# Debugger Tests", function suite() { ]); // launch debugger - vscode.debug.startDebugging(workspaceFolder, { + await vscode.debug.startDebugging(workspaceFolder, { name: "Launch foo.qs", type: "qsharp", request: "launch", @@ -167,7 +167,7 @@ suite("Q# Debugger Tests", function suite() { ]); // launch debugger - vscode.debug.startDebugging(workspaceFolder, { + await vscode.debug.startDebugging(workspaceFolder, { name: "Launch foo.qs", type: "qsharp", request: "launch", @@ -214,7 +214,7 @@ suite("Q# Debugger Tests", function suite() { test("Step into other file", async () => { // launch debugger - vscode.debug.startDebugging(workspaceFolder, { + await vscode.debug.startDebugging(workspaceFolder, { name: "Launch foo.qs", type: "qsharp", request: "launch", @@ -290,7 +290,7 @@ suite("Q# Debugger Tests", function suite() { ]); // launch debugger - vscode.debug.startDebugging(workspaceFolder, { + await vscode.debug.startDebugging(workspaceFolder, { name: "Launch foo.qs", type: "qsharp", request: "launch", From acf6507536de4223d94714e2170f83964c174175 Mon Sep 17 00:00:00 2001 From: Mine Starks <16928427+minestarks@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:36:20 +0000 Subject: [PATCH 06/10] change order of cleanup --- vscode/test/suites/debugger/debugger.test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/vscode/test/suites/debugger/debugger.test.ts b/vscode/test/suites/debugger/debugger.test.ts index d28c6ce5db..bab0cb3c09 100644 --- a/vscode/test/suites/debugger/debugger.test.ts +++ b/vscode/test/suites/debugger/debugger.test.ts @@ -6,6 +6,12 @@ import { assert } from "chai"; import { activateExtension, waitForCondition } from "../extensionUtils"; import { DebugProtocol } from "@vscode/debugprotocol"; +/** + * Set to true to log Debug Adapter Protocol messages to the console. + * This is useful for debugging test failures. + */ +const logDapMessages = true; + suite("Q# Debugger Tests", function suite() { const workspaceFolder = vscode.workspace.workspaceFolders && vscode.workspace.workspaceFolders[0]; @@ -32,9 +38,9 @@ suite("Q# Debugger Tests", function suite() { this.afterEach(async () => { disposable.dispose(); tracker = undefined; - vscode.debug.removeBreakpoints(vscode.debug.breakpoints); await terminateSession(); vscode.commands.executeCommand("workbench.action.closeAllEditors"); + vscode.debug.removeBreakpoints(vscode.debug.breakpoints); }); test("Launch with debugEditorContents command", async () => { @@ -455,13 +461,22 @@ class Tracker implements vscode.DebugAdapterTracker { const stepMs = performance.now() - start; if (stepMs > 700) { // Not much we can control here if the debugger is taking too long, - // but log a warning so that we see it in the test log if we hit test timeouts. + // but log a warning so that we see it in the test log if we get + // close to hitting test timeouts. // The default mocha test timeout is 2000ms. console.log(`qsharp-tests: debugger took ${stepMs}ms to stop`); } } + onWillReceiveMessage(message: any): void { + if (logDapMessages) { + console.log(`qsharp-tests: -> ${JSON.stringify(message)}`); + } + } onDidSendMessage(message: any): void { + if (logDapMessages) { + console.log(`qsharp-tests: <- ${JSON.stringify(message)}`); + } if (message.type === "event") { if (message.event === "stopped") { this.stoppedCount++; From 728ad66e6eb74b923277b97a0ccfae72c0d3fd61 Mon Sep 17 00:00:00 2001 From: Mine Starks <16928427+minestarks@users.noreply.github.com> Date: Fri, 2 Feb 2024 23:39:38 +0000 Subject: [PATCH 07/10] fix test bug, add logging --- vscode/test/suites/debugger/debugger.test.ts | 178 +++++++++++++----- .../suites/debugger/test-workspace/src/foo.qs | 1 + vscode/test/suites/extensionUtils.ts | 2 +- 3 files changed, 128 insertions(+), 53 deletions(-) diff --git a/vscode/test/suites/debugger/debugger.test.ts b/vscode/test/suites/debugger/debugger.test.ts index bab0cb3c09..1035c02585 100644 --- a/vscode/test/suites/debugger/debugger.test.ts +++ b/vscode/test/suites/debugger/debugger.test.ts @@ -10,7 +10,7 @@ import { DebugProtocol } from "@vscode/debugprotocol"; * Set to true to log Debug Adapter Protocol messages to the console. * This is useful for debugging test failures. */ -const logDapMessages = true; +const logDebugAdapterActivity = false; suite("Q# Debugger Tests", function suite() { const workspaceFolder = @@ -49,7 +49,7 @@ suite("Q# Debugger Tests", function suite() { // launch debugger await vscode.commands.executeCommand("qsharp-vscode.debugEditorContents"); - await waitUntilStopped([ + await waitUntilPaused([ { id: 0, source: { @@ -78,7 +78,7 @@ suite("Q# Debugger Tests", function suite() { program: "${workspaceFolder}src/foo.qs", }); - await waitUntilStopped([ + await waitUntilPaused([ { id: 0, source: { @@ -97,35 +97,75 @@ suite("Q# Debugger Tests", function suite() { ]); }); - // test("Launch with launch.json configuration - file substitution", async () => { - // await vscode.window.showTextDocument(fooUri); - - // // ${file} will expand to the filesystem path of the currently opened file - // await vscode.debug.startDebugging(workspaceFolder, { - // name: "Launch foo.qs", - // type: "qsharp", - // request: "launch", - // program: "${file}", - // }); - - // await waitUntilStopped([ - // { - // id: 0, - // source: { - // name: "foo.qs", - // path: "vscode-test-web://mount/src/foo.qs", - // sourceReference: 0, - // adapterData: "qsharp-adapter-data", - // }, - // line: 5, - // column: 9, - // name: "Foo ", - // endLine: 5, - // endColumn: 15, - // }, - // { id: 0, line: 0, column: 0, name: "entry", source: undefined }, - // ]); - // }); + test("Launch with launch.json configuration - file substitution", async () => { + await vscode.window.showTextDocument(fooUri); + + // ${file} will expand to the filesystem path of the currently opened file + await vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${file}", + }); + + await waitUntilPaused([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 9, + name: "Foo ", + endLine: 5, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + }); + + test("Run until completion", async () => { + // launch debugger + await vscode.debug.startDebugging(workspaceFolder, { + name: "Launch foo.qs", + type: "qsharp", + request: "launch", + program: "${workspaceFolder}src/foo.qs", + stopOnEntry: true, + }); + + // should hit the breakpoint we set above + await waitUntilPaused([ + { + id: 0, + source: { + name: "foo.qs", + path: "vscode-test-web://mount/src/foo.qs", + sourceReference: 0, + adapterData: "qsharp-adapter-data", + }, + line: 5, + column: 9, + name: "Foo ", + endLine: 5, + endColumn: 15, + }, + { id: 0, line: 0, column: 0, name: "entry", source: undefined }, + ]); + + vscode.commands.executeCommand("workbench.action.debug.continue"); + + // wait until there's no longer an active debug session + await waitForCondition( + () => !vscode.debug.activeDebugSession, + vscode.debug.onDidChangeActiveDebugSession, + 2000, + "timed out waiting for the debugger to be terminated", + ); + }); test("Set breakpoint in main file", async () => { // Set a breakpoint on line 6 of foo.qs (5 when 0-indexed) @@ -145,7 +185,7 @@ suite("Q# Debugger Tests", function suite() { }); // should hit the breakpoint we set above - await waitUntilStopped([ + await waitUntilPaused([ { id: 0, source: { @@ -182,7 +222,7 @@ suite("Q# Debugger Tests", function suite() { }); // should hit the breakpoint we set above - await waitUntilStopped([ + await waitUntilPaused([ { id: 1, source: { @@ -229,7 +269,7 @@ suite("Q# Debugger Tests", function suite() { }); // should break on entry (per debug config above) - await waitUntilStopped([ + await waitUntilPaused([ { id: 0, source: { @@ -250,7 +290,7 @@ suite("Q# Debugger Tests", function suite() { // step into call (will be a call into bar.qs) await vscode.commands.executeCommand("workbench.action.debug.stepInto"); - await waitUntilStopped([ + await waitUntilPaused([ { id: 1, source: { @@ -305,7 +345,7 @@ suite("Q# Debugger Tests", function suite() { }); // should hit the breakpoint we set above - await waitUntilStopped([ + await waitUntilPaused([ { id: 0, source: { @@ -326,7 +366,7 @@ suite("Q# Debugger Tests", function suite() { // step into call (will be a call into intrinsic.qs) await vscode.commands.executeCommand("workbench.action.debug.stepInto"); - await waitUntilStopped([ + await waitUntilPaused([ { id: 1, source: { @@ -365,12 +405,12 @@ suite("Q# Debugger Tests", function suite() { }); /** - * Wait until the debugger has entered stopped state. + * Wait until the debugger has entered the paused state. * * @param expectedStackTrace assert that the stack trace matches this value */ - function waitUntilStopped(expectedStackTrace: DebugProtocol.StackFrame[]) { - tracker!.waitUntilStopped(expectedStackTrace); + function waitUntilPaused(expectedStackTrace: DebugProtocol.StackFrame[]) { + return tracker!.waitUntilPaused(expectedStackTrace); } }); @@ -404,9 +444,9 @@ async function waitForTextEditorOn(uri: vscode.Uri) { /** * This class will listen to the communication between VS Code and the debug adapter (our code). * - * VS Code does not provide an easy way to hook into debugger state for our tests. But there + * VS Code does not provide an easy way to hook into debug session state for our tests. But there * is a predictable pattern of Debug Adapter Protocol messages we can listen to, - * to figure out when the debugger has entered the stop state (as a result of a breakpoint, step, breaking on entry, etc.). + * to figure out when the debugger has entered the paused state (as a result of a breakpoint, step, breaking on entry, etc.). * * 1. a "stopped" event coming from the debug adapter. * 2. a response to a "stackTrace" request. @@ -425,12 +465,12 @@ class Tracker implements vscode.DebugAdapterTracker { private onVariablesResponse: ((e: any) => void) | undefined; /** - * Wait until the debugger has entered stopped state by waiting for the + * Wait until the debugger has entered the paused state by waiting for the * appropriate sequence of messages in the debug adapter. * * @param expectedStackTrace assert that the stack trace matches this value */ - async waitUntilStopped(expectedStackTrace: DebugProtocol.StackFrame[]) { + async waitUntilPaused(expectedStackTrace: DebugProtocol.StackFrame[]) { const start = performance.now(); await waitForCondition( @@ -439,12 +479,12 @@ class Tracker implements vscode.DebugAdapterTracker { this.onVariablesResponse = listener; return { dispose() { - this.variables = undefined; + this.onVariablesResponse = undefined; }, }; }, - 2000, - "timed out waiting for the debugger to stop after continue", + 1800, + "timed out waiting for the debugger to stop", ); assert.deepEqual( @@ -466,17 +506,27 @@ class Tracker implements vscode.DebugAdapterTracker { // The default mocha test timeout is 2000ms. console.log(`qsharp-tests: debugger took ${stepMs}ms to stop`); } + if (logDebugAdapterActivity) { + console.log(`qsharp-tests: debugger paused`); + } } + onWillReceiveMessage(message: any): void { - if (logDapMessages) { - console.log(`qsharp-tests: -> ${JSON.stringify(message)}`); + if (logDebugAdapterActivity) { + console.log(`qsharp-tests: -> ${JSON.stringify(message)}`); } } onDidSendMessage(message: any): void { - if (logDapMessages) { - console.log(`qsharp-tests: <- ${JSON.stringify(message)}`); + if (logDebugAdapterActivity) { + if (message.type === "response") { + console.log(`qsharp-tests: <- ${JSON.stringify(message)}`); + } else { + // message.type === "event" + console.log(`qsharp-tests: <-* ${JSON.stringify(message)}`); + } } + if (message.type === "event") { if (message.event === "stopped") { this.stoppedCount++; @@ -490,4 +540,28 @@ class Tracker implements vscode.DebugAdapterTracker { } } } + + onWillStartSession(): void { + if (logDebugAdapterActivity) { + console.log(`qsharp-tests: starting debug session`); + } + } + + onWillStopSession(): void { + if (logDebugAdapterActivity) { + console.log(`qsharp-tests: stopping debug session`); + } + } + + onError(error: Error): void { + console.log(`qsharp-tests: [error] error in debug session: ${error}`); + } + + onExit(code: number, signal: string): void { + if (logDebugAdapterActivity) { + console.log( + `qsharp-tests: debug session exited with code ${code} and signal ${signal}`, + ); + } + } } diff --git a/vscode/test/suites/debugger/test-workspace/src/foo.qs b/vscode/test/suites/debugger/test-workspace/src/foo.qs index 53c3a63aff..b354cf1c78 100644 --- a/vscode/test/suites/debugger/test-workspace/src/foo.qs +++ b/vscode/test/suites/debugger/test-workspace/src/foo.qs @@ -6,5 +6,6 @@ namespace Foo { Bar(); use q = Qubit(); H(q); + Reset(q); } } diff --git a/vscode/test/suites/extensionUtils.ts b/vscode/test/suites/extensionUtils.ts index 6cf5d4a7dd..6f762a6fd0 100644 --- a/vscode/test/suites/extensionUtils.ts +++ b/vscode/test/suites/extensionUtils.ts @@ -62,7 +62,7 @@ export async function waitForCondition( } }); - // Resolve immediately if condition is already true + // Resolve immediately if condition is already met if (condition()) { done = true; resolve(); From d0b1f4dc9882bfa73716fec14bc322df1649358d Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Mon, 11 Mar 2024 15:34:19 -0700 Subject: [PATCH 08/10] update integration test --- vscode/test/suites/debugger/debugger.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vscode/test/suites/debugger/debugger.test.ts b/vscode/test/suites/debugger/debugger.test.ts index 1035c02585..e8d1df0fdf 100644 --- a/vscode/test/suites/debugger/debugger.test.ts +++ b/vscode/test/suites/debugger/debugger.test.ts @@ -375,10 +375,10 @@ suite("Q# Debugger Tests", function suite() { sourceReference: 0, adapterData: "qsharp-adapter-data", }, - line: 162, + line: 165, column: 13, name: "H ", - endLine: 162, + endLine: 165, endColumn: 44, }, { From b6078f81e0f6cb7f0044f5abb377c7db480035b5 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Mon, 11 Mar 2024 16:22:33 -0700 Subject: [PATCH 09/10] fix basename in stack --- vscode/src/debugger/session.ts | 5 +++-- vscode/test/suites/debugger/debugger.test.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/vscode/src/debugger/session.ts b/vscode/src/debugger/session.ts index cbdb3921a5..402e8bcdd7 100644 --- a/vscode/src/debugger/session.ts +++ b/vscode/src/debugger/session.ts @@ -666,12 +666,13 @@ export class QscDebugSession extends LoggingDebugSession { .map(async (f, id) => { log.trace(`frames: location %O`, f.location); + const uri = f.location.source; const sf: DebugProtocol.StackFrame = new StackFrame( id, f.name, new Source( - basename(f.location.source) ?? f.location.source, - f.location.source, + basename(vscode.Uri.parse(uri).path) ?? uri, + uri, undefined, undefined, "qsharp-adapter-data", diff --git a/vscode/test/suites/debugger/debugger.test.ts b/vscode/test/suites/debugger/debugger.test.ts index e8d1df0fdf..2a832845b6 100644 --- a/vscode/test/suites/debugger/debugger.test.ts +++ b/vscode/test/suites/debugger/debugger.test.ts @@ -370,7 +370,7 @@ suite("Q# Debugger Tests", function suite() { { id: 1, source: { - name: "qsharp-library-source:intrinsic.qs", + name: "intrinsic.qs", path: "qsharp-library-source:intrinsic.qs", sourceReference: 0, adapterData: "qsharp-adapter-data", From f2eeba25c279bf039c2753ee9f6076d09ac893db Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Mon, 11 Mar 2024 16:33:00 -0700 Subject: [PATCH 10/10] more verbose comment --- vscode/src/debugger/session.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/vscode/src/debugger/session.ts b/vscode/src/debugger/session.ts index 402e8bcdd7..89e5adee92 100644 --- a/vscode/src/debugger/session.ts +++ b/vscode/src/debugger/session.ts @@ -855,16 +855,24 @@ export class QscDebugSession extends LoggingDebugSession { /** * Attemps to resolve a DebugProtocol.Source.path to a URI. * - * This method will attempt to undo the lossy conversion - * from URI to path that the VS Code debug adapter client performs. + * In Debug Protocol requests, the VS Code debug adapter client + * will strip file URIs to just the filesystem path part. + * But for non-file URIs, the full URI is sent. + * * See: https://github.com/microsoft/vscode/blob/3246d63177e1e5ae211029e7ab0021c33342a3c7/src/vs/workbench/contrib/debug/common/debugSource.ts#L90 * - * If the path was originally constructed from a file:// URI, it's a - * filesystem path. We use our knownPaths map to resolve it back to a URI. - * Filesystem paths we don't know about *won't* be resolved, and that's ok. + * Here, we need the original URI, but we don't know if we're + * dealing with a filesystem path or URI. We cannot determine + * which one it is based on the input alone (the syntax is ambiguous). + * But we do have a set of *known* filesystem paths that we + * constructed at initialization, and we can use that to resolve + * any known fileystem paths back to the original URI. + * + * Filesystem paths we don't know about *won't* be resolved, + * and that's ok in this use case. * - * If the path was originally constructed from a URI, it's a URI, - * use it directly. + * If the path was originally constructed from a URI, it won't + * be in our known paths map, so we'll treat the string as a URI. */ asUri(pathOrUri: string): vscode.Uri | undefined { pathOrUri = this.knownPaths.get(pathOrUri) || pathOrUri;