Skip to content

Commit 639b9ac

Browse files
committed
Address kou's review comments: extract safe_mtime helper, cleanups
* Add RDoc.safe_mtime(file) module method that wraps File.mtime with an explicit rescue SystemCallError. Previously, five call sites used `File.mtime(file) rescue nil`, which silently swallows every StandardError. The narrower rescue lets unrelated bugs surface. * Use the helper at the five sites that needed the same pattern: rdoc.rb's rbs_signature_mtimes, and four spots in server.rb (file_mtimes_for, the change-detection loop, the post-parse mtime record, and the rbs reload block). * Anchor the remove_unparseable file extension regex with \z instead of $ so the match only succeeds at end-of-string. Same change for the tags regex on the next line. * Use the existing rbs_signature_file? predicate in two more spots instead of re-inlining File.extname(file) == '.rbs'.
1 parent 9866f31 commit 639b9ac

3 files changed

Lines changed: 19 additions & 9 deletions

File tree

lib/rdoc.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,16 @@ def self.home
151151
end
152152
end
153153

154+
##
155+
# Returns +File.mtime(file)+, or +nil+ if the file cannot be stat'd
156+
# (missing, permission denied, etc.).
157+
158+
def self.safe_mtime(file)
159+
File.mtime(file)
160+
rescue SystemCallError
161+
nil
162+
end
163+
154164
autoload :RDoc, "#{__dir__}/rdoc/rdoc"
155165

156166
autoload :CrossReference, "#{__dir__}/rdoc/cross_reference"

lib/rdoc/rdoc.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,8 @@ def parse_files(files)
430430

431431
def remove_unparseable(files)
432432
files.reject do |file, *|
433-
file =~ /\.(?:class|eps|erb|rbs|scpt\.txt|svg|ttf|yml)$/i or
434-
(file =~ /tags$/i and
433+
file =~ /\.(?:class|eps|erb|rbs|scpt\.txt|svg|ttf|yml)\z/i or
434+
(file =~ /tags\z/i and
435435
/\A(\f\n[^,]+,\d+$|!_TAG_)/.match?(File.binread(file, 100)))
436436
end
437437
end
@@ -582,7 +582,7 @@ def rbs_signature_files
582582

583583
def rbs_signatures_changed?
584584
current = rbs_signature_mtimes
585-
previous = @last_modified.select { |file, _| File.extname(file) == '.rbs' }
585+
previous = @last_modified.select { |file, _| rbs_signature_file?(file) }
586586

587587
return true unless (previous.keys - current.keys).empty?
588588

@@ -597,7 +597,7 @@ def rbs_signatures_changed?
597597
# and the live server watcher can see signature-only edits.
598598

599599
def record_rbs_signature_mtimes
600-
@last_modified.reject! { |file, _| File.extname(file) == '.rbs' }
600+
@last_modified.reject! { |file, _| rbs_signature_file?(file) }
601601
@last_modified.merge! rbs_signature_mtimes
602602
end
603603

@@ -614,7 +614,7 @@ def rbs_signature_file?(file) # :nodoc:
614614

615615
def rbs_signature_mtimes # :nodoc:
616616
rbs_signature_files.each_with_object({}) do |file, mtimes|
617-
mtime = File.mtime(file) rescue nil
617+
mtime = RDoc.safe_mtime(file)
618618
mtimes[file] = mtime if mtime
619619
end
620620
end

lib/rdoc/server.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ def start_watcher(source_files)
310310

311311
def file_mtimes_for(files)
312312
files.each_with_object({}) do |f, h|
313-
h[f] = File.mtime(f) rescue nil
313+
h[f] = RDoc.safe_mtime(f)
314314
end
315315
end
316316

@@ -334,7 +334,7 @@ def check_for_changes
334334
next
335335
end
336336

337-
current_mtime = File.mtime(file) rescue nil
337+
current_mtime = RDoc.safe_mtime(file)
338338
next unless current_mtime
339339
next unless old_mtime.nil? || current_mtime > old_mtime
340340

@@ -397,7 +397,7 @@ def reparse_and_refresh(changed_files, removed_files, rbs_changed: false)
397397
begin
398398
@store.clear_file_contributions(relative, keep_position: true)
399399
@rdoc.parse_file(f)
400-
@file_mtimes[f] = File.mtime(f) rescue nil
400+
@file_mtimes[f] = RDoc.safe_mtime(f)
401401
rescue => e
402402
$stderr.puts "Error parsing #{f}: #{e.message}"
403403
end
@@ -415,7 +415,7 @@ def reparse_and_refresh(changed_files, removed_files, rbs_changed: false)
415415
@rdoc.load_rbs_signatures
416416
@rdoc.record_rbs_signature_mtimes
417417
@rdoc.rbs_signature_files.each do |file|
418-
@file_mtimes[file] = File.mtime(file) rescue nil
418+
@file_mtimes[file] = RDoc.safe_mtime(file)
419419
end
420420
end
421421
$stderr.puts "Reloaded RBS signatures (#{duration_ms}ms)" if rbs_changed

0 commit comments

Comments
 (0)