From 305a82b36c78f11b8b72165cd626103590e4c6d2 Mon Sep 17 00:00:00 2001 From: Peter Byfield Date: Mon, 6 Jan 2025 10:50:46 +0100 Subject: [PATCH] InternalImportsPathQueryBuilder --- Cargo.lock | 85 ++++++++++++++ Cargo.toml | 1 + README.md | 5 +- src/contracts/layers.rs | 9 +- src/imports_info/mod.rs | 2 +- src/imports_info/queries/internal_imports.rs | 115 +++++++++---------- src/lib.rs | 3 +- 7 files changed, 151 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a870f2c..4523259e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -93,6 +93,41 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" +[[package]] +name = "darling" +version = "0.20.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f63b86c8a8826a49b8c21f08a2d07338eec8d900540f8630dc76284be802989" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.20.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95133861a8032aaea082871032f5815eb9e98cef03fa916ab4500513994df9e5" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.20.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d336a2a514f6ccccaa3e09b02d41d35330c07ddf03a62165fcec10bb561c7806" +dependencies = [ + "darling_core", + "quote", + "syn", +] + [[package]] name = "deprecate-until" version = "0.1.1" @@ -127,6 +162,37 @@ dependencies = [ "syn", ] +[[package]] +name = "derive_builder" +version = "0.20.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "507dfb09ea8b7fa618fcf76e953f4f5e192547945816d5358edffe39f6f94947" +dependencies = [ + "derive_builder_macro", +] + +[[package]] +name = "derive_builder_core" +version = "0.20.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d5bcf7b024d6835cfb3d473887cd966994907effbe9227e8c8219824d06c4e8" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "derive_builder_macro" +version = "0.20.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab63b0e2bf4d5928aff72e83a7dace85d7bba5fe12dcc3c5a572d78caffd3f3c" +dependencies = [ + "derive_builder_core", + "syn", +] + [[package]] name = "derive_more" version = "0.99.18" @@ -180,6 +246,12 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "fuchsia-cprng" version = "0.1.1" @@ -233,6 +305,12 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "indexmap" version = "1.9.3" @@ -530,6 +608,7 @@ dependencies = [ "anyhow", "derive-getters", "derive-new", + "derive_builder", "derive_more 1.0.0", "itertools 0.14.0", "lazy_static", @@ -791,6 +870,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "syn" version = "2.0.90" diff --git a/Cargo.toml b/Cargo.toml index 6c421722..48438362 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ tap = "1.0.1" derive_more = { version = "1.0.0", features = ["full"] } derive-getters = "0.5.0" derive-new = "0.7.0" +derive_builder = "0.20.2" [dev-dependencies] parameterized = "2.0.0" diff --git a/README.md b/README.md index 5d83396a..d03b58d3 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ use anyhow::Result; use maplit::{hashmap,hashset}; use pyimports::prelude::*; -use pyimports::{PackageInfo,ImportsInfo,PackageItemToken,InternalImportsPathQuery}; +use pyimports::{PackageInfo,ImportsInfo,PackageItemToken,InternalImportsPathQueryBuilder}; // You shouldn't use `testpackage!`, it just creates a fake python package // in a temporary directory. It's (unfortunately) included in the public API @@ -63,9 +63,10 @@ fn main() -> Result<()> { assert_eq!( imports_info.internal_imports().find_path( - &InternalImportsPathQuery::new() + &InternalImportsPathQueryBuilder::default() .from(root_pkg) .to(d) + .build()? )?, Some(vec![root_pkg, root_init, b, d]) ); diff --git a/src/contracts/layers.rs b/src/contracts/layers.rs index d4ea06fd..52b3b77c 100644 --- a/src/contracts/layers.rs +++ b/src/contracts/layers.rs @@ -100,7 +100,9 @@ //! ``` use crate::contracts::{ContractViolation, ForbiddenImport, ImportsContract}; -use crate::{ExtendWithDescendants, ImportsInfo, InternalImportsPathQuery, PackageItemToken}; +use crate::{ + ExtendWithDescendants, ImportsInfo, InternalImportsPathQueryBuilder, PackageItemToken, +}; use anyhow::Result; use itertools::Itertools; use maplit::hashset; @@ -191,10 +193,11 @@ impl ImportsContract for LayeredArchitectureContract { .with_descendants(imports_info.package_info()); let path = imports_info.internal_imports().find_path( - &InternalImportsPathQuery::new() + &InternalImportsPathQueryBuilder::default() .from(from) .to(to) - .excluding_paths_via(except_via), + .excluding_paths_via(except_via) + .build()?, )?; if let Some(path) = path { violations.push(ContractViolation::ForbiddenImport { diff --git a/src/imports_info/mod.rs b/src/imports_info/mod.rs index eacec58c..7a1af127 100644 --- a/src/imports_info/mod.rs +++ b/src/imports_info/mod.rs @@ -3,7 +3,7 @@ mod queries; pub use crate::imports_info::queries::external_imports::ExternalImportsQueries; pub use crate::imports_info::queries::internal_imports::{ - InternalImportsPathQuery, InternalImportsQueries, + InternalImportsPathQuery, InternalImportsPathQueryBuilder, InternalImportsQueries, }; use crate::{ package_info::{PackageInfo, PackageItemToken}, diff --git a/src/imports_info/queries/internal_imports.rs b/src/imports_info/queries/internal_imports.rs index 42b95915..8a50417b 100644 --- a/src/imports_info/queries/internal_imports.rs +++ b/src/imports_info/queries/internal_imports.rs @@ -1,6 +1,9 @@ use std::collections::{HashMap, HashSet}; use anyhow::Result; +use derive_builder::Builder; +use derive_getters::Getters; +use derive_new::new; use pathfinding::prelude::{bfs, bfs_reach}; use crate::{Error, ImportMetadata, ImportsInfo, PackageItemToken}; @@ -10,54 +13,28 @@ pub struct InternalImportsQueries<'a> { pub(crate) imports_info: &'a ImportsInfo, } -#[derive(Debug, Clone)] -/// An object used to build an internal imports path query. +/// An object representing an internal imports path query. +#[derive(Debug, Clone, new, Getters, Builder)] +#[builder(setter(into))] pub struct InternalImportsPathQuery { + /// Package items from which paths may start. + #[new(into)] from: HashSet, - to: HashSet, - excluding_paths_via: HashSet, -} - -impl Default for InternalImportsPathQuery { - fn default() -> Self { - Self::new() - } -} - -impl InternalImportsPathQuery { - /// Creates a new [InternalImportsPathQuery]. - pub fn new() -> Self { - InternalImportsPathQuery { - from: HashSet::new(), - to: HashSet::new(), - excluding_paths_via: HashSet::new(), - } - } - /// Adds package items from which paths may start. - pub fn from>>(mut self, items: T) -> Self { - let items: HashSet = items.into(); - self.from.extend(items); - self - } - - /// Adds package items to which paths may end. - pub fn to>>(mut self, items: T) -> Self { - let items: HashSet = items.into(); - self.to.extend(items); - self - } + /// Package items where paths may end. + #[new(into)] + to: HashSet, - /// Exclude paths that would go via the passed package items. + /// Paths that would go via these package items should be excluded. /// /// ``` /// # use std::collections::HashSet; /// # use anyhow::Result; /// # use maplit::{hashmap, hashset}; - /// # use pyimports::{testpackage,TestPackage,PackageInfo,ImportsInfo,InternalImportsPathQuery}; + /// # use pyimports::{testpackage,TestPackage,PackageInfo,ImportsInfo,InternalImportsPathQuery,InternalImportsPathQueryBuilder}; /// # fn main() -> Result<()> { /// let test_package = testpackage! { - /// "__init__.py" => "", + /// "__init__.py" => "", /// "a.py" => "from testpackage import b, e", /// "b.py" => "from testpackage import c", /// "c.py" => "", @@ -87,9 +64,10 @@ impl InternalImportsPathQuery { /// // Sanity check: The shortest path goes via `b`. /// assert_eq!( /// imports_info.internal_imports().find_path( - /// &InternalImportsPathQuery::new() + /// &InternalImportsPathQueryBuilder::default() /// .from(a) /// .to(c) + /// .build()? /// )?, /// Some(vec![a, b, c]) /// ); @@ -97,21 +75,20 @@ impl InternalImportsPathQuery { /// // If we exclude `b`, we get the longer path via `e`. /// assert_eq!( /// imports_info.internal_imports().find_path( - /// &InternalImportsPathQuery::new() + /// &InternalImportsPathQueryBuilder::default() /// .from(a) /// .to(c) /// .excluding_paths_via(b) + /// .build()? /// )?, /// Some(vec![a, e, d, c]) /// ); /// # Ok(()) /// # } /// ``` - pub fn excluding_paths_via>>(mut self, items: T) -> Self { - let items: HashSet = items.into(); - self.excluding_paths_via.extend(items); - self - } + #[builder(default)] + #[new(into)] + excluding_paths_via: HashSet, } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -486,7 +463,7 @@ impl<'a> InternalImportsQueries<'a> { /// # use std::collections::HashSet; /// # use anyhow::Result; /// # use maplit::{hashmap, hashset}; - /// # use pyimports::{testpackage,TestPackage,PackageInfo,ImportsInfo,InternalImportsPathQuery}; + /// # use pyimports::{testpackage,TestPackage,PackageInfo,ImportsInfo,InternalImportsPathQuery, InternalImportsPathQueryBuilder}; /// # fn main() -> Result<()> { /// let test_package = testpackage! { /// "__init__.py" => "from testpackage import a, b", @@ -513,9 +490,10 @@ impl<'a> InternalImportsQueries<'a> { /// /// assert_eq!( /// imports_info.internal_imports().find_path( - /// &InternalImportsPathQuery::new() + /// &InternalImportsPathQueryBuilder::default() /// .from(root_init) /// .to(c) + /// .build()? /// )?, /// Some(vec![root_init, b, c]) /// ); @@ -576,7 +554,7 @@ impl<'a> InternalImportsQueries<'a> { /// # use std::collections::HashSet; /// # use anyhow::Result; /// # use maplit::{hashmap, hashset}; - /// # use pyimports::{testpackage,TestPackage,PackageInfo,ImportsInfo,InternalImportsPathQuery}; + /// # use pyimports::{testpackage,TestPackage,PackageInfo,ImportsInfo,InternalImportsPathQuery,InternalImportsPathQueryBuilder}; /// # fn main() -> Result<()> { /// let test_package = testpackage! { /// "__init__.py" => "from testpackage import a, b", @@ -603,12 +581,12 @@ impl<'a> InternalImportsQueries<'a> { /// /// assert!( /// imports_info.internal_imports().path_exists( - /// &InternalImportsPathQuery::new().from(root_init).to(c) + /// &InternalImportsPathQueryBuilder::default().from(root_init).to(c).build()? /// )?, /// ); /// assert!( /// !imports_info.internal_imports().path_exists( - /// &InternalImportsPathQuery::new().from(c).to(root_init) + /// &InternalImportsPathQueryBuilder::default().from(c).to(root_init).build()? /// )?, /// ); /// # Ok(()) @@ -864,9 +842,12 @@ from testpackage import books", let e = imports_info._item("testpackage.e"); assert_eq!( - imports_info - .internal_imports() - .find_path(&InternalImportsPathQuery::new().from(a).to(e))?, + imports_info.internal_imports().find_path( + &InternalImportsPathQueryBuilder::default() + .from(a) + .to(e) + .build()? + )?, Some(vec![a, c, e]) ); @@ -894,18 +875,22 @@ from testpackage import books", let e = imports_info._item("testpackage.e"); assert_eq!( - imports_info - .internal_imports() - .find_path(&InternalImportsPathQuery::new().from(a).to(c))?, + imports_info.internal_imports().find_path( + &InternalImportsPathQueryBuilder::default() + .from(a) + .to(c) + .build()? + )?, Some(vec![a, b, c]) ); assert_eq!( imports_info.internal_imports().find_path( - &InternalImportsPathQuery::new() + &InternalImportsPathQueryBuilder::default() .from(a) .to(c) .excluding_paths_via(b) + .build()? )?, Some(vec![a, e, d, c]) ); @@ -930,12 +915,18 @@ from testpackage import books", let a = imports_info._item("testpackage.a"); let e = imports_info._item("testpackage.e"); - assert!(imports_info - .internal_imports() - .path_exists(&InternalImportsPathQuery::new().from(a).to(e))?); - assert!(!imports_info - .internal_imports() - .path_exists(&InternalImportsPathQuery::new().from(e).to(a))?); + assert!(imports_info.internal_imports().path_exists( + &InternalImportsPathQueryBuilder::default() + .from(a) + .to(e) + .build()? + )?); + assert!(!imports_info.internal_imports().path_exists( + &InternalImportsPathQueryBuilder::default() + .from(e) + .to(a) + .build()? + )?); Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index e3a802ab..502faf44 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,8 @@ pub use testutils::TestPackage; pub use errors::Error; pub use imports_info::{ ExplicitImportMetadata, ExternalImportsQueries, ImportMetadata, ImportsInfo, - ImportsInfoBuildOptions, InternalImportsPathQuery, InternalImportsQueries, + ImportsInfoBuildOptions, InternalImportsPathQuery, InternalImportsPathQueryBuilder, + InternalImportsQueries, }; pub use package_info::{ ExtendWithDescendants, Module, ModuleToken, Package, PackageInfo, PackageItem,