Switch ScssLint pre-commit hook to use JSON formatter

There is a pull request on the scss-lint project to change the default
formatter to include more information and progress:

  sds/scss-lint#695

Unfortunately, changing the default formatter output breaks this hook.
Fix this by switching the hook to use the JSON formatter.

Author: sds

CHANGELOG.md

@@ -11,6 +11,8 @@
 * Fix `Minitest` pre-push hook to include all test files
 * Add `MessageFormat` commit-msg hook to validate commit messages against
   a regex pattern
+* Switch `ScssLint` pre-commit hook to use the JSON output formatter instead
+  of the default formatter
 
 ## 0.32.0.rc1
 

config/default.yml

@@ -482,7 +482,9 @@ PreCommit:
   ScssLint:
     enabled: false
     description: 'Analyzing with scss-lint'
+    required_library: 'json'
     required_executable: 'scss-lint'
+    flags: ['--format', 'JSON']
     install_command: 'gem install scss-lint'
     include: '**/*.scss'
 

lib/overcommit/hook/pre_commit/scss_lint.rb

@@ -3,10 +3,6 @@ module Overcommit::Hook::PreCommit
   #
   # @see https://github.com/brigade/scss-lint
   class ScssLint < Base
-    MESSAGE_TYPE_CATEGORIZER = lambda do |type|
-      type.include?('W') ? :warning : :error
-    end
-
     def run
       result = execute(command, args: applicable_files)
 
@@ -16,13 +12,30 @@ def run
       return :pass if [0, 81].include?(result.status)
 
       # Any status that isn't indicating lint warnings or errors indicates failure
-      return :fail, result.stdout unless [1, 2].include?(result.status)
+      return :fail, (result.stdout + result.stderr) unless [1, 2].include?(result.status)
+
+      begin
+        collect_lint_messages(JSON.parse(result.stdout))
+      rescue JSON::ParserError => ex
+        return :fail, "Unable to parse JSON returned by SCSS-Lint: #{ex.message}\n" \
+                      "STDOUT: #{result.stdout}\nSTDERR: #{result.stderr}"
+      end
+    end
+
+    private
+
+    def collect_lint_messages(files_to_lints)
+      files_to_lints.flat_map do |path, lints|
+        lints.map do |lint|
+          severity = lint['severity'] == 'warning' ? :warning : :error
+
+          message = lint['reason']
+          message = "#{lint['linter']}: #{message}" if lint['linter']
+          message = "#{path}:#{lint['line']} #{message}"
 
-      extract_messages(
-        result.stdout.split("\n"),
-        /^(?<file>(?:\w:)?[^:]+):(?<line>\d+)[^ ]* (?<type>[^ ]+)/,
-        MESSAGE_TYPE_CATEGORIZER,
-      )
+          Overcommit::Hook::Message.new(severity, path, lint['line'], message)
+        end
+      end
     end
   end
 end

spec/overcommit/hook/pre_commit/scss_lint_spec.rb

@@ -30,9 +30,21 @@
     context 'and it reports a warning' do
       before do
         result.stub(:status).and_return(1)
-        result.stub(:stdout).and_return([
-          'file1.scss:1 [W] Prefer single quoted strings',
-        ].join("\n"))
+        result.stub(:stderr).and_return('')
+        result.stub(:stdout).and_return(<<-JSON)
+          {
+            "test.scss": [
+              {
+                "line": 1,
+                "column": 1,
+                "length": 2,
+                "severity": "warning",
+                "reason": "Empty rule",
+                "linter": "EmptyRule"
+              }
+            ]
+          }
+        JSON
       end
 
       it { should warn }
@@ -41,17 +53,39 @@
     context 'and it reports an error' do
       before do
         result.stub(:status).and_return(2)
-        result.stub(:stdout).and_return([
-          'file1.scss:1 [E] Syntax Error: Invalid CSS',
-        ].join("\n"))
+        result.stub(:stderr).and_return('')
+        result.stub(:stdout).and_return(<<-JSON)
+          {
+            "test.scss": [
+              {
+                "line": 1,
+                "column": 1,
+                "length": 2,
+                "severity": "error",
+                "reason": "Syntax error",
+              }
+            ]
+          }
+        JSON
       end
 
       it { should fail_hook }
     end
 
+    context 'and it returns invalid JSON' do
+      before do
+        result.stub(:status).and_return(1)
+        result.stub(:stderr).and_return('')
+        result.stub(:stdout).and_return('This is not JSON')
+      end
+
+      it { should fail_hook /Unable to parse JSON returned by SCSS-Lint/ }
+    end
+
     context 'and it returns status code indicating all files were filtered' do
       before do
         result.stub(:status).and_return(81)
+        result.stub(:stderr).and_return('')
         result.stub(:stdout).and_return('')
       end