From 465a4012af6d0cf5f4b5848718b8bbf067fdcfa3 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 25 Sep 2024 17:38:55 -0700 Subject: [PATCH] Add unit tests for parsing attributes, and add base_dir And other things, like documentation and changelog. --- CHANGELOG.md | 102 ++++--- README.md | 50 +++- rstest/src/lib.rs | 16 +- rstest/tests/resources/rstest/files.rs | 43 ++- rstest/tests/rstest/mod.rs | 24 +- rstest_macros/src/parse/rstest/files.rs | 342 ++++++++++++++++++------ 6 files changed, 433 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6538538..6b01786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ ### Add +- You can now use environment variables in `#[files]` with an optional default value (see [#277](https://github.com/la10736/rstest/pull/277)). +- You can now set a base_dir for `#[files]` with the `$[base_dir = "..."]` attribute (see [#277](https://github.com/la10736/rstest/pull/277)). + ### Fixed ## [0.22.0] 2024/8/4 @@ -27,25 +30,25 @@ ### Changed - Add feature `crate-name` enabled by default to opt-in crate rename -support. See [#258](https://github.com/la10736/rstest/issues/258) + support. See [#258](https://github.com/la10736/rstest/issues/258) ## [0.20.0] 2024/5/30 ### Add - Implemented `#[by_ref]` attribute to take get a local lifetime for test arguments. -See [#241](https://github.com/la10736/rstest/issues/241) for more details. Thanks to -@narpfel for suggesting it and useful discussions. + See [#241](https://github.com/la10736/rstest/issues/241) for more details. Thanks to + @narpfel for suggesting it and useful discussions. - Support for import `rstest` with another name. See [#221](https://github.com/la10736/rstest/issues/221) ### Fixed - Don't remove Lifetimes from test function if any. See [#230](https://github.com/la10736/rstest/issues/230) -[#241](https://github.com/la10736/rstest/issues/241) for more details. + [#241](https://github.com/la10736/rstest/issues/241) for more details. - [`PathBuf`](https://doc.rust-lang.org/std/path/struct.PathBuf.html) does no longer need to be -in scope when using `#[files]` (see [#242](https://github.com/la10736/rstest/pull/242)) + in scope when using `#[files]` (see [#242](https://github.com/la10736/rstest/pull/242)) - `#[from(now::accept::also::path::for::fixture)]` See [#246](https://github.com/la10736/rstest/issues/246) -for more details + for more details ## [0.19.0] 2024/4/9 @@ -56,9 +59,9 @@ for more details ### Fixed - `#[once]` fixtures now require the returned type to be -[`Sync`](https://doc.rust-lang.org/std/marker/trait.Sync.html) to prevent UB -when tests are executed in parallel. (see [#235](https://github.com/la10736/rstest/issues/235) -for more details) + [`Sync`](https://doc.rust-lang.org/std/marker/trait.Sync.html) to prevent UB + when tests are executed in parallel. (see [#235](https://github.com/la10736/rstest/issues/235) + for more details) - `#[future(awt)]` and `#[awt]` now properly handle mutable (`mut`) parameters by treating futures as immutable and treating the awaited rebinding as mutable. @@ -68,7 +71,7 @@ for more details) ### Changed - Now `#[files]` accept also parent folders (see [#205](https://github.com/la10736/rstest/issues/205) -for more details). + for more details). ## [0.18.1] 2023/7/5 @@ -82,20 +85,20 @@ for more details). ### Add - Add support for `RSTEST_TIMEOUT` environment variable to define a max timeout -for each function (see [#190](https://github.com/la10736/rstest/issues/190) for details). -Thanks to @aviramha for idea and PR -- `#[files("glob path")]` attribute to generate tests based on files that -satisfy the given glob path (see [#163](https://github.com/la10736/rstest/issues/163) for details). + for each function (see [#190](https://github.com/la10736/rstest/issues/190) for details). + Thanks to @aviramha for idea and PR +- `#[files("glob path")]` attribute to generate tests based on files that + satisfy the given glob path (see [#163](https://github.com/la10736/rstest/issues/163) for details). ### Changed -- Switch to `syn` 2.0 and edition 2021 : minimal Rust version now is 1.56.0 -both for `rstest` and `rstest_reuse` (see [#187](https://github.com/la10736/rstest/issues/187)) +- Switch to `syn` 2.0 and edition 2021 : minimal Rust version now is 1.56.0 + both for `rstest` and `rstest_reuse` (see [#187](https://github.com/la10736/rstest/issues/187)) ### Fixed - Fixed wired behavior on extraction `#[awt]` function attrs (See -[#189](https://github.com/la10736/rstest/issues/189)) + [#189](https://github.com/la10736/rstest/issues/189)) ## [0.17.0] 2023/3/19 @@ -108,10 +111,11 @@ both for `rstest` and `rstest_reuse` (see [#187](https://github.com/la10736/rste - Fixed wrong message when timeout tests panic before timeout expire (See #171) ## [0.16.0] 2022/11/27 + ### Changed -- Show `TEST START` banner only when trace some argument: See #158 for details. -- Add values to test name: See #160 for details. +- Show `TEST START` banner only when trace some argument: See #158 for details. +- Add values to test name: See #160 for details. ### Fixed @@ -120,12 +124,14 @@ both for `rstest` and `rstest_reuse` (see [#187](https://github.com/la10736/rste ## [0.15.0] 2022/06/27 ### Fixed - - Timeout not compile if one of its test arguments il not a copy - type [see #154] + +- Timeout not compile if one of its test arguments il not a copy + type [see #154] ## [0.14.0] 2022/06/19 ### Changed + - Feature gated async timeout via `async-timeout` feature [see #148] ### Fixed @@ -162,31 +168,31 @@ both for `rstest` and `rstest_reuse` (see [#187](https://github.com/la10736/rste ### Add -- Rename fixture (See #107 and #108) +- Rename fixture (See #107 and #108) ### Fixed - Wired behaviour in `#[fixture]` with generics types that have transitive -reference (See #116) + reference (See #116) ## [0.9.0] 2021/05/2 ### Add -- `#[future]` arg attribute to remove `impl Future<>` boilerplate. (See #98) +- `#[future]` arg attribute to remove `impl Future<>` boilerplate. (See #98) ## [0.8.0] 2021/4/25 ### Add -- Magic Conversion: use literal string for define values where type implements -`FromStr` trait (See #111) +- Magic Conversion: use literal string for define values where type implements + `FromStr` trait (See #111) ### Changed -- `#[default]` arg attribute cannot use key = arbitrary rust expression syntax -(is unstable https://github.com/rust-lang/rust/issues/78835). So we switched -to `#[default(expression)]` syntax. (See #117) +- `#[default]` arg attribute cannot use key = arbitrary rust expression syntax + (is unstable https://github.com/rust-lang/rust/issues/78835). So we switched + to `#[default(expression)]` syntax. (See #117) ### Fixed @@ -194,67 +200,75 @@ to `#[default(expression)]` syntax. (See #117) ## [0.7.0] 2021/3/21 -This version intruduce the new more composable syntax. And async +This version intruduce the new more composable syntax. And async fixtures (thanks to @rubdos) ### Add + - New syntax that leverage on function and argument attributes -to implement all features (See #99, #100, #101, #103, #109 and #102) + to implement all features (See #99, #100, #101, #103, #109 and #102) - `async` fixtures (See #86, #96. Thanks to @rubdos). ### Changed + - Moved integration tests resouces in `test` directory (See #97) ## [0.6.4] 2020/6/20 ### Add + - Implemented reusable test list with `rstest_reuse` external crate (See #80) ## [0.6.3] 2020/4/19 ### Add + - Define default values instead use trivial fixtures (See #72). ## [0.6.2] 2020/4/13 ### Add -- Injecting test attribute. You can choose your own test attribute (should be something like `*::test`) -for each test. This feature enable every async runtime (See #91). + +- Injecting test attribute. You can choose your own test attribute (should be something like `*::test`) + for each test. This feature enable every async runtime (See #91). ### Changed -- Start to use `rstest` to test `rstest` (On going task #92) + +- Start to use `rstest` to test `rstest` (On going task #92) ## [0.6.1] 2020/4/5 ### Add + - Introducing async tests support. Leverage on `async_std::test` to automatically switch to -async test (both for single, cases and matrix) (See #73) + async test (both for single, cases and matrix) (See #73) ## [0.6.0] 2020/3/5 ### Add + - Hook argument name to fixture by remove starting `_` (See #70) - Every `case` can have a specific set of attributes (See #82) ### Changed - Removed useless `rstest_parametrize` and `rstest_matrix` (See #81). From 0.5.0 you -can use just `rstest` to create cases and values list. + can use just `rstest` to create cases and values list. - Removed `cargo-edit` test dependecy (See #61) ## [0.5.3] 2020/1/23 ### Fixed -- Fixed a false _unused mut_ warning regression introduced by -partial fixtures (See [8a0ff08](https://github.com/la10736/rstest/commit/8a0ff0874dc8186edfaefb1ddef64d53666b94da)) +- Fixed a false _unused mut_ warning regression introduced by + partial fixtures (See [8a0ff08](https://github.com/la10736/rstest/commit/8a0ff0874dc8186edfaefb1ddef64d53666b94da)) ## [0.5.2] 2019/12/29 ### Fixed - Fixed _unused attribute_ warning when use `should_panic` -attribute (See #79) + attribute (See #79) ## [0.5.1] 2019/12/14 @@ -269,14 +283,14 @@ attribute (See #79) - Use just `rstest` for implementing all kind of tests (See #42) - New matrix tests render: indicate argument name and nest groups -in modules (See #68 for details) + in modules (See #68 for details) - CI (github actions) build and tests (See #46) ### Changed - Better `README.md` that introduce all features - (From rustc 1.40) Deprecated `rstest_parametrize` and `rstest_matrix`: -`rstest` now cover all features + `rstest` now cover all features - Refactored ### Fixed @@ -295,7 +309,7 @@ in modules (See #68 for details) - Injecting fixture with partial values in all tests (See #48) - Add new `rstest_matrix` macro to build tests by carthesian product of -input arguments (See #38) + input arguments (See #38) ### Fixed @@ -312,9 +326,9 @@ input arguments (See #38) ### Added - Introduced `fixture` macro: Now you must annotate your fixture by -this tag. See #5 + this tag. See #5 - Support for arbitrary rust code without use `Unwrap(str_lit)` trick. -See #19 and #20 (deprecate `Unwrap()`) + See #19 and #20 (deprecate `Unwrap()`) - Support for tests that return `Result()`, See #23 - Support for dump test arguments - `rstest_parametrize` use module to group cases (See #13) diff --git a/README.md b/README.md index 260bd81..b44aeb6 100644 --- a/README.md +++ b/README.md @@ -37,12 +37,12 @@ pub fn fixture() -> u32 { 42 } #[rstest] fn should_success(fixture: u32) { -    assert_eq!(fixture, 42); +     assert_eq!(fixture, 42); } #[rstest] fn should_fail(fixture: u32) { -    assert_ne!(fixture, 42); +     assert_ne!(fixture, 42); } ``` @@ -201,7 +201,8 @@ async fn base() -> u32 { 42 } #[rstest] #[case(21, async { 2 })] #[case(6, async { 7 })] -async fn my_async_test(#[future] base: u32, #[case] expected: u32, #[future] #[case] div: u32) { +async fn my_async_test(#[future] base: u32, #[case] expected: u32, #[future] +#[case] div: u32) { assert_eq!(expected, base.await / div.await); } ``` @@ -219,13 +220,15 @@ use rstest::*; #[case(21, async { 2 })] #[case(6, async { 7 })] #[awt] -async fn global(#[future] base: u32, #[case] expected: u32, #[future] #[case] div: u32) { +async fn global(#[future] base: u32, #[case] expected: u32, #[future] +#[case] div: u32) { assert_eq!(expected, base / div); } #[rstest] #[case(21, async { 2 })] #[case(6, async { 7 })] -async fn single(#[future] base: u32, #[case] expected: u32, #[future(awt)] #[case] div: u32) { +async fn single(#[future] base: u32, #[case] expected: u32, #[future(awt)] +#[case] div: u32) { assert_eq!(expected, base.await / div); } ``` @@ -238,7 +241,8 @@ satisfy the given glob path. ```rust #[rstest] -fn for_each_file(#[files("src/**/*.rs")] #[exclude("test")] path: PathBuf) { +fn for_each_file(#[files("src/**/*.rs")] + #[exclude("test")] path: PathBuf) { assert!(check_file(&path)) } ``` @@ -249,6 +253,17 @@ used more than once on the same variable, and you can also create some custom exclusion rules with the `#[exclude("regex")]` attributes that filter out all paths that verify the regular expression. +You can pass in environment variables by using `${ENV_VAR_NAME}` in the glob +path, e.g. `#[files("${SOME_ENV}/hello")]`. To set a default value for the +environment variable, use `${ENV_VAR_NAME:-default_value}`. + +Files are resolved at compile time against your Cargo project root +(the `CARGO_MANIFEST_DIR` environment variable). If you need to change this +behavior, you can use the `#[base_dir = "..."]` attribute to specify a different +base directory. That directory MUST exist, and will be used as the root for +the files, as well as to resolve the relative path when creating the test name. +Similar to the `files` attribute, you can use `${ENV_VAR_NAME}` in the `base_dir`. + ### Default timeout You can set a default timeout for test using the `RSTEST_TIMEOUT` environment variable. @@ -264,7 +279,7 @@ expression that should return a `std::time::Duration`. Follow a simple async exa use rstest::*; use std::time::Duration; -async fn delayed_sum(a: u32, b: u32,delay: Duration) -> u32 { +async fn delayed_sum(a: u32, b: u32, delay: Duration) -> u32 { async_std::task::sleep(delay).await; a + b } @@ -319,7 +334,8 @@ use std::future::Future; #[case(2, async { 4 })] #[case(21, async { 42 })] #[actix_rt::test] -async fn my_async_test(#[case] a: u32, #[case] #[future] result: u32) { +async fn my_async_test(#[case] a: u32, #[case] +#[future] result: u32) { assert_eq!(2 * a, result.await); } ``` @@ -363,7 +379,7 @@ fn make_e_from_bool<'a>(_bump: &'a (), b: bool) -> E<'a> { #[fixture] fn bump() -> () {} - + #[rstest] #[case(true, E::A(true))] fn it_works<'a>(#[by_ref] bump: &'a (), #[case] b: bool, #[case] expected: E<'a>) { @@ -399,7 +415,7 @@ fn alice() -> User { #[rstest] #[case::authorized_user(alice())] // We can use `fixture` also as standard function #[case::guest(User::Guest)] // We can give a name to every case : `guest` in this case - // and `authorized_user` +// and `authorized_user` #[should_panic(expected = "Invalid query error")] // We would test a panic fn should_be_invalid_query_error( repository: impl Repository, @@ -530,21 +546,31 @@ See [CHANGELOG.md](/CHANGELOG.md) Licensed under either of * Apache License, Version 2.0, ([LICENSE-APACHE](/LICENSE-APACHE) or -[license-apache-link]) + [license-apache-link]) * MIT license [LICENSE-MIT](/LICENSE-MIT) or [license-MIT-link] -at your option. + at your option. [//]: # (links) [crate-image]: https://img.shields.io/crates/v/rstest.svg + [crate-link]: https://crates.io/crates/rstest + [docs-image]: https://docs.rs/rstest/badge.svg + [docs-link]: https://docs.rs/rstest/ + [test-action-image]: https://github.com/la10736/rstest/workflows/Test/badge.svg + [test-action-link]: https://github.com/la10736/rstest/actions?query=workflow:Test + [license-apache-image]: https://img.shields.io/badge/license-Apache2.0-blue.svg + [license-mit-image]: https://img.shields.io/badge/license-MIT-blue.svg + [license-apache-link]: http://www.apache.org/licenses/LICENSE-2.0 + [license-MIT-link]: http://opensource.org/licenses/MIT + [reuse-crate-link]: https://crates.io/crates/rstest_reuse diff --git a/rstest/src/lib.rs b/rstest/src/lib.rs index f1e0e93..1a448b1 100644 --- a/rstest/src/lib.rs +++ b/rstest/src/lib.rs @@ -728,7 +728,7 @@ pub use rstest_macros::fixture; /// /// ``` /// use rstest::rstest; -/// +/// /// fn sum(a: usize, b: usize) -> usize { a + b } /// /// #[rstest] @@ -966,6 +966,7 @@ pub use rstest_macros::fixture; /// assert!(check_file(&path)) /// } /// ``` +/// /// The default behavior is to ignore the files that start with `"."`, but you can /// modify this by use `#[include_dot_files]` attribute. The `files` attribute can be /// used more than once on the same variable, and you can also create some custom @@ -979,6 +980,17 @@ pub use rstest_macros::fixture; /// `valid_call.yaml` in the folder `../test_cases` (from your crate root) a test name could be /// `path_1__UP_test_cases_valid_call_yaml`. /// +/// Sometimes you want to change the base path for the test files. You can do that by using +/// `#[base_dir = "..."]` attribute. The `base_dir` is resolved relative to the crate root +/// (similar to the `files` attribute without `base_dir`). If you want to use an absolute +/// path you can use `#[base_dir = "/path/to/your/files"]`. +/// +/// Both `files` and `base_dir` attributes can use environment variables using the syntax +/// `$VAR` or `${VAR}`. If the environment variable is not set, the attribute will cause +/// an error. This can be ignored using the `#[ignore_missing_env_vars]` attribute. A +/// default value can be provided for the environment variable using the syntax +/// `${VAR:-default}` (similar to bash). +/// /// ## Use Parametrize definition in more tests /// /// If you need to use a test list for more than one test you can use @@ -1186,7 +1198,7 @@ pub use rstest_macros::fixture; /// /// #[fixture] /// fn bump() -> () {} -/// +/// /// #[rstest] /// #[case(true, E::A(true))] /// fn it_works<'a>(#[by_ref] bump: &'a (), #[case] b: bool, #[case] expected: E<'a>) { diff --git a/rstest/tests/resources/rstest/files.rs b/rstest/tests/resources/rstest/files.rs index ac981b9..df1f81a 100644 --- a/rstest/tests/resources/rstest/files.rs +++ b/rstest/tests/resources/rstest/files.rs @@ -40,7 +40,12 @@ fn ignore_missing_env_vars( #[exclude("exclude")] path: PathBuf, ) { - let _ = path; + let name = path.file_name().unwrap(); + let mut f = File::open(&path).unwrap(); + let mut contents = String::new(); + f.read_to_string(&mut contents).unwrap(); + + assert!(contents.starts_with(name.to_str().unwrap())) } #[rstest] @@ -49,7 +54,41 @@ fn env_vars( #[exclude("exclude")] path: PathBuf, ) { - let _ = path; + let name = path.file_name().unwrap(); + let mut f = File::open(&path).unwrap(); + let mut contents = String::new(); + f.read_to_string(&mut contents).unwrap(); + + assert!(contents.starts_with(name.to_str().unwrap())) +} + +#[rstest] +fn env_vars_unknown( + #[files("${__UNKNOWN__:-files}/**/*.txt")] + #[exclude("exclude")] + path: PathBuf, +) { + let name = path.file_name().unwrap(); + let mut f = File::open(&path).unwrap(); + let mut contents = String::new(); + f.read_to_string(&mut contents).unwrap(); + + assert!(contents.starts_with(name.to_str().unwrap())) +} + +#[rstest] +fn env_vars_base_dir( + #[files("**/*.txt")] + #[base_dir = "files"] + #[exclude("exclude")] + path: PathBuf, +) { + let name = path.file_name().unwrap(); + let mut f = File::open(&path).unwrap(); + let mut contents = String::new(); + f.read_to_string(&mut contents).unwrap(); + + assert!(contents.starts_with(name.to_str().unwrap())) } mod module { diff --git a/rstest/tests/rstest/mod.rs b/rstest/tests/rstest/mod.rs index 485d166..bcd3c6d 100644 --- a/rstest/tests/rstest/mod.rs +++ b/rstest/tests/rstest/mod.rs @@ -69,11 +69,27 @@ fn files() { .ok("env_vars::path_3_files_element_2_txt") .ok("env_vars::path_4_files_element_3_txt") .ok("env_vars::path_5_files_sub_sub_dir_file_txt") + .ok("env_vars_base_dir::path_1_element_0_txt") + .ok("env_vars_base_dir::path_2_element_1_txt") + .ok("env_vars_base_dir::path_3_element_2_txt") + .ok("env_vars_base_dir::path_4_element_3_txt") + .ok("env_vars_base_dir::path_5_sub_sub_dir_file_txt") + .ok("env_vars_unknown::path_1_files_element_0_txt") + .ok("env_vars_unknown::path_2_files_element_1_txt") + .ok("env_vars_unknown::path_3_files_element_2_txt") + .ok("env_vars_unknown::path_4_files_element_3_txt") + .ok("env_vars_unknown::path_5_files_sub_sub_dir_file_txt") .ok("ignore_missing_env_vars::path_1_files_element_0_txt") .ok("ignore_missing_env_vars::path_2_files_element_1_txt") - .ok("ignore_missing_env_vars::path_4_files_element_3_txt") .ok("ignore_missing_env_vars::path_3_files_element_2_txt") + .ok("ignore_missing_env_vars::path_4_files_element_3_txt") .ok("ignore_missing_env_vars::path_5_files_sub_sub_dir_file_txt") + .ok("module::pathbuf_need_not_be_in_scope::path_1_files__ignore_me_txt") + .ok("module::pathbuf_need_not_be_in_scope::path_2_files_element_0_txt") + .ok("module::pathbuf_need_not_be_in_scope::path_3_files_element_1_txt") + .ok("module::pathbuf_need_not_be_in_scope::path_4_files_element_2_txt") + .ok("module::pathbuf_need_not_be_in_scope::path_5_files_element_3_txt") + .ok("module::pathbuf_need_not_be_in_scope::path_6_files_sub_sub_dir_file_txt") .ok("start_with_name::path_1__UP_files_test_sub_folder_from_parent_folder_txt") .ok("start_with_name::path_2_files_element_0_txt") .ok("start_with_name::path_3_files_element_1_txt") @@ -86,12 +102,6 @@ fn files() { .ok("start_with_name_with_include::path_4_files_element_2_txt") .ok("start_with_name_with_include::path_5_files_element_3_txt") .ok("start_with_name_with_include::path_6_files_sub_sub_dir_file_txt") - .ok("module::pathbuf_need_not_be_in_scope::path_1_files__ignore_me_txt") - .ok("module::pathbuf_need_not_be_in_scope::path_2_files_element_0_txt") - .ok("module::pathbuf_need_not_be_in_scope::path_3_files_element_1_txt") - .ok("module::pathbuf_need_not_be_in_scope::path_4_files_element_2_txt") - .ok("module::pathbuf_need_not_be_in_scope::path_5_files_element_3_txt") - .ok("module::pathbuf_need_not_be_in_scope::path_6_files_sub_sub_dir_file_txt") .assert(output); } diff --git a/rstest_macros/src/parse/rstest/files.rs b/rstest_macros/src/parse/rstest/files.rs index 620d728..1f83ad9 100644 --- a/rstest_macros/src/parse/rstest/files.rs +++ b/rstest_macros/src/parse/rstest/files.rs @@ -1,11 +1,5 @@ use std::{env, path::PathBuf}; -use glob::glob; -use quote::ToTokens; -use regex::Regex; -use relative_path::RelativePath; -use syn::{parse_quote, visit_mut::VisitMut, Attribute, Expr, FnArg, Ident, ItemFn, LitStr}; - use crate::{ error::ErrorsVec, parse::{ @@ -15,51 +9,15 @@ use crate::{ refident::{IntoPat, MaybeIdent}, utils::attr_is, }; - -/// Replace environment variables in a string. -fn replace_env_vars(attr: &LitStrAttr, ignore_missing: bool) -> Result { - let re = - Regex::new(r"\$([a-zA-Z_][a-zA-Z0-9_]*)|\$\{([^}]*)}").expect("Could not build the regex"); - - let path = attr.value(); - let haystack = path.as_str(); - let mut result = String::with_capacity(attr.value().len()); - let mut last_match = 0; - - for caps in re.captures_iter(haystack) { - let m = caps.get(0).expect("The regex should have matched"); - let var_name = caps - .get(1) - .or_else(|| caps.get(2)) - .expect("The regex should have matched either $VAR or ${VAR}") - .as_str(); - - let replacement = match env::var(var_name) { - Ok(val) => val, - Err(_) if ignore_missing => String::new(), - Err(_) => { - return Err(attr.error(&format!( - "Could not find the environment variable {:?}", - var_name - ))) - } - }; - - // Make sure cargo would rerun if the value of this environment variable changes. - println!("cargo::rerun-if-env-changed={var_name}"); - - result.push_str(&haystack[last_match..m.start()]); - result.push_str(&replacement); - last_match = m.end(); - } - - result.push_str(&haystack[last_match..]); - - Ok(result) -} +use glob::glob; +use quote::ToTokens; +use regex::Regex; +use relative_path::RelativePath; +use syn::{parse_quote, visit_mut::VisitMut, Attribute, Expr, FnArg, Ident, ItemFn, LitStr}; #[derive(Debug, Clone, PartialEq)] pub(crate) struct FilesGlobReferences { + base_dir: Option, glob: Vec, exclude: Vec, ignore_dot_files: bool, @@ -67,12 +25,90 @@ pub(crate) struct FilesGlobReferences { } impl FilesGlobReferences { + /// Replace environment variables in a string. + fn replace_env_vars(&self, attr: &LitStrAttr) -> Result { + let re = Regex::new(r"\$([a-zA-Z_][a-zA-Z0-9_]+)|\$\{([^}]+)}") + .expect("Could not build the regex"); + + let ignore_missing = self.ignore_missing_env_vars; + + let path = attr.value(); + let haystack = path.as_str(); + let mut result = String::with_capacity(attr.value().len()); + let mut last_match = 0; + + for caps in re.captures_iter(haystack) { + let match_all = caps.get(0).expect("The regex should have matched"); + + // Match the name of the variable and its default value (if any). + let (var_name, default) = if let Some(m) = caps.get(1) { + // In the first case ($VAR), the default value is None. + (m.as_str(), None) + } else { + // In the second case we have to split the variable name and the default value + // if there's a `:-` separator. + let m = caps.get(2).expect("The regex should have matched"); + + let (var_name, default) = + if let Some((var_name, default)) = m.as_str().split_once(":-") { + (var_name, Some(default)) + } else { + (m.as_str(), None) + }; + + var_name + .chars() + .all(|c| c.is_alphanumeric() || c == '_') + .then_some((var_name, default)) + .ok_or_else(|| { + attr.error(&format!( + "The variable \"{}\" name does not match [a-zA-Z0-9_]+", + m.as_str() + )) + })? + }; + + let v = env::var(var_name); + let replacement = match (v.as_ref().map(String::as_str), default) { + (Ok(""), Some(default)) => default.to_string(), + (Ok(val), _) => val.to_string(), + (Err(_), Some(default)) => default.to_string(), + (Err(_), _) if ignore_missing => String::new(), + (Err(_), _) => { + return Err(attr.error(&format!( + "Could not find the environment variable {:?}", + var_name + ))) + } + }; + + // Make sure cargo would rerun if the value of this environment variable changes. + println!("cargo::rerun-if-env-changed={var_name}"); + + result.push_str(&haystack[last_match..match_all.start()]); + result.push_str(&replacement); + last_match = match_all.end(); + } + + result.push_str(&haystack[last_match..]); + + Ok(result) + } + + /// Return the base_dir attribute if set, with variables replaced. + fn base_dir(&self) -> Result, syn::Error> { + self.base_dir + .as_ref() + .map(|attr| self.replace_env_vars(attr).map(PathBuf::from)) + .transpose() + } + /// Return the tuples attribute, path string if they are valid relative paths fn paths(&self, base_dir: &PathBuf) -> Result, syn::Error> { self.glob .iter() .map(|attr| { - let path = replace_env_vars(&attr, self.ignore_missing_env_vars)?; + let path = self.replace_env_vars(attr)?; RelativePath::from_path(&path) .map_err(|e| attr.error(&format!("Invalid glob path: {e}"))) .map(|p| p.to_logical_path(base_dir)) @@ -97,15 +133,26 @@ impl ToTokens for LitStrAttr { } impl FilesGlobReferences { - fn new(glob: Vec, exclude: Vec, ignore_dot_files: bool, ignore_missing_env_vars: bool) -> Self { + fn new( + glob: Vec, + exclude: Vec, + ignore_dot_files: bool, + ignore_missing_env_vars: bool, + ) -> Self { Self { glob, + base_dir: None, exclude, ignore_dot_files, - ignore_missing_env_vars + ignore_missing_env_vars, } } + fn with_base_dir_opt(mut self, base_dir: Option) -> Self { + self.base_dir = base_dir; + self + } + fn is_valid(&self, p: &RelativePath) -> bool { if self.ignore_dot_files && p.components() @@ -125,6 +172,23 @@ struct LitStrAttr { } impl LitStrAttr { + fn parse_meta_name_value(attr: &Attribute) -> Result { + let meta = attr.meta.require_name_value()?; + if let syn::Expr::Lit(expr) = &meta.value { + if let syn::Lit::Str(value) = expr.lit.clone() { + return Ok(Self { + attr: attr.clone(), + value, + }); + } + } + + Err(syn::Error::new_spanned( + attr, + "expected a string literal value", + )) + } + fn value(&self) -> String { self.value.value() } @@ -157,7 +221,7 @@ impl TryFrom for Exclude { fn try_from(attr: Attribute) -> Result { let attr: LitStrAttr = attr.try_into()?; - let r = regex::Regex::new(&attr.value()).map_err(|e| { + let r = Regex::new(&attr.value()).map_err(|e| { syn::Error::new_spanned( &attr, format!(r#""{}" Should be a valid regex: {e}"#, attr.value()), @@ -246,13 +310,29 @@ impl ValueFilesExtractor { node, |a| attr_is(a, "ignore_missing_env_vars"), |attr| { - attr.meta - .require_path_only() - .map_err(|_| attr.error("Use #[ignore_missing_env_vars] to ignore missing environment variables"))?; + attr.meta.require_path_only().map_err(|_| { + attr.error( + "Use #[ignore_missing_env_vars] to ignore missing environment variables", + ) + })?; Ok(attr) }, ) } + + fn extract_base_dir(&mut self, node: &mut FnArg) -> Vec { + self.extract_argument_attrs( + node, + |a| attr_is(a, "base_dir"), + |attr| { + LitStrAttr::parse_meta_name_value(&attr).map_err(|_| { + attr.error( + "Use #[base_dir = \"...\"] to define the base directory for the glob path", + ) + }) + }, + ) + } } impl VisitMut for ValueFilesExtractor { @@ -278,10 +358,23 @@ impl VisitMut for ValueFilesExtractor { .push(attr.error("Cannot use #[ignore_missing_env_vars] more than once")) }) } + let base_dir = self.extract_base_dir(node); + if base_dir.len() > 1 { + base_dir.iter().skip(1).for_each(|attr| { + self.errors + .push(attr.error(r#"Cannot use #[base_dir = "..."] more than once"#)) + }) + } if !files.is_empty() { self.files.push(( name, - FilesGlobReferences::new(files, excludes, include_dot_files.is_empty(), !ignore_missing_env_vars.is_empty()), + FilesGlobReferences::new( + files, + excludes, + include_dot_files.is_empty(), + !ignore_missing_env_vars.is_empty(), + ) + .with_base_dir_opt(base_dir.into_iter().next()), )) } else { excludes.into_iter().for_each(|e| { @@ -295,8 +388,13 @@ impl VisitMut for ValueFilesExtractor { .push(attr.error("You cannot use #[include_dot_files] without #[files(...)]")) }); ignore_missing_env_vars.into_iter().for_each(|attr| { + self.errors.push( + attr.error("You cannot use #[ignore_missing_env_vars] without #[files(...)]"), + ) + }); + base_dir.into_iter().for_each(|attr| { self.errors - .push(attr.error("You cannot use #[ignore_missing_env_vars] without #[files(...)]")) + .push(attr.error(r#"You cannot use #[base_dir = "..."] without #[files(...)]"#)) }); } } @@ -370,10 +468,23 @@ impl<'a> ValueListFromFiles<'a> { } fn file_list_values(&self, refs: FilesGlobReferences) -> Result, syn::Error> { - let base_dir = self + let default_base_dir = self .base_dir .base_dir() .map_err(|msg| refs.glob[0].error(&msg))?; + + let base_dir = match refs.base_dir()? { + Some(mut p) => { + if p.is_relative() { + p = default_base_dir.join(p); + } + p.canonicalize().map_err(|e| { + refs.glob[0].error(&format!("Cannot canonicalize base dir due {e}")) + })? + } + None => default_base_dir, + }; + let resolved_paths = refs.paths(&base_dir)?; let base_dir = base_dir .into_os_string() @@ -469,10 +580,24 @@ mod should { .unwrap() } + fn name_value_attr(name: &str, value: impl AsRef) -> LitStrAttr { + LitStrAttr::parse_meta_name_value( + &attrs(&format!(r#"#[{name} = "{}"]"#, value.as_ref())) + .into_iter() + .next() + .unwrap(), + ) + .expect("Could not parse attribute") + } + fn files_attr(lstr: impl AsRef) -> LitStrAttr { lit_str_attr("files", lstr) } + fn base_dir_attr(lstr: impl AsRef) -> LitStrAttr { + name_value_attr("base_dir", lstr) + } + fn exclude_attr(lstr: impl AsRef) -> LitStrAttr { lit_str_attr("exclude", lstr) } @@ -491,36 +616,43 @@ mod should { fn from(value: &str) -> Self { Self { attr: exclude_attr(value), - r: regex::Regex::new(value).unwrap(), + r: Regex::new(value).unwrap(), } } } #[rstest] - #[case::simple(r#"fn f(#[files("some_glob")] a: PathBuf) {}"#, "fn f(a: PathBuf) {}", &[("a", &["some_glob"], &[], true)])] + #[case::simple(r#"fn f(#[files("some_glob")] a: PathBuf) {}"#, "fn f(a: PathBuf) {}", &[("a", None, &["some_glob"], &[], true, false)])] #[case::more_than_one( r#"fn f(#[files("first")] a: PathBuf, b: u32, #[files("third")] c: PathBuf) {}"#, r#"fn f(a: PathBuf, b: u32, c: PathBuf) {}"#, - &[("a", &["first"], &[], true), ("c", &["third"], &[], true)], + &[("a", None, &["first"], &[], true, false), ("c", None, &["third"], &[], true, false)], )] #[case::more_globs_on_the_same_var( r#"fn f(#[files("first")] #[files("second")] a: PathBuf) {}"#, r#"fn f(a: PathBuf) {}"#, - &[("a", &["first", "second"], &[], true)], + &[("a", None, &["first", "second"], &[], true, false)], )] #[case::exclude(r#"fn f(#[files("some_glob")] #[exclude("exclude")] a: PathBuf) {}"#, - "fn f(a: PathBuf) {}", &[("a", &["some_glob"], &["exclude"], true)])] + "fn f(a: PathBuf) {}", &[("a", None, &["some_glob"], &["exclude"], true, false)])] #[case::exclude_more(r#"fn f(#[files("some_glob")] #[exclude("first")] #[exclude("second")] a: PathBuf) {}"#, - "fn f(a: PathBuf) {}", &[("a", &["some_glob"], &["first", "second"], true)])] + "fn f(a: PathBuf) {}", &[("a", None, &["some_glob"], &["first", "second"], true, false)])] #[case::include_dot_files(r#"fn f(#[files("some_glob")] #[include_dot_files] a: PathBuf) {}"#, - "fn f(a: PathBuf) {}", &[("a", &["some_glob"], &[], false)])] - + "fn f(a: PathBuf) {}", &[("a", None, &["some_glob"], &[], false, false)])] + #[case::base_dir(r#"fn f(#[files("some_glob")] #[base_dir = "/base/"] a: PathBuf) {}"#, + "fn f(a: PathBuf) {}", &[("a", Some("/base/"), &["some_glob"], &[], true, false)])] + #[case::ignore_env_vars(r#"fn f(#[files("some_glob")] #[ignore_missing_env_vars] a: PathBuf) {}"#, + "fn f(a: PathBuf) {}", &[("a", None, &["some_glob"], &[], true, true)])] + #[case::ignore_env_vars_unknown(r#"fn f(#[files("some${SOME_VAR}_glob")] #[ignore_missing_env_vars] a: PathBuf) {}"#, + "fn f(a: PathBuf) {}", &[("a", None, &["some${SOME_VAR}_glob"], &[], true, true)])] + #[case::env_vars_default(r#"fn f(#[files("some${__UNKNOWN__:-default/value}_glob")] a: PathBuf) {}"#, + "fn f(a: PathBuf) {}", &[("a", None, &["some${__UNKNOWN__:-default/value}_glob"], &[], true, false)])] fn extract<'a, G: AsRef<[&'a str]>, E: AsRef<[&'a str]>>( #[case] item_fn: &str, #[case] expected: &str, - #[case] expected_files: &[(&str, G, E, bool)], + #[case] expected_files: &[(&str, Option<&str>, G, E, bool, bool)], ) { let mut item_fn: ItemFn = item_fn.ast(); let expected: ItemFn = expected.ast(); @@ -532,15 +664,18 @@ mod should { files, expected_files .into_iter() - .map(|(id, globs, ex, ignore)| ( - ident(id), - FilesGlobReferences::new( - globs.as_ref().iter().map(files_attr).collect(), - ex.as_ref().iter().map(|&ex| ex.into()).collect(), - *ignore, - false + .map(|(id, base_dir, globs, ex, ignore, ignore_envvars)| { + ( + ident(id), + FilesGlobReferences::new( + globs.as_ref().iter().map(files_attr).collect(), + ex.as_ref().iter().map(|&ex| ex.into()).collect(), + *ignore, + *ignore_envvars, + ) + .with_base_dir_opt(base_dir.map(base_dir_attr)), ) - )) + }) .collect::>() ); } @@ -576,6 +711,18 @@ mod should { r#"fn f(#[files("some")] #[include_dot_files] #[include_dot_files] a: PathBuf) {}"#, "more than once" )] + #[case::ignore_missing_env_vars_without_files( + r#"fn f(#[ignore_missing_env_vars] a: PathBuf) {}"#, + "#[ignore_missing_env_vars] without #[files(...)]" + )] + #[case::ignore_missing_env_vars_more_than_once( + r#"fn f(#[files("some")] #[ignore_missing_env_vars] #[ignore_missing_env_vars] a: PathBuf) {}"#, + "more than once" + )] + #[case::override_base_dir_more_than_once( + r#"fn f(#[files("some")] #[base_dir = "/"] #[base_dir = "/"] a: PathBuf) {}"#, + "more than once" + )] fn raise_error(#[case] item_fn: &str, #[case] message: &str) { let mut item_fn: ItemFn = item_fn.ast(); @@ -584,6 +731,37 @@ mod should { assert_in!(format!("{:?}", err), message); } + #[rstest] + #[case::unknown_env_var( + r#"fn f(#[files("so${__UNKNOWN__}me")] a: PathBuf) {}"#, + "Could not find the environment variable \\\"__UNKNOWN__\\\"" + )] + #[case::invalid_env_var( + r#"fn f(#[files("so${__INVALID%%__}me")] a: PathBuf) {}"#, + "The variable \\\"__INVALID%%__\\\" name does not match [a-zA-Z0-9_]+" + )] + #[case::base_dir_unknown_env_var( + r#"fn f(#[base_dir = "${__UNKNOWN__}"] #[files("some")] a: PathBuf) {}"#, + "Could not find the environment variable \\\"__UNKNOWN__\\\"" + )] + fn raise_error_on_paths(#[case] item_fn: &str, #[case] message: &str) { + let mut item_fn: ItemFn = item_fn.ast(); + + let ok = extract_files(&mut item_fn).unwrap(); + let err: syn::Error = ok + .into_iter() + .map(|(_id, refs)| { + let base_dir = refs.base_dir()?; + let x = refs.paths(base_dir.as_ref().unwrap_or(&PathBuf::from("/default")))?; + eprintln!("x {:?}", x); + Ok(()) + }) + .collect::>() + .unwrap_err(); + + assert_in!(format!("{:?}", err), message); + } + #[derive(Default)] struct FakeBaseDir(PathBuf); impl From<&str> for FakeBaseDir { @@ -755,7 +933,12 @@ mod should { ValueListFromFiles::new(ErrorBaseDir::default(), FakeResolver::default()) .to_value_list(vec![( ident("a"), - FilesGlobReferences::new(vec![files_attr("no_mater")], Default::default(), true, false), + FilesGlobReferences::new( + vec![files_attr("no_mater")], + Default::default(), + true, + false, + ), )]) .unwrap(); } @@ -766,7 +949,12 @@ mod should { ValueListFromFiles::new(FakeBaseDir::default(), FakeResolver::default()) .to_value_list(vec![( ident("a"), - FilesGlobReferences::new(vec![files_attr("no_mater")], Default::default(), true, false), + FilesGlobReferences::new( + vec![files_attr("no_mater")], + Default::default(), + true, + false, + ), )]) .unwrap(); }