From eeab6ab3e2e33ff42db1ef7e8e6d03096d72548e Mon Sep 17 00:00:00 2001 From: Alexander Fisher Date: Thu, 19 Jan 2023 17:05:26 +0000 Subject: [PATCH 1/3] Add support for generating codeclimate report --- README.md | 24 ++++++++++ lib/puppet-lint.rb | 14 +++--- lib/puppet-lint/bin.rb | 5 ++ lib/puppet-lint/configuration.rb | 1 + lib/puppet-lint/optparser.rb | 4 ++ lib/puppet-lint/report/codeclimate.rb | 48 ++++++++++++++++++++ lib/puppet-lint/tasks/puppet-lint.rb | 9 +++- spec/fixtures/test/reports/code_climate.json | 38 ++++++++++++++++ spec/unit/puppet-lint/bin_spec.rb | 24 ++++++++++ spec/unit/puppet-lint/configuration_spec.rb | 32 ++++++++----- 10 files changed, 181 insertions(+), 18 deletions(-) create mode 100644 lib/puppet-lint/report/codeclimate.rb create mode 100644 spec/fixtures/test/reports/code_climate.json diff --git a/README.md b/README.md index b422e7bfc..3aaf59338 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,30 @@ There is a GitHub Actions action available to get linter feedback in workflows: * [puppet-lint-action](https://github.com/marketplace/actions/puppet-lint-action) +## Integration with GitLab Code Quality + +[GitLab](https://gitlab.com/) users can use the `--codeclimate-report-file` configuration option to generate a report for use with the +[Code Quality](https://docs.gitlab.com/ee/ci/testing/code_quality.html) feature. + +The easiest way to set this option, (and without having to modify rake tasks), is with the `CODECLIMATE_REPORT_FILE` environment variable. + +For example, the following GitLab job sets the environment variable and +[archives the report](https://docs.gitlab.com/ee/ci/yaml/artifacts_reports.html#artifactsreportscodequality) produced. +```yaml +validate lint check rubocop-Ruby 2.7.2-Puppet ~> 7: + stage: syntax + image: ruby:2.7.2 + script: + - bundle exec rake validate lint check rubocop + variables: + PUPPET_GEM_VERSION: '~> 7' + CODECLIMATE_REPORT_FILE: 'gl-code-quality-report.json' + artifacts: + reports: + codequality: gl-code-quality-report.json + expire_in: 1 week +``` + ## Options See `puppet-lint --help` for a full list of command line options and checks. diff --git a/lib/puppet-lint.rb b/lib/puppet-lint.rb index a45e31da3..d4507f1e5 100644 --- a/lib/puppet-lint.rb +++ b/lib/puppet-lint.rb @@ -175,14 +175,16 @@ def report(problems) next unless message[:kind] == :fixed || [message[:kind], :all].include?(configuration.error_level) - if configuration.json || configuration.sarif + if configuration.json || configuration.sarif || configuration.codeclimate_report_file message['context'] = get_context(message) if configuration.with_context json << message - else - format_message(message) - print_context(message) if configuration.with_context - print_github_annotation(message) if configuration.github_actions end + + next if configuration.json || configuration.sarif + + format_message(message) + print_context(message) if configuration.with_context + print_github_annotation(message) if configuration.github_actions end $stderr.puts 'Try running `puppet parser validate `' if problems.any? { |p| p[:check] == :syntax } json @@ -225,7 +227,7 @@ def run # Public: Print any problems that were found out to stdout. # - # Returns nothing. + # Returns an array of problems. def print_problems report(@problems) end diff --git a/lib/puppet-lint/bin.rb b/lib/puppet-lint/bin.rb index 7457d51c3..7ae04d8d3 100644 --- a/lib/puppet-lint/bin.rb +++ b/lib/puppet-lint/bin.rb @@ -1,6 +1,7 @@ require 'pathname' require 'uri' require 'puppet-lint/optparser' +require 'puppet-lint/report/codeclimate' # Internal: The logic of the puppet-lint bin script, contained in a class for # ease of testing. @@ -104,6 +105,10 @@ def run puts JSON.pretty_generate(all_problems) end + if PuppetLint.configuration.codeclimate_report_file + PuppetLint::Report::CodeClimateReporter.write_report_file(all_problems, PuppetLint.configuration.codeclimate_report_file) + end + return_val rescue PuppetLint::NoCodeError puts 'puppet-lint: no file specified or specified file does not exist' diff --git a/lib/puppet-lint/configuration.rb b/lib/puppet-lint/configuration.rb index 21bca77fa..041d50aba 100644 --- a/lib/puppet-lint/configuration.rb +++ b/lib/puppet-lint/configuration.rb @@ -153,5 +153,6 @@ def defaults self.show_ignored = false self.ignore_paths = ['vendor/**/*.pp'] self.github_actions = ENV.key?('GITHUB_ACTION') + self.codeclimate_report_file = ENV['CODECLIMATE_REPORT_FILE'] end end diff --git a/lib/puppet-lint/optparser.rb b/lib/puppet-lint/optparser.rb index e4d3c76c6..22b0014f0 100644 --- a/lib/puppet-lint/optparser.rb +++ b/lib/puppet-lint/optparser.rb @@ -102,6 +102,10 @@ def self.build(args = []) PuppetLint.configuration.sarif = true end + opts.on('--codeclimate-report-file FILE', 'Save a code climate compatible report to this file') do |file| + PuppetLint.configuration.codeclimate_report_file = file + end + opts.on('--list-checks', 'List available check names.') do PuppetLint.configuration.list_checks = true end diff --git a/lib/puppet-lint/report/codeclimate.rb b/lib/puppet-lint/report/codeclimate.rb new file mode 100644 index 000000000..56f31bb37 --- /dev/null +++ b/lib/puppet-lint/report/codeclimate.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'digest' +require 'json' + +class PuppetLint::Report + # Formats problems and writes them to a file as a code climate compatible report. + class CodeClimateReporter + def self.write_report_file(problems, report_file) + report = [] + problems.each do |messages| + messages.each do |message| + case message[:kind] + when :warning + severity = 'minor' + when :error + severity = 'major' + else + next + end + + issue = { + type: :issue, + check_name: message[:check], + description: message[:message], + categories: [:Style], + severity: severity, + location: { + path: message[:path], + lines: { + begin: message[:line], + end: message[:line], + } + }, + } + + issue[:fingerprint] = Digest::MD5.hexdigest(Marshal.dump(issue)) + + if message.key?(:description) && message.key?(:help_uri) + issue[:content] = "#{message[:description].chomp('.')}. See [this page](#{message[:help_uri]}) for more information about the `#{message[:check]}` check." + end + report << issue + end + end + File.write(report_file, "#{JSON.pretty_generate(report)}\n") + end + end +end diff --git a/lib/puppet-lint/tasks/puppet-lint.rb b/lib/puppet-lint/tasks/puppet-lint.rb index b620558d5..cd0e25243 100644 --- a/lib/puppet-lint/tasks/puppet-lint.rb +++ b/lib/puppet-lint/tasks/puppet-lint.rb @@ -4,6 +4,7 @@ require 'puppet-lint/optparser' require 'rake' require 'rake/tasklib' +require 'puppet-lint/report/codeclimate' # Public: A Rake task that can be loaded and used with everything you need. # @@ -90,6 +91,7 @@ def define(args, &task_block) RakeFileUtils.send(:verbose, true) do linter = PuppetLint.new matched_files = FileList[@pattern] + all_problems = [] matched_files = matched_files.exclude(*@ignore_paths) @@ -97,12 +99,17 @@ def define(args, &task_block) next unless File.file?(puppet_file) linter.file = puppet_file linter.run - linter.print_problems + all_problems << linter.print_problems if PuppetLint.configuration.fix && linter.problems.none? { |e| e[:check] == :syntax } IO.write(puppet_file, linter.manifest) end end + + if PuppetLint.configuration.codeclimate_report_file + PuppetLint::Report::CodeClimateReporter.write_report_file(all_problems, PuppetLint.configuration.codeclimate_report_file) + end + abort if linter.errors? || ( linter.warnings? && PuppetLint.configuration.fail_on_warnings ) diff --git a/spec/fixtures/test/reports/code_climate.json b/spec/fixtures/test/reports/code_climate.json new file mode 100644 index 000000000..5e658271a --- /dev/null +++ b/spec/fixtures/test/reports/code_climate.json @@ -0,0 +1,38 @@ +[ + { + "type": "issue", + "check_name": "autoloader_layout", + "description": "test::foo not in autoload module layout", + "categories": [ + "Style" + ], + "severity": "major", + "location": { + "path": "spec/fixtures/test/manifests/fail.pp", + "lines": { + "begin": 2, + "end": 2 + } + }, + "fingerprint": "f12bce7a776454ab9ffac2d20dcc34ba", + "content": "Test the manifest tokens for any classes or defined types that are not in an appropriately named file for the autoloader to detect and record an error of each instance found. See [this page](https://puppet.com/docs/puppet/latest/style_guide.html#separate-files) for more information about the `autoloader_layout` check." + }, + { + "type": "issue", + "check_name": "parameter_order", + "description": "optional parameter listed before required parameter", + "categories": [ + "Style" + ], + "severity": "minor", + "location": { + "path": "spec/fixtures/test/manifests/warning.pp", + "lines": { + "begin": 2, + "end": 2 + } + }, + "fingerprint": "e34cf289e008446b633d1be7cf58120e", + "content": "Test the manifest tokens for any parameterised classes or defined types that take parameters and record a warning if there are any optional parameters listed before required parameters. See [this page](https://puppet.com/docs/puppet/latest/style_guide.html#display-order-of-parameters) for more information about the `parameter_order` check." + } +] diff --git a/spec/unit/puppet-lint/bin_spec.rb b/spec/unit/puppet-lint/bin_spec.rb index e7034894b..ad841d2f2 100644 --- a/spec/unit/puppet-lint/bin_spec.rb +++ b/spec/unit/puppet-lint/bin_spec.rb @@ -437,6 +437,30 @@ def initialize(args) end end + context 'when outputting code climate report' do + let(:report_file) do + Tempfile.new('report_file.json') + end + + let(:args) do + [ + '--codeclimate-report-file', + report_file.path, + 'spec/fixtures/test/manifests/fail.pp', + 'spec/fixtures/test/manifests/warning.pp', + ] + end + + after(:each) do + report_file.unlink + end + + it 'creates a code climate report' do + expect(bin.exitstatus).to eq(1) + expect(FileUtils.compare_file(report_file.path, 'spec/fixtures/test/reports/code_climate.json')).to be_truthy + end + end + context 'when hiding ignored problems' do let(:args) do [ diff --git a/spec/unit/puppet-lint/configuration_spec.rb b/spec/unit/puppet-lint/configuration_spec.rb index 21f23a2fa..3dbcc4d67 100644 --- a/spec/unit/puppet-lint/configuration_spec.rb +++ b/spec/unit/puppet-lint/configuration_spec.rb @@ -55,17 +55,18 @@ end expect(config.settings).to eq( - 'with_filename' => false, - 'fail_on_warnings' => false, - 'error_level' => :all, - 'log_format' => '', - 'sarif' => false, - 'with_context' => false, - 'fix' => false, - 'github_actions' => false, - 'show_ignored' => false, - 'json' => false, - 'ignore_paths' => ['vendor/**/*.pp'], + 'with_filename' => false, + 'fail_on_warnings' => false, + 'codeclimate_report_file' => nil, + 'error_level' => :all, + 'log_format' => '', + 'sarif' => false, + 'with_context' => false, + 'fix' => false, + 'github_actions' => false, + 'show_ignored' => false, + 'json' => false, + 'ignore_paths' => ['vendor/**/*.pp'], ) end @@ -78,6 +79,15 @@ expect(config.settings['github_actions']).to be(true) end + it 'defaults codeclimate_report_file to the CODECLIMATE_REPORT_FILE environment variable' do + override_env do + ENV['CODECLIMATE_REPORT_FILE'] = '/path/to/report.json' + config.defaults + end + + expect(config.settings['codeclimate_report_file']).to eq('/path/to/report.json') + end + def override_env old_env = ENV.to_h yield From b89153da4b87a3de6f4d3c9b3513806ccb71dd0f Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 14 Feb 2023 13:36:55 +0100 Subject: [PATCH 2/3] Refactor context handling --- lib/puppet-lint.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/puppet-lint.rb b/lib/puppet-lint.rb index d4507f1e5..7fed5882a 100644 --- a/lib/puppet-lint.rb +++ b/lib/puppet-lint.rb @@ -124,6 +124,7 @@ def format_message(message) puts format % message puts " #{message[:reason]}" if message[:kind] == :ignored && !message[:reason].nil? + print_context(message) end # Internal: Format a problem message and print it to STDOUT so GitHub Actions @@ -154,7 +155,8 @@ def get_context(message) def print_context(message) return if message[:check] == 'documentation' return if message[:kind] == :fixed - line = get_context(message) + line = message[:context] + return unless line offset = line.index(%r{\S}) || 1 puts "\n #{line.strip}" printf("%#{message[:column] + 2 - offset}s\n\n", '^') @@ -175,16 +177,16 @@ def report(problems) next unless message[:kind] == :fixed || [message[:kind], :all].include?(configuration.error_level) + message[:context] = get_context(message) if configuration.with_context + if configuration.json || configuration.sarif || configuration.codeclimate_report_file - message['context'] = get_context(message) if configuration.with_context json << message end - next if configuration.json || configuration.sarif - - format_message(message) - print_context(message) if configuration.with_context - print_github_annotation(message) if configuration.github_actions + unless configuration.json || configuration.sarif + format_message(message) + print_github_annotation(message) if configuration.github_actions + end end $stderr.puts 'Try running `puppet parser validate `' if problems.any? { |p| p[:check] == :syntax } json From 736b9445b7128c25a7414daaa48188a366d9d26f Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 14 Feb 2023 13:40:06 +0100 Subject: [PATCH 3/3] Always return an array of problems in report --- lib/puppet-lint.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/puppet-lint.rb b/lib/puppet-lint.rb index 7fed5882a..2ce571d23 100644 --- a/lib/puppet-lint.rb +++ b/lib/puppet-lint.rb @@ -170,6 +170,8 @@ def print_context(message) # Returns array of problem. def report(problems) json = [] + print_stdout = !(configuration.json || configuration.sarif) + problems.each do |message| next if message[:kind] == :ignored && !PuppetLint.configuration.show_ignored @@ -179,11 +181,9 @@ def report(problems) message[:context] = get_context(message) if configuration.with_context - if configuration.json || configuration.sarif || configuration.codeclimate_report_file - json << message - end + json << message - unless configuration.json || configuration.sarif + if print_stdout format_message(message) print_github_annotation(message) if configuration.github_actions end