Skip to content

Commit b951bdb

Browse files
committed
[rail_inspector] Extract NewFrameworkDefaultsFile
To make it easier to change what gets passed to the FrameworkDefaults check.
1 parent 233f023 commit b951bdb

File tree

4 files changed

+59
-58
lines changed

4 files changed

+59
-58
lines changed

tools/rail_inspector/lib/rail_inspector/configuring.rb

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
require "pathname"
44
require_relative "./configuring/check/general_configuration"
55
require_relative "./configuring/check/framework_defaults"
6+
require_relative "./configuring/check/new_framework_defaults_file"
67
require_relative "./configuring/document"
8+
require_relative "./visitor/framework_default"
79

810
module RailInspector
911
class Configuring
@@ -60,15 +62,27 @@ def initialize(rails_path)
6062
end
6163

6264
def check
63-
[Check::GeneralConfiguration, Check::FrameworkDefaults].each do |check|
64-
check.new(self).check
65-
end
65+
[
66+
Check::GeneralConfiguration.new(self),
67+
Check::FrameworkDefaults.new(self),
68+
Check::NewFrameworkDefaultsFile.new(
69+
self,
70+
framework_defaults_by_version[rails_version].keys,
71+
files.new_framework_defaults.read
72+
),
73+
].each(&:check)
6674
end
6775

6876
def doc
6977
@doc ||= Configuring::Document.parse(files.doc_path.read)
7078
end
7179

80+
def framework_defaults_by_version
81+
@framework_defaults_by_version ||= Visitor::FrameworkDefault.new.tap { |visitor|
82+
visitor.visit(files.application_configuration.parse)
83+
}.config_map
84+
end
85+
7286
def rails_version
7387
@rails_version ||= files.rails_version.read.to_f.to_s
7488
end

tools/rail_inspector/lib/rail_inspector/configuring/check/framework_defaults.rb

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,11 @@
11
# frozen_string_literal: true
22

33
require "tempfile"
4-
require_relative "../../visitor/framework_default"
54

65
module RailInspector
76
class Configuring
87
module Check
98
class FrameworkDefaults
10-
class NewFrameworkDefaultsFile
11-
attr_reader :checker, :visitor
12-
13-
# Defaults are strings like:
14-
# self.yjit
15-
# action_controller.escape_json_responses
16-
def initialize(checker, defaults, file_content)
17-
@checker = checker
18-
@defaults = defaults
19-
@file_content = file_content
20-
end
21-
22-
def check
23-
@defaults.each do |config|
24-
if config.start_with? "self"
25-
next if @file_content.include? config.gsub(/^self/, "config")
26-
next if @file_content.include? config.gsub(/^self/, "configuration")
27-
end
28-
29-
next if @file_content.include? config
30-
31-
next if config == "self.yjit"
32-
33-
checker.errors << <<~MESSAGE
34-
#{checker.files.new_framework_defaults}: Missing new default
35-
#{config}
36-
37-
MESSAGE
38-
end
39-
end
40-
end
41-
429
attr_reader :checker
4310

4411
def initialize(checker)
@@ -48,12 +15,6 @@ def initialize(checker)
4815
def check
4916
header, *defaults_by_version = documented_defaults
5017

51-
NewFrameworkDefaultsFile.new(
52-
checker,
53-
visitor.config_map[checker.rails_version].keys,
54-
checker.files.new_framework_defaults.read
55-
).check
56-
5718
checker.doc.versioned_defaults =
5819
header +
5920
defaults_by_version
@@ -62,18 +23,14 @@ def check
6223
end
6324

6425
private
65-
def app_config_tree
66-
checker.files.application_configuration.parse
67-
end
68-
6926
def check_defaults(defaults)
7027
header, configs = defaults[0], defaults[2, defaults.length - 3]
7128
configs ||= []
7229

7330
version = header.match(/\d\.\d/)[0]
7431

7532
generated_doc =
76-
visitor.config_map[version]
33+
checker.framework_defaults_by_version[version]
7734
.map do |config, value|
7835
full_config =
7936
case config
@@ -119,15 +76,6 @@ def documented_defaults
11976
.slice_before { |line| line.start_with?("####") }
12077
.to_a
12178
end
122-
123-
def visitor
124-
@visitor ||=
125-
begin
126-
visitor = Visitor::FrameworkDefault.new
127-
visitor.visit(app_config_tree)
128-
visitor
129-
end
130-
end
13179
end
13280
end
13381
end
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# frozen_string_literal: true
2+
3+
module RailInspector
4+
class Configuring
5+
module Check
6+
class NewFrameworkDefaultsFile
7+
attr_reader :checker, :visitor
8+
9+
# Defaults are strings like:
10+
# self.yjit
11+
# action_controller.escape_json_responses
12+
def initialize(checker, defaults, file_content)
13+
@checker = checker
14+
@defaults = defaults
15+
@file_content = file_content
16+
end
17+
18+
def check
19+
@defaults.each do |config|
20+
if config.start_with? "self"
21+
next if @file_content.include? config.gsub(/^self/, "config")
22+
next if @file_content.include? config.gsub(/^self/, "configuration")
23+
end
24+
25+
next if @file_content.include? config
26+
27+
next if config == "self.yjit"
28+
29+
checker.errors << <<~MESSAGE
30+
#{checker.files.new_framework_defaults}: Missing new default
31+
#{config}
32+
33+
MESSAGE
34+
end
35+
end
36+
end
37+
end
38+
end
39+
end

tools/rail_inspector/test/rail_inspector/configuring/check/framework_defaults_test.rb renamed to tools/rail_inspector/test/rail_inspector/configuring/check/new_framework_defaults_file_test.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
require "test_helper"
44
require "rail_inspector/configuring"
55

6-
class TestFrameworkDefaults < ActiveSupport::TestCase
6+
class NewFrameworkDefaultsFileTest < ActiveSupport::TestCase
77
def test_identifies_self_when_file_uses_config
88
defaults = ["self.log_file_size"]
99

@@ -30,7 +30,7 @@ def test_identifies_self_when_file_uses_configuration
3030

3131
private
3232
def check(defaults, file_content)
33-
@check ||= RailInspector::Configuring::Check::FrameworkDefaults::NewFrameworkDefaultsFile.new(checker, defaults, file_content)
33+
@check ||= RailInspector::Configuring::Check::NewFrameworkDefaultsFile.new(checker, defaults, file_content)
3434
end
3535

3636
def checker

0 commit comments

Comments
 (0)