From 95d2110e49e160ffb785a74b22727593f17d552b Mon Sep 17 00:00:00 2001 From: Andreas Eger Date: Fri, 26 Apr 2019 15:00:30 +0200 Subject: [PATCH] Revert adding psych as runtime dependency (#6885) --- CHANGELOG.md | 2 + lib/rubocop/config_loader.rb | 19 +++-- lib/rubocop/yaml_duplication_checker.rb | 2 +- rubocop.gemspec | 1 - spec/rubocop/config_loader_spec.rb | 42 ++++++++--- spec/rubocop/node_pattern_spec.rb | 3 +- spec/rubocop/yaml_duplication_checker_spec.rb | 70 +++++++++++++------ 7 files changed, 99 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6022538e101..c4ae30bf1d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ * [#6917](https://github.com/rubocop-hq/rubocop/issues/6917): Bump Bundler dependency to >= 1.15.0. ([@koic][]) * Add `--auto-gen-only-exclude` to the command outputted in `rubocop_todo.yml` if the option is specified. ([@dvandersluis][]) * [#6887](https://github.com/rubocop-hq/rubocop/pull/6887): Allow `Lint/UnderscorePrefixedVariableName` cop to be configured to allow use of block keyword args. ([@dduugg][]) +* [#6885](https://github.com/rubocop-hq/rubocop/pull/6885): Revert adding psych >= 3.1 as runtime dependency. ([@andreaseger][]) ## 0.67.2 (2019-04-05) @@ -3963,3 +3964,4 @@ [@Mange]: https://github.com/Mange [@jmanian]: https://github.com/jmanian [@vfonic]: https://github.com/vfonic +[@andreaseger]: https://github.com/andreaseger diff --git a/lib/rubocop/config_loader.rb b/lib/rubocop/config_loader.rb index 5862ccc9e3e..db0d7e4eb84 100644 --- a/lib/rubocop/config_loader.rb +++ b/lib/rubocop/config_loader.rb @@ -206,10 +206,16 @@ 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}" + # .start_line is only available since ruby 2.5 / psych 3.0 + message = if key1.respond_to? :start_line + line1 = key1.start_line + 1 + line2 = key2.start_line + 1 + "#{smart_path}:#{line1}: " \ + "`#{value}` is concealed by line #{line2}" + else + "#{smart_path}: " \ + "`#{value}` is concealed by duplicate" + end warn Rainbow(message).yellow end end @@ -228,7 +234,8 @@ def yaml_safe_load(yaml_code, filename) if defined?(SafeYAML) && SafeYAML.respond_to?(:load) SafeYAML.load(yaml_code, filename, whitelisted_tags: %w[!ruby/regexp]) - else + # Ruby 2.6+ + elsif Gem::Version.new(Psych::VERSION) >= Gem::Version.new('3.1.0') YAML.safe_load( yaml_code, permitted_classes: [Regexp, Symbol], @@ -236,6 +243,8 @@ def yaml_safe_load(yaml_code, filename) aliases: false, filename: filename ) + else + YAML.safe_load(yaml_code, [Regexp, Symbol], [], false, filename) end end end diff --git a/lib/rubocop/yaml_duplication_checker.rb b/lib/rubocop/yaml_duplication_checker.rb index f0923eb0849..7af9d0c96ea 100644 --- a/lib/rubocop/yaml_duplication_checker.rb +++ b/lib/rubocop/yaml_duplication_checker.rb @@ -5,7 +5,7 @@ module RuboCop 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) + tree = YAML.parse(yaml_string, filename) return unless tree traverse(tree, &on_duplicated) diff --git a/rubocop.gemspec b/rubocop.gemspec index 652d5836cfb..af6a76a5e29 100644 --- a/rubocop.gemspec +++ b/rubocop.gemspec @@ -36,7 +36,6 @@ Gem::Specification.new do |s| s.add_runtime_dependency('jaro_winkler', '~> 1.5.1') s.add_runtime_dependency('parallel', '~> 1.10') s.add_runtime_dependency('parser', '>= 2.5', '!= 2.5.1.1') - s.add_runtime_dependency('psych', '>= 3.1.0') s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0') s.add_runtime_dependency('ruby-progressbar', '~> 1.7') s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 1.6') diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index 6092ef0e216..48e549dfa74 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -1049,19 +1049,39 @@ 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 + context '< Ruby 2.5', if: RUBY_VERSION < '2.5' do + 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 + Style/Encoding: + Enabled: false + YAML - expect do - load_file - end.to output(%r{`Style/Encoding` is concealed by line 4}).to_stderr + expect do + load_file + end.to output(%r{`Style/Encoding` is concealed by duplicat}).to_stderr + end + end + end + + context '>= Ruby 2.5', if: RUBY_VERSION >= '2.5' do + 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 end diff --git a/spec/rubocop/node_pattern_spec.rb b/spec/rubocop/node_pattern_spec.rb index 4f63df91657..56de66bad03 100644 --- a/spec/rubocop/node_pattern_spec.rb +++ b/spec/rubocop/node_pattern_spec.rb @@ -127,8 +127,7 @@ describe 'yaml compatibility' do let(:instance) do - YAML.safe_load(YAML.dump(super()), - permitted_classes: [described_class]) + YAML.safe_load(YAML.dump(super()), [described_class]) end let(:ruby) { 'obj.method' } diff --git a/spec/rubocop/yaml_duplication_checker_spec.rb b/spec/rubocop/yaml_duplication_checker_spec.rb index 4b02dedefb5..10f61afdf3d 100644 --- a/spec/rubocop/yaml_duplication_checker_spec.rb +++ b/spec/rubocop/yaml_duplication_checker_spec.rb @@ -26,17 +26,32 @@ def check(yaml, &block) 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 + context '< Ruby 2.5', if: RUBY_VERSION < '2.5' do + 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.value).to eq('Layout/Tab') + expect(key2.value).to eq('Layout/Tab') + end + end + + context '>= Ruby 2.5', if: RUBY_VERSION >= '2.5' do + 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 - 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 @@ -49,17 +64,32 @@ def check(yaml, &block) 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 + context '< Ruby 2.5', if: RUBY_VERSION < '2.5' do + 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.value).to eq('Enabled') + expect(key2.value).to eq('Enabled') + end + end + + context '>= Ruby 2.5', if: RUBY_VERSION >= '2.5' do + 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 - 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