Skip to content

Commit d4eacff

Browse files
sferiksferik
committed
Fix Metrics/MethodLength offenses by refactoring
1 parent 2e1ee5a commit d4eacff

8 files changed

Lines changed: 103 additions & 104 deletions

File tree

‎.rubocop.yml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,6 @@ Metrics/BlockNesting:
7676
Metrics/ClassLength:
7777
Max: 300
7878

79-
Metrics/MethodLength:
80-
Description: Checks if the length of a method exceeds some maximum value.
81-
CountComments: false
82-
Max: 12 # TODO: Lower to 10
83-
8479
Style/Next:
8580
Description: Prefer `if` guards over `next` inside loops for readability.
8681
Enabled: false

‎lib/simplecov.rb‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -405,17 +405,15 @@ def lookup_corresponding_ruby_coverage_name(criterion)
405405
# the line-by-line coverage to zero (if relevant) or nil (comments / whitespace etc).
406406
#
407407
def add_not_loaded_files(result)
408-
not_loaded_files = Set.new
409-
410-
if tracked_files
411-
result = result.dup
412-
Dir[tracked_files].each do |file|
413-
absolute_path = File.expand_path(file)
414-
unless result.key?(absolute_path)
415-
result[absolute_path] = SimulateCoverage.call(absolute_path)
416-
not_loaded_files << absolute_path
417-
end
418-
end
408+
return [result, Set.new] unless tracked_files
409+
410+
result = result.dup
411+
not_loaded_files = Dir[tracked_files].each_with_object(Set.new) do |file, set|
412+
absolute_path = File.expand_path(file)
413+
next if result.key?(absolute_path)
414+
415+
result[absolute_path] = SimulateCoverage.call(absolute_path)
416+
set << absolute_path
419417
end
420418

421419
[result, not_loaded_files]

‎lib/simplecov/command_guesser.rb‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,36 +28,36 @@ def parallel_data
2828
"(#{number}/#{ENV.fetch('PARALLEL_TEST_GROUPS', nil)})"
2929
end
3030

31+
COMMAND_LINE_FRAMEWORKS = {
32+
%r{test/functional/} => "Functional Tests",
33+
%r{test/\{.*functional.*\}/} => "Functional Tests",
34+
%r{test/integration/} => "Integration Tests",
35+
%r{test/} => "Unit Tests",
36+
/spec/ => "RSpec",
37+
/cucumber/ => "Cucumber Features",
38+
/features/ => "Cucumber Features"
39+
}.freeze
40+
private_constant :COMMAND_LINE_FRAMEWORKS
41+
3142
def from_command_line_options
32-
case original_run_command
33-
when /test\/functional\//, /test\/\{.*functional.*\}\//
34-
"Functional Tests"
35-
when /test\/integration\//
36-
"Integration Tests"
37-
when /test\//
38-
"Unit Tests"
39-
when /spec/
40-
"RSpec"
41-
when /cucumber/, /features/
42-
"Cucumber Features"
43-
end
43+
COMMAND_LINE_FRAMEWORKS.find { |pattern, _| pattern.match?(original_run_command.to_s) }&.last
4444
end
4545

46+
DEFINED_CONSTANT_FRAMEWORKS = [
47+
["RSpec", -> { defined?(::RSpec) }],
48+
["Unit Tests", -> { defined?(Test::Unit) }],
49+
["Minitest", -> { defined?(::Minitest) }],
50+
["MiniTest", -> { defined?(MiniTest) }]
51+
].freeze
52+
private_constant :DEFINED_CONSTANT_FRAMEWORKS
53+
54+
# If the command regexps fail, let's try checking defined constants.
4655
def from_defined_constants
47-
# If the command regexps fail, let's try checking defined constants.
48-
if defined?(RSpec)
49-
"RSpec"
50-
elsif defined?(Test::Unit)
51-
"Unit Tests"
52-
elsif defined?(Minitest)
53-
"Minitest"
54-
elsif defined?(MiniTest)
55-
"MiniTest"
56-
else
57-
# TODO: Provide link to docs/wiki article
58-
warn "SimpleCov failed to recognize the test framework and/or suite used. Please specify manually using SimpleCov.command_name 'Unit Tests'."
59-
"Unknown Test Framework"
60-
end
56+
DEFINED_CONSTANT_FRAMEWORKS.each { |name, defined_check| return name if defined_check.call }
57+
58+
# TODO: Provide link to docs/wiki article
59+
warn "SimpleCov failed to recognize the test framework and/or suite used. Please specify manually using SimpleCov.command_name 'Unit Tests'."
60+
"Unknown Test Framework"
6161
end
6262
end
6363
end

