Skip to content

Commit

Permalink
Project system refactor in preparation for packages work (#1663)
Browse files Browse the repository at this point in the history
This is a large refactor of project system code in preparation for the
packages work.

This aims to collapse a lot of types across the JS/VSCode/WASM/compiler
layers that are similar-but-not-quite the same. I did end up getting rid
of a lot of types, but in some places confusion is unavoidable (e.g.
`ProjectConfig` vs `ProgramConfig`). I tried to document those places as
much as possible.

**Highlights**
- 🎉 Get rid of the deprecated overloads in `ICompiler`
- 🎊 Replace a bunch of separately-passed-in callbacks in the WASM layer
with a trait and eliminate a _lot_ of layers to plumb those callbacks
through `ProjectLoader`, `LanguageService`, etc. This also eliminates
the need for some complex macros and manually-defined types such as
`type AsyncFunction<'a, Arg, Return> = Box<dyn Fn(Arg) ->
PinnedFuture<Return> + 'a>`
- 🍾 Define a `ProgramConfig` type to group the compiler-related
parameters that are commonly passed into the WASM functions, so we don't
have to update 1,000,000 spots when we add a new compiler setting, or,
say, package support.
- 🙌 Consolidate `FileSystem` and `FileSystemAsync`
- 🥳 The manifest is now consistently parsed in the native layer, rather
than the parsing sometimes being done in JS.

Changes by layer:

- `qsc_project`
- Consolidate `FileSystem` and `FileSystemAsync` implementations by
making one call the other. Previously, we had two copies of the
implementation, one sync and one async - and they're about to get a lot
more complicated with packages work. So I tried an approach where the
sync implementation just wraps the `async` one.
- This also means that the `async` feature is no longer optional in this
crate.
- Define a trait, `JSProjectHost`, to define the manifest/filesystem
functions, instead of using a struct with callbacks. This helped get rid
of a lot of boxed async function types we were having to store in
different places, and replaced them with more intuitive trait impls.
- The manifest is now being parsed in the native layer instead of JS.
Hopefully this will centralize our error handling a bit and make
behavior more consistent across scenarios (Python, JS). It also lays the
groundwork for package dependencies, where we'll have to parse
dependency manifests in the native layer anyway.
- The project loading code now returns the `name` and `manifest_path`,
values which are used in various extension features. Previously these
values were being generated JS-side, but it makes sense to populate them
at the point we're loading the project.

- `language_service` 
- Call the new `find_manifest_directory` host function instead of
`get_manifest` which used to do the manifest parsing JS-side.
- Remove the project system callbacks in favor of the unified
`JSProjectHost` trait.
- Remove `LoadManifestResult` in favor of the `Project` type already
defined in `qsc_project`
- Tests: Split out the in-memory FS used in a few different tests, to a
shared module `test_fs`.

- `npm`
- Get rid of the deprecated overloads in `ICompiler`. QCOM has already
been updated to use the new signatures.
- Where applicable, `ICompiler` and `IDebugService` methods all take a
consistent `ProgramConfig` type, which contains the sources and compiler
settings, which also has a straightforward conversion to the type used
in the WASM crate.
- Verified backcompat with QCOM. (`IDebugService` is a breaking change
but it's not used anywhere but VS Code).

- `pip`
- Only change here is to trivially plumb through the new `resolve_path`
filesystem method.

- `vscode`
- Fix up the various spots where the `ICompiler` methods were updated to
take `ProgramConfig`.
- We don't parse the manifest in the VS Code layer anymore. Instead we
just discover it (in `findManifestDirectory`), and the native code does
all the rest.

- `wasm`
- Consolidate parameters such as `sources`, `language_features`,
`target_profile` into one `ProgramConfig` type. This is _almost_ the
same as the `ProgramConfig` in compiler.ts , except this one is more
strictly typed since backcompat isn't a concern.
- Remove tons of code that was needed make the async JS callbacks work,
replacing it with a `wasm-bindgen` attributes where applicable, and
using a `trait` instead of separate callbacks.
  • Loading branch information
minestarks authored Jun 26, 2024
1 parent ed137c4 commit 21a2961
Show file tree
Hide file tree
Showing 41 changed files with 1,064 additions and 1,207 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions compiler/qsc_project/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ serde_json = { workspace = true }
thiserror = { workspace = true }
miette = { workspace = true }
regex-lite = { workspace = true }
async-trait = { workspace = true, optional = true }
async-trait = { workspace = true }
qsc_linter = { path = "../qsc_linter" }
futures = { workspace = true }

[dev-dependencies]
expect-test = { workspace = true }
qsc_project = { path = ".", features = ["fs"] }

[features]
fs = []
async = ["async-trait"]

[lints]
workspace = true
Expand Down
31 changes: 30 additions & 1 deletion compiler/qsc_project/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{DirEntry, EntryType, FileSystem};
use miette::{Context, IntoDiagnostic};
use std::convert::Infallible;
use std::fs::DirEntry as StdEntry;
use std::path::Path;
use std::path::{Component, Path};
use std::{path::PathBuf, sync::Arc};

/// This struct represents management of Q# projects from the [`std::fs`] filesystem implementation.
Expand Down Expand Up @@ -91,4 +91,33 @@ impl FileSystem for StdFs {
.collect::<Result<_, _>>()
.map_err(crate::Error::from)?)
}

fn resolve_path(&self, base: &Path, path: &Path) -> miette::Result<PathBuf> {
let joined = base.join(path);
// Adapted from https://github.com/rust-lang/cargo/blob/a879a1ca12e3997d9fdd71b70f34f1f3c866e1da/crates/cargo-util/src/paths.rs#L84
let mut components = joined.components().peekable();
let mut normalized = if let Some(c @ Component::Prefix(..)) = components.peek().copied() {
components.next();
PathBuf::from(c.as_os_str())
} else {
PathBuf::new()
};

for component in components {
match component {
Component::Prefix(..) => unreachable!(),
Component::RootDir => {
normalized.push(component.as_os_str());
}
Component::CurDir => {}
Component::ParentDir => {
normalized.pop();
}
Component::Normal(c) => {
normalized.push(c);
}
}
}
Ok(normalized)
}
}
61 changes: 43 additions & 18 deletions compiler/qsc_project/src/js.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

