Skip to content
This repository has been archived by the owner on Dec 11, 2024. It is now read-only.

Commit

Permalink
Fix clippy
Browse files Browse the repository at this point in the history
  • Loading branch information
afsalthaj committed May 29, 2024
1 parent d34d439 commit c102494
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 139 deletions.
1 change: 0 additions & 1 deletion wasm-rpc-stubgen/src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use cargo_toml::{
};
use serde::{Deserialize, Serialize};
use toml::Value;
use wit_parser::UnresolvedPackage;

use golem_wasm_rpc::WASM_RPC_VERSION;

Expand Down
23 changes: 12 additions & 11 deletions wasm-rpc-stubgen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ use crate::cargo::generate_cargo_toml;
use crate::compilation::compile;
use crate::rust::generate_stub_source;
use crate::stub::StubDefinition;
use crate::wit::{copy_wit_files, generate_stub_wit, get_stub_wit, verify_action, WitAction};
use crate::wit::{
copy_wit_files, generate_stub_wit, get_stub_wit, verify_action, StubTypeGen, WitAction,
};
use anyhow::{anyhow, Context};
use clap::Parser;
use fs_extra::dir::CopyOptions;
Expand All @@ -32,10 +34,10 @@ use golem_wasm_ast::component::Component;
use golem_wasm_ast::IgnoreAllButMetadata;
use heck::ToSnakeCase;
use std::fs;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use tempdir::TempDir;
use wasm_compose::config::Dependency;
use wit_parser::{Resolve, UnresolvedPackage};
use wit_parser::UnresolvedPackage;

#[derive(Parser, Debug)]
#[command(name = "wasm-rpc-stubgen", version)]
Expand Down Expand Up @@ -239,7 +241,7 @@ pub async fn build(args: BuildArgs) -> anyhow::Result<()> {
Ok(())
}

