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

Rubocop: 0.50.0 and Ruby 2.3 #3183

Merged
merged 3 commits into from
Sep 25, 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
23 changes: 20 additions & 3 deletions Library/.rubocop.yml
@@ -1,5 +1,5 @@
AllCops:
TargetRubyVersion: 2.0
TargetRubyVersion: 2.3
Exclude:
- '**/Casks/**/*'
- '**/vendor/**/*'
Expand Down Expand Up @@ -119,7 +119,7 @@ Style/Encoding:
Enabled: true

# dashes in filenames are typical
Style/FileName:
Naming/FileName:
Regex: !ruby/regexp /^[\w\@\-\+\.]+(\.rb)?$/

# falsely flags e.g. curl formatting arguments as format strings
Expand Down Expand Up @@ -189,8 +189,25 @@ Style/TrailingCommaInArguments:
EnforcedStyleForMultiline: comma

# we have too many variables like sha256 where this harms readability
Style/VariableNumber:
Naming/VariableNumber:
Enabled: false

Style/WordArray:
MinSize: 4

# we want to add this slowly and manually
Style/FrozenStringLiteralComment:
Enabled: false

# generally rescuing StandardError is fine
Lint/RescueWithoutErrorClass:
Enabled: false

# implicitly allow EOS as we use it everywhere
Copy link
Member

@DomT4 DomT4 Sep 22, 2017

Choose a reason for hiding this comment

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

Thanks for catching this. My $EDITOR's Rubocop plugin has already been driving me up the wall over EOS stuff.

Edit - Up the wall, not up to the wall.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can thank me for the fit I pitched about it lol

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what the issue is with EOS to be honest. It'd seem silly to get ultra-specific inside formulae at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

rubocop/rubocop#4467

I get the sense that whatever conversations there were about this didn't really occur in the open.

Copy link
Member

Choose a reason for hiding this comment

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

One nice thing is that in TextMate you get nested syntax highlighting inside heredocs if you use e.g. RUBY, but if it's just text, I'm also in favour of EOS.

Naming/HeredocDelimiterNaming:
Blacklist:
- END, EOD, EOF

# we output how to use interpolated strings too often
Lint/InterpolationCheck:
Enabled: false
4 changes: 2 additions & 2 deletions Library/Homebrew/.rubocop.yml
Expand Up @@ -42,12 +42,12 @@ Style/HashSyntax:
EnforcedStyle: ruby19_no_mixed_keys

# we won't change backward compatible method names
Style/MethodName:
Naming/MethodName:
Exclude:
- 'compat/**/*'

# we won't change backward compatible predicate names
Style/PredicateName:
Naming/PredicateName:
Exclude:
- 'compat/**/*'
NameWhitelist: is_32_bit?, is_64_bit?
9 changes: 1 addition & 8 deletions Library/Homebrew/.rubocop_todo.yml
Expand Up @@ -81,7 +81,7 @@ Security/MarshalLoad:
- 'utils/fork.rb'

# Offense count: 1
Style/AccessorMethodName:
Naming/AccessorMethodName:
Exclude:
- 'extend/ENV/super.rb'

Expand Down Expand Up @@ -136,10 +136,3 @@ Style/MutableConstant:
- 'formulary.rb'
- 'tab.rb'
- 'tap.rb'

# Offense count: 8
Style/OpMethod:
Exclude:
- 'dependencies.rb'
- 'install_renamed.rb'
- 'options.rb'
20 changes: 9 additions & 11 deletions Library/Homebrew/brew.rb
Expand Up @@ -105,18 +105,16 @@
possible_tap = OFFICIAL_CMD_TAPS.find { |_, cmds| cmds.include?(cmd) }
possible_tap = Tap.fetch(possible_tap.first) if possible_tap

