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

rubocops: add unless_multiple_conditions #10256

Merged
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: 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