diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c3859539e..a85517bdf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,9 +1,7 @@ # Contributing This project is not open to unsolicited code contributions (for the time being). -You are more than free to play around with the code though! The instructions below should be enough to get -you started. I suggest looking at [`ARCHITECTURE.md`](ARCHITECTURE.md) as well to get a sense of the overall project -structure. +You are more than free to play around with the code though! The instructions below should be enough to get you started. I suggest looking at [`ARCHITECTURE.md`](ARCHITECTURE.md) as well to get a sense of the overall project structure. # Prerequisites @@ -19,8 +17,7 @@ cargo nextest run ``` We primarily rely on end-to-end testing to check that `pavex`'s behaviour meets our expectations. -All tests are located in `libs/pavex_cli/tests` and are launched using a custom test runner that you can find -in `libs/pavex_test_runner`. +All tests are located in `libs/pavex_cli/tests` and are launched using a custom test runner that you can find in `libs/pavex_test_runner`. In a nutshell: @@ -30,9 +27,21 @@ In a nutshell: - if the test is expected to pass, we check the generated code and the graph diagnostics; - if the test is expected to fail, we check `stderr` to verify the quality of the error message returned to users. -For each, a runtime environment is created as a sub-folder of `ui_test_envs`, which is in turn generated at the root -of `pavex`'s workspace. -We use a consistent folder to leverage `cargo` caching and speed up successive test runs. It also allows you to easily -inspect the artifacts generated during the test run. -If you suspect that something funny is going on due to cross-run contamination, delete the `ui_test_envs` folder to get -a clean slate. \ No newline at end of file +## Test runtime environment + +For each test, a runtime environment is created as a sub-folder of `ui_test_envs`, which is in turn generated at the root of `pavex`'s workspace. +We use a consistent folder to leverage `cargo` caching and speed up successive test runs. It also allows you to easily inspect the artifacts generated during the test run. +If you suspect that something funny is going on due to cross-run contamination, delete the `ui_test_envs` folder to get a clean slate. + +## Updating saved snapshots + +The generated code or the graph diagnostics may not match our expectations. +The test runner will save the unexpected output in a file named like the expectation file with an additional `.snap` suffix. You can then choose to update the saved snapshot via our utility CLI: + +```bash +# It must be run from the root folder of the workspace +cargo r --bin snaps +``` + +It will cycle through all `.snap` files and print the changeset with respect to our previous expectations. +You will then be prompted to decide if you want to update the saved snapshot to match the new value or if you prefer to keep it as it. diff --git a/examples/app_blueprint/Cargo.toml b/examples/app_blueprint/Cargo.toml index ae455de96..861665cec 100644 --- a/examples/app_blueprint/Cargo.toml +++ b/examples/app_blueprint/Cargo.toml @@ -7,14 +7,6 @@ edition = "2021" path = "src/bin.rs" name = "bp" -[[bin]] -path = "src/app.rs" -name = "app" - [dependencies] pavex_builder = { path = "../../libs/pavex_builder" } -http = "0.2" -hyper = { version = "0.14", features = ["server", "http1", "http2", "runtime"] } -matchit = "0.6.0" -tokio = { version = "1"} -anyhow = { version = "<=1.0.59"} +pavex_runtime = { path = "../../libs/pavex_runtime" } \ No newline at end of file diff --git a/examples/app_blueprint/blueprint.json b/examples/app_blueprint/blueprint.ron similarity index 84% rename from examples/app_blueprint/blueprint.json rename to examples/app_blueprint/blueprint.ron index 4ba900525..2a8835f08 100644 --- a/examples/app_blueprint/blueprint.json +++ b/examples/app_blueprint/blueprint.ron @@ -20,14 +20,14 @@ ), ], component_lifecycles: { - ( - registered_at: "app_blueprint", - import_path: "crate :: extract_path", - ): RequestScoped, ( registered_at: "app_blueprint", import_path: "crate :: http_client", ): Singleton, + ( + registered_at: "app_blueprint", + import_path: "crate :: extract_path", + ): RequestScoped, ( registered_at: "app_blueprint", import_path: "crate :: logger", @@ -45,7 +45,7 @@ import_path: "crate :: stream_file", ): [ ( - line: 40, + line: 41, column: 10, file: "examples/app_blueprint/src/lib.rs", ), @@ -56,25 +56,25 @@ registered_at: "app_blueprint", import_path: "crate :: extract_path", ): ( - line: 38, - column: 10, - file: "examples/app_blueprint/src/lib.rs", + line: 58, + column: 32, + file: "libs/pavex_builder/src/app.rs", ), ( registered_at: "app_blueprint", import_path: "crate :: logger", ): ( - line: 39, - column: 10, - file: "examples/app_blueprint/src/lib.rs", + line: 58, + column: 32, + file: "libs/pavex_builder/src/app.rs", ), ( registered_at: "app_blueprint", import_path: "crate :: http_client", ): ( - line: 37, - column: 10, - file: "examples/app_blueprint/src/lib.rs", + line: 58, + column: 32, + file: "libs/pavex_builder/src/app.rs", ), }, ) \ No newline at end of file diff --git a/examples/app_blueprint/src/app.rs b/examples/app_blueprint/src/app.rs deleted file mode 100644 index 0b5adb77b..000000000 --- a/examples/app_blueprint/src/app.rs +++ /dev/null @@ -1,65 +0,0 @@ -struct ServerState { - router: matchit::Router, - application_state: ApplicationState, -} -pub struct ApplicationState { - s0: app_blueprint::HttpClient, -} -pub fn build_application_state(v0: app_blueprint::Config) -> ApplicationState { - let v1 = app_blueprint::http_client(v0); - ApplicationState { s0: v1 } -} -pub async fn run( - server_builder: hyper::server::Builder, - application_state: ApplicationState, -) -> Result<(), anyhow::Error> { - let server_state = std::sync::Arc::new(ServerState { - router: build_router()?, - application_state, - }); - let make_service = hyper::service::make_service_fn(move |_| { - let server_state = server_state.clone(); - async move { - Ok::< - _, - hyper::Error, - >( - hyper::service::service_fn(move |request| { - let server_state = server_state.clone(); - async move { - Ok::<_, hyper::Error>(route_request(request, server_state)) - } - }), - ) - } - }); - server_builder.serve(make_service).await.map_err(Into::into) -} -fn build_router() -> Result, matchit::InsertError> { - let mut router = matchit::Router::new(); - router.insert("/home", 0u32)?; - Ok(router) -} -fn route_request( - request: http::Request, - server_state: std::sync::Arc, -) -> http::Response { - let route_id = server_state - .router - .at(request.uri().path()) - .expect("Failed to match incoming request path"); - match route_id.value { - 0u32 => route_handler_0(server_state.application_state.s0.clone(), request), - _ => panic!("This is a bug, no route registered for a route id"), - } -} -pub fn route_handler_0( - v0: app_blueprint::HttpClient, - v1: http::request::Request, -) -> http::response::Response { - let v2 = app_blueprint::extract_path(v1); - let v3 = app_blueprint::logger(); - app_blueprint::stream_file(v2, v3, v0) -} - -fn main () {} \ No newline at end of file diff --git a/examples/app_blueprint/src/bin.rs b/examples/app_blueprint/src/bin.rs index d15c4b14e..bcfe4333e 100644 --- a/examples/app_blueprint/src/bin.rs +++ b/examples/app_blueprint/src/bin.rs @@ -1,10 +1,11 @@ -use app_blueprint::app_blueprint; use std::error::Error; use std::path::PathBuf; use std::str::FromStr; +use app_blueprint::app_blueprint; + fn main() -> Result<(), Box> { - let path = PathBuf::from_str("blueprint.json")?; + let path = PathBuf::from_str("blueprint.ron")?; app_blueprint().persist(&path)?; std::process::Command::new("../../target/debug/pavex_cli") diff --git a/examples/app_blueprint/src/lib.rs b/examples/app_blueprint/src/lib.rs index 402e9a587..47a06843c 100644 --- a/examples/app_blueprint/src/lib.rs +++ b/examples/app_blueprint/src/lib.rs @@ -1,9 +1,10 @@ -use pavex_builder::{f, AppBlueprint, Lifecycle}; use std::path::PathBuf; +use pavex_builder::{f, AppBlueprint, Lifecycle}; + pub struct Logger; -pub fn extract_path(_inner: http::Request) -> PathBuf { +pub fn extract_path(_inner: pavex_runtime::http::Request) -> PathBuf { todo!() } @@ -15,7 +16,7 @@ pub fn stream_file( _inner: PathBuf, _logger: Logger, _http_client: HttpClient, -) -> http::Response { +) -> pavex_runtime::http::Response { todo!() } diff --git a/libs/pavex/Cargo.toml b/libs/pavex/Cargo.toml index ed1ac7c1f..3f9b7b935 100644 --- a/libs/pavex/Cargo.toml +++ b/libs/pavex/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" [dependencies] pavex_builder = { path = "../pavex_builder" } syn = { version = "1.0.96", features = ["full", "extra-traits", "visit"] } -rustdoc-types = "0.14.0" +rustdoc-types = "0.17.0" serde_json = "1" anyhow = "1.0.58" fs-err = "2.7.0" diff --git a/libs/pavex/src/language/callable_path.rs b/libs/pavex/src/language/callable_path.rs index fb8105f88..15eaac637 100644 --- a/libs/pavex/src/language/callable_path.rs +++ b/libs/pavex/src/language/callable_path.rs @@ -1,18 +1,20 @@ use std::fmt::{Display, Formatter}; -use quote::quote; -use syn::ExprPath; +use syn::{ExprPath, GenericArgument, PathArguments, Type}; use pavex_builder::RawCallableIdentifiers; /// A path that can be used in expression position (i.e. to refer to a function or a static method). #[derive(Clone, Debug, Hash, Eq, PartialEq)] -pub(crate) struct CallPath(pub ExprPath); +pub(crate) struct CallPath { + pub has_leading_colon: bool, + pub segments: Vec, +} -impl AsRef for CallPath { - fn as_ref(&self) -> &ExprPath { - &self.0 - } +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub(crate) struct CallPathSegment { + pub ident: syn::Ident, + pub generic_arguments: Vec, } impl CallPath { @@ -22,7 +24,48 @@ impl CallPath { raw_identifiers: callable_identifiers.to_owned(), parsing_error: e, })?; - Ok(Self(callable_path)) + Self::parse_from_path(callable_path.path) + } + + pub(crate) fn parse_from_path(path: syn::Path) -> Result { + let has_leading_colon = path.leading_colon.is_some(); + let mut segments = Vec::with_capacity(path.segments.len()); + for syn_segment in path.segments { + let generic_arguments = match syn_segment.arguments { + PathArguments::None => vec![], + PathArguments::AngleBracketed(syn_arguments) => { + let mut arguments = Vec::with_capacity(syn_arguments.args.len()); + for syn_argument in syn_arguments.args { + let argument = match syn_argument { + GenericArgument::Type(p) => match p { + Type::Path(p) => Self::parse_from_path(p.path)?, + _ => unreachable!(), + }, + GenericArgument::Lifetime(_) + | GenericArgument::Binding(_) + | GenericArgument::Constraint(_) + | GenericArgument::Const(_) => todo!( + "We can only handle generic type parameters for the time being." + ), + }; + arguments.push(argument) + } + arguments + } + PathArguments::Parenthesized(_) => { + todo!("We do not handle paranthesized generic parameters") + } + }; + let segment = CallPathSegment { + ident: syn_segment.ident, + generic_arguments, + }; + segments.push(segment) + } + Ok(Self { + has_leading_colon, + segments, + }) } /// Return the first segment in the path. @@ -31,21 +74,36 @@ impl CallPath { /// return `my_module`. pub fn leading_path_segment(&self) -> &proc_macro2::Ident { // This unwrap never fails thanks to the validation done in `parse` - &self.0.path.segments.first().unwrap().ident + &self.segments.first().unwrap().ident + } +} + +impl Display for CallPath { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let leading_colon = if self.has_leading_colon { "::" } else { "" }; + write!(f, "{}", leading_colon)?; + let last_segment_index = self.segments.len().saturating_sub(1); + for (i, segment) in self.segments.iter().enumerate() { + write!(f, "{}", segment)?; + if i != last_segment_index { + write!(f, "::")?; + } + } + Ok(()) } } -impl std::fmt::Display for CallPath { +impl Display for CallPathSegment { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let path = &self.0; - let s = quote! { #path }.to_string(); - write!( - f, - "{}", - s.replace(" :: ", "::") - .replace("< ", "<") - .replace(" >", ">") - ) + write!(f, "{}", self.ident)?; + let last_argument_index = self.generic_arguments.len().saturating_sub(1); + for (j, argument) in self.generic_arguments.iter().enumerate() { + write!(f, "{}", argument)?; + if j != last_argument_index { + write!(f, ", ")?; + } + } + Ok(()) } } diff --git a/libs/pavex/src/language/mod.rs b/libs/pavex/src/language/mod.rs index f43c10129..7f970a0d1 100644 --- a/libs/pavex/src/language/mod.rs +++ b/libs/pavex/src/language/mod.rs @@ -1,3 +1,8 @@ +pub(crate) use callable::Callable; +pub(crate) use callable_path::{CallPath, InvalidCallPath}; +pub(crate) use resolved_path::{ParseError, ResolvedPath, ResolvedPathSegment, UnknownPath}; +pub(crate) use resolved_type::ResolvedType; + mod callable; mod callable_path; mod resolved_path; @@ -5,8 +10,3 @@ mod resolved_type; // E.g. `["std", "path", "PathBuf"]`. pub type ImportPath = Vec; - -pub(crate) use callable::Callable; -pub(crate) use callable_path::{CallPath, InvalidCallPath}; -pub(crate) use resolved_path::{EncodedResolvedPath, ParseError, ResolvedPath, UnknownPath}; -pub(crate) use resolved_type::ResolvedType; diff --git a/libs/pavex/src/language/resolved_path.rs b/libs/pavex/src/language/resolved_path.rs index df53b289f..88f4f262a 100644 --- a/libs/pavex/src/language/resolved_path.rs +++ b/libs/pavex/src/language/resolved_path.rs @@ -1,16 +1,18 @@ +use std::fmt::Write; use std::fmt::{Display, Formatter}; use std::hash::Hash; +use anyhow::Context; +use bimap::BiHashMap; use guppy::PackageId; use itertools::Itertools; use quote::format_ident; use rustdoc_types::{Item, ItemEnum}; -use syn::ExprPath; use pavex_builder::RawCallableIdentifiers; use crate::language::{CallPath, InvalidCallPath}; -use crate::rustdoc::CrateCollection; +use crate::rustdoc::{CrateCollection, STD_PACKAGE_ID, TOOLCHAIN_CRATES}; /// A resolved import path. /// @@ -35,23 +37,27 @@ use crate::rustdoc::CrateCollection; /// and well-defined within a `cargo` workspace. #[derive(Clone, Debug, Hash, Eq)] pub(crate) struct ResolvedPath { - pub path: CallPath, + pub segments: Vec, pub package_id: PackageId, } +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub(crate) struct ResolvedPathSegment { + pub ident: String, + pub generic_arguments: Vec, +} + impl PartialEq for ResolvedPath { fn eq(&self, other: &Self) -> bool { - let is_equal = self.package_id == other.package_id - && self.path.0.attrs == other.path.0.attrs - && self.path.0.qself == other.path.0.qself - && self.path.0.path.segments.len() == other.path.0.path.segments.len(); + let is_equal = + self.package_id == other.package_id && self.segments.len() == other.segments.len(); if is_equal { // We want to ignore the first segment of the path, because dependencies can be // renamed and this can lead to equivalent paths not being considered equal. // Given that we already have the package id as part of the type, it is safe // to disregard the first segment when determining equality. - let self_segments = self.path.0.path.segments.iter().skip(1); - let other_segments = other.path.0.path.segments.iter().skip(1); + let self_segments = self.segments.iter().skip(1); + let other_segments = other.segments.iter().skip(1); for (self_segment, other_segment) in self_segments.zip_eq(other_segments) { if self_segment != other_segment { return false; @@ -70,25 +76,43 @@ impl ResolvedPath { graph: &guppy::graph::PackageGraph, ) -> Result { let mut path = CallPath::parse(identifiers)?; - // Normalize by removing the leading semicolon for external paths. - path.0.path.leading_colon = None; if path.leading_path_segment() == "crate" { let first_segment = path - .0 - .path .segments .first_mut() - .expect("Bug: an `ExprPath` with no path segments!"); + .expect("Bug: a `CallPath` with no path segments!"); // Unwrapping here is safe: there is always at least one path segment in a successfully // parsed `ExprPath`. first_segment.ident = format_ident!("{}", identifiers.registered_at()); } + Self::parse_call_path(path, identifiers, graph) + } + + fn parse_call_path( + path: CallPath, + identifiers: &RawCallableIdentifiers, + graph: &guppy::graph::PackageGraph, + ) -> Result { + let registered_at = identifiers.registered_at(); + let krate_name_candidate = path.leading_path_segment().to_string(); + + let mut segments = vec![]; + for raw_segment in path.segments { + let generic_arguments = raw_segment + .generic_arguments + .into_iter() + .map(|arg| Self::parse_call_path(arg, identifiers, graph)) + .collect::, _>>()?; + let segment = ResolvedPathSegment { + ident: raw_segment.ident.to_string(), + generic_arguments, + }; + segments.push(segment); + } - let registration_crate_name = identifiers.registered_at(); let registration_package = graph.packages() - .find(|p| p.name() == registration_crate_name) + .find(|p| p.name() == registered_at) .expect("There is no package in the current workspace whose name matches the registration crate for these identifiers"); - let krate_name_candidate = path.leading_path_segment().to_string(); let package_id = if registration_package.name() == krate_name_candidate { registration_package.id().to_owned() } else if let Some(dependency) = registration_package @@ -96,20 +120,26 @@ impl ResolvedPath { .find(|d| d.resolved_name() == krate_name_candidate) { dependency.to().id().to_owned() + } else if TOOLCHAIN_CRATES.contains(&krate_name_candidate.as_str()) { + PackageId::new(STD_PACKAGE_ID) } else { return Err(PathMustBeAbsolute { raw_identifiers: identifiers.to_owned(), } .into()); }; - Ok(Self { path, package_id }) + Ok(Self { + segments, + package_id, + }) } /// Return the name of the crate that this type belongs to. /// /// E.g. `my_crate::my_module::MyType` will return `my_crate`. - pub fn crate_name(&self) -> &proc_macro2::Ident { - self.path.leading_path_segment() + pub fn crate_name(&self) -> &str { + // This unwrap never fails thanks to the validation done in `parse` + &self.segments.first().unwrap().ident } /// Find information about the type that the path points at. @@ -119,9 +149,6 @@ impl ResolvedPath { .get_or_compute_by_id(&self.package_id) .unwrap(); let path_segments: Vec<_> = self - .path - .0 - .path .segments .iter() .map(|path_segment| path_segment.ident.to_string()) @@ -162,11 +189,68 @@ impl ResolvedPath { } Err(UnknownPath(self.to_owned().into())) } + + pub fn render_path(&self, id2name: &BiHashMap<&PackageId, String>) -> String { + let mut buffer = String::new(); + let crate_name = id2name + .get_by_left(&self.package_id) + .with_context(|| { + format!( + "The package id '{}' is missing from the id<>name mapping for crates.", + self.package_id + ) + }) + .unwrap(); + write!(&mut buffer, "{}", crate_name).unwrap(); + for path_segment in &self.segments[1..] { + write!(&mut buffer, "::{}", path_segment.ident).unwrap(); + let generic_arguments = &path_segment.generic_arguments; + if !generic_arguments.is_empty() { + write!(&mut buffer, "<").unwrap(); + let mut arguments = generic_arguments.iter().peekable(); + while let Some(argument) = arguments.next() { + write!(&mut buffer, "{}", argument.render_path(id2name)).unwrap(); + if arguments.peek().is_some() { + write!(&mut buffer, ", ").unwrap(); + } + } + write!(&mut buffer, ">").unwrap(); + } + } + buffer + } } impl Display for ResolvedPath { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.path) + let last_segment_index = self.segments.len().saturating_sub(1); + for (i, segment) in self.segments.iter().enumerate() { + write!(f, "{}", segment)?; + if i != last_segment_index { + write!(f, "::")?; + } + } + Ok(()) + } +} + +impl Display for ResolvedPathSegment { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.ident)?; + if !self.generic_arguments.is_empty() { + write!(f, "::<")?; + } + let last_argument_index = self.generic_arguments.len().saturating_sub(1); + for (j, argument) in self.generic_arguments.iter().enumerate() { + write!(f, "{}", argument)?; + if j != last_argument_index { + write!(f, ", ")?; + } + } + if !self.generic_arguments.is_empty() { + write!(f, ">")?; + } + Ok(()) } } @@ -190,59 +274,16 @@ impl Display for PathMustBeAbsolute { } } -impl AsRef for ResolvedPath { - fn as_ref(&self) -> &ExprPath { - self.path.as_ref() - } -} - -#[derive(Clone, Debug, Hash, Eq, PartialEq)] -/// Why does this type even exist? -/// -/// It turns out that most of `syn`'s types are neither `Send` nor `Sync` because, deep down, they -/// are holding on to one or more `Span`s, which are not `Send` nor `Sync`. -/// -/// As a consequence, we can't store `syn`'s types as fields on our error types, because `miette` -/// expects the source error types to be `Send` and `Sync`. -/// -/// This type is a crutch - every time we need to store a [`ResolvedPath`] in an error -/// type, we encode it as a string. By wrapping it in a new type, we make it obvious what that -/// string means and we keep track of the fact that this can be infallibly deserialized back -/// into a `syn::ExprPath`. -pub(crate) struct EncodedResolvedPath(String, PackageId); - -impl EncodedResolvedPath { - pub fn decode(&self) -> ResolvedPath { - ResolvedPath { - path: CallPath(syn::parse_str(&self.0).unwrap()), - package_id: self.1.clone(), - } - } -} - -impl std::fmt::Display for EncodedResolvedPath { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let decoded = self.decode(); - std::fmt::Display::fmt(&decoded, f) - } -} - -impl From for EncodedResolvedPath { - fn from(p: ResolvedPath) -> Self { - Self(p.path.to_string(), p.package_id) - } -} - #[derive(thiserror::Error, Debug)] -pub(crate) struct UnknownPath(pub EncodedResolvedPath); +pub(crate) struct UnknownPath(pub ResolvedPath); impl Display for UnknownPath { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let decoded = self.0.decode(); - let krate = decoded.crate_name().to_string(); + let path = &self.0; + let krate = path.crate_name().to_string(); write!( f, - "I could not find '{decoded}' in the auto-generated documentation for '{krate}'" + "I could not find '{path}' in the auto-generated documentation for '{krate}'" ) } } diff --git a/libs/pavex/src/language/resolved_type.rs b/libs/pavex/src/language/resolved_type.rs index d13591b8f..8538f637b 100644 --- a/libs/pavex/src/language/resolved_type.rs +++ b/libs/pavex/src/language/resolved_type.rs @@ -12,6 +12,8 @@ use crate::language::ImportPath; pub struct ResolvedType { #[serde(serialize_with = "serialize_package_id")] #[serde(deserialize_with = "deserialize_package_id")] + // `PackageId` does not implement serde::Deserialize/serde::Serialize, therefore we must + // manually specify deserializer and serializer for make the whole `ResolvedType` (de)serializable. pub package_id: PackageId, pub base_type: ImportPath, pub generic_arguments: Vec, diff --git a/libs/pavex/src/lib.rs b/libs/pavex/src/lib.rs index ca9a009a7..54d964d76 100644 --- a/libs/pavex/src/lib.rs +++ b/libs/pavex/src/lib.rs @@ -1,8 +1,6 @@ -extern crate core; +pub use web::App; mod graphmap; pub(crate) mod language; mod rustdoc; mod web; - -pub use web::App; diff --git a/libs/pavex/src/rustdoc.rs b/libs/pavex/src/rustdoc.rs index 54b5d830b..ed755bb45 100644 --- a/libs/pavex/src/rustdoc.rs +++ b/libs/pavex/src/rustdoc.rs @@ -1,12 +1,13 @@ -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; use std::default::Default; use std::fmt::{Display, Formatter}; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::str::FromStr; use anyhow::{anyhow, Context}; use guppy::graph::{PackageGraph, PackageMetadata, PackageSource}; use guppy::{PackageId, Version}; -use rustdoc_types::ItemEnum; +use rustdoc_types::{ItemEnum, Visibility}; use crate::language::ImportPath; @@ -18,16 +19,55 @@ pub struct CannotGetCrateData { pub source: anyhow::Error, } +pub const STD_PACKAGE_ID: &str = "std"; +pub const TOOLCHAIN_CRATES: [&str; 3] = ["std", "core", "alloc"]; + pub fn get_crate_data( root_folder: &Path, package_id_spec: &PackageIdSpecification, ) -> Result { - _get_crate_data(root_folder, package_id_spec).map_err(|e| CannotGetCrateData { + // Some crates are not compiled as part of the dependency tree of the current workspace. + // They are instead bundled as part of Rust's toolchain and automatically available for import + // and usage in your crate: the standard library (`std`), `core` (a smaller subset of `std` + // that does not require an allocator), `alloc` (a smaller subset of `std` that assumes you + // can allocate). + // Since those crates are pre-compiled (and somewhat special), we can't generate their + // documentation on the fly. We assume that their JSON docs have been pre-computed and are + // available for us to look at. + if TOOLCHAIN_CRATES.contains(&package_id_spec.name.as_str()) { + get_toolchain_crate_data(package_id_spec) + } else { + _get_crate_data(root_folder, package_id_spec) + } + .map_err(|e| CannotGetCrateData { package_spec: package_id_spec.to_string(), source: e, }) } +fn get_toolchain_crate_data( + package_id_spec: &PackageIdSpecification, +) -> Result { + // TODO: determine the correct path to the files using `rustup show home`, + // `rustup show active-toolchain` and `rustup component list --installed --toolchain <...>`. + let root_folder = PathBuf::from_str("/Users/luca/code/pavex/json-docs").unwrap(); + let json_path = root_folder.join(format!("{}.json", package_id_spec.name)); + let json = fs_err::read_to_string(json_path).with_context(|| { + format!( + "Failed to retrieve the JSON docs for {}", + package_id_spec.name + ) + })?; + serde_json::from_str::(&json) + .with_context(|| { + format!( + "Failed to deserialize the JSON docs for {}", + package_id_spec.name + ) + }) + .map_err(Into::into) +} + fn _get_crate_data( target_directory: &Path, package_id_spec: &PackageIdSpecification, @@ -74,8 +114,16 @@ impl CrateCollection { &mut self, package_id: &PackageId, ) -> Result<&Crate, CannotGetCrateData> { - let package_metadata = self.1.metadata(package_id).expect("Unknown package ID"); - let package_spec = PackageIdSpecification::new(&package_metadata); + let package_spec = if TOOLCHAIN_CRATES.contains(&package_id.repr()) { + PackageIdSpecification { + source: None, + name: package_id.repr().to_string(), + version: None, + } + } else { + let package_metadata = self.1.metadata(package_id).expect("Unknown package ID"); + PackageIdSpecification::new(&package_metadata) + }; if self.0.get(&package_spec.to_string()).is_none() { let krate = get_crate_data( self.1.workspace().target_directory().as_std_path(), @@ -86,6 +134,78 @@ impl CrateCollection { } Ok(&self.0[&package_spec.to_string()]) } + + /// Retrieve the package id where a certain item was originally defined. + pub fn get_defining_package_id_for_item( + &mut self, + used_by_package_id: &PackageId, + item_id: &rustdoc_types::Id, + ) -> Result { + let package_graph = self.1.clone(); + let used_by_krate = self.get_or_compute_by_id(used_by_package_id)?; + let type_summary = used_by_krate.get_summary_by_id(item_id)?; + + let type_package_id = if type_summary.crate_id == 0 { + used_by_krate.package_id().to_owned() + } else { + let (owning_crate, owning_crate_version) = used_by_krate + .get_external_crate_name(type_summary.crate_id) + .unwrap(); + if TOOLCHAIN_CRATES.contains(&owning_crate.name.as_str()) { + PackageId::new(owning_crate.name.clone()) + } else { + let transitive_dependencies = package_graph + .query_forward([used_by_package_id]) + .unwrap() + .resolve(); + let mut iterator = + transitive_dependencies.links(guppy::graph::DependencyDirection::Forward); + iterator + .find(|link| { + link.to().name() == owning_crate.name + && owning_crate_version + .as_ref() + .map(|v| link.to().version() == v) + .unwrap_or(true) + }) + .ok_or_else(|| { + anyhow!( + "I could not find the package id for the crate where `{}` is defined", + type_summary.path.join("::") + ) + }) + .unwrap() + .to() + .id() + .to_owned() + } + }; + Ok(type_package_id) + } + + pub fn get_canonical_import_path( + &mut self, + used_by_package_id: &PackageId, + item_id: &rustdoc_types::Id, + ) -> Result, anyhow::Error> { + let definition_package_id = + self.get_defining_package_id_for_item(used_by_package_id, item_id)?; + + let used_by_krate = self.get_or_compute_by_id(used_by_package_id)?; + let type_summary = used_by_krate.get_summary_by_id(item_id)?; + let referenced_base_type_path = type_summary.path.clone(); + let base_type = if type_summary.crate_id == 0 { + self.get_or_compute_by_id(used_by_package_id)? + .get_importable_path(item_id) + } else { + // The crate where the type is actually defined. + let source_crate = self.get_or_compute_by_id(&definition_package_id)?; + let type_definition_id = source_crate.get_id_by_path(&referenced_base_type_path)?; + source_crate.get_importable_path(type_definition_id) + } + .to_owned(); + Ok(base_type) + } } /// A selector that follows the [package ID specification](https://doc.rust-lang.org/cargo/reference/pkgid-spec.html). @@ -138,7 +258,7 @@ impl PackageIdSpecification { } } -impl std::fmt::Display for PackageIdSpecification { +impl Display for PackageIdSpecification { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { if let Some(source) = &self.source { write!(f, "{}#", source)?; @@ -162,6 +282,7 @@ pub struct Crate { package_id: PackageId, krate: rustdoc_types::Crate, path_index: HashMap, rustdoc_types::Id>, + public_local_path_index: HashMap>>, } impl Crate { @@ -171,11 +292,24 @@ impl Crate { .iter() .map(|(id, summary)| (summary.path.clone(), id.to_owned())) .collect(); - index_re_exports(&krate, vec![], &mut path_index, &krate.root); + + let mut public_local_path_index = HashMap::new(); + index_local_items(&krate, vec![], &mut public_local_path_index, &krate.root); + + path_index.reserve(public_local_path_index.len()); + for (id, public_paths) in &public_local_path_index { + for public_path in public_paths { + if path_index.get(public_path).is_none() { + path_index.insert(public_path.to_owned(), id.to_owned()); + } + } + } + Self { package_id, krate, path_index, + public_local_path_index, } } @@ -192,7 +326,7 @@ impl Crate { url.trim_end_matches('/') .split('/') .last() - .map(guppy::Version::parse) + .map(Version::parse) .and_then(|x| x.ok()) } else { None @@ -245,18 +379,47 @@ impl Crate { let id = self.get_id_by_path(path)?; Ok(self.get_type_by_id(id)) } + + pub fn get_importable_path(&self, id: &rustdoc_types::Id) -> &[String] { + if let Some(path) = self.public_local_path_index.get(id) { + return path.iter().next().unwrap(); + } + + let item = self.get_type_by_id(id); + if item.crate_id != 0 { + let external_crate = &self.krate.external_crates[&item.crate_id]; + panic!( + "You can only retrieve a path that is guaranteed to be public for local types. \ + `{}` is not local. That id belongs to {} (crate_id={}).", + &item.id.0, &external_crate.name, item.crate_id + ) + } + + panic!( + "Failed to find a publicly importable path for the type id `{}`. \ + This is likely to be a bug in our handling of rustdoc's JSON output.", + id.0 + ) + } } -fn index_re_exports<'a>( +fn index_local_items<'a>( krate: &'a rustdoc_types::Crate, mut current_path: Vec<&'a str>, - path_index: &mut HashMap, rustdoc_types::Id>, + path_index: &mut HashMap>>, current_item_id: &rustdoc_types::Id, ) { - // TODO: handle visibility // TODO: the way we handle `current_path` is extremely wasteful, // we can likely reuse the same buffer throughout. let current_item = &krate.index[current_item_id]; + + // We do not want to index private items. + if let Visibility::Default | Visibility::Crate | Visibility::Restricted { .. } = + current_item.visibility + { + return; + } + match ¤t_item.inner { ItemEnum::Module(m) => { let current_path_segment = current_item @@ -265,7 +428,7 @@ fn index_re_exports<'a>( .expect("All 'module' items have a 'name' property"); current_path.push(current_path_segment); for item_id in &m.items { - index_re_exports(krate, current_path.clone(), path_index, item_id); + index_local_items(krate, current_path.clone(), path_index, item_id); } } ItemEnum::Import(i) => { @@ -275,14 +438,15 @@ fn index_re_exports<'a>( if let Some(imported_summary) = krate.paths.get(imported_id) { debug_assert!(imported_summary.crate_id != 0); } else { - panic!("The imported id is not listed in the index nor in the path section of rustdoc's JSON output") + // TODO: this is firing for std's JSON docs. File a bug report. + // panic!("The imported id ({}) is not listed in the index nor in the path section of rustdoc's JSON output", imported_id.0) } } Some(imported_item) => { if let ItemEnum::Module(_) = imported_item.inner { current_path.push(&i.name); } - index_re_exports(krate, current_path.clone(), path_index, imported_id); + index_local_items(krate, current_path.clone(), path_index, imported_id); } } } @@ -293,10 +457,11 @@ fn index_re_exports<'a>( .as_deref() .expect("All 'struct' items have a 'name' property"); current_path.push(struct_name); - path_index.insert( - current_path.into_iter().map(|s| s.to_string()).collect(), - current_item_id.to_owned(), - ); + let path = current_path.into_iter().map(|s| s.to_string()).collect(); + path_index + .entry(current_item_id.to_owned()) + .or_default() + .insert(path); } ItemEnum::Enum(_) => { let enum_name = current_item @@ -304,10 +469,11 @@ fn index_re_exports<'a>( .as_deref() .expect("All 'enum' items have a 'name' property"); current_path.push(enum_name); - path_index.insert( - current_path.into_iter().map(|s| s.to_string()).collect(), - current_item_id.to_owned(), - ); + let path = current_path.into_iter().map(|s| s.to_string()).collect(); + path_index + .entry(current_item_id.to_owned()) + .or_default() + .insert(path); } ItemEnum::Function(_) => { let function_name = current_item @@ -315,10 +481,11 @@ fn index_re_exports<'a>( .as_deref() .expect("All 'function' items have a 'name' property"); current_path.push(function_name); - path_index.insert( - current_path.into_iter().map(|s| s.to_string()).collect(), - current_item_id.to_owned(), - ); + let path = current_path.into_iter().map(|s| s.to_string()).collect(); + path_index + .entry(current_item_id.to_owned()) + .or_default() + .insert(path); } _ => {} } diff --git a/libs/pavex/src/web/app.rs b/libs/pavex/src/web/app.rs index 28aeb1edf..b5a1a2089 100644 --- a/libs/pavex/src/web/app.rs +++ b/libs/pavex/src/web/app.rs @@ -1,5 +1,4 @@ -use std::collections::BTreeMap; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::Debug; use std::io::{BufWriter, Write}; use std::path::{Path, PathBuf}; @@ -11,7 +10,7 @@ use guppy::PackageId; use indexmap::{IndexMap, IndexSet}; use miette::{miette, NamedSource, SourceSpan}; use proc_macro2::{Ident, TokenStream}; -use quote::{format_ident, quote}; +use quote::format_ident; use rustdoc_types::{GenericArg, GenericArgs, ItemEnum, Type}; use syn::spanned::Spanned; use syn::FnArg; @@ -20,8 +19,8 @@ use pavex_builder::Lifecycle; use pavex_builder::{AppBlueprint, RawCallableIdentifiers}; use crate::language::{Callable, ParseError, ResolvedType}; -use crate::language::{EncodedResolvedPath, ResolvedPath, UnknownPath}; -use crate::rustdoc::{CannotGetCrateData, Crate, CrateCollection}; +use crate::language::{ResolvedPath, UnknownPath}; +use crate::rustdoc::{CannotGetCrateData, CrateCollection, STD_PACKAGE_ID}; use crate::web::application_state_call_graph::ApplicationStateCallGraph; use crate::web::dependency_graph::CallableDependencyGraph; use crate::web::diagnostic::{ @@ -31,7 +30,6 @@ use crate::web::diagnostic::{ use crate::web::handler_call_graph::HandlerCallGraph; use crate::web::{codegen, diagnostic}; -pub(crate) const STD_PACKAGE_ID: &str = "std"; pub(crate) const GENERATED_APP_PACKAGE_ID: &str = "crate"; pub struct App { @@ -63,8 +61,7 @@ impl GeneratedApp { Self::inject_app_into_workspace_members(&workspace, &directory)?; - let lib_rs = prettyplease::unparse(&syn::parse2(self.lib_rs)?) - .replace("hyper::body::body::Body", "hyper::body::Body"); + let lib_rs = prettyplease::unparse(&syn::parse2(self.lib_rs)?); if let Some(dependencies) = &mut self.cargo_toml.dependencies { for dependency in dependencies.values_mut() { @@ -272,11 +269,9 @@ impl App { // We only report a single registration site in the error report even though // the same callable might have been registered in multiple locations. // We may or may not want to change this in the future. - let type_path = e.0.decode(); - let raw_identifier = resolved_paths2identifiers[&type_path] - .iter() - .next() - .unwrap(); + let type_path = &e.0; + let raw_identifier = + resolved_paths2identifiers[type_path].iter().next().unwrap(); let location = app_blueprint.handler_locations[raw_identifier] .first() .unwrap(); @@ -359,8 +354,8 @@ impl App { } }; - let callable_path = e.callable_path.decode(); - let raw_identifier = resolved_paths2identifiers[&callable_path] + let callable_path = &e.callable_path; + let raw_identifier = resolved_paths2identifiers[callable_path] .iter() .next() .unwrap(); @@ -601,13 +596,14 @@ fn framework_bindings( RawCallableIdentifiers::from_raw_parts(raw_path.into(), "pavex_runtime".into()); let path = ResolvedPath::parse(&identifiers, package_graph).unwrap(); let type_ = path.find_type(krate_collection).unwrap(); - let krate = krate_collection - .get_or_compute_by_id(&path.package_id) + let package_id = krate_collection + .get_defining_package_id_for_item(&path.package_id, &type_.id) + .unwrap(); + let type_base_path = krate_collection + .get_canonical_import_path(&path.package_id, &type_.id) .unwrap(); - let summary = krate.get_summary_by_id(&type_.id).unwrap(); - let type_base_path = summary.path.clone(); ResolvedType { - package_id: path.package_id, + package_id, base_type: type_base_path, generic_arguments: vec![], } @@ -636,7 +632,7 @@ pub(crate) enum BuildError { fn process_constructors( constructor_paths: &IndexSet, krate_collection: &mut CrateCollection, - package_graph: &guppy::graph::PackageGraph, + package_graph: &PackageGraph, ) -> Result< ( BiHashMap, @@ -660,7 +656,7 @@ fn process_constructors( fn process_constructor( constructor_path: &ResolvedPath, krate_collection: &mut CrateCollection, - package_graph: &guppy::graph::PackageGraph, + package_graph: &PackageGraph, ) -> Result { let constructor = process_callable(krate_collection, constructor_path, package_graph)?; if constructor.output_fq_path.base_type == vec!["()"] { @@ -676,7 +672,7 @@ fn process_constructor( fn process_handlers( handler_paths: &IndexSet, krate_collection: &mut CrateCollection, - package_graph: &guppy::graph::PackageGraph, + package_graph: &PackageGraph, ) -> Result<(HashMap, IndexSet), CallableResolutionError> { let mut handlers = IndexSet::with_capacity(handler_paths.len()); let mut handler_resolver = HashMap::new(); @@ -689,9 +685,12 @@ fn process_handlers( } fn process_type( - type_: &rustdoc_types::Type, - krate: &Crate, - package_graph: &guppy::graph::PackageGraph, + type_: &Type, + // The package id where the type we are trying to process has been referenced (e.g. as an + // input/output parameter). + used_by_package_id: &PackageId, + package_graph: &PackageGraph, + krate_collection: &mut CrateCollection, ) -> Result { match type_ { Type::ResolvedPath(rustdoc_types::Path { id, args, .. }) => { @@ -709,8 +708,9 @@ fn process_type( GenericArg::Type(generic_type) => { generics.push(process_type( generic_type, - krate, + used_by_package_id, package_graph, + krate_collection, )?); } GenericArg::Const(_) => { @@ -729,45 +729,12 @@ fn process_type( } } } - let type_summary = krate.get_summary_by_id(id)?; - let type_package_id = if type_summary.crate_id == 0 { - krate.package_id().to_owned() - } else { - let (owning_crate, owning_crate_version) = krate - .get_external_crate_name(type_summary.crate_id) - .unwrap(); - if owning_crate.name == "std" { - PackageId::new(STD_PACKAGE_ID) - } else { - let transitive_dependencies = package_graph - .query_forward([krate.package_id()]) - .unwrap() - .resolve(); - let mut iterator = - transitive_dependencies.links(guppy::graph::DependencyDirection::Forward); - iterator - .find(|link| { - link.to().name() == owning_crate.name - && owning_crate_version - .as_ref() - .map(|v| link.to().version() == v) - .unwrap_or(true) - }) - .ok_or_else(|| { - anyhow!( - "I could not find the package id for the crate where `{}` is defined", - type_summary.path.join("::") - ) - }) - .unwrap() - .to() - .id() - .to_owned() - } - }; + let type_package_id = + krate_collection.get_defining_package_id_for_item(used_by_package_id, id)?; + let base_type = krate_collection.get_canonical_import_path(used_by_package_id, id)?; Ok(ResolvedType { package_id: type_package_id, - base_type: type_summary.path.clone(), + base_type, generic_arguments: generics, }) } @@ -781,17 +748,16 @@ fn process_type( fn process_callable( krate_collection: &mut CrateCollection, callable_path: &ResolvedPath, - package_graph: &guppy::graph::PackageGraph, + package_graph: &PackageGraph, ) -> Result { let type_ = callable_path.find_type(krate_collection)?; - let krate = krate_collection.get_or_compute_by_id(&callable_path.package_id)?; + let used_by_package_id = &callable_path.package_id; let decl = match &type_.inner { ItemEnum::Function(f) => &f.decl, ItemEnum::Method(m) => &m.decl, kind => { - let path = &callable_path.path.0; return Err(UnsupportedCallableKind { - import_path: quote! { #path }.to_string(), + import_path: callable_path.to_string(), // TODO: review how this gets formatted item_kind: format!("{:?}", kind), } @@ -801,7 +767,12 @@ fn process_callable( let mut parameter_paths = Vec::with_capacity(decl.inputs.len()); for (parameter_index, (_, parameter_type)) in decl.inputs.iter().enumerate() { - match process_type(parameter_type, krate, package_graph) { + match process_type( + parameter_type, + used_by_package_id, + package_graph, + krate_collection, + ) { Ok(p) => parameter_paths.push(p), Err(e) => { return Err(ParameterResolutionError { @@ -824,7 +795,12 @@ fn process_callable( }, Some(output_type) => { // TODO: distinguish between output and parameters - match process_type(output_type, krate, package_graph) { + match process_type( + output_type, + used_by_package_id, + package_graph, + krate_collection, + ) { Ok(p) => p, Err(e) => { return Err(OutputTypeResolutionError { @@ -869,9 +845,9 @@ pub(crate) struct UnsupportedCallableKind { #[derive(Debug, thiserror::Error)] #[error("One of the input parameters for `{callable_path}` has a type that I cannot handle.")] pub(crate) struct ParameterResolutionError { - pub callable_path: EncodedResolvedPath, + pub callable_path: ResolvedPath, pub callable_item: rustdoc_types::Item, - pub parameter_type: rustdoc_types::Type, + pub parameter_type: Type, pub parameter_index: usize, #[source] pub source: anyhow::Error, @@ -880,9 +856,9 @@ pub(crate) struct ParameterResolutionError { #[derive(Debug, thiserror::Error)] #[error("I do not know how to handle the type returned by `{callable_path}`.")] pub(crate) struct OutputTypeResolutionError { - pub callable_path: EncodedResolvedPath, + pub callable_path: ResolvedPath, pub callable_item: rustdoc_types::Item, - pub output_type: rustdoc_types::Type, + pub output_type: Type, #[source] pub source: anyhow::Error, } diff --git a/libs/pavex/src/web/application_state_call_graph.rs b/libs/pavex/src/web/application_state_call_graph.rs index 445f29cbc..a65815110 100644 --- a/libs/pavex/src/web/application_state_call_graph.rs +++ b/libs/pavex/src/web/application_state_call_graph.rs @@ -9,11 +9,11 @@ use petgraph::visit::{DfsPostOrder, Reversed}; use petgraph::Direction; use proc_macro2::{Ident, TokenStream}; use quote::{quote, ToTokens}; -use syn::{parse_quote, ExprStruct, ItemFn}; +use syn::{ExprStruct, ItemFn}; use pavex_builder::Lifecycle; -use crate::language::{CallPath, Callable, ResolvedPath, ResolvedType}; +use crate::language::{Callable, ResolvedPath, ResolvedPathSegment, ResolvedType}; use crate::web::app::GENERATED_APP_PACKAGE_ID; use crate::web::codegen_utils::{codegen_call_block, Fragment, VariableNameGenerator}; use crate::web::dependency_graph::{CallableDependencyGraph, DependencyGraphNode}; @@ -43,7 +43,16 @@ impl ApplicationStateCallGraph { generic_arguments: vec![], }, callable_fq_path: ResolvedPath { - path: CallPath(syn::parse_str("crate::ApplicationState").unwrap()), + segments: vec![ + ResolvedPathSegment { + ident: "crate".into(), + generic_arguments: vec![], + }, + ResolvedPathSegment { + ident: "ApplicationState".into(), + generic_arguments: vec![], + }, + ], package_id: PackageId::new(GENERATED_APP_PACKAGE_ID), }, inputs: runtime_singleton_bindings.right_values().cloned().collect(), @@ -150,6 +159,7 @@ impl ApplicationStateCallGraph { node_index, &mut blocks, &mut variable_generator, + package_id2name, )?; let block = quote! { let #parameter_name = #block; @@ -167,6 +177,7 @@ impl ApplicationStateCallGraph { node_index, &mut blocks, &mut variable_generator, + package_id2name, )?; blocks.insert(node_index, block); } @@ -181,6 +192,7 @@ impl ApplicationStateCallGraph { node_index, &mut blocks, &mut variable_generator, + package_id2name, )?; blocks.insert(node_index, block); } @@ -211,12 +223,13 @@ impl ApplicationStateCallGraph { } Fragment::Statement(s) => s.to_token_stream(), }; - parse_quote! { + syn::parse2(quote! { pub fn build_application_state(#(#inputs),*) -> #output_type { #(#singleton_constructors)* #b } - } + }) + .unwrap() } }; Ok(code) @@ -252,14 +265,15 @@ impl ApplicationStateCallGraph { pub(crate) fn codegen_struct_init_block( call_graph: &StableDiGraph, callable: &Callable, - struct_fields: &BiHashMap, + struct_fields: &BiHashMap, node_index: NodeIndex, blocks: &mut HashMap, variable_generator: &mut VariableNameGenerator, + package_id2name: &BiHashMap<&PackageId, String>, ) -> Result { let dependencies = call_graph.neighbors_directed(node_index, Direction::Incoming); let mut block = quote! {}; - let mut dependency_bindings: BiHashMap = BiHashMap::new(); + let mut dependency_bindings: BiHashMap = BiHashMap::new(); for dependency_index in dependencies { let fragment = &blocks[&dependency_index]; let dependency_type = match &call_graph[dependency_index] { @@ -290,13 +304,15 @@ pub(crate) fn codegen_struct_init_block( &callable.callable_fq_path, struct_fields, &dependency_bindings, + package_id2name, )?; - let block: syn::Block = parse_quote! { + let block: syn::Block = syn::parse2(quote! { { #block #constructor_invocation } - }; + }) + .unwrap(); if block.stmts.len() == 1 { Ok(Fragment::Statement(Box::new( block.stmts.first().unwrap().to_owned(), @@ -308,21 +324,23 @@ pub(crate) fn codegen_struct_init_block( pub(crate) fn codegen_struct_init( struct_path: &ResolvedPath, - struct_fields: &BiHashMap, - variable_bindings: &BiHashMap, + struct_fields: &BiHashMap, + variable_bindings: &BiHashMap, + id2name: &BiHashMap<&PackageId, String>, ) -> Result { - let struct_path = struct_path.as_ref(); + let struct_path: syn::ExprPath = syn::parse_str(&struct_path.render_path(id2name)).unwrap(); let fields = struct_fields.iter().map(|(field_name, field_type)| { let binding = variable_bindings.get_by_left(field_type).unwrap(); quote! { #field_name: #binding } }); - Ok(parse_quote! { + Ok(syn::parse2(quote! { #struct_path { #(#fields),* } }) + .unwrap()) } /// Return the set of types that must be provided as input to build the application state. diff --git a/libs/pavex/src/web/codegen.rs b/libs/pavex/src/web/codegen.rs index 242040c6d..84776ff7c 100644 --- a/libs/pavex/src/web/codegen.rs +++ b/libs/pavex/src/web/codegen.rs @@ -8,11 +8,12 @@ use guppy::{PackageId, Version}; use indexmap::{IndexMap, IndexSet}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; -use syn::{parse_quote, ItemFn, ItemStruct}; +use syn::{ItemFn, ItemStruct}; use crate::language::ResolvedPath; use crate::language::{Callable, ResolvedType}; -use crate::web::app::{GENERATED_APP_PACKAGE_ID, STD_PACKAGE_ID}; +use crate::rustdoc::STD_PACKAGE_ID; +use crate::web::app::GENERATED_APP_PACKAGE_ID; use crate::web::application_state_call_graph::ApplicationStateCallGraph; use crate::web::dependency_graph::DependencyGraphNode; use crate::web::handler_call_graph::{codegen, HandlerCallGraph}; @@ -82,7 +83,7 @@ pub(crate) fn codegen_app( } fn server_startup() -> ItemFn { - parse_quote! { + syn::parse2(quote! { pub async fn run( server_builder: pavex_runtime::hyper::server::Builder, application_state: ApplicationState @@ -102,7 +103,7 @@ fn server_startup() -> ItemFn { }); server_builder.serve(make_service).await.map_err(Into::into) } - } + }).unwrap() } fn define_application_state( @@ -113,20 +114,22 @@ fn define_application_state( let field_type = type_.syn_type(package_id2name); quote! { #field_name: #field_type } }); - parse_quote! { + syn::parse2(quote! { pub struct ApplicationState { #(#singleton_fields),* } - } + }) + .unwrap() } fn define_server_state() -> ItemStruct { - parse_quote! { + syn::parse2(quote! { struct ServerState { router: pavex_runtime::routing::Router, application_state: ApplicationState } - } + }) + .unwrap() } fn get_application_state_init( @@ -148,12 +151,12 @@ fn get_router_init(route_id2path: &BiHashMap) -> ItemFn { router.insert(#path, #route_id)?; }; } - parse_quote! { + syn::parse2(quote! { fn build_router() -> Result, pavex_runtime::routing::InsertError> { #router_init Ok(router) } - } + }).unwrap() } fn get_request_dispatcher( @@ -183,7 +186,7 @@ fn get_request_dispatcher( } } - parse_quote! { + syn::parse2(quote! { fn route_request(request: pavex_runtime::http::Request, server_state: std::sync::Arc) -> pavex_runtime::http::Response { let route_id = server_state.router.at(request.uri().path()).expect("Failed to match incoming request path"); match route_id.value { @@ -191,7 +194,7 @@ fn get_request_dispatcher( _ => panic!("This is a bug, no route registered for a route id"), } } - } + }).unwrap() } pub(crate) fn codegen_manifest<'a>( diff --git a/libs/pavex/src/web/codegen_utils.rs b/libs/pavex/src/web/codegen_utils.rs index e22eb04d8..4b278f39a 100644 --- a/libs/pavex/src/web/codegen_utils.rs +++ b/libs/pavex/src/web/codegen_utils.rs @@ -1,12 +1,14 @@ -use crate::language::{Callable, ResolvedType}; -use crate::web::dependency_graph::DependencyGraphNode; +use std::collections::HashMap; + use bimap::BiHashMap; +use guppy::PackageId; use petgraph::stable_graph::{NodeIndex, StableDiGraph}; use petgraph::Direction; use proc_macro2::TokenStream; use quote::{format_ident, quote, ToTokens}; -use std::collections::HashMap; -use syn::parse_quote; + +use crate::language::{Callable, ResolvedType}; +use crate::web::dependency_graph::DependencyGraphNode; pub(crate) enum Fragment { VariableReference(syn::Ident), @@ -44,6 +46,7 @@ pub(crate) fn codegen_call_block( node_index: NodeIndex, blocks: &mut HashMap, variable_generator: &mut VariableNameGenerator, + package_id2name: &BiHashMap<&PackageId, String>, ) -> Result { let dependencies = call_graph.neighbors_directed(node_index, Direction::Incoming); let mut block = quote! {}; @@ -74,13 +77,14 @@ pub(crate) fn codegen_call_block( blocks.remove(&dependency_index); } } - let constructor_invocation = codegen_call(callable, &dependency_bindings)?; - let block: syn::Block = parse_quote! { + let constructor_invocation = codegen_call(callable, &dependency_bindings, package_id2name)?; + let block: syn::Block = syn::parse2(quote! { { #block #constructor_invocation } - }; + }) + .unwrap(); if block.stmts.len() == 1 { Ok(Fragment::Statement(Box::new( block.stmts.first().unwrap().to_owned(), @@ -93,13 +97,16 @@ pub(crate) fn codegen_call_block( pub(crate) fn codegen_call( callable: &Callable, variable_bindings: &BiHashMap, + package_id2name: &BiHashMap<&PackageId, String>, ) -> Result { - let callable_path = callable.callable_fq_path.as_ref(); + let callable_path: syn::ExprPath = + syn::parse_str(&callable.callable_fq_path.render_path(package_id2name)).unwrap(); let parameters = callable .inputs .iter() .map(|i| variable_bindings.get_by_left(i).unwrap()); - Ok(parse_quote! { + Ok(syn::parse2(quote! { #callable_path(#(#parameters),*) }) + .unwrap()) } diff --git a/libs/pavex/src/web/handler_call_graph.rs b/libs/pavex/src/web/handler_call_graph.rs index 47ea4c35a..3570afc12 100644 --- a/libs/pavex/src/web/handler_call_graph.rs +++ b/libs/pavex/src/web/handler_call_graph.rs @@ -9,7 +9,7 @@ use petgraph::visit::Reversed; use petgraph::Direction; use proc_macro2::TokenStream; use quote::{quote, ToTokens}; -use syn::{parse_quote, ItemFn}; +use syn::ItemFn; use pavex_builder::Lifecycle; @@ -191,7 +191,7 @@ pub(crate) fn codegen<'a>( let mut parameter_bindings = HashMap::new(); let mut variable_generator = VariableNameGenerator::default(); - let mut scoped_constructors = HashMap::::new(); + let mut scoped_constructors = IndexMap::::new(); let mut blocks = HashMap::::new(); while let Some(node_index) = dfs.next(Reversed(call_graph)) { @@ -216,6 +216,7 @@ pub(crate) fn codegen<'a>( node_index, &mut blocks, &mut variable_generator, + package_id2name, )?; blocks.insert(node_index, block); } @@ -232,6 +233,7 @@ pub(crate) fn codegen<'a>( node_index, &mut blocks, &mut variable_generator, + package_id2name, )?; let block = quote! { let #parameter_name = #block; @@ -250,6 +252,7 @@ pub(crate) fn codegen<'a>( node_index, &mut blocks, &mut variable_generator, + package_id2name, )?; blocks.insert(node_index, block); } @@ -280,12 +283,13 @@ pub(crate) fn codegen<'a>( } Fragment::Statement(s) => s.to_token_stream(), }; - parse_quote! { + syn::parse2(quote! { pub fn handler(#(#inputs),*) -> #output_type { #(#scoped_constructors)* #b } - } + }) + .unwrap() } }; Ok(code) diff --git a/libs/pavex_cli/tests/ui_tests/app_builder/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/app_builder/expectations/app.rs index f90bc0489..9a0bdd1cb 100644 --- a/libs/pavex_cli/tests/ui_tests/app_builder/expectations/app.rs +++ b/libs/pavex_cli/tests/ui_tests/app_builder/expectations/app.rs @@ -63,8 +63,8 @@ fn route_request( } pub fn route_handler_0( v0: app::HttpClient, - v1: http::request::Request, -) -> http::response::Response { + v1: http::Request, +) -> http::Response { let v2 = app::extract_path(v1); let v3 = app::logger(); app::stream_file(v2, v3, v0) diff --git a/libs/pavex_cli/tests/ui_tests/app_builder/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/app_builder/expectations/diagnostics.dot index 54a137978..845429756 100644 --- a/libs/pavex_cli/tests/ui_tests/app_builder/expectations/diagnostics.dot +++ b/libs/pavex_cli/tests/ui_tests/app_builder/expectations/diagnostics.dot @@ -1,9 +1,9 @@ digraph "/home" { - 0 [ label = "app::stream_file(std::path::PathBuf, app::Logger, app::HttpClient) -> http::response::Response"] + 0 [ label = "app::stream_file(std::path::PathBuf, app::Logger, app::HttpClient) -> http::Response"] 1 [ label = "app::HttpClient"] 2 [ label = "app::Logger"] 3 [ label = "std::path::PathBuf"] - 4 [ label = "http::request::Request"] + 4 [ label = "http::Request"] 1 -> 0 [ ] 2 -> 0 [ ] 3 -> 0 [ ] diff --git a/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/app.rs index 154d2ff83..038ce160a 100644 --- a/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/app.rs +++ b/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/app.rs @@ -58,8 +58,8 @@ fn route_request( _ => panic!("This is a bug, no route registered for a route id"), } } -pub fn route_handler_0() -> http_1::response::Response { - let v1 = app::header1(); +pub fn route_handler_0() -> http_1::Response { let v0 = app::header2(); + let v1 = app::header1(); app::stream_file(v1, v0) } \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/diagnostics.dot index b82bad20e..9b399e991 100644 --- a/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/diagnostics.dot +++ b/libs/pavex_cli/tests/ui_tests/multiple_versions_for_the_same_crate_are_supported/expectations/diagnostics.dot @@ -1,7 +1,7 @@ digraph "/home" { - 0 [ label = "app::stream_file(http_0::header::name::HeaderName, http_1::header::name::HeaderName) -> http_1::response::Response"] - 1 [ label = "http_1::header::name::HeaderName"] - 2 [ label = "http_0::header::name::HeaderName"] + 0 [ label = "app::stream_file(http_0::header::HeaderName, http_1::header::HeaderName) -> http_1::Response"] + 1 [ label = "http_1::header::HeaderName"] + 2 [ label = "http_0::header::HeaderName"] 1 -> 0 [ ] 2 -> 0 [ ] } diff --git a/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/app.rs index 07fc1b7aa..abcbefd8d 100644 --- a/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/app.rs +++ b/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/app.rs @@ -61,8 +61,6 @@ fn route_request( _ => panic!("This is a bug, no route registered for a route id"), } } -pub fn route_handler_0( - v0: app::Streamer, -) -> http::response::Response { +pub fn route_handler_0(v0: app::Streamer) -> http::Response { app::stream_file(v0) } \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/diagnostics.dot index 8ce509f62..d9604820c 100644 --- a/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/diagnostics.dot +++ b/libs/pavex_cli/tests/ui_tests/pattern_bindings_in_input_parameters_are_supported/expectations/diagnostics.dot @@ -1,5 +1,5 @@ digraph "/home" { - 0 [ label = "app::stream_file(app::Streamer) -> http::response::Response"] + 0 [ label = "app::stream_file(app::Streamer) -> http::Response"] 1 [ label = "app::Streamer"] 1 -> 0 [ ] } diff --git a/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/app.rs index 11f648570..bcc3ce7c1 100644 --- a/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/app.rs +++ b/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/app.rs @@ -58,6 +58,6 @@ fn route_request( _ => panic!("This is a bug, no route registered for a route id"), } } -pub fn route_handler_0() -> http::response::Response { +pub fn route_handler_0() -> http::Response { app::Streamer::stream_file() } \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/diagnostics.dot index 79dca5e90..937b65650 100644 --- a/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/diagnostics.dot +++ b/libs/pavex_cli/tests/ui_tests/static_methods_are_supported/expectations/diagnostics.dot @@ -1,5 +1,5 @@ digraph "/home" { - 0 [ label = "app::Streamer::stream_file() -> http::response::Response"] + 0 [ label = "app::Streamer::stream_file() -> http::Response"] } digraph app_state { 0 [ label = "crate::ApplicationState() -> crate::ApplicationState"] diff --git a/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/app.rs index 8bb419681..d01e2c41f 100644 --- a/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/app.rs +++ b/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/app.rs @@ -58,7 +58,7 @@ fn route_request( _ => panic!("This is a bug, no route registered for a route id"), } } -pub fn route_handler_0() -> http::response::Response { +pub fn route_handler_0() -> http::Response { let v0 = dep::new_logger(); app::Streamer::stream_file(v0) } \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/diagnostics.dot index 28c97e0dc..fad4809a0 100644 --- a/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/diagnostics.dot +++ b/libs/pavex_cli/tests/ui_tests/the_latest_registered_constructor_takes_precedence/expectations/diagnostics.dot @@ -1,5 +1,5 @@ digraph "/home" { - 0 [ label = "app::Streamer::stream_file(dep::Logger) -> http::response::Response"] + 0 [ label = "app::Streamer::stream_file(dep::Logger) -> http::Response"] 1 [ label = "dep::Logger"] 1 -> 0 [ ] } diff --git a/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/app.rs b/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/app.rs index 63b15f7bf..730dd2c7e 100644 --- a/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/app.rs +++ b/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/app.rs @@ -58,7 +58,7 @@ fn route_request( _ => panic!("This is a bug, no route registered for a route id"), } } -pub fn route_handler_0() -> http::response::Response { +pub fn route_handler_0() -> http::Response { let v0 = app::new_logger(); app::Streamer::stream_file(v0) } \ No newline at end of file diff --git a/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/diagnostics.dot b/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/diagnostics.dot index c073d20f9..76a94095e 100644 --- a/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/diagnostics.dot +++ b/libs/pavex_cli/tests/ui_tests/the_same_constructor_can_be_registered_multiple_times_using_the_same_lifecycle/expectations/diagnostics.dot @@ -1,5 +1,5 @@ digraph "/home" { - 0 [ label = "app::Streamer::stream_file(app::Logger) -> http::response::Response"] + 0 [ label = "app::Streamer::stream_file(app::Logger) -> http::Response"] 1 [ label = "app::Logger"] 1 -> 0 [ ] } diff --git a/libs/pavex_test_runner/Cargo.toml b/libs/pavex_test_runner/Cargo.toml index e1a315efb..c16fc543e 100644 --- a/libs/pavex_test_runner/Cargo.toml +++ b/libs/pavex_test_runner/Cargo.toml @@ -3,6 +3,10 @@ name = "pavex_test_runner" version = "0.1.0" edition = "2021" +[[bin]] +path = "src/main.rs" +name = "snaps" + [dependencies] anyhow = "1.0.59" console = "0.15.1" @@ -12,3 +16,4 @@ serde = { version = "1.0.141", features = ["derive"] } similar = { version = "2.2.0", features = ["inline"] } textwrap = "0.15.0" toml = { version = "0.5.9", features = ["preserve_order"] } +miette = "5" \ No newline at end of file diff --git a/libs/pavex_test_runner/src/lib.rs b/libs/pavex_test_runner/src/lib.rs index 22be77033..268b74cce 100644 --- a/libs/pavex_test_runner/src/lib.rs +++ b/libs/pavex_test_runner/src/lib.rs @@ -1,12 +1,17 @@ -use crate::snapshot::SnapshotTest; +use std::collections::HashMap; +use std::path::PathBuf; +use std::process::Output; + use anyhow::Context; use console::style; use libtest_mimic::Conclusion; use libtest_mimic::Outcome; -use std::collections::HashMap; -use std::path::PathBuf; use toml::toml; +pub use snapshot::print_changeset; + +use crate::snapshot::SnapshotTest; + mod snapshot; /// Create a test case for each folder in `definition_directory`. @@ -249,13 +254,24 @@ fn run_test(test: &libtest_mimic::Test) -> Outcome { match test.data.load_configuration() { // Ensure that the test description is always injected on top of the failure message Ok(c) => match _run_test(&c, test) { - Ok(Outcome::Failed { msg }) => Outcome::Failed { - msg: msg.map(|msg| enrich_failure_message(&c, msg)), + Ok(TestOutcome { + outcome: Outcome::Failed { msg }, + source_generation_output, + }) => Outcome::Failed { + msg: msg.map(|msg| { + enrich_failure_message( + &c, + format!( + "{msg}\n\n--- STDOUT:\n{}\n--- STDERR:\n{}", + source_generation_output.stdout, source_generation_output.stderr + ), + ) + }), }, Err(e) => Outcome::Failed { msg: Some(enrich_failure_message(&c, unexpected_failure_message(&e))), }, - Ok(o) => o, + Ok(o) => o.outcome, }, // We do not have the test description if we fail to load the test configuration, so... Err(e) => Outcome::Failed { @@ -267,7 +283,7 @@ fn run_test(test: &libtest_mimic::Test) -> Outcome { fn _run_test( test_config: &TestConfig, test: &libtest_mimic::Test, -) -> Result { +) -> Result { test.data .seed_test_filesystem(test_config) .context("Failed to seed the filesystem for the test runtime folder")?; @@ -280,44 +296,44 @@ fn _run_test( .current_dir(&test.data.runtime_directory) .output() .unwrap(); + let source_generation_output: CommandOutput = (&output).try_into()?; let expectations_directory = test.data.definition_directory.join("expectations"); if !output.status.success() { - let stdout = std::str::from_utf8(&output.stdout) - .context("The application printed invalid UTF8 data to stdout")?; - let stderr = std::str::from_utf8(&output.stderr) - .context("The application printed invalid UTF8 data to stderr")?; return match test_config.expectations.codegen { - ExpectedOutcome::Pass => { - Ok(Outcome::Failed { - msg: Some(format!( - "We failed to generate the application code.\n\n--- STDOUT:\n{}\n--- STDERR:\n{}", - stdout, stderr - )), - }) - } + ExpectedOutcome::Pass => Ok(TestOutcome { + outcome: Outcome::Failed { + msg: Some(format!("We failed to generate the application code.",)), + }, + source_generation_output, + }), ExpectedOutcome::Fail => { let stderr_snapshot = SnapshotTest::new(expectations_directory.join("stderr.txt")); - if stderr_snapshot.verify(stderr).is_err() { - return Ok(Outcome::Failed { - msg: Some( - "The failure message returned by code generation does not match what we expected".into()), + if stderr_snapshot + .verify(&source_generation_output.stderr) + .is_err() + { + return Ok(TestOutcome { + outcome: Outcome::Failed { + msg: Some( + "The failure message returned by code generation does not match what we expected".into()) + }, + source_generation_output, }); } - Ok(Outcome::Passed) + Ok(TestOutcome { + outcome: Outcome::Passed, + source_generation_output, + }) } }; } else if ExpectedOutcome::Fail == test_config.expectations.codegen { - let stdout = std::str::from_utf8(&output.stdout) - .context("The application printed invalid UTF8 data to stdout")?; - let stderr = std::str::from_utf8(&output.stderr) - .context("The application printed invalid UTF8 data to stderr")?; - return Ok(Outcome::Failed { - msg: Some(format!( - "We expected code generation to fail, but it succeeded!\n\n--- STDOUT:\n{}\n--- STDERR:\n{}", - stdout, stderr - )), + return Ok(TestOutcome { + outcome: Outcome::Failed { + msg: Some("We expected code generation to fail, but it succeeded!".into()), + }, + source_generation_output, }); }; @@ -325,10 +341,14 @@ fn _run_test( let actual_diagnostics = fs_err::read_to_string(test.data.runtime_directory.join("diagnostics.dot"))?; if diagnostics_snapshot.verify(&actual_diagnostics).is_err() { - return Ok(Outcome::Failed { - msg: Some( - "Diagnostics for the generated application do not match what we expected".into(), - ), + return Ok(TestOutcome { + outcome: Outcome::Failed { + msg: Some( + "Diagnostics for the generated application do not match what we expected" + .into(), + ), + }, + source_generation_output, }); } @@ -342,12 +362,44 @@ fn _run_test( ) .unwrap(); if app_code_snapshot.verify(&actual_app_code).is_err() { - return Ok(Outcome::Failed { - msg: Some("The generated application code does not match what we expected".into()), + return Ok(TestOutcome { + outcome: Outcome::Failed { + msg: Some("The generated application code does not match what we expected".into()), + }, + source_generation_output, }); } - Ok(Outcome::Passed) + Ok(TestOutcome { + outcome: Outcome::Passed, + source_generation_output, + }) +} + +struct TestOutcome { + outcome: Outcome, + source_generation_output: CommandOutput, +} + +/// A refined `std::process::Output` that assumes that both stderr and stdout are valid UTF8. +struct CommandOutput { + stdout: String, + stderr: String, +} + +impl TryFrom<&Output> for CommandOutput { + type Error = anyhow::Error; + + fn try_from(o: &Output) -> Result { + let stdout = std::str::from_utf8(&o.stdout) + .context("The application printed invalid UTF8 data to stdout")?; + let stderr = std::str::from_utf8(&o.stderr) + .context("The application printed invalid UTF8 data to stderr")?; + Ok(Self { + stdout: stdout.to_string(), + stderr: stderr.to_string(), + }) + } } fn unexpected_failure_message(e: &anyhow::Error) -> String { diff --git a/libs/pavex_test_runner/src/main.rs b/libs/pavex_test_runner/src/main.rs new file mode 100644 index 000000000..7213b1f67 --- /dev/null +++ b/libs/pavex_test_runner/src/main.rs @@ -0,0 +1,101 @@ +use std::path::PathBuf; + +use console::{style, Key}; + +use pavex_test_runner::print_changeset; + +fn main() -> Result<(), Box> { + miette::set_hook(Box::new(move |_| { + let config = miette::MietteHandlerOpts::new(); + Box::new(config.build()) + })) + .unwrap(); + let test_folder: PathBuf = "libs/pavex_cli/tests/ui_tests".into(); + let terminal = console::Term::stdout(); + for ui_test_dir in fs_err::read_dir(&test_folder)? { + let ui_test_dir = ui_test_dir?; + assert!(ui_test_dir.metadata()?.is_dir()); + let test_name = ui_test_dir.file_name().to_string_lossy().to_string(); + let expectations_dir = ui_test_dir.path().join("expectations"); + for file in fs_err::read_dir(&expectations_dir)? { + let file = file?; + let file_name = file.file_name().to_string_lossy().to_string(); + if let Some(expected_filename) = file_name.strip_suffix(".snap") { + let actual_snapshot = fs_err::read_to_string(file.path())?; + let expected_path = expectations_dir.join(expected_filename); + let expected_snapshot = fs_err::read_to_string(&expected_path)?; + + match review_snapshot( + &terminal, + &test_name, + expected_filename, + &expected_snapshot, + &actual_snapshot, + )? { + Decision::Accept => { + fs_err::rename(file.path(), &expected_path)?; + } + Decision::Reject => { + fs_err::remove_file(file.path())?; + } + Decision::Skip => continue, + } + } + } + } + + Ok(()) +} + +fn review_snapshot( + terminal: &console::Term, + test_name: &str, + snapshot_name: &str, + expected_snapshot: &str, + actual_snapshot: &str, +) -> std::io::Result { + terminal.clear_screen()?; + println!( + "{}{}\n{}{}", + style("Test name: ").bold(), + style(format!("{}", &test_name)).yellow().bold(), + style("Snapshot name: ").bold(), + style(format!("{}", &snapshot_name)).green().bold(), + ); + print_changeset(&expected_snapshot, &actual_snapshot); + + prompt(&terminal) +} + +fn prompt(terminal: &console::Term) -> std::io::Result { + println!(); + println!( + " {} accept {}", + style("a").green().bold(), + style("keep the new snapshot").dim() + ); + println!( + " {} reject {}", + style("r").red().bold(), + style("keep the old snapshot").dim() + ); + println!( + " {} skip {}", + style("s").yellow().bold(), + style("keep both for now").dim() + ); + loop { + match terminal.read_key()? { + Key::Char('a') | Key::Enter => return Ok(Decision::Accept), + Key::Char('r') | Key::Escape => return Ok(Decision::Reject), + Key::Char('s') | Key::Char(' ') => return Ok(Decision::Skip), + _ => {} + } + } +} + +enum Decision { + Accept, + Reject, + Skip, +} diff --git a/libs/pavex_test_runner/src/snapshot.rs b/libs/pavex_test_runner/src/snapshot.rs index a8adcbcbd..2b27b3272 100644 --- a/libs/pavex_test_runner/src/snapshot.rs +++ b/libs/pavex_test_runner/src/snapshot.rs @@ -1,9 +1,10 @@ -use console::style; -use similar::{Algorithm, ChangeTag, TextDiff}; use std::io::ErrorKind; use std::path::PathBuf; use std::time::Duration; +use console::style; +use similar::{Algorithm, ChangeTag, TextDiff}; + fn term_width() -> usize { console::Term::stdout().size().1 as usize } @@ -46,7 +47,7 @@ impl SnapshotTest { } } -fn print_changeset(old: &str, new: &str) { +pub fn print_changeset(old: &str, new: &str) { let width = term_width(); let diff = TextDiff::configure() .algorithm(Algorithm::Patience)