Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revert adding psych as runtime dependency #6885

Merged
merged 1 commit into from Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down Expand Up @@ -3963,3 +3964,4 @@
[@Mange]: https://github.com/Mange
[@jmanian]: https://github.com/jmanian
[@vfonic]: https://github.com/vfonic
[@andreaseger]: https://github.com/andreaseger
19 changes: 14 additions & 5 deletions lib/rubocop/config_loader.rb
Expand Up @@ -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
andreaseger marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -228,14 +234,17 @@ 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+
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you say this would be ok on 2.5+?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reverted the exact comment that was removed in #6766
I only checked the start_line bit and saw that it is available since 2.5

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I checked and the kwargs used here where introduced in psych 3.1 (which is used since ruby 2.6)

elsif Gem::Version.new(Psych::VERSION) >= Gem::Version.new('3.1.0')
YAML.safe_load(
yaml_code,
permitted_classes: [Regexp, Symbol],
permitted_symbols: [],
aliases: false,
filename: filename
)
else
YAML.safe_load(yaml_code, [Regexp, Symbol], [], false, filename)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/yaml_duplication_checker.rb
Expand Up @@ -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)
andreaseger marked this conversation as resolved.
Show resolved Hide resolved
return unless tree

traverse(tree, &on_duplicated)
Expand Down
1 change: 0 additions & 1 deletion rubocop.gemspec
Expand Up @@ -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')
Expand Down
42 changes: 31 additions & 11 deletions spec/rubocop/config_loader_spec.rb
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions spec/rubocop/node_pattern_spec.rb
Expand Up @@ -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' }

Expand Down
70 changes: 50 additions & 20 deletions spec/rubocop/yaml_duplication_checker_spec.rb
Expand Up @@ -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

Expand All @@ -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

Expand Down