Skip to content

Commit

Permalink
Merge pull request #10256 from SeekingMeaning/unless_multiple_conditions
Browse files Browse the repository at this point in the history
rubocops: add `unless_multiple_conditions`
  • Loading branch information
SeekingMeaning committed Jan 9, 2021
2 parents c8e2f34 + 2a80427 commit c8afe19
Show file tree
Hide file tree
Showing 32 changed files with 221 additions and 61 deletions.
2 changes: 1 addition & 1 deletion Library/Homebrew/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def initialize(formula, options, args:)
def post_superenv_hacks
# Only allow Homebrew-approved directories into the PATH, unless
# a formula opts-in to allowing the user's path.
return unless formula.env.userpaths? || reqs.any? { |rq| rq.env.userpaths? }
return if !formula.env.userpaths? && reqs.none? { |rq| rq.env.userpaths? }

ENV.userpaths!
end
Expand Down
9 changes: 5 additions & 4 deletions Library/Homebrew/cask/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def check_untrusted_pkg
return if tap.nil?
return if tap.user != "Homebrew"

return unless cask.artifacts.any? { |k| k.is_a?(Artifact::Pkg) && k.stanza_options.key?(:allow_untrusted) }
return if cask.artifacts.none? { |k| k.is_a?(Artifact::Pkg) && k.stanza_options.key?(:allow_untrusted) }

add_error "allow_untrusted is not permitted in official Homebrew Cask taps"
end
Expand Down Expand Up @@ -472,7 +472,7 @@ def check_token_valid
add_error "cask token should only contain lowercase alphanumeric characters and hyphens"
end

return unless cask.token.start_with?("-") || cask.token.end_with?("-")
return if !cask.token.start_with?("-") && !cask.token.end_with?("-")

add_error "cask token should not have leading or trailing hyphens"
end
Expand All @@ -498,7 +498,8 @@ def check_token_bad_words

add_warning "cask token mentions architecture" if token.end_with? "x86", "32_bit", "x86_64", "64_bit"

return unless token.end_with?("cocoa", "qt", "gtk", "wx", "java") && %w[cocoa qt gtk wx java].exclude?(token)
frameworks = %w[cocoa qt gtk wx java]
return if frameworks.include?(token) || !token.end_with?(*frameworks)

add_warning "cask token mentions framework"
end
Expand All @@ -517,7 +518,7 @@ def core_formula_url
end

def check_download
return unless download && cask.url
return if download.blank? || cask.url.blank?

