Handle boolean params with false default values (#1352)

luckyframework · Dec 7, 2020 · 49f1021 · 49f1021
1 parent f9f45e8
commit 49f1021
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 6 deletions.
diff --git a/spec/lucky/action_spec.cr b/spec/lucky/action_spec.cr
@@ -102,9 +102,11 @@ end
 
 class RequiredParams::Index < TestAction
   param required_page : Int32
+  # This is to test that the default value of 'false' is not treated as 'nil'
+  param bool_with_false_default : Bool = false
 
   route do
-    plain_text "required param: #{required_page}"
+    plain_text "required param: #{required_page} #{bool_with_false_default}"
   end
 end
 
@@ -268,8 +270,9 @@ describe Lucky::Action do
     end
 
     it "returns required param declarations" do
-      RequiredParams::Index.query_param_declarations.size.should eq 1
-      RequiredParams::Index.query_param_declarations.first.should eq "required_page : Int32"
+      RequiredParams::Index.query_param_declarations.size.should eq 2
+      RequiredParams::Index.query_param_declarations.should contain "required_page : Int32"
+      RequiredParams::Index.query_param_declarations.should contain "bool_with_false_default : Bool"
     end
 
     it "returns optional param declarations" do
@@ -291,6 +294,7 @@ describe Lucky::Action do
 
     it "adds named arguments to the path" do
       RequiredParams::Index.path(required_page: 7).should eq "/required_params?required_page=7"
+      RequiredParams::Index.path(required_page: 7, bool_with_false_default: true).should eq "/required_params?required_page=7&bool_with_false_default=true"
     end
 
     it "adds named arguments to the route" do

diff --git a/src/lucky/routable.cr b/src/lucky/routable.cr
@@ -275,7 +275,7 @@ module Lucky::Routable
     {% end %}
 
     {% params_with_defaults = PARAM_DECLARATIONS.select do |decl|
-         decl.value || decl.type.is_a?(Union) && decl.type.types.last.id == Nil.id
+         !decl.value.is_a?(Nop) || decl.type.is_a?(Union) && decl.type.types.last.id == Nil.id
        end %}
     {% params_without_defaults = PARAM_DECLARATIONS.reject do |decl|
          params_with_defaults.includes? decl
@@ -446,12 +446,15 @@ module Lucky::Routable
       val = params.get?(:{{ type_declaration.var.id }})
 
       if val.nil?
-        default_or_nil = {{ type_declaration.value || nil }}
+        default_or_nil = {{ type_declaration.value.is_a?(Nop) ? nil : type_declaration.value }}
         {% if is_nilable_type %}
           return default_or_nil
         {% else %}
-          return default_or_nil ||
+          if default_or_nil.nil?
             raise Lucky::MissingParamError.new("{{ type_declaration.var.id }}")
+          else
+            return default_or_nil
+          end
         {% end %}
       end