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

Bump rubocop to use v0.50.x #6368

Merged
merged 6 commits into from Sep 22, 2017
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
14 changes: 11 additions & 3 deletions .rubocop.yml
Expand Up @@ -21,7 +21,7 @@ Layout/EmptyLinesAroundAccessModifier:
Layout/EmptyLinesAroundModuleBody:
Enabled: false
Layout/EndOfLine:
EnforcedStyle: lf
EnforcedStyle: native
Copy link
Member

Choose a reason for hiding this comment

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

Done we want lf here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, we do.. but if a developer on Windows were to run script/fmt they'd unnecessarily get taxed by Rubocop. In my own experience, it has been irritating..
A properly configured Git on Windows automatically indexes files compatible for cross-platform use.

Copy link
Member

Choose a reason for hiding this comment

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

👍

Layout/ExtraSpacing:
AllowForAlignment: true
Layout/FirstParameterIndentation:
Expand All @@ -44,10 +44,14 @@ Layout/SpaceInsideBrackets:
Enabled: false
Lint/EndAlignment:
Severity: error
Lint/RescueWithoutErrorClass:
Enabled: false
Copy link
Member

Choose a reason for hiding this comment

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

Oooo we should enable this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure about it though, since unnamed rescue means rescuing StandardError by default.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but it's always better to rescue explicitly what you want to rescue instead of everything. We can do this in a subsequent pass.

Lint/UnreachableCode:
Severity: error
Lint/UselessAccessModifier:
Enabled: false
Lint/Void:
Enabled: false
Copy link
Member

Choose a reason for hiding this comment

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

What even is this? 😂

Copy link
Member Author

Choose a reason for hiding this comment

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

This cop checks for the use of a return with a value in a context where it will be ignored.
In our case specifically, this cop (incorrectly ?) flags Site#config= similar to the documented example

Copy link
Member

Choose a reason for hiding this comment

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

Oh we must return some value and it doesn't like that! We can enable it in a subsequent pass.

Metrics/AbcSize:
Max: 21
Metrics/BlockLength:
Expand Down Expand Up @@ -82,6 +86,10 @@ Metrics/ParameterLists:
Max: 4
Metrics/PerceivedComplexity:
Max: 8
Naming/FileName:
Enabled: false
Copy link
Member

Choose a reason for hiding this comment

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

What fails for this one?

Copy link
Member Author

Choose a reason for hiding this comment

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

From their docs,

This cop makes sure that Ruby source files have snake_case names. Ruby scripts (i.e. source files with a shebang in the first line) are ignored.

and flags us for the following:
- 'Gemfile'
- 'Rakefile'
- 'lib/theme_template/Gemfile'
- 'test/fixtures/test-dependency-theme/test-dependency-theme.gemspec'
- 'test/fixtures/test-theme/test-theme.gemspec'

which IMO is a bug because this cop existed as an enabled-by-default-cop earlier known as Style/FileName

Copy link
Member

Choose a reason for hiding this comment

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

Yeah... that looks like a bug. Weird.

Naming/HeredocDelimiterNaming:
Enabled: false
Security/MarshalLoad:
Exclude:
- !ruby/regexp /test\/.*.rb$/
Expand Down Expand Up @@ -109,8 +117,8 @@ Style/Documentation:
- !ruby/regexp /features\/.*.rb$/
Style/DoubleNegation:
Enabled: false
Style/FileName:
Enabled: false
Style/Encoding:
EnforcedStyle: when_needed
Style/GuardClause:
Enabled: false
Style/HashSyntax:
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -30,7 +30,7 @@ group :test do
gem "nokogiri", RUBY_VERSION >= "2.2" ? "~> 1.7" : "~> 1.7.0"
gem "rspec"
gem "rspec-mocks"
gem "rubocop", "~> 0.49.1"
gem "rubocop", "~> 0.50.0"
gem "test-dependency-theme", :path => File.expand_path("test/fixtures/test-dependency-theme", __dir__)
gem "test-theme", :path => File.expand_path("test/fixtures/test-theme", __dir__)

Expand Down
1 change: 0 additions & 1 deletion jekyll.gemspec
@@ -1,4 +1,3 @@
# coding: utf-8
# frozen_string_literal: true

lib = File.expand_path("lib", __dir__)
Expand Down
4 changes: 2 additions & 2 deletions lib/jekyll.rb
Expand Up @@ -119,15 +119,15 @@ def configuration(override = {})
# timezone - the IANA Time Zone
#
# Returns nothing
# rubocop:disable Style/AccessorMethodName
# rubocop:disable Naming/AccessorMethodName
def set_timezone(timezone)
ENV["TZ"] = if Utils::Platforms.really_windows?
Utils::WinTZ.calculate(timezone)
else
timezone
end
end
# rubocop:enable Style/AccessorMethodName
# rubocop:enable Naming/AccessorMethodName

