Skip to content

Commit

Permalink
Add integration tests for VS Code debugging, eliminate FileAccessor (…
Browse files Browse the repository at this point in the history 
…#1107)

Depends on #1106

VSCode's debugging related APIs sometimes use filesystem paths instead
of URIs to represent sources. Previously, our solution was to have a
`FileAccessor` which tried to treat the path both ways (first as a
filesystem path, then a URI). But by being strategic about exactly where
we expect filesystem paths and sanitizing them, we can just consistently
use URIs in the rest of the code.

Additionally this PR introduces integration tests for the debugger.

We also fix two small-ish bugs here:
- launch.json variables like `{file}` and `{workspaceFolder}` weren't
treated properly on all platforms/filesystems
- when launching the debugger on an untitled file, the user may be
prompted to save the file. If they choose to save the file, the debugger
will fail to launch. (This is probably not as big an issue anymore,
since we disabled the auto-save behavior by default since I've opened
this PR.)
  • Loading branch information
@minestarks
minestarks authored Mar 11, 2024
1 parent 458d1d9 commit 19fcae9
Show file tree
Hide file tree
Showing 12 changed files with 771 additions and 224 deletions.
27 changes: 8 additions & 19 deletions compiler/qsc/src/interpret.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub use qsc_eval::{
use crate::{
error::{self, WithStack},
incremental::Compiler,
location::Location,
};
use debug::format_call_stack;
use miette::Diagnostic;
Expand Down Expand Up @@ -437,25 +438,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),
),
}
})
Expand Down Expand Up @@ -541,10 +532,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)]
Expand Down
8 changes: 0 additions & 8 deletions vscode/src/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TextDocument>;
openUri(uri: Uri): Promise<TextDocument>;
}

export function basename(path: string): string | undefined {
return path.replace(/\/+$/, "").split("/").pop();
}
Expand Down
104 changes: 52 additions & 52 deletions vscode/src/debugger/activate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
Expand All @@ -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")
Expand Down Expand Up @@ -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<vscode.TextDocument> {
const uri: vscode.Uri = this.resolvePathToUri(path);
return this.openUri(uri);
},
async openUri(uri: vscode.Uri): Promise<vscode.TextDocument> {
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
{
Expand All @@ -194,12 +197,9 @@ class InlineDebugAdapterFactory
_executable: vscode.DebugAdapterExecutable | undefined,
): Promise<vscode.DebugAdapterDescriptor> {
const worker = debugServiceWorkerFactory();
const uri = workspaceFileAccessor.resolvePathToUri(
session.configuration.program,
);
const uri = vscode.Uri.parse(session.configuration.programUri);
const project = await loadProject(uri);
const qscSession = new QscDebugSession(
workspaceFileAccessor,
worker,
session.configuration,
project.sources,
Expand Down
Loading

0 comments on commit 19fcae9

Please sign in to comment.