Skip to content

Commit e9d0be4

Browse files
committed
Address kou and tompng's review comments
* 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'. * Move @type_name_lookup invalidation out of Store#complete and into an explicit Store#invalidate_type_name_lookup call in the server's reparse_and_refresh — only the server's class-set churn actually needs it. complete itself shouldn't carry the responsibility. * Fold extract_type_signature! into parse_comment_text_to_directives so a comment is parsed in one place. consecutive_comment now just returns whatever the parse function gives it.
1 parent f8f7048 commit e9d0be4

5 files changed

Lines changed: 34 additions & 17 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/parser/ruby.rb

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -461,16 +461,15 @@ def skip_comments_until(line_no_until)
461461
def consecutive_comment(line_no)
462462
return unless @unprocessed_comments.first&.first == line_no
463463
_line_no, start_line, text = @unprocessed_comments.shift
464-
type_signature_lines = extract_type_signature!(text, start_line)
465-
result = parse_comment_text_to_directives(text, start_line)
466-
return unless result
467-
comment, directives = result
468-
[comment, directives, type_signature_lines]
464+
parse_comment_text_to_directives(text, start_line)
469465
end
470466

471-
# Parses comment text and returns a pair of RDoc::Comment and directives
467+
# Parses comment text and returns +[RDoc::Comment, directives, type_signature_lines]+,
468+
# or +nil+ if the comment is a section header (which has no associated code
469+
# object).
472470

473471
def parse_comment_text_to_directives(comment_text, start_line) # :nodoc:
472+
type_signature_lines = extract_type_signature!(comment_text, start_line)
474473
comment_text, directives = @preprocess.parse_comment(comment_text, start_line, :ruby)
475474
comment = RDoc::Comment.new(comment_text, @top_level, :ruby)
476475
comment.normalized = true
@@ -484,7 +483,7 @@ def parse_comment_text_to_directives(comment_text, start_line) # :nodoc:
484483
return
485484
end
486485
@preprocess.run_post_processes(comment, @container)
487-
[comment, directives]
486+
[comment, directives, type_signature_lines]
488487
end
489488

490489
# Extracts the comment for this section from the normalized comment block.

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: 5 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
@@ -409,13 +409,14 @@ def reparse_and_refresh(changed_files, removed_files, rbs_changed: false)
409409
end
410410

411411
@store.complete(@options.visibility)
412+
@store.invalidate_type_name_lookup unless changed_files.empty? && removed_files.empty?
412413

413414
if rbs_changed || !changed_files.empty?
414415
duration_ms = measure do
415416
@rdoc.load_rbs_signatures
416417
@rdoc.record_rbs_signature_mtimes
417418
@rdoc.rbs_signature_files.each do |file|
418-
@file_mtimes[file] = File.mtime(file) rescue nil
419+
@file_mtimes[file] = RDoc.safe_mtime(file)
419420
end
420421
end
421422
$stderr.puts "Reloaded RBS signatures (#{duration_ms}ms)" if rbs_changed

lib/rdoc/store.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,14 @@ def all_classes_and_modules
350350
# (where multiple classes share the same name) are excluded to avoid
351351
# wrong links. Cached after first call.
352352

353+
##
354+
# Invalidates the cached type name lookup. Server mode calls this after
355+
# re-parsing changes the set of classes and modules.
356+
357+
def invalidate_type_name_lookup # :nodoc:
358+
@type_name_lookup = nil
359+
end
360+
353361
def type_name_lookup
354362
@type_name_lookup ||= begin
355363
lookup = {}
@@ -530,7 +538,6 @@ def clean_cache_collection(collection) # :nodoc:
530538
# See also RDoc::Context#remove_from_documentation?
531539

532540
def complete(min_visibility)
533-
@type_name_lookup = nil
534541
fix_basic_object_inheritance
535542

536543
# cache included modules before they are removed from the documentation

0 commit comments

Comments
 (0)