Repository: ProbablyClem/utoipauto (language: rust)

Issue:
Macro errors with `expected ','`
Hi, and thanks for making this crate!

When trying to replace my (working) manual `#[openapi]` annotation with `utoipauto`, I'm getting the error 

```
error: expected `,`
  --> service-libs/web-backend/src/api/mod.rs:23:1
   |
23 | #[utoipauto::utoipauto(paths = "./service-libs/web-backend/src/api/internal")]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: this error originates in the attribute macro `utoipauto::utoipauto` (in Nightly builds, run with -Z macro-backtrace for more info)
```

With `-Z macro-backtrace`, the compiler helpfully traces the error back to the macro:

```
::: /<PATH>/index.crates.io-6f17d22bba15001f/utoipauto-macro-0.1.4/src/lib.rs:15:1
   |
15 | / pub fn utoipauto(
16 | |     attributes: proc_macro::TokenStream, // #[utoipauto(paths = "(MODULE_TREE_PATH => MODULE_SRC_PATH) ;")]
17 | |     item: proc_macro::TokenStream,       // #[openapi(paths = "")]
18 | | ) -> proc_macro::TokenStream {
   | |____________________________- in this expansion of `#[utoipauto::utoipauto]`
```

The project is part of a workspace, so I'm using `#[utoipauto::utoipauto(paths = "./service-libs/web-backend/src/api")]` to make `utoipauto` look at the correct source path (there's nothing at `./src`).

I've tried to figure out where the error occurs or which of my types or handlers causes it, but when I point it at one of the files with 1 `ToResponse` derive, 2 `ToSchema`s and 1 handler, the error only goes away if I remove the `utoipa` macros / derives from all of them. I also couldn't manage to reproduce the error by modifying your tests, which is weird. Could it be that the different file structure in a workspace messes things up?


Discussion / hints:
Hello, 
could you please show me the full declaration of your OpenApi struct please ? 
This whole block
```rust 
#[utoipauto]
#[derive(OpenApi)]
#[openapi(
    paths(
        test_controller::service::func_get_1,
    ),
)]
pub struct ApiDoc;
```
This may be an issue happening with matching auto import and manual imports (which should be supported)
@ProbablyClem the full code was something like

```rs
#[derive(Debug, OpenApi)]
#[openapi(
    modifiers(&SecurityAddon),
    info(
        version = "1",
    ),
    tags(
        (name = "auth", description = "Endpoints for authentication and authorization."),
    ),
    paths(
        internal::permissions::get
    ),
    components(
        responses(
            internal::permissions::PermissionsResponse,
        ),
        schemas(
            crate::permissions::Permission,
            crate::permissions::Role,
            internal::permissions::PermissionInfo,
            internal::permissions::RoleInfo,
        )
    )
)]
pub struct ApiDoc;
```

But I removed all `paths` and `components` when replacing them with the `#[utoipauto]` invocation, so I did not declare paths manually while the struct was annotated with your macro.
Is it part of a public repo that I can check ?
Could you put utoipauto back and run cargo-expand to see the generated code please ?
> Is it part of a public repo that I can check ?

It is not, unfortunately. But I've run `cargo expand` (I had previously tried to expand via rust-analyzer, but that just gives up) and this is what it outputs:

```rs
#[openapi(
    paths(
        crate::web-backend::src::api::v1::users::login,
        crate::web-backend::src::api::internal::permissions::get,
    ),
    components(
        schemas(
            crate::web-backend::src::api::v1::users::Username,
            crate::web-backend::src::api::v1::users::AuthenticationData,
            crate::web-backend::src::api::v1::users::RefreshTokenOptions,
            crate::web-backend::src::api::v1::users::RefreshTokenMethod,
            crate::web-backend::src::api::internal::permissions::PermissionInfo,
            crate::web-backend::src::api::internal::permissions::RoleInfo,
            crate::web-backend::src::permissions::Permission,
            crate::web-backend::src::permissions::Role,
        ),
        responses(
            crate::web-backend::src::api::v1::users::AuthenticationResponse,
            crate::web-backend::src::api::internal::permissions::PermissionsResponse,
        )
    ),
    modifiers(&SecurityAddon),
    info(version = "1"),
    tags(
        (name = "auth", description = "Endpoints for authentication and authorization."),
    ),
)]
pub struct ApiDoc;
```

