From c2236acf3a630e844928edd607699e8738992a5c Mon Sep 17 00:00:00 2001
From: "V. Elenhaupt" <3624712+veelenga@users.noreply.github.com>
Date: Thu, 17 May 2018 11:07:40 +0300
Subject: [PATCH] Code fixes reported by static code analysis (#450)
---
spec/exception_handler_spec.cr | 8 ++++----
spec/handler_spec.cr | 6 +++---
spec/helpers_spec.cr | 2 +-
spec/init_handler_spec.cr | 2 +-
spec/log_handler_spec.cr | 1 -
spec/param_parser_spec.cr | 24 ++++++++++++------------
spec/route_handler_spec.cr | 9 ++++-----
spec/route_spec.cr | 6 +++---
spec/spec_helper.cr | 2 +-
spec/static_file_handler_spec.cr | 1 -
spec/websocket_handler_spec.cr | 6 +++---
src/kemal.cr | 4 ++--
src/kemal/exception_handler.cr | 22 ++++++++++------------
src/kemal/helpers/templates.cr | 32 ++++++++++++++++----------------
14 files changed, 60 insertions(+), 65 deletions(-)
diff --git a/spec/exception_handler_spec.cr b/spec/exception_handler_spec.cr
index 63490c7..78da426 100644
--- a/spec/exception_handler_spec.cr
+++ b/spec/exception_handler_spec.cr
@@ -2,7 +2,7 @@ require "./spec_helper"
describe "Kemal::ExceptionHandler" do
it "renders 404 on route not found" do
- get "/" do |env|
+ get "/" do
"Hello"
end
@@ -39,7 +39,7 @@ describe "Kemal::ExceptionHandler" do
end
it "renders custom 500 error" do
- error 500 do |env|
+ error 500 do
"Something happened"
end
get "/" do |env|
@@ -60,7 +60,7 @@ describe "Kemal::ExceptionHandler" do
end
it "keeps the specified error Content-Type" do
- error 500 do |env|
+ error 500 do
"Something happened"
end
get "/" do |env|
@@ -82,7 +82,7 @@ describe "Kemal::ExceptionHandler" do
end
it "renders custom error with env and error" do
- error 500 do |env, err|
+ error 500 do |_, err|
err.message
end
get "/" do |env|
diff --git a/spec/handler_spec.cr b/spec/handler_spec.cr
index c3268d7..9b1019f 100644
--- a/spec/handler_spec.cr
+++ b/spec/handler_spec.cr
@@ -79,7 +79,7 @@ describe "Handler" do
end
add_handler CustomTestHandler.new
- get "/" do |env|
+ get "/" do
" Great"
end
request = HTTP::Request.new("GET", "/")
@@ -89,7 +89,7 @@ describe "Handler" do
end
it "runs specified only_routes in middleware" do
- get "/only" do |env|
+ get "/only" do
"Get"
end
add_handler OnlyHandler.new
@@ -99,7 +99,7 @@ describe "Handler" do
end
it "doesn't run specified exclude_routes in middleware" do
- get "/" do |env|
+ get "/" do
"Get"
end
get "/exclude" do
diff --git a/spec/helpers_spec.cr b/spec/helpers_spec.cr
index 26b1e22..21c0cdf 100644
--- a/spec/helpers_spec.cr
+++ b/spec/helpers_spec.cr
@@ -31,7 +31,7 @@ describe "Macros" do
describe "#halt" do
it "can break block with halt macro" do
- get "/non-breaking" do |env|
+ get "/non-breaking" do
"hello"
"world"
end
diff --git a/spec/init_handler_spec.cr b/spec/init_handler_spec.cr
index 11b7d71..601bbc1 100644
--- a/spec/init_handler_spec.cr
+++ b/spec/init_handler_spec.cr
@@ -6,7 +6,7 @@ describe "Kemal::InitHandler" do
io = IO::Memory.new
response = HTTP::Server::Response.new(io)
context = HTTP::Server::Context.new(request, response)
- Kemal::InitHandler::INSTANCE.next = ->(context : HTTP::Server::Context) {}
+ Kemal::InitHandler::INSTANCE.next = ->(_context : HTTP::Server::Context) {}
Kemal::InitHandler::INSTANCE.call(context)
context.response.headers["Content-Type"].should eq "text/html"
end
diff --git a/spec/log_handler_spec.cr b/spec/log_handler_spec.cr
index e715cbd..5ee9c86 100644
--- a/spec/log_handler_spec.cr
+++ b/spec/log_handler_spec.cr
@@ -2,7 +2,6 @@ require "./spec_helper"
describe "Kemal::LogHandler" do
it "logs to the given IO" do
- config = Kemal.config
io = IO::Memory.new
logger = Kemal::LogHandler.new io
logger.write "Something"
diff --git a/spec/param_parser_spec.cr b/spec/param_parser_spec.cr
index 4c1992f..5b3b2ab 100644
--- a/spec/param_parser_spec.cr
+++ b/spec/param_parser_spec.cr
@@ -2,7 +2,7 @@ require "./spec_helper"
describe "ParamParser" do
it "parses query params" do
- route = Route.new "POST", "/" do |env|
+ Route.new "POST", "/" do |env|
hasan = env.params.query["hasan"]
"Hello #{hasan}"
end
@@ -12,7 +12,7 @@ describe "ParamParser" do
end
it "parses multiple values for query params" do
- route = Route.new "POST", "/" do |env|
+ Route.new "POST", "/" do |env|
hasan = env.params.query["hasan"]
"Hello #{hasan}"
end
@@ -28,7 +28,7 @@ describe "ParamParser" do
end
request = HTTP::Request.new("POST", "/hello/cemal")
# Radix tree MUST be run to parse url params.
- io_with_context = create_request_and_return_io(kemal, request)
+ create_request_and_return_io(kemal, request)
url_params = Kemal::ParamParser.new(request).url
url_params["hasan"].should eq "cemal"
end
@@ -43,7 +43,7 @@ describe "ParamParser" do
end
request = HTTP::Request.new("POST", "/hello/sam%2Bspec%40gmail.com/%2419.99/a%C3%B1o")
# Radix tree MUST be run to parse url params.
- io_with_context = create_request_and_return_io(kemal, request)
+ create_request_and_return_io(kemal, request)
url_params = Kemal::ParamParser.new(request).url
url_params["email"].should eq "sam+spec@gmail.com"
url_params["money"].should eq "$19.99"
@@ -51,7 +51,7 @@ describe "ParamParser" do
end
it "parses request body" do
- route = Route.new "POST", "/" do |env|
+ Route.new "POST", "/" do |env|
name = env.params.query["name"]
age = env.params.query["age"]
hasan = env.params.body["hasan"]
@@ -77,7 +77,7 @@ describe "ParamParser" do
end
it "parses multiple values in request body" do
- route = Route.new "POST", "/" do |env|
+ Route.new "POST", "/" do |env|
hasan = env.params.body["hasan"]
"Hello #{hasan}"
end
@@ -95,7 +95,7 @@ describe "ParamParser" do
context "when content type is application/json" do
it "parses request body" do
- route = Route.new "POST", "/" { }
+ Route.new "POST", "/" { }
request = HTTP::Request.new(
"POST",
@@ -109,7 +109,7 @@ describe "ParamParser" do
end
it "parses request body when passed charset" do
- route = Route.new "POST", "/" { }
+ Route.new "POST", "/" { }
request = HTTP::Request.new(
"POST",
@@ -123,7 +123,7 @@ describe "ParamParser" do
end
it "parses request body for array" do
- route = Route.new "POST", "/" { }
+ Route.new "POST", "/" { }
request = HTTP::Request.new(
"POST",
@@ -137,7 +137,7 @@ describe "ParamParser" do
end
it "parses request body and query params" do
- route = Route.new "POST", "/" { }
+ Route.new "POST", "/" { }
request = HTTP::Request.new(
"POST",
@@ -156,7 +156,7 @@ describe "ParamParser" do
end
it "handles no request body" do
- route = Route.new "GET", "/" { }
+ Route.new "GET", "/" { }
request = HTTP::Request.new(
"GET",
@@ -180,7 +180,7 @@ describe "ParamParser" do
context "when content type is incorrect" do
it "does not parse request body" do
- route = Route.new "POST", "/" do |env|
+ Route.new "POST", "/" do |env|
name = env.params.body["name"]
age = env.params.body["age"]
hasan = env.params.query["hasan"]
diff --git a/spec/route_handler_spec.cr b/spec/route_handler_spec.cr
index f1a53bb..67d9b71 100644
--- a/spec/route_handler_spec.cr
+++ b/spec/route_handler_spec.cr
@@ -104,7 +104,7 @@ describe "Kemal::RouteHandler" do
end
it "checks for _method param in POST request to simulate PUT" do
- put "/" do |env|
+ put "/" do
"Hello World from PUT"
end
request = HTTP::Request.new(
@@ -118,7 +118,7 @@ describe "Kemal::RouteHandler" do
end
it "checks for _method param in POST request to simulate PATCH" do
- patch "/" do |env|
+ patch "/" do
"Hello World from PATCH"
end
request = HTTP::Request.new(
@@ -132,10 +132,9 @@ describe "Kemal::RouteHandler" do
end
it "checks for _method param in POST request to simulate DELETE" do
- delete "/" do |env|
+ delete "/" do
"Hello World from DELETE"
end
- json_payload = {"_method": "DELETE"}
request = HTTP::Request.new(
"POST",
"/",
@@ -147,7 +146,7 @@ describe "Kemal::RouteHandler" do
end
it "can process HTTP HEAD requests for defined GET routes" do
- get "/" do |env|
+ get "/" do
"Hello World from GET"
end
request = HTTP::Request.new("HEAD", "/")
diff --git a/spec/route_spec.cr b/spec/route_spec.cr
index 0765281..7634d51 100644
--- a/spec/route_spec.cr
+++ b/spec/route_spec.cr
@@ -3,10 +3,10 @@ require "./spec_helper"
describe "Route" do
describe "match?" do
it "matches the correct route" do
- get "/route1" do |env|
+ get "/route1" do
"Route 1"
end
- get "/route2" do |env|
+ get "/route2" do
"Route 2"
end
request = HTTP::Request.new("GET", "/route2")
@@ -16,7 +16,7 @@ describe "Route" do
it "doesn't allow a route declaration start without /" do
expect_raises Kemal::Exceptions::InvalidPathStartException, "Route declaration get \"route\" needs to start with '/', should be get \"/route\"" do
- get "route" do |env|
+ get "route" do
"Route 1"
end
end
diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr
index 78066d7..a4876c2 100644
--- a/spec/spec_helper.cr
+++ b/spec/spec_helper.cr
@@ -66,7 +66,7 @@ def build_main_handler
Kemal.config.setup
main_handler = Kemal.config.handlers.first
current_handler = main_handler
- Kemal.config.handlers.each_with_index do |handler, index|
+ Kemal.config.handlers.each do |handler|
current_handler.next = handler
current_handler = handler
end
diff --git a/spec/static_file_handler_spec.cr b/spec/static_file_handler_spec.cr
index 6a29227..7b9b099 100644
--- a/spec/static_file_handler_spec.cr
+++ b/spec/static_file_handler_spec.cr
@@ -14,7 +14,6 @@ end
describe Kemal::StaticFileHandler do
file = File.open "#{__DIR__}/static/dir/test.txt"
file_size = file.size
- file_text = file.to_s
it "should serve a file with content type and etag" do
response = handle HTTP::Request.new("GET", "/dir/test.txt")
diff --git a/spec/websocket_handler_spec.cr b/spec/websocket_handler_spec.cr
index 5f2974d..a3c0634 100644
--- a/spec/websocket_handler_spec.cr
+++ b/spec/websocket_handler_spec.cr
@@ -22,8 +22,8 @@ describe "Kemal::WebSocketHandler" do
it "matches on given route" do
handler = Kemal::WebSocketHandler::INSTANCE
- ws "/" { |socket, context| socket.send("Match") }
- ws "/no_match" { |socket, context| socket.send "No Match" }
+ ws "/" { |socket| socket.send("Match") }
+ ws "/no_match" { |socket| socket.send "No Match" }
headers = HTTP::Headers{
"Upgrade" => "websocket",
"Connection" => "Upgrade",
@@ -37,7 +37,7 @@ describe "Kemal::WebSocketHandler" do
it "fetches named url parameters" do
handler = Kemal::WebSocketHandler::INSTANCE
- ws "/:id" { |s, c| c.params.url["id"] }
+ ws "/:id" { |_, c| c.params.url["id"] }
headers = HTTP::Headers{
"Upgrade" => "websocket",
"Connection" => "Upgrade",
diff --git a/src/kemal.cr b/src/kemal.cr
index 13880ea..514478f 100644
--- a/src/kemal.cr
+++ b/src/kemal.cr
@@ -34,7 +34,7 @@ module Kemal
config.port = port if port
unless Kemal.config.error_handlers.has_key?(404)
- error 404 do |env|
+ error 404 do
render_404
end
end
@@ -50,7 +50,7 @@ module Kemal
# This route serves the built-in images for not_found and exceptions.
get "/__kemal__/404.png" do |env|
file_path = File.expand_path("lib/kemal/images/404.png", Dir.current)
-
+
if File.exists? file_path
send_file env, file_path
else
diff --git a/src/kemal/exception_handler.cr b/src/kemal/exception_handler.cr
index 155fbe3..657ce45 100644
--- a/src/kemal/exception_handler.cr
+++ b/src/kemal/exception_handler.cr
@@ -5,18 +5,16 @@ module Kemal
INSTANCE = new
def call(context : HTTP::Server::Context)
- begin
- call_next(context)
- rescue ex : Kemal::Exceptions::RouteNotFound
- call_exception_with_status_code(context, ex, 404)
- rescue ex : Kemal::Exceptions::CustomException
- call_exception_with_status_code(context, ex, context.response.status_code)
- rescue ex : Exception
- log("Exception: #{ex.inspect_with_backtrace}")
- return call_exception_with_status_code(context, ex, 500) if Kemal.config.error_handlers.has_key?(500)
- verbosity = Kemal.config.env == "production" ? false : true
- return render_500(context, ex.inspect_with_backtrace, verbosity)
- end
+ call_next(context)
+ rescue ex : Kemal::Exceptions::RouteNotFound
+ call_exception_with_status_code(context, ex, 404)
+ rescue ex : Kemal::Exceptions::CustomException
+ call_exception_with_status_code(context, ex, context.response.status_code)
+ rescue ex : Exception
+ log("Exception: #{ex.inspect_with_backtrace}")
+ return call_exception_with_status_code(context, ex, 500) if Kemal.config.error_handlers.has_key?(500)
+ verbosity = Kemal.config.env == "production" ? false : true
+ return render_500(context, ex.inspect_with_backtrace, verbosity)
end
private def call_exception_with_status_code(context : HTTP::Server::Context, exception : Exception, status_code : Int32)
diff --git a/src/kemal/helpers/templates.cr b/src/kemal/helpers/templates.cr
index fe3b83f..42e2650 100644
--- a/src/kemal/helpers/templates.cr
+++ b/src/kemal/helpers/templates.cr
@@ -2,22 +2,22 @@
# Currently it contains templates for 404 and 500 error codes.
def render_404
- template = <<-HTML
-
-
-
-
-
-
- Kemal doesn't know this way.
- 
-
-
+ <<-HTML
+
+
+
+
+
+
+ Kemal doesn't know this way.
+
+
+
HTML
end