Code cleaning (#1771)

* updated ameba. Fixing ameba issues which now bork on not_nil calls.

* Adding a lot more return types

Author: jwoertink

shard.yml

@@ -61,7 +61,7 @@ dependencies:
 development_dependencies:
   ameba:
     github: crystal-ameba/ameba
-    version: ~> 1.1.0
+    version: ~> 1.3.0
 
 scripts:
   postinstall: BUILD_WITHOUT_DEVELOPMENT=true script/precompile_tasks

spec/charms/cookie_spec.cr

@@ -30,7 +30,7 @@ describe HTTP::Cookie do
     it "sets expiration 20 years from now" do
       cookie = test_cookie.permanent
 
-      cookie.expires.not_nil!.should be_close(20.years.from_now, 1.minute)
+      cookie.expires.as(Time).should be_close(20.years.from_now, 1.minute)
     end
   end
 end

spec/lucky/action_pipes_spec.cr

@@ -218,7 +218,7 @@ describe Lucky::Action do
         events << event
       end
       Pipes::HaltedBefore.new(build_context, params).call
-      halted_pipe = events.find { |e| e.name == "redirect_me" }.not_nil!
+      halted_pipe = events.find! { |e| e.name == "redirect_me" }
       halted_pipe.continued.should eq false
       halted_pipe.position.to_s.should eq "Before"
       halted_pipe.before?.should eq true
@@ -230,7 +230,7 @@ describe Lucky::Action do
         events << event
       end
       Pipes::HaltedAfter.new(build_context, params).call
-      halted_pipe = events.find { |e| e.name == "redirect_me" }.not_nil!
+      halted_pipe = events.find! { |e| e.name == "redirect_me" }
       halted_pipe.continued.should eq false
       halted_pipe.position.to_s.should eq "After"
       halted_pipe.after?.should eq true

spec/lucky/base_http_client_spec.cr

@@ -36,15 +36,15 @@ describe Lucky::BaseHTTPClient do
         request = TestServer.last_request
         request.path.should eq "/hello"
         request.method.should eq("POST")
-        request.body.not_nil!.gets_to_end.should eq("{}")
+        request.body.to_s.should eq("{}")
         response.body.should eq "world"
       end
 
       it "allows passing params" do
         response = MyClient.new.exec(HelloWorldAction, foo: "bar")
 
         request = TestServer.last_request
-        request.body.not_nil!.gets_to_end.should eq({foo: "bar"}.to_json)
+        request.body.to_s.should eq({foo: "bar"}.to_json)
       end
     end
 
@@ -55,15 +55,15 @@ describe Lucky::BaseHTTPClient do
         request = TestServer.last_request
         request.path.should eq "/hello"
         request.method.should eq("POST")
-        request.body.not_nil!.gets_to_end.should eq("{}")
+        request.body.to_s.should eq("{}")
         response.body.should eq "world"
       end
 
       it "allows passing params" do
         response = MyClient.new.exec(HelloWorldAction.route, foo: "bar")
 
         request = TestServer.last_request
-        request.body.not_nil!.gets_to_end.should eq({foo: "bar"}.to_json)
+        request.body.to_s.should eq({foo: "bar"}.to_json)
       end
     end
   end
@@ -79,7 +79,7 @@ describe Lucky::BaseHTTPClient do
         response = MyClient.new.exec_raw(HelloWorldAction, test_data)
 
         request = TestServer.last_request
-        request.body.not_nil!.gets_to_end.should eq(test_data)
+        request.body.to_s.should eq(test_data)
       end
     end
 
@@ -93,7 +93,7 @@ describe Lucky::BaseHTTPClient do
         response = MyClient.new.exec_raw(HelloWorldAction.route, test_data)
 
         request = TestServer.last_request
-        request.body.not_nil!.gets_to_end.should eq(test_data)
+        request.body.to_s.should eq(test_data)
       end
     end
   end
@@ -109,7 +109,7 @@ describe Lucky::BaseHTTPClient do
         request = TestServer.last_request
         request.method.should eq({{ method.id.stringify }}.upcase)
         request.path.should eq "hello"
-        request.body.not_nil!.gets_to_end.should eq({foo: "bar"}.to_json)
+        request.body.to_s.should eq({foo: "bar"}.to_json)
       end
 
       it "works without params" do
@@ -118,7 +118,7 @@ describe Lucky::BaseHTTPClient do
         request = TestServer.last_request
         request.method.should eq({{ method.id.stringify }}.upcase)
         request.path.should eq "hello"
-        request.body.not_nil!.gets_to_end.should eq("{}")
+        request.body.to_s.should eq("{}")
       end
     end
   {% end %}
@@ -140,7 +140,7 @@ describe Lucky::BaseHTTPClient do
       request = TestServer.last_request
       request.method.should eq("HEAD")
       request.path.should eq "hello"
-      request.body.not_nil!.gets_to_end.should eq("{}")
+      request.body.to_s.should eq("{}")
     end
   end
 end

spec/lucky/cookies/cookie_jar_spec.cr

@@ -28,8 +28,8 @@ describe Lucky::CookieJar do
     jar.get_raw("symbol").value.should eq("symbol_value")
     jar.get_raw(:cookie).value.should eq(value)
     jar.get_raw("cookie").value.should eq(value)
-    jar.get_raw?(:cookie).not_nil!.value.should eq(value)
-    jar.get_raw?("cookie").not_nil!.value.should eq(value)
+    jar.get_raw?(:cookie).as(HTTP::Cookie).value.should eq(value)
+    jar.get_raw?("cookie").as(HTTP::Cookie).value.should eq(value)
     jar.get_raw?(:missing).should be_nil
     jar.get_raw?("missing").should be_nil
   end
@@ -136,7 +136,7 @@ describe Lucky::CookieJar do
       jar.set(:tabs_or_spaces, "stop it").http_only(false).expires(time)
 
       jar.get_raw(:tabs_or_spaces).http_only.should be_false
-      jar.get_raw(:tabs_or_spaces).expires.not_nil!.should eq(time)
+      jar.get_raw(:tabs_or_spaces).expires.as(Time).should eq(time)
     end
 
     it "raises an error if the cookie is > 4096 bytes" do

spec/lucky/paginator/paginator_spec.cr

@@ -1,11 +1,11 @@
 require "../../spec_helper"
 
-private def build_pages(page = 1, per_page = 1, item_count = 1, full_path = "/items")
-  Lucky::Paginator.new \
+private def build_pages(page = 1, per_page = 1, item_count = 1, full_path = "/items") : Lucky::Paginator
+  Lucky::Paginator.new(
     page: page,
     per_page: per_page,
     item_count: item_count,
-    full_path: full_path
+    full_path: full_path)
 end
 
 describe Lucky::Paginator do
@@ -134,7 +134,7 @@ describe Lucky::Paginator do
     it "returns the path to the next page if there is a next page" do
       path = build_pages(page: 1, per_page: 1, item_count: 2).path_to_next
 
-      path.not_nil!.should end_with("?page=2")
+      path.to_s.should end_with("?page=2")
     end
 
     it "returns nil if there is not next page" do
@@ -148,7 +148,7 @@ describe Lucky::Paginator do
     it "returns the path to the previous page if there is a previous page" do
       path = build_pages(page: 2, per_page: 1, item_count: 2).path_to_previous
 
-      path.not_nil!.should end_with("?page=1")
+      path.to_s.should end_with("?page=1")
     end
 
     it "returns nil if there is not previous page" do

spec/lucky/param_parser_spec.cr

@@ -96,7 +96,7 @@ describe Lucky::ParamParser do
         result = Lucky::ParamParser.parse(formatted_time.value, Time)
 
         result.should_not be_nil
-        result = result.not_nil!
+        result = result.as(Time)
         result.year.should eq(time.year) if formatted_time.components.includes? TimeComponent::Year
         result.month.should eq(time.month) if formatted_time.components.includes? TimeComponent::Month
         result.day.should eq(time.day) if formatted_time.components.includes? TimeComponent::Day

spec/lucky/static_compression_handler_spec.cr

@@ -71,7 +71,7 @@ describe Lucky::StaticCompressionHandler do
 
       call_handler_with(first_context) { }
 
-      last_modified = HTTP.parse_time(first_context.response.headers["Last-Modified"]).not_nil!
+      last_modified = HTTP.parse_time(first_context.response.headers["Last-Modified"]).as(Time)
 
       context = build_context(path: PATH)
       context.request.headers["Accept-Encoding"] = "gzip"

spec/support/routes_helper.cr

@@ -2,7 +2,7 @@ module RoutesHelper
   private def assert_route_added?(method, path, expected_action)
     result = Lucky.router.find_action(method, path)
     result.should_not be_nil
-    result.not_nil!.payload.should eq expected_action
+    result.as(LuckyRouter::Match).payload.should eq expected_action
   end
 
   private def assert_route_not_added?(method, path)

spec/support/test_server.cr

@@ -2,7 +2,7 @@ class TestServer < Lucky::BaseAppServer
   class_setter last_request : HTTP::Request?
 
   def self.last_request : HTTP::Request
-    @@last_request.not_nil!
+    @@last_request.as(HTTP::Request)
   end
 
   def middleware : Array(HTTP::Handler)
@@ -16,8 +16,8 @@ class TestServer < Lucky::BaseAppServer
     raise "unimplemented"
   end
 
-  def last_request
-    self.class.last_request.not_nil!
+  def last_request : HTTP::Request
+    self.class.last_request.as(HTTP::Request)
   end
 
   class LastRequestHandler

src/charms/hash_extensions.cr

@@ -1,4 +1,4 @@
-class Hash
+class Hash(K, V)
   # Return the **nilable** value of a hash key
   #
   # This returns a value stored in a hash. The key can be specified as a String
@@ -11,7 +11,7 @@ class Hash
   # hash.get("name") # => "Karin" : (String | Nil)
   # hash.get(:asdf)  # => nil : (String | Nil)
   # ```
-  def get(key : String | Symbol)
+  def get(key : String | Symbol) : V?
     self[key.to_s]?
   end
 
@@ -28,7 +28,7 @@ class Hash
   # hash.get("name") # => "Karin" : String
   # hash.get(:asdf)  # => KeyError
   # ```
-  def get!(key : String | Symbol)
+  def get!(key : String | Symbol) : V
     self[key.to_s]
   end
 end

src/charms/object.cr

@@ -2,21 +2,21 @@ class Object
   include ::Lucky::QuickDef
   include ::Lucky::Memoizable
 
-  def blank?
+  def blank? : Bool
     if self.responds_to?(:empty?)
       self.empty?
     else
       false
     end
   end
 
-  def present?
+  def present? : Bool
     !blank?
   end
 end
 
 struct Char
-  def blank?
+  def blank? : Bool
     case ord
     when 9, 0xa, 0xb, 0xc, 0xd, 0x20, 0x85, 0xa0, 0x1680, 0x180e,
          0x2000, 0x2001, 0x2002, 0x2003, 0x2004, 0x2005, 0x2006,
@@ -30,7 +30,7 @@ struct Char
 end
 
 struct Nil
-  def blank?
+  def blank? : Bool
     true
   end
 end

src/lucky/asset_helpers.cr

@@ -90,7 +90,7 @@ module Lucky::AssetHelpers
   # NOTE: This method does *not* check assets at compile time. The asset path
   # is found at runtime so it is possible the asset does not exist. Be sure to
   # manually test that the asset is returned as expected.
-  def dynamic_asset(path)
+  def dynamic_asset(path) : String
     fingerprinted_path = Lucky::AssetHelpers::ASSET_MANIFEST[path]?
     if fingerprinted_path
       Lucky::Server.settings.asset_host + fingerprinted_path

src/lucky/base_component.cr

@@ -16,7 +16,7 @@ abstract class Lucky::BaseComponent
   end
 
   # :nodoc:
-  def view(@view : IO)
+  def view(@view : IO) : self
     # This is used by Lucky::MountComponent to set the view.
     self
   end
@@ -25,7 +25,7 @@ abstract class Lucky::BaseComponent
     @context || raise "No context was set in #{self.class.name}. Use 'mount' or set it with 'context(@context)' before rendering."
   end
 
-  def context(@context : HTTP::Server::Context?)
+  def context(@context : HTTP::Server::Context?) : self
     # This is used by Lucky::MountComponent to set the context.
     self
   end

src/lucky/base_http_client.cr

@@ -66,7 +66,7 @@ abstract class Lucky::BaseHTTPClient
   #   .accept_plain_text
   #   .get("/some-path")
   # ```
-  def headers(**header_values)
+  def headers(**header_values) : self
     @client.before_request do |request|
       header_values.each do |key, value|
         request.headers[key.to_s.gsub("-", "_")] = value.to_s

src/lucky/cookies/cookie_jar.cr

@@ -2,8 +2,8 @@ class Lucky::CookieJar
   MAX_COOKIE_SIZE         = 4096
   LUCKY_ENCRYPTION_PREFIX = Base64.strict_encode("lucky") + "--"
   alias Key = String | Symbol
-  private property cookies
-  private property set_cookies
+  private property cookies : HTTP::Cookies
+  private property set_cookies : HTTP::Cookies
 
   Habitat.create do
     setting on_set : (HTTP::Cookie -> HTTP::Cookie)?

src/lucky/events/pipe_event.cr

@@ -13,11 +13,11 @@ class Lucky::Events::PipeEvent < Pulsar::Event
   )
   end
 
-  def before?
+  def before? : Bool
     position == Position::Before
   end
 
-  def after?
+  def after? : Bool
     position == Position::After
   end
 end

src/lucky/events/request_complete_event.cr

@@ -1,5 +1,5 @@
 class Lucky::Events::RequestCompleteEvent < Pulsar::Event
-  getter :duration
+  getter duration : Time::Span
 
   def initialize(@duration : Time::Span)
   end