Warn duplicated keys in .rubocop.yml

CHANGELOG.md

@@ -10,6 +10,7 @@
 ### Changes
 
 * [#6766](https://github.com/rubocop-hq/rubocop/pull/6766): Drop support for Ruby 2.2.0 and 2.2.1. ([@pocke][])
+* [#6733](https://github.com/rubocop-hq/rubocop/pull/6733): Warn duplicated keys in `.rubocop.yml`. ([@pocke][])
 
 ## 0.64.0 (2019-02-10)
 

config/default.yml

@@ -3928,7 +3928,7 @@ Style/SafeNavigation:
                   safe navigation (`&.`).
   Enabled: true
   VersionAdded: '0.43'
-  VersionChanged: '0.44'
+  VersionChanged: '0.56'
   # Safe navigation may cause a statement to start returning `nil` in addition
   # to whatever it used to return.
   ConvertCodeThatCanStartToReturnNil: false
@@ -3938,7 +3938,6 @@ Style/SafeNavigation:
     - presence
     - try
     - try!
-  VersionChanged: '0.56'
 
 Style/SelfAssignment:
   Description: >-

lib/rubocop.rb

@@ -650,3 +650,4 @@
 require_relative 'rubocop/cli'
 require_relative 'rubocop/options'
 require_relative 'rubocop/remote_config'
+require_relative 'rubocop/yaml_duplication_checker'

lib/rubocop/config_loader.rb

@@ -158,6 +158,7 @@ def resolver
 
       def load_yaml_configuration(absolute_path)
         yaml_code = read_file(absolute_path)
+        check_duplication(yaml_code, absolute_path)
         hash = yaml_safe_load(yaml_code, absolute_path) || {}
 
         puts "configuration from #{absolute_path}" if debug?
@@ -169,6 +170,18 @@ def load_yaml_configuration(absolute_path)
         hash
       end
 
+      def check_duplication(yaml_code, absolute_path)
+        smart_path = PathUtil.smart_path(absolute_path)
+        YAMLDuplicationChecker.check(yaml_code, absolute_path) do |key1, key2|
+          value = key1.value
+          line1 = key1.start_line + 1
+          line2 = key2.start_line + 1
+          message = "#{smart_path}:#{line1}: " \
+                    "`#{value}` is concealed by line #{line2}"
+          warn Rainbow(message).yellow
+        end
+      end
+
       # Read the specified file, or exit with a friendly, concise message on
       # stderr. Care is taken to use the standard OS exit code for a "file not
       # found" error.

lib/rubocop/path_util.rb

@@ -62,6 +62,13 @@ def self.reset_pwd
       @pwd = nil
     end
 
+    def self.chdir(dir, &block)
+      reset_pwd
+      Dir.chdir(dir, &block)
+    ensure
+      reset_pwd
+    end
+
     def hidden_file_in_not_hidden_dir?(pattern, path)
       File.fnmatch?(
         pattern, path,

lib/rubocop/rspec/shared_contexts.rb

@@ -23,7 +23,7 @@
         working_dir = File.join(tmpdir, 'work')
         Dir.mkdir(working_dir)
 
-        Dir.chdir(working_dir) do
+        RuboCop::PathUtil.chdir(working_dir) do
           example.run
         end
       ensure

lib/rubocop/yaml_duplication_checker.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module RuboCop
+  # Find duplicated keys from YAML.
+  module YAMLDuplicationChecker
+    def self.check(yaml_string, filename, &on_duplicated)
+      # Specify filename to display helpful message when it raises an error.
+      tree = YAML.parse(yaml_string, filename: filename)
+      return unless tree
+
+      traverse(tree, &on_duplicated)
+    end
+
+    def self.traverse(tree, &on_duplicated)
+      case tree
+      when Psych::Nodes::Mapping
+        tree.children.each_slice(2).with_object([]) do |(key, value), keys|
+          exist = keys.find { |key2| key2.value == key.value }
+          on_duplicated.call(exist, key) if exist
+          keys << key
+          traverse(value, &on_duplicated)
+        end
+      else
+        children = tree.children
+        return unless children
+
+        children.each { |c| traverse(c, &on_duplicated) }
+      end
+    end
+
+    private_class_method :traverse
+  end
+end

spec/project_spec.rb

@@ -54,6 +54,15 @@
 
       raise errors.join("\n") unless errors.empty?
     end
+
+    it 'does not have nay duplication' do
+      fname = File.expand_path('../config/default.yml', __dir__)
+      content = File.read(fname)
+      RuboCop::YAMLDuplicationChecker.check(content, fname) do |key1, key2|
+        raise "#{fname} has duplication of #{key1.value} " \
+              "on line #{key1.start_line} and line #{key2.start_line}"
+      end
+    end
   end
 
   describe 'cop message' do

spec/rubocop/cli/cli_auto_gen_config_spec.rb

@@ -398,7 +398,9 @@ def f
           Metrics/LineLength:
             Max: 95
         YAML
-        Dir.chdir('dir') { expect(cli.run(%w[--auto-gen-config])).to eq(0) }
+        RuboCop::PathUtil.chdir('dir') do
+          expect(cli.run(%w[--auto-gen-config])).to eq(0)
+        end
         expect($stderr.string).to eq('')
         # expect($stdout.string).to include('Created .rubocop_todo.yml.')
         expect(Dir['dir/.*']).to include('dir/.rubocop_todo.yml')

spec/rubocop/cli_spec.rb

@@ -21,7 +21,7 @@
       it 'checks a Rakefile but Style/FileName does not report' do
         create_file('Rakefile', 'x = 1')
         create_empty_file('other/empty')
-        Dir.chdir('other') do
+        RuboCop::PathUtil.chdir('other') do
           expect(cli.run(['--format', 'simple', checked_path])).to eq(1)
         end
         expect($stdout.string)
@@ -602,7 +602,7 @@ def and_with_args
             Exclude:
               - lib/example.rb
         YAML
-        Dir.chdir('lib') { expect(cli.run([])).to eq(0) }
+        RuboCop::PathUtil.chdir('lib') { expect(cli.run([])).to eq(0) }
         expect($stdout.string).to include('no offenses detected')
       end
 
@@ -624,7 +624,7 @@ def and_with_args
             Exclude:
               - lib/example.rb
         YAML
-        Dir.chdir('lib') { expect(cli.run([])).to eq(0) }
+        RuboCop::PathUtil.chdir('lib') { expect(cli.run([])).to eq(0) }
         expect($stdout.string).to include('no offenses detected')
         expect($stderr.string.empty?).to be(true)
       end

spec/rubocop/config_loader_spec.rb

@@ -973,6 +973,22 @@ def cop_enabled?(cop_class)
         )
       end
     end
+
+    context 'when the file has duplicated keys' do
+      it 'outputs a warning' do
+        create_file(configuration_path, <<-YAML.strip_indent)
+          Style/Encoding:
+            Enabled: true
+
+          Style/Encoding:
+            Enabled: false
+        YAML
+
+        expect do
+          load_file
+        end.to output(%r{`Style/Encoding` is concealed by line 4}).to_stderr
+      end
+    end
   end
 
   describe '.merge' do
@@ -1052,7 +1068,7 @@ def cop_enabled?(cop_class)
 
     it 'uses paths relative to the .rubocop.yml, not cwd' do
       config_path = described_class.configuration_file_for('.')
-      Dir.chdir '..' do
+      RuboCop::PathUtil.chdir '..' do
         described_class.configuration_from_file(config_path)
         expect(defined?(MyClass)).to be_truthy
       end
@@ -1070,7 +1086,7 @@ def cop_enabled?(cop_class)
     it 'works without a starting .' do
       config_path = described_class.configuration_file_for('.')
       $LOAD_PATH.unshift(File.dirname(config_path))
-      Dir.chdir '..' do
+      RuboCop::PathUtil.chdir '..' do
         described_class.configuration_from_file(config_path)
         expect(defined?(MyClass)).to be_truthy
       end

spec/rubocop/cop/generator/require_file_injector_spec.rb

@@ -13,7 +13,7 @@
 
   around do |example|
     Dir.mktmpdir('rubocop-require_file_injector_spec-') do |dir|
-      Dir.chdir(dir) do
+      RuboCop::PathUtil.chdir(dir) do
         Dir.mkdir('lib')
         example.run
       end

spec/rubocop/formatter/html_formatter_spec.rb

@@ -7,7 +7,7 @@
     project_path = File.join(spec_root, 'fixtures/html_formatter/project')
     FileUtils.cp_r(project_path, '.')
 
-    Dir.chdir(File.basename(project_path)) do
+    RuboCop::PathUtil.chdir(File.basename(project_path)) do
       example.run
     end
   end

spec/rubocop/target_finder_spec.rb

@@ -97,7 +97,7 @@
       let(:args) { [] }
 
       it 'finds files under the current directory' do
-        Dir.chdir('dir1') do
+        RuboCop::PathUtil.chdir('dir1') do
           expect(found_files.empty?).to be(false)
           found_files.each do |file|
             expect(file).to include('/dir1/')
@@ -111,7 +111,7 @@
       let(:args) { ['../dir2'] }
 
       it 'finds files under the specified directory' do
-        Dir.chdir('dir1') do
+        RuboCop::PathUtil.chdir('dir1') do
           expect(found_files.empty?).to be(false)
           found_files.each do |file|
             expect(file).to include('/dir2/')

spec/rubocop/yaml_duplication_checker_spec.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::YAMLDuplicationChecker do
+  def check(yaml, &block)
+    described_class.check(yaml, 'dummy.yaml', &block)
+  end
+
+  shared_examples 'call block' do
+    it 'calls block' do
+      called = false
+      check(yaml) do
+        called = true
+      end
+      expect(called).to be(true)
+    end
+  end
+
+  context 'when yaml has duplicated keys in the top level' do
+    let(:yaml) { <<-YAML.strip_indent }
+      Layout/Tab:
+        Enabled: true
+
+      Layout/Tab:
+        Enabled: false
+    YAML
+
+    include_examples 'call block'
+
+    it 'calls block with keys' do
+      key1 = nil
+      key2 = nil
+      check(yaml) do |key_a, key_b|
+        key1 = key_a
+        key2 = key_b
+      end
+      expect(key1.start_line).to eq(0)
+      expect(key2.start_line).to eq(3)
+      expect(key1.value).to eq('Layout/Tab')
+      expect(key2.value).to eq('Layout/Tab')
+    end
+  end
+
+  context 'when yaml has duplicated keys in the second level' do
+    let(:yaml) { <<-YAML.strip_indent }
+      Layout/Tab:
+        Enabled: true
+        Enabled: false
+    YAML
+
+    include_examples 'call block'
+
+    it 'calls block with keys' do
+      key1 = nil
+      key2 = nil
+      check(yaml) do |key_a, key_b|
+        key1 = key_a
+        key2 = key_b
+      end
+      expect(key1.start_line).to eq(1)
+      expect(key2.start_line).to eq(2)
+      expect(key1.value).to eq('Enabled')
+      expect(key2.value).to eq('Enabled')
+    end
+  end
+
+  context 'when yaml does not have any duplication' do
+    let(:yaml) { <<-YAML.strip_indent }
+      Style/TrailingCommaInHashLiteral:
+        Enabled: true
+        EnforcedStyleForMultiline: no_comma
+      Style/TrailingBodyOnModule:
+        Enabled: true
+    YAML
+
+    it 'does not call block' do
+      called = false
+      described_class.check(yaml, 'dummy.yaml') do
+        called = true
+      end
+      expect(called).to be(false)
+    end
+  end
+end