Introduce the notion of settings validation

5 files changed, 118 insertions, 22 deletions

diff --git a/lib/bundler/cli/doctor.rb b/lib/bundler/cli/doctor.rb
index ae27983240..7f28a5eb13 100644
--- a/lib/bundler/cli/doctor.rb
+++ b/lib/bundler/cli/doctor.rb
@@ -62,6 +62,7 @@ module Bundler
 
     def run
       Bundler.ui.level = "error" if options[:quiet]
+      Bundler.settings.validate!
       check!
 
       definition = Bundler.definition
diff --git a/lib/bundler/cli/install.rb b/lib/bundler/cli/install.rb
index cc41cce7a6..fed2cf0b22 100644
--- a/lib/bundler/cli/install.rb
+++ b/lib/bundler/cli/install.rb
@@ -154,8 +154,8 @@ module Bundler
 
       check_for_group_conflicts_in_cli_options
 
-      Bundler.settings.set_command_option_if_given :with, options[:with]
-      Bundler.settings.set_command_option_if_given :without, options[:without]
+      Bundler.settings.set_command_option :with, nil if options[:with] == []
+      Bundler.settings.set_command_option :without, nil if options[:without] == []
 
       with = options.fetch(:with, [])
       with |= Bundler.settings[:with].map(&:to_s)
@@ -189,8 +189,13 @@ module Bundler
 
       Bundler.settings.set_command_option_if_given :clean, options["clean"]
 
-      Bundler.settings.set_command_option :without, options[:without] unless Bundler.settings[:without] == options[:without]
-      Bundler.settings.set_command_option :with,    options[:with]    unless Bundler.settings[:with]    == options[:with]
+      unless Bundler.settings[:without] == options[:without] && Bundler.settings[:with] == options[:with]
+        # need to nil them out first to get around validation for backwards compatibility
+        Bundler.settings.set_command_option :without, nil
+        Bundler.settings.set_command_option :with,    nil
+        Bundler.settings.set_command_option :without, options[:without] - options[:with]
+        Bundler.settings.set_command_option :with,    options[:with]
+      end
 
       disable_shared_gems = Bundler.settings[:path] ? true : nil
       Bundler.settings.set_command_option :disable_shared_gems, disable_shared_gems unless Bundler.settings[:disable_shared_gems] == disable_shared_gems
diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index fa1483f804..9cc80a0f99 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -6,6 +6,7 @@ module Bundler
   class Settings
     autoload :Mirror,  "bundler/mirror"
     autoload :Mirrors, "bundler/mirror"
