feat(derive): Support arrays as number_of_values

clean up Support optional fixed array Remove `number_of_occurrences` currently Add tests for deriving fixed array Add tests for deriving positional fixed array arguments Add ui test Fix clippy Put fixed array support under feature flag Fix makefile
clap-rs · Apr 30, 2022 · 03246d2 · 03246d2
1 parent 3ca1b77
commit 03246d2
Show file tree

Hide file tree

Showing 17 changed files with 390 additions and 10 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -112,6 +112,10 @@ jobs:
   ui:
     name: UI Tests
     runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        features: [default, next]
     steps:
     - name: Checkout repository
       uses: actions/checkout@v2
@@ -123,7 +127,7 @@ jobs:
         override: true
     - uses: Swatinem/rust-cache@v1
     - name: UI Tests
-      run: make test-ui
+      run: make test-ui-${{ matrix.features }}
   docs:
     name: Docs
     runs-on: ubuntu-latest

diff --git a/Cargo.toml b/Cargo.toml
@@ -82,7 +82,8 @@ unicode = ["textwrap/unicode-width", "unicase"]  # Support for unicode character
 unstable-replace = []
 unstable-multicall = []
 unstable-grouped = []
-unstable-v4 = []
+# note: this will always enable clap_derive, change this to `clap_derive?/unstable-v4` when MSRV is bigger than 1.60
+unstable-v4 = ["clap_derive/unstable-v4"]
 
 [lib]
 bench = false

diff --git a/Makefile b/Makefile
@@ -33,5 +33,5 @@ test-%:
 clippy-%:
 	cargo clippy ${_FEATURES_${@:clippy-%=%}} ${ARGS} --all-targets -- -D warnings -A deprecated
 
-test-ui:
-	cargo +${MSRV} test --test derive_ui --features derive
+test-ui-%:
+	cargo +${MSRV} test --test derive_ui --features derive ${_FEATURES_${@:test-ui-%=%}}
diff --git a/clap_derive/Cargo.toml b/clap_derive/Cargo.toml
@@ -44,6 +44,8 @@ proc-macro-error = "1"
 [features]
 default = []
 debug = []
+unstable-v4 = ["unstable-array"]
+unstable-array = []
 
 [package.metadata.docs.rs]
 targets = ["x86_64-unknown-linux-gnu"]
diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs
@@ -159,6 +159,12 @@ impl Attrs {
                             "Option<Vec<T>> type is not allowed for subcommand"
                         );
                     }
+                    Ty::OptionArray => {
+                        abort!(
+                            field_ty,
+                            "Option<[T; N]> type is not allowed for subcommand"
+                        );
+                    }
                     _ => (),
                 }
 
@@ -277,6 +283,12 @@ impl Attrs {
                             "Option<Vec<T>> type is not allowed for subcommand"
                         );
                     }
+                    Ty::OptionArray => {
+                        abort!(
+                            field.ty,
+                            "Option<[T; N]> type is not allowed for subcommand"
+                        );
+                    }
                     _ => (),
                 }
 
@@ -298,7 +310,7 @@ impl Attrs {
                 let mut ty = Ty::from_syn_ty(&field.ty);
                 if res.has_custom_parser {
                     match *ty {
-                        Ty::Option | Ty::Vec | Ty::OptionVec => (),
+                        Ty::Option | Ty::Vec | Ty::Array | Ty::OptionVec | Ty::OptionArray => (),
                         _ => ty = Sp::new(Ty::Other, ty.span()),
                     }
                 }
@@ -345,7 +357,14 @@ impl Attrs {
                             )
                         }
                     }
-
+                    Ty::OptionArray => {
+                        if res.is_positional() {
+                            abort!(
+                                field.ty,
+                                "Option<[T; N]> type is meaningless for positional argument"
+                            )
+                        }
+                    }
                     _ => (),
                 }
                 res.kind = Sp::new(Kind::Arg(ty), orig_ty.span());

diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
@@ -15,7 +15,7 @@
 use crate::{
     attrs::{Attrs, Kind, Name, ParserKind, DEFAULT_CASING, DEFAULT_ENV_CASING},
     dummies,
-    utils::{inner_type, sub_type, Sp, Ty},
+    utils::{array_ty_len, inner_type, sub_type, Sp, Ty},
 };
 
 use proc_macro2::{Ident, Span, TokenStream};
@@ -310,6 +310,18 @@ pub fn gen_augment(
                         #allow_invalid_utf8
                     },
 
