Add SARIF support (#40)

* add SARIF support
* SARIF base path uri should end with `/`
* Use better sample data in SARIF template
* Add help_uri and description to SARIF output
* update example values in sarif template
* continue-on-error: true
* use STDERR.put for warnings
* remove clean step* use Pathname.new
* use delete_if* remove not needed strategy matrix
* fix rule index
* upgrade to use upload-sarif@v2 instead of upload-sarif@v1
* removed example scan.yml

lib/puppet-lint.rb

@@ -161,7 +161,7 @@ def print_context(message)
   # problems - An Array of problem Hashes as returned by
   #            PuppetLint::Checks#run.
   #
-  # Returns nothing.
+  # Returns array of problem.
   def report(problems)
     json = []
     problems.each do |message|
@@ -171,7 +171,7 @@ def report(problems)
 
       next unless message[:kind] == :fixed || [message[:kind], :all].include?(configuration.error_level)
 
-      if configuration.json
+      if configuration.json || configuration.sarif
         message['context'] = get_context(message) if configuration.with_context
         json << message
       else
@@ -180,9 +180,8 @@ def report(problems)
         print_github_annotation(message) if configuration.github_actions
       end
     end
-    puts JSON.pretty_generate(json) if configuration.json
-
     $stderr.puts 'Try running `puppet parser validate <file>`' if problems.any? { |p| p[:check] == :syntax }
+    json
   end
 
   # Public: Determine if PuppetLint found any errors in the manifest.

lib/puppet-lint/bin.rb

@@ -1,3 +1,5 @@
+require 'pathname'
+require 'uri'
 require 'puppet-lint/optparser'
 
 # Internal: The logic of the puppet-lint bin script, contained in a class for
@@ -46,6 +48,21 @@ def run
 
     begin
       path = @args[0]
+      full_path = File.expand_path(path, ENV['PWD'])
+      full_base_path = if File.directory?(full_path)
+                         full_path
+                       else
+                         File.dirname(full_path)
+                       end
+
+      full_base_path_uri = if full_base_path.start_with?('/')
+                             'file://' + full_base_path
+                           else
+                             'file:///' + full_base_path
+                           end
+
+      full_base_path_uri += '/' unless full_base_path_uri.end_with?('/')
+
       path = path.gsub(File::ALT_SEPARATOR, File::SEPARATOR) if File::ALT_SEPARATOR
       path = if File.directory?(path)
                Dir.glob("#{path}/**/*.pp")
@@ -57,15 +74,14 @@ def run
 
       return_val = 0
       ignore_paths = PuppetLint.configuration.ignore_paths
+      all_problems = []
 
-      puts '[' if PuppetLint.configuration.json
       path.each do |f|
         next if ignore_paths.any? { |p| File.fnmatch(p, f) }
         l = PuppetLint.new
         l.file = f
         l.run
-        l.print_problems
-        puts ',' if f != path.last && PuppetLint.configuration.json
+        all_problems << l.print_problems
 
         if l.errors? || (l.warnings? && PuppetLint.configuration.fail_on_warnings)
           return_val = 1
@@ -76,7 +92,17 @@ def run
           fd.write(l.manifest)
         end
       end
-      puts ']' if PuppetLint.configuration.json
+
+      if PuppetLint.configuration.sarif
+        report_sarif(all_problems, full_base_path, full_base_path_uri)
+      elsif PuppetLint.configuration.json
+        all_problems.each do |problems|
+          problems.each do |problem|
+            problem.delete_if { |key, _| [:description, :help_uri].include?(key) }
+          end
+        end
+        puts JSON.pretty_generate(all_problems)
+      end
 
       return return_val
     rescue PuppetLint::NoCodeError
@@ -85,4 +111,42 @@ def run
       return 1
     end
   end
+
+  # Internal: Print the reported problems in SARIF format to stdout.
+  #
+  # problems - An Array of problem Hashes as returned by
+  #            PuppetLint::Checks#run.
+  #
+  # Returns nothing.
+  def report_sarif(problems, base_path, base_path_uri)
+    sarif_file = File.read(File.join(__dir__, 'report', 'sarif_template.json'))
+    sarif = JSON.parse(sarif_file)
+    sarif['runs'][0]['originalUriBaseIds']['ROOTPATH']['uri'] = base_path_uri
+    rules = sarif['runs'][0]['tool']['driver']['rules'] = []
+    results = sarif['runs'][0]['results'] = []
+    problems.each do |messages|
+      messages.each do |message|
+        relative_path = Pathname.new(message[:fullpath]).relative_path_from(Pathname.new(base_path))
+        rules = sarif['runs'][0]['tool']['driver']['rules']
+        rule_exists = rules.any? { |r| r['id'] == message[:check] }
+        unless rule_exists
+          new_rule = { 'id' => message[:check] }
+          new_rule['fullDescription'] = { 'text' => message[:description] } unless message[:description].nil?
+          new_rule['fullDescription'] = { 'text' => 'Check for any syntax error and record an error of each instance found.' } if new_rule['fullDescription'].nil? && message[:check] == :syntax
+          new_rule['defaultConfiguration'] = { 'level' => message[:KIND].downcase } if %w[error warning].include?(message[:KIND].downcase)
+          new_rule['helpUri'] = message[:help_uri] unless message[:help_uri].nil?
+          rules << new_rule
+        end
+        rule_index = rules.index { |r| r['id'] == message[:check] }
+        result = {
+          'ruleId' => message[:check],
+          'ruleIndex' => rule_index,
+          'message' => { 'text' => message[:message] },
+          'locations' => [{ 'physicalLocation' => { 'artifactLocation' => { 'uri' => relative_path, 'uriBaseId' => 'ROOTPATH' }, 'region' => { 'startLine' => message[:line], 'startColumn' => message[:column] } } }],
+        }
+        results << result
+      end
+    end
+    puts JSON.pretty_generate(sarif)
+  end
 end

lib/puppet-lint/checkplugin.rb

@@ -158,10 +158,12 @@ def default_info
   #
   # kind    - The Symbol problem type (:warning or :error).
   # problem - A Hash containing the attributes of the problem
-  #   :message - The String message describing the problem.
-  #   :line    - The Integer line number of the location of the problem.
-  #   :column  - The Integer column number of the location of the problem.
-  #   :check   - The Symbol name of the check that detected the problem.
+  #   :message      - The String message describing the problem.
+  #   :line         - The Integer line number of the location of the problem.
+  #   :column       - The Integer column number of the location of the problem.
+  #   :check        - The Symbol name of the check that detected the problem.
+  #   :description  - The description of the check that detected the problem.
+  #   :help_uri     - The help web page of the check that detected the problem.
   #
   # Returns nothing.
   def notify(kind, problem)

lib/puppet-lint/configuration.rb

@@ -150,6 +150,7 @@ def defaults
       self.with_context = false
       self.fix = false
       self.json = false
+      self.sarif = false
       self.show_ignored = false
       self.ignore_paths = ['vendor/**/*.pp']
       self.github_actions = ENV.key?('GITHUB_ACTION')

lib/puppet-lint/data.rb

@@ -591,7 +591,7 @@ def parse_control_comments
             if top_override.nil?
               # TODO: refactor to provide a way to expose problems from
               # PuppetLint::Data via the normal problem reporting mechanism.
-              puts "WARNING: lint:endignore comment with no opening lint:ignore:<check> comment found on line #{token.line}"
+              $stderr.puts "WARNING: lint:endignore comment with no opening lint:ignore:<check> comment found on line #{token.line}"
             else
               top_override.each do |start|
                 next if start.nil?
@@ -607,7 +607,7 @@ def parse_control_comments
       end
 
       stack.each do |control|
-        puts "WARNING: lint:ignore:#{control[0][2]} comment on line #{control[0][0]} with no closing lint:endignore comment"
+        $stderr.puts "WARNING: lint:ignore:#{control[0][2]} comment on line #{control[0][0]} with no closing lint:endignore comment"
       end
     end
   end

lib/puppet-lint/optparser.rb

@@ -98,6 +98,10 @@ def self.build(args = [])
         PuppetLint.configuration.json = true
       end
 
+      opts.on('--sarif', 'Log output as SARIF') do
+        PuppetLint.configuration.sarif = true
+      end
+
       opts.on('--list-checks', 'List available check names.') do
         PuppetLint.configuration.list_checks = true
       end

lib/puppet-lint/plugins/check_classes/arrow_on_right_operand_line.rb

@@ -9,10 +9,12 @@ def check
 
       notify(
         :warning,
-        :message => "arrow should be on the right operand's line",
-        :line    => token.line,
-        :column  => token.column,
-        :token   => token
+        :message     => "arrow should be on the right operand's line",
+        :line        => token.line,
+        :column      => token.column,
+        :token       => token,
+        :description => 'Test the manifest tokens for chaining arrow that is on the line of the left operand when the right operand is on another line.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#chaining-arrow-syntax'
       )
     end
   end

lib/puppet-lint/plugins/check_classes/autoloader_layout.rb

@@ -25,9 +25,11 @@ def check
 
       notify(
         :error,
-        :message => "#{title_token.value} not in autoload module layout",
-        :line    => title_token.line,
-        :column  => title_token.column
+        :message     => "#{title_token.value} not in autoload module layout",
+        :line        => title_token.line,
+        :column      => title_token.column,
+        :description => '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.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#separate-files'
       )
     end
   end

lib/puppet-lint/plugins/check_classes/class_inherits_from_params_class.rb

@@ -9,9 +9,11 @@ def check
 
       notify(
         :warning,
-        :message => 'class inheriting from params class',
-        :line    => class_idx[:inherited_token].line,
-        :column  => class_idx[:inherited_token].column
+        :message     => 'class inheriting from params class',
+        :line        => class_idx[:inherited_token].line,
+        :column      => class_idx[:inherited_token].column,
+        :description => 'Check the manifest tokens for any classes that inherit a params subclass and record a warning for each instance found.',
+        :help_uri    => nil
       )
     end
   end

lib/puppet-lint/plugins/check_classes/code_on_top_scope.rb

@@ -11,9 +11,11 @@ def check
 
       notify(
         :warning,
-        :message => "code outside of class or define block - #{token.value}",
-        :line    => token.line,
-        :column  => token.column
+        :message     => "code outside of class or define block - #{token.value}",
+        :line        => token.line,
+        :column      => token.column,
+        :description => 'Test that no code is outside of a class or define scope.',
+        :help_uri    => nil
       )
     end
   end

lib/puppet-lint/plugins/check_classes/inherits_across_namespaces.rb

@@ -14,9 +14,11 @@ def check
 
       notify(
         :warning,
-        :message => 'class inherits across module namespaces',
-        :line    => class_idx[:inherited_token].line,
-        :column  => class_idx[:inherited_token].column
+        :message     => 'class inherits across module namespaces',
+        :line        => class_idx[:inherited_token].line,
+        :column      => class_idx[:inherited_token].column,
+        :description => 'Test the manifest tokens for any classes that inherit across namespaces and record a warning for each instance found.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#class-inheritance'
       )
     end
   end

lib/puppet-lint/plugins/check_classes/names_containing_dash.rb

@@ -15,9 +15,11 @@ def check
 
       notify(
         :error,
-        :message => "#{obj_type} name containing a dash",
-        :line    => class_idx[:name_token].line,
-        :column  => class_idx[:name_token].column
+        :message     => "#{obj_type} name containing a dash",
+        :line        => class_idx[:name_token].line,
+        :column      => class_idx[:name_token].column,
+        :description => 'Check the manifest tokens for any classes or defined types that have a dash in their name and record an error for each instance found.',
+        :help_uri    => nil
       )
     end
   end

lib/puppet-lint/plugins/check_classes/names_containing_uppercase.rb

@@ -15,10 +15,12 @@ def check
 
       notify(
         :error,
-        :message => "#{obj_type} '#{class_idx[:name_token].value}' contains illegal uppercase",
-        :line    => class_idx[:name_token].line,
-        :column  => class_idx[:name_token].column,
-        :token   => class_idx[:name_token]
+        :message     => "#{obj_type} '#{class_idx[:name_token].value}' contains illegal uppercase",
+        :line        => class_idx[:name_token].line,
+        :column      => class_idx[:name_token].column,
+        :token       => class_idx[:name_token],
+        :description => 'Find and warn about module names with illegal uppercase characters.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/modules_fundamentals.html#allowed-module-names'
       )
     end
   end

lib/puppet-lint/plugins/check_classes/nested_classes_or_defines.rb

@@ -17,9 +17,11 @@ def check
 
         notify(
           :warning,
-          :message => "#{type} defined inside a class",
-          :line    => token.line,
-          :column  => token.column
+          :message     => "#{type} defined inside a class",
+          :line        => token.line,
+          :column      => token.column,
+          :description => 'Test the manifest tokens for any classes or defined types that are defined inside another class.',
+          :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#nested-classes-or-defined-types'
         )
       end
     end

lib/puppet-lint/plugins/check_classes/parameter_order.rb

@@ -31,9 +31,11 @@ def check
         msg = 'optional parameter listed before required parameter'
         notify(
           :warning,
-          :message => msg,
-          :line    => token.line,
-          :column  => token.column
+          :message     => msg,
+          :line        => token.line,
+          :column      => token.column,
+          :description => '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.',
+          :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#display-order-of-parameters'
         )
       end
     end

lib/puppet-lint/plugins/check_classes/right_to_left_relationship.rb

@@ -7,9 +7,11 @@ def check
     tokens.select { |r| r.type == :OUT_EDGE }.each do |token|
       notify(
         :warning,
-        :message =>  'right-to-left (<-) relationship',
-        :line    => token.line,
-        :column  => token.column
+        :message     => 'right-to-left (<-) relationship',
+        :line        => token.line,
+        :column      => token.column,
+        :description => 'Test the manifest tokens for any right-to-left (<-) chaining operators and record a warning for each instance found.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#chaining-arrow-syntax'
       )
     end
   end

lib/puppet-lint/plugins/check_classes/variable_scope.rb

@@ -130,9 +130,13 @@ def check
 
         notify(
           :warning,
-          :message => msg,
-          :line    => token.line,
-          :column  => token.column
+          :message     => msg,
+          :line        => token.line,
+          :column      => token.column,
+          :description => 'Test the manifest tokens for any variables that are referenced in the manifest. ' \
+                          'If the variables are not fully qualified or one of the variables automatically created in the scope, ' \
+                          'check that they have been defined in the local scope and record a warning for each variable that has not.',
+          :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#namespacing-variables'
         )
       end
     end

lib/puppet-lint/plugins/check_comments/slash_comments.rb

@@ -9,10 +9,12 @@ def check
     }.each do |token|
       notify(
         :warning,
-        :message => '// comment found',
-        :line    => token.line,
-        :column  => token.column,
-        :token   => token
+        :message     => '// comment found',
+        :line        => token.line,
+        :column      => token.column,
+        :token       => token,
+        :description => 'Check the manifest tokens for any comments started with slashes (//) and record a warning for each instance found.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#comments'
       )
     end
   end

lib/puppet-lint/plugins/check_comments/star_comments.rb

@@ -9,10 +9,12 @@ def check
     }.each do |token|
       notify(
         :warning,
-        :message => '/* */ comment found',
-        :line    => token.line,
-        :column  => token.column,
-        :token   => token
+        :message     => '/* */ comment found',
+        :line        => token.line,
+        :column      => token.column,
+        :token       => token,
+        :description => 'Check the manifest tokens for any comments encapsulated with slash-asterisks (/* */) and record a warning for each instance found.',
+        :help_uri    => 'https://puppet.com/docs/puppet/latest/style_guide.html#comments'
       )
     end
   end