odebug "Auditing download"
download.fetch
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/cmd/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def self.upgrade_casks(
end
else
casks.select do |cask|
raise CaskNotInstalledError, cask unless cask.installed? || force
raise CaskNotInstalledError, cask if !cask.installed? && !force

cask.outdated?(greedy: true)
end
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ def save_config_file
def uninstall
oh1 "Uninstalling Cask #{Formatter.identifier(@cask)}"
uninstall_artifacts(clear: true)
remove_config_file unless reinstall? || upgrade?
remove_config_file if !reinstall? && !upgrade?
purge_versioned_files
purge_caskroom_path if force?
end
Expand All @@ -435,7 +435,7 @@ def backup
end

def restore_backup
return unless backup_path.directory? && backup_metadata_path.directory?
return if !backup_path.directory? || !backup_metadata_path.directory?

Pathname.new(@cask.staged_path).rmtree if @cask.staged_path.exist?
Pathname.new(@cask.metadata_versioned_path).rmtree if @cask.metadata_versioned_path.exist?
Expand Down
5 changes: 1 addition & 4 deletions Library/Homebrew/cask/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ def metadata_timestamped_path(version: self.version, timestamp: :latest, create:

def metadata_subdir(leaf, version: self.version, timestamp: :latest, create: false)
raise CaskError, "Cannot create metadata subdir when timestamp is :latest." if create && timestamp == :latest

unless leaf.respond_to?(:empty?) && !leaf.empty?
raise CaskError, "Cannot create metadata subdir for empty leaf."
end
raise CaskError, "Cannot create metadata subdir for empty leaf." if !leaf.respond_to?(:empty?) || leaf.empty?

parent = metadata_timestamped_path(version: version, timestamp: timestamp, create: create)

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/caveats.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def function_completion_caveats(shell)

completion_installed = keg.completion_installed?(shell)
functions_installed = keg.functions_installed?(shell)
return unless completion_installed || functions_installed
return if !completion_installed && !functions_installed

installed = []
installed << "completions" if completion_installed
Expand Down
5 changes: 2 additions & 3 deletions Library/Homebrew/cmd/migrate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ def migrate

args.named.to_resolved_formulae.each do |f|
if f.oldname
unless (rack = HOMEBREW_CELLAR/f.oldname).exist? && !rack.subdirs.empty?
raise NoSuchKegError, f.oldname
end
rack = HOMEBREW_CELLAR/f.oldname
raise NoSuchKegError, f.oldname if !rack.exist? || rack.subdirs.empty?
raise "#{rack} is a symlink" if rack.symlink?
end

Expand Down
4 changes: 1 addition & 3 deletions Library/Homebrew/debrew.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ def self.debrew
end

def self.debug(e)
original_raise(e) unless active? &&
debugged_exceptions.add?(e) &&
try_lock
original_raise(e) if !active? || !debugged_exceptions.add?(e) || !try_lock

begin
puts e.backtrace.first.to_s
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/deprecate_disable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ module DeprecateDisable
}.freeze

def deprecate_disable_info(formula)
return unless formula.deprecated? || formula.disabled?

if formula.deprecated?
type = :deprecated
reason = formula.deprecation_reason
else
elsif formula.disabled?
type = :disabled
reason = formula.disable_reason
else
return
end

reason = DEPRECATE_DISABLE_REASONS[reason] if DEPRECATE_DISABLE_REASONS.key? reason
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/dev-cmd/bottle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def keg_contain?(string, keg, ignores, formula_and_runtime_deps_names = nil, arg
end
end

next unless args.verbose? && !text_matches.empty?
next if !args.verbose? || text_matches.empty?

print_filename.call(string, file)
text_matches.first(MAXIMUM_STRING_MATCHES).each do |match, offset|
Expand All @@ -190,7 +190,7 @@ def keg_contain?(string, keg, ignores, formula_and_runtime_deps_names = nil, arg
def keg_contain_absolute_symlink_starting_with?(string, keg, args:)
absolute_symlinks_start_with_string = []
keg.find do |pn|
next unless pn.symlink? && (link = pn.readlink).absolute?
next if !pn.symlink? || !(link = pn.readlink).absolute?

absolute_symlinks_start_with_string << pn if link.to_s.start_with?(string)
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/dev-cmd/bump-unversioned-casks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def self.bump_unversioned_cask(cask, state:, dry_run:)

unversioned_cask_checker = UnversionedCaskChecker.new(cask)

unless unversioned_cask_checker.single_app_cask? || unversioned_cask_checker.single_pkg_cask?
if !unversioned_cask_checker.single_app_cask? && !unversioned_cask_checker.single_pkg_cask?
opoo "Skipping, not a single-app or PKG cask."
return
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,7 @@ def check_cask_staging_location

add_info "Homebrew Cask Staging Location", user_tilde(path.to_s)

return unless path.exist? && !path.writable?
return if !path.exist? || path.writable?

<<~EOS
The staging path #{user_tilde(path.to_s)} is not writable by the current user.
Expand Down
6 changes: 2 additions & 4 deletions Library/Homebrew/download_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ def fetch

version.update_commit(last_commit) if head?

return unless @ref_type == :tag
return unless @revision && current_revision
return if current_revision == @revision
return if @ref_type != :tag || @revision.blank? || current_revision.blank? || current_revision == @revision

raise <<~EOS
#{@ref} tag should be #{@revision}
Expand Down Expand Up @@ -827,7 +825,7 @@ def config_repo
end

def update_repo
return unless @ref_type == :branch || !ref?
return if @ref_type != :branch && ref?

if !shallow_clone? && shallow_dir?
command! "git",
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/os/mac/keg_relocate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def mach_o_files
mach_o_files = []
path.find do |pn|
next if pn.symlink? || pn.directory?
next unless pn.dylib? || pn.mach_o_bundle? || pn.mach_o_executable?
next if !pn.dylib? && !pn.mach_o_bundle? && !pn.mach_o_executable?
# if we've already processed a file, ignore its hardlinks (which have the same dev ID and inode)
# this prevents relocations from being performed on a binary more than once
next unless hardlinks.add? [pn.stat.dev, pn.stat.ino]
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/extend/pathname.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ def install(*sources)

sig { params(src: T.any(String, Pathname), new_basename: String).void }
def install_p(src, new_basename)
raise Errno::ENOENT, src.to_s unless File.symlink?(src) || File.exist?(src)

src = Pathname(src)
raise Errno::ENOENT, src.to_s if !src.symlink? && !src.exist?

dst = join(new_basename)
dst = yield(src, dst) if block_given?
return unless dst
Expand Down
10 changes: 5 additions & 5 deletions Library/Homebrew/fetch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ module Fetch
def fetch_bottle?(f, args:)
bottle = f.bottle

return true if args.force_bottle? && bottle
return false unless bottle && f.pour_bottle?
return false if args.build_from_source_formulae.include?(f.full_name)
return false unless bottle.compatible_locations?
return true if args.force_bottle? && bottle.present?

true
bottle.present? &&
f.pour_bottle? &&
args.build_from_source_formulae.exclude?(f.full_name) &&
bottle.compatible_locations?
end
end
end
6 changes: 3 additions & 3 deletions Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,13 @@ def determine_active_spec(requested)
end

def validate_attributes!
raise FormulaValidationError.new(full_name, :name, name) if name.blank? || name =~ /\s/
raise FormulaValidationError.new(full_name, :name, name) if name.blank? || name.match?(/\s/)

url = active_spec.url
raise FormulaValidationError.new(full_name, :url, url) if url.blank? || url =~ /\s/
raise FormulaValidationError.new(full_name, :url, url) if url.blank? || url.match?(/\s/)

val = version.respond_to?(:to_str) ? version.to_str : version
return unless val.blank? || val =~ /\s/
return if val.present? && !val.match?(/\s/)

raise FormulaValidationError.new(full_name, :version, val)
end
Expand Down
12 changes: 6 additions & 6 deletions Library/Homebrew/formula_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,12 @@ def pour_bottle?(output_warning: false)
sig { params(dep: Formula, build: BuildOptions).returns(T::Boolean) }
def install_bottle_for?(dep, build)
return pour_bottle? if dep == formula
return false if @build_from_source_formulae.include?(dep.full_name)
return false unless dep.bottle && dep.pour_bottle?
return false unless build.used_options.empty?
return false unless dep.bottle&.compatible_locations?

true
@build_from_source_formulae.exclude?(dep.full_name) &&
dep.bottle.present? &&
dep.pour_bottle? &&
build.used_options.empty? &&
dep.bottle&.compatible_locations?
end

sig { void }
Expand Down Expand Up @@ -803,7 +803,7 @@ def finish
keg = Keg.new(formula.prefix)
link(keg)

fix_dynamic_linkage(keg) unless @poured_bottle && formula.bottle_specification.skip_relocation?
fix_dynamic_linkage(keg) if !@poured_bottle || !formula.bottle_specification.skip_relocation?

if build_bottle?
ohai "Not running post_install as we're building a bottle"
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/formula_pin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def path
def pin_at(version)
HOMEBREW_PINNED_KEGS.mkpath
version_path = @f.rack/version
path.make_relative_symlink(version_path) unless pinned? || !version_path.exist?
path.make_relative_symlink(version_path) if !pinned? && version_path.exist?
end

def pin
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/keg_relocate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def libexec

def text_files
text_files = []
return text_files unless which("file") && which("xargs")
return text_files if !which("file") || !which("xargs")

# file has known issues with reading files on other locales. Has
# been fixed upstream for some time, but a sufficiently new enough
Expand Down
3 changes: 1 addition & 2 deletions Library/Homebrew/linkage_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,8 @@ def dylib_found_via_dlopen(dylib)
def check_formula_deps
filter_out = proc do |dep|
next true if dep.build?
next false unless dep.optional? || dep.recommended?

formula.build.without?(dep)
(dep.optional? || dep.recommended?) && formula.build.without?(dep)
end

declared_deps_full_names = formula.deps
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/livecheck/strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def from_url(url, livecheck_strategy: nil, url_provided: nil, regex_provided: ni
if strategy == PageMatch
# Only treat the `PageMatch` strategy as usable if a regex is
# present in the `livecheck` block
next unless regex_provided || block_provided
next if !regex_provided && !block_provided
elsif strategy.const_defined?(:PRIORITY) &&
!strategy::PRIORITY.positive? &&
from_symbol(livecheck_strategy) != strategy
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/lock_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def with_lock
private

def create_lockfile
return unless @lockfile.nil? || @lockfile.closed?
return if @lockfile.present? && !@lockfile.closed?

@lockfile = @path.open(File::RDWR | File::CREAT)
@lockfile.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/rubocops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@
require "rubocops/keg_only"
require "rubocops/version"
require "rubocops/deprecate_disable"
require "rubocops/unless_multiple_conditions"

require "rubocops/rubocop-cask"
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/extend/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def find_method_def(node, method_name = nil)

node.each_child_node(:def) do |def_node|
def_method_name = method_name(def_node)
next unless method_name == def_method_name || method_name.nil?
next if method_name != def_method_name && method_name.present?

@offensive_node = def_node
@offense_source_range = def_node.source_range
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/patches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def patch_problems(patch)
end

def inline_patch_problems(patch)
return unless patch_data?(patch) && !patch_end?
return if !patch_data?(patch) || patch_end?

offending_node(patch)
problem "patch is missing '__END__'"
Expand Down
39 changes: 39 additions & 0 deletions Library/Homebrew/rubocops/unless_multiple_conditions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# typed: strict
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks that `unless` is not used with multiple conditions.
#
# @api private
class UnlessMultipleConditions < Cop
extend T::Sig

MSG = "Avoid using `unless` with multiple conditions."

sig { params(node: RuboCop::AST::IfNode).void }
def on_if(node)
return if !node.unless? || (!node.condition.and_type? && !node.condition.or_type?)

add_offense(node, location: node.condition.source_range.with(begin_pos: node.loc.keyword.begin_pos))
end

sig { params(node: RuboCop::AST::IfNode).returns(T.proc.params(arg0: RuboCop::Cop::Corrector).void) }
def autocorrect(node)
lambda do |corrector|
corrector.replace(node.loc.keyword, "if")
corrector.replace(node.condition.loc.operator, node.condition.inverse_operator)
[node.condition.lhs, node.condition.rhs].each do |subcondition|
if !subcondition.source.start_with?("(") || !subcondition.source.end_with?(")")
corrector.insert_before(subcondition, "(")
corrector.insert_after(subcondition, ")")
end
corrector.insert_before(subcondition, "!")
end
end
end
end
end
end
end
2 changes: 1 addition & 1 deletion Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def git_last_commit_date
# e.g. `https://github.com/user/homebrew-repo/issues`
sig { returns(T.nilable(String)) }
def issues_url
return unless official? || !custom_remote?
return if !official? && custom_remote?

"#{default_remote}/issues"
end
Expand Down

0 comments on commit c8afe19

Please sign in to comment.