(GH-26) Use document store root path during manifest validation

Now that the workspace root detection has been moved from the manifest validator
to the Document Store, the manifest validator can use the new methods.  This
commit;
* Updates the validator to use the new Document Store root path detection
* Removes redundant method from the validator
* Removes the workspace argument as it's no longer required
* Updates all call-sites and tests with the removal of the workspace argument

Author: glennsarti

lib/puppet-languageserver/epp/validation_provider.rb

@@ -1,7 +1,7 @@
 module PuppetLanguageServer
   module Epp
     module ValidationProvider
-      def self.validate(content, _workspace, _max_problems = 100)
+      def self.validate(content, _max_problems = 100)
         result = []
         # TODO: Need to implement max_problems
         _problems = 0

lib/puppet-languageserver/manifest/validation_provider.rb

@@ -2,39 +2,15 @@
 module PuppetLanguageServer
   module Manifest
     module ValidationProvider
-      def self.find_module_root_from_path(path)
-        return nil if path.nil?
-
-        filepath = Pathname.new(path).expand_path
-        return nil unless filepath.exist?
-
-        if filepath.directory?
-          directory = filepath
-        else
-          directory = filepath.dirname
-        end
-
-        module_root = nil
-        directory.ascend do |p|
-          if p.join('metadata.json').exist?
-            module_root = p
-            break
-          end
-        end
-
-        module_root
-      end
-
       # Similar to 'validate' this will run puppet-lint and returns
       # the manifest with any fixes applied
       #
       # Returns:
       #  [ <Int> Number of problems fixed,
       #    <String> New Content
       #  ]
-      def self.fix_validate_errors(content, workspace)
-        # Find module root and attempt to build PuppetLint options
-        module_root = find_module_root_from_path(workspace)
+      def self.fix_validate_errors(content)
+        module_root = PuppetLanguageServer::DocumentStore.store_root_path
         linter_options = nil
         if module_root.nil?
           linter_options = PuppetLint::OptParser.build
@@ -52,13 +28,12 @@ def self.fix_validate_errors(content, workspace)
         [problems_fixed, linter.manifest]
       end
 
-      def self.validate(content, workspace, _max_problems = 100)
+      def self.validate(content, _max_problems = 100)
         result = []
         # TODO: Need to implement max_problems
         problems = 0
 
-        # Find module root and attempt to build PuppetLint options
-        module_root = find_module_root_from_path(workspace)
+        module_root = PuppetLanguageServer::DocumentStore.store_root_path
         linter_options = nil
         if module_root.nil?
           linter_options = PuppetLint::OptParser.build

lib/puppet-languageserver/message_router.rb

@@ -2,9 +2,7 @@ module PuppetLanguageServer
   class MessageRouter
     attr_accessor :json_rpc_handler
 
-    def initialize(options = {})
-      options = {} if options.nil?
-      @workspace = options[:workspace]
+    def initialize(_options = {})
     end
 
     def documents
@@ -90,7 +88,7 @@ def receive_request(request)
 
           case documents.document_type(file_uri)
           when :manifest
-            changes, new_content = PuppetLanguageServer::Manifest::ValidationProvider.fix_validate_errors(content, @workspace)
+            changes, new_content = PuppetLanguageServer::Manifest::ValidationProvider.fix_validate_errors(content)
           else
             raise "Unable to fixDiagnosticErrors on #{file_uri}"
           end
@@ -194,7 +192,7 @@ def receive_notification(method, params)
         content = params['textDocument']['text']
         doc_version = params['textDocument']['version']
         documents.set_document(file_uri, content, doc_version)
-        PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, @json_rpc_handler)
+        PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @json_rpc_handler)
 
       when 'textDocument/didClose'
         PuppetLanguageServer.log_message(:info, 'Received textDocument/didClose notification.')
@@ -207,7 +205,7 @@ def receive_notification(method, params)
         content = params['contentChanges'][0]['text'] # TODO: Bad hardcoding zero
         doc_version = params['textDocument']['version']
         documents.set_document(file_uri, content, doc_version)
