Skip to content

Commit

Permalink
Add railspect requires for auditing requires
Browse files Browse the repository at this point in the history
This first commit sets up the command, a visitor to gather data, and a
simple lint to demonstrate its use.

This first lint ensures that "active_support/rails" is required in all
frameworks (one was missing this require but this was addressed in a
[previous commit][1]), and then checks that none of the requires in
"active_support/rails" are called again (except in Active Support since
it does not require "active_support/rails").

[1]: 2e920b0
  • Loading branch information
skipkayhil committed Jan 11, 2025
1 parent ebdebf9 commit 78dbb9f
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 0 deletions.
10 changes: 10 additions & 0 deletions tools/rail_inspector/lib/rail_inspector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,14 @@
# SOFTWARE.

module RailInspector
class << self
def frameworks(rails_path)
@frameworks ||= begin
spec = Gem::Specification.load(rails_path.join("rails.gemspec").to_s)
names = spec.dependencies.map(&:name)
names.delete("bundler")
names
end
end
end
end
9 changes: 9 additions & 0 deletions tools/rail_inspector/lib/rail_inspector/cli.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require "thor"
require_relative "../rail_inspector"

module RailInspector
class Cli < Thor
Expand Down Expand Up @@ -30,5 +31,13 @@ def configuration(rails_path)

checker.write!
end

desc "requires RAILS_PATH", "Check for autoloads being required"
option :autocorrect, type: :boolean, aliases: :a
def requires(rails_path)
require_relative "./requires"

exit Requires.new(rails_path, options[:autocorrect]).call
end
end
end
92 changes: 92 additions & 0 deletions tools/rail_inspector/lib/rail_inspector/requires.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# frozen_string_literal: true

require "active_support"
require "active_support/core_ext/string"
require "prism"

require_relative "./visitor/load"

module RailInspector
class Requires
def initialize(rails_path, autocorrect)
@loads = {}
@rails_path = Pathname.new(rails_path)
@exit = true
@autocorrect = autocorrect
end

def call
populate_loads

prevent_active_support_rails_requires

@exit
end

private
def populate_loads
current_file = nil
v = Visitor::Load.new { @loads[current_file] }

@rails_path.glob("{#{frameworks.join(",")}}/lib/**/*.rb") do |file_pathname|
current_file = file_pathname.to_s

@loads[current_file] = { requires: [], autoloads: [] }

Prism.parse_file(current_file).value.accept(v)
end
end

def prevent_active_support_rails_requires
frameworks.each do |framework|
next if framework == "activesupport"

@rails_path.glob("#{framework}/lib/*.rb").each do |root_path|
root_requires = @loads[root_path.to_s][:requires]
next if root_requires.include?("active_support/rails")

# required transitively
next if root_requires.include?("action_dispatch")
# action_pack namespace doesn't include any code
# arel does not depend on active_support at all
next if ["action_pack.rb", "arel.rb"].include?(root_path.basename.to_s)

@exit = false
puts root_path
puts " + \"active_support/rails\" (framework root)"
end
end

active_support_rails_requires = @loads["activesupport/lib/active_support/rails.rb"][:requires]

duplicated_requires = {}

@loads.each do |path, file_loads|
next if path.start_with? "activesupport"

if active_support_rails_requires.intersect?(file_loads[:requires])
duplicated_requires[path] = active_support_rails_requires.intersection(file_loads[:requires])
end
end

duplicated_requires.each do |path, offenses|
@exit = false
puts path
offenses.each do |duplicate_require|
puts " - #{duplicate_require} (active_support/rails)"

next unless @autocorrect

file = File.read(path)
file.gsub!("require \"#{duplicate_require}\"\n", "")

File.write(path, file)
end
end
end

def frameworks
RailInspector.frameworks(@rails_path)
end
end
end
50 changes: 50 additions & 0 deletions tools/rail_inspector/lib/rail_inspector/visitor/load.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require "prism"

module RailInspector
module Visitor
class Load < Prism::Visitor
def initialize(&block)
@current_loads = block
@namespace_stack = []
end

def visit_module_node(node)
case node.constant_path
in Prism::ConstantReadNode[name:]
@namespace_stack << name
in Prism::ConstantPathNode
@namespace_stack << node.constant_path.full_name
end

super

@namespace_stack.pop
end
alias visit_class_node visit_module_node

def visit_call_node(node)
case node.name
when :require
case node.arguments.arguments[0]
in Prism::StringNode[unescaped:]
@current_loads.call[:requires] << unescaped
else
# dynamic require, like "active_support/cache/#{store}"
end
when :autoload
case node.arguments.arguments
in [Prism::SymbolNode[unescaped:]]
namespaced_const = @namespace_stack.join("::")
namespaced_const << "::" << unescaped

@current_loads.call[:autoloads] << namespaced_const.underscore
in [Prism::SymbolNode, Prism::StringNode[unescaped:]]
@current_loads.call[:autoloads] << unescaped
end
end
end
end
end
end
35 changes: 35 additions & 0 deletions tools/rail_inspector/test/rail_inspector/visitor/load_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

require "test_helper"
require "rail_inspector/visitor/load"

class LoadTest < Minitest::Test
def test_finds_requires_and_autoloads
source = <<~FILE
require "a"
require "b"
module D
require "k"
autoload :L
autoload :M, "n/o"
class E::F
require "p/q"
autoload :G
autoload :H, "i/j"
end
end
FILE

loads = { requires: [], autoloads: [] }

visitor = RailInspector::Visitor::Load.new { loads }
Prism.parse(source).value.accept(visitor)

assert_equal ["a", "b", "k", "p/q"], loads[:requires]
assert_equal ["d/l", "n/o", "d/e/f/g", "i/j"], loads[:autoloads]
end
end

0 comments on commit 78dbb9f

Please sign in to comment.