(GH-28) Add mechanism for Puppetfile validation

glennsarti · glennsarti · commit 90bc4cfc4f29 · 2018-05-15T22:13:18.000+08:00
Previously the language server could identify a Puppetfile file but would not
actually do anything with it.  This commit;

* Adds a basic Puppetfile parser based on the skeleton of the R10K gem
* Adds valiadation that the ruby style Puppetfile could be parsed and reports
  diagnostic errors if found
* Integrates into the async validation process
* Adds unit tests for the provider
diff --git a/lib/puppet-languageserver/providers.rb b/lib/puppet-languageserver/providers.rb
@@ -4,6 +4,8 @@
   manifest/definition_provider
   manifest/validation_provider
   manifest/hover_provider
+  puppetfile/r10k/puppetfile
+  puppetfile/validation_provider
 ].each do |lib|
   begin
     require "puppet-languageserver/#{lib}"
diff --git a/lib/puppet-languageserver/puppetfile/r10k/puppetfile.rb b/lib/puppet-languageserver/puppetfile/r10k/puppetfile.rb
@@ -0,0 +1,37 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      PUPPETFILE_MONIKER ||= 'Puppetfile'.freeze
+
+      class Puppetfile
+        def load!(puppetfile_contents)
+          puppetfile = DSL.new(self)
+          puppetfile.instance_eval(puppetfile_contents, PUPPETFILE_MONIKER)
+        end
+
+        class DSL
+          def initialize(parent)
+            @parent = parent
+          end
+
+          # @param [String] name
+          # @param [*Object] args
+          def mod(_name, _args = nil)
+          end
+
+          # @param [String] forge
+          def forge(_location)
+          end
+
+          # @param [String] moduledir
+          def moduledir(_location)
+          end
+
+          def method_missing(method, *_args) # rubocop:disable Style/MethodMissing
+            raise NoMethodError, format("Unknown method '%<method>s'", method: method)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/puppet-languageserver/puppetfile/validation_provider.rb b/lib/puppet-languageserver/puppetfile/validation_provider.rb
@@ -0,0 +1,43 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module ValidationProvider
+      def self.max_line_length
+        # TODO: ... need to figure out the actual line length
+        1000
+      end
+
+      def self.validate(content, _workspace, _max_problems = 100)
+        result = []
+        # TODO: Need to implement max_problems
+        _problems = 0
+
+        # Attempt to parse the file
+        puppetfile = nil
+        begin
+          puppetfile = PuppetLanguageServer::Puppetfile::R10K::Puppetfile.new
+          puppetfile.load!(content)
+        rescue StandardError, SyntaxError, LoadError => detail
+          # Find the originating error from within the puppetfile
+          loc = detail.backtrace_locations
+                      .select { |item| item.absolute_path == PuppetLanguageServer::Puppetfile::R10K::PUPPETFILE_MONIKER }
+                      .first
+          start_line_number = loc.nil? ? 0 : loc.lineno - 1 # Line numbers from ruby are base 1
+          end_line_number = loc.nil? ? content.lines.count - 1 : loc.lineno - 1 # Line numbers from ruby are base 1
+          # Note - Ruby doesn't give a character position so just highlight the entire line
+          result << LanguageServer::Diagnostic.create('severity' => LanguageServer::DIAGNOSTICSEVERITY_ERROR,
+                                                      'fromline' => start_line_number,
+                                                      'toline'   => end_line_number,
+                                                      'fromchar' => 0,
+                                                      'tochar'   => max_line_length,
+                                                      'source'   => 'Puppet',
+                                                      'message'  => detail.to_s)
+
+          puppetfile = nil
+        end
+        return result if puppetfile.nil?
+
+        result
+      end
+    end
+  end
+end
diff --git a/lib/puppet-languageserver/validation_queue.rb b/lib/puppet-languageserver/validation_queue.rb
@@ -14,7 +14,7 @@ module ValidationQueue
     def self.enqueue(file_uri, doc_version, workspace, connection_object)
       document_type = PuppetLanguageServer::DocumentStore.document_type(file_uri)
 