-        PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, @json_rpc_handler)
+        PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @json_rpc_handler)
 
       when 'textDocument/didSave'
         PuppetLanguageServer.log_message(:info, 'Received textDocument/didSave notification.')

lib/puppet-languageserver/validation_queue.rb

@@ -11,7 +11,7 @@ module ValidationQueue
     @queue_thread = nil
 
     # Enqueue a file to be validated
-    def self.enqueue(file_uri, doc_version, workspace, connection_object)
+    def self.enqueue(file_uri, doc_version, connection_object)
       document_type = PuppetLanguageServer::DocumentStore.document_type(file_uri)
 
       unless %i[manifest epp puppetfile].include?(document_type)
@@ -27,7 +27,6 @@ def self.enqueue(file_uri, doc_version, workspace, connection_object)
           'file_uri'          => file_uri,
           'doc_version'       => doc_version,
           'document_type'     => document_type,
-          'workspace'         => workspace,
           'connection_object' => connection_object
         }
       end
@@ -47,11 +46,11 @@ def self.enqueue(file_uri, doc_version, workspace, connection_object)
     end
 
     # Synchronously validate a file
-    def self.validate_sync(file_uri, doc_version, workspace, connection_object)
+    def self.validate_sync(file_uri, doc_version, connection_object)
       document_type = PuppetLanguageServer::DocumentStore.document_type(file_uri)
       content = documents.document(file_uri, doc_version)
       return nil if content.nil?
-      result = validate(document_type, content, workspace)
+      result = validate(document_type, content)
 
       # Send the response
       connection_object.reply_diagnostics(file_uri, result)
@@ -79,15 +78,15 @@ def self.reset_queue(initial_state = [])
     end
 
     # Validate a document
-    def self.validate(document_type, content, workspace)
+    def self.validate(document_type, content)
       # Perform validation
       case document_type
       when :manifest
-        PuppetLanguageServer::Manifest::ValidationProvider.validate(content, workspace)
+        PuppetLanguageServer::Manifest::ValidationProvider.validate(content)
       when :epp
-        PuppetLanguageServer::Epp::ValidationProvider.validate(content, workspace)
+        PuppetLanguageServer::Epp::ValidationProvider.validate(content)
       when :puppetfile
-        PuppetLanguageServer::Puppetfile::ValidationProvider.validate(content, workspace)
+        PuppetLanguageServer::Puppetfile::ValidationProvider.validate(content)
       else
         []
       end
@@ -109,7 +108,6 @@ def self.worker
         doc_version       = work_item['doc_version']
         connection_object = work_item['connection_object']
         document_type     = work_item['document_type']
-        workspace         = work_item['workspace']
 
         # Check if the document is the latest version
         content = documents.document(file_uri, doc_version)
@@ -119,7 +117,7 @@ def self.worker
         end
 
         # Perform validation
-        result = validate(document_type, content, workspace)
+        result = validate(document_type, content)
 
         # Check if the document is still latest version
         current_version = documents.document_version(file_uri)

spec/languageserver/integration/puppet-languageserver/manifest/validation_provider_spec.rb

@@ -8,7 +8,7 @@
       let(:manifest) { 'user { \'Bob\'' }
 
       it "should return no changes" do
-        problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
+        problems_fixed, new_content = subject.fix_validate_errors(manifest)
         expect(problems_fixed).to eq(0)
         expect(new_content).to eq(manifest)
       end
@@ -27,7 +27,7 @@
       }
 
       it "should return changes" do
-        problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
+        problems_fixed, new_content = subject.fix_validate_errors(manifest)
         expect(problems_fixed).to eq(1)
         expect(new_content).to eq(new_manifest)
       end
@@ -50,7 +50,7 @@
       }
 
       it "should return changes" do
-        problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
+        problems_fixed, new_content = subject.fix_validate_errors(manifest)
         expect(problems_fixed).to eq(3)
         expect(new_content).to eq(new_manifest)
       end
