From 742bc0fc3c452d04a71be5338fdde5a4b9e9c37f Mon Sep 17 00:00:00 2001 From: Aslan Dukaev Date: Sun, 8 Mar 2020 01:50:55 +0300 Subject: [PATCH] Separate explanation --- lib/fasterer/explanation.rb | 111 +++++++++++++++++++++++ lib/fasterer/file_traverser.rb | 10 +- lib/fasterer/offense.rb | 105 --------------------- spec/lib/fasterer/file_traverser_spec.rb | 2 +- 4 files changed, 119 insertions(+), 109 deletions(-) create mode 100644 lib/fasterer/explanation.rb diff --git a/lib/fasterer/explanation.rb b/lib/fasterer/explanation.rb new file mode 100644 index 0000000..528bb3d --- /dev/null +++ b/lib/fasterer/explanation.rb @@ -0,0 +1,111 @@ +module Fasterer + class Explanation + def initialize(offense_name) + @offense_name = offense_name + end + + def call + @explanation ||= begin + info, link = EXPLANATIONS.fetch(@offense_name).values + "#{info}. See more: #{link}" + end + end + + EXPLANATIONS = { + rescue_vs_respond_to: { + info: 'Don\'t rescue NoMethodError, rather check with respond_to?', + link: 'https://github.com/JuanitoFatas/fast-ruby#beginrescue-vs-respond_to-for-control-flow-code' + }, + + module_eval: { + info: 'Using module_eval is slower than define_method', + link: 'https://github.com/JuanitoFatas/fast-ruby#define_method-vs-module_eval-for-defining-methods-code' + }, + + shuffle_first_vs_sample: { + info: 'Array#shuffle.first is slower than Array#sample', + link: 'https://github.com/JuanitoFatas/fast-ruby#arrayshufflefirst-vs-arraysample-code' + }, + + for_loop_vs_each: { + info: 'For loop is slower than using each', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerableeach-vs-for-loop-code' + }, + + each_with_index_vs_while: { + info: 'Using each_with_index is slower than while loop', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerableeach_with_index-vs-while-loop-code' + }, + + map_flatten_vs_flat_map: { + info: 'Array#map.flatten(1) is slower than Array#flat_map', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerablemaparrayflatten-vs-enumerableflat_map-code' + }, + + reverse_each_vs_reverse_each: { + info: 'Array#reverse.each is slower than Array#reverse_each', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerablereverseeach-vs-enumerablereverse_each-code' + }, + + select_first_vs_detect: { + info: 'Array#select.first is slower than Array#detect', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerabledetect-vs-enumerableselectfirst-code' + }, + + sort_vs_sort_by: { + info: 'Enumerable#sort is slower than Enumerable#sort_by', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerablesort-vs-enumerablesort_by-code' + }, + + fetch_with_argument_vs_block: { + info: 'Hash#fetch with second argument is slower than Hash#fetch with block', + link: 'https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code' + }, + + keys_each_vs_each_key: { + info: 'Hash#keys.each is slower than Hash#each_key. N.B. Hash#each_key cannot be used if the hash is modified during the each block', + link: 'https://github.com/JuanitoFatas/fast-ruby#hasheach_key-instead-of-hashkeyseach-code' + }, + + hash_merge_bang_vs_hash_brackets: { + info: 'Hash#merge! with one argument is slower than Hash#[]', + link: 'https://github.com/JuanitoFatas/fast-ruby#hashmerge-vs-hash-code' + }, + + block_vs_symbol_to_proc: { + info: 'Calling argumentless methods within blocks is slower than using symbol to proc', + link: 'https://github.com/JuanitoFatas/fast-ruby#block-vs-symbolto_proc-code' + }, + + proc_call_vs_yield: { + info: 'Calling blocks with call is slower than yielding', + link: 'https://github.com/JuanitoFatas/fast-ruby#proccall-and-block-arguments-vs-yieldcode' + }, + + gsub_vs_tr: { + info: 'Using tr is faster than gsub when replacing a single character in a string with another single character', + link: 'https://github.com/JuanitoFatas/fast-ruby#stringgsub-vs-stringtr-code' + }, + + select_last_vs_reverse_detect: { + info: 'Array#select.last is slower than Array#reverse.detect', + link: 'https://github.com/JuanitoFatas/fast-ruby#enumerableselectlast-vs-enumerablereversedetect-code' + }, + + getter_vs_attr_reader: { + info: 'Use attr_reader for reading ivars', + link: 'https://github.com/JuanitoFatas/fast-ruby#attr_accessor-vs-getter-and-setter-code' + }, + + setter_vs_attr_writer: { + info: 'Use attr_writer for writing to ivars', + link: 'https://github.com/JuanitoFatas/fast-ruby#attr_accessor-vs-getter-and-setter-code' + }, + + include_vs_cover_on_range: { + info: 'Use #cover? instead of #include? on ranges', + link: 'https://github.com/JuanitoFatas/fast-ruby#cover-vs-include-code' + } + } + end +end diff --git a/lib/fasterer/file_traverser.rb b/lib/fasterer/file_traverser.rb index 7f3af28..ac80241 100644 --- a/lib/fasterer/file_traverser.rb +++ b/lib/fasterer/file_traverser.rb @@ -4,6 +4,7 @@ require_relative 'analyzer' require_relative 'config' +require_relative 'explanation' module Fasterer class FileTraverser @@ -20,6 +21,7 @@ def initialize(path) @parse_error_paths = [] @config = Config.new @offenses_total_count = 0 + @explanation = {} end def traverse @@ -83,9 +85,7 @@ def output(analyzer) offenses_grouped_by_type(analyzer).each do |error_group_name, error_occurences| error_occurences.map(&:line_number).each do |line| file_and_line = "#{analyzer.file_path}:#{line}" - message = Fasterer::Offense.new(error_group_name, line).explanation - - print "#{file_and_line.colorize(:red)} #{message}\n" + print "#{file_and_line.colorize(:red)} #{explanation(error_group_name)}\n" end end @@ -128,6 +128,10 @@ def ignored_files def nil_config_file config.nil_file end + + def explanation(offense_name) + @explanations[offense_name] ||= Fasterer::Explanation.new(offense_name).call + end end ErrorData = Struct.new(:file_path, :error_class, :error_message) do diff --git a/lib/fasterer/offense.rb b/lib/fasterer/offense.rb index d347380..36900ca 100644 --- a/lib/fasterer/offense.rb +++ b/lib/fasterer/offense.rb @@ -8,111 +8,6 @@ class Offense def initialize(offense_name, line_number) @offense_name = offense_name @line_number = line_number - explanation # Set explanation right away. end - - def explanation - @explanation ||= begin - info, link = EXPLANATIONS.fetch(offense_name).values - "#{info}. See more: #{link}" - end - end - - EXPLANATIONS = { - rescue_vs_respond_to: { - info: 'Don\'t rescue NoMethodError, rather check with respond_to?', - link: 'https://github.com/JuanitoFatas/fast-ruby#beginrescue-vs-respond_to-for-control-flow-code' - }, - - module_eval: { - info: 'Using module_eval is slower than define_method', - link: 'https://github.com/JuanitoFatas/fast-ruby#define_method-vs-module_eval-for-defining-methods-code' - }, - - shuffle_first_vs_sample: { - info: 'Array#shuffle.first is slower than Array#sample', - link: 'https://github.com/JuanitoFatas/fast-ruby#arrayshufflefirst-vs-arraysample-code' - }, - - for_loop_vs_each: { - info: 'For loop is slower than using each', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerableeach-vs-for-loop-code' - }, - - each_with_index_vs_while: { - info: 'Using each_with_index is slower than while loop', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerableeach_with_index-vs-while-loop-code' - }, - - map_flatten_vs_flat_map: { - info: 'Array#map.flatten(1) is slower than Array#flat_map', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerablemaparrayflatten-vs-enumerableflat_map-code' - }, - - reverse_each_vs_reverse_each: { - info: 'Array#reverse.each is slower than Array#reverse_each', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerablereverseeach-vs-enumerablereverse_each-code' - }, - - select_first_vs_detect: { - info: 'Array#select.first is slower than Array#detect', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerabledetect-vs-enumerableselectfirst-code' - }, - - sort_vs_sort_by: { - info: 'Enumerable#sort is slower than Enumerable#sort_by', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerablesort-vs-enumerablesort_by-code' - }, - - fetch_with_argument_vs_block: { - info: 'Hash#fetch with second argument is slower than Hash#fetch with block', - link: 'https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code' - }, - - keys_each_vs_each_key: { - info: 'Hash#keys.each is slower than Hash#each_key. N.B. Hash#each_key cannot be used if the hash is modified during the each block', - link: 'https://github.com/JuanitoFatas/fast-ruby#hasheach_key-instead-of-hashkeyseach-code' - }, - - hash_merge_bang_vs_hash_brackets: { - info: 'Hash#merge! with one argument is slower than Hash#[]', - link: 'https://github.com/JuanitoFatas/fast-ruby#hashmerge-vs-hash-code' - }, - - block_vs_symbol_to_proc: { - info: 'Calling argumentless methods within blocks is slower than using symbol to proc', - link: 'https://github.com/JuanitoFatas/fast-ruby#block-vs-symbolto_proc-code' - }, - - proc_call_vs_yield: { - info: 'Calling blocks with call is slower than yielding', - link: 'https://github.com/JuanitoFatas/fast-ruby#proccall-and-block-arguments-vs-yieldcode' - }, - - gsub_vs_tr: { - info: 'Using tr is faster than gsub when replacing a single character in a string with another single character', - link: 'https://github.com/JuanitoFatas/fast-ruby#stringgsub-vs-stringtr-code' - }, - - select_last_vs_reverse_detect: { - info: 'Array#select.last is slower than Array#reverse.detect', - link: 'https://github.com/JuanitoFatas/fast-ruby#enumerableselectlast-vs-enumerablereversedetect-code' - }, - - getter_vs_attr_reader: { - info: 'Use attr_reader for reading ivars', - link: 'https://github.com/JuanitoFatas/fast-ruby#attr_accessor-vs-getter-and-setter-code' - }, - - setter_vs_attr_writer: { - info: 'Use attr_writer for writing to ivars', - link: 'https://github.com/JuanitoFatas/fast-ruby#attr_accessor-vs-getter-and-setter-code' - }, - - include_vs_cover_on_range: { - info: 'Use #cover? instead of #include? on ranges', - link: 'https://github.com/JuanitoFatas/fast-ruby#cover-vs-include-code' - } - } end end diff --git a/spec/lib/fasterer/file_traverser_spec.rb b/spec/lib/fasterer/file_traverser_spec.rb index 5750e69..d70c703 100644 --- a/spec/lib/fasterer/file_traverser_spec.rb +++ b/spec/lib/fasterer/file_traverser_spec.rb @@ -345,7 +345,7 @@ end context "when print offenses" do - let(:explanation) { Fasterer::Offense.new(:for_loop_vs_each, 0).explanation } + let(:explanation) { Fasterer::Explanation.new(:for_loop_vs_each).call } it 'should print offense' do match = "\e[0;31;49m#{test_file_path}:1\e[0m #{explanation}\n\n"