use crate::{DirEntry, EntryType};
use crate::{DirEntry, EntryType, FileSystemAsync};
use async_trait::async_trait;
use miette::Error;
use std::{convert::Infallible, path::PathBuf, sync::Arc};

#[derive(Debug)]
Expand All @@ -21,21 +23,44 @@ impl DirEntry for JSFileEntry {
PathBuf::from(&self.name)
}
}
/// the desugared return type of an "async fn"
type PinnedFuture<T> = Pin<Box<dyn Future<Output = T>>>;

/// represents a unary async fn where `Arg` is the input
/// parameter and `Return` is the return type. The lifetime
/// `'a` represents the lifetime of the contained `dyn Fn`.
type AsyncFunction<'a, Arg, Return> = Box<dyn Fn(Arg) -> PinnedFuture<Return> + 'a>;
use std::{future::Future, pin::Pin};

pub struct ProjectSystemCallbacks<'a> {
/// Callback which lets the service read a file from the target filesystem
pub read_file: AsyncFunction<'a, String, (Arc<str>, Arc<str>)>,
/// Callback which lets the service list directory contents
/// on the target file system
pub list_directory: AsyncFunction<'a, String, Vec<JSFileEntry>>,
/// Fetch the manifest file for a specific path
pub get_manifest: AsyncFunction<'a, String, Option<crate::ManifestDescriptor>>,

/// Trait for interacting with a project host in JavaScript.
#[async_trait(?Send)]
pub trait JSProjectHost {
async fn read_file(&self, uri: &str) -> (Arc<str>, Arc<str>);
async fn list_directory(&self, dir_uri: &str) -> Vec<JSFileEntry>;
async fn resolve_path(&self, base: &str, path: &str) -> Option<Arc<str>>;
async fn find_manifest_directory(&self, doc_uri: &str) -> Option<Arc<str>>;
}