‎lib/simplecov/filter.rb‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,19 @@ def self.build_filter(filter_argument)
3535
end
3636

3737
def self.class_for_argument(filter_argument)
38-
case filter_argument
39-
when String
40-
SimpleCov::StringFilter
41-
when Regexp
42-
SimpleCov::RegexFilter
43-
when Array
44-
SimpleCov::ArrayFilter
45-
when Proc
46-
SimpleCov::BlockFilter
47-
else
48-
raise ArgumentError, "You have provided an unrecognized filter type"
49-
end
38+
filter_classes_by_argument_type.find { |type, _| filter_argument.is_a?(type) }&.last ||
39+
raise(ArgumentError, "You have provided an unrecognized filter type")
40+
end
41+
42+
def self.filter_classes_by_argument_type
43+
@filter_classes_by_argument_type ||= {
44+
String => SimpleCov::StringFilter,
45+
Regexp => SimpleCov::RegexFilter,
46+
Array => SimpleCov::ArrayFilter,
47+
Proc => SimpleCov::BlockFilter
48+
}.freeze
5049
end
50+
private_class_method :filter_classes_by_argument_type
5151
end
5252

5353
class StringFilter < SimpleCov::Filter
@@ -63,23 +63,26 @@ def matches?(source_file)
6363
private
6464

6565
def segment_pattern
66-
@segment_pattern ||= begin
67-
normalized = filter_argument.delete_prefix("/")
68-
escaped = Regexp.escape(normalized)
69-
boundary = '(?:\A|/)'
70-
if normalized.include?(".")
71-
# Contains a dot — looks like a filename pattern. Allow substring
72-
# match within the last path segment (e.g. "test.rb" matches
73-
# "faked_test.rb") while still anchoring to a segment boundary.
74-
/#{boundary}[^\/]*#{escaped}/
75-
elsif normalized.end_with?("/")
76-
# Trailing slash signals directory-only matching
77-
/#{boundary}#{escaped}/
78-
else
79-
# No dot — looks like a directory or path. Require segment-boundary
80-
# match so "lib" matches "lib/" but not "library/".
81-
/#{boundary}#{escaped}(?=[\/.]|\z)/
82-
end
66+
@segment_pattern ||= compute_segment_pattern
67+
end
68+
69+
def compute_segment_pattern
70+
normalized = filter_argument.delete_prefix("/")
71+
escaped = Regexp.escape(normalized)
72+
boundary = '(?:\A|/)'
73+
74+
if normalized.include?(".")
75+
# Filename pattern (e.g. "test.rb" matches "faked_test.rb"): allow
76+
# substring match within the last path segment, anchored to a
77+
# segment boundary.
78+
/#{boundary}[^\/]*#{escaped}/
79+
elsif normalized.end_with?("/")
80+
# Trailing slash signals directory-only matching.
81+
/#{boundary}#{escaped}/
82+
else
83+
# Directory or path: require a segment-boundary match so "lib"
84+
# matches "lib/" but not "library/".
85+
/#{boundary}#{escaped}(?=[\/.]|\z)/
8386
end
8487
end
8588
end