# Public: Fetch the logger instance for this Jekyll process.
#
Expand Down
2 changes: 1 addition & 1 deletion lib/jekyll/commands/doctor.rb
Expand Up @@ -86,7 +86,7 @@ def fsnotify_buggy?(_site)
def urls_only_differ_by_case(site)
urls_only_differ_by_case = false
urls = case_insensitive_urls(site.pages + site.docs_to_write, site.dest)
urls.each do |_case_insensitive_url, real_urls|
urls.each_value do |real_urls|
Copy link
Member

Choose a reason for hiding this comment

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

Cool!

next unless real_urls.uniq.size > 1
urls_only_differ_by_case = true
Jekyll.logger.warn "Warning:", "The following URLs only differ" \
Expand Down
2 changes: 1 addition & 1 deletion lib/jekyll/commands/serve/servlet.rb
Expand Up @@ -27,7 +27,7 @@ def search_file(req, res, basename)
super || super(req, res, ".html") || super(req, res, "#{basename}.html")
end

# rubocop:disable Style/MethodName
# rubocop:disable Naming/MethodName
def do_GET(req, res)
rtn = super
validate_and_ensure_charset(req, res)
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/configuration.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
4 changes: 2 additions & 2 deletions lib/jekyll/converters/markdown.rb
Expand Up @@ -27,7 +27,7 @@ def setup
# Rubocop does not allow reader methods to have names starting with `get_`
# To ensure compatibility, this check has been disabled on this method
#
# rubocop:disable Style/AccessorMethodName
# rubocop:disable Naming/AccessorMethodName
def get_processor
case @config["markdown"].downcase
when "redcarpet" then return RedcarpetParser.new(@config)
Expand All @@ -37,7 +37,7 @@ def get_processor
custom_processor
end
end
# rubocop:enable Style/AccessorMethodName
# rubocop:enable Naming/AccessorMethodName

# Public: Provides you with a list of processors, the ones we
# support internally and the ones that you have provided to us (if you
Expand Down
2 changes: 2 additions & 0 deletions lib/jekyll/converters/markdown/kramdown_parser.rb
Expand Up @@ -42,12 +42,14 @@ def convert(content)
end

private
# rubocop:disable Performance/HashEachMethods
def make_accessible(hash = @config)
hash.keys.each do |key|
Copy link
Member

Choose a reason for hiding this comment

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

Can we use each_key here?

Copy link
Member Author

Choose a reason for hiding this comment

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

each_key broke our test-suite..

hash[key.to_sym] = hash[key]
make_accessible(hash[key]) if hash[key].is_a?(Hash)
end
end
# rubocop:enable Performance/HashEachMethods

# config[kramdown][syntax_higlighter] >
# config[kramdown][enable_coderay] >
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/convertible.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

require "set"
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/document.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/collection_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/document_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/excerpt_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/jekyll_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/site_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/unified_payload_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/drops/url_drop.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
3 changes: 1 addition & 2 deletions lib/jekyll/reader.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

require "csv"
Expand All @@ -25,7 +24,7 @@ def read

# Sorts posts, pages, and static files.
def sort_files!
site.collections.values.each { |c| c.docs.sort! }
site.collections.each_value { |c| c.docs.sort! }
site.pages.sort_by!(&:name)
site.static_files.sort_by!(&:relative_path)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/jekyll/readers/collection_reader.rb
Expand Up @@ -14,7 +14,7 @@ def initialize(site)
#
# Returns nothing.
def read
site.collections.each do |_, collection|
site.collections.each_value do |collection|
collection.read unless SPECIAL_COLLECTIONS.include?(collection.label)
end
end
Expand Down
1 change: 0 additions & 1 deletion lib/jekyll/renderer.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down
15 changes: 7 additions & 8 deletions lib/jekyll/site.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

require "csv"
Expand Down Expand Up @@ -85,11 +84,11 @@ def print_stats
#
# Returns nothing
def reset
if config["time"]
self.time = Utils.parse_date(config["time"].to_s, "Invalid time in _config.yml.")
else
self.time = Time.now
end
self.time = if config["time"]
Utils.parse_date(config["time"].to_s, "Invalid time in _config.yml.")
else
Time.now
end
self.layouts = {}
self.pages = []
self.static_files = []
Expand Down Expand Up @@ -238,7 +237,7 @@ def post_attr_hash(post_attr)
posts.docs.each do |p|
p.data[post_attr].each { |t| hash[t] << p } if p.data[post_attr]
end
hash.values.each { |posts| posts.sort!.reverse! }
hash.each_value { |posts| posts.sort!.reverse! }
hash
end

