Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add links to report #78

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
111 changes: 111 additions & 0 deletions lib/fasterer/explanation.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 7 additions & 1 deletion lib/fasterer/file_traverser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

require_relative 'analyzer'
require_relative 'config'
require_relative 'explanation'

module Fasterer
class FileTraverser
Expand All @@ -20,6 +21,7 @@ def initialize(path)
@parse_error_paths = []
@config = Config.new
@offenses_total_count = 0
@explanations = {}
end

def traverse
Expand Down Expand Up @@ -83,7 +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}"
print "#{file_and_line.colorize(:red)} #{Fasterer::Offense::EXPLANATIONS[error_group_name]}.\n"
print "#{file_and_line.colorize(:red)} #{explanations(error_group_name)}\n"
end
end

Expand Down Expand Up @@ -126,6 +128,10 @@ def ignored_files
def nil_config_file
config.nil_file
end

def explanations(offense_name)
@explanations[offense_name] ||= Fasterer::Explanation.new(offense_name).call
end
end

ErrorData = Struct.new(:file_path, :error_class, :error_message) do
Expand Down
64 changes: 0 additions & 64 deletions lib/fasterer/offense.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,70 +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 ||= EXPLANATIONS.fetch(offense_name)
end

EXPLANATIONS = {
rescue_vs_respond_to:
'Don\'t rescue NoMethodError, rather check with respond_to?',

module_eval:
'Using module_eval is slower than define_method',

shuffle_first_vs_sample:
'Array#shuffle.first is slower than Array#sample',

for_loop_vs_each:
'For loop is slower than using each',

each_with_index_vs_while:
'Using each_with_index is slower than while loop',

map_flatten_vs_flat_map:
'Array#map.flatten(1) is slower than Array#flat_map',

reverse_each_vs_reverse_each:
'Array#reverse.each is slower than Array#reverse_each',

select_first_vs_detect:
'Array#select.first is slower than Array#detect',

sort_vs_sort_by:
'Enumerable#sort is slower than Enumerable#sort_by',

fetch_with_argument_vs_block:
'Hash#fetch with second argument is slower than Hash#fetch with block',

keys_each_vs_each_key:
'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',

hash_merge_bang_vs_hash_brackets:
'Hash#merge! with one argument is slower than Hash#[]',

block_vs_symbol_to_proc:
'Calling argumentless methods within blocks is slower than using symbol to proc',

proc_call_vs_yield:
'Calling blocks with call is slower than yielding',

gsub_vs_tr:
'Using tr is faster than gsub when replacing a single character in a string with another single character',

select_last_vs_reverse_detect:
'Array#select.last is slower than Array#reverse.detect',

getter_vs_attr_reader:
'Use attr_reader for reading ivars',

setter_vs_attr_writer:
'Use attr_writer for writing to ivars',

include_vs_cover_on_range:
'Use #cover? instead of #include? on ranges'
}
end
end
4 changes: 2 additions & 2 deletions spec/lib/fasterer/file_traverser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,10 @@
end

context "when print offenses" do
let(:explanation) { Fasterer::Offense::EXPLANATIONS[:for_loop_vs_each] }
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"
match = "\e[0;31;49m#{test_file_path}:1\e[0m #{explanation}\n\n"

expect { file_traverser.send(:output, analyzer) }.to output(match).to_stdout
end
Expand Down