I had added some more types in the meantime, but that seems reasonable and like what I would expect on first glance, except that the correct paths would be just `crate::XY` and not `crate::web-backend::src::XY`. The compiler probably can't deal with the hyphen in `web-backend` and thinks the path is over, so it wants a `,` to transition to the next path!

Are you generating these based on the `paths` that are given to the macro? Maybe there's a different way for me to specify where to search relative to the source file containing the macro 🤔 


Yeah I think this assumption:

https://github.com/ProbablyClem/utoipauto/blob/e2ca0d84228e32b2e032e38812b7e1e59d3b03a9/utoipauto-core/src/file_utils.rs#L84-L86

is not general enough. It's only true if the structure of your workspace is such that it directly contains all the crates. It doesn't work for me because the crates in the workspace are "grouped" into an intermediate level of folders, and it also wouldn't work for the thing people do sometimes where all the crates _are_ in the same folder, but it's not the workspace folder but `workspace/crates`.

The fully correct thing would probably be to actually resolve the `Cargo.toml` responsible for the file / module to determine the crate root. But I think wrangling the `paths` from the macro invocation is fine if instead of hardcoding `src` to have to appear as path segment 1, the logic was more like "skip until you hit `src` or `tests`, then convert the remaining file path to `crate::XYZ`".

Alternatively, maybe the library could use the "old" syntax to support this? If I read this correctly, the left-hand side of a `crate::module::foo => ./category/crate/src/module/foo` is currently ignored. But you could also use that information to take the discovered paths relative to the file path and append them to the module path on the left that the user has given (modulo `mod.rs` being on the same level).
I would rather not bring back the old syntax (that comes from the forked repo), since it's really annoying to have to specify the file paths and the source paths.
Obviously, we might found a case someday where we absolutely need it, but I would like to keep it last resort.

I have a lot of work to do currently, so feel free to submit a PR if you can.
Thanks for your help