Expand Down Expand Up @@ -449,7 +448,7 @@ def configure_file_read_opts

private
def render_docs(payload)
collections.each do |_, collection|
collections.each_value do |collection|
collection.docs.each do |document|
if regenerator.regenerate?(document)
document.output = Jekyll::Renderer.new(self, document, payload).run
Expand Down
8 changes: 4 additions & 4 deletions lib/jekyll/tags/highlight.rb
Expand Up @@ -18,13 +18,13 @@ def initialize(tag_name, markup, tokens)
@lang = Regexp.last_match(1).downcase
@highlight_options = parse_options(Regexp.last_match(2))
else
raise SyntaxError, <<-eos
raise SyntaxError, <<-MSG
Syntax Error in tag 'highlight' while parsing the following markup:

#{markup}

Valid syntax: highlight <lang> [linenos]
eos
MSG
end
end

Expand Down Expand Up @@ -95,14 +95,14 @@ def render_pygments(code, is_safe)
)

if highlighted_code.nil?
Jekyll.logger.error <<eos
Jekyll.logger.error <<-MSG
There was an error highlighting your code:

#{code}

While attempting to convert the above code, Pygments.rb returned an unacceptable value.
This is usually a timeout problem solved by running `jekyll build` again.
eos
MSG
raise ArgumentError, "Pygments.rb returned an unacceptable value "\
"when attempting to highlight some code."
end
Expand Down
9 changes: 4 additions & 5 deletions lib/jekyll/tags/include.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

module Jekyll
Expand Down Expand Up @@ -61,7 +60,7 @@ def parse_params(context)

def validate_file_name(file)
if file !~ %r!^[a-zA-Z0-9_/\.-]+$! || file =~ %r!\./! || file =~ %r!/\.!
raise ArgumentError, <<-eos
raise ArgumentError, <<-MSG
Invalid syntax for include tag. File contains invalid characters or sequences:

#{file}
Expand All @@ -70,14 +69,14 @@ def validate_file_name(file)

#{syntax_example}

eos
MSG
end
end

def validate_params
full_valid_syntax = %r!\A\s*(?:#{VALID_SYNTAX}(?=\s|\z)\s*)*\z!
unless @params =~ full_valid_syntax
raise ArgumentError, <<-eos
raise ArgumentError, <<-MSG
Invalid syntax for include tag:

#{@params}
Expand All @@ -86,7 +85,7 @@ def validate_params

#{syntax_example}

eos
MSG
end
end

Expand Down
4 changes: 2 additions & 2 deletions lib/jekyll/tags/link.rb
Expand Up @@ -24,11 +24,11 @@ def render(context)
return item.url if item.relative_path == "/#{@relative_path}"
end

raise ArgumentError, <<eos
raise ArgumentError, <<-MSG
Could not find document '#{@relative_path}' in tag '#{self.class.tag_name}'.

Make sure the document exists and the path is correct.
eos
MSG
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/jekyll/tags/post_url.rb
Expand Up @@ -60,13 +60,13 @@ def initialize(tag_name, post, tokens)
begin
@post = PostComparer.new(@orig_post)
rescue => e
raise Jekyll::Errors::PostURLError, <<-eos
raise Jekyll::Errors::PostURLError, <<-MSG
Could not parse name of post "#{@orig_post}" in tag 'post_url'.

Make sure the post exists and the name is correct.

#{e.class}: #{e.message}
eos
MSG
end
end

Expand All @@ -90,11 +90,11 @@ def render(context)
return p.url
end

raise Jekyll::Errors::PostURLError, <<-eos
raise Jekyll::Errors::PostURLError, <<-MSG
Could not find post "#{@orig_post}" in tag 'post_url'.

Make sure the post exists and the name is correct.
eos
MSG
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/test_ansi.rb
Expand Up @@ -8,7 +8,7 @@ class TestAnsi < JekyllUnitTest
@subject = Jekyll::Utils::Ansi
end

Jekyll::Utils::Ansi::COLORS.each do |color, _val|
Jekyll::Utils::Ansi::COLORS.each_key do |color|
should "respond_to? #{color}" do
assert @subject.respond_to?(color)
end
Expand Down
1 change: 1 addition & 0 deletions test/test_utils.rb
@@ -1,3 +1,4 @@
# encoding: utf-8
# frozen_string_literal: true

require "helper"
Expand Down