-      unless %i[manifest epp].include?(document_type)
+      unless %i[manifest epp puppetfile].include?(document_type)
         # Can't validate these types so just emit an empty validation result
         connection_object.reply_diagnostics(file_uri, [])
         return
@@ -86,6 +86,8 @@ def self.validate(document_type, content, workspace)
         PuppetLanguageServer::Manifest::ValidationProvider.validate(content, workspace)
       when :epp
         PuppetLanguageServer::Epp::ValidationProvider.validate(content, workspace)
+      when :puppetfile
+        PuppetLanguageServer::Puppetfile::ValidationProvider.validate(content, workspace)
       else
         []
       end
diff --git a/spec/languageserver/unit/puppet-languageserver/puppetfile/validation_provider_spec.rb b/spec/languageserver/unit/puppet-languageserver/puppetfile/validation_provider_spec.rb
@@ -0,0 +1,162 @@
+require 'spec_helper'
+
+describe 'PuppetLanguageServer::Puppetfile::ValidationProvider' do
+  let(:subject) { PuppetLanguageServer::Puppetfile::ValidationProvider }
+
+  describe "#validate" do
+    context 'with an empty Puppetfile' do
+      let(:content) { '' }
+      it 'should return no validation errors' do
+        result = subject.validate(content, nil)
+
+        expect(result).to eq([])
+      end
+    end
+
+    context 'with a valid Puppetfile' do
+      let(:content) do <<-EOT
+        forge 'https://forge.puppetlabs.com/'
+
+        # Modules from the Puppet Forge
+        mod 'puppetlabs-somemodule',      '1.0.0'
+
+        # Git style modules
+        mod 'gitcommitmodule',
+          :git => 'https://github.com/username/repo',
+          :commit => 'abc123'
+        mod 'gittagmodule',
+          :git => 'https://github.com/username/repo',
+          :tag => '0.1'
+        EOT
+      end
+
+      it 'should return no validation errors' do
+        result = subject.validate(content, nil)
+
+        expect(result).to eq([])
+      end
+    end
+
+    context 'with a syntax error in the Puppetfile' do
+      let(:content) do <<-EOT
+        forge 'https://forge.puppetlabs.com/'
+
+        # Modules from the Puppet Forge
+        mod 'puppetlabs-somemodule',      '1.0.0'
+
+        # Git style modules
+        mod 'gitcommitmodule',
+          :git => 'https://github.com/username/repo',
+          :commit => 'abc123'
+        mod 'gittagmodule',
+          :git => 'https://github.com/username/repo',
+          :tag => '0.1'
+        } # I am a sytnax error
+        EOT
+      end
+
+      it 'should return a validation error' do
+        lint_error = subject.validate(content, nil)[0]
+
+        expect(lint_error['source']).to eq('Puppet')
+        expect(lint_error['message']).to match('syntax error')
+        expect(lint_error['range']).to_not be_nil
+        expect(lint_error['severity']).to eq(LanguageServer::DIAGNOSTICSEVERITY_ERROR)
+      end
+    end
+
+    context 'with a loading error in the Puppetfile' do
+      let(:content) do <<-EOT
+        forge 'https://forge.puppetlabs.com/'
+
+        # Modules from the Puppet Forge
+        mod 'puppetlabs-somemodule',      '1.0.0'
+
+        require 'not_loadable' # I am a load error
+
+        # Git style modules
+        mod 'gitcommitmodule',
+          :git => 'https://github.com/username/repo',
+          :commit => 'abc123'
+        mod 'gittagmodule',
+          :git => 'https://github.com/username/repo',
+          :tag => '0.1'
+        EOT
+      end
+
+      it 'should return a validation error' do
+        lint_error = subject.validate(content, nil)[0]
+
+        expect(lint_error['source']).to eq('Puppet')
+        expect(lint_error['message']).to match('not_loadable')
+        expect(lint_error['range']['start']['line']).to eq(5)
+        expect(lint_error['range']['end']['line']).to eq(5)
+        expect(lint_error['range']).to_not be_nil
+        expect(lint_error['severity']).to eq(LanguageServer::DIAGNOSTICSEVERITY_ERROR)
+      end
+    end
+
+    context 'with a standard error in the Puppetfile' do
+      let(:content) do <<-EOT
+        forge 'https://forge.puppetlabs.com/'
+
+        # Modules from the Puppet Forge
+        mod 'puppetlabs-somemodule',      '1.0.0'
+
+        # Git style modules
+        mod 'gitcommitmodule',
+          :git => 'https://github.com/username/repo',
+          :commit => 'abc123'
+
+        raise 'A Mock Runtime Error'
+
+        mod 'gittagmodule',
+          :git => 'https://github.com/username/repo',
+          :tag => '0.1'
+        EOT
+      end
+
+      it 'should return a validation error' do
+        lint_error = subject.validate(content, nil)
+        expect(lint_error.count).to eq(1)
+        lint_error = lint_error[0]
+
+        expect(lint_error['source']).to eq('Puppet')
+        expect(lint_error['message']).to match('A Mock Runtime Error')
+        expect(lint_error['range']['start']['line']).to eq(10)
+        expect(lint_error['range']['end']['line']).to eq(10)
+        expect(lint_error['range']).to_not be_nil
+        expect(lint_error['severity']).to eq(LanguageServer::DIAGNOSTICSEVERITY_ERROR)
+      end
+    end
+
+
+    context 'with an unknown method in the Puppetfile' do
+      let(:content) do <<-EOT
+        forge 'https://forge.puppetlabs.com/'
+
+        # Modules from the Puppet Forge
+        mod 'puppetlabs-somemodule',      '1.0.0'
+
+        # Git style modules
+        mod 'gitcommitmodule',
+          :git => 'https://github.com/username/repo',
+          :commit => 'abc123'
+        mod_BROKEN 'gittagmodule',
+          :git => 'https://github.com/username/repo',
+          :tag => '0.1'
+        EOT
+      end
+
+      it 'should return a validation error on the specified line' do
+        lint_error = subject.validate(content, nil)[0]
+
+        expect(lint_error['source']).to eq('Puppet')
+        expect(lint_error['message']).to match('mod_BROKEN')
+        expect(lint_error['range']['start']['line']).to eq(9)
+        expect(lint_error['range']['end']['line']).to eq(9)
+        expect(lint_error['severity']).to eq(LanguageServer::DIAGNOSTICSEVERITY_ERROR)
+      end
+    end
+  end
+end
diff --git a/spec/languageserver/unit/puppet-languageserver/validation_queue_spec.rb b/spec/languageserver/unit/puppet-languageserver/validation_queue_spec.rb
@@ -30,6 +30,7 @@
 
       allow(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).and_raise("PuppetLanguageServer::Manifest::ValidationProvider.validate mock should not be called")
       allow(PuppetLanguageServer::Epp::ValidationProvider).to receive(:validate).and_raise("PuppetLanguageServer::Epp::ValidationProvider.validate mock should not be called")