@@ -66,7 +66,7 @@
       }
 
       it "should return no changes" do
-        problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
+        problems_fixed, new_content = subject.fix_validate_errors(manifest)
         expect(problems_fixed).to eq(0)
         expect(new_content).to eq(manifest)
       end
@@ -78,7 +78,7 @@
 
       it "should preserve CRLF" do
         pending('Release of https://github.com/rodjek/puppet-lint/commit/2a850ab3fd3694a4dd0c4d2f22a1e60b9ca0a495')
-        problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
+        problems_fixed, new_content = subject.fix_validate_errors(manifest)
         expect(problems_fixed).to eq(1)
         expect(new_content).to eq(new_manifest)
       end
@@ -92,7 +92,7 @@
       }
 
       it "should return no changes" do
-        problems_fixed, new_content = subject.fix_validate_errors(manifest, nil)
+        problems_fixed, new_content = subject.fix_validate_errors(manifest)
         expect(problems_fixed).to eq(0)
         expect(new_content).to eq(manifest)
       end
@@ -104,7 +104,7 @@
       let(:manifest) { 'user { "Bob"' }
 
       it "should return at least one error" do
-        result = subject.validate(manifest, nil)
+        result = subject.validate(manifest)
         expect(result.length).to be > 0
       end
     end
@@ -113,7 +113,7 @@
       let(:manifest) { "user { 'Bob': ensure => 'present' }" }
 
       it "should return an empty array" do
-        expect(subject.validate(manifest, nil)).to eq([])
+        expect(subject.validate(manifest)).to eq([])
       end
     end
 
@@ -123,8 +123,8 @@
       let(:manifest_crlf) { File.open(manifest_fixture, 'r') { |file| file.read }.gsub("\n","\r\n") }
 
       it "should return same errors for both LF and CRLF line endings" do
-        lint_error_lf = subject.validate(manifest_lf, nil)
-        lint_error_crlf = subject.validate(manifest_crlf, nil)
+        lint_error_lf = subject.validate(manifest_lf)
+        lint_error_crlf = subject.validate(manifest_crlf)
         expect(lint_error_crlf).to eq(lint_error_lf)
       end
    end
@@ -138,11 +138,11 @@
       }
 
       it "should return an array with one entry" do
-        expect(subject.validate(manifest, nil).count).to eq(1)
+        expect(subject.validate(manifest).count).to eq(1)
       end
 
       it "should return an entry with linting error information" do
-        lint_error = subject.validate(manifest, nil)[0]
+        lint_error = subject.validate(manifest)[0]
 
         expect(lint_error['source']).to eq('Puppet')
         expect(lint_error['message']).to match('140')
@@ -161,7 +161,7 @@
           }
 
           it "should return an empty array" do
-            expect(subject.validate(manifest, nil)).to eq([])
+            expect(subject.validate(manifest)).to eq([])
           end
         end
 
@@ -176,7 +176,7 @@
           }
 
           it "should return an empty array" do
-            expect(subject.validate(manifest, nil)).to eq([])
+            expect(subject.validate(manifest)).to eq([])
           end
         end
       end

spec/languageserver/unit/puppet-languageserver/message_router_spec.rb

@@ -538,7 +538,7 @@
         end
 
         it 'should enqueue the file for validation' do
-          expect(PuppetLanguageServer::ValidationQueue).to receive(:enqueue).with(file_uri, 1, Object, Object)
+          expect(PuppetLanguageServer::ValidationQueue).to receive(:enqueue).with(file_uri, 1, Object)
           subject.receive_notification(notification_method, notification_params)
         end
       end
@@ -609,7 +609,7 @@
         end
 
         it 'should enqueue the file for validation' do
-          expect(PuppetLanguageServer::ValidationQueue).to receive(:enqueue).with(file_uri, 2, Object, Object)
+          expect(PuppetLanguageServer::ValidationQueue).to receive(:enqueue).with(file_uri, 2, Object)
           subject.receive_notification(notification_method, notification_params)
         end
       end