(GH-28) Add basic Puppetfile validation

Previously basic validation for Puppetfile's was added.  This commit;

* Adds more detailed parsing of Puppetfile to extract different types of modules
  git, local, svn or forge.  These types have enough information to identify a
  module type, and basic name/title/version validation
* Adds an internal only module type called invalid which is used to track
  invalid module definitions.  Without this, the validation process would find
  the first invalid module and ignore subsequent entries.
* Due to semantic_puppet not being available in all puppet editions, the version
  regexes are vendored into the code instead of calling out to Semantic Puppet.
* Adds detection of invalid modules and duplicate module definitions
* Adds tests for these scenarios

Author: glennsarti

lib/puppet-languageserver/providers.rb

@@ -4,6 +4,12 @@
   manifest/definition_provider
   manifest/validation_provider
   manifest/hover_provider
+  puppetfile/r10k/module/base
+  puppetfile/r10k/module/forge
+  puppetfile/r10k/module/invalid
+  puppetfile/r10k/module/local
+  puppetfile/r10k/module/git
+  puppetfile/r10k/module/svn
   puppetfile/r10k/puppetfile
   puppetfile/validation_provider
 ].each do |lib|

lib/puppet-languageserver/puppetfile/r10k/module/base.rb

@@ -0,0 +1,63 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      module Module
+        def self.from_puppetfile(title, args)
+          return Git.new(title, args) if Git.implements?(title, args)
+          return Svn.new(title, args) if Svn.implements?(title, args)
+          return Local.new(title, args) if Local.implements?(title, args)
+          return Forge.new(title, args) if Forge.implements?(title, args)
+
+          Invalid.new(title, args)
+        end
+
+        class Base
+          # The full title of the module
+          attr_reader :title
+
+          # The name of the module
+          attr_reader :name
+
+          # The line number where this module is first found in Puppetfile
+          attr_reader :puppetfile_line_number
+
+          def initialize(title, args)
+            @title = title
+            @args = args
+            @owner, @name = parse_title(@title)
+
+            @puppetfile_line_number = find_load_location
+          end
+
+          # Should be overridden in concrete module classes
+          def version
+            nil
+          end
+
+          # Should be overridden in concrete module classes
+          def properties
+            {}
+          end
+
+          private
+
+          def parse_title(title)
+            if (match = title.match(/\A(\w+)\Z/))
+              [nil, match[1]]
+            elsif (match = title.match(/\A(\w+)[-\/](\w+)\Z/))
+              [match[1], match[2]]
+            else
+              raise ArgumentError, format("Module name (%<title>s) must match either 'modulename' or 'owner/modulename'", title: title)
+            end
+          end
+
+          def find_load_location
+            loc = Kernel.caller_locations
+                        .find { |call_loc| call_loc.absolute_path == PuppetLanguageServer::Puppetfile::R10K::PUPPETFILE_MONIKER }
+            loc.nil? ? 0 : loc.lineno - 1 # Line numbers from ruby are base 1
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet-languageserver/puppetfile/r10k/module/forge.rb

@@ -0,0 +1,42 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      module Module
+        class Forge < PuppetLanguageServer::Puppetfile::R10K::Module::Base
+          def self.implements?(name, args)
+            !name.match(/\A(\w+)[-\/](\w+)\Z/).nil? && valid_version?(args)
+          end
+
+          def self.valid_version?(value)
+            return false unless value.is_a?(String) || value.is_a?(Symbol)
+            value == :latest || value.nil? || valid_version_string?(value)
+          end
+
+          def properties
+            {
+              :type => :forge
+            }
+          end
+
+          # Version string matching regexes
+          # From Semantic Puppet gem
+          REGEX_NUMERIC = '(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)'.freeze # Major . Minor . Patch
+          REGEX_PRE     = '(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?'.freeze # Prerelease
+          REGEX_BUILD   = '(?:\+([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?'.freeze # Build
+          REGEX_FULL    = REGEX_NUMERIC + REGEX_PRE + REGEX_BUILD.freeze
+          REGEX_FULL_RX = /\A#{REGEX_FULL}\Z/
+
+          def self.valid_version_string?(value)
+            match = value.match(REGEX_FULL_RX)
+            if match.nil?
+              false
+            else
+              prerelease = match[4]
+              prerelease.nil? || prerelease.split('.').all? { |x| x !~ /^0\d+$/ }
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet-languageserver/puppetfile/r10k/module/git.rb

@@ -0,0 +1,21 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      module Module
+        class Git < PuppetLanguageServer::Puppetfile::R10K::Module::Base
+          def self.implements?(_name, args)
+            args.is_a?(Hash) && args.key?(:git)
+          rescue StandardError
+            false
+          end
+
+          def properties
+            {
+              :type => :git
+            }
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet-languageserver/puppetfile/r10k/module/invalid.rb

