Skip to content

Commit fcfab87

Browse files
drwlpeterfication
drwl
authored and
peterfication
committed
Refactor Parser (ctran#641)
Refactored Parser to isolate changes being made to ENV. This way we have an intermediate step where we know the environment variables being set.
1 parent 262bd07 commit fcfab87

File tree

2 files changed

+168
-155
lines changed

2 files changed

+168
-155
lines changed

‎lib/annotate/parser.rb

+166-153
Original file line numberDiff line numberDiff line change
@@ -3,220 +3,233 @@
33
module Annotate
44
# Class for handling command line arguments
55
class Parser # rubocop:disable Metrics/ClassLength
6-
def self.parse(args)
7-
new(args).parse
6+
def self.parse(args, env = {})
7+
new(args, env).parse
88
end
99

10-
attr_reader :args
10+
attr_reader :args, :options, :env
1111

1212
ANNOTATION_POSITIONS = %w[before top after bottom].freeze
1313
FILE_TYPE_POSITIONS = %w[position_in_class position_in_factory position_in_fixture position_in_test position_in_routes position_in_serializer].freeze
1414
EXCLUSION_LIST = %w[tests fixtures factories serializers].freeze
1515
FORMAT_TYPES = %w[bare rdoc markdown].freeze
1616

17-
def initialize(args)
17+
def initialize(args, env)
1818
@args = args
19+
@options = default_options
20+
@env = env
1921
end
2022

2123
def parse
22-
options = default_options
24+
# To split up because right now this method parses and commits
25+
parser.parse!(args)
2326

24-
parser(options).parse!(args)
27+
commit
2528

2629
options
2730
end
2831

2932
private
3033

31-
def parser(options) # rubocop:disable Metrics/MethodLength
34+
def commit
35+
env.each_pair do |key, value|
36+
ENV[key] = value
37+
end
38+
end
39+
40+
def parser
41+
OptionParser.new do |option_parser|
42+
add_options_to_parser(option_parser)
43+
end
44+
end
45+
46+
def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength
3247
has_set_position = {}
3348
positions = ANNOTATION_POSITIONS
3449

35-
OptionParser.new do |opts|
36-
opts.banner = 'Usage: annotate [options] [model_file]*'
50+
option_parser.banner = 'Usage: annotate [options] [model_file]*'
3751

38-
opts.on('-d', '--delete', 'Remove annotations from all model files or the routes.rb file') do
39-
options[:target_action] = :remove_annotations
40-
end
52+
option_parser.on('-d', '--delete', 'Remove annotations from all model files or the routes.rb file') do
53+
@options[:target_action] = :remove_annotations
54+
end
4155

42-
opts.on('-p', '--position [before|top|after|bottom]', positions,
43-
'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |p|
44-
ENV['position'] = p
56+
option_parser.on('-p', '--position [before|top|after|bottom]', positions,
57+
'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |p|
58+
env['position'] = p
4559

46-
FILE_TYPE_POSITIONS.each do |key|
47-
ENV[key] = p unless has_set_position[key]
48-
end
60+
FILE_TYPE_POSITIONS.each do |key|
61+
env[key] = p unless has_set_position[key]
4962
end
63+
end
5064

51-
opts.on('--pc', '--position-in-class [before|top|after|bottom]', positions,
52-
'Place the annotations at the top (before) or the bottom (after) of the model file') do |p|
53-
ENV['position_in_class'] = p
54-
has_set_position['position_in_class'] = true
55-
end
65+
option_parser.on('--pc', '--position-in-class [before|top|after|bottom]', positions,
66+
'Place the annotations at the top (before) or the bottom (after) of the model file') do |p|
67+
env['position_in_class'] = p
68+
has_set_position['position_in_class'] = true
69+
end
5670

57-
opts.on('--pf', '--position-in-factory [before|top|after|bottom]', positions,
58-
'Place the annotations at the top (before) or the bottom (after) of any factory files') do |p|
59-
ENV['position_in_factory'] = p
60-
has_set_position['position_in_factory'] = true
61-
end
71+
option_parser.on('--pf', '--position-in-factory [before|top|after|bottom]', positions,
72+
'Place the annotations at the top (before) or the bottom (after) of any factory files') do |p|
73+
env['position_in_factory'] = p
74+
has_set_position['position_in_factory'] = true
75+
end
6276

63-
opts.on('--px', '--position-in-fixture [before|top|after|bottom]', positions,
64-
'Place the annotations at the top (before) or the bottom (after) of any fixture files') do |p|
65-
ENV['position_in_fixture'] = p
66-
has_set_position['position_in_fixture'] = true
67-
end
77+
option_parser.on('--px', '--position-in-fixture [before|top|after|bottom]', positions,
78+
'Place the annotations at the top (before) or the bottom (after) of any fixture files') do |p|
79+
env['position_in_fixture'] = p
80+
has_set_position['position_in_fixture'] = true
81+
end
6882

69-
opts.on('--pt', '--position-in-test [before|top|after|bottom]', positions,
70-
'Place the annotations at the top (before) or the bottom (after) of any test files') do |p|
71-
ENV['position_in_test'] = p
72-
has_set_position['position_in_test'] = true
73-
end
83+
option_parser.on('--pt', '--position-in-test [before|top|after|bottom]', positions,
84+
'Place the annotations at the top (before) or the bottom (after) of any test files') do |p|
85+
env['position_in_test'] = p
86+
has_set_position['position_in_test'] = true
87+
end
7488

75-
opts.on('--pr', '--position-in-routes [before|top|after|bottom]', positions,
76-
'Place the annotations at the top (before) or the bottom (after) of the routes.rb file') do |p|
77-
ENV['position_in_routes'] = p
78-
has_set_position['position_in_routes'] = true
79-
end
89+
option_parser.on('--pr', '--position-in-routes [before|top|after|bottom]', positions,
90+
'Place the annotations at the top (before) or the bottom (after) of the routes.rb file') do |p|
91+
env['position_in_routes'] = p
92+
has_set_position['position_in_routes'] = true
93+
end
8094

81-
opts.on('--ps', '--position-in-serializer [before|top|after|bottom]', positions,
82-
'Place the annotations at the top (before) or the bottom (after) of the serializer files') do |p|
83-
ENV['position_in_serializer'] = p
84-
has_set_position['position_in_serializer'] = true
85-
end
95+
option_parser.on('--ps', '--position-in-serializer [before|top|after|bottom]', positions,
96+
'Place the annotations at the top (before) or the bottom (after) of the serializer files') do |p|
97+
env['position_in_serializer'] = p
98+
has_set_position['position_in_serializer'] = true
99+
end
86100

87-
opts.on('--w', '--wrapper STR', 'Wrap annotation with the text passed as parameter.',
88-
'If --w option is used, the same text will be used as opening and closing') do |p|
89-
ENV['wrapper'] = p
90-
end
101+
option_parser.on('--w', '--wrapper STR', 'Wrap annotation with the text passed as parameter.',
102+
'If --w option is used, the same text will be used as opening and closing') do |p|
103+
env['wrapper'] = p
104+
end
91105

92-
opts.on('--wo', '--wrapper-open STR', 'Annotation wrapper opening.') do |p|
93-
ENV['wrapper_open'] = p
94-
end
106+
option_parser.on('--wo', '--wrapper-open STR', 'Annotation wrapper opening.') do |p|
107+
env['wrapper_open'] = p
108+
end
95109

96-
opts.on('--wc', '--wrapper-close STR', 'Annotation wrapper closing') do |p|
97-
ENV['wrapper_close'] = p
98-
end
110+
option_parser.on('--wc', '--wrapper-close STR', 'Annotation wrapper closing') do |p|
111+
env['wrapper_close'] = p
112+
end
99113

100-
opts.on('-r', '--routes', "Annotate routes.rb with the output of 'rake routes'") do
101-
ENV['routes'] = 'true'
102-
end
114+
option_parser.on('-r', '--routes', "Annotate routes.rb with the output of 'rake routes'") do
115+
env['routes'] = 'true'
116+
end
103117

104-
opts.on('-a', '--active-admin', 'Annotate active_admin models') do
105-
ENV['active_admin'] = 'true'
106-
end
118+
option_parser.on('-a', '--active-admin', 'Annotate active_admin models') do
119+
env['active_admin'] = 'true'
120+
end
107121

108-
opts.on('-v', '--version', 'Show the current version of this gem') do
109-
puts "annotate v#{Annotate.version}"
110-
options[:exit] = true
111-
end
122+
option_parser.on('-v', '--version', 'Show the current version of this gem') do
123+
puts "annotate v#{Annotate.version}"
124+
@options[:exit] = true
125+
end
112126

113-
opts.on('-m', '--show-migration', 'Include the migration version number in the annotation') do
114-
ENV['include_version'] = 'yes'
115-
end
127+
option_parser.on('-m', '--show-migration', 'Include the migration version number in the annotation') do
128+
env['include_version'] = 'yes'
129+
end
116130

117-
opts.on('-k', '--show-foreign-keys',
118-
"List the table's foreign key constraints in the annotation") do
119-
ENV['show_foreign_keys'] = 'yes'
120-
end
131+
option_parser.on('-k', '--show-foreign-keys',
132+
"List the table's foreign key constraints in the annotation") do
133+
env['show_foreign_keys'] = 'yes'
134+
end
121135

122-
opts.on('--ck',
123-
'--complete-foreign-keys', 'Complete foreign key names in the annotation') do
124-
ENV['show_foreign_keys'] = 'yes'
125-
ENV['show_complete_foreign_keys'] = 'yes'
126-
end
136+
option_parser.on('--ck',
137+
'--complete-foreign-keys', 'Complete foreign key names in the annotation') do
138+
env['show_foreign_keys'] = 'yes'
139+
env['show_complete_foreign_keys'] = 'yes'
140+
end
127141

128-
opts.on('-i', '--show-indexes',
129-
"List the table's database indexes in the annotation") do
130-
ENV['show_indexes'] = 'yes'
131-
end
142+
option_parser.on('-i', '--show-indexes',
143+
"List the table's database indexes in the annotation") do
144+
env['show_indexes'] = 'yes'
145+
end
132146

133-
opts.on('-s', '--simple-indexes',
134-
"Concat the column's related indexes in the annotation") do
135-
ENV['simple_indexes'] = 'yes'
136-
end
147+
option_parser.on('-s', '--simple-indexes',
148+
"Concat the column's related indexes in the annotation") do
149+
env['simple_indexes'] = 'yes'
150+
end
137151

138-
opts.on('--model-dir dir',
139-
"Annotate model files stored in dir rather than app/models, separate multiple dirs with commas") do |dir|
140-
ENV['model_dir'] = dir
141-
end
152+
option_parser.on('--model-dir dir',
153+
"Annotate model files stored in dir rather than app/models, separate multiple dirs with commas") do |dir|
154+
env['model_dir'] = dir
155+
end
142156

143-
opts.on('--root-dir dir',
144-
"Annotate files stored within root dir projects, separate multiple dirs with commas") do |dir|
145-
ENV['root_dir'] = dir
146-
end
157+
option_parser.on('--root-dir dir',
158+
"Annotate files stored within root dir projects, separate multiple dirs with commas") do |dir|
159+
env['root_dir'] = dir
160+
end
147161

148-
opts.on('--ignore-model-subdirects',
149-
"Ignore subdirectories of the models directory") do |_dir|
150-
ENV['ignore_model_sub_dir'] = 'yes'
151-
end
162+
option_parser.on('--ignore-model-subdirects',
163+
"Ignore subdirectories of the models directory") do |_dir|
164+
env['ignore_model_sub_dir'] = 'yes'
165+
end
152166

153-
opts.on('--sort',
154-
"Sort columns alphabetically, rather than in creation order") do |_dir|
155-
ENV['sort'] = 'yes'
156-
end
167+
option_parser.on('--sort',
168+
"Sort columns alphabetically, rather than in creation order") do |_dir|
169+
env['sort'] = 'yes'
170+
end
157171

158-
opts.on('--classified-sort',
159-
"Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns") do |_dir|
160-
ENV['classified_sort'] = 'yes'
161-
end
172+
option_parser.on('--classified-sort',
173+
"Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns") do |_dir|
174+
env['classified_sort'] = 'yes'
175+
end
162176

163-
opts.on('-R', '--require path',
164-
"Additional file to require before loading models, may be used multiple times") do |path|
165-
ENV['require'] = if !ENV['require'].blank?
166-
ENV['require'] + ",#{path}"
167-
else
168-
path
169-
end
170-
end
177+
option_parser.on('-R', '--require path',
178+
"Additional file to require before loading models, may be used multiple times") do |path|
179+
env['require'] = if !env['require'].blank?
180+
env['require'] + ",#{path}"
181+
else
182+
path
183+
end
184+
end
171185

172-
opts.on('-e', '--exclude [tests,fixtures,factories,serializers]', Array, "Do not annotate fixtures, test files, factories, and/or serializers") do |exclusions|
173-
exclusions ||= EXCLUSION_LIST
174-
exclusions.each { |exclusion| ENV["exclude_#{exclusion}"] = 'yes' }
175-
end
186+
option_parser.on('-e', '--exclude [tests,fixtures,factories,serializers]', Array, "Do not annotate fixtures, test files, factories, and/or serializers") do |exclusions|
187+
exclusions ||= EXCLUSION_LIST
188+
exclusions.each { |exclusion| env["exclude_#{exclusion}"] = 'yes' }
189+
end
176190

177-
opts.on('-f', '--format [bare|rdoc|markdown]', FORMAT_TYPES, 'Render Schema Infomation as plain/RDoc/Markdown') do |fmt|
178-
ENV["format_#{fmt}"] = 'yes'
179-
end
191+
option_parser.on('-f', '--format [bare|rdoc|markdown]', FORMAT_TYPES, 'Render Schema Infomation as plain/RDoc/Markdown') do |fmt|
192+
env["format_#{fmt}"] = 'yes'
193+
end
180194

181-
opts.on('--force', 'Force new annotations even if there are no changes.') do |_force|
182-
ENV['force'] = 'yes'
183-
end
195+
option_parser.on('--force', 'Force new annotations even if there are no changes.') do |_force|
196+
env['force'] = 'yes'
197+
end
184198

185-
opts.on('--frozen', 'Do not allow to change annotations. Exits non-zero if there are going to be changes to files.') do
186-
ENV['frozen'] = 'yes'
187-
end
199+
option_parser.on('--frozen', 'Do not allow to change annotations. Exits non-zero if there are going to be changes to files.') do
200+
env['frozen'] = 'yes'
201+
end
188202

189-
opts.on('--timestamp', 'Include timestamp in (routes) annotation') do
190-
ENV['timestamp'] = 'true'
191-
end
203+
option_parser.on('--timestamp', 'Include timestamp in (routes) annotation') do
204+
env['timestamp'] = 'true'
205+
end
192206

193-
opts.on('--trace', 'If unable to annotate a file, print the full stack trace, not just the exception message.') do |_value|
194-
ENV['trace'] = 'yes'
195-
end
207+
option_parser.on('--trace', 'If unable to annotate a file, print the full stack trace, not just the exception message.') do |_value|
208+
env['trace'] = 'yes'
209+
end
196210

197-
opts.on('-I', '--ignore-columns REGEX', "don't annotate columns that match a given REGEX (i.e., `annotate -I '^(id|updated_at|created_at)'`") do |regex|
198-
ENV['ignore_columns'] = regex
199-
end
211+
option_parser.on('-I', '--ignore-columns REGEX', "don't annotate columns that match a given REGEX (i.e., `annotate -I '^(id|updated_at|created_at)'`") do |regex|
212+
env['ignore_columns'] = regex
213+
end
200214

201-
opts.on('--ignore-routes REGEX', "don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'`") do |regex|
202-
ENV['ignore_routes'] = regex
203-
end
215+
option_parser.on('--ignore-routes REGEX', "don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'`") do |regex|
216+
env['ignore_routes'] = regex
217+
end
204218

205-
opts.on('--hide-limit-column-types VALUES', "don't show limit for given column types, separated by commas (i.e., `integer,boolean,text`)") do |values|
206-
ENV['hide_limit_column_types'] = values.to_s
207-
end
219+
option_parser.on('--hide-limit-column-types VALUES', "don't show limit for given column types, separated by commas (i.e., `integer,boolean,text`)") do |values|
220+
env['hide_limit_column_types'] = values.to_s
221+
end
208222

209-
opts.on('--hide-default-column-types VALUES', "don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`)") do |values|
210-
ENV['hide_default_column_types'] = values.to_s
211-
end
223+
option_parser.on('--hide-default-column-types VALUES', "don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`)") do |values|
224+
env['hide_default_column_types'] = values.to_s
225+
end
212226

213-
opts.on('--ignore-unknown-models', "don't display warnings for bad model files") do |_values|
214-
ENV['ignore_unknown_models'] = 'true'
215-
end
227+
option_parser.on('--ignore-unknown-models', "don't display warnings for bad model files") do |_values|
228+
env['ignore_unknown_models'] = 'true'
229+
end
216230

217-
opts.on('--with-comment', "include database comments in model annotations") do |_values|
218-
ENV['with_comment'] = 'true'
219-
end
231+
option_parser.on('--with-comment', "include database comments in model annotations") do |_values|
232+
env['with_comment'] = 'true'
220233
end
221234
end
222235

‎spec/annotate/parser_spec.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,9 @@ module Annotate # rubocop:disable Metrics/ModuleLength
345345
context "when ENV['require'] is already set" do
346346
let(:preset_require_value) { 'some_dir/' }
347347
it "appends the path to ENV['require']" do
348-
allow(ENV).to receive(:[]).and_return(preset_require_value)
348+
env = { 'require' => preset_require_value }
349349
expect(ENV).to receive(:[]=).with(env_key, "#{preset_require_value},#{set_value}")
350-
Parser.parse([option, set_value])
350+
Parser.parse([option, set_value], env)
351351
end
352352
end
353353
end

0 commit comments

Comments
 (0)