Fix patch:
diff --git a/utoipauto-core/Cargo.lock b/utoipauto-core/Cargo.lock
index eb138c2..8c2db81 100644
--- a/utoipauto-core/Cargo.lock
+++ b/utoipauto-core/Cargo.lock
@@ -163,7 +163,7 @@ dependencies = [
 
 [[package]]
 name = "utoipauto-core"
-version = "0.1.2"
+version = "0.1.4"
 dependencies = [
  "proc-macro2",
  "quote",
diff --git a/utoipauto-core/src/attribute_utils.rs b/utoipauto-core/src/attribute_utils.rs
index db61212..0a8bcc7 100644
--- a/utoipauto-core/src/attribute_utils.rs
+++ b/utoipauto-core/src/attribute_utils.rs
@@ -8,18 +8,16 @@ pub fn update_openapi_macro_attributes(
     uto_reponses: &String,
 ) {
     let mut is_ok = false;
-    #[warn(clippy::needless_range_loop)]
-    for i in 0..macro_attibutes.len() {
-        if !macro_attibutes[i].path().is_ident("openapi") {
+    for attr in macro_attibutes {
+        if !attr.path().is_ident("openapi") {
             continue;
         }
         is_ok = true;
-        let mut src_uto_macro = macro_attibutes[i].to_token_stream().to_string();
+        let mut src_uto_macro = attr.to_token_stream().to_string();
 
         src_uto_macro = src_uto_macro.replace("#[openapi(", "");
         src_uto_macro = src_uto_macro.replace(")]", "");
-        macro_attibutes[i] =
-            build_new_openapi_attributes(src_uto_macro, uto_paths, uto_models, uto_reponses);
+        *attr = build_new_openapi_attributes(src_uto_macro, uto_paths, uto_models, uto_reponses);
     }
     if !is_ok {
         panic!("No utoipa::openapi Macro found !");
@@ -58,7 +56,7 @@ pub fn build_new_openapi_attributes(
 fn remove_paths(src_uto_macro: String) -> String {
     if src_uto_macro.contains("paths(") {
         let paths = src_uto_macro.split("paths(").collect::<Vec<&str>>()[1];
-        let paths = paths.split(")").collect::<Vec<&str>>()[0];
+        let paths = paths.split(')').collect::<Vec<&str>>()[0];
         src_uto_macro
             .replace(format!("paths({})", paths).as_str(), "")
             .replace(",,", ",")
@@ -70,7 +68,7 @@ fn remove_paths(src_uto_macro: String) -> String {
 fn remove_schemas(src_uto_macro: String) -> String {
     if src_uto_macro.contains("schemas(") {
         let schemas = src_uto_macro.split("schemas(").collect::<Vec<&str>>()[1];
-        let schemas = schemas.split(")").collect::<Vec<&str>>()[0];
+        let schemas = schemas.split(')').collect::<Vec<&str>>()[0];
         src_uto_macro
             .replace(format!("schemas({})", schemas).as_str(), "")
             .replace(",,", ",")
@@ -82,7 +80,7 @@ fn remove_schemas(src_uto_macro: String) -> String {
 fn remove_components(src_uto_macro: String) -> String {
     if src_uto_macro.contains("components(") {
         let components = src_uto_macro.split("components(").collect::<Vec<&str>>()[1];
-        let components = components.split(")").collect::<Vec<&str>>()[0];
+        let components = components.split(')').collect::<Vec<&str>>()[0];
         src_uto_macro
             .replace(format!("components({})", components).as_str(), "")
             .replace(",,", ",")
@@ -94,7 +92,7 @@ fn remove_components(src_uto_macro: String) -> String {
 fn remove_responses(src_uto_macro: String) -> String {
     if src_uto_macro.contains("responses(") {
         let responses = src_uto_macro.split("responses(").collect::<Vec<&str>>()[1];
-        let responses = responses.split(")").collect::<Vec<&str>>()[0];
+        let responses = responses.split(')').collect::<Vec<&str>>()[0];
         src_uto_macro
             .replace(format!("responses({})", responses).as_str(), "")
             .replace(",,", ",")
@@ -106,7 +104,7 @@ fn remove_responses(src_uto_macro: String) -> String {
 fn extract_paths(src_uto_macro: String) -> String {
     if src_uto_macro.contains("paths(") {
         let paths = src_uto_macro.split("paths(").collect::<Vec<&str>>()[1];
-        let paths = paths.split(")").collect::<Vec<&str>>()[0];
+        let paths = paths.split(')').collect::<Vec<&str>>()[0];
         paths.to_string()
     } else {
         "".to_string()
@@ -116,7 +114,7 @@ fn extract_paths(src_uto_macro: String) -> String {
 fn extract_schemas(src_uto_macro: String) -> String {
     if src_uto_macro.contains("schemas(") {
         let schemas = src_uto_macro.split("schemas(").collect::<Vec<&str>>()[1];
-        let schemas = schemas.split(")").collect::<Vec<&str>>()[0];
+        let schemas = schemas.split(')').collect::<Vec<&str>>()[0];
         schemas.to_string()
     } else {
         "".to_string()
@@ -126,7 +124,7 @@ fn extract_schemas(src_uto_macro: String) -> String {
 fn extract_responses(src_uto_macro: String) -> String {
     if src_uto_macro.contains("responses(") {
         let responses = src_uto_macro.split("responses(").collect::<Vec<&str>>()[1];
-        let responses = responses.split(")").collect::<Vec<&str>>()[0];
+        let responses = responses.split(')').collect::<Vec<&str>>()[0];
         responses.to_string()
     } else {
         "".to_string()
@@ -168,7 +166,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src),components(schemas(),responses()),)]".to_string()
         );
     }
@@ -184,7 +182,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(),responses()),)]".to_string()
         );
     }
@@ -200,7 +198,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(model),responses()),)]".to_string()
         );
     }
@@ -216,7 +214,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(model,m1),responses()),)]".to_string()
         );
     }
@@ -232,7 +230,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(),responses(response,r1)),)]".to_string()
         );
     }
@@ -248,7 +246,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(model,m1),responses(response,r1)),)]"
                 .to_string()
         );