/// [`FileSystemAsync`] implementation for types that implement [`JSProjectHost`].
#[async_trait(?Send)]
impl<T> FileSystemAsync for T
where
T: JSProjectHost + ?Sized,
{
type Entry = JSFileEntry;

async fn read_file(
&self,
path: &std::path::Path,
) -> miette::Result<(std::sync::Arc<str>, std::sync::Arc<str>)> {
return Ok(self.read_file(&path.to_string_lossy()).await);
}

async fn list_directory(&self, path: &std::path::Path) -> miette::Result<Vec<Self::Entry>> {
return Ok(self.list_directory(&path.to_string_lossy()).await);
}

async fn resolve_path(
&self,
base: &std::path::Path,
path: &std::path::Path,
) -> miette::Result<std::path::PathBuf> {
let res = self
.resolve_path(&base.to_string_lossy(), &path.to_string_lossy())
.await
.ok_or(Error::msg("Path could not be resolved"))?;
return Ok(PathBuf::from(res.to_string()));
}
}
3 changes: 1 addition & 2 deletions compiler/qsc_project/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ mod project;
pub use error::Error;
#[cfg(feature = "fs")]
pub use fs::StdFs;
pub use js::{JSFileEntry, ProjectSystemCallbacks};
pub use js::{JSFileEntry, JSProjectHost};
pub use manifest::{Manifest, ManifestDescriptor, MANIFEST_FILE_NAME};
#[cfg(feature = "async")]
pub use project::FileSystemAsync;
pub use project::{DirEntry, EntryType, FileSystem, Project};
13 changes: 1 addition & 12 deletions compiler/qsc_project/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{

pub use qsc_linter::LintConfig;
use serde::{Deserialize, Serialize};
use std::{path::PathBuf, sync::Arc};
use std::path::PathBuf;

pub const MANIFEST_FILE_NAME: &str = "qsharp.json";

Expand All @@ -34,17 +34,6 @@ pub struct ManifestDescriptor {
pub manifest_dir: PathBuf,
}

impl ManifestDescriptor {
/// Generate a canonical compilation URI for the project associated with this manifest
#[must_use]
pub fn compilation_uri(&self) -> Arc<str> {
Arc::from(format!(
"{}/qsharp.json",
self.manifest_dir.to_string_lossy()
))
}
}

#[cfg(feature = "fs")]
impl Manifest {
/// Starting from the current directory, traverse ancestors until
Expand Down
134 changes: 85 additions & 49 deletions compiler/qsc_project/src/project.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

use crate::manifest::ManifestDescriptor;
use crate::{Manifest, ManifestDescriptor};
use async_trait::async_trait;
use futures::FutureExt;
use std::{
path::{Path, PathBuf},
sync::Arc,
};

/// Describes a Q# project
#[derive(Default, Debug)]
#[derive(Debug)]
pub struct Project {
/// Friendly name, typically based on project directory name
pub name: Arc<str>,
/// Full path to the project's `qsharp.json` file
pub manifest_path: Arc<str>,
pub sources: Vec<(Arc<str>, Arc<str>)>,
pub manifest: crate::Manifest,
}
Expand Down Expand Up @@ -48,18 +54,20 @@ pub trait DirEntry {
/// an OS filesystem. It could be a virtual filesystem on vscode.dev, or perhaps a
/// cached implementation. This interface defines the minimal filesystem requirements
/// for the Q# project system to function correctly.
#[cfg(feature = "async")]
use async_trait::async_trait;
#[cfg(feature = "async")]
#[async_trait(?Send)]
pub trait FileSystemAsync {
type Entry: DirEntry + Send + Sync;
type Entry: DirEntry;
/// Given a path, parse its contents and return a tuple representing (FileName, FileContents).
async fn read_file(&self, path: &Path) -> miette::Result<(Arc<str>, Arc<str>)>;

/// Given a path, list its directory contents (if any).
/// This function should only return files that end in *.qs and folders.
async fn list_directory(&self, path: &Path) -> miette::Result<Vec<Self::Entry>>;

/// Given a base path and a relative path, join the segments and normalize
/// the path, i.e. replace '..', '.', and redundant separators.
async fn resolve_path(&self, base: &Path, path: &Path) -> miette::Result<PathBuf>;

/// Given an initial path, fetch files matching <initial_path>/**/*.qs
async fn collect_project_sources(
&self,
Expand Down Expand Up @@ -97,6 +105,7 @@ pub trait FileSystemAsync {
}
Ok(files)
}

/// Given a [ManifestDescriptor], load project sources.
async fn load_project(&self, manifest: &ManifestDescriptor) -> miette::Result<Project> {
let project_path = manifest.manifest_dir.clone();
Expand All @@ -109,11 +118,43 @@ pub trait FileSystemAsync {
sources.push(self.read_file(&path).await?);
}

let manifest_path = self
.resolve_path(&manifest.manifest_dir, Path::new("qsharp.json"))
.await?;

Ok(Project {
name: manifest
.manifest_dir
.file_name()
.map(|f| f.to_string_lossy().into())
.unwrap_or(format!("Q# project at {}", manifest.manifest_dir.display()).into()),
manifest_path: manifest_path.to_string_lossy().into(),
manifest: manifest.manifest.clone(),
sources,
})
}

/// Given a directory path, parse the manifest and load the project sources.
async fn load_project_in_dir(&self, directory: &Path) -> miette::Result<Project> {
let manifest = self.parse_manifest_in_dir(directory).await?;

self.load_project(&ManifestDescriptor {
manifest_dir: directory.to_path_buf(),
manifest,
})
.await
}

async fn parse_manifest_in_dir(&self, directory: &Path) -> Result<Manifest, miette::Error> {
let manifest_path = self
.resolve_path(directory, Path::new("qsharp.json"))
.await?;
let (_, manifest_content) = self.read_file(&manifest_path).await?;
let manifest = serde_json::from_str::<Manifest>(&manifest_content).map_err(|e| {
miette::ErrReport::msg(format!("Failed to parse `qsharp.json` file: {e}"))
})?;
Ok(manifest)
}
}

/// Filters out any hidden files (files that start with '.')
Expand All @@ -135,54 +176,49 @@ pub trait FileSystem {

/// Given a path, list its directory contents (if any).
fn list_directory(&self, path: &Path) -> miette::Result<Vec<Self::Entry>>;
/// Given an initial path, fetch files matching <`initial_path`>/**/*.qs
fn collect_project_sources(&self, initial_path: &Path) -> miette::Result<Vec<Self::Entry>> {
let listing = self.list_directory(initial_path)?;
if let Some(src_dir) = listing.into_iter().find(|x| {
let Ok(entry_type) = x.entry_type() else {
return false;
};
entry_type == EntryType::Folder && x.entry_name() == "src"
}) {
self.collect_project_sources_inner(&src_dir.path())
} else {
Err(miette::ErrReport::msg(
"No `src` directory found for project.",
))
}
}

fn collect_project_sources_inner(
&self,
initial_path: &Path,
) -> miette::Result<Vec<Self::Entry>> {
let listing = self.list_directory(initial_path)?;
let mut files = vec![];
for item in filter_hidden_files(listing.into_iter()) {
match item.entry_type() {
Ok(EntryType::File) if item.entry_extension() == "qs" => files.push(item),
Ok(EntryType::Folder) => {
files.append(&mut self.collect_project_sources_inner(&item.path())?);
}
_ => (),
}
}
Ok(files)
}
fn resolve_path(&self, base: &Path, path: &Path) -> miette::Result<PathBuf>;

/// Given a [`ManifestDescriptor`], load project sources.
fn load_project(&self, manifest: &ManifestDescriptor) -> miette::Result<Project> {
let project_path = manifest.manifest_dir.clone();
let qs_files = self.collect_project_sources(&project_path)?;

let qs_files = qs_files.into_iter().map(|file| file.path());
// Rather than rewriting all the async code in the project loader,
// we call the async implementation here, doing some tricks to make it
// run synchronously.

let fs = ToFileSystemAsync { fs: self };

// WARNING: This will panic if there are *any* await points in the
// load_project implementation. Right now, we know that will never be the case
// because we just passed in our synchronous FS functions to the project loader.
// Proceed with caution if you make the `FileSystemAsync` implementation any
// more complex.
FutureExt::now_or_never(fs.load_project(manifest)).expect("load_project should never await")
}
}

let qs_sources = qs_files.map(|path| self.read_file(&path));
/// Trivial wrapper to turn a `FileSystem` into a `FileSystemAsync`
struct ToFileSystemAsync<'a, FS>
where
FS: ?Sized,
{
fs: &'a FS,
}

let sources = qs_sources.collect::<miette::Result<_>>()?;
Ok(Project {
manifest: manifest.manifest.clone(),
sources,
})
#[async_trait(?Send)]
impl<FS, E> FileSystemAsync for ToFileSystemAsync<'_, FS>
where
E: DirEntry,
FS: FileSystem<Entry = E> + ?Sized,
{
type Entry = E;

async fn read_file(&self, path: &Path) -> miette::Result<(Arc<str>, Arc<str>)> {
self.fs.read_file(path)
}
async fn list_directory(&self, path: &Path) -> miette::Result<Vec<Self::Entry>> {
self.fs.list_directory(path)
}
async fn resolve_path(&self, base: &Path, path: &Path) -> miette::Result<PathBuf> {
self.fs.resolve_path(base, path)
}
}
Loading

0 comments on commit 21a2961

Please sign in to comment.