Merge pull request #143 from sue445/feature/sinatra_serve_static_file

Impl `Isucon/Sinatra/ServeStaticFile`

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'

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

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"

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