@@ -265,7 +263,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(m1),responses(response,r1)),)]"
                 .to_string()
         );
@@ -282,7 +280,7 @@ mod test {
             )
             .to_token_stream()
             .to_string()
-            .replace(" ", ""),
+            .replace(' ', ""),
             "#[openapi(paths(./src,p1),components(schemas(model,m1),responses(r1)),)]".to_string()
         );
     }
diff --git a/utoipauto-core/src/discover.rs b/utoipauto-core/src/discover.rs
index c0b729c..5cb10ef 100644
--- a/utoipauto-core/src/discover.rs
+++ b/utoipauto-core/src/discover.rs
@@ -41,14 +41,14 @@ enum DiscoverType {
     Response(String),
 }
 
-fn parse_module_items(module_path: &String, items: Vec<Item>) -> Vec<DiscoverType> {
+fn parse_module_items(module_path: &str, items: Vec<Item>) -> Vec<DiscoverType> {
     items
         .into_iter()
-        .filter(|e| match e {
-            syn::Item::Mod(_) | syn::Item::Fn(_) | syn::Item::Struct(_) | syn::Item::Enum(_) => {
-                true
-            }
-            _ => false,
+        .filter(|e| {
+            matches!(
+                e,
+                syn::Item::Mod(_) | syn::Item::Fn(_) | syn::Item::Struct(_) | syn::Item::Enum(_)
+            )
         })
         .map(|v| match v {
             syn::Item::Mod(m) => m.content.map_or(Vec::<DiscoverType>::new(), |cs| {
@@ -56,13 +56,13 @@ fn parse_module_items(module_path: &String, items: Vec<Item>) -> Vec<DiscoverTyp
             }),
             syn::Item::Fn(f) => parse_function(&f)
                 .into_iter()
-                .map(|item| DiscoverType::Fn(build_path(&module_path, &item)))
+                .map(|item| DiscoverType::Fn(build_path(module_path, &item)))
                 .collect(),
             syn::Item::Struct(s) => {
-                parse_from_attr(&s.attrs, &build_path(&module_path, &s.ident.to_string()))
+                parse_from_attr(&s.attrs, &build_path(module_path, &s.ident.to_string()))
             }
             syn::Item::Enum(e) => {
-                parse_from_attr(&e.attrs, &build_path(&module_path, &e.ident.to_string()))
+                parse_from_attr(&e.attrs, &build_path(module_path, &e.ident.to_string()))
             }
             _ => vec![],
         })
@@ -73,7 +73,7 @@ fn parse_module_items(module_path: &String, items: Vec<Item>) -> Vec<DiscoverTyp
 }
 
 /// Search for ToSchema and ToResponse implementations in attr
-fn parse_from_attr(a: &Vec<Attribute>, name: &String) -> Vec<DiscoverType> {
+fn parse_from_attr(a: &Vec<Attribute>, name: &str) -> Vec<DiscoverType> {
     let mut out: Vec<DiscoverType> = vec![];
 
     for attr in a {
@@ -87,10 +87,10 @@ fn parse_from_attr(a: &Vec<Attribute>, name: &String) -> Vec<DiscoverType> {
                 .unwrap();
             for nested_meta in nested {
                 if nested_meta.path().is_ident("ToSchema") {
-                    out.push(DiscoverType::Model(name.clone()));
+                    out.push(DiscoverType::Model(name.to_string()));
                 }
                 if nested_meta.path().is_ident("ToResponse") {
-                    out.push(DiscoverType::Response(name.clone()));
+                    out.push(DiscoverType::Response(name.to_string()));
                 }
             }
         }
@@ -131,6 +131,6 @@ fn is_ignored(f: &ItemFn) -> bool {
     })
 }
 
-fn build_path(file_name: &String, fn_name: &String) -> String {
+fn build_path(file_name: &str, fn_name: &str) -> String {
     format!("{}::{}", file_name, fn_name)
 }
diff --git a/utoipauto-core/src/file_utils.rs b/utoipauto-core/src/file_utils.rs
index 34351df..f49d9da 100644
--- a/utoipauto-core/src/file_utils.rs
+++ b/utoipauto-core/src/file_utils.rs
@@ -1,7 +1,8 @@
 use std::{
     fs::{self, File},
     io::{self, Read},
-    path::PathBuf,
+    iter,
+    path::{Path, PathBuf},
 };
 
 pub fn parse_file<T: Into<PathBuf>>(filepath: T) -> Result<syn::File, io::Error> {
@@ -42,7 +43,7 @@ pub fn parse_files<T: Into<PathBuf>>(path: T) -> Result<Vec<(String, syn::File)>
     Ok(files)
 }
 
-fn is_rust_file(path: &PathBuf) -> bool {
+fn is_rust_file(path: &Path) -> bool {
     path.is_file() && path.extension().unwrap().to_str().unwrap().eq("rs")
 }
 
@@ -55,39 +56,33 @@ fn is_rust_file(path: &PathBuf) -> bool {
 /// );
 /// assert_eq!(
 ///  module_name,
-/// "crate::tests::controllers::controller1".to_string()
+/// "crate::controllers::controller1".to_string()
 /// );
 /// ```
-pub fn extract_module_name_from_path(path: &String) -> String {
-    let mut path = path.to_string();
-    if path.ends_with(".rs") {
-        path = path.replace(".rs", "");
-    }
-    if path.ends_with("/mod") {
-        path = path.replace("/mod", "");
-    }
-    if path.ends_with("/lib") {
-        path = path.replace("/lib", "");
-    }
-    if path.ends_with("/main") {
-        path = path.replace("/main", "");
-    }
-    path = path.replace("./", "");
-    //remove first word
-    let path_vec = path
-        .split('/')
-        .enumerate()
-        .filter_map(|(idx, segment)| match (idx, segment) {
-            // Remove first segment and replace with `crate`, so that 'root/thing' becomes
-            // `crate::thing` in the end
-            (0, _) => Some("crate"),
-            // When using cargo workspaces, paths look like  './subcrate/src/my/module',
-            // so we need to remove the 'src' to produce `crate::my::module`
-            (1, "src") => None,
-            (_, segment) => Some(segment),
-        })
-        .collect::<Vec<&str>>();
-    path_vec.join("::")
+pub fn extract_module_name_from_path(path: &str) -> String {
+    let path = path
+        .trim_end_matches(".rs")
+        .trim_end_matches("/mod")
+        .trim_end_matches("/lib")
+        .trim_end_matches("/main")
+        .trim_start_matches("./");
+    let segments: Vec<_> = path.split('/').collect();
+
+    // In general, paths will look like `./src/my/module`, which should turn into `crate::my::module`.
+    // When using cargo workspaces, paths may look like `./subcrate/src/my/module`,
+    // `./crates/subcrate/src/my/module`, etc., so we need to remove anything up to `src`
+    // (or `tests`) to still produce `crate::my::module`.
+    let segments_inside_crate = match segments
+        .iter()
+        .position(|&segment| segment == "src" || segment == "tests")
+    {
+        Some(idx) => &segments[(idx + 1)..],
+        None => &segments,
+    };
+    let full_crate_path: Vec<_> = iter::once("crate")
+        .chain(segments_inside_crate.iter().copied())
+        .collect();
+    full_crate_path.join("::")
 }
 
 #[cfg(test)]
@@ -97,43 +92,41 @@ mod tests {
     #[test]
     fn test_extract_module_name_from_path() {
         assert_eq!(
-            extract_module_name_from_path(
-                &"./utoipa-auto-macro/tests/controllers/controller1.rs".to_string()
-            ),
-            "crate::tests::controllers::controller1"
+            extract_module_name_from_path("./utoipa-auto-macro/tests/controllers/controller1.rs"),
+            "crate::controllers::controller1"
         );
     }
 
     #[test]
     fn test_extract_module_name_from_mod() {
         assert_eq!(
-            extract_module_name_from_path(
-                &"./utoipa-auto-macro/tests/controllers/mod.rs".to_string()
-            ),
-            "crate::tests::controllers"
+            extract_module_name_from_path("./utoipa-auto-macro/tests/controllers/mod.rs"),
+            "crate::controllers"
         );
     }
 
     #[test]
     fn test_extract_module_name_from_lib() {
-        assert_eq!(
-            extract_module_name_from_path(&"./src/lib.rs".to_string()),
-            "crate"
-        );
+        assert_eq!(extract_module_name_from_path("./src/lib.rs"), "crate");
     }
 
     #[test]
     fn test_extract_module_name_from_main() {
+        assert_eq!(extract_module_name_from_path("./src/main.rs"), "crate");
+    }
+
+    #[test]
+    fn test_extract_module_name_from_workspace() {
         assert_eq!(
-            extract_module_name_from_path(&"./src/main.rs".to_string()),
-            "crate"
+            extract_module_name_from_path("./server/src/routes/asset.rs"),
+            "crate::routes::asset"
         );
     }
 
     #[test]
-    fn test_extract_module_name_from_workspace() {
+    fn test_extract_module_name_from_workspace_nested() {
         assert_eq!(
-            extract_module_name_from_path(&"./server/src/routes/asset.rs".to_string()),
+            extract_module_name_from_path("./crates/server/src/routes/asset.rs"),
             "crate::routes::asset"
         );
     }
@@ -141,7 +134,7 @@ mod tests {
     #[test]
     fn test_extract_module_name_from_folders() {
         assert_eq!(
-            extract_module_name_from_path(&"./src/routing/api/audio.rs".to_string()),
+            extract_module_name_from_path("./src/routing/api/audio.rs"),
             "crate::routing::api::audio"
         );
     }
diff --git a/utoipauto-core/src/string_utils.rs b/utoipauto-core/src/string_utils.rs
index 8b16655..b004d29 100644
--- a/utoipauto-core/src/string_utils.rs
+++ b/utoipauto-core/src/string_utils.rs
@@ -43,17 +43,16 @@ pub fn trim_parentheses(str: &str) -> String {
 /// );
 /// ```
 pub fn extract_paths(attributes: String) -> Vec<String> {
-    let paths;
-    let attributes = trim_parentheses(rem_first_and_last(&attributes.as_str()));
+    let attributes = trim_parentheses(rem_first_and_last(&attributes));
 
     if attributes.contains('|') {
         panic!("Please use the new syntax ! paths=\"(MODULE_TREE_PATH => MODULE_SRC_PATH) ;\"")
     }
-    if attributes.contains("=>") {
-        paths = extract_paths_arrow(attributes);
+    let paths = if attributes.contains("=>") {
+        extract_paths_arrow(attributes)
     } else {
-        paths = extract_paths_coma(attributes);
-    }
+        extract_paths_coma(attributes)
+    };
     if paths.is_empty() {
         panic!("utoipauto: No paths specified !")
     }


Test patch: