Code cleanups (#1828)

* Bump LuckyTask

* Just some return types added

shard.yml

@@ -29,7 +29,7 @@ targets:
 dependencies:
   lucky_task:
     github: luckyframework/lucky_task
-    version: ~> 0.1.1
+    version: ~> 0.2.0
   habitat:
     github: luckyframework/habitat
     version: ~> 0.4.7

src/lucky/action.cr

@@ -1,7 +1,8 @@
 require "./*"
 
 abstract class Lucky::Action
-  getter :context, :route_params
+  getter context : HTTP::Server::Context
+  getter route_params : Hash(String, String)
 
   def initialize(@context : HTTP::Server::Context, @route_params : Hash(String, String))
     context.params.route_params = @route_params

src/lucky/base_http_client.cr

@@ -5,14 +5,12 @@ require "http/client"
 # Makes it easy to pass params, use Lucky route helpers, and chain header methods.
 abstract class Lucky::BaseHTTPClient
   @@app : Lucky::BaseAppServer?
-  private getter client
-
-  @client : HTTP::Client
+  private getter client : HTTP::Client
 
   def self.app(@@app : Lucky::BaseAppServer)
   end
 
-  def initialize(@client = build_client)
+  def initialize(@client : HTTP::Client = build_client)
   end
 
   private def build_client : HTTP::Client

src/lucky/json_body_parser.cr

@@ -3,7 +3,7 @@ class Lucky::JsonBodyParser
   getter body : String
   getter request : HTTP::Request
 
-  def initialize(@body, @request)
+  def initialize(@body : String, @request : HTTP::Request)
   end
 
   def parsed_json : JSON::Any

src/lucky/mime_type.cr

@@ -5,7 +5,8 @@ class Lucky::MimeType
   class_getter accept_header_formats = {} of MediaType => Format
 
   struct MediaType
-    property type, subtype
+    property subtype : String
+    property type : String
 
     def initialize(@type : String, @subtype : String)
     end
@@ -103,7 +104,7 @@ class Lucky::MimeType
   end
 
   class AcceptList
-    getter list
+    getter list : Array(MediaRange)
 
     ACCEPT_SEP = /[ \t]*,[ \t]*/
 
@@ -226,11 +227,11 @@ class Lucky::MimeType
       @type == "*" || (@type == media.type && self.class.match_type?(@subtype, media.subtype))
     end
 
-    def catch_all?
+    def catch_all? : Bool
       @type == "*" && @subtype == "*"
     end
 
-    protected def self.match_type?(pattern, value)
+    protected def self.match_type?(pattern, value) : Bool
       pattern == "*" || pattern == value
     end
   end

src/lucky/redirectable.cr

@@ -29,7 +29,7 @@ module Lucky::Redirectable
   # ```
   # redirect_back fallback: Users::Index
   # ```
-  def redirect_back(*, fallback : Lucky::Action.class, status = 302, allow_external = false)
+  def redirect_back(*, fallback : Lucky::Action.class, status = 302, allow_external = false) : Lucky::TextResponse
     redirect_back fallback: fallback.route, status: status, allow_external: allow_external
   end
 
@@ -38,7 +38,7 @@ module Lucky::Redirectable
   # ```
   # redirect_back fallback: Users::Show.with(user.id)
   # ```
-  def redirect_back(*, fallback : Lucky::RouteHelper, status = 302, allow_external = false)
+  def redirect_back(*, fallback : Lucky::RouteHelper, status = 302, allow_external = false) : Lucky::TextResponse
     redirect_back fallback: fallback.path, status: status, allow_external: allow_external
   end
 
@@ -47,7 +47,7 @@ module Lucky::Redirectable
   # ```
   # redirect_back fallback: "/users", status: HTTP::Status::MOVED_PERMANENTLY
   # ```
-  def redirect_back(*, fallback : String, status : HTTP::Status, allow_external = false)
+  def redirect_back(*, fallback : String, status : HTTP::Status, allow_external = false) : Lucky::TextResponse
     redirect_back fallback: fallback, status: status.value, allow_external: allow_external
   end
 
@@ -74,7 +74,7 @@ module Lucky::Redirectable
   # They can be explicitly allowed if necessary
   #
   # redirect_back fallback: "/home", allow_external: true
-  def redirect_back(*, fallback : String, status : Int32 = 302, allow_external : Bool = false)
+  def redirect_back(*, fallback : String, status : Int32 = 302, allow_external : Bool = false) : Lucky::TextResponse
     referer = request.headers["Referer"]?
 
     if referer && (allow_external || allowed_host?(referer))

src/lucky/routable.cr

@@ -222,7 +222,7 @@ module Lucky::Routable
     {% for param in optional_path_params %}
       {{ param.gsub(/^\?:/, "").id }} = nil,
     {% end %}
-    )
+    ) : String
       path = path_from_parts(
         {% for param in path_params %}
           {{ param.gsub(/:/, "").id }},
@@ -241,7 +241,7 @@ module Lucky::Routable
     {% for param in optional_path_params %}
       {{ param.gsub(/^\?:/, "").id }} = nil,
     {% end %}
-    )
+    ) : String
       path = path_from_parts(
         {% for param in path_params %}
           {{ param.gsub(/:/, "").id }},
@@ -281,7 +281,7 @@ module Lucky::Routable
       {{ param.gsub(/^\?:/, "").id }} = nil,
     {% end %}
     anchor : String? = nil
-      ) : Lucky::RouteHelper
+    ) : Lucky::RouteHelper
       path = path_from_parts(
         {% for param in path_params %}
           {{ param.gsub(/:/, "").id }},
@@ -328,7 +328,7 @@ module Lucky::Routable
         {{ param.gsub(/^\?:/, "").id }} = nil,
       {% end %}
       anchor : String? = nil
-        ) : Lucky::RouteHelper
+    ) : Lucky::RouteHelper
       \{% begin %}
       route(
         \{% for arg in @def.args %}
@@ -346,7 +346,7 @@ module Lucky::Routable
         {% for param in optional_path_params %}
           {{ param.gsub(/^\?:/, "").id }},
         {% end %}
-    )
+    ) : String
       path = String.build do |path|
         {% for part in path_parts %}
           {% if part.starts_with?("?:") %}

src/lucky/route_helper.cr

@@ -1,10 +1,11 @@
 class Lucky::RouteHelper
-  getter path, method
-
   Habitat.create do
     setting base_uri : String
   end
 
+  getter method : Symbol
+  getter path : String
+
   def initialize(@method : Symbol, @path : String)
   end
 

src/lucky/route_inferrer.cr

@@ -1,17 +1,17 @@
 require "wordsmith"
 
 class Lucky::RouteInferrer
-  getter? nested_route
-  getter action_class_name
+  getter? nested_route : Bool
+  getter action_class_name : String
 
   def initialize(@action_class_name : String, @nested_route : Bool = false)
   end
 
-  def generate_inferred_route
+  def generate_inferred_route : String
     %(#{http_method} "#{path}")
   end
 
-  private def http_method
+  private def http_method : Symbol
     case action_name
     when "delete"
       :delete
@@ -24,23 +24,23 @@ class Lucky::RouteInferrer
     end
   end
 
-  private def path
-    "/" + all_pieces.join("/")
+  private def path : String
+    '/' + all_pieces.join('/')
   end
 
-  private def all_pieces
+  private def all_pieces : Array(String)
     (namespace_pieces + parent_resource_pieces + resource_pieces).reject(&.empty?)
   end
 
-  private def resource
+  private def resource : String
     action_pieces[-2]
   end
 
-  private def action_name
+  private def action_name : String
     action_pieces.last
   end
 
-  private def namespace_pieces
+  private def namespace_pieces : Array(String)
     _namespace_pieces = action_pieces.reject { |piece| piece == action_name || piece == resource }
     if nested_route?
       _namespace_pieces.reject { |piece| piece == parent_resource_name }
@@ -49,7 +49,7 @@ class Lucky::RouteInferrer
     end
   end
 
-  private def resource_pieces
+  private def resource_pieces : Array(String)
     case action_name
     when "index", "create"
       [resource]
@@ -64,7 +64,7 @@ class Lucky::RouteInferrer
     end
   end
 
-  private def resource_id_param_name
+  private def resource_id_param_name : String
     ":#{Wordsmith::Inflector.singularize(resource)}_id"
   end
 
@@ -82,7 +82,7 @@ class Lucky::RouteInferrer
     ERROR
   end
 
-  private def parent_resource_pieces
+  private def parent_resource_pieces : Array(String)
     if nested_route?
       singularized_param_name = ":#{Wordsmith::Inflector.singularize(parent_resource_name)}_id"
       [parent_resource_name, singularized_param_name]
@@ -91,11 +91,11 @@ class Lucky::RouteInferrer
     end
   end
 
-  private def parent_resource_name
+  private def parent_resource_name : String
     action_pieces[-3]
   end
 
-  private def action_pieces
+  private def action_pieces : Array(String)
     action_class_name.split("::").map(&.underscore)
   end
 end

src/lucky/server_settings.cr

@@ -36,7 +36,7 @@ module Lucky::ServerSettings
     end
   end
 
-  private def yaml_settings_file
+  private def yaml_settings_file : String
     if File.exists?(YAML_SETTINGS_PATH)
       File.read YAML_SETTINGS_PATH
     else

src/lucky/text_response.cr

@@ -38,13 +38,15 @@ class Lucky::TextResponse < Lucky::Response
     @status || context.response.status_code || DEFAULT_STATUS
   end
 
-  private def gzip
+  private def gzip : Nil
     context.response.headers["Content-Encoding"] = "gzip"
     context.response.output = Compress::Gzip::Writer.new(context.response.output, sync_close: true)
   end
 
-  private def should_gzip?
-    {% if !flag?(:without_zlib) %}
+  private def should_gzip? : Bool
+    {% if flag?(:without_zlib) %}
+      false
+    {% else %}
       Lucky::Server.settings.gzip_enabled &&
         context.request.headers.includes_word?("Accept-Encoding", "gzip") &&
         Lucky::Server.settings.gzip_content_types.includes?(content_type)

tasks/exec.cr

@@ -3,7 +3,7 @@ require "habitat"
 require "lucky_task"
 
 class Lucky::Exec < LuckyTask::Task
-  name "exec"
+  task_name "exec"
   summary "Execute code. Use this in place of a console/REPL"
   arg :editor, "Which editor to use", shortcut: "-e", optional: true
   arg :back, "Load code from this many sessions back. Default is 1.",
@@ -28,7 +28,7 @@ class Lucky::Exec < LuckyTask::Task
 
     example: lucky exec -e emacs -b 3 -o
 
-    Run this task with 'lucky #{name} [OPTIONS]'
+    Run this task with 'lucky #{task_name} [OPTIONS]'
     TEXT
   end