admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Merge pull request #3091 from GauthamGoli/audit_line_rubocop_part_3

Browse Source 
audit: Port line_problems to rubocop and add tests part 3
This commit is contained in:
Mike McQuaid 2017-09-05 18:10:02 +01:00 committed by GitHub
commit bf0ab938e7
7 changed files with 366 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
Library/Homebrew/dev-cmd/audit.rb
View File

@ -878,10 +878,6 @@ class FormulaAuditor
problem "Use spaces instead of tabs for indentation" if line =~ /^[ ]*\t/
if line.include?("ENV.x11")
problem "Use \"depends_on :x11\" instead of \"ENV.x11\""
end
if line.include?("ENV.java_cache")
problem "In-formula ENV.java_cache usage has been deprecated & should be removed."
end
@ -899,14 +895,6 @@ class FormulaAuditor
problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment"
end
if formula.name != "wine" && line =~ /ENV\.universal_binary/
problem "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated."
end
if line =~ /build\.universal\?/
problem "macOS has been 64-bit only so build.universal? is deprecated."
end
if line =~ /version == ['"]HEAD['"]/
problem "Use 'build.head?' instead of inspecting 'version'"
end
@ -947,12 +935,6 @@ class FormulaAuditor
problem "Use build instead of ARGV to check options"
end
problem "Use new-style option definitions" if line.include?("def options")
if line.end_with?("def test")
problem "Use new-style test definitions (test do)"
end
if line.include?("MACOS_VERSION")
problem "Use MacOS.version instead of MACOS_VERSION"
end
@ -966,11 +948,6 @@ class FormulaAuditor
problem "\"#{$&}\" is deprecated, use a comparison to MacOS.version instead"
end
if line =~ /skip_clean\s+:all/
problem "`skip_clean :all` is deprecated; brew no longer strips symbols\n" \
"\tPass explicit paths to prevent Homebrew from removing empty folders."
end
if line =~ /depends_on [A-Z][\w:]+\.new$/
problem "`depends_on` can take requirement classes instead of instances"
end
@ -1009,30 +986,6 @@ class FormulaAuditor
problem "Use `assert_match` instead of `assert ...include?`"
end
if line.include?('system "npm", "install"') && !line.include?("Language::Node") &&
formula.name !~ /^kibana(\@\d+(\.\d+)?)?$/
problem "Use Language::Node for npm install args"
end
if line.include?("fails_with :llvm")
problem "'fails_with :llvm' is now a no-op so should be removed"
end
if line =~ /system\s+['"](otool|install_name_tool|lipo)/ && formula.name != "cctools"
problem "Use ruby-macho instead of calling #{Regexp.last_match(1)}"
end
if formula.tap.to_s == "homebrew/core"
["OS.mac?", "OS.linux?"].each do |check|
next unless line.include?(check)
problem "Don't use #{check}; Homebrew/core only supports macOS"
end
end
if line =~ /((revision|version_scheme)\s+0)/
problem "'#{Regexp.last_match(1)}' should be removed"
end
return unless @strict
problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/

2
Library/Homebrew/rubocops/conflicts_cop.rb
View File

@ -18,7 +18,7 @@ module RuboCop
def audit_formula(_node, _class_node, _parent_class_node, body)
return unless versioned_formula?
problem MSG if !formula_file_name.start_with?(*WHITELIST) &&
problem MSG if !@formula_name.start_with?(*WHITELIST) &&
method_called_ever?(body, :conflicts_with)
end
end

35
Library/Homebrew/rubocops/extend/formula_cop.rb
View File

@ -4,16 +4,17 @@ require_relative "../../extend/string"
module RuboCop
module Cop
class FormulaCop < Cop
attr_accessor :file_path
@registry = Cop.registry
# This method is called by RuboCop and is the main entry point
def on_class(node)
file_path = processed_source.buffer.name
return unless file_path_allowed?(file_path)
@file_path = processed_source.buffer.name
return unless file_path_allowed?
return unless formula_class?(node)
return unless respond_to?(:audit_formula)
class_node, parent_class_node, @body = *node
@formula_name = class_name(class_node)
@formula_name = Pathname.new(@file_path).basename(".rb").to_s
audit_formula(node, class_node, parent_class_node, @body)
end
@ -100,19 +101,22 @@ module RuboCop
def find_method_with_args(node, method_name, *args)
methods = find_every_method_call_by_name(node, method_name)
methods.each do |method|
next unless parameters_passed?(method, *args)
yield method
yield method if parameters_passed?(method, *args)
end
end
# Matches a method with a receiver,
# EX: to match `Formula.factory(name)`
# call `find_instance_method_call(node, "Formula", :factory)`
# EX: to match `build.head?`
# call `find_instance_method_call(node, :build, :head?)`
# yields to a block with matching method node
def find_instance_method_call(node, instance, method_name)
methods = find_every_method_call_by_name(node, method_name)
methods.each do |method|
next unless method.receiver && method.receiver.const_name == instance
next if method.receiver.nil?
next if method.receiver.const_name != instance &&
method.receiver.method_name != instance
@offense_source_range = method.source_range
@offensive_node = method
yield method
@ -400,12 +404,7 @@ module RuboCop
# Returns true if the formula is versioned
def versioned_formula?
formula_file_name.include?("@") || @formula_name.match(/AT\d+/)
end
# Returns filename of the formula without the extension
def formula_file_name
File.basename(processed_source.buffer.name, ".rb")
@formula_name.include?("@")
end
# Returns printable component name
@ -414,6 +413,12 @@ module RuboCop
method_name(component_node) if component_node.def_type?
end
# Returns the formula tap
def formula_tap
return unless match_obj = @file_path.match(%r{/(homebrew-\w+)/})
match_obj[1]
end
def problem(msg)
add_offense(@offensive_node, @offense_source_range, msg)
end
@ -432,11 +437,11 @@ module RuboCop
class_node && class_names.include?(string_content(class_node))
end
def file_path_allowed?(file_path)
def file_path_allowed?
paths_to_exclude = [%r{/Library/Homebrew/compat/},
%r{/Library/Homebrew/test/}]
return true if file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs
file_path !~ Regexp.union(paths_to_exclude)
return true if @file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs
@file_path !~ Regexp.union(paths_to_exclude)
end
end
end

67
Library/Homebrew/rubocops/lines_cop.rb
View File

@ -21,7 +21,7 @@ module RuboCop
begin_pos = start_column(parent_class_node)
end_pos = end_column(class_node)
return unless begin_pos-end_pos != 3
problem "Use a space in class inheritance: class #{@formula_name} < #{class_name(parent_class_node)}"
problem "Use a space in class inheritance: class #{class_name(class_node)} < #{class_name(parent_class_node)}"
end
end
@ -67,8 +67,73 @@ module RuboCop
next unless block_arg.source.size>1
problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{block_arg.source}|\"."
end
[:rebuild, :version_scheme].each do |method_name|
find_method_with_args(body_node, method_name, 0) do
problem "'#{method_name} 0' should be removed"
end
end
[:mac?, :linux?].each do |method_name|
next unless formula_tap == "homebrew-core"
find_instance_method_call(body_node, "OS", method_name) do |check|
problem "Don't use #{check.source}; Homebrew/core only supports macOS"
end
end
find_method_with_args(body_node, :fails_with, :llvm) do
problem "'fails_with :llvm' is now a no-op so should be removed"
end
find_method_with_args(body_node, :system, /^(otool|install_name_tool|lipo)$/) do
next if @formula_name == "cctools"
problem "Use ruby-macho instead of calling #{@offensive_node.source}"
end
find_every_method_call_by_name(body_node, :system).each do |method_node|
# Skip Kibana: npm cache edge (see formula for more details)
next if @formula_name =~ /^kibana(\@\d+(\.\d+)?)?$/
first_param, second_param = parameters(method_node)
next if !node_equals?(first_param, "npm") ||
!node_equals?(second_param, "install")
offending_node(method_node)
problem "Use Language::Node for npm install args" unless languageNodeModule?(method_node)
end
if find_method_def(body_node, :test)
problem "Use new-style test definitions (test do)"
end
if find_method_def(body_node, :options)
problem "Use new-style option definitions"
end
find_method_with_args(body_node, :skip_clean, :all) do
problem <<-EOS.undent.chomp
`skip_clean :all` is deprecated; brew no longer strips symbols
Pass explicit paths to prevent Homebrew from removing empty folders.
EOS
end
find_instance_method_call(body_node, :build, :universal?) do
next if @formula_name == "wine"
problem "macOS has been 64-bit only since 10.6 so build.universal? is deprecated."
end
find_instance_method_call(body_node, "ENV", :universal_binary) do
problem "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated."
end
find_instance_method_call(body_node, "ENV", :x11) do
problem 'Use "depends_on :x11" instead of "ENV.x11"'
end
end
# Node Pattern search for Language::Node
def_node_search :languageNodeModule?, <<-EOS.undent
(const (const nil :Language) :Node)
EOS
end
end
end
end

4
Library/Homebrew/test/rubocops/conflicts_cop_spec.rb
View File

@ -22,7 +22,7 @@ describe RuboCop::Cop::FormulaAudit::Conflicts do
column: 2,
source: source }]
inspect_source(cop, source)
inspect_source(cop, source, "/homebrew-core/Formula/foo@2.0.rb")
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
@ -36,7 +36,7 @@ describe RuboCop::Cop::FormulaAudit::Conflicts do
desc 'Bar'
end
EOS
inspect_source(cop, source)
inspect_source(cop, source, "/homebrew-core/Formula/foo@2.0.rb")
expect(cop.offenses).to eq([])
end
end

