From 595d6f784899753893b6cfe61227b3fdb4b5648e Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Fri, 8 Feb 2013 16:54:24 -0800
Subject: [PATCH] Record invalid key accesses as an error on config

---
 lib/vagrant/config/v2/loader.rb          | 12 ++++++++-
 lib/vagrant/config/v2/root.rb            | 33 +++++++++++++++++++-----
 templates/locales/en.yml                 |  3 +++
 test/unit/vagrant/config/v2/root_test.rb | 12 ++++++---
 4 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/lib/vagrant/config/v2/loader.rb b/lib/vagrant/config/v2/loader.rb
index 11f66caf7..d7f0a8e3d 100644
--- a/lib/vagrant/config/v2/loader.rb
+++ b/lib/vagrant/config/v2/loader.rb
@@ -77,8 +77,18 @@ module Vagrant
             end
           end
 
+          # Merge the missing keys
+          new_missing_key_calls =
+            old_state["missing_key_calls"] + new_state["missing_key_calls"]
+
           # Return the final root object
-          V2::Root.new(config_map, keys)
+          V2::Root.new(config_map).tap do |result|
+            result.__set_internal_state({
+              "config_map"        => config_map,
+              "keys"              => keys,
+              "missing_key_calls" => new_missing_key_calls
+            })
+          end
         end
 
         # Upgrade a V1 configuration to a V2 configuration. We do this by
diff --git a/lib/vagrant/config/v2/root.rb b/lib/vagrant/config/v2/root.rb
index 419319c29..5204ea462 100644
--- a/lib/vagrant/config/v2/root.rb
+++ b/lib/vagrant/config/v2/root.rb
@@ -1,3 +1,6 @@
+require "ostruct"
+require "set"
+
 require "vagrant/config/v2/util"
 
 module Vagrant
@@ -11,8 +14,9 @@ module Vagrant
         #
         # @param [Hash] config_map Map of key to config class.
         def initialize(config_map, keys=nil)
-          @keys       = keys || {}
-          @config_map = config_map
+          @keys              = keys || {}
+          @config_map        = config_map
+          @missing_key_calls = Set.new
         end
 
         # We use method_missing as a way to get the configuration that is
@@ -27,8 +31,9 @@ module Vagrant
             @keys[name] = config_klass.new
             return @keys[name]
           else
-            # Super it up to probably raise a NoMethodError
-            super
+            # Record access to a missing key as an error
+            @missing_key_calls.add(name.to_s)
+            return OpenStruct.new
           end
         end
 
@@ -66,6 +71,13 @@ module Vagrant
             errors.delete(key) if errors[key].empty?
           end
 
+          # If we have missing keys, record those as errors
+          if !@missing_key_calls.empty?
+            errors["Vagrant"] = @missing_key_calls.to_a.sort.map do |key|
+              I18n.t("vagrant.config.root.bad_key", :key => key)
+            end
+          end
+
           errors
         end
 
@@ -75,10 +87,19 @@ module Vagrant
         # clashes with potential configuration keys.
         def __internal_state
           {
-            "config_map" => @config_map,
-            "keys"       => @keys
+            "config_map"        => @config_map,
+            "keys"              => @keys,
+            "missing_key_calls" => @missing_key_calls
           }
         end
+
+        # This sets the internal state. This is used by the core to do some
+        # merging logic and shouldn't be used by the general public.
+        def __set_internal_state(state)
+          @config_map        = state["config_map"] if state.has_key?("config_map")
+          @keys              = state["keys"] if state.has_key?("keys")
+          @missing_key_calls = state["missing_key_calls"] if state.has_key?("missing_key_calls")
+        end
       end
     end
   end
diff --git a/templates/locales/en.yml b/templates/locales/en.yml
index 6267f8753..1950215b2 100644
--- a/templates/locales/en.yml
+++ b/templates/locales/en.yml
@@ -396,6 +396,9 @@ en:
         run_list_empty: "Run list must not be empty."
         server_url_empty: "Chef server URL must be populated."
         validation_key_path: "Validation key path must be valid path to your chef server validation key."
+      root:
+        bad_key: |-
+          Unknown configuration section '%{key}'.
       ssh:
         private_key_missing: "`private_key_path` file must exist: %{path}"
       vm:
diff --git a/test/unit/vagrant/config/v2/root_test.rb b/test/unit/vagrant/config/v2/root_test.rb
index a705153ba..50a6c775a 100644
--- a/test/unit/vagrant/config/v2/root_test.rb
+++ b/test/unit/vagrant/config/v2/root_test.rb
@@ -1,3 +1,5 @@
+require "set"
+
 require File.expand_path("../../../../base", __FILE__)
 
 describe Vagrant::Config::V2::Root do
@@ -13,9 +15,10 @@ describe Vagrant::Config::V2::Root do
     instance.foo.should eql(foo)
   end
 
-  it "should raise a proper NoMethodError if a config key doesn't exist" do
+  it "record a missing key call if invalid key used" do
     instance = described_class.new({})
-    expect { instance.foo }.to raise_error(NoMethodError)
+    expect { instance.foo }.to_not raise_error
+    instance.__internal_state["missing_key_calls"].include?("foo").should be
   end
 
   it "can be created with initial state" do
@@ -27,8 +30,9 @@ describe Vagrant::Config::V2::Root do
     map      = { "foo" => Object, "bar" => Object }
     instance = described_class.new(map)
     instance.__internal_state.should == {
-      "config_map" => map,
-      "keys"       => {}
+      "config_map"        => map,
+      "keys"              => {},
+      "missing_key_calls" => Set.new
     }
   end