diff --git a/config/default.yml b/config/default.yml index 312307ca..66e1260c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -65,3 +65,8 @@ Isucon/Sinatra/DisableLogging: Description: 'Disable sinatra logging' Enabled: true VersionAdded: '0.1.0' + +Isucon/Sinatra/ServeStaticFile: + Description: 'Serve static files on front server (e.g. nginx)' + Enabled: true + VersionAdded: '0.1.0' diff --git a/lib/rubocop/cop/isucon/sinatra/serve_static_file.rb b/lib/rubocop/cop/isucon/sinatra/serve_static_file.rb new file mode 100644 index 00000000..d6bec272 --- /dev/null +++ b/lib/rubocop/cop/isucon/sinatra/serve_static_file.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Isucon + module Sinatra + # Serve static files on front server (e.g. nginx) + # + # @example + # # bad + # class App < Sinatra::Base + # get '/' do + # content_type :html + # File.read(File.join(__dir__, '..', 'public', 'index.html')) + # end + # end + # + # # good (e.g. Serve on nginx) + # location / { + # try_files $uri $uri/ /index.html; + # } + # + class ServeStaticFile < Base + MSG = "Serve static files on front server (e.g. nginx)" + + def_node_matcher :file_read_method?, <<~PATTERN + (send (const nil? :File) :read ...) + PATTERN + + def_node_matcher :get_method?, <<~PATTERN + (block (send nil? :get ...) ...) + PATTERN + + # @param node [RuboCop::AST::Node] + def on_send(node) + return unless file_read_method?(node) + + parent = parent_get_node(node) + return unless parent + + return unless end_of_block?(node: node, parent: parent) + + add_offense(node) + end + + private + + # @param node [RuboCop::AST::Node] + # @return [RuboCop::AST::Node] + def parent_get_node(node) + node.each_ancestor.find { |ancestor| get_method?(ancestor) } + end + + # @param node [RuboCop::AST::Node] + # @param parent [RuboCop::AST::Node] + # @return [Boolean] + def end_of_block?(node:, parent:) + parent.child_nodes.last&.child_nodes&.last == node + end + end + end + end + end +end diff --git a/lib/rubocop/cop/isucon_cops.rb b/lib/rubocop/cop/isucon_cops.rb index 64d32670..36333cdd 100644 --- a/lib/rubocop/cop/isucon_cops.rb +++ b/lib/rubocop/cop/isucon_cops.rb @@ -10,3 +10,4 @@ require_relative "isucon/mysql2/select_asterisk" require_relative "isucon/mysql2/where_without_index" require_relative "isucon/sinatra/disable_logging" +require_relative "isucon/sinatra/serve_static_file" diff --git a/spec/rubocop/cop/isucon/sinatra/serve_static_file_spec.rb b/spec/rubocop/cop/isucon/sinatra/serve_static_file_spec.rb new file mode 100644 index 00000000..cead111f --- /dev/null +++ b/spec/rubocop/cop/isucon/sinatra/serve_static_file_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Isucon::Sinatra::ServeStaticFile, :config do + let(:config) { RuboCop::Config.new } + + context "`File.read` is at end of block" do + it "registers an offense" do + # c.f. https://github.com/isucon/isucon8-final/blob/38c4f6e20388d1c4f1ed393fb75b38d472e44abf/webapp/ruby/app.rb#L55-L58 + expect_offense(<<~RUBY) + get '/' do + content_type :html + File.read(File.join(__dir__, '..', 'public', 'index.html')) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Serve static files on front server (e.g. nginx) + end + RUBY + end + end + + context "`File.read` isn't at end of block" do + it "registers an offense" do + expect_no_offenses(<<~RUBY) + get '/' do + content_type :html + File.read(File.join(__dir__, '..', 'public', 'index.html')) + "" + end + RUBY + end + end +end