12
Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb
View File

@ -37,7 +37,7 @@ describe RuboCop::Cop::FormulaAuditStrict::DescLength do
msg = <<-EOS.undent
Description is too long. "name: desc" should be less than 80 characters.
Length is calculated as Foo + desc. (currently 95)
Length is calculated as foo + desc. (currently 95)
EOS
expected_offenses = [{ message: msg,
severity: :convention,
@ -45,7 +45,7 @@ describe RuboCop::Cop::FormulaAuditStrict::DescLength do
column: 2,
source: source }]
inspect_source(cop, source)
inspect_source(cop, source, "/homebrew-core/Formula/foo.rb")
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
@ -62,7 +62,7 @@ describe RuboCop::Cop::FormulaAuditStrict::DescLength do
msg = <<-EOS.undent
Description is too long. "name: desc" should be less than 80 characters.
Length is calculated as Foo + desc. (currently 98)
Length is calculated as foo + desc. (currently 98)
EOS
expected_offenses = [{ message: msg,
severity: :convention,
@ -70,7 +70,7 @@ describe RuboCop::Cop::FormulaAuditStrict::DescLength do
column: 2,
source: source }]
inspect_source(cop, source)
inspect_source(cop, source, "/homebrew-core/Formula/foo.rb")
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
@ -156,7 +156,7 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
column: 8,
source: source }]
inspect_source(cop, source)
inspect_source(cop, source, "/homebrew-core/Formula/foo.rb")
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
@ -176,7 +176,7 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
end
EOS
corrected_source = autocorrect_source(cop, source)
corrected_source = autocorrect_source(cop, source, "/homebrew-core/Formula/foo.rb")
expect(corrected_source).to eq(correct_source)
end
end

