Few correction from suggestions

9 files changed, 68 insertions, 32 deletions

diff --git a/lib/bundler/lockfile_parser.rb b/lib/bundler/lockfile_parser.rb
index bbbed381..af01976f 100644
--- a/lib/bundler/lockfile_parser.rb
+++ b/lib/bundler/lockfile_parser.rb
@@ -22,7 +22,7 @@ module Bundler
     GIT          = "GIT".freeze
     GEM          = "GEM".freeze
     PATH         = "PATH".freeze
-    PLUGIN       = "PLUGIN".freeze
+    PLUGIN       = "PLUGIN SOURCE".freeze
     SPECS        = "  specs:".freeze
     OPTIONS      = /^  ([a-z]+): (.*)$/i
     SOURCE       = [GIT, GEM, PATH, PLUGIN].freeze
@@ -128,7 +128,7 @@ module Bundler
 
     def parse_source(line)
       case line
-      when GIT, GEM, PATH, PLUGIN
+      when *SOURCE
         @current_source = nil
         @opts = {}
         @type = line
diff --git a/lib/bundler/plugin.rb b/lib/bundler/plugin.rb
index 98a03362..425f6460 100644
--- a/lib/bundler/plugin.rb
+++ b/lib/bundler/plugin.rb
@@ -128,11 +128,11 @@ module Bundler
     #
     # @param [Hash] plugins mapped to their installtion path
     # @param [Array<String>] names of auto added source plugins that can be ignored
-    def save_plugins(plugins, optionals = [])
+    def save_plugins(plugins, optional_plugins = [])
       plugins.each do |name, path|
         path = Pathname.new path
         validate_plugin! path
-        register_plugin name, path, optionals.include?(name)
+        register_plugin name, path, optional_plugins.include?(name)
         Bundler.ui.info "Installed plugin #{name}"
       end
     end
@@ -153,9 +153,9 @@ module Bundler
     #
     # @param [String] name the name of the plugin
     # @param [Pathname] path the path where the plugin is installed at
-    # @param [Boolean] optional_plugin, removed if there is conflict (used for
-    #                       default source plugins)
-    def register_plugin(name, path, optional = false)
+    # @param [Boolean] optional_plugin, removed if there is conflict with any
+    #                     other plugin (used for default source plugins)
+    def register_plugin(name, path, optional_plugin = false)
       commands = @commands
       sources = @sources
 
@@ -168,7 +168,7 @@ module Bundler
         raise MalformattedPlugin, "#{e.class}: #{e.message}"
       end
 
-      if optional && @sources.keys.any? {|s| source? s }
+      if optional_plugin && @sources.keys.any? {|s| source? s }
         Bundler.rm_rf(path)
       else
         index.register_plugin name, path.to_s, @commands.keys, @sources.keys
diff --git a/lib/bundler/plugin/api.rb b/lib/bundler/plugin/api.rb
index 14ed8d21..35d34b2e 100644
--- a/lib/bundler/plugin/api.rb
+++ b/lib/bundler/plugin/api.rb
@@ -65,7 +65,7 @@ module Bundler
       end
 
       def respond_to_missing?(name, include_private = false)
-        Bundler.respond_to?(name) || super
+        Bundler.respond_to?(name, include_private) || super
       end
     end
   end
diff --git a/lib/bundler/plugin/api/source.rb b/lib/bundler/plugin/api/source.rb
index 701264ef..5401a97e 100644
--- a/lib/bundler/plugin/api/source.rb
+++ b/lib/bundler/plugin/api/source.rb
@@ -10,9 +10,9 @@ module Bundler
       # `uri_hash`, `gem_install_dir`; they are helpers).
       #
       # Defaults for methods, where ever possible are provided which is
-      # expected to work. But, all source plugins have to override `fetch_gemfiles`
-      # and `install`. Defaults are also not provided for `remote!`, `cache!`
-      # and `unlock!`.
+      # expected to work. But, all source plugins have to override
+      # `fetch_gemspec_files` and `install`. Defaults are also not provided for
+      # `remote!`, `cache!` and `unlock!`.
       #
       # The defaults shall work for most situations but nevertheless they can
       # be (preferably should be) overridden as per the plugins' needs safely