‎lib/simplecov/formatter/simple_formatter.rb‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@ module Formatter
88
class SimpleFormatter
99
# Takes a SimpleCov::Result and generates a string out of it
1010
def format(result)
11-
output = +""
12-
result.groups.each do |name, files|
13-
output << "Group: #{name}\n"
14-
output << ("=" * 40)
15-
output << "\n"
16-
files.each do |file|
17-
output << "#{file.filename} (coverage: #{file.covered_percent.floor(2)}%)\n"
18-
end
19-
output << "\n"
20-
end
21-
output
11+
result.groups.map { |name, files| format_group(name, files) }.join
12+
end
13+
14+
private
15+
16+
def format_group(name, files)
17+
header = "Group: #{name}\n#{'=' * 40}\n"
18+
body = files.map { |file| "#{file.filename} (coverage: #{file.covered_percent.floor(2)}%)\n" }.join
19+
"#{header}#{body}\n"
2220
end
2321
end
2422
end

‎lib/simplecov/result_adapter.rb‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,15 @@ def adapt_oneshot_lines_if_needed(file_name, cover_statistic)
5252
return unless cover_statistic.key?(:oneshot_lines)
5353

5454
oneshot_lines = cover_statistic.delete(:oneshot_lines)
55-
line_stub = begin
56-
Coverage.line_stub(file_name)
57-
rescue Errno::ENOENT, SyntaxError
58-
Array.new(oneshot_lines.max || 0, nil)
59-
end
60-
oneshot_lines.each do |covered_line|
61-
line_stub[covered_line - 1] = 1
62-
end
55+
line_stub = build_line_stub(file_name, oneshot_lines)
56+
oneshot_lines.each { |covered_line| line_stub[covered_line - 1] = 1 }
6357
cover_statistic[:lines] = line_stub
6458
end
59+
60+
def build_line_stub(file_name, oneshot_lines)
61+
Coverage.line_stub(file_name)
62+
rescue Errno::ENOENT, SyntaxError
63+
Array.new(oneshot_lines.max || 0, nil)
64+
end
6565
end
6666
end

‎lib/simplecov/result_merger.rb‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,19 @@ def store_result(result) # rubocop:disable Naming/PredicateMethod
142142
command_name, data = result.to_hash.first
143143
new_resultset[command_name] = data
144144

145-
# Write to a temp file first, then atomically rename to avoid
146-
# other processes reading a truncated/incomplete .resultset.json
147-
temp_path = "#{resultset_path}.#{Process.pid}.tmp"
148-
File.open(temp_path, "w") do |f_|
149-
f_.puts JSON.pretty_generate(new_resultset)
150-
end
151-
File.rename(temp_path, resultset_path)
145+
atomic_write_resultset(new_resultset)
152146
end
153147
true
154148
end
155149

150+
# Write to a temp file first, then atomically rename to avoid other
151+
# processes reading a truncated/incomplete .resultset.json.
152+
def atomic_write_resultset(resultset)
153+
temp_path = "#{resultset_path}.#{Process.pid}.tmp"
154+
File.open(temp_path, "w") { |f| f.puts JSON.pretty_generate(resultset) }
155+
File.rename(temp_path, resultset_path)
156+
end
157+
156158
# Ensure only one process is reading or writing the resultset at any
157159
# given time
158160
def synchronize_resultset

‎lib/simplecov/source_file.rb‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -388,16 +388,19 @@ def parse_ruby_array_string(str) # rubocop:disable Metrics
388388
def scan_quoted_string(scanner)
389389
string = +""
390390
until scanner.scan(/"/)
391-
if scanner.scan(/\\(.)/)
392-
string << scanner[1]
393-
elsif scanner.scan(/[^"\\]+/)
394-
string << scanner.matched
395-
else
396-
break
397-
end
391+
chunk = scan_quoted_string_chunk(scanner)
392+
break if chunk.nil?
393+
394+
string << chunk
398395
end
399396
string
400397
end
398+
399+
def scan_quoted_string_chunk(scanner)
400+
return scanner[1] if scanner.scan(/\\(.)/)
401+
402+
scanner.matched if scanner.scan(/[^"\\]+/)
403+
end
401404
# rubocop:enable Style/RedundantRegexpArgument
402405

403406
def build_branches_from(condition, branches)

0 commit comments

Comments
 (0)