[#3600] Add new Bundler/DuplicatedGem cop

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* [#3600](https://github.com/bbatsov/rubocop/issues/3600): Add new `Bundler/DuplicatedGem` cop. ([@jmks][])
+
 ### Bug fixes
 
 * [#3662](https://github.com/bbatsov/rubocop/issues/3662): Fix the auto-correction of `Lint/UnneededSplatExpansion` when the splat expansion is inside of another array. ([@rrosenblum][])

config/enabled.yml

@@ -1559,3 +1559,12 @@ Security/JSONLoad:
   # Autocorrect here will change to a method that may cause crashes depending
   # on the value of the argument.
   AutoCorrect: false
+
+##################### Bundler #############################
+
+Bundler/DuplicatedGem:
+  Description: 'Checks for duplicate gem entries in Gemfile.'
+  Enabled: true
+  Include:
+    - '**/Gemfile'
+    - '**/gems.rb'

lib/rubocop.rb

@@ -91,6 +91,8 @@
 require 'rubocop/cop/mixin/trailing_comma'
 require 'rubocop/cop/mixin/unused_argument'
 
+require 'rubocop/cop/bundler/duplicated_gem'
+
 require 'rubocop/cop/lint/ambiguous_operator'
 require 'rubocop/cop/lint/ambiguous_regexp_literal'
 require 'rubocop/cop/lint/assignment_in_condition'

lib/rubocop/cop/bundler/duplicated_gem.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Bundler
+      # A Gem's requirements should be listed only once in a Gemfile.
+      # @example
+      #   # bad
+      #   gem 'rubocop'
+      #   gem 'rubocop'
+      #
+      #   # bad
+      #   group :development do
+      #     gem 'rubocop'
+      #   end
+      #
+      #   group :test do
+      #     gem 'rubocop'
+      #   end
+      #
+      #   # good
+      #   group :development, :test do
+      #     gem 'rubocop'
+      #   end
+      #
+      #   # good
+      #   gem 'rubocop', groups: [:development, :test]
+      class DuplicatedGem < Cop
+        MSG = 'Gem `%s` requirements already given on line %d ' \
+              'of the Gemfile.'.freeze
+
+        def investigate(processed_source)
+          return unless processed_source.ast
+
+          duplicated_gem_nodes.each do |nodes|
+            nodes[1..-1].each do |node|
+              offense(
+                node,
+                node.method_args.first.to_a.first,
+                nodes.first.loc.line
+              )
+            end
+          end
+        end
+
+        private
+
+        def_node_search :gem_declarations, '(send nil :gem str ...)'
+
+        def duplicated_gem_nodes
+          gem_declarations(processed_source.ast)
+            .group_by { |e| e.method_args.first }
+            .keep_if { |_, nodes| nodes.length > 1 }
+            .values
+        end
+
+        def offense(node, gem_name, line_of_first_occurence)
+          line_range = node.loc.column...node.loc.last_column
+
+          add_offense(
+            node,
+            source_range(processed_source.buffer, node.loc.line, line_range),
+            format(MSG, gem_name, line_of_first_occurence)
+          )
+        end
+      end
+    end
+  end
+end

lib/rubocop/rspec/cop_helper.rb

@@ -12,6 +12,10 @@ def inspect_source_file(cop, source)
     Tempfile.open('tmp') { |f| inspect_source(cop, source, f) }
   end
 
+  def inspect_gemfile(cop, source)
+    inspect_source(cop, source, 'Gemfile')
+  end
+
   def inspect_source(cop, source, file = nil)
     if source.is_a?(Array) && source.size == 1
       raise "Don't use an array for a single line of code: #{source}"

manual/cops.md

@@ -58,6 +58,10 @@ Rails:
   Enabled: true
 ```
 
+### Bundler
+
+Bundler cops check for style or bad practices in Bundler files, e.g. `Gemfile`.
+
 ### Available cops
 
 In the following section you find all available cops:
@@ -377,4 +381,7 @@ In the following section you find all available cops:
 
 #### Type [Security](cops_security.md)
 * [Security/JSONLoad](cops_security.md#securityjsonload)
-## 
+
+#### Type [Bundler](cops_bundler.md)
+
+* [Bundler/DuplicatedGem](cops_bundler.md#bundlerduplicatedgem)

manual/cops_bundler.md

@@ -0,0 +1,16 @@
+# Bundler
+
+## Bundler/DuplicatedGem
+
+Enabled by default | Supports autocorrection
+--- | ---
+Enabled | No
+
+This cop checks for duplicate gem entries in Gemfiles. Bundler currently
+only prints a warning (unless there is a requirements conflict).
+
+### Important attributes
+
+Attribute | Value |
+--- | --- |
+Include | \*\*/Gemfile, \*\*/gems.rb |

mkdocs.yml

@@ -23,6 +23,7 @@ pages:
     - Rails Cops: cops_rails.md
     - Security Cops: cops_security.md
     - Style Cops: cops_style.md
+    - Bundler Cops: cops_bundler.md
 extra_css:
   - css/extra.css
 theme: readthedocs

spec/rubocop/cop/bundler/duplicated_gem_spec.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Bundler::DuplicatedGem, :config do
+  let(:cop_config) { { 'Include' => ['**/Gemfile'] } }
+  subject(:cop) { described_class.new(config) }
+
+  context 'when investigating Ruby files' do
+    let(:source) { <<-END }
+      # cop will not read these contents
+      gem('rubocop')
+      gem('rubocop')
+    END
+
+    it 'does not register any offenses' do
+      inspect_source_file(cop, source)
+      expect(cop.offenses).to be_empty
+    end
+  end
+
+  context 'when investigating Gemfiles' do
+    context 'and the file is empty' do
+      let(:source) { '' }
+
+      it 'does not raise an error' do
+        expect { inspect_source(cop, source, 'gems.rb') }.not_to raise_error
+      end
+
+      it 'does not register any offenses' do
+        expect(cop.offenses).to be_empty
+      end
+    end
+
+    context 'and no duplicate gems are present' do
+      let(:source) { <<-GEM }
+        gem 'rubocop'
+        gem 'flog'
+      GEM
+
+      it 'does not register any offenses' do
+        inspect_gemfile(cop, source)
+        expect(cop.offenses).to be_empty
+      end
+    end
+
+    context 'and a gem is duplicated in default group' do
+      let(:source) { <<-GEM }
+        source 'https://rubygems.org'
+        gem 'rubocop'
+        gem 'rubocop'
+      GEM
+
+      it 'registers an offense' do
+        inspect_gemfile(cop, source)
+        expect(cop.offenses.size).to eq(1)
+      end
+
+      it "references gem's first occurance in message" do
+        inspect_gemfile(cop, source)
+        expect(cop.offenses.first.message).to include('2')
+      end
+
+      it 'highlights the duplicate gem' do
+        inspect_gemfile(cop, source)
+        expect(cop.highlights).to eq(["gem 'rubocop'"])
+      end
+    end
+
+    context 'and a duplicated gem is in a git/path/group/platforms block' do
+      let(:source) { <<-GEM }
+        gem 'rubocop'
+        group :development do
+          gem 'rubocop', path: '/path/to/gem'
+        end
+      GEM
+
+      it 'registers an offense' do
+        inspect_gemfile(cop, source)
+        expect(cop.offenses.size).to eq(1)
+      end
+
+      it 'highlights the duplicate gem' do
+        inspect_gemfile(cop, source)
+        expect(cop.highlights).to eq(["gem 'rubocop', path: '/path/to/gem'"])
+      end
+    end
+  end
+end