Apply suggestions from code review

This commit is contained in:
fn ⌃ ⌥ 2021-10-27 10:08:36 -07:00
parent d51ef27d54
commit b7bb282b99
5 changed files with 26 additions and 31 deletions

View file

@ -19,8 +19,7 @@ module Ameba
def initialize(path, issues_by_iteration, loop_start = -1) def initialize(path, issues_by_iteration, loop_start = -1)
root_cause = root_cause =
issues_by_iteration[loop_start..-1] issues_by_iteration[loop_start..-1]
.map(&.map(&.rule.name).uniq!.join(", ")) .join(" -> ", &.map(&.rule.name).uniq!.join(", "))
.join(" -> ")
message = String.build do |io| message = String.build do |io|
io << "Infinite loop" io << "Infinite loop"
@ -210,7 +209,7 @@ module Ameba
private def check_for_infinite_loop(source, corrected_issues, processed_sources) private def check_for_infinite_loop(source, corrected_issues, processed_sources)
checksum = Digest::SHA1.hexdigest(source.code) checksum = Digest::SHA1.hexdigest(source.code)
if (loop_start = processed_sources.index(checksum)) if loop_start = processed_sources.index(checksum)
raise InfiniteCorrectionLoopError.new( raise InfiniteCorrectionLoopError.new(
source.path, source.path,
corrected_issues, corrected_issues,

View file

@ -11,9 +11,6 @@ module Ameba
# Crystal code (content of a source file). # Crystal code (content of a source file).
getter code : String getter code : String
@lines : Array(String)?
@ast : Crystal::ASTNode?
# Creates a new source by `code` and `path`. # Creates a new source by `code` and `path`.
# #
# For example: # For example:
@ -50,9 +47,7 @@ module Ameba
# source.lines # => ["a = 1", "b = 2"] # source.lines # => ["a = 1", "b = 2"]
# ``` # ```
# #
def lines : Array(String) getter lines : Array(String) { code.split('\n') }
@lines ||= code.split('\n')
end
# Returns AST nodes constructed by `Crystal::Parser`. # Returns AST nodes constructed by `Crystal::Parser`.
# #
@ -61,12 +56,11 @@ module Ameba
# source.ast # source.ast
# ``` # ```
# #
def ast : Crystal::ASTNode getter ast : Crystal::ASTNode do
@ast ||= Crystal::Parser.new(code)
Crystal::Parser.new(code) .tap(&.wants_doc = true)
.tap(&.wants_doc = true) .tap(&.filename = path)
.tap(&.filename = path) .parse
.parse
end end
getter fullpath : String do getter fullpath : String do

View file

@ -8,7 +8,7 @@ class Ameba::Source
def initialize(code : String) def initialize(code : String)
@rewriter = Rewriter.new(code) @rewriter = Rewriter.new(code)
@line_sizes = code.lines(chomp: false).map(&.size) @line_sizes = code.each_line(chomp: false).map(&.size).to_a
end end
# Replaces the code of the given range with *content*. # Replaces the code of the given range with *content*.

View file

@ -59,9 +59,9 @@ class Ameba::Source
# The updates are organized in a tree, according to the ranges they act on # The updates are organized in a tree, according to the ranges they act on
# (where children are strictly contained by their parent). # (where children are strictly contained by their parent).
class Rewriter class Rewriter
getter code getter code : String
def initialize(@code : String) def initialize(@code)
@action_root = Rewriter::Action.new(0, code.size) @action_root = Rewriter::Action.new(0, code.size)
end end

View file

@ -73,21 +73,23 @@ class Ameba::Source::Rewriter
family = analyse_hierarchy(action) family = analyse_hierarchy(action)
sibling_left, sibling_right = family[:sibling_left], family[:sibling_right] sibling_left, sibling_right = family[:sibling_left], family[:sibling_right]
if (fusible = family[:fusible]) if fusible = family[:fusible]
child = family[:child] child = family[:child]
child ||= [] of Action child ||= [] of Action
fuse_deletions(action, fusible, sibling_left + child + sibling_right) fuse_deletions(action, fusible, sibling_left + child + sibling_right)
else else
extra_sibling = if (parent = family[:parent]) extra_sibling =
# action should be a descendant of one of the children case
parent.do_combine(action) when parent = family[:parent]
elsif (child = family[:child]) # action should be a descendant of one of the children
# or it should become the parent of some of the children, parent.do_combine(action)
action.with(children: child).combine_children(action.children) when child = family[:child]
else # or it should become the parent of some of the children,
# or else it should become an additional child action.with(children: child).combine_children(action.children)
action else
end # or else it should become an additional child
action
end
self.with(children: sibling_left + [extra_sibling] + sibling_right) self.with(children: sibling_left + [extra_sibling] + sibling_right)
end end
end end
@ -102,8 +104,8 @@ class Ameba::Source::Rewriter
protected def fuse_deletions(action, fusible, other_siblings) protected def fuse_deletions(action, fusible, other_siblings)
without_fusible = self.with(children: other_siblings) without_fusible = self.with(children: other_siblings)
fusible = [action] + fusible fusible = [action] + fusible
fused_begin_pos = fusible.map(&.begin_pos).min fused_begin_pos = fusible.min_of(&.begin_pos)
fused_end_pos = fusible.map(&.end_pos).max fused_end_pos = fusible.max_of(&.end_pos)
fused_deletion = action.with(begin_pos: fused_begin_pos, end_pos: fused_end_pos) fused_deletion = action.with(begin_pos: fused_begin_pos, end_pos: fused_end_pos)
without_fusible.do_combine(fused_deletion) without_fusible.do_combine(fused_deletion)
end end