From de5908840775c7cc3b80d7d3d506d0581791835b Mon Sep 17 00:00:00 2001 From: Bert Hajee Date: Tue, 24 Oct 2017 23:07:18 +0200 Subject: [PATCH 1/2] Add support for Puppet metadata-json-lint pre-commit hook --- CHANGELOG.md | 1 + README.md | 1 + config/default.yml | 9 +++ .../hook/pre_commit/metadata_json_lint.rb | 50 ++++++++++++ .../pre_commit/metadata_json_lint_spec.rb | 77 +++++++++++++++++++ 5 files changed, 138 insertions(+) create mode 100644 lib/overcommit/hook/pre_commit/metadata_json_lint.rb create mode 100644 spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c15e8e..1b6b96e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## master (unreleased) +* Add [`metadata-json-lint`](https://voxpupuli.org/blog/2014/11/06/linting-metadata-json/) pre-commit hook * Fix `LineEndings` pre-commit hook handling of file paths with spaces ## 0.41.0 diff --git a/README.md b/README.md index 0a62737f..f22f154b 100644 --- a/README.md +++ b/README.md @@ -519,6 +519,7 @@ issue](https://github.com/brigade/overcommit/issues/238) for more details. * [LocalPathsInGemfile](lib/overcommit/hook/pre_commit/local_paths_in_gemfile.rb) * [Mdl](lib/overcommit/hook/pre_commit/mdl.rb) * [`*`MergeConflicts](lib/overcommit/hook/pre_commit/merge_conflicts.rb) +* [MetadataJsonLint](lib/overcommit/hook/pre_commit/metadata_json_lint.rb) * [NginxTest](lib/overcommit/hook/pre_commit/nginx_test.rb) * [PhpCs](lib/overcommit/hook/pre_commit/php_cs.rb) * [PhpLint](lib/overcommit/hook/pre_commit/php_lint.rb) diff --git a/config/default.yml b/config/default.yml index 2f73647d..fc1e5f17 100644 --- a/config/default.yml +++ b/config/default.yml @@ -427,6 +427,15 @@ PreCommit: required_executable: 'grep' flags: ['-IHn', "^<<<<<<<[ \t]"] + MetadataJsonLint: + enabled: false + strict_license: true + strict_dependencies: false + fail_on_warning: true + description: 'Checking Puppet module metadata' + required_executable: 'metadata-json-lint' + install_command: 'gem install metadata-json-lint semantic_puppet' + NginxTest: enabled: false description: 'Test nginx configs' diff --git a/lib/overcommit/hook/pre_commit/metadata_json_lint.rb b/lib/overcommit/hook/pre_commit/metadata_json_lint.rb new file mode 100644 index 00000000..1b0d9388 --- /dev/null +++ b/lib/overcommit/hook/pre_commit/metadata_json_lint.rb @@ -0,0 +1,50 @@ +module Overcommit::Hook::PreCommit + # + # Run's the Puppet metadata linter. It has support for adding options + # in the .overcommit.yaml + # + # PreCommit: + # MetadataJsonLint: + # enabled: true + # strict_license: false + # strict_dependencies: false + # fail_on_warning: true + # description: 'Checking module metadata' + # + # @see https://voxpupuli.org/blog/2014/11/06/linting-metadata-json/ + # + class MetadataJsonLint < Base + MESSAGE_REGEX = /\((?.*)\).*/ + + MESSAGE_TYPE_CATEGORIZER = lambda do |type| + type == 'WARN' ? :warning : :error + end + + def options + [:strict_license, :strict_dependencies, :fail_on_warning].collect do |option| + name = option.to_s.tr('_', '-') + value = config.fetch(option.to_s) { true } + if value + "--#{name}" + else + "--no-#{name}" + end + end + end + + def run + # When metadata.json, not modified return pass + return :pass unless applicable_files.include?('metadata.json') + + arguments = options << 'metadata.json' + result = execute(command, args: arguments) + output = result.stdout.chomp.gsub(/^"|"$/, '') + return :pass if result.success? && output.empty? + extract_messages( + output.split("\n"), + MESSAGE_REGEX, + MESSAGE_TYPE_CATEGORIZER + ) + end + end +end diff --git a/spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb b/spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb new file mode 100644 index 00000000..b6704ac1 --- /dev/null +++ b/spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Overcommit::Hook::PreCommit::MetadataJsonLint do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + context 'metadata.json not modified' do + before do + subject.stub(:applicable_files).and_return(%w[file1.pp file2.pp]) + end + + it { should pass } + end + + context 'metadata.json is modified' do + before do + subject.stub(:applicable_files).and_return(%w[file1.pp file2.pp metadata.json]) + end + + context 'when metadata-json-lint exits successfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(true) + subject.stub(:execute).and_return(result) + end + + context 'with no output' do + before do + result.stub(:stdout).and_return('') + end + + it { should pass } + end + + context 'and it reports a warning' do + before do + result.stub(:stdout).and_return(normalize_indent(<<-OUT)) + (WARN) requirements: The 'pe' requirement is no longer supported by the Forge. + OUT + end + + it { should warn } + end + end + + context 'when metadata-json-lint exits unsuccessfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(false) + subject.stub(:execute).and_return(result) + end + + context 'and it reports a warning' do + before do + result.stub(:stdout).and_return(normalize_indent(<<-OUT)) + (WARN) requirements: The 'pe' requirement is no longer supported by the Forge. + OUT + end + + it { should warn } + end + + context 'and it reports an error' do + before do + result.stub(:stdout).and_return(normalize_indent(<<-OUT)) + (ERR) requirements: The 'pe' requirement is no longer supported by the Forge. + OUT + end + + it { should fail_hook } + end + end + end +end From a2dbd9ddac78204beb5cef34745d48eb683f0a29 Mon Sep 17 00:00:00 2001 From: Bert Hajee Date: Wed, 25 Oct 2017 16:07:57 +0200 Subject: [PATCH 2/2] Fix review remarks for PuppetMetadataLint hook --- README.md | 2 +- config/default.yml | 11 ++- .../hook/pre_commit/metadata_json_lint.rb | 50 ------------ .../pre_commit/puppet_metadata_json_lint.rb | 27 +++++++ .../pre_commit/metadata_json_lint_spec.rb | 77 ------------------- .../puppet_metadata_json_lint_spec.rb | 67 ++++++++++++++++ 6 files changed, 100 insertions(+), 134 deletions(-) delete mode 100644 lib/overcommit/hook/pre_commit/metadata_json_lint.rb create mode 100644 lib/overcommit/hook/pre_commit/puppet_metadata_json_lint.rb delete mode 100644 spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb create mode 100644 spec/overcommit/hook/pre_commit/puppet_metadata_json_lint_spec.rb diff --git a/README.md b/README.md index f22f154b..b311322a 100644 --- a/README.md +++ b/README.md @@ -519,11 +519,11 @@ issue](https://github.com/brigade/overcommit/issues/238) for more details. * [LocalPathsInGemfile](lib/overcommit/hook/pre_commit/local_paths_in_gemfile.rb) * [Mdl](lib/overcommit/hook/pre_commit/mdl.rb) * [`*`MergeConflicts](lib/overcommit/hook/pre_commit/merge_conflicts.rb) -* [MetadataJsonLint](lib/overcommit/hook/pre_commit/metadata_json_lint.rb) * [NginxTest](lib/overcommit/hook/pre_commit/nginx_test.rb) * [PhpCs](lib/overcommit/hook/pre_commit/php_cs.rb) * [PhpLint](lib/overcommit/hook/pre_commit/php_lint.rb) * [PuppetLint](lib/overcommit/hook/pre_commit/puppet_lint.rb) +* [PuppetMetadataJsonLint](lib/overcommit/hook/pre_commit/puppet_metadata_json_lint.rb) * [Pycodestyle](lib/overcommit/hook/pre_commit/pycodestyle.rb) * [Pydocstyle](lib/overcommit/hook/pre_commit/pydocstyle.rb) * [Pyflakes](lib/overcommit/hook/pre_commit/pyflakes.rb) diff --git a/config/default.yml b/config/default.yml index fc1e5f17..bac9a8a5 100644 --- a/config/default.yml +++ b/config/default.yml @@ -427,14 +427,13 @@ PreCommit: required_executable: 'grep' flags: ['-IHn', "^<<<<<<<[ \t]"] - MetadataJsonLint: + PuppetMetadataJsonLint: enabled: false - strict_license: true - strict_dependencies: false - fail_on_warning: true - description: 'Checking Puppet module metadata' + description: 'Checking module metadata' + flags: ['--strict-license', '--strict-dependencies', '--fail-on-warning'] + include: 'metadata.json' required_executable: 'metadata-json-lint' - install_command: 'gem install metadata-json-lint semantic_puppet' + install_command: 'gem install metadata-json-lint' NginxTest: enabled: false diff --git a/lib/overcommit/hook/pre_commit/metadata_json_lint.rb b/lib/overcommit/hook/pre_commit/metadata_json_lint.rb deleted file mode 100644 index 1b0d9388..00000000 --- a/lib/overcommit/hook/pre_commit/metadata_json_lint.rb +++ /dev/null @@ -1,50 +0,0 @@ -module Overcommit::Hook::PreCommit - # - # Run's the Puppet metadata linter. It has support for adding options - # in the .overcommit.yaml - # - # PreCommit: - # MetadataJsonLint: - # enabled: true - # strict_license: false - # strict_dependencies: false - # fail_on_warning: true - # description: 'Checking module metadata' - # - # @see https://voxpupuli.org/blog/2014/11/06/linting-metadata-json/ - # - class MetadataJsonLint < Base - MESSAGE_REGEX = /\((?.*)\).*/ - - MESSAGE_TYPE_CATEGORIZER = lambda do |type| - type == 'WARN' ? :warning : :error - end - - def options - [:strict_license, :strict_dependencies, :fail_on_warning].collect do |option| - name = option.to_s.tr('_', '-') - value = config.fetch(option.to_s) { true } - if value - "--#{name}" - else - "--no-#{name}" - end - end - end - - def run - # When metadata.json, not modified return pass - return :pass unless applicable_files.include?('metadata.json') - - arguments = options << 'metadata.json' - result = execute(command, args: arguments) - output = result.stdout.chomp.gsub(/^"|"$/, '') - return :pass if result.success? && output.empty? - extract_messages( - output.split("\n"), - MESSAGE_REGEX, - MESSAGE_TYPE_CATEGORIZER - ) - end - end -end diff --git a/lib/overcommit/hook/pre_commit/puppet_metadata_json_lint.rb b/lib/overcommit/hook/pre_commit/puppet_metadata_json_lint.rb new file mode 100644 index 00000000..60fb3d52 --- /dev/null +++ b/lib/overcommit/hook/pre_commit/puppet_metadata_json_lint.rb @@ -0,0 +1,27 @@ +module Overcommit::Hook::PreCommit + # + # Run's the Puppet metadata linter. It has support for adding options + # in the .overcommit.yaml + # + # @see https://voxpupuli.org/blog/2014/11/06/linting-metadata-json/ + # + class PuppetMetadataJsonLint < Base + MESSAGE_REGEX = /\((?.*)\).*/ + + MESSAGE_TYPE_CATEGORIZER = lambda do |type| + type == 'WARN' ? :warning : :error + end + + def run + result = execute(command, args: applicable_files) + output = result.stdout.chomp.gsub(/^"|"$/, '') + return :pass if result.success? && output.empty? + + extract_messages( + output.split("\n"), + MESSAGE_REGEX, + MESSAGE_TYPE_CATEGORIZER + ) + end + end +end diff --git a/spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb b/spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb deleted file mode 100644 index b6704ac1..00000000 --- a/spec/overcommit/hook/pre_commit/metadata_json_lint_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'spec_helper' - -describe Overcommit::Hook::PreCommit::MetadataJsonLint do - let(:config) { Overcommit::ConfigurationLoader.default_configuration } - let(:context) { double('context') } - subject { described_class.new(config, context) } - - context 'metadata.json not modified' do - before do - subject.stub(:applicable_files).and_return(%w[file1.pp file2.pp]) - end - - it { should pass } - end - - context 'metadata.json is modified' do - before do - subject.stub(:applicable_files).and_return(%w[file1.pp file2.pp metadata.json]) - end - - context 'when metadata-json-lint exits successfully' do - let(:result) { double('result') } - - before do - result.stub(:success?).and_return(true) - subject.stub(:execute).and_return(result) - end - - context 'with no output' do - before do - result.stub(:stdout).and_return('') - end - - it { should pass } - end - - context 'and it reports a warning' do - before do - result.stub(:stdout).and_return(normalize_indent(<<-OUT)) - (WARN) requirements: The 'pe' requirement is no longer supported by the Forge. - OUT - end - - it { should warn } - end - end - - context 'when metadata-json-lint exits unsuccessfully' do - let(:result) { double('result') } - - before do - result.stub(:success?).and_return(false) - subject.stub(:execute).and_return(result) - end - - context 'and it reports a warning' do - before do - result.stub(:stdout).and_return(normalize_indent(<<-OUT)) - (WARN) requirements: The 'pe' requirement is no longer supported by the Forge. - OUT - end - - it { should warn } - end - - context 'and it reports an error' do - before do - result.stub(:stdout).and_return(normalize_indent(<<-OUT)) - (ERR) requirements: The 'pe' requirement is no longer supported by the Forge. - OUT - end - - it { should fail_hook } - end - end - end -end diff --git a/spec/overcommit/hook/pre_commit/puppet_metadata_json_lint_spec.rb b/spec/overcommit/hook/pre_commit/puppet_metadata_json_lint_spec.rb new file mode 100644 index 00000000..43da8eb7 --- /dev/null +++ b/spec/overcommit/hook/pre_commit/puppet_metadata_json_lint_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe Overcommit::Hook::PreCommit::PuppetMetadataJsonLint do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + before do + subject.stub(:applicable_files).and_return(%w[file1.pp file2.pp metadata.json]) + end + + context 'when metadata-json-lint exits successfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(true) + subject.stub(:execute).and_return(result) + end + + context 'with no output' do + before do + result.stub(:stdout).and_return('') + end + + it { should pass } + end + + context 'and it reports a warning' do + before do + result.stub(:stdout).and_return(normalize_indent(<<-OUT)) + (WARN) requirements: The 'pe' requirement is no longer supported by the Forge. + OUT + end + + it { should warn } + end + end + + context 'when metadata-json-lint exits unsuccessfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(false) + subject.stub(:execute).and_return(result) + end + + context 'and it reports a warning' do + before do + result.stub(:stdout).and_return(normalize_indent(<<-OUT)) + (WARN) requirements: The 'pe' requirement is no longer supported by the Forge. + OUT + end + + it { should warn } + end + + context 'and it reports an error' do + before do + result.stub(:stdout).and_return(normalize_indent(<<-OUT)) + (ERR) requirements: The 'pe' requirement is no longer supported by the Forge. + OUT + end + + it { should fail_hook } + end + end +end