@@ -0,0 +1,27 @@
+# This is a special module definition.  It's the catchall when no other module type can handle it
+
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      module Module
+        class Invalid < PuppetLanguageServer::Puppetfile::R10K::Module::Base
+          def self.implements?(_name, _args)
+            true
+          end
+
+          def initialize(title, args)
+            super
+            @error_message = format("Module %<title>s with args %<args>s doesn't have an implementation. (Are you using the right arguments?)", title: title, args: args.inspect)
+          end
+
+          def properties
+            {
+              :type => :invalid,
+              :error_message => @error_message
+            }
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet-languageserver/puppetfile/r10k/module/local.rb

@@ -0,0 +1,21 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      module Module
+        class Local < PuppetLanguageServer::Puppetfile::R10K::Module::Base
+          def self.implements?(_name, args)
+            args.is_a?(Hash) && args.key?(:local)
+          rescue StandardError
+            false
+          end
+
+          def properties
+            {
+              :type => :local
+            }
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet-languageserver/puppetfile/r10k/module/svn.rb

@@ -0,0 +1,21 @@
+module PuppetLanguageServer
+  module Puppetfile
+    module R10K
+      module Module
+        class Svn < PuppetLanguageServer::Puppetfile::R10K::Module::Base
+          def self.implements?(_name, args)
+            args.is_a?(Hash) && args.key?(:svn)
+          rescue StandardError
+            false
+          end
+
+          def properties
+            {
+              :type => :svn
+            }
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet-languageserver/puppetfile/r10k/puppetfile.rb

@@ -4,19 +4,27 @@ module R10K
       PUPPETFILE_MONIKER ||= 'Puppetfile'.freeze
 
       class Puppetfile
+        attr_reader :modules
+
         def load!(puppetfile_contents)
           puppetfile = DSL.new(self)
+          @modules = []
           puppetfile.instance_eval(puppetfile_contents, PUPPETFILE_MONIKER)
         end
 
+        def add_module(name, args)
+          @modules << Module.from_puppetfile(name, args)
+        end
+
         class DSL
           def initialize(parent)
             @parent = parent
           end
 
           # @param [String] name
           # @param [*Object] args
-          def mod(_name, _args = nil)
+          def mod(name, args = nil)
+            @parent.add_module(name, args)
           end
 
           # @param [String] forge
@@ -27,7 +35,7 @@ def forge(_location)
           def moduledir(_location)
           end
 
-          def method_missing(method, *_args) # rubocop:disable Style/MethodMissing
+          def method_missing(method, *_args) # rubocop:disable Style/MethodMissingSuper, Style/MissingRespondToMissing
             raise NoMethodError, format("Unknown method '%<method>s'", method: method)
           end
         end

lib/puppet-languageserver/puppetfile/validation_provider.rb

@@ -36,6 +36,37 @@ def self.validate(content, _workspace, _max_problems = 100)
         end
         return result if puppetfile.nil?
 
+        # Check for invalid module definitions
+        puppetfile.modules.each do |mod|
+          next unless mod.properties[:type] == :invalid
+          # Note - Ruby doesn't give a character position so just highlight the entire line
+          result << LanguageServer::Diagnostic.create('severity' => LanguageServer::DIAGNOSTICSEVERITY_ERROR,
+                                                      'fromline' => mod.puppetfile_line_number,
+                                                      'toline'   => mod.puppetfile_line_number,
+                                                      'fromchar' => 0,
+                                                      'tochar'   => max_line_length,
+                                                      'source'   => 'Puppet',
+                                                      'message'  => mod.properties[:error_message])
+        end
+
+        # Check for duplicate module definitions
+        dupes = puppetfile.modules
+                          .group_by { |mod| mod.name }
+                          .select { |_, v| v.size > 1 }
+                          .map(&:first)
+        dupes.each do |dupe_module_name|
+          puppetfile.modules.select { |mod| mod.name == dupe_module_name }.each do |puppet_module|
+            # Note - Ruby doesn't give a character position so just highlight the entire line
+            result << LanguageServer::Diagnostic.create('severity' => LanguageServer::DIAGNOSTICSEVERITY_ERROR,
+                                                        'fromline' => puppet_module.puppetfile_line_number,
+                                                        'toline'   => puppet_module.puppetfile_line_number,
+                                                        'fromchar' => 0,
+                                                        'tochar'   => max_line_length,
+                                                        'source'   => 'Puppet',
+                                                        'message'  => "Duplicate module definition for '#{puppet_module.name}'")
+          end
+        end
+
         result
       end
     end