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

Migrate style exceptions to homebrew/core #9326

Merged
merged 15 commits into from Dec 1, 2020
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
3 changes: 3 additions & 0 deletions Library/Homebrew/cli/args.rbi
Expand Up @@ -129,6 +129,9 @@ module Homebrew
sig { returns(T.nilable(T::Boolean)) }
def markdown?; end

sig { returns(T.nilable(T::Boolean)) }
def reset_cache?; end

sig { returns(T.nilable(String)) }
def tag; end

Expand Down
8 changes: 7 additions & 1 deletion Library/Homebrew/dev-cmd/style.rb
Expand Up @@ -27,6 +27,8 @@ def style_args
description: "Fix style violations automatically using RuboCop's auto-correct feature."
switch "--display-cop-names",
description: "Include the RuboCop cop name for each violation in the output."
switch "--reset-cache",
description: "Reset the RuboCop cache."
comma_array "--only-cops",
description: "Specify a comma-separated <cops> list to check for violations of only the "\
"listed RuboCop cops."
Expand All @@ -51,7 +53,11 @@ def style
except_cops = args.except_cops

options = {
fix: args.fix?, display_cop_names: args.display_cop_names?, debug: args.debug?, verbose: args.verbose?
fix: args.fix?,
display_cop_names: args.display_cop_names?,
reset_cache: args.reset_cache?,
debug: args.debug?,
verbose: args.verbose?,
}
if only_cops
options[:only_cops] = only_cops
Expand Down
7 changes: 1 addition & 6 deletions Library/Homebrew/rubocops/components_order.rb
Expand Up @@ -11,11 +11,6 @@ module FormulaAudit
# - `component_precedence_list` has component hierarchy in a nested list
# where each sub array contains components' details which are at same precedence level
class ComponentsOrder < FormulaCop
# `aspell`: options and resources should be grouped by language
COMPONENT_ALLOWLIST = %w[
aspell
].freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
component_precedence_list = [
[{ name: :include, type: :method_call }],
Expand Down Expand Up @@ -234,7 +229,7 @@ def check_order(component_precedence_list, body_node)

# Method to format message for reporting component precedence violations.
def component_problem(c1, c2)
return if COMPONENT_ALLOWLIST.include?(@formula_name)
return if tap_style_exception? :components_order_exceptions

problem "`#{format_component(c1)}` (line #{line_number(c1)}) " \
"should be put before `#{format_component(c2)}` " \
Expand Down
6 changes: 1 addition & 5 deletions Library/Homebrew/rubocops/conflicts.rb
Expand Up @@ -12,10 +12,6 @@ class Conflicts < FormulaCop
MSG = "Versioned formulae should not use `conflicts_with`. " \
"Use `keg_only :versioned_formula` instead."

ALLOWLIST = %w[
bash-completion@2
].freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
find_method_calls_by_name(body_node, :conflicts_with).each do |conflicts_with_call|
next unless parameters(conflicts_with_call).last.respond_to? :values
Expand All @@ -35,7 +31,7 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)

return unless versioned_formula?

problem MSG if !ALLOWLIST.include?(@formula_name) &&
problem MSG if !tap_style_exception?(:versioned_formulae_conflicts_allowlist) &&
method_called_ever?(body_node, :conflicts_with)
end

Expand Down
27 changes: 27 additions & 0 deletions Library/Homebrew/rubocops/extend/formula.rb
Expand Up @@ -35,6 +35,7 @@ def on_class(node)

class_node, parent_class_node, @body = *node
@formula_name = Pathname.new(@file_path).basename(".rb").to_s
@tap_style_exceptions = nil
audit_formula(node, class_node, parent_class_node, @body)
end

Expand Down Expand Up @@ -472,6 +473,32 @@ def formula_tap
match_obj[1]
end

# Returns whether the given formula exists in the given style exception list.
# Defaults to the current formula being checked.
def tap_style_exception?(list, formula = nil)
if @tap_style_exceptions.nil? && !formula_tap.nil?
@tap_style_exceptions = {}

style_exceptions_dir = "#{File.dirname(File.dirname(@file_path))}/style_exceptions/*.json"
Pathname.glob(style_exceptions_dir).each do |exception_file|
list_name = exception_file.basename.to_s.chomp(".json").to_sym
Rylan12 marked this conversation as resolved.
Show resolved Hide resolved
list_contents = begin
JSON.parse exception_file.read
rescue JSON::ParserError
nil
end
next if list_contents.nil? || list_contents.count.zero?