271
Library/Homebrew/test/rubocops/lines_cop_spec.rb
View File

@ -200,5 +200,276 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do
expect_offense(expected, actual)
end
end
it "with invalid rebuild" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
bottle do
rebuild 0
sha256 "fe0679b932dd43a87fd415b609a7fbac7a069d117642ae8ebaac46ae1fb9f0b3" => :sierra
end
end
EOS
expected_offenses = [{ message: "'rebuild 0' should be removed",
severity: :convention,
line: 5,
column: 4,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with OS.linux? check" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
bottle do
if OS.linux?
nil
end
sha256 "fe0679b932dd43a87fd415b609a7fbac7a069d117642ae8ebaac46ae1fb9f0b3" => :sierra
end
end
EOS
expected_offenses = [{ message: "Don't use OS.linux?; Homebrew/core only supports macOS",
severity: :convention,
line: 5,
column: 7,
source: source }]
inspect_source(cop, source, "/homebrew-core/")
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with fails_with :llvm" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
bottle do
sha256 "fe0679b932dd43a87fd415b609a7fbac7a069d117642ae8ebaac46ae1fb9f0b3" => :sierra
end
fails_with :llvm do
build 2335
cause "foo"
end
end
EOS
expected_offenses = [{ message: "'fails_with :llvm' is now a no-op so should be removed",
severity: :convention,
line: 7,
column: 2,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with def test" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
def test
assert_equals "1", "1"
end
end
EOS
expected_offenses = [{ message: "Use new-style test definitions (test do)",
severity: :convention,
line: 5,
column: 2,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with def options" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
def options
[["--bar", "desc"]]
end
end
EOS
expected_offenses = [{ message: "Use new-style option definitions",
severity: :convention,
line: 5,
column: 2,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with deprecated skip_clean call" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
skip_clean :all
end
EOS
expected_offenses = [{ message: <<-EOS.undent.chomp,
`skip_clean :all` is deprecated; brew no longer strips symbols
Pass explicit paths to prevent Homebrew from removing empty folders.
EOS
severity: :convention,
line: 4,
column: 2,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with build.universal?" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
if build.universal?
"foo"
end
end
EOS
expected_offenses = [{ message: "macOS has been 64-bit only since 10.6 so build.universal? is deprecated.",
severity: :convention,
line: 4,
column: 5,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with ENV.universal_binary" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
if build?
ENV.universal_binary
end
end
EOS
expected_offenses = [{ message: "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated.",
severity: :convention,
line: 5,
column: 5,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with ENV.universal_binary" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
if build?
ENV.x11
end
end
EOS
expected_offenses = [{ message: 'Use "depends_on :x11" instead of "ENV.x11"',
severity: :convention,
line: 5,
column: 5,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with ruby-macho alternatives" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
system "install_name_tool", "-id"
end
EOS
expected_offenses = [{ message: 'Use ruby-macho instead of calling "install_name_tool"',
severity: :convention,
line: 4,
column: 10,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "with npm install without language::Node args" do
source = <<-EOS.undent
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
system "npm", "install"
end
EOS
expected_offenses = [{ message: "Use Language::Node for npm install args",
severity: :convention,
line: 4,
column: 2,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
end
end