Skip to content

Commit 22675b2

Browse files
authored
Fix annotations swallowing comments (#72)
This PR attempts to fix a long standing bug that was carried over from the original gem. Comments near an annotation would get removed when adding or updating annotations. ctran/annotate_models#951, https://github.com/ctran/annotate_models/#warning **Context** Prior to this change, the AnnotateRb and the old gem Annotate would annotate model files in 1 of 2 ways: 1. The "update" method, existing annotations would be found using a regex and then new annotations would be changed in place. If a human made comment was added after the annotation block **without** a new line, it would get removed during the update process. 2. The "overwrite" method, which would happen when annotations did not exist OR if the `:force` option was set to true. A regex would be used to find annotation, remove it from the file, and then add annotations again to the file depending on options such as `:position`. Relying on regexes to identify annotations in a file made it hard to reason about and hard to change, so most of them were removed and replaced with a custom Ruby file parser using Ripper [1], which makes working with Ruby files deterministic. This PR makes the following behavioral changes: - When updating annotations (i.e. annotations exist && `force: false`), it only updates the annotations and should no longer delete any human comments that are before or after an annotation block - When generating (no annotations exist in the file) or re-generating annotations (`force: true`), it removes the annotation block and surrounding whitespace, if it exists, and then depending on the `position` option, will add annotations before the class declaration or after the class declaration. In the case of re-generating annotations, any surrounding whitespace will be preserved assuming annotations are being written to the same position. [1] A great write up on Ripper: https://kddnewton.com/2022/02/14/formatting-ruby-part-1.html
1 parent 643aff0 commit 22675b2

21 files changed

+2792
-541
lines changed

lib/annotate_rb/model_annotator.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ module ModelAnnotator
77
autoload :BadModelFileError, "annotate_rb/model_annotator/bad_model_file_error"
88
autoload :FileNameResolver, "annotate_rb/model_annotator/file_name_resolver"
99
autoload :SingleFileAnnotationRemover, "annotate_rb/model_annotator/single_file_annotation_remover"
10-
autoload :AnnotationPatternGenerator, "annotate_rb/model_annotator/annotation_pattern_generator"
1110
autoload :ModelClassGetter, "annotate_rb/model_annotator/model_class_getter"
1211
autoload :ModelFilesGetter, "annotate_rb/model_annotator/model_files_getter"
1312
autoload :SingleFileAnnotator, "annotate_rb/model_annotator/single_file_annotator"
@@ -22,7 +21,6 @@ module ModelAnnotator
2221
autoload :SingleFileRemoveAnnotationInstruction, "annotate_rb/model_annotator/single_file_remove_annotation_instruction"
2322
autoload :AnnotationDiffGenerator, "annotate_rb/model_annotator/annotation_diff_generator"
2423
autoload :AnnotationDiff, "annotate_rb/model_annotator/annotation_diff"
25-
autoload :FileComponents, "annotate_rb/model_annotator/file_components"
2624
autoload :ProjectAnnotator, "annotate_rb/model_annotator/project_annotator"
2725
autoload :ProjectAnnotationRemover, "annotate_rb/model_annotator/project_annotation_remover"
2826
autoload :AnnotatedFile, "annotate_rb/model_annotator/annotated_file"

lib/annotate_rb/model_annotator/annotated_file/generator.rb

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,56 @@ module ModelAnnotator
55
module AnnotatedFile
66
# Generates the file with fresh annotations
77
class Generator
8-
def initialize(file_components, annotation_position, options)
9-
@file_components = file_components
8+
def initialize(file_content, new_annotations, annotation_position, options)
109
@annotation_position = annotation_position
1110
@options = options
1211

13-
@new_wrapped_annotations = wrapped_content(@file_components.new_annotations)
12+
@new_wrapped_annotations = wrapped_content(new_annotations)
13+
14+
@new_annotations = new_annotations
15+
@file_content = file_content
16+
17+
@parsed_file = FileParser::ParsedFile.new(@file_content, @new_annotations, options).parse
1418
end
1519

1620
def generate
1721
# Need to keep `.to_s` for now since the it can be either a String or Symbol
1822
annotation_write_position = @options[@annotation_position].to_s
1923

24+
# New method: first remove annotations
25+
content_without_annotations = if @parsed_file.has_annotations?
26+
@file_content.sub(@parsed_file.annotations_with_whitespace, "")
27+
elsif @options[:force]
28+
@file_content.sub(@parsed_file.annotations_with_whitespace, "")
29+
else
30+
@file_content
31+
end
32+
33+
# We need to get class start and class end depending on the position
34+
parsed = FileParser::CustomParser.new(content_without_annotations, "", 0).tap(&:parse)
35+
36+
same_write_position = @parsed_file.has_annotations? && @parsed_file.annotation_position.to_s == annotation_write_position
37+
38+
# Could error if there's no class or module declaration
39+
_constant_name, line_number_before = parsed.starts.first
40+
content_with_annotations_written_before = []
41+
content_with_annotations_written_before << content_without_annotations.lines[0...line_number_before]
42+
content_with_annotations_written_before << $/ if @parsed_file.has_leading_whitespace? && same_write_position
43+
content_with_annotations_written_before << @new_wrapped_annotations.lines
44+
content_with_annotations_written_before << $/ if @parsed_file.has_trailing_whitespace? && same_write_position
45+
content_with_annotations_written_before << content_without_annotations.lines[line_number_before..]
46+
47+
_constant_name, line_number_after = parsed.ends.last
48+
content_with_annotations_written_after = []
49+
content_with_annotations_written_after << content_without_annotations.lines[0..line_number_after]
50+
content_with_annotations_written_after << $/
51+
content_with_annotations_written_after << @new_wrapped_annotations.lines
52+
content_with_annotations_written_after << content_without_annotations.lines[(line_number_after + 1)..]
53+
2054
_content = if %w[after bottom].include?(annotation_write_position)
21-
@file_components.magic_comments + (@file_components.pure_file_content.rstrip + "\n\n" + @new_wrapped_annotations)
22-
elsif @file_components.magic_comments.empty?
23-
@file_components.magic_comments + @new_wrapped_annotations + @file_components.pure_file_content.lstrip
55+
content_with_annotations_written_after.join
2456
else
25-
@file_components.magic_comments + "\n" + @new_wrapped_annotations + @file_components.pure_file_content.lstrip
57+
content_with_annotations_written_before.join
2658
end
2759
end
2860

lib/annotate_rb/model_annotator/annotated_file/updater.rb

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,21 @@ module ModelAnnotator
55
module AnnotatedFile
66
# Updates existing annotations
77
class Updater
8-
def initialize(file_components, annotation_position, options)
9-
@file_components = file_components
10-
@annotation_position = annotation_position
8+
def initialize(file_content, new_annotations, _annotation_position, options)
119
@options = options
1210

13-
@new_wrapped_annotations = wrapped_content(@file_components.new_annotations)
11+
@new_annotations = new_annotations
12+
@file_content = file_content
13+
14+
@parsed_file = FileParser::ParsedFile.new(@file_content, @new_annotations, options).parse
1415
end
1516

1617
def update
17-
return "" if !@file_components.has_annotations?
18-
19-
annotation_pattern = AnnotationPatternGenerator.call(@options)
18+
return "" if !@parsed_file.has_annotations?
2019

21-
new_annotation = @file_components.space_before_annotation + @new_wrapped_annotations + @file_components.space_after_annotation
20+
new_annotation = wrapped_content(@new_annotations)
2221

23-
_content = @file_components.current_file_content.sub(annotation_pattern, new_annotation)
22+
_content = @file_content.sub(@parsed_file.annotations, new_annotation)
2423
end
2524

2625
private

lib/annotate_rb/model_annotator/annotation_pattern_generator.rb

Lines changed: 0 additions & 19 deletions
This file was deleted.

lib/annotate_rb/model_annotator/file_components.rb

Lines changed: 0 additions & 81 deletions
This file was deleted.

lib/annotate_rb/model_annotator/file_parser.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
module AnnotateRb
44
module ModelAnnotator
55
module FileParser
6-
autoload :MagicCommentParser, "annotate_rb/model_annotator/file_parser/magic_comment_parser"
6+
autoload :AnnotationFinder, "annotate_rb/model_annotator/file_parser/annotation_finder"
7+
autoload :CustomParser, "annotate_rb/model_annotator/file_parser/custom_parser"
8+
autoload :ParsedFile, "annotate_rb/model_annotator/file_parser/parsed_file"
9+
autoload :ParsedFileResult, "annotate_rb/model_annotator/file_parser/parsed_file_result"
710
end
811
end
912
end
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# frozen_string_literal: true
2+
3+
module AnnotateRb
4+
module ModelAnnotator
5+
module FileParser
6+
class AnnotationFinder
7+
COMPAT_PREFIX = "== Schema Info"
8+
COMPAT_PREFIX_MD = "## Schema Info"
9+
DEFAULT_ANNOTATION_ENDING = "#"
10+
11+
class MalformedAnnotation < StandardError; end
12+
13+
class NoAnnotationFound < StandardError; end
14+
15+
# Returns the line index (not the line number) that the annotation starts.
16+
attr_reader :annotation_start
17+
# Returns the line index (not the line number) that the annotation ends, inclusive.
18+
attr_reader :annotation_end
19+
20+
attr_reader :parser
21+
22+
def initialize(content, wrapper_open, wrapper_close)
23+
@content = content
24+
@wrapper_open = wrapper_open
25+
@wrapper_close = wrapper_close
26+
@annotation_start = nil
27+
@annotation_end = nil
28+
@parser = nil
29+
end
30+
31+
# Find the annotation's line start and line end
32+
def run
33+
# CustomParser returns line numbers as 0-indexed
34+
@parser = FileParser::CustomParser.new(@content, "", 0).tap(&:parse)
35+
comments = @parser.comments
36+
37+
start = comments.find_index { |comment, _| comment.include?(COMPAT_PREFIX) || comment.include?(COMPAT_PREFIX_MD) }
38+
raise NoAnnotationFound if start.nil? # Stop execution because we did not find
39+
40+
if @wrapper_open
41+
prev_comment, _prev_line_number = comments[start - 1]
42+
43+
# Change start to the line before if wrapper_open is defined and we find the wrapper open comment
44+
if prev_comment&.include?(@wrapper_open)
45+
start -= 1
46+
end
47+
end
48+
49+
# Find a contiguous block of comments from the starting point
50+
ending = start
51+
while ending < comments.size - 1
52+
_comment, line_number = comments[ending]
53+
_next_comment, next_line_number = comments[ending + 1]
54+
55+
if next_line_number - line_number == 1
56+
ending += 1
57+
else
58+
break
59+
end
60+
end
61+
62+
raise MalformedAnnotation if start == ending
63+
64+
if @wrapper_close
65+
if comments[ending].first.include?(@wrapper_close)
66+
# We can end here because it's the end of the annotation block
67+
else
68+
# Walk back until we find the end of the annotation comment block or the wrapper close to be flexible
69+
# We check if @wrapper_close is a substring because `comments` contains strings with the comment character
70+
while ending > start && comments[ending].first != DEFAULT_ANNOTATION_ENDING && !comments[ending].first.include?(@wrapper_close)
71+
ending -= 1
72+
end
73+
end
74+
else
75+
# Walk back until we find the end of the annotation comment block
76+
while ending > start && comments[ending].first != DEFAULT_ANNOTATION_ENDING
77+
ending -= 1
78+
end
79+
end
80+
81+
# We want .last because we want the line indexes
82+
@annotation_start = comments[start].last
83+
@annotation_end = comments[ending].last
84+
85+
[@annotation_start, @annotation_end]
86+
end
87+
88+
def annotation
89+
@annotation ||=
90+
begin
91+
lines = @content.lines
92+
lines[@annotation_start..@annotation_end].join
93+
end
94+
end
95+
96+
# Returns true if annotations are detected in the file content
97+
def annotated?
98+
@annotation_start && @annotation_end
99+
end
100+
end
101+
end
102+
end
103+
end

0 commit comments

Comments
 (0)