@tap_style_exceptions[list_name] = list_contents
end
end

return false if @tap_style_exceptions.nil? || @tap_style_exceptions.count.zero?
return false unless @tap_style_exceptions.key? list

@tap_style_exceptions[list].include?(formula || @formula_name)
end

private

def formula_class?(node)
Expand Down
22 changes: 1 addition & 21 deletions Library/Homebrew/rubocops/lines.rb
Expand Up @@ -623,33 +623,13 @@ module FormulaAuditStrict
#
# @api private
class MakeCheck < FormulaCop
MAKE_CHECK_ALLOWLIST = %w[
beecrypt
ccrypt
git
gmp
gnupg
gnupg@1.4
google-sparsehash
jemalloc
jpeg-turbo
mpfr
nettle
open-mpi
openssl@1.1
pcre
protobuf
wolfssl
xz
].freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
return if formula_tap != "homebrew-core"

# Avoid build-time checks in homebrew/core
find_every_method_call_by_name(body_node, :system).each do |method|
next if @formula_name.start_with?("lib")
next if MAKE_CHECK_ALLOWLIST.include?(@formula_name)
next if tap_style_exception? :make_check_allowlist

params = parameters(method)
next unless node_equals?(params[0], "make")
Expand Down
4 changes: 1 addition & 3 deletions Library/Homebrew/rubocops/livecheck.rb
Expand Up @@ -218,10 +218,8 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
#
# @api private
class LivecheckRegexCaseInsensitive < FormulaCop
REGEX_CASE_SENSITIVE_ALLOWLIST = %w[].freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
return if REGEX_CASE_SENSITIVE_ALLOWLIST.include?(@formula_name)
return if tap_style_exception? :regex_case_sensitive_allowlist

livecheck_node = find_block(body_node, :livecheck)
return if livecheck_node.blank?
Expand Down
45 changes: 2 additions & 43 deletions Library/Homebrew/rubocops/urls.rb
Expand Up @@ -10,47 +10,6 @@ module FormulaAudit
#
# @api private
class Urls < FormulaCop
# These are parts of URLs that look like binaries but actually aren't.
NOT_A_BINARY_URL_PREFIX_ALLOWLIST = %w[
https://downloads.sourceforge.net/project/astyle/astyle/
https://downloads.sourceforge.net/project/bittwist/
https://downloads.sourceforge.net/project/launch4j/
https://github.com/ChrisJohnsen/tmux-MacOSX-pasteboard/archive/
https://github.com/obihann/archey-osx
https://github.com/sindresorhus/macos-wallpaper/archive/
https://raw.githubusercontent.com/liyanage/macosx-shell-scripts/
https://osxbook.com/book/bonus/chapter8/core/download/gcore
https://naif.jpl.nasa.gov/pub/naif/toolkit/C/MacIntel_OSX_AppleC_64bit/packages/
https://artifacts.videolan.org/x264/release-macos/
https://github.com/vifm/vifm/releases/download/v0.11/vifm-osx-0.11.tar.bz2
].freeze

# These are formulae that, sadly, require an upstream binary to bootstrap.
BINARY_BOOTSTRAP_FORMULA_URLS_ALLOWLIST = %w[
clozure-cl
crystal
fpc
ghc
ghc@8.6
ghc@8.8
go
go@1.9
go@1.10
go@1.11
go@1.12
go@1.13
go@1.14
haskell-stack
ldc
mlton
openjdk
openjdk@11
openjdk@8
pypy
sbcl
rust
].freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
urls = find_every_func_call_by_name(body_node, :url)
mirrors = find_every_func_call_by_name(body_node, :mirror)
Expand Down Expand Up @@ -281,8 +240,8 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
audit_urls(urls, /(darwin|macos|osx)/i) do |match, url|
next if @formula_name.include?(match.to_s.downcase)
next if url.match?(/.(patch|diff)(\?full_index=1)?$/)
next if NOT_A_BINARY_URL_PREFIX_ALLOWLIST.any? { |prefix| url.start_with?(prefix) }
next if BINARY_BOOTSTRAP_FORMULA_URLS_ALLOWLIST.include?(@formula_name)
next if tap_style_exception? :not_a_binary_url_prefix_allowlist
next if tap_style_exception? :binary_bootstrap_formula_urls_allowlist