+      allow(PuppetLanguageServer::Puppetfile::ValidationProvider).to receive(:validate).and_raise("PuppetLanguageServer::Puppetfile::ValidationProvider.validate mock should not be called")
     end
 
     context 'for an invalid or missing documents' do
@@ -56,24 +57,28 @@
 
       it 'should only return the most recent validation results' do
         # Configure the document store
-        subject.documents.set_document(MANIFEST_FILENAME, file_content0, document_version + 0)
-        subject.documents.set_document(MANIFEST_FILENAME, file_content1, document_version + 1)
-        subject.documents.set_document(MANIFEST_FILENAME, file_content3, document_version + 3)
-        subject.documents.set_document(EPP_FILENAME,      file_content1, document_version + 1)
+        subject.documents.set_document(MANIFEST_FILENAME,   file_content0, document_version + 0)
+        subject.documents.set_document(MANIFEST_FILENAME,   file_content1, document_version + 1)
+        subject.documents.set_document(MANIFEST_FILENAME,   file_content3, document_version + 3)
+        subject.documents.set_document(EPP_FILENAME,        file_content1, document_version + 1)
+        subject.documents.set_document(PUPPETFILE_FILENAME, file_content1, document_version + 1)
 
         # Preconfigure the validation queue
         subject.reset_queue([
-          { 'file_uri' => MANIFEST_FILENAME, 'doc_version' => document_version + 0, 'document_type' => :manifest, 'workspace' => workspace, 'connection_object' => connection },
-          { 'file_uri' => MANIFEST_FILENAME, 'doc_version' => document_version + 1, 'document_type' => :manifest, 'workspace' => workspace, 'connection_object' => connection },
-          { 'file_uri' => MANIFEST_FILENAME, 'doc_version' => document_version + 3, 'document_type' => :manifest, 'workspace' => workspace, 'connection_object' => connection },
-          { 'file_uri' => EPP_FILENAME,      'doc_version' => document_version + 1, 'document_type' => :epp,      'workspace' => workspace, 'connection_object' => connection },
+          { 'file_uri' => MANIFEST_FILENAME,   'doc_version' => document_version + 0, 'document_type' => :manifest,   'workspace' => workspace, 'connection_object' => connection },
+          { 'file_uri' => MANIFEST_FILENAME,   'doc_version' => document_version + 1, 'document_type' => :manifest,   'workspace' => workspace, 'connection_object' => connection },
+          { 'file_uri' => MANIFEST_FILENAME,   'doc_version' => document_version + 3, 'document_type' => :manifest,   'workspace' => workspace, 'connection_object' => connection },
+          { 'file_uri' => EPP_FILENAME,        'doc_version' => document_version + 1, 'document_type' => :epp,        'workspace' => workspace, 'connection_object' => connection },
+          { 'file_uri' => PUPPETFILE_FILENAME, 'doc_version' => document_version + 1, 'document_type' => :puppetfile, 'workspace' => workspace, 'connection_object' => connection },
         ])
 
         # We only expect the following results to be returned
         expect(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).with(file_content2, workspace).and_return(validation_result)
         expect(PuppetLanguageServer::Epp::ValidationProvider).to receive(:validate).with(file_content1, workspace).and_return(validation_result)