+    autoload :Validator, "bundler/settings/validator"
 
     BOOL_KEYS = %w[
       allow_bundler_dependency_conflicts
@@ -230,6 +231,15 @@ module Bundler
       @app_cache_path ||= self[:cache_path] || "vendor/cache"
     end
 
+    def validate!
+      all.each do |raw_key|
+        [@local_config, ENV, @global_config].each do |settings|
+          value = converted_value(settings[key_for(raw_key)], raw_key)
+          Validator.validate(raw_key, value, settings.to_h.dup)
+        end
+      end
+    end
+
   private
 
     def key_for(key)
@@ -282,24 +292,25 @@ module Bundler
       array.join(":").tr(" ", ":")
     end
 
-    def set_key(key, value, hash, file)
-      value = array_to_s(value) if is_array(key)
+    def set_key(raw_key, value, hash, file)
+      raw_key = raw_key.to_s
+      value = array_to_s(value) if is_array(raw_key)
 
-      key = key_for(key)
+      key = key_for(raw_key)
 
-      unless hash[key] == value
-        hash[key] = value
-        hash.delete(key) if value.nil?
-        if file
-          SharedHelpers.filesystem_access(file) do |p|
-            FileUtils.mkdir_p(p.dirname)
-            require "bundler/yaml_serializer"
-            p.open("w") {|f| f.write(YAMLSerializer.dump(hash)) }
-          end
-        end
-      end
+      return if hash[key] == value
+
+      hash[key] = value
+      hash.delete(key) if value.nil?
+
+      Validator.validate(raw_key, converted_value(value, raw_key), hash)
 
-      value
+      return unless file
+      SharedHelpers.filesystem_access(file) do |p|
+        FileUtils.mkdir_p(p.dirname)
+        require "bundler/yaml_serializer"
+        p.open("w") {|f| f.write(YAMLSerializer.dump(hash)) }
+      end
     end
 
     def converted_value(value, key)
diff --git a/lib/bundler/settings/validator.rb b/lib/bundler/settings/validator.rb
new file mode 100644
index 0000000000..263e6d9325
--- /dev/null
+++ b/lib/bundler/settings/validator.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+module Bundler
+  class Settings
+    class Validator
+      class Rule
+        attr_reader :description
+
+        def initialize(keys, description, &validate)
+          @keys = keys
+          @description = description
+          @validate = validate
+        end
+
+        def validate(key, value, settings)
+          instance_exec(key, value, settings, &@validate)
+        end
+
+        def fail!(key, value, *reasons)
+          reasons.unshift @description
+          raise InvalidOption, "Setting `#{key}` to #{value.inspect} failed:\n#{reasons.map {|r| " - #{r}" }.join("\n")}"
+        end
+
+        def set(settings, key, value, *reasons)
+          hash_key = k(key)
+          return if settings[hash_key] == value
+          reasons.unshift @description
+          Bundler.ui.info "Setting `#{key}` to #{value.inspect}, since #{reasons.join(", ")}"
+          if value.nil?
+            settings.delete(hash_key)
+          else
+            settings[hash_key] = value
+          end
+        end
+
+        def k(key)
+          Bundler.settings.send(:key_for, key)
+        end
+      end
+
+      def self.rules
+        @rules ||= Hash.new {|h, k| h[k] = [] }
+      end
+      private_class_method :rules
+
+      def self.rule(keys, description, &blk)
+        rule = Rule.new(keys, description, &blk)
+        keys.each {|k| rules[k] << rule }
+      end
+      private_class_method :rule
+
+      def self.validate(key, value, settings)
+        rules_to_validate = rules[key]
+        rules_to_validate.each {|rule| rule.validate(key, value, settings) }
+      end
+
+      rule %w[path disable_shared_gems], "path and disable_shared_gems are mutually exclusive" do |key, value, settings|
+        if key == "path" && value
+          set(settings, :disabled_shared_gems, nil)
+        elsif key == "disable_shared_gems" && value
+          set(settings, :path, nil)
+        end
+      end
+
+      rule %w[with without], "a group cannot be in both `with` & `without` simultaneously" do |key, value, settings|
+        with = settings.fetch(k(:with), "").split(":").map(&:to_sym)
+        without = settings.fetch(k(:without), "").split(":").map(&:to_sym)
+
+        other_key = key == "with" ? :without : :with
+        other_setting = key == "with" ? without : with
+
+        conflicting = with & without
+        if conflicting.any?
+          fail!(key, value, "`#{other_key}` is current set to #{other_setting}", "the `#{conflicting.join("`, `")}` groups conflict")
+        end
+      end
+    end
+  end
+end
diff --git a/spec/install/gemfile/groups_spec.rb b/spec/install/gemfile/groups_spec.rb
index dc55f1d8d3..19c379e188 100644
--- a/spec/install/gemfile/groups_spec.rb
+++ b/spec/install/gemfile/groups_spec.rb
@@ -192,7 +192,7 @@ RSpec.describe "bundle install with groups" do
         expect(the_bundle).not_to include_gems "thin 1.0"
       end
 
-      it "does remove groups from without when passed at with" do
+      it "does remove groups from without when passed at --with", :bundler => "< 2" do
         bundle :install, forgotten_command_line_options(:without => "emo")
         bundle :install, forgotten_command_line_options(:with => "emo")
         expect(the_bundle).to include_gems "activesupport 2.3.5"
@@ -211,12 +211,12 @@ RSpec.describe "bundle install with groups" do
       end
 
       it "allows the BUNDLE_WITH setting to override BUNDLE_WITHOUT" do
-        bundle! "config --local with debugging"
+        ENV["BUNDLE_WITH"] = "debugging"
 
         bundle! :install
         expect(the_bundle).to include_gem "thin 1.0"
 
-        bundle! "config --local without debugging"
+        ENV["BUNDLE_WITHOUT"] = "debugging"
         expect(the_bundle).to include_gem "thin 1.0"
 
         bundle! :install