From c70511b97087d674afbad98d4b7c4fc326250a28 Mon Sep 17 00:00:00 2001
From: Glenn Sarti <glennsarti@users.noreply.github.com>
Date: Wed, 30 Dec 2020 10:35:36 +0800
Subject: [PATCH] (GH-189) Reset PuppetLint configuration for each call

Previously after a call to fix_validation_errors, many validation errors came
in as hints not errors.  This was due to PuppetLint not resetting the
`PuppetLint.configuration.fix` setting between calls.  This commit updates the
validation provider to reset this setting each call.  This commit also adds
tests for this scenario.
---
 .../manifest/validation_provider.rb           | 41 +++++++++----------
 .../manifest/validation_provider_spec.rb      | 15 +++++++
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/lib/puppet-languageserver/manifest/validation_provider.rb b/lib/puppet-languageserver/manifest/validation_provider.rb
index 382c3d00..3bb7e281 100644
--- a/lib/puppet-languageserver/manifest/validation_provider.rb
+++ b/lib/puppet-languageserver/manifest/validation_provider.rb
@@ -12,14 +12,7 @@ module ValidationProvider
       #    <String> New Content
       #  ]
       def self.fix_validate_errors(session_state, content)
-        module_root = session_state.documents.store_root_path
-        linter_options = nil
-        if module_root.nil?
-          linter_options = PuppetLint::OptParser.build
-        else
-          Dir.chdir(module_root.to_s) { linter_options = PuppetLint::OptParser.build }
-        end
-        linter_options.parse!(['--fix'])
+        init_puppet_lint(session_state.documents.store_root_path, ['--fix'])
 
         linter = PuppetLint::Checks.new
         linter.load_data(nil, content)
@@ -40,19 +33,7 @@ def self.validate(session_state, content, options = {})
         # TODO: Need to implement max_problems
         problems = 0
 
-        module_root = session_state.documents.store_root_path
-        linter_options = nil
-        if module_root.nil?
-          linter_options = PuppetLint::OptParser.build
-        else
-          begin
-            Dir.chdir(module_root.to_s) { linter_options = PuppetLint::OptParser.build }
-          rescue OptionParser::InvalidOption => e
-            PuppetLanguageServer.log_message(:error, "(#{name}) Error reading Puppet Lint configuration.  Using default: #{e}")
-            linter_options = PuppetLint::OptParser.build
-          end
-        end
-        linter_options.parse!([])
+        init_puppet_lint(session_state.documents.store_root_path, [])
 
         begin
           linter = PuppetLint::Checks.new
@@ -128,6 +109,24 @@ def self.validate(session_state, content, options = {})
 
         result
       end
+
+      def self.init_puppet_lint(root_dir, lint_options = [])
+        linter_options = nil
+        if root_dir.nil?
+          linter_options = PuppetLint::OptParser.build
+        else
+          begin
+            Dir.chdir(module_root.to_s) { linter_options = PuppetLint::OptParser.build }
+          rescue OptionParser::InvalidOption => e
+            PuppetLanguageServer.log_message(:error, "(#{name}) Error reading Puppet Lint configuration.  Using default: #{e}")
+            linter_options = PuppetLint::OptParser.build
+          end
+        end
+        # Reset the fix flag
+        PuppetLint.configuration.fix = false
+        linter_options.parse!(lint_options)
+      end
+      private_class_method :init_puppet_lint
     end
   end
 end
diff --git a/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb b/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb
index ecb60560..ca752a31 100644
--- a/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb
+++ b/spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb
@@ -194,5 +194,20 @@
         end
       end
     end
+
+    describe "Given a complete manifest with validation errors" do
+      let(:manifest) { "class bad_formatting {\n  user { 'username':\n    ensure => absent,\n    auth_membership => 'false',\n  }\n} " }
+
+      it "should return errors and warnings even after fix_validate_errors" do
+        fixes = subject.fix_validate_errors(session_state, manifest)
+        validation = subject.validate(session_state, manifest)
+
+        expect(validation.count).to_not be_zero
+
+        validation.each do |problem|
+          expect([LSP::DiagnosticSeverity::ERROR, LSP::DiagnosticSeverity::WARNING]).to include(problem.severity)
+        end
+      end
+    end
   end
 end