+        expect(PuppetLanguageServer::Puppetfile::ValidationProvider).to receive(:validate).with(file_content1, workspace).and_return(validation_result)
         expect(connection).to receive(:reply_diagnostics).with(MANIFEST_FILENAME, validation_result)
         expect(connection).to receive(:reply_diagnostics).with(EPP_FILENAME, validation_result)
+        expect(connection).to receive(:reply_diagnostics).with(PUPPETFILE_FILENAME, validation_result)
 
         # Simulate a new document begin added by adding it to the document store and
         # enqueue validation for a version that it's in the middle of the versions in the queue
@@ -96,7 +101,11 @@
       end
 
       context 'of a Puppetfile file' do
-        validation_result = []
+        validation_result = [{ 'result' => 'MockResult' }]
+
+        before(:each) do
+          expect(PuppetLanguageServer::Puppetfile::ValidationProvider).to receive(:validate).with(FILE_CONTENT, workspace).and_return(validation_result)
+        end
 
         it_should_behave_like "single document which sends validation results", PUPPETFILE_FILENAME, FILE_CONTENT, validation_result
       end
@@ -134,6 +143,7 @@
 
       allow(PuppetLanguageServer::Manifest::ValidationProvider).to receive(:validate).and_raise("PuppetLanguageServer::Manifest::ValidationProvider.validate mock should not be called")
       allow(PuppetLanguageServer::Epp::ValidationProvider).to receive(:validate).and_raise("PuppetLanguageServer::Epp::ValidationProvider.validate mock should not be called")
+      allow(PuppetLanguageServer::Puppetfile::ValidationProvider).to receive(:validate).and_raise("PuppetLanguageServer::Puppetfile::ValidationProvider.validate mock should not be called")
     end
 
     it 'should not send validation results for documents that do not exist' do
@@ -153,7 +163,11 @@
     end
 
     context 'for a Puppetfile file' do
-      validation_result = []
+      validation_result = [{ 'result' => 'MockResult' }]
+
+      before(:each) do
+        expect(PuppetLanguageServer::Puppetfile::ValidationProvider).to receive(:validate).with(FILE_CONTENT, workspace).and_return(validation_result)
+      end
 
       it_should_behave_like "document which sends validation results", PUPPETFILE_FILENAME, FILE_CONTENT, validation_result
     end
