Skip to content

Commit 78dbb9f

Browse files
committed
Add railspect requires for auditing requires
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
1 parent ebdebf9 commit 78dbb9f

File tree

5 files changed

+196
-0
lines changed

5 files changed

+196
-0
lines changed

tools/rail_inspector/lib/rail_inspector.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,14 @@
2525
# SOFTWARE.
2626

2727
module RailInspector
28+
class << self
29+
def frameworks(rails_path)
30+
@frameworks ||= begin
31+
spec = Gem::Specification.load(rails_path.join("rails.gemspec").to_s)
32+
names = spec.dependencies.map(&:name)
33+
names.delete("bundler")
34+
names
35+
end
36+
end
37+
end
2838
end

tools/rail_inspector/lib/rail_inspector/cli.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require "thor"
4+
require_relative "../rail_inspector"
45

56
module RailInspector
67
class Cli < Thor
@@ -30,5 +31,13 @@ def configuration(rails_path)
3031

3132
checker.write!
3233
end
34+
35+
desc "requires RAILS_PATH", "Check for autoloads being required"
36+
option :autocorrect, type: :boolean, aliases: :a
37+
def requires(rails_path)
38+
require_relative "./requires"
39+
40+
exit Requires.new(rails_path, options[:autocorrect]).call
41+
end
3342
end
3443
end
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# frozen_string_literal: true
2+
3+
require "active_support"
4+
require "active_support/core_ext/string"
5+
require "prism"
6+
7+
require_relative "./visitor/load"
8+
9+
module RailInspector
10+
class Requires
11+
def initialize(rails_path, autocorrect)
12+
@loads = {}
13+
@rails_path = Pathname.new(rails_path)
14+
@exit = true
15+
@autocorrect = autocorrect
16+
end
17+
18+
def call
19+
populate_loads
20+
21+
prevent_active_support_rails_requires
22+
23+
@exit
24+
end
25+
26+
private
27+
def populate_loads
28+
current_file = nil
29+
v = Visitor::Load.new { @loads[current_file] }
30+
31+
@rails_path.glob("{#{frameworks.join(",")}}/lib/**/*.rb") do |file_pathname|
32+
current_file = file_pathname.to_s
33+
34+
@loads[current_file] = { requires: [], autoloads: [] }
35+
36+
Prism.parse_file(current_file).value.accept(v)
37+
end
38+
end
39+
40+
def prevent_active_support_rails_requires
41+
frameworks.each do |framework|
42+
next if framework == "activesupport"
43+
44+
@rails_path.glob("#{framework}/lib/*.rb").each do |root_path|
45+
root_requires = @loads[root_path.to_s][:requires]
46+
next if root_requires.include?("active_support/rails")
47+
48+
# required transitively
49+
next if root_requires.include?("action_dispatch")
50+
# action_pack namespace doesn't include any code
51+
# arel does not depend on active_support at all
52+
next if ["action_pack.rb", "arel.rb"].include?(root_path.basename.to_s)
53+
54+
@exit = false
55+
puts root_path
56+
puts " + \"active_support/rails\" (framework root)"
57+
end
58+
end
59+
60+
active_support_rails_requires = @loads["activesupport/lib/active_support/rails.rb"][:requires]
61+
62+
duplicated_requires = {}
63+
64+
@loads.each do |path, file_loads|
65+
next if path.start_with? "activesupport"
66+
67+
if active_support_rails_requires.intersect?(file_loads[:requires])
68+
duplicated_requires[path] = active_support_rails_requires.intersection(file_loads[:requires])
69+
end
70+
end
71+
72+
duplicated_requires.each do |path, offenses|
73+
@exit = false
74+
puts path
75+
offenses.each do |duplicate_require|
76+
puts " - #{duplicate_require} (active_support/rails)"
77+
78+
next unless @autocorrect
79+
80+
file = File.read(path)
81+
file.gsub!("require \"#{duplicate_require}\"\n", "")
82+
83+
File.write(path, file)
84+
end
85+
end
86+
end
87+
88+
def frameworks
89+
RailInspector.frameworks(@rails_path)
90+
end
91+
end
92+
end
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# frozen_string_literal: true
2+
3+
require "prism"
4+
5+
module RailInspector
6+
module Visitor
7+
class Load < Prism::Visitor
8+
def initialize(&block)
9+
@current_loads = block
10+
@namespace_stack = []
11+
end
12+
13+
def visit_module_node(node)
14+
case node.constant_path
15+
in Prism::ConstantReadNode[name:]
16+
@namespace_stack << name
17+
in Prism::ConstantPathNode
18+
@namespace_stack << node.constant_path.full_name
19+
end
20+
21+
super
22+
23+
@namespace_stack.pop
24+
end
25+
alias visit_class_node visit_module_node
26+
27+
def visit_call_node(node)
28+
case node.name
29+
when :require
30+
case node.arguments.arguments[0]
31+
in Prism::StringNode[unescaped:]
32+
@current_loads.call[:requires] << unescaped
33+
else
34+
# dynamic require, like "active_support/cache/#{store}"
35+
end
36+
when :autoload
37+
case node.arguments.arguments
38+
in [Prism::SymbolNode[unescaped:]]
39+
namespaced_const = @namespace_stack.join("::")
40+
namespaced_const << "::" << unescaped
41+
42+
@current_loads.call[:autoloads] << namespaced_const.underscore
43+
in [Prism::SymbolNode, Prism::StringNode[unescaped:]]
44+
@current_loads.call[:autoloads] << unescaped
45+
end
46+
end
47+
end
48+
end
49+
end
50+
end
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# frozen_string_literal: true
2+
3+
require "test_helper"
4+
require "rail_inspector/visitor/load"
5+
6+
class LoadTest < Minitest::Test
7+
def test_finds_requires_and_autoloads
8+
source = <<~FILE
9+
require "a"
10+
require "b"
11+
12+
module D
13+
require "k"
14+
15+
autoload :L
16+
autoload :M, "n/o"
17+
18+
class E::F
19+
require "p/q"
20+
21+
autoload :G
22+
autoload :H, "i/j"
23+
end
24+
end
25+
FILE
26+
27+
loads = { requires: [], autoloads: [] }
28+
29+
visitor = RailInspector::Visitor::Load.new { loads }
30+
Prism.parse(source).value.accept(visitor)
31+
32+
assert_equal ["a", "b", "k", "p/q"], loads[:requires]
33+
assert_equal ["d/l", "n/o", "d/e/f/g", "i/j"], loads[:autoloads]
34+
end
35+
end

0 commit comments

Comments
 (0)