+                    Ty::OptionArray => {
+                        let len = array_ty_len(sub_type(&field.ty).unwrap()).unwrap();
+                        quote_spanned! { ty.span()=>
+                            .takes_value(true)
+                            .value_name(#value_name)
+                            .number_of_values(#len)
+                            #possible_values
+                            #validator
+                            #allow_invalid_utf8
+                        }
+                    }
+
                     Ty::Vec => {
                         quote_spanned! { ty.span()=>
                             .takes_value(true)
@@ -321,6 +333,19 @@ pub fn gen_augment(
                         }
                     }
 
+                    Ty::Array => {
+                        let len = array_ty_len(&field.ty).unwrap();
+                        quote_spanned! { ty.span()=>
+                            .takes_value(true)
+                            .value_name(#value_name)
+                            .required(true)
+                            .number_of_values(#len)
+                            #possible_values
+                            #validator
+                            #allow_invalid_utf8
+                        }
+                    }
+
                     Ty::Other if occurrences => quote_spanned! { ty.span()=>
                         .multiple_occurrences(true)
                     },
@@ -612,6 +637,21 @@ fn gen_parsers(
             }
         },
 
+        Ty::OptionArray => quote_spanned! { ty.span()=>
+            if #arg_matches.is_present(#name) {
+                Some(
+                    std::convert::TryInto::try_into(
+                        #arg_matches.#values_of(#name)
+                            .map(|v| v.map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
+                            .transpose()?
+                            .unwrap_or_else(Vec::new)
+                    ).unwrap()
+                )
+            } else {
+                None
+            }
+        },
+
         Ty::Vec => {
             quote_spanned! { ty.span()=>
                 #arg_matches.#values_of(#name)
@@ -621,6 +661,17 @@ fn gen_parsers(
             }
         }
 
+        Ty::Array => {
+            quote_spanned! { ty.span()=>
+                std::convert::TryInto::try_into(
+                    #arg_matches.#values_of(#name)
+                        .map(|v| v.map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
+                        .transpose()?
+                        .unwrap_or_else(Vec::new)
+                ).unwrap()
+            }
+        }
+
         Ty::Other if occurrences => quote_spanned! { ty.span()=>
             #parse(#arg_matches.#value_of(#name))
         },

diff --git a/clap_derive/src/utils/mod.rs b/clap_derive/src/utils/mod.rs
@@ -5,5 +5,5 @@ mod ty;
 pub use self::{
     doc_comments::process_doc_comment,
     spanned::Sp,
-    ty::{inner_type, is_simple_ty, sub_type, subty_if_name, Ty},
+    ty::{array_ty_len, inner_type, is_simple_ty, sub_type, subty_if_name, Ty},
 };
diff --git a/clap_derive/src/utils/ty.rs b/clap_derive/src/utils/ty.rs
@@ -3,17 +3,19 @@
 use super::spanned::Sp;
 
 use syn::{
-    spanned::Spanned, GenericArgument, Path, PathArguments, PathArguments::AngleBracketed,
-    PathSegment, Type, TypePath,
+    spanned::Spanned, Expr, GenericArgument, Path, PathArguments, PathArguments::AngleBracketed,
+    PathSegment, Type, TypeArray, TypePath,
 };
 
 #[derive(Copy, Clone, PartialEq, Debug)]
 pub enum Ty {
     Bool,
     Vec,
+    Array,
     Option,
     OptionOption,
     OptionVec,
+    OptionArray,
     Other,
 }
 
@@ -26,11 +28,15 @@ impl Ty {
             t(Bool)
         } else if is_generic_ty(ty, "Vec") {
             t(Vec)
+        } else if cfg!(feature = "unstable-array") && is_array_ty(ty) {
+            t(Array)
         } else if let Some(subty) = subty_if_name(ty, "Option") {
             if is_generic_ty(subty, "Option") {
                 t(OptionOption)
             } else if is_generic_ty(subty, "Vec") {
                 t(OptionVec)
+            } else if cfg!(feature = "unstable-array") && is_array_ty(subty) {
+                t(OptionArray)
             } else {
                 t(Option)
             }
@@ -43,9 +49,13 @@ impl Ty {
 pub fn inner_type(ty: Ty, field_ty: &syn::Type) -> &syn::Type {
     match ty {
         Ty::Vec | Ty::Option => sub_type(field_ty).unwrap_or(field_ty),
+        Ty::Array => array_ty_type(field_ty).unwrap_or(field_ty),
         Ty::OptionOption | Ty::OptionVec => {
             sub_type(field_ty).and_then(sub_type).unwrap_or(field_ty)
         }
+        Ty::OptionArray => sub_type(field_ty)
+            .and_then(array_ty_type)
+            .unwrap_or(field_ty),
         _ => field_ty,
     }
 }
@@ -113,6 +123,25 @@ fn is_generic_ty(ty: &syn::Type, name: &str) -> bool {
     subty_if_name(ty, name).is_some()
 }
 
+fn is_array_ty(ty: &Type) -> bool {
+    matches!(ty, Type::Array(TypeArray { .. }))
+}
+
+fn array_ty_type(ty: &syn::Type) -> Option<&syn::Type> {
+    if let Type::Array(TypeArray { elem, .. }) = ty {
+        Some(&*elem)
+    } else {
+        None
+    }
+}
+
+pub fn array_ty_len(ty: &Type) -> Option<&Expr> {
+    match ty {
+        Type::Array(TypeArray { len, .. }) => Some(len),
+        _ => None,
+    }
+}
+
 fn only_one<I, T>(mut iter: I) -> Option<T>
 where
     I: Iterator<Item = T>,