From 5d30bfd8af37802e0e6d5ed1521096ae561afbcc Mon Sep 17 00:00:00 2001 From: Diego J Date: Mon, 9 Jul 2018 14:10:00 +0200 Subject: [PATCH 1/5] [FEATURE] New pre-commit hook: flay --- README.md | 1 + config/default.yml | 10 ++++ lib/overcommit/hook/pre_commit/flay.rb | 43 +++++++++++++++ spec/overcommit/hook/pre_commit/flay_spec.rb | 58 ++++++++++++++++++++ 4 files changed, 112 insertions(+) create mode 100644 lib/overcommit/hook/pre_commit/flay.rb create mode 100644 spec/overcommit/hook/pre_commit/flay_spec.rb diff --git a/README.md b/README.md index c18a525d..6a31251e 100644 --- a/README.md +++ b/README.md @@ -496,6 +496,7 @@ issue](https://github.com/brigade/overcommit/issues/238) for more details. * [ExecutePermissions](lib/overcommit/hook/pre_commit/execute_permissions.rb) * [Fasterer](lib/overcommit/hook/pre_commit/fasterer.rb) * [FixMe](lib/overcommit/hook/pre_commit/fix_me.rb) +* [Flay](lib/overcommit/hook/pre_commit/flay.rb) * [Foodcritic](lib/overcommit/hook/pre_commit/foodcritic.rb) * [ForbiddenBranches](lib/overcommit/hook/pre_commit/forbidden_branches.rb) * [GoLint](lib/overcommit/hook/pre_commit/go_lint.rb) diff --git a/config/default.yml b/config/default.yml index 34cb6cd6..6ba21f37 100644 --- a/config/default.yml +++ b/config/default.yml @@ -283,6 +283,16 @@ PreCommit: flags: ['-IEHnw'] keywords: ['BROKEN', 'BUG', 'ERROR', 'FIXME', 'HACK', 'NOTE', 'OPTIMIZE', 'REVIEW', 'TODO', 'WTF', 'XXX'] + Flay: + enabled: false + description: 'Analyze ruby code for structural similarities with Flay' + required_executable: 'flay' + install_command: 'gem install flay' + mass_threshold: 16 + fuzzy: 1 + liberal: false + include: '**/*.rb' + Foodcritic: enabled: false description: 'Analyze with Foodcritic' diff --git a/lib/overcommit/hook/pre_commit/flay.rb b/lib/overcommit/hook/pre_commit/flay.rb new file mode 100644 index 00000000..796f9efd --- /dev/null +++ b/lib/overcommit/hook/pre_commit/flay.rb @@ -0,0 +1,43 @@ +module Overcommit::Hook::PreCommit + # Runs `flay` against any modified files. + # + # @see https://github.com/seattlerb/flay + class Flay < Base + # Flay prints two kinds of messages: + # + # 1) IDENTICAL code found in :defn (mass*2 = MASS) + # file_path_1.rb:LINE_1 + # file_path_2.rb:LINE_2 + # + # 2) Similar code found in :defn (mass = MASS) + # file_path_1.rb:LINE_1 + # file_path_2.rb:LINE_2 + # + + def run + command = ['flay', '--mass', @config['mass_threshold'].to_s, '--fuzzy', @config['fuzzy'].to_s] + # Use a more liberal detection method + command += ['--liberal'] if @config['liberal'] + # Run the command + result = execute(command, args: applicable_files) + results = result.stdout.split("\n\n") + total_score_title = results.shift + # Check if the issues surpasses the total score threshold + if @config['total_score_threshold'] + _, total_score = total_score_title.split(/\s*=\s*/) + :pass if @config['total_score_threshold'].to_f >= total_score.to_f + end + # Format the error messages + results. + map(&:strip). + reject(&:empty?). + map { |error| message(error) } + end + + private + + def message(error) + Overcommit::Hook::Message.new(:error, nil, nil, error) + end + end +end diff --git a/spec/overcommit/hook/pre_commit/flay_spec.rb b/spec/overcommit/hook/pre_commit/flay_spec.rb new file mode 100644 index 00000000..7b09896e --- /dev/null +++ b/spec/overcommit/hook/pre_commit/flay_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Overcommit::Hook::PreCommit::Flay do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + let(:applicable_files) { %w[file1.rb file2.rb] } + subject { described_class.new(config, context) } + + before do + subject.stub(:applicable_files).and_return(applicable_files) + end + + around do |example| + repo do + example.run + end + end + + before do + subject.stub(:execute).with(['flay', '--mass', '16', '--fuzzy', '1'], args: applicable_files).and_return(result) + end + + context 'flay discovered two issues' do + let(:result) do + double( + success?: false, + stdout: <<-MSG +Total score (lower is better) = 268 + +1) IDENTICAL code found in :defn (mass*2 = 148) + app/whatever11.rb:105 + app/whatever12.rb:76 + +2) Similar code found in :defn (mass = 120) + app/whatever21.rb:105 + app/whatever22.rb:76 + +MSG + ) + end + + it { should fail_hook } + end + + context 'flay discovered no issues' do + let(:result) do + double( + success?: false, + stdout: <<-MSG +Total score (lower is better) = 0 +MSG + ) + end + + it { should pass } + end + +end From 378d35bfa7a315a6d06339acf01fa71b519f8cf7 Mon Sep 17 00:00:00 2001 From: Diego J Date: Mon, 9 Jul 2018 16:18:50 +0200 Subject: [PATCH 2/5] [FIX] Run flay for each file Run flay separately for each because flay does not allow multiple files passed as parameters --- lib/overcommit/hook/pre_commit/flay.rb | 31 ++++++++------------ spec/overcommit/hook/pre_commit/flay_spec.rb | 5 ++-- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/flay.rb b/lib/overcommit/hook/pre_commit/flay.rb index 796f9efd..b2f4669a 100644 --- a/lib/overcommit/hook/pre_commit/flay.rb +++ b/lib/overcommit/hook/pre_commit/flay.rb @@ -18,26 +18,19 @@ def run command = ['flay', '--mass', @config['mass_threshold'].to_s, '--fuzzy', @config['fuzzy'].to_s] # Use a more liberal detection method command += ['--liberal'] if @config['liberal'] - # Run the command - result = execute(command, args: applicable_files) - results = result.stdout.split("\n\n") - total_score_title = results.shift - # Check if the issues surpasses the total score threshold - if @config['total_score_threshold'] - _, total_score = total_score_title.split(/\s*=\s*/) - :pass if @config['total_score_threshold'].to_f >= total_score.to_f + messages = [] + # Run the command for each file + applicable_files.each do |file| + result = execute(command, args: [file]) + results = result.stdout.split("\n\n") + if results.length.positive? + results.shift + error_message = results.join("\n").gsub(/^\d+\)\s*/, '') + message = Overcommit::Hook::Message.new(:error, nil, nil, error_message) + messages << message if results.length.positive? + end end - # Format the error messages - results. - map(&:strip). - reject(&:empty?). - map { |error| message(error) } - end - - private - - def message(error) - Overcommit::Hook::Message.new(:error, nil, nil, error) + messages end end end diff --git a/spec/overcommit/hook/pre_commit/flay_spec.rb b/spec/overcommit/hook/pre_commit/flay_spec.rb index 7b09896e..0a4c9a5b 100644 --- a/spec/overcommit/hook/pre_commit/flay_spec.rb +++ b/spec/overcommit/hook/pre_commit/flay_spec.rb @@ -3,7 +3,7 @@ describe Overcommit::Hook::PreCommit::Flay do let(:config) { Overcommit::ConfigurationLoader.default_configuration } let(:context) { double('context') } - let(:applicable_files) { %w[file1.rb file2.rb] } + let(:applicable_files) { %w[file1.rb] } subject { described_class.new(config, context) } before do @@ -17,7 +17,8 @@ end before do - subject.stub(:execute).with(['flay', '--mass', '16', '--fuzzy', '1'], args: applicable_files).and_return(result) + command = %w[flay --mass 16 --fuzzy 1] + subject.stub(:execute).with(command, args: applicable_files).and_return(result) end context 'flay discovered two issues' do From 6df9602c7aea1d8c7f44dcb00071076ff36acbe4 Mon Sep 17 00:00:00 2001 From: Diego J Date: Mon, 9 Jul 2018 16:52:53 +0200 Subject: [PATCH 3/5] [FIX] undefined method positive? for Fixnum --- lib/overcommit/hook/pre_commit/flay.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/flay.rb b/lib/overcommit/hook/pre_commit/flay.rb index b2f4669a..968ff1cb 100644 --- a/lib/overcommit/hook/pre_commit/flay.rb +++ b/lib/overcommit/hook/pre_commit/flay.rb @@ -23,11 +23,11 @@ def run applicable_files.each do |file| result = execute(command, args: [file]) results = result.stdout.split("\n\n") - if results.length.positive? + if results.length > 0 results.shift error_message = results.join("\n").gsub(/^\d+\)\s*/, '') message = Overcommit::Hook::Message.new(:error, nil, nil, error_message) - messages << message if results.length.positive? + messages << message if results.length > 0 end end messages From 980a90fb6e8e94fb171a9d1215c0d217354f3e36 Mon Sep 17 00:00:00 2001 From: Diego J Date: Mon, 9 Jul 2018 17:22:27 +0200 Subject: [PATCH 4/5] Fix for Rubocop Rubocop doesn't lik length > 0 --- lib/overcommit/hook/pre_commit/flay.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/flay.rb b/lib/overcommit/hook/pre_commit/flay.rb index 968ff1cb..c30ee76b 100644 --- a/lib/overcommit/hook/pre_commit/flay.rb +++ b/lib/overcommit/hook/pre_commit/flay.rb @@ -23,11 +23,11 @@ def run applicable_files.each do |file| result = execute(command, args: [file]) results = result.stdout.split("\n\n") - if results.length > 0 - results.shift + results.shift + unless results.empty? error_message = results.join("\n").gsub(/^\d+\)\s*/, '') message = Overcommit::Hook::Message.new(:error, nil, nil, error_message) - messages << message if results.length > 0 + messages << message end end messages From f696244e052622ee6c6622c306bc3772b5943301 Mon Sep 17 00:00:00 2001 From: Diego J Date: Mon, 9 Jul 2018 17:31:21 +0200 Subject: [PATCH 5/5] [FIX] Remove extra empty line in flay_spec.rb --- spec/overcommit/hook/pre_commit/flay_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/overcommit/hook/pre_commit/flay_spec.rb b/spec/overcommit/hook/pre_commit/flay_spec.rb index 0a4c9a5b..e2b032da 100644 --- a/spec/overcommit/hook/pre_commit/flay_spec.rb +++ b/spec/overcommit/hook/pre_commit/flay_spec.rb @@ -55,5 +55,4 @@ it { should pass } end - end