Configuration classes can now be validated. Nice error message shown …

…in case of failure.

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 0.6.0 (unreleased)
 
+  - Configuration is now validated so improper input can be found in
+    Vagrantfiles.
   - Fixed issue with not detecting Vagrantfile at root directory ("/").
   - Vagrant now gives a nice error message if there is a syntax error
     in any Vagrantfile. [GH-154]

lib/vagrant/config.rb

@@ -36,6 +36,7 @@ def execute!(config_object=nil)
         config_object ||= config
 
         run_procs!(config_object)
+        config_object.validate!
         config_object
       end
     end
@@ -91,6 +92,23 @@ def initialize(env=nil)
 
         @env = env
       end
+
+      # Validates the configuration classes of this instance and raises an
+      # exception if they are invalid.
+      def validate!
+        # Validate each of the configured classes and store the results into
+        # a hash.
+        errors = self.class.configures_list.inject({}) do |container, data|
+          key, _ = data
+          recorder = ErrorRecorder.new
+          send(key.to_sym).validate(recorder)
+          container[key.to_sym] = recorder if !recorder.errors.empty?
+          container
+        end
+
+        return if errors.empty?
+        raise Errors::ConfigValidationFailed.new(:messages => Util::TemplateRenderer.render("config/validation_failed", :errors => errors))
+      end
     end
   end
 end

lib/vagrant/config/base.rb

@@ -18,7 +18,16 @@ def self.json_create(data)
         end
       end
 
-      # Converts the configuration to a raw hash.
+      # Called by {Top} after the configuration is loaded to validate
+      # the configuaration objects. Subclasses should implement this
+      # method and add any errors to the `errors` object given.
+      #
+      # @param [ErrorRecorder] errors
+      def validate(errors); end
+
+      # Converts the configuration to a raw hash by calling `#to_hash`
+      # on all instance variables (if it can) and putting them into
+      # a hash.
       def to_hash
         instance_variables_hash.inject({}) do |acc, data|
           k,v = data
@@ -28,11 +37,15 @@ def to_hash
         end
       end
 
+      # Converts to JSON, with the `json_class` field set so that when
+      # the JSON is parsed back, it can be loaded back into the proper class.
+      # See {json_create}.
       def to_json(*a)
         result = { 'json_class' => self.class.name }
         result.merge(instance_variables_hash).to_json(*a)
       end
 
+      # Returns the instance variables as a hash of key-value pairs.
       def instance_variables_hash
         instance_variables.inject({}) do |acc, iv|
           acc[iv.to_s[1..-1]] = instance_variable_get(iv) unless iv.to_sym == :@env

lib/vagrant/config/error_recorder.rb

@@ -0,0 +1,21 @@
+module Vagrant
+  class Config
+    # A class which is passed into the various {Base#validate} methods and
+    # can be used as a helper to add error messages about a single config
+    # class.
+    class ErrorRecorder
+      attr_reader :errors
+
+      def initialize
+        @errors = []
+      end
+
+      # Adds an error to the list of errors. The message key must be a key
+      # to an I18n translatable error message. Opts can be specified as
+      # interpolation variables for the message.
+      def add(message_key, opts=nil)
+        @errors << I18n.t(message_key, opts)
+      end
+    end
+  end
+end

lib/vagrant/errors.rb

@@ -94,6 +94,11 @@ class CLIMissingEnvironment < VagrantError
       error_key(:cli_missing_env)
     end
 
+    class ConfigValidationFailed < VagrantError
+      status_code(42)
+      error_key(:config_validation)
+    end
+
     class DownloaderFileDoesntExist < VagrantError
       status_code(37)
       error_key(:file_missing, "vagrant.downloaders.file")

templates/config/validation_failed.erb

@@ -0,0 +1,7 @@
+<% errors.each do |key, container| -%>
+<%= key %>:
+<% container.errors.each do |error| -%>
+* <%= error %>
+<% end -%>
+
+<% end -%>

templates/locales/en.yml

@@ -8,6 +8,11 @@ en:
       base_vm_not_found: The base VM with the name '%{name}' was not found.
       box_not_found: Box '%{name}' could not be found.
       cli_missing_env: This command requires that a Vagrant environment be properly passed in as the last parameter.
+      config_validation: |-
+        There was a problem with the configuration of Vagrant. The error messages
+        or messages are printed below:
+
+        %{messages}
       interrupted: Vagrant exited after cleanup due to external interrupt.
       multi_vm_required: A multi-vm environment is required for name specification to this command.
       multi_vm_target_required: `vagrant %{command}` requires a specific VM name to target in a multi-VM environment.
@@ -77,6 +82,13 @@ en:
       vm_creation_required: VM must be created before running this command. Run `vagrant up` first.
       vm_not_found: A VM by the name of %{name} was not found.
 
+#-------------------------------------------------------------------------------
+# Translations for config validation errors
+#-------------------------------------------------------------------------------
+    config:
+      base:
+        foo: UH OH FOO!
+
 #-------------------------------------------------------------------------------
 # Translations for commands. e.g. `vagrant x`
 #-------------------------------------------------------------------------------

test/vagrant/config/error_recorder_test.rb

@@ -0,0 +1,24 @@
+require "test_helper"
+
+class ConfigErrorsTest < Test::Unit::TestCase
+  setup do
+    @klass = Vagrant::Config::ErrorRecorder
+    @instance = @klass.new
+  end
+
+  should "not have any errors to start" do
+    assert @instance.errors.empty?
+  end
+
+  should "add errors" do
+    key = "vagrant.test.errors.test_key"
+    @instance.add(key)
+    assert_equal I18n.t(key), @instance.errors.first
+  end
+
+  should "interpolate error messages if options given" do
+    key = "vagrant.test.errors.test_key_with_interpolation"
+    @instance.add(key, :key => "hey")
+    assert_equal I18n.t(key, :key => "hey"), @instance.errors.first
+  end
+end

test/vagrant/config_test.rb

@@ -120,7 +120,9 @@ class ConfigTest < Test::Unit::TestCase
     end
 
     should "run the proc stack with the config when execute is called" do
-      @klass.expects(:run_procs!).with(@klass.config).once
+      seq = sequence('seq')
+      @klass.expects(:run_procs!).with(@klass.config).once.in_sequence(seq)
+      @klass.config.expects(:validate!).once.in_sequence(seq)
       @klass.execute!
     end
 
@@ -188,5 +190,29 @@ class ConfigTest < Test::Unit::TestCase
         assert_equal instance, config.send(key)
       end
     end
+
+    context "validation" do
+      should "do nothing if no errors are added" do
+        valid_class = Class.new(@klass::Base)
+        @klass::Top.configures(:subconfig, valid_class)
+        instance = @klass::Top.new
+        assert_nothing_raised { instance.validate! }
+      end
+
+      should "raise an exception if there are errors" do
+        invalid_class = Class.new(@klass::Base) do
+          def validate(errors)
+            errors.add("vagrant.test.errors.test_key")
+          end
+        end
+
+        @klass::Top.configures(:subconfig, invalid_class)
+        instance = @klass::Top.new
+
+        assert_raises(Vagrant::Errors::ConfigValidationFailed) {
+          instance.validate!
+        }
+      end
+    end
   end
 end