problem "#{url} looks like a binary package, not a source archive; " \
"homebrew/core is source-only."
Expand Down
89 changes: 14 additions & 75 deletions Library/Homebrew/rubocops/uses_from_macos.rb
Expand Up @@ -6,82 +6,20 @@
module RuboCop
module Cop
module FormulaAudit
# This cop audits formulae that are keg-only because they are provided by macos.
class ProvidedByMacos < FormulaCop
def audit_formula(_node, _class_node, _parent_class_node, body_node)
find_method_with_args(body_node, :keg_only, :provided_by_macos) do
unless tap_style_exception? :provided_by_macos_formulae
problem "Formulae that are `keg_only :provided_by_macos` should be added to "\
Rylan12 marked this conversation as resolved.
Show resolved Hide resolved
"`style_exceptions/provided_by_macos_formulae.json`"
end
end
end
end

# This cop audits `uses_from_macos` dependencies in formulae.
class UsesFromMacos < FormulaCop
# Generate with:
#
# ```
# brew ruby -e 'puts Formula.select {|f| f.keg_only_reason&.provided_by_macos? }.map(&:name).sort.join("\n")'
# ```
#
# Not done at runtime as it's too slow and RuboCop doesn't have access.
PROVIDED_BY_MACOS_FORMULAE = %w[
apr
bc
bison
bzip2
cups
curl
dyld-headers
ed
expat
file-formula
flex
gcore
gnu-getopt
icu4c
krb5
libarchive
libedit
libffi
libiconv
libpcap
libressl
libxml2
libxslt
llvm
lsof
m4
ncompress
ncurses
net-snmp
openldap
openlibm
pod2man
rpcgen
ruby
sqlite
ssh-copy-id
swift
tcl-tk
texinfo
unifdef
unzip
zip
zlib
].freeze

# These formulae aren't `keg_only :provided_by_macos` but are provided by
# macOS (or very similarly, e.g. OpenSSL where system provides LibreSSL).
# TODO: consider making some of these keg-only.
ALLOWED_USES_FROM_MACOS_DEPS = (PROVIDED_BY_MACOS_FORMULAE + %w[
bash
cpio
expect
groff
gzip
openssl
openssl@1.1
perl
php
python
python@3
rsync
vim
xz
zsh
]).freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
find_method_with_args(body_node, :uses_from_macos, /^"(.+)"/).each do |method|
dep = if parameters(method).first.instance_of?(RuboCop::AST::StrNode)
Expand All @@ -90,7 +28,8 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
parameters(method).first.keys.first
end

next if ALLOWED_USES_FROM_MACOS_DEPS.include?(string_content(dep))
next if tap_style_exception? :provided_by_macos_formulae, string_content(dep)
next if tap_style_exception? :non_keg_only_provided_by_macos_formulae, string_content(dep)

problem "`uses_from_macos` should only be used for macOS dependencies, not #{string_content(dep)}."
end
Expand Down
6 changes: 5 additions & 1 deletion Library/Homebrew/style.rb
Expand Up @@ -40,6 +40,7 @@ def check_style_impl(files, output_type,
fix: false,
except_cops: nil, only_cops: nil,
display_cop_names: false,
reset_cache: false,
debug: false, verbose: false)
raise ArgumentError, "Invalid output type: #{output_type.inspect}" unless [:print, :json].include?(output_type)

Expand All @@ -54,6 +55,7 @@ def check_style_impl(files, output_type,
fix: fix,
except_cops: except_cops, only_cops: only_cops,
display_cop_names: display_cop_names,
reset_cache: reset_cache,
debug: debug, verbose: verbose)
end

Expand All @@ -71,7 +73,7 @@ def check_style_impl(files, output_type,
end

def run_rubocop(files, output_type,
fix: false, except_cops: nil, only_cops: nil, display_cop_names: false,
fix: false, except_cops: nil, only_cops: nil, display_cop_names: false, reset_cache: false,
debug: false, verbose: false)
Homebrew.install_bundler_gems!
require "rubocop"
Expand Down Expand Up @@ -130,6 +132,8 @@ def run_rubocop(files, output_type,

cache_env = { "XDG_CACHE_HOME" => "#{HOMEBREW_CACHE}/style" }

FileUtils.rm_rf cache_env["XDG_CACHE_HOME"] if reset_cache

case output_type
when :print
args << "--debug" if debug
Expand Down