fn find_if_same_package(dep_dir: &PathBuf, target_wit: &UnresolvedPackage) -> anyhow::Result<bool> {
fn find_if_same_package(dep_dir: &Path, target_wit: &UnresolvedPackage) -> anyhow::Result<bool> {
let dep_package_name = UnresolvedPackage::parse_dir(dep_dir)?.name;
let dest_package = target_wit.name.clone();

Expand All @@ -248,8 +250,7 @@ fn find_if_same_package(dep_dir: &PathBuf, target_wit: &UnresolvedPackage) -> an
} else {
println!(
"Skipping the copy of cyclic dependencies {} to the the same as {}",
dep_package_name,
dest_package
dep_package_name, dest_package
);
Ok(false)
}
Expand All @@ -265,21 +266,21 @@ pub fn add_stub_dependency(args: AddStubDependencyArgs) -> anyhow::Result<()> {
// We filter the dependencies of stub that's already existing in dest_wit_root
let filtered_source_deps = source_deps
.into_iter()
.filter(|dep| find_if_same_package(&dep, &destination_wit_root).unwrap())
.filter(|dep| find_if_same_package(dep, &destination_wit_root).unwrap())
.collect::<Vec<_>>();

let main_wit = args.stub_wit_root.join("_stub.wit");

let stub_dependency = StubDefinition::new(
&args.dest_wit_root,
&args.stub_wit_root.parent().unwrap(),
args.stub_wit_root.parent().unwrap(),
&None, // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency
&"0.0.1".to_string(), // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency
"0.0.1", // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency
&None, // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency
)?;

let new_stub =
get_stub_wit(&stub_dependency, false).context("Failed to regenerate inlined stub")?;
let new_stub = get_stub_wit(&stub_dependency, StubTypeGen::InlineRootTypes)
.context("Failed to regenerate inlined stub")?;

let main_wit_package_name = wit::get_package_name(&main_wit)?;

Expand Down
2 changes: 1 addition & 1 deletion wasm-rpc-stubgen/src/stub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ fn collect_stub_interfaces(resolve: &Resolve, world: &World) -> anyhow::Result<V
.exports
.iter()
.filter_map(|(name, item)| match item {
WorldItem::Type(t) => Some((name.clone().unwrap_name(), t)),
WorldItem::Type(t) => Some((name.clone().unwrap_name(), t)),
_ => None,
})
.collect::<Vec<_>>();
Expand Down
204 changes: 78 additions & 126 deletions wasm-rpc-stubgen/src/wit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@

use crate::stub::{FunctionParamStub, FunctionResultStub, InterfaceStubTypeDef, StubDefinition};
use anyhow::{anyhow, bail, Context};
use regex::Regex;
use std::ffi::OsStr;
use std::fmt::{Display, Formatter, Write};
use std::fs;
use std::path::{Path, PathBuf};
use wit_parser::{Field, Handle, PackageName, Resolve, Type, TypeDefKind, UnresolvedPackage};
use regex::Regex;
use crate::stub;

pub fn generate_stub_wit(def: &StubDefinition) -> anyhow::Result<()> {
let out = get_stub_wit(def, true)?;
let out = get_stub_wit(def, StubTypeGen::ImportRootTypes)?;
println!(
"Generating stub WIT to {}",
def.target_wit_path().to_string_lossy()
Expand All @@ -33,7 +32,15 @@ pub fn generate_stub_wit(def: &StubDefinition) -> anyhow::Result<()> {
Ok(())
}

pub fn get_stub_wit(def: &StubDefinition, bool: bool) -> anyhow::Result<String> {
pub enum StubTypeGen {
InlineRootTypes,
ImportRootTypes,
}

pub fn get_stub_wit(
def: &StubDefinition,
type_gen_strategy: StubTypeGen,
) -> anyhow::Result<String> {
let world = def.resolve.worlds.get(def.world_id).unwrap();

let mut out = String::new();
Expand All @@ -50,54 +57,57 @@ pub fn get_stub_wit(def: &StubDefinition, bool: bool) -> anyhow::Result<String>

writeln!(out, " use golem:rpc/[email protected].{{uri}};")?;

if bool {
for import in all_imports {
writeln!(out, " use {}.{{{}}};", import.path, import.name)?;
match type_gen_strategy {
StubTypeGen::ImportRootTypes => {
for import in all_imports {
writeln!(out, " use {}.{{{}}};", import.path, import.name)?;
}
writeln!(out)?;
}
writeln!(out)?;
} else {
let mut inline_types: Vec<InterfaceStubTypeDef> = vec![];
StubTypeGen::InlineRootTypes => {
let mut inline_types: Vec<InterfaceStubTypeDef> = vec![];

for import in all_imports {
match &import.package_name {
Some(package) if package == &def.root_package_name => {
inline_types.push(import.clone());
for import in all_imports {
match &import.package_name {
Some(package) if package == &def.root_package_name => {
inline_types.push(import.clone());
}
_ => writeln!(out, " use {}.{{{}}};", import.path, import.name)?,
}
_ => writeln!(out, " use {}.{{{}}};", import.path, import.name)?,
}
}

writeln!(out)?;
writeln!(out)?;

for typ in inline_types {
let typ_kind = typ.clone().type_def.kind;
let kind_str = typ_kind.as_str();
let name = typ.clone().name;

write!(out, " {}", kind_str)?;
write!(out, " {}", name)?;

for typ in inline_types {
let typ_kind = typ.clone().type_def.kind;
let kind_str = typ_kind.as_str();
let name = typ.clone().name;

write!(out, " {}", kind_str)?;
write!(out, " {}", name)?;

match typ_kind {
TypeDefKind::Record(record) => {
write!(out, " {{")?;
writeln!(out)?;
write_field_list(&mut out, record.fields, def)?;
writeln!(out)?;
writeln!(out, " }}")?;
match typ_kind {
TypeDefKind::Record(record) => {
write!(out, " {{")?;
writeln!(out)?;
write_field_list(&mut out, record.fields, def)?;
writeln!(out)?;
writeln!(out, " }}")?;
}
TypeDefKind::Resource => {}
TypeDefKind::Handle(_) => {}
TypeDefKind::Flags(_) => {}
TypeDefKind::Tuple(_) => {}
TypeDefKind::Variant(_) => {}
TypeDefKind::Enum(_) => {}
TypeDefKind::Option(_) => {}
TypeDefKind::Result(_) => {}
TypeDefKind::List(_) => {}
TypeDefKind::Future(_) => {}
TypeDefKind::Stream(_) => {}
TypeDefKind::Type(_) => {}
TypeDefKind::Unknown => {}
}
TypeDefKind::Resource => {}
TypeDefKind::Handle(_) => {}
TypeDefKind::Flags(_) => {}
TypeDefKind::Tuple(_) => {}
TypeDefKind::Variant(_) => {}
TypeDefKind::Enum(_) => {}
TypeDefKind::Option(_) => {}
TypeDefKind::Result(_) => {}
TypeDefKind::List(_) => {}
TypeDefKind::Future(_) => {}
TypeDefKind::Stream(_) => {}
TypeDefKind::Type(_) => {}
TypeDefKind::Unknown => {}
}
}
}
Expand Down Expand Up @@ -227,7 +237,7 @@ fn write_param_list(
pub fn copy_wit_files(def: &StubDefinition) -> anyhow::Result<()> {
let mut all = def.unresolved_deps.clone();
all.push(def.unresolved_root.clone());
let stub_package_name = format!("{}-stub", def.root_package_name);
let stub_package_name = format!("{}-stub", def.root_package_name);

let dest_wit_root = def.target_wit_root();
fs::create_dir_all(&dest_wit_root)?;
Expand All @@ -245,19 +255,26 @@ pub fn copy_wit_files(def: &StubDefinition) -> anyhow::Result<()> {

fs::create_dir_all(&dep_dir)?;
for source in unresolved.source_files() {
// While copying this root WIT (from which the stub is generated) to the deps directory
// of the stub, we ensure there is no import of the same stub's types. This happens
// when you regenerate a stub from a WIT, which the user may have manually modified to refer to the same stub.
// This occurs only when there is a cyclic dependency. That's when the package from which the stub is generated is using the stub itself.
//
// Replacing this import with `none` is closer to being the right thing to do at this stage,
// because there is no reason a stub WIT needs to cyclically depend on its own types to generate the stub.
// Now the question is whether we need to parse this WIT to Resolved and then have our own Display
// for it, or just use regex replace. The latter is chosen because we hardly refer to the same stub without
// `import`, and the actual stub package name.
// The former may look cleaner, but slower, and may not bring about anymore significant correctness or reliability.
let read_data = fs::read_to_string(&source)?;
let re = Regex::new(format!(r"import\s+{}(/[^;]*)?;", regex::escape(stub_package_name.as_str())).as_str()).unwrap();
// While copying this root WIT (from which the stub is generated) to the deps directory
// of the stub, we ensure there is no import of the same stub's types. This happens
// when you regenerate a stub from a WIT, which the user may have manually modified to refer to the same stub.
// This occurs only when there is a cyclic dependency. That's when the package from which the stub is generated is using the stub itself.
//
// Replacing this import with `none` is closer to being the right thing to do at this stage,
// because there is no reason a stub WIT needs to cyclically depend on its own types to generate the stub.
// Now the question is whether we need to parse this WIT to Resolved and then have our own Display
// for it, or just use regex replace. The latter is chosen because we hardly refer to the same stub without
// `import`, and the actual stub package name.
// The former may look cleaner, but slower, and may not bring about anymore significant correctness or reliability.
let read_data = fs::read_to_string(source)?;
let re = Regex::new(
format!(
r"import\s+{}(/[^;]*)?;",
regex::escape(stub_package_name.as_str())
)
.as_str(),
)
.unwrap();
let new_data = re.replace_all(&read_data, "");

let dest = dep_dir.join(source.file_name().unwrap());
Expand All @@ -274,7 +291,7 @@ pub fn copy_wit_files(def: &StubDefinition) -> anyhow::Result<()> {
println!("Copying package {}", unresolved.name);

for source in unresolved.source_files() {
let parsed = UnresolvedPackage::parse_path(&source)?;
let parsed = UnresolvedPackage::parse_path(source)?;
let stub_package_name = format!("{}-stub", def.root_package_name);

if parsed.name.to_string() == stub_package_name {
Expand Down Expand Up @@ -409,11 +426,6 @@ pub enum WitAction {
CopyDepDir {
source_dir: PathBuf,
},
CopyDepWit {
source_wit: PathBuf,
dir_name: String,
},

CopyWitStr {
source_wit: String,
dir_name: String,
Expand All @@ -425,9 +437,9 @@ impl Display for WitAction {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
WitAction::CopyWitStr {
source_wit,
dir_name,
file_name,
..
} => {
write!(
f,
Expand All @@ -442,17 +454,6 @@ impl Display for WitAction {
source_dir.to_string_lossy()
)
}
WitAction::CopyDepWit {
source_wit,
dir_name,
} => {
write!(
f,
"copy stub WIT from {} as dependency {}",
source_wit.to_string_lossy(),
dir_name
)
}
}
}
}
Expand Down Expand Up @@ -491,17 +492,6 @@ impl WitAction {
)
.context("Failed to copy the dependency directory")?;
}
WitAction::CopyDepWit {
source_wit,
dir_name,
} => {
let target_dir = target_wit_root.join("deps").join(dir_name);
if !target_dir.exists() {
fs::create_dir_all(&target_dir).context("Create target directory")?;
}
fs::copy(source_wit, target_dir.join(source_wit.file_name().unwrap()))
.context("Copy the WIT file")?;
}
}

Ok(())
Expand All @@ -514,7 +504,6 @@ impl WitAction {
.context("Get wit dependency directory name")?
.to_string_lossy()
.to_string()),
WitAction::CopyDepWit { dir_name, .. } => Ok(dir_name.clone()),
WitAction::CopyWitStr { dir_name, .. } => Ok(dir_name.clone()),
}
}
Expand Down Expand Up @@ -566,42 +555,5 @@ pub fn verify_action(
Ok(true)
}
}
WitAction::CopyDepWit {
source_wit,
dir_name,
} => {
let target_dir = target_wit_root.join("deps").join(dir_name);
let source_file_name = source_wit.file_name().context("Get source wit file name")?;
let target_wit = target_dir.join(source_file_name);
if target_dir.exists() && target_dir.is_dir() {
let mut existing_entries = Vec::new();
for entry in fs::read_dir(&target_dir)? {
let entry = entry?;
let name = entry
.path()
.file_name()
.context("Get existing wit directory's name")?
.to_string_lossy()
.to_string();
existing_entries.push(name);
}
if existing_entries.contains(&source_file_name.to_string_lossy().to_string()) {
let source_contents = fs::read_to_string(source_wit)?;
let target_contents = fs::read_to_string(&target_wit)?;
if source_contents == target_contents {
Ok(true)
} else if overwrite {
println!("Overwriting {}", target_wit.to_string_lossy());
Ok(true)
} else {
Ok(false)
}
} else {
Ok(true)
}
} else {
Ok(true)
}
}
}
}

0 comments on commit c102494

Please sign in to comment.