if possible_tap && !possible_tap.installed?
brew_uid = HOMEBREW_BREW_FILE.stat.uid
tap_commands = []
if Process.uid.zero? && !brew_uid.zero?
tap_commands += %W[/usr/bin/sudo -u ##{brew_uid}]
end
tap_commands += %W[#{HOMEBREW_BREW_FILE} tap #{possible_tap}]
safe_system(*tap_commands)
exec HOMEBREW_BREW_FILE, cmd, *ARGV
else
odie "Unknown command: #{cmd}"
odie "Unknown command: #{cmd}" if !possible_tap || possible_tap.installed?

brew_uid = HOMEBREW_BREW_FILE.stat.uid
tap_commands = []
if Process.uid.zero? && !brew_uid.zero?
tap_commands += %W[/usr/bin/sudo -u ##{brew_uid}]
end
tap_commands += %W[#{HOMEBREW_BREW_FILE} tap #{possible_tap}]
safe_system(*tap_commands)
exec HOMEBREW_BREW_FILE, cmd, *ARGV
end
rescue UsageError => e
require "cmd/help"
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/cask_loader.rb
Expand Up @@ -56,7 +56,7 @@ def cask(header_token, **options, &block)

class FromURILoader < FromPathLoader
def self.can_load?(ref)
ref.to_s.match?(::URI.regexp)
ref.to_s.match?(::URI::DEFAULT_PARSER.make_regexp)
end

attr_reader :url
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/naked.rb
Expand Up @@ -16,7 +16,7 @@ def extract

def target_file
return @path.basename if @nested
URI.decode(File.basename(@cask.url.path))
CGI.unescape(File.basename(@cask.url.path))
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/dsl.rb
Expand Up @@ -161,7 +161,7 @@ def container(*args)
begin
DSL::Container.new(*args).tap do |container|
# TODO: remove this backward-compatibility section after removing nested_container
if container && container.nested
if container&.nested
artifacts[:nested_container] << Artifact::NestedContainer.new(cask, container.nested)
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/dsl/version.rb
Expand Up @@ -49,7 +49,7 @@ def conversion_method_name(left_divider, right_divider)
end
end

DIVIDERS.keys.each do |divider|
DIVIDERS.each_key do |divider|
define_divider_methods(divider)
end

Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cask/lib/hbc/installer.rb
Expand Up @@ -159,7 +159,7 @@ def extract_primary_container
odebug "Extracting primary container"

FileUtils.mkdir_p @cask.staged_path
container = if @cask.container && @cask.container.type
container = if @cask.container&.type
Container.from_type(@cask.container.type)
else
Container.for_path(@downloaded_path, @command)
Expand Down Expand Up @@ -361,7 +361,7 @@ def save_caskfile

savedir = @cask.metadata_subdir("Casks", timestamp: :now, create: true)
FileUtils.copy @cask.sourcefile_path, savedir
old_savedir.rmtree unless old_savedir.nil?
old_savedir&.rmtree
end

def uninstall
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/system_command.rb
Expand Up @@ -61,7 +61,7 @@ def process_options!
end

def assert_success
return if processed_status && processed_status.success?
return if processed_status&.success?
raise CaskCommandFailedError.new(command, processed_output[:stdout], processed_output[:stderr], processed_status)
end

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/caveats.rb
Expand Up @@ -163,7 +163,7 @@ def elisp_caveats

def plist_caveats
s = []
if f.plist || (keg && keg.plist_installed?)
if f.plist || (keg&.plist_installed?)
Copy link
Contributor

Choose a reason for hiding this comment

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

are the parentheses still needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

plist_domain = f.plist_path.basename(".plist")

# we readlink because this path probably doesn't exist since caveats
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/prune.rb
Expand Up @@ -55,7 +55,7 @@ def prune
else
n, d = ObserverPathnameExtension.counts
print "Pruned #{n} symbolic links "
print "and #{d} directories " if d > 0
print "and #{d} directories " if d.positive?
Copy link
Contributor

Choose a reason for hiding this comment

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

oh how gross. can we opt out of 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.

Use Python?

Copy link
Contributor

Choose a reason for hiding this comment

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

you first

puts "from #{HOMEBREW_PREFIX}"
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/search.rb
Expand Up @@ -67,7 +67,7 @@ def search

ohai "Searching blacklisted, migrated and deleted formulae..."
if reason = Homebrew::MissingFormula.reason(query, silent: true)
if count > 0
if count.positive?
puts
puts "If you meant #{query.inspect} specifically:"
end
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cmd/tap-info.rb
Expand Up @@ -64,10 +64,10 @@ def print_tap_info(taps)
if tap.installed?
info += tap.pinned? ? "pinned" : "unpinned"
info += ", private" if tap.private?
if (formula_count = tap.formula_files.size) > 0
if (formula_count = tap.formula_files.size).positive?
info += ", #{Formatter.pluralize(formula_count, "formula")}"
end
if (command_count = tap.command_files.size) > 0
if (command_count = tap.command_files.size).positive?
info += ", #{Formatter.pluralize(command_count, "command")}"
end
info += ", no formulae/commands" if (formula_count + command_count).zero?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/unlinkapps.rb
Expand Up @@ -77,7 +77,7 @@ def unlinkapps_from_dir(target_dir, opts = {})
def unlinkapps_unlink?(target_app, opts = {})
# Skip non-symlinks and symlinks that don't point into the Homebrew prefix.
app = target_app.readlink.to_s if target_app.symlink?
return false unless app && app.start_with?(*UNLINKAPPS_PREFIXES)
return false unless app&.start_with?(*UNLINKAPPS_PREFIXES)

if opts.fetch(:prune, false)
!File.exist?(app) # Remove only broken symlinks in prune mode.
Expand Down
6 changes: 4 additions & 2 deletions Library/Homebrew/constants.rb
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# RuboCop version used for `brew style` and `brew cask style`
HOMEBREW_RUBOCOP_VERSION = "0.49.1".freeze
HOMEBREW_RUBOCOP_CASK_VERSION = "~> 0.13.1".freeze # has to be updated when RuboCop version changes
HOMEBREW_RUBOCOP_VERSION = "0.50.0"
HOMEBREW_RUBOCOP_CASK_VERSION = "~> 0.14.2" # has to be updated when RuboCop version changes
2 changes: 1 addition & 1 deletion Library/Homebrew/debrew.rb
Expand Up @@ -57,7 +57,7 @@ def self.choose
input.chomp!

i = input.to_i
if i > 0
if i.positive?
choice = menu.entries[i - 1]
else
possible = menu.entries.find_all { |e| e.name.start_with?(input) }
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/debrew/irb.rb
Expand Up @@ -16,7 +16,7 @@ def start_within(binding)
workspace = WorkSpace.new(binding)
irb = Irb.new(workspace)

@CONF[:IRB_RC].call(irb.context) if @CONF[:IRB_RC]
@CONF[:IRB_RC]&.call(irb.context)
@CONF[:MAIN_CONTEXT] = irb.context

trap("SIGINT") do
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/dependency.rb
Expand Up @@ -51,7 +51,7 @@ def missing_options(inherited_options)
end

def modify_build_environment
env_proc.call unless env_proc.nil?
env_proc&.call
end

def inspect
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/dev-cmd/audit.rb
Expand Up @@ -358,7 +358,7 @@ def audit_file
end
valid_alias_names = [alias_name_major, alias_name_major_minor]

if formula.tap && !formula.tap.core_tap?
unless formula.tap&.core_tap?
versioned_aliases.map! { |a| "#{formula.tap}/#{a}" }
valid_alias_names.map! { |a| "#{formula.tap}/#{a}" }
end
Expand Down Expand Up @@ -707,7 +707,7 @@ def audit_specs
end

stable = formula.stable
case stable && stable.url
case stable&.url
when /[\d\._-](alpha|beta|rc\d)/
matched = Regexp.last_match(1)
version_prefix = stable.version.to_s.sub(/\d+$/, "")
Expand Down Expand Up @@ -1018,7 +1018,7 @@ def line_problems(line, _lineno)
def audit_reverse_migration
# Only enforce for new formula being re-added to core and official taps
return unless @strict
return unless formula.tap && formula.tap.official?
return unless formula.tap&.official?
return unless formula.tap.tap_migrations.key?(formula.name)

problem <<-EOS.undent
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/dev-cmd/bottle.rb
Expand Up @@ -47,7 +47,7 @@
<% elsif cellar != BottleSpecification::DEFAULT_CELLAR %>
cellar "<%= cellar %>"
<% end %>
<% if rebuild > 0 %>
<% if rebuild.positive? %>
rebuild <%= rebuild %>
<% end %>
<% checksums.each do |checksum_type, checksum_values| %>
Expand Down Expand Up @@ -186,7 +186,7 @@ def bottle_formula(f)
ohai "Determining #{f.full_name} bottle rebuild..."
versions = FormulaVersions.new(f)
rebuilds = versions.bottle_version_map("origin/master")[f.pkg_version]
rebuilds.pop if rebuilds.last.to_i > 0
rebuilds.pop if rebuilds.last.to_i.positive?
rebuild = rebuilds.empty? ? 0 : rebuilds.max.to_i + 1
end

Expand Down Expand Up @@ -283,7 +283,7 @@ def bottle_formula(f)
raise
ensure
ignore_interrupts do
original_tab.write if original_tab
original_tab&.write
unless ARGV.include? "--skip-relocation"
keg.replace_placeholders_with_locations changed_files
end
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/dev-cmd/bump-formula-pr.rb
Expand Up @@ -89,7 +89,7 @@ def fetch_pull_requests(formula)

def check_for_duplicate_pull_requests(formula)
pull_requests = fetch_pull_requests(formula)
return unless pull_requests && !pull_requests.empty?
return unless pull_requests&.empty?
duplicates_message = <<-EOS.undent
These open pull requests may be duplicates:
#{pull_requests.map { |pr| "#{pr["title"]} #{pr["html_url"]}" }.join("\n")}
Expand Down Expand Up @@ -124,7 +124,7 @@ def bump_formula_pr
Formula.each do |f|
if is_devel && f.devel && f.devel.url && f.devel.url.match(base_url)
guesses << f
elsif f.stable && f.stable.url && f.stable.url.match(base_url)
elsif f.stable&.url && f.stable.url.match(base_url)
guesses << f
end
end
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/dev-cmd/pull.rb
Expand Up @@ -69,13 +69,13 @@ def pull
tap = nil

ARGV.named.each do |arg|
if arg.to_i > 0
if arg.to_i.positive?
issue = arg
url = "https://github.com/Homebrew/homebrew-core/pull/#{arg}"
tap = CoreTap.instance
elsif (testing_match = arg.match %r{/job/Homebrew.*Testing/(\d+)/})
tap = ARGV.value("tap")
tap = if tap && tap.start_with?("homebrew/")
tap = if tap&.start_with?("homebrew/")
Tap.fetch("homebrew", tap.strip_prefix("homebrew/"))
elsif tap
odie "Tap option did not start with \"homebrew/\": #{tap}"
Expand Down Expand Up @@ -350,7 +350,7 @@ def files_changed_in_patch(patchfile, tap)
files << Regexp.last_match(1) if line =~ %r{^\+\+\+ b/(.*)}
end
files.each do |file|
if tap && tap.formula_file?(file)
if tap&.formula_file?(file)
formula_name = File.basename(file, ".rb")
formulae << formula_name unless formulae.include?(formula_name)
else
Expand Down
4 changes: 1 addition & 3 deletions Library/Homebrew/dev-cmd/release-notes.rb
Expand Up @@ -10,10 +10,8 @@ module Homebrew

def release_notes
previous_tag = ARGV.named.first
unless previous_tag
previous_tag = Utils.popen_read("git tag --list --sort=-version:refname")
previous_tag ||= Utils.popen_read("git tag --list --sort=-version:refname")
.lines.first.chomp
end
odie "Could not find any previous tags!" unless previous_tag

end_ref = ARGV.named[1] || "origin/master"
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/diagnostic.rb
Expand Up @@ -522,7 +522,7 @@ def check_for_gettext
homebrew_owned = @found.all? do |path|
Pathname.new(path).realpath.to_s.start_with? "#{HOMEBREW_CELLAR}/gettext"
end
return if gettext && gettext.linked_keg.directory? && homebrew_owned
return if gettext&.linked_keg&.directory? && homebrew_owned

inject_file_list @found, <<-EOS.undent
gettext files detected at a system prefix.
Expand All @@ -540,7 +540,7 @@ def check_for_iconv
rescue
nil
end
if libiconv && libiconv.linked_keg.directory?
if libiconv&.linked_keg&.directory?
unless libiconv.keg_only?
<<-EOS.undent
A libiconv formula is installed and linked.
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/exceptions.rb
Expand Up @@ -416,7 +416,7 @@ def dump

puts

if issues && !issues.empty?
unless issues&.empty?
puts "These open issues may also help:"
puts issues.map { |i| "#{i["title"]} #{i["html_url"]}" }.join("\n")
end
Expand Down