From e54029d8eda4fb527b723ee1f5185ea2d12767fb Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 15 Nov 2022 20:45:35 +0100 Subject: [PATCH] Few more readability refactors --- src/ameba/formatter/disabled_formatter.cr | 8 +-- src/ameba/formatter/dot_formatter.cr | 2 + src/ameba/formatter/explain_formatter.cr | 11 ++-- src/ameba/formatter/flycheck_formatter.cr | 13 ++--- src/ameba/formatter/json_formatter.cr | 65 ++++++++++++++--------- src/ameba/formatter/todo_formatter.cr | 7 +-- src/ameba/rule/style/guard_clause.cr | 10 ++-- src/ameba/rule/style/redundant_begin.cr | 4 +- src/ameba/rule/style/verbose_block.cr | 5 +- src/ameba/runner.cr | 1 - 10 files changed, 72 insertions(+), 54 deletions(-) diff --git a/src/ameba/formatter/disabled_formatter.cr b/src/ameba/formatter/disabled_formatter.cr index cbe7905b..37865c0f 100644 --- a/src/ameba/formatter/disabled_formatter.cr +++ b/src/ameba/formatter/disabled_formatter.cr @@ -6,10 +6,10 @@ module Ameba::Formatter sources.each do |source| source.issues.select(&.disabled?).each do |e| - if loc = e.location - output << "#{source.path}:#{loc.line_number}".colorize(:cyan) - output << " #{e.rule.name}\n" - end + next unless loc = e.location + + output << "#{source.path}:#{loc.line_number}".colorize(:cyan) + output << " #{e.rule.name}\n" end end end diff --git a/src/ameba/formatter/dot_formatter.cr b/src/ameba/formatter/dot_formatter.cr index c19bc354..788d9135 100644 --- a/src/ameba/formatter/dot_formatter.cr +++ b/src/ameba/formatter/dot_formatter.cr @@ -72,6 +72,7 @@ module Ameba::Formatter private def finished_in_message(started, finished) return unless started && finished + "Finished in #{to_human(finished - started)}".colorize(:default) end @@ -92,6 +93,7 @@ module Ameba::Formatter minutes = span.minutes seconds = span.seconds + "#{minutes}:#{seconds < 10 ? "0" : ""}#{seconds} minutes" end diff --git a/src/ameba/formatter/explain_formatter.cr b/src/ameba/formatter/explain_formatter.cr index ebd985a1..a24c5bd1 100644 --- a/src/ameba/formatter/explain_formatter.cr +++ b/src/ameba/formatter/explain_formatter.cr @@ -12,19 +12,22 @@ module Ameba::Formatter getter output : IO::FileDescriptor | IO::Memory getter location : Crystal::Location - # Creates a new instance of ExplainFormatter. - # Accepts *output* which indicates the io where the explanation will be wrtitten to. + # Creates a new instance of `ExplainFormatter`. + # + # Accepts *output* which indicates the io where the explanation will be written to. # Second argument is *location* which indicates the location to explain. # # ``` # ExplainFormatter.new output, - # {file: path, line: line_number, column: column_number} + # file: path, + # line: line_number, + # column: column_number # ``` def initialize(@output, location) @location = Crystal::Location.new(location[:file], location[:line], location[:column]) end - # Reports the explainations at the *@location*. + # Reports the explanations at the *@location*. def finished(sources) source = sources.find(&.path.==(@location.filename)) return unless source diff --git a/src/ameba/formatter/flycheck_formatter.cr b/src/ameba/formatter/flycheck_formatter.cr index 63039b82..936bb001 100644 --- a/src/ameba/formatter/flycheck_formatter.cr +++ b/src/ameba/formatter/flycheck_formatter.cr @@ -6,12 +6,13 @@ module Ameba::Formatter source.issues.each do |e| next if e.disabled? next if e.correctable? && config[:autocorrect]? - if loc = e.location - @mutex.synchronize do - output.printf "%s:%d:%d: %s: [%s] %s\n", - source.path, loc.line_number, loc.column_number, e.rule.severity.symbol, - e.rule.name, e.message.gsub('\n', " ") - end + + next unless loc = e.location + + @mutex.synchronize do + output.printf "%s:%d:%d: %s: [%s] %s\n", + source.path, loc.line_number, loc.column_number, e.rule.severity.symbol, + e.rule.name, e.message.gsub('\n', " ") end end end diff --git a/src/ameba/formatter/json_formatter.cr b/src/ameba/formatter/json_formatter.cr index da16fe1f..6cd3a328 100644 --- a/src/ameba/formatter/json_formatter.cr +++ b/src/ameba/formatter/json_formatter.cr @@ -62,7 +62,6 @@ module Ameba::Formatter # }, # } # ``` - # class JSONFormatter < BaseFormatter def initialize(@output = STDOUT) @result = AsJSON::Result.new @@ -75,10 +74,17 @@ module Ameba::Formatter def source_finished(source : Source) json_source = AsJSON::Source.new source.path - source.issues.each do |e| - next if e.disabled? - next if e.correctable? && config[:autocorrect]? - json_source.issues << AsJSON::Issue.new(e.rule.name, e.rule.severity.to_s, e.location, e.end_location, e.message) + source.issues.each do |issue| + next if issue.disabled? + next if issue.correctable? && config[:autocorrect]? + + json_source.issues << AsJSON::Issue.new( + issue.rule.name, + issue.rule.severity.to_s, + issue.location, + issue.end_location, + issue.message + ) @result.summary.issues_count += 1 end @@ -96,7 +102,11 @@ module Ameba::Formatter metadata = Metadata.new, summary = Summary.new do def to_json(json) - {sources: sources, metadata: metadata, summary: summary}.to_json(json) + { + sources: sources, + metadata: metadata, + summary: summary, + }.to_json(json) end end @@ -104,7 +114,10 @@ module Ameba::Formatter path : String, issues = [] of Issue do def to_json(json) - {path: path, issues: issues}.to_json(json) + { + path: path, + issues: issues, + }.to_json(json) end end @@ -115,15 +128,19 @@ module Ameba::Formatter end_location : Crystal::Location?, message : String do def to_json(json) - json.object do - json.field :rule_name, rule_name - json.field :severity, severity - json.field :message, message - json.field :location, - {line: location.try &.line_number, column: location.try &.column_number} - json.field :end_location, - {line: end_location.try &.line_number, column: end_location.try &.column_number} - end + { + rule_name: rule_name, + severity: severity, + message: message, + location: { + line: location.try &.line_number, + column: location.try &.column_number, + }, + end_location: { + line: end_location.try &.line_number, + column: end_location.try &.column_number, + }, + }.to_json(json) end end @@ -131,10 +148,10 @@ module Ameba::Formatter ameba_version : String = Ameba::VERSION, crystal_version : String = Crystal::VERSION do def to_json(json) - json.object do - json.field :ameba_version, ameba_version - json.field :crystal_version, crystal_version - end + { + ameba_version: ameba_version, + crystal_version: crystal_version, + }.to_json(json) end end @@ -143,10 +160,10 @@ module Ameba::Formatter property issues_count = 0 def to_json(json) - json.object do - json.field :target_sources_count, target_sources_count - json.field :issues_count, issues_count - end + { + target_sources_count: target_sources_count, + issues_count: issues_count, + }.to_json(json) end end end diff --git a/src/ameba/formatter/todo_formatter.cr b/src/ameba/formatter/todo_formatter.cr index 8c8524f9..eaee9efc 100644 --- a/src/ameba/formatter/todo_formatter.cr +++ b/src/ameba/formatter/todo_formatter.cr @@ -20,9 +20,9 @@ module Ameba::Formatter return end - file = generate_todo_config issues - @output.puts "Created #{file.path}" - file + generate_todo_config(issues).tap do |file| + @output.puts "Created #{file.path}" + end end private def generate_todo_config(issues) @@ -43,6 +43,7 @@ module Ameba::Formatter issues.each do |issue| next if issue.disabled? || issue.rule.is_a?(Rule::Lint::Syntax) next if issue.correctable? && config[:autocorrect]? + (h[issue.rule] ||= Array(Issue).new) << issue end end diff --git a/src/ameba/rule/style/guard_clause.cr b/src/ameba/rule/style/guard_clause.cr index 4689f6e9..618c4c36 100644 --- a/src/ameba/rule/style/guard_clause.cr +++ b/src/ameba/rule/style/guard_clause.cr @@ -160,7 +160,7 @@ module Ameba::Rule::Style end private def guard_clause(node) - node = node.right if node.is_a?(Crystal::And) || node.is_a?(Crystal::Or) + node = node.right if node.is_a?(Crystal::BinaryOp) return unless location = node.location return unless end_location = node.end_location @@ -175,11 +175,9 @@ module Ameba::Rule::Style end def guard_clause_source(source, guard_clause, parent) - if parent.is_a?(Crystal::And) || parent.is_a?(Crystal::Or) - node_source(parent, source.lines) - else - node_source(guard_clause, source.lines) - end + node = parent.is_a?(Crystal::BinaryOp) ? parent : guard_clause + + node_source(node, source.lines) end end end diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index 1a5a7101..be21d73d 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -80,8 +80,8 @@ module Ameba::Rule::Style begin_loc, end_loc = begin_range begin_loc, end_loc = def_loc.seek(begin_loc), def_loc.seek(end_loc) - begin_end_loc = begin_loc.adjust(column_number: {{"begin".size - 1}}) - end_end_loc = end_loc.adjust(column_number: {{"end".size - 1}}) + begin_end_loc = begin_loc.adjust(column_number: {{ "begin".size - 1 }}) + end_end_loc = end_loc.adjust(column_number: {{ "end".size - 1 }}) issue_for begin_loc, begin_end_loc, MSG do |corrector| corrector.remove(begin_loc, begin_end_loc) diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr index e7e4bcdd..a97661cb 100644 --- a/src/ameba/rule/style/verbose_block.cr +++ b/src/ameba/rule/style/verbose_block.cr @@ -62,10 +62,7 @@ module Ameba::Rule::Style end protected def operator?(name) - name.each_char do |char| - return false unless char.in?(OPERATOR_CHARS) - end - !name.empty? + !name.empty? && name[0].in?(OPERATOR_CHARS) end protected def setter?(name) diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index fb709464..a987db12 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -9,7 +9,6 @@ module Ameba # runner = Ameba::Runner.new config # runner.run.success? # => true or false # ``` - # class Runner # An error indicating that the inspection loop got stuck correcting # issues back and forth.