@@ -23,7 +23,20 @@ module Bundler
       # able to match objects representing same sources, but may be created in
       # different situation (like form gemfile and lockfile). The default ones
       # checks only for class and uri, but elaborate source plugins may need
-      # more comparisons (like, git checking on branch or tag).
+      # more comparisons (e.g. git checking on branch or tag).
+      #
+      # @!attribute [r] uri
+      #   @return [String] the remote specified with `source` block in Gemfile
+      #
+      # @!attribute [r] options
+      #   @return [String] options passed during initialization (either from
+      #     lockfile or Gemfile)
+      #
+      # @!attribute [rw] dependency_names
+      #   @return [Array<String>] Names of dependencies that the source should
+      #     try to resolve. It is not necessary to use this list intenally. This
+      #     is present to be compatible with `Definition` and is used by
+      #     rubygems source.
       module Source
         attr_reader :uri, :options
         attr_accessor :dependency_names
@@ -41,8 +54,9 @@ module Bundler
         #
         # Note: If the spec method is overridden, this function is not necessary
         #
-        # @return [Array<String>] paths of the gemfiles that can be installed
-        def fetch_gemfiles
+        # @return [Array<String>] paths of the gemspec files for gems that can
+        #                         be installed
+        def fetch_gemspec_files
           []
         end
 
@@ -78,7 +92,7 @@ module Bundler
             end
         end
 
-        # Parses the gemfiles to find the specs for the gems that can be
+        # Parses the gemspec files to find the specs for the gems that can be
         # satisfied by the source.
         #
         # Few important points to keep in mind:
@@ -91,7 +105,7 @@ module Bundler
         #
         # @return [Bundler::Index] index containing the specs
         def specs
-          files = fetch_gemfiles
+          files = fetch_gemspec_files
 
           Bundler::Index.build do |index|
             files.each do |file|
@@ -123,18 +137,31 @@ module Bundler
 
         # This shall check if two source object represent the same source.
         #
+        # The comparison shall take place only on the attribute that can be
+        # inferred from the options passed from Gemfile and not on attibutes
+        # that are used to pin down the gem to specific version (e.g. Git
+        # sources should compare on branch and tag but not on commit hash)
+        #
         # The sources objects are constructed from Gemfile as well as from
         # lockfile. To converge the sources, it is necessary that they match.
+        #
+        # The same applies for `eql?` and `hash`
         def ==(other)
           other.is_a?(self.class) && uri == other.uri
         end
 
+        # When overriding `eql?` please preserve the behaviour as mentioned in
+        # docstring for `==` method.
         alias_method :eql?, :==
 
+        # When overriding `hash` please preserve the behaviour as mentioned in
+        # docstring for `==` method, i.e. two methods equal by above comparison
+        # should have same hash.
         def hash
           [self.class, uri].hash
         end
 
+        # A helper method, not necessary if not used internally.
         def installed?
           File.directory?(install_path)
         end
@@ -148,7 +175,7 @@ module Bundler
         end
 
         def to_lock
-          out = String.new("PLUGIN\n")
+          out = String.new("#{LockfileParser::PLUGIN}\n")
           out << "  remote: #{@uri}\n"
           out << "  type: #{@type}\n"
           options_to_lock.each do |opt, value|
diff --git a/lib/bundler/plugin/dsl.rb b/lib/bundler/plugin/dsl.rb
index 296bfdbc..aefa761c 100644
--- a/lib/bundler/plugin/dsl.rb
+++ b/lib/bundler/plugin/dsl.rb
@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 module Bundler
-  # Dsl to parse the Gemfile looking for plugins to install
   module Plugin
+    # Dsl to parse the Gemfile looking for plugins to install
     class DSL < Bundler::Dsl
       class PluginGemfileError < PluginError; end
       alias_method :_gem, :gem # To use for plugin installation as gem
@@ -12,6 +12,14 @@ module Bundler
       # They will be handled by method_missing
       [:gemspec, :gem, :path, :install_if, :platforms, :env].each {|m| undef_method m }
 
+      # This lists the plugins that was added automatically and not specified by
+      # the user.
+      #
+      # When we encounter :type attribute with a source block, we add a plugin
+      # by name bundler-source-<type> to list of plugins to be installed.
+      #
+      # These plugins are optional and are not installed when there is conflict
+      # with any other plugin.
       attr_reader :auto_plugins
 
       def initialize
@@ -34,10 +42,11 @@ module Bundler
         return super unless options.key?("type")
 
         plugin_name = "bundler-source-#{options["type"]}"
-        unless @auto_plugins.include? plugin_name
-          plugin(plugin_name)
-          @auto_plugins << plugin_name
-        end
+
+        return if @auto_plugins.include? plugin_name
+
+        plugin(plugin_name)
+        @auto_plugins << plugin_name
       end
     end
   end
diff --git a/spec/plugins/command.rb b/spec/plugins/command_spec.rb
index 71e87a5b..71e87a5b 100644
--- a/spec/plugins/command.rb
+++ b/spec/plugins/command_spec.rb
diff --git a/spec/plugins/install.rb b/spec/plugins/install_spec.rb
index e0db0afa..e0db0afa 100644
--- a/spec/plugins/install.rb
+++ b/spec/plugins/install_spec.rb
diff --git a/spec/plugins/source/example.rb b/spec/plugins/source/example_spec.rb
index 7817d8cc..15671f67 100644
--- a/spec/plugins/source/example.rb
+++ b/spec/plugins/source/example_spec.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 require "spec_helper"
 
-describe "read source plugins" do
+describe "real source plugins" do
   context "with a minimal source plugin" do
     before do
       build_repo2 do
@@ -20,8 +20,8 @@ describe "read source plugins" do
                 @path = Pathname.new options["uri"]
               end
 
-              def fetch_gemfiles
-                @gemfiles ||= begin
+              def fetch_gemspec_files
+                @spec_files ||= begin
                   glob = "{,*,*/*}.gemspec"
                   if installed?
                     search_path = install_path
@@ -63,7 +63,7 @@ describe "read source plugins" do
       bundle "install"
 
       lockfile_should_be <<-G
-        PLUGIN
+        PLUGIN SOURCE
           remote: #{lib_path("a-path-gem-1.0")}
           type: mpath
           specs:
@@ -87,7 +87,7 @@ describe "read source plugins" do
     context "with lockfile" do
       before do
         lockfile <<-G
-          PLUGIN
+          PLUGIN SOURCE
             remote: #{lib_path("a-path-gem-1.0")}
             type: mpath
             specs:
@@ -139,8 +139,8 @@ describe "read source plugins" do
 
               alias_method :==, :eql?
 
-              def fetch_gemfiles
-                @gemfiles ||= begin
+              def fetch_gemspec_files
+                @spec_files ||= begin
                   glob = "{,*,*/*}.gemspec"
                   if !cached?
                     cache_repo
@@ -262,7 +262,7 @@ describe "read source plugins" do
       bundle "install"
 
       lockfile_should_be <<-G
-        PLUGIN
+        PLUGIN SOURCE
           remote: file://#{lib_path("ma-gitp-gem-1.0")}
           type: gitp
           revision: #{revision}
@@ -288,7 +288,7 @@ describe "read source plugins" do
       before do
         revision = revision_for(lib_path("ma-gitp-gem-1.0"))
         lockfile <<-G
-          PLUGIN
+          PLUGIN SOURCE
             remote: file://#{lib_path("ma-gitp-gem-1.0")}
             type: gitp
             revision: #{revision}
diff --git a/spec/plugins/source.rb b/spec/plugins/source_spec.rb
index a5217086..a5217086 100644
--- a/spec/plugins/source.rb
+++ b/spec/plugins/source_spec.rb