From f81e89193e51459f7ec0ff02ddf3b8529f2411f8 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 02:19:31 +1100 Subject: [PATCH 01/19] rubocops: update helpers for rubocop v1 API --- Library/Homebrew/rubocops/checksum.rb | 1 - Library/Homebrew/rubocops/components_order.rb | 4 --- Library/Homebrew/rubocops/extend/formula.rb | 26 +++---------------- Library/Homebrew/rubocops/lines.rb | 2 +- .../Homebrew/rubocops/shared/desc_helper.rb | 1 - .../rubocops/shared/helper_functions.rb | 5 ++-- 6 files changed, 6 insertions(+), 33 deletions(-) diff --git a/Library/Homebrew/rubocops/checksum.rb b/Library/Homebrew/rubocops/checksum.rb index 2078dfbe5a..daeab70415 100644 --- a/Library/Homebrew/rubocops/checksum.rb +++ b/Library/Homebrew/rubocops/checksum.rb @@ -28,7 +28,6 @@ module RuboCop return if checksum.nil? if regex_match_group(checksum, /^$/) - @offense_source_range = @offensive_node.source_range problem "sha256 is empty" return end diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index 148605d1e9..eea8093512 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -27,7 +27,6 @@ module RuboCop if on_macos_blocks.length > 1 @offensive_node = on_macos_blocks.second - @offense_source_range = on_macos_blocks.second.source_range problem "there can only be one `on_macos` block in a formula." end @@ -37,7 +36,6 @@ module RuboCop if on_linux_blocks.length > 1 @offensive_node = on_linux_blocks.second - @offense_source_range = on_linux_blocks.second.source_range problem "there can only be one `on_linux` block in a formula." end @@ -58,7 +56,6 @@ module RuboCop end @offensive_node = resource_block - @offense_source_range = resource_block.source_range next if on_macos_blocks.length.zero? && on_linux_blocks.length.zero? @@ -122,7 +119,6 @@ module RuboCop valid_node ||= on_os_allowed_methods.include? child.method_name.to_s @offensive_node = child - @offense_source_range = child.source_range next if valid_node problem "`#{on_os_block.method_name}` cannot include `#{child.method_name}`. " \ diff --git a/Library/Homebrew/rubocops/extend/formula.rb b/Library/Homebrew/rubocops/extend/formula.rb index 894f680754..bf63a1d24e 100644 --- a/Library/Homebrew/rubocops/extend/formula.rb +++ b/Library/Homebrew/rubocops/extend/formula.rb @@ -18,7 +18,7 @@ module RuboCop # Superclass for all formula cops. # # @api private - class FormulaCop < Cop + class FormulaCop < Base include RangeHelp include HelperFunctions @@ -71,19 +71,16 @@ module RuboCop next unless method_node.method_name == method_name @offensive_node = method_node - @offense_source_range = method_node.source_range return method_node end # If not found then, parent node becomes the offensive node @offensive_node = node.parent - @offense_source_range = node.parent.source_range nil end # Sets the given node as the offending node when required in custom cops. def offending_node(node) @offensive_node = node - @offense_source_range = node.source_range end # Returns an array of method call nodes matching method_name inside node with depth first order (child nodes). @@ -142,7 +139,6 @@ module RuboCop next if method.receiver.const_name != instance && !(method.receiver.send_type? && method.receiver.method_name == instance) - @offense_source_range = method.source_range @offensive_node = method return true unless block_given? @@ -160,7 +156,6 @@ module RuboCop next if method_node.receiver.const_name != name && !(method_node.receiver.send_type? && method_node.receiver.method_name == name) - @offense_source_range = method_node.receiver.source_range @offensive_node = method_node.receiver return true unless block_given? @@ -179,7 +174,6 @@ module RuboCop end return if idx.nil? - @offense_source_range = dependency_nodes[idx].source_range @offensive_node = dependency_nodes[idx] end @@ -207,10 +201,7 @@ module RuboCop type_match = false end - if type_match || name_match - @offensive_node = node - @offense_source_range = node.source_range - end + @offensive_node = node if type_match || name_match type_match && name_match end @@ -223,7 +214,6 @@ module RuboCop next unless const_node.const_name == const_name @offensive_node = const_node - @offense_source_range = const_node.source_range yield const_node if block_given? return true end @@ -259,12 +249,10 @@ module RuboCop next if block_node.method_name != block_name @offensive_node = block_node - @offense_source_range = block_node.source_range return block_node end # If not found then, parent node becomes the offensive node @offensive_node = node.parent - @offense_source_range = node.parent.source_range nil end @@ -299,14 +287,12 @@ module RuboCop next if method_name != def_method_name && method_name.present? @offensive_node = def_node - @offense_source_range = def_node.source_range return def_node end return if node.parent.nil? # If not found then, parent node becomes the offensive node @offensive_node = node.parent - @offense_source_range = node.parent.source_range nil end @@ -317,7 +303,6 @@ module RuboCop next unless call_node.method_name == method_name @offensive_node = call_node - @offense_source_range = call_node.source_range return true end false @@ -345,7 +330,6 @@ module RuboCop next unless call_node.method_name == method_name @offensive_node = call_node - @offense_source_range = call_node.source_range return true end false @@ -365,7 +349,6 @@ module RuboCop def component_precedes?(first_node, next_node) return false if line_number(first_node) < line_number(next_node) - @offense_source_range = first_node.source_range @offensive_node = first_node true end @@ -394,7 +377,6 @@ module RuboCop def parameters_passed?(method_node, *params) method_params = parameters(method_node) @offensive_node = method_node - @offense_source_range = method_node.source_range params.all? do |given_param| method_params.any? do |method_param| if given_param.instance_of?(Regexp) @@ -420,9 +402,8 @@ module RuboCop # Yields to a block with comment text as parameter. def audit_comments - @processed_source.comments.each do |comment_node| + processed_source.comments.each do |comment_node| @offensive_node = comment_node - @offense_source_range = :expression yield comment_node.text end end @@ -435,7 +416,6 @@ module RuboCop # Returns the class node's name, or nil if not a class node. def class_name(node) @offensive_node = node - @offense_source_range = node.source_range node.const_name end diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index d3de64bbac..1d83506998 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -534,7 +534,7 @@ module RuboCop "Pass explicit paths to prevent Homebrew from removing empty folders." end - if find_method_def(@processed_source.ast) + if find_method_def(processed_source.ast) problem "Define method #{method_name(@offensive_node)} in the class body, not at the top-level" end diff --git a/Library/Homebrew/rubocops/shared/desc_helper.rb b/Library/Homebrew/rubocops/shared/desc_helper.rb index 53300c906c..f39269503c 100644 --- a/Library/Homebrew/rubocops/shared/desc_helper.rb +++ b/Library/Homebrew/rubocops/shared/desc_helper.rb @@ -27,7 +27,6 @@ module RuboCop end @offensive_node = desc_call - @offense_source_range = desc_call.source_range desc = desc_call.first_argument diff --git a/Library/Homebrew/rubocops/shared/helper_functions.rb b/Library/Homebrew/rubocops/shared/helper_functions.rb index 09ba102a7a..a2270efa47 100644 --- a/Library/Homebrew/rubocops/shared/helper_functions.rb +++ b/Library/Homebrew/rubocops/shared/helper_functions.rb @@ -28,7 +28,6 @@ module RuboCop @length = match_object.to_s.length @line_no = line_number(node) @source_buf = source_buffer(node) - @offense_source_range = source_range(@source_buf, @line_no, @column, @length) @offensive_node = node match_object end @@ -77,8 +76,8 @@ module RuboCop end end - def problem(msg) - add_offense(@offensive_node, location: @offense_source_range, message: msg) + def problem(msg, &block) + add_offense(@offensive_node, message: msg, &block) end end end From b8fb56802196c6ba03525bbe9884ab9d017f5b06 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 02:21:51 +1100 Subject: [PATCH 02/19] rubocops/lines: use rubocop v1 API --- Library/Homebrew/rubocops/lines.rb | 86 +- Library/Homebrew/test/rubocops/lines_spec.rb | 1146 ++++++++---------- 2 files changed, 511 insertions(+), 721 deletions(-) diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index 1d83506998..cbfa6a5328 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -197,20 +197,17 @@ module RuboCop # # @api private class MpiCheck < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) # Enforce use of OpenMPI for MPI dependency in core return unless formula_tap == "homebrew-core" find_method_with_args(body_node, :depends_on, "mpich") do problem "Formulae in homebrew/core should use 'depends_on \"open-mpi\"' " \ - "instead of '#{@offensive_node.source}'." - end - end - - def autocorrect(node) - # The dependency nodes may need to be re-sorted because of this - lambda do |corrector| - corrector.replace(node.source_range, "depends_on \"open-mpi\"") + "instead of '#{@offensive_node.source}'." do |corrector| + corrector.replace(@offensive_node.source_range, "depends_on \"open-mpi\"") + end end end end @@ -219,6 +216,8 @@ module RuboCop # # @api private class SafePopenCommands < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) test = find_block(body_node, :test) @@ -233,23 +232,21 @@ module RuboCop find_instance_method_call(body_node, "Utils", unsafe_command) do |method| unless test_methods.include?(method.source_range) - problem "Use `Utils.safe_#{unsafe_command}` instead of `Utils.#{unsafe_command}`" + problem "Use `Utils.safe_#{unsafe_command}` instead of `Utils.#{unsafe_command}`" do |corrector| + corrector.replace(@offensive_node.loc.selector, "safe_#{@offensive_node.method_name}") + end end end end end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.loc.selector, "safe_#{node.method_name}") - end - end end # This cop makes sure that environment variables are passed correctly to `popen_*` calls. # # @api private class ShellVariables < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) popen_commands = [ :popen, @@ -265,23 +262,21 @@ module RuboCop good_args = "Utils.#{command}({ \"#{match[1]}\" => \"#{match[2]}\" }, \"#{match[3]}\")" - problem "Use `#{good_args}` instead of `#{method.source}`" + problem "Use `#{good_args}` instead of `#{method.source}`" do |corrector| + corrector.replace(@offensive_node.source_range, + "{ \"#{match[1]}\" => \"#{match[2]}\" }, \"#{match[3]}\"") + end end end end - - def autocorrect(node) - lambda do |corrector| - match = regex_match_group(node, /^([^"' ]+)=([^"' ]+)(?: (.*))?$/) - corrector.replace(node.source_range, "{ \"#{match[1]}\" => \"#{match[2]}\" }, \"#{match[3]}\"") - end - end end # This cop makes sure that `license` has the correct format. # # @api private class LicenseArrays < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) license_node = find_node_method_by_name(body_node, :license) return unless license_node @@ -289,12 +284,8 @@ module RuboCop license = parameters(license_node).first return unless license.array_type? - problem "Use `license any_of: #{license.source}` instead of `license #{license.source}`" - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.source_range, "license any_of: #{parameters(node).first.source}") + problem "Use `license any_of: #{license.source}` instead of `license #{license.source}`" do |corrector| + corrector.replace(license_node.source_range, "license any_of: #{parameters(license_node).first.source}") end end end @@ -324,6 +315,8 @@ module RuboCop # # @api private class PythonVersions < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) python_formula_node = find_every_method_call_by_name(body_node, :depends_on).find do |dep| string_content(parameters(dep).first).start_with? "python@" @@ -334,25 +327,22 @@ module RuboCop python_version = string_content(parameters(python_formula_node).first).split("@").last find_strings(body_node).each do |str| - string_content = string_content(str) + content = string_content(str) - next unless match = string_content.match(/^python(@)?(\d\.\d+)$/) + next unless match = content.match(/^python(@)?(\d\.\d+)$/) next if python_version == match[2] - @fix = if match[1] + fix = if match[1] "python@#{python_version}" else "python#{python_version}" end offending_node(str) - problem "References to `#{string_content}` should match the specified python dependency (`#{@fix}`)" - end - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.source_range, "\"#{@fix}\"") + problem "References to `#{content}` should "\ + "match the specified python dependency (`#{fix}`)" do |corrector| + corrector.replace(str.source_range, "\"#{fix}\"") + end end end end @@ -661,6 +651,8 @@ module RuboCop # # @api private class ShellCommands < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) # Match shell commands separated by spaces in the same string shell_cmd_with_spaces_regex = /[^"' ]*(?:\s[^"' ]*)+/ @@ -682,7 +674,9 @@ module RuboCop good_args = match[0].gsub(" ", "\", \"") offending_node(parameters(method).first) - problem "Separate `system` commands into `\"#{good_args}\"`" + problem "Separate `system` commands into `\"#{good_args}\"`" do |corrector| + corrector.replace(@offensive_node.source_range, @offensive_node.source.gsub(" ", "\", \"")) + end end popen_commands.each do |command| @@ -696,17 +690,13 @@ module RuboCop good_args = match[0].gsub(" ", "\", \"") offending_node(parameters(method)[index]) - problem "Separate `Utils.#{command}` commands into `\"#{good_args}\"`" + problem "Separate `Utils.#{command}` commands into `\"#{good_args}\"`" do |corrector| + good_args = @offensive_node.source.gsub(" ", "\", \"") + corrector.replace(@offensive_node.source_range, good_args) + end end end end - - def autocorrect(node) - lambda do |corrector| - good_args = node.source.gsub(" ", "\", \"") - corrector.replace(node.source_range, good_args) - end - end end end end diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index 8abd8ea169..3b11159129 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -6,76 +6,80 @@ require "rubocops/lines" describe RuboCop::Cop::FormulaAudit::Lines do subject(:cop) { described_class.new } - it "reports an offense when using depends_on :automake" do - expect_offense(<<~RUBY) - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - depends_on :automake - ^^^^^^^^^^^^^^^^^^^^ :automake is deprecated. Usage should be \"automake\". - end - RUBY - end + context "when auditing deprecated special dependencies" do + it "reports an offense when using depends_on :automake" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + depends_on :automake + ^^^^^^^^^^^^^^^^^^^^ :automake is deprecated. Usage should be \"automake\". + end + RUBY + end - it "reports an offense when using depends_on :autoconf" do - expect_offense(<<~RUBY) - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - depends_on :autoconf - ^^^^^^^^^^^^^^^^^^^^ :autoconf is deprecated. Usage should be \"autoconf\". - end - RUBY - end + it "reports an offense when using depends_on :autoconf" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + depends_on :autoconf + ^^^^^^^^^^^^^^^^^^^^ :autoconf is deprecated. Usage should be \"autoconf\". + end + RUBY + end - it "reports an offense when using depends_on :libtool" do - expect_offense(<<~RUBY) - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - depends_on :libtool - ^^^^^^^^^^^^^^^^^^^ :libtool is deprecated. Usage should be \"libtool\". - end - RUBY - end + it "reports an offense when using depends_on :libtool" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + depends_on :libtool + ^^^^^^^^^^^^^^^^^^^ :libtool is deprecated. Usage should be \"libtool\". + end + RUBY + end - it "reports an offense when using depends_on :apr" do - expect_offense(<<~RUBY) - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - depends_on :apr - ^^^^^^^^^^^^^^^ :apr is deprecated. Usage should be \"apr-util\". - end - RUBY - end + it "reports an offense when using depends_on :apr" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + depends_on :apr + ^^^^^^^^^^^^^^^ :apr is deprecated. Usage should be \"apr-util\". + end + RUBY + end - it "reports an offense when using depends_on :tex" do - expect_offense(<<~RUBY) - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - depends_on :tex - ^^^^^^^^^^^^^^^ :tex is deprecated. - end - RUBY + it "reports an offense when using depends_on :tex" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + depends_on :tex + ^^^^^^^^^^^^^^^ :tex is deprecated. + end + RUBY + end end end describe RuboCop::Cop::FormulaAudit::ClassInheritance do subject(:cop) { described_class.new } - it "reports an offense when not using spaces for class inheritance" do - expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") - class Foo, :exist?` instead of `assert File.exist? "default.ini"` - end - RUBY - end + it "reports an offense when assert ... exist? is used without a negation" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + assert File.exist? "default.ini" + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `assert_predicate , :exist?` instead of `assert File.exist? "default.ini"` + end + RUBY + end - it "assert ...exist? with a negation" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - assert !File.exist?("default.ini") - ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `refute_predicate , :exist?` instead of `assert !File.exist?("default.ini")` - end - RUBY - end + it "reports an offense when assert ... exist? is used with a negation" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + assert !File.exist?("default.ini") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `refute_predicate , :exist?` instead of `assert !File.exist?("default.ini")` + end + RUBY + end - it "assert ...executable? without a negation" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - assert File.executable? f - ^^^^^^^^^^^^^^^^^^ Use `assert_predicate , :executable?` instead of `assert File.executable? f` - end - RUBY + it "reports an offense when assert ... executable? is used without a negation" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + assert File.executable? f + ^^^^^^^^^^^^^^^^^^ Use `assert_predicate , :executable?` instead of `assert File.executable? f` + end + RUBY + end end end describe RuboCop::Cop::FormulaAudit::OptionDeclarations do subject(:cop) { described_class.new } - it "build.without? in homebrew/core" do - expect_offense(<<~RUBY, "/homebrew-core/") - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def install - build.without? "bar" - ^^^^^^^^^^^^^^^^^^^^ Formulae in homebrew/core should not use `build.without?`. + context "auditing options" do + it "reports an offense when `build.without?` is used in homebrew/core" do + expect_offense(<<~RUBY, "/homebrew-core/") + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def install + build.without? "bar" + ^^^^^^^^^^^^^^^^^^^^ Formulae in homebrew/core should not use `build.without?`. + end end - end - RUBY - end + RUBY + end - it "build.with? in homebrew/core" do - expect_offense(<<~RUBY, "/homebrew-core/") - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def install - build.with? "bar" - ^^^^^^^^^^^^^^^^^ Formulae in homebrew/core should not use `build.with?`. + it "reports an offense when `build.with?` is used in homebrew/core" do + expect_offense(<<~RUBY, "/homebrew-core/") + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def install + build.with? "bar" + ^^^^^^^^^^^^^^^^^ Formulae in homebrew/core should not use `build.with?`. + end end - end - RUBY - end + RUBY + end - it "build.without? in dependencies" do - expect_offense(<<~RUBY) - class Foo < Formula - depends_on "bar" if build.without?("baz") - ^^^^^^^^^^^^^^^^^^^^^ Use `:optional` or `:recommended` instead of `if build.without?("baz")` - end - RUBY - end - - it "build.with? in dependencies" do - expect_offense(<<~RUBY) - class Foo < Formula - depends_on "bar" if build.with?("baz") - ^^^^^^^^^^^^^^^^^^ Use `:optional` or `:recommended` instead of `if build.with?("baz")` - end - RUBY - end - - it "unless build.without? conditional" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return unless build.without? "bar" - ^^^^^^^^^^^^^^^^^^^^ Use if build.with? "bar" instead of unless build.without? "bar" + it "reports an offense when `build.without?` is used for a conditional dependency" do + expect_offense(<<~RUBY) + class Foo < Formula + depends_on "bar" if build.without?("baz") + ^^^^^^^^^^^^^^^^^^^^^ Use `:optional` or `:recommended` instead of `if build.without?("baz")` end - end - RUBY - end + RUBY + end - it "unless build.with? conditional" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return unless build.with? "bar" - ^^^^^^^^^^^^^^^^^ Use if build.without? "bar" instead of unless build.with? "bar" + it "reports an offense when `build.without?` is used for a conditional dependency" do + expect_offense(<<~RUBY) + class Foo < Formula + depends_on "bar" if build.with?("baz") + ^^^^^^^^^^^^^^^^^^ Use `:optional` or `:recommended` instead of `if build.with?("baz")` end - end - RUBY - end + RUBY + end - it "negated build.with? conditional" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return if !build.with? "bar" - ^^^^^^^^^^^^^^^^^^ Don't negate 'build.with?': use 'build.without?' + it "reports an offense when `build.without?` is used with `unless`" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return unless build.without? "bar" + ^^^^^^^^^^^^^^^^^^^^ Use if build.with? "bar" instead of unless build.without? "bar" + end end - end - RUBY - end + RUBY + end - it "negated build.without? conditional" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return if !build.without? "bar" - ^^^^^^^^^^^^^^^^^^^^^ Don't negate 'build.without?': use 'build.with?' + it "reports an offense when `build.with?` is used with `unless`" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return unless build.with? "bar" + ^^^^^^^^^^^^^^^^^ Use if build.without? "bar" instead of unless build.with? "bar" + end end - end - RUBY - end + RUBY + end - it "unnecessary build.without? conditional" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return if build.without? "--without-bar" - ^^^^^^^^^^^^^ Don't duplicate 'without': Use `build.without? \"bar\"` to check for \"--without-bar\" + it "reports an offense when `build.with?` is negated" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return if !build.with? "bar" + ^^^^^^^^^^^^^^^^^^ Don't negate 'build.with?': use 'build.without?' + end end - end - RUBY - end + RUBY + end - it "unnecessary build.with? conditional" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return if build.with? "--with-bar" - ^^^^^^^^^^ Don't duplicate 'with': Use `build.with? \"bar\"` to check for \"--with-bar\" + it "reports an offense when `build.without?` is negated" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return if !build.without? "bar" + ^^^^^^^^^^^^^^^^^^^^^ Don't negate 'build.without?': use 'build.with?' + end end - end - RUBY - end + RUBY + end - it "build.include? deprecated" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - def post_install - return if build.include? "foo" - ^^^^^^^^^^^^^^^^^^^^ `build.include?` is deprecated + it "reports an offense when a `build.without?` conditional is unnecessary" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return if build.without? "--without-bar" + ^^^^^^^^^^^^^^^ Don't duplicate 'without': Use `build.without? \"bar\"` to check for \"--without-bar\" + end end - end - RUBY - end + RUBY + end - it "def options usage" do - expect_offense(<<~RUBY) - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - - def options - ^^^^^^^^^^^ Use new-style option definitions - [["--bar", "desc"]] + it "reports an offense when a `build.with?` conditional is unnecessary" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return if build.with? "--with-bar" + ^^^^^^^^^^^^ Don't duplicate 'with': Use `build.with? \"bar\"` to check for \"--with-bar\" + end end - end - RUBY + RUBY + end + + it "reports an offense when `build.include?` is used" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + def post_install + return if build.include? "foo" + ^^^^^^^^^^^^^^^^^^^^ `build.include?` is deprecated + end + end + RUBY + end + + it "reports an offense when `def option` is used" do + expect_offense(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + + def options + ^^^^^^^^^^^ Use new-style option definitions + [["--bar", "desc"]] + end + end + RUBY + end end end describe RuboCop::Cop::FormulaAudit::MpiCheck do subject(:cop) { described_class.new } - context "When auditing formula" do - it "reports an offense when using depends_on \"mpich\"" do + context "when auditing MPI dependencies" do + it "reports and corrects an offense when using depends_on \"mpich\" in homebrew/core" do expect_offense(<<~RUBY, "/homebrew-core/") class Foo < Formula desc "foo" @@ -354,8 +362,8 @@ end describe RuboCop::Cop::FormulaAudit::SafePopenCommands do subject(:cop) { described_class.new } - context "When auditing popen commands" do - it "Utils.popen_read should become Utils.safe_popen_read" do + context "when auditing popen commands" do + it "reports and corrects `Utils.popen_read` usage" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -364,9 +372,17 @@ describe RuboCop::Cop::FormulaAudit::SafePopenCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_read "foo" + end + end + RUBY end - it "Utils.safe_popen_write should become Utils.popen_write" do + it "reports and corrects `Utils.popen_write` usage" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -375,9 +391,17 @@ describe RuboCop::Cop::FormulaAudit::SafePopenCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_write "foo" + end + end + RUBY end - it "does not correct Utils.popen_read in test block" do + it "does not report an offense when `Utils.popen_read` is used in a test block" do expect_no_offenses(<<~RUBY) class Foo < Formula def install; end @@ -387,62 +411,6 @@ describe RuboCop::Cop::FormulaAudit::SafePopenCommands do end RUBY end - - it "corrects Utils.popen_read to Utils.safe_popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read "foo" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.safe_popen_read "foo" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "corrects Utils.popen_write to Utils.safe_popen_write" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_write "foo" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.safe_popen_write "foo" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "does not correct to Utils.safe_popen_read in test block" do - source = <<~RUBY - class Foo < Formula - def install; end - test do - Utils.popen_write "foo" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(source) - end end end @@ -450,132 +418,80 @@ describe RuboCop::Cop::FormulaAudit::ShellVariables do subject(:cop) { described_class.new } context "When auditing shell variables" do - it "Shell variables should be expanded in Utils.popen" do + it "reports and corrects unexpanded shell variables in `Utils.popen`" do expect_offense(<<~RUBY) class Foo < Formula def install Utils.popen "SHELL=bash foo" - ^^^^^^^^^^^^^^ Use `Utils.popen({ "SHELL" => "bash" }, "foo")` instead of `Utils.popen "SHELL=bash foo"` + ^^^^^^^^^^^^^^^^ Use `Utils.popen({ "SHELL" => "bash" }, "foo")` instead of `Utils.popen "SHELL=bash foo"` + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.popen { "SHELL" => "bash" }, "foo" end end RUBY end - it "Shell variables should be expanded in Utils.safe_popen_read" do + it "reports and corrects unexpanded shell variables in `Utils.safe_popen_read`" do expect_offense(<<~RUBY) class Foo < Formula def install Utils.safe_popen_read "SHELL=bash foo" - ^^^^^^^^^^^^^^ Use `Utils.safe_popen_read({ "SHELL" => "bash" }, "foo")` instead of `Utils.safe_popen_read "SHELL=bash foo"` + ^^^^^^^^^^^^^^^^ Use `Utils.safe_popen_read({ "SHELL" => "bash" }, "foo")` instead of `Utils.safe_popen_read "SHELL=bash foo"` + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_read { "SHELL" => "bash" }, "foo" end end RUBY end - it "Shell variables should be expanded in Utils.safe_popen_write" do + it "reports and corrects unexpanded shell variables in `Utils.safe_popen_write`" do expect_offense(<<~RUBY) class Foo < Formula def install Utils.safe_popen_write "SHELL=bash foo" - ^^^^^^^^^^^^^^ Use `Utils.safe_popen_write({ "SHELL" => "bash" }, "foo")` instead of `Utils.safe_popen_write "SHELL=bash foo"` + ^^^^^^^^^^^^^^^^ Use `Utils.safe_popen_write({ "SHELL" => "bash" }, "foo")` instead of `Utils.safe_popen_write "SHELL=bash foo"` + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_write { "SHELL" => "bash" }, "foo" end end RUBY end - it "Shell variables should be expanded and keep inline string variables in the arguments" do + it "reports and corrects unexpanded shell variables while preserving string interpolation" do expect_offense(<<~RUBY) class Foo < Formula def install Utils.popen "SHELL=bash \#{bin}/foo" - ^^^^^^^^^^^^^^^^^^^^^ Use `Utils.popen({ "SHELL" => "bash" }, "\#{bin}/foo")` instead of `Utils.popen "SHELL=bash \#{bin}/foo"` + ^^^^^^^^^^^^^^^^^^^^^^^ Use `Utils.popen({ "SHELL" => "bash" }, "\#{bin}/foo")` instead of `Utils.popen "SHELL=bash \#{bin}/foo"` end end RUBY - end - it "corrects shell variables in Utils.popen" do - source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula def install - Utils.popen("SHELL=bash foo") + Utils.popen { "SHELL" => "bash" }, "\#{bin}/foo" end end RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen({ "SHELL" => "bash" }, "foo") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "corrects shell variables in Utils.safe_popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.safe_popen_read("SHELL=bash foo") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.safe_popen_read({ "SHELL" => "bash" }, "foo") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "corrects shell variables in Utils.safe_popen_write" do - source = <<~RUBY - class Foo < Formula - def install - Utils.safe_popen_write("SHELL=bash foo") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.safe_popen_write({ "SHELL" => "bash" }, "foo") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "corrects shell variables with inline string variable in arguments" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen("SHELL=bash \#{bin}/foo") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen({ "SHELL" => "bash" }, "\#{bin}/foo") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end end @@ -583,8 +499,8 @@ end describe RuboCop::Cop::FormulaAudit::LicenseArrays do subject(:cop) { described_class.new } - context "When auditing licenses" do - it "allow license strings" do + context "When auditing license arrays" do + it "reports no offenses for license strings" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -594,7 +510,7 @@ describe RuboCop::Cop::FormulaAudit::LicenseArrays do RUBY end - it "allow license symbols" do + it "reports no offenses for license symbols" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -604,7 +520,7 @@ describe RuboCop::Cop::FormulaAudit::LicenseArrays do RUBY end - it "allow license hashes" do + it "reports no offenses for license hashes" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -614,7 +530,7 @@ describe RuboCop::Cop::FormulaAudit::LicenseArrays do RUBY end - it "require using :any_of instead of a license array" do + it "reports and corrects use of a license array" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -623,27 +539,14 @@ describe RuboCop::Cop::FormulaAudit::LicenseArrays do ^^^^^^^^^^^^^^^^^^^^^^^ Use `license any_of: ["MIT", "0BSD"]` instead of `license ["MIT", "0BSD"]` end RUBY - end - it "corrects license arrays to hash with :any_of" do - source = <<~RUBY - class Foo < Formula - desc "foo" - url 'https://brew.sh/foo-1.0.tgz' - license ["MIT", "0BSD"] - end - RUBY - - corrected_source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' license any_of: ["MIT", "0BSD"] end RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end end @@ -652,7 +555,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do subject(:cop) { described_class.new } context "When auditing licenses" do - it "allow license strings" do + it "reports no offenses for license strings" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -662,7 +565,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do RUBY end - it "allow license symbols" do + it "reports no offenses for license symbols" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -672,7 +575,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do RUBY end - it "allow license hashes" do + it "reports no offenses for license hashes" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -682,7 +585,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do RUBY end - it "allow license exceptions" do + it "reports no offenses for license exceptions" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -692,7 +595,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do RUBY end - it "allow multiline nested license hashes" do + it "reports no offenses for multiline nested license hashes" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -705,7 +608,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do RUBY end - it "allow multiline nested license hashes with exceptions" do + it "reports no offenses for multiline nested license hashes with exceptions" do expect_no_offenses(<<~RUBY) class Foo < Formula desc "foo" @@ -719,7 +622,7 @@ describe RuboCop::Cop::FormulaAudit::Licenses do RUBY end - it "require multiple lines for nested license hashes" do + it "reports an offense for nested license hashes on a single line" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -735,8 +638,8 @@ end describe RuboCop::Cop::FormulaAudit::PythonVersions do subject(:cop) { described_class.new } - context "When auditing python versions" do - it "allow python with no dependency" do + context "When auditing Python versions" do + it "reports no offenses for Python with no dependency" do expect_no_offenses(<<~RUBY) class Foo < Formula def install @@ -746,7 +649,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "allow non versioned python references" do + it "reports no offenses for unversioned Python references" do expect_no_offenses(<<~RUBY) class Foo < Formula depends_on "python@3.9" @@ -758,7 +661,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "allow python with no version" do + it "reports no offenses for Python with no version" do expect_no_offenses(<<~RUBY) class Foo < Formula depends_on "python@3.9" @@ -770,7 +673,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "allow matching versions" do + it "reports no offenses when a Python reference matches its dependency" do expect_no_offenses(<<~RUBY) class Foo < Formula depends_on "python@3.9" @@ -782,7 +685,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "allow matching versions without `@`" do + it "reports no offenses when a Pytohn reference matches its dependency without `@`" do expect_no_offenses(<<~RUBY) class Foo < Formula depends_on "python@3.9" @@ -794,7 +697,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "allow matching versions with two digits" do + it "reports no offenses when a Python reference matches its two-digit dependency" do expect_no_offenses(<<~RUBY) class Foo < Formula depends_on "python@3.10" @@ -806,7 +709,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "allow matching versions without `@` with two digits" do + it "reports no offenses when a Python reference matches its two-digit dependency without `@`" do expect_no_offenses(<<~RUBY) class Foo < Formula depends_on "python@3.10" @@ -818,7 +721,7 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do RUBY end - it "do not allow mismatching versions" do + it "reports and corrects Python references with mismatched versions" do expect_offense(<<~RUBY) class Foo < Formula depends_on "python@3.9" @@ -829,9 +732,19 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + depends_on "python@3.9" + + def install + puts "python@3.9" + end + end + RUBY end - it "do not allow mismatching versions without `@`" do + it "reports and corrects Python references with mismatched versions without `@`" do expect_offense(<<~RUBY) class Foo < Formula depends_on "python@3.9" @@ -842,9 +755,19 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + depends_on "python@3.9" + + def install + puts "python3.9" + end + end + RUBY end - it "do not allow mismatching versions with two digits" do + it "reports and corrects Python references with mismatched two-digit versions" do expect_offense(<<~RUBY) class Foo < Formula depends_on "python@3.11" @@ -855,9 +778,19 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + depends_on "python@3.11" + + def install + puts "python@3.11" + end + end + RUBY end - it "do not allow mismatching versions without `@` with two digits" do + it "reports and corrects Python references with mismatched two-digit versions without `@`" do expect_offense(<<~RUBY) class Foo < Formula depends_on "python@3.11" @@ -868,95 +801,8 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do end end RUBY - end - it "autocorrects mismatching versions" do - source = <<~RUBY - class Foo < Formula - depends_on "python@3.9" - - def install - puts "python@3.8" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - depends_on "python@3.9" - - def install - puts "python@3.9" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "autocorrects mismatching versions without `@`" do - source = <<~RUBY - class Foo < Formula - depends_on "python@3.9" - - def install - puts "python3.8" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - depends_on "python@3.9" - - def install - puts "python3.9" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "autocorrects mismatching versions with two digits" do - source = <<~RUBY - class Foo < Formula - depends_on "python@3.10" - - def install - puts "python@3.9" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - depends_on "python@3.10" - - def install - puts "python@3.10" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "autocorrects mismatching versions without `@` with two digits" do - source = <<~RUBY - class Foo < Formula - depends_on "python@3.11" - - def install - puts "python3.10" - end - end - RUBY - - corrected_source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula depends_on "python@3.11" @@ -965,9 +811,6 @@ describe RuboCop::Cop::FormulaAudit::PythonVersions do end end RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end end @@ -975,8 +818,8 @@ end describe RuboCop::Cop::FormulaAudit::Miscellaneous do subject(:cop) { described_class.new } - context "When auditing formula" do - it "FileUtils usage" do + context "When auditing formula miscellany" do + it "reports an offense for unneeded `FileUtils` usage" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -987,7 +830,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "long inreplace block vars" do + it "reports an offense for long `inreplace` block variable names" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1000,7 +843,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "an invalid rebuild statement" do + it "reports an offense for invalid `rebuild` numbers" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1014,7 +857,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "OS.linux? check" do + it "reports an offense when `OS.linux?` is used in homebrew/core" do expect_offense(<<~RUBY, "/homebrew-core/") class Foo < Formula desc "foo" @@ -1030,7 +873,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "fails_with :llvm block" do + it "reports an offense when a useless `fails_with :llvm` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1047,7 +890,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "def test's deprecated usage" do + it "reports an offense when `def test` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1061,7 +904,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "with deprecated skip_clean call" do + it "reports an offense when `skip_clean` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1072,7 +915,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "build.universal? deprecated usage" do + it "reports an offense when `build.universal?` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1085,7 +928,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "build.universal? deprecation exempted formula" do + it "reports no offenses when `build.universal?` is used in an exempt formula" do expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/wine.rb") class Wine < Formula desc "foo" @@ -1097,7 +940,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "deprecated ENV.universal_binary usage" do + it "reports an offense when `ENV.universal_binary` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1110,7 +953,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "ENV.universal_binary deprecation exempted formula" do + it "reports no offenses when `ENV.universal_binary` is used in an exempt formula" do expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/wine.rb") class Wine < Formula desc "foo" @@ -1122,18 +965,18 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "install_name_tool usage instead of ruby-macho" do + it "reports an offense when `install_name_tool` is called" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' system "install_name_tool", "-id" - ^^^^^^^^^^^^^^^^^ Use ruby-macho instead of calling "install_name_tool" + ^^^^^^^^^^^^^^^^^^^ Use ruby-macho instead of calling "install_name_tool" end RUBY end - it "npm install without language::Node args" do + it "reports an offense when `npm install` is called without Language::Node arguments" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1144,7 +987,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "npm install without language::Node args in kibana(exempted formula)" do + it "reports no offenses when `npm install` is called without Language::Node arguments in an exempt formula" do expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/kibana@4.4.rb") class KibanaAT44 < Formula desc "foo" @@ -1154,7 +997,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "depends_on with an instance as argument" do + it "reports an offense when `depends_on` is called with an instance" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1165,30 +1008,30 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "non glob DIR usage" do + it "reports an offense when `Dir` is called without a globbing argument" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' rm_rf Dir["src/{llvm,test,librustdoc,etc/snapshot.pyc}"] rm_rf Dir["src/snapshot.pyc"] - ^^^^^^^^^^^^^^^^ Dir(["src/snapshot.pyc"]) is unnecessary; just use "src/snapshot.pyc" + ^^^^^^^^^^^^^^^^^^ Dir(["src/snapshot.pyc"]) is unnecessary; just use "src/snapshot.pyc" end RUBY end - it "system call to fileUtils Method" do + it "reports an offense when executing a system command for which there is a Ruby FileUtils equivalent" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' system "mkdir", "foo" - ^^^^^ Use the `mkdir` Ruby method instead of `system "mkdir", "foo"` + ^^^^^^^ Use the `mkdir` Ruby method instead of `system "mkdir", "foo"` end RUBY end - it "top-level function def outside class body" do + it "reports an offense when top-level functions are defined outside of a class body" do expect_offense(<<~RUBY) def test ^^^^^^^^ Define method test in the class body, not at the top-level @@ -1201,173 +1044,171 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it 'man+"man8" usage' do + it 'reports an offense when `man+"man8"` is used' do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install man1.install man+"man8" => "faad.1" - ^^^^ "man+"man8"" should be "man8" + ^^^^^^ "man+"man8"" should be "man8" end end RUBY end - it "hardcoded gcc compiler system" do + it "reports an offense when a hard-coded `gcc` is referenced" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install system "/usr/bin/gcc", "foo" - ^^^^^^^^^^^^ Use "#{ENV.cc}" instead of hard-coding "gcc" + ^^^^^^^^^^^^^^ Use "#{ENV.cc}" instead of hard-coding "gcc" end end RUBY end - it "hardcoded g++ compiler system" do + it "reports an offense when a hard-coded `g++` is referenced" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install system "/usr/bin/g++", "-o", "foo", "foo.cc" - ^^^^^^^^^^^^ Use "#{ENV.cxx}" instead of hard-coding "g++" + ^^^^^^^^^^^^^^ Use "#{ENV.cxx}" instead of hard-coding "g++" end end RUBY end - it "hardcoded llvm-g++ compiler COMPILER_PATH" do + it "reports an offense when a hard-coded `llvm-g++` is set as COMPILER_PATH" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install ENV["COMPILER_PATH"] = "/usr/bin/llvm-g++" - ^^^^^^^^^^^^^^^^^ Use "#{ENV.cxx}" instead of hard-coding "llvm-g++" + ^^^^^^^^^^^^^^^^^^^ Use "#{ENV.cxx}" instead of hard-coding "llvm-g++" end end RUBY end - it "hardcoded gcc compiler COMPILER_PATH" do + it "reports an offense when a hard-coded `gcc` is set as COMPILER_PATH" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install ENV["COMPILER_PATH"] = "/usr/bin/gcc" - ^^^^^^^^^^^^ Use \"\#{ENV.cc}\" instead of hard-coding \"gcc\" + ^^^^^^^^^^^^^^ Use \"\#{ENV.cc}\" instead of hard-coding \"gcc\" end end RUBY end - it "formula path shortcut : man" do + it "reports an offense when the formula path shortcut `man` could be used" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install mv "#{share}/man", share - ^^^^ "#{share}/man" should be "#{man}" + ^^^^ "#{share}/man" should be "#{man}" end end RUBY end - it "formula path shortcut : libexec" do + it "reports an offense when the formula path shortcut `libexec` could be used" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install mv "#{prefix}/libexec", share - ^^^^^^^^ "#{prefix}/libexec" should be "#{libexec}" + ^^^^^^^^ "#{prefix}/libexec" should be "#{libexec}" end end RUBY end - it "formula path shortcut : info" do + it "reports an offense when the formula path shortcut `info` could be used" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install system "./configure", "--INFODIR=#{prefix}/share/info" - ^^^^^^ "#{prefix}/share" should be "#{share}" - ^^^^^^^^^^^ "#{prefix}/share/info" should be "#{info}" + ^^^^^^^^^^^ "#{prefix}/share/info" should be "#{info}" end end RUBY end - it "formula path shortcut : man8" do + it "reports an offense when the formula path shortcut `man8` could be used" do expect_offense(<<~'RUBY') class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' def install system "./configure", "--MANDIR=#{prefix}/share/man/man8" - ^^^^^^ "#{prefix}/share" should be "#{share}" - ^^^^^^^^^^^^^^^ "#{prefix}/share/man/man8" should be "#{man8}" + ^^^^^^^^^^^^^^^ "#{prefix}/share/man/man8" should be "#{man8}" end end RUBY end - it "dependencies which have to vendored" do + it "reports an offense when unvendored lua modules are used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' depends_on "lpeg" => :lua51 - ^^^^^ lua modules should be vendored rather than use deprecated `depends_on \"lpeg\" => :lua51` + ^^^^^^ lua modules should be vendored rather than use deprecated `depends_on \"lpeg\" => :lua51` end RUBY end - it "manually setting env" do + it "reports an offense when `export` is used to set environment variables" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' system "export", "var=value" - ^^^^^^ Use ENV instead of invoking 'export' to modify the environment + ^^^^^^^^ Use ENV instead of invoking 'export' to modify the environment end RUBY end - it "dependencies with invalid options which lead to force rebuild" do + it "reports an offense when dependencies with invalid options are used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' depends_on "foo" => "with-bar" - ^^^^^^^^ Dependency foo should not use option with-bar + ^^^^^^^^^^ Dependency foo should not use option with-bar end RUBY end - it "dependencies with invalid options in array value which lead to force rebuild" do + it "reports an offense when dependencies with invalid options are used in an array" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" url 'https://brew.sh/foo-1.0.tgz' depends_on "httpd" => [:build, :test] depends_on "foo" => [:optional, "with-bar"] - ^^^^^^^^ Dependency foo should not use option with-bar + ^^^^^^^^^^ Dependency foo should not use option with-bar depends_on "icu4c" => [:optional, "c++11"] - ^^^^^ Dependency icu4c should not use option c++11 + ^^^^^^^ Dependency icu4c should not use option c++11 end RUBY end - it "inspecting version manually" do + it "reports an offense when `build.head?` could be used instead of checking `version`" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1380,7 +1221,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "deprecated ARGV.include? (--HEAD) usage" do + it "reports an offense when `ARGV.include? (--HEAD)` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1394,7 +1235,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "deprecated needs :openmp usage" do + it "reports an offense when `needs :openmp` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1405,7 +1246,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "deprecated MACOS_VERSION const usage" do + it "reports an offense when `MACOS_VERSION` is used" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1418,7 +1259,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "deprecated if build.with? conditional dependency" do + it "reports an offense when `build.with?` is used for a conditional dependency" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1429,7 +1270,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "unless conditional dependency with build.without?" do + it "reports an offense when `build.without?` is used for a negated conditional dependency" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1440,7 +1281,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do RUBY end - it "unless conditional dependency with build.include?" do + it "reports an offense when `build.include?` is used for a negated conditional dependency" do expect_offense(<<~RUBY) class Foo < Formula desc "foo" @@ -1469,7 +1310,7 @@ describe RuboCop::Cop::FormulaAuditStrict::MakeCheck do JSON end - it "build-time checks in homebrew/core" do + it "reports an offense when formulae in homebrew/core run build-time checks" do setup_style_exceptions expect_offense(<<~RUBY, "#{path}/Formula/foo.rb") @@ -1482,7 +1323,7 @@ describe RuboCop::Cop::FormulaAuditStrict::MakeCheck do RUBY end - it "build-time checks in homebrew/core in allowlist" do + it "reports no offenses when exempted formulae in homebrew/core run build-time checks" do setup_style_exceptions expect_no_offenses(<<~RUBY, "#{path}/Formula/bar.rb") @@ -1499,7 +1340,7 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do subject(:cop) { described_class.new } context "When auditing shell commands" do - it "system arguments should be separated" do + it "reports and corrects an offense when `system` arguments should be separated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1508,9 +1349,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + system "foo", "bar" + end + end + RUBY end - it "system arguments with string interpolation should be separated" do + it "reports and corrects an offense when `system` arguments with string interpolation should be separated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1519,9 +1368,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + system "\#{bin}/foo", "bar" + end + end + RUBY end - it "system arguments with metacharacters should not be separated" do + it "reports no offenses when `system` with metacharacter arguments are called" do expect_no_offenses(<<~RUBY) class Foo < Formula def install @@ -1531,7 +1388,7 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do RUBY end - it "only the first system argument should be separated" do + it "reports no offenses when trailing arguments to `system` are unseparated" do expect_no_offenses(<<~RUBY) class Foo < Formula def install @@ -1541,7 +1398,7 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do RUBY end - it "Utils.popen arguments should not be separated" do + it "reports no offenses when `Utils.popen` arguments are unseparated" do expect_no_offenses(<<~RUBY) class Foo < Formula def install @@ -1551,7 +1408,7 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do RUBY end - it "Utils.popen_read arguments should be separated" do + it "reports and corrects an offense when `Utils.popen_read` arguments are unseparated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1560,9 +1417,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo", "bar") + end + end + RUBY end - it "Utils.safe_popen_read arguments should be separated" do + it "reports and corrects an offense when `Utils.safe_popen_read` arguments are unseparated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1571,9 +1436,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_read("foo", "bar") + end + end + RUBY end - it "Utils.popen_write arguments should be separated" do + it "reports and corrects an offense when `Utils.popen_write` arguments are unseparated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1582,9 +1455,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.popen_write("foo", "bar") + end + end + RUBY end - it "Utils.safe_popen_write arguments should be separated" do + it "reports and corrects an offense when `Utils.safe_popen_write` arguments are unseparated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1593,9 +1474,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_write("foo", "bar") + end + end + RUBY end - it "Utils.popen_read arguments with string interpolation should be separated" do + it "reports and corrects an offense when `Utils.popen_read` arguments with interpolation are unseparated" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1604,9 +1493,17 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("\#{bin}/foo", "bar") + end + end + RUBY end - it "Utils.popen_read arguments with metacharacters should not be separated" do + it "reports no offenses when `Utils.popen_read` arguments with metacharacters are unseparated" do expect_no_offenses(<<~RUBY) class Foo < Formula def install @@ -1616,7 +1513,7 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do RUBY end - it "only the first Utils.popen_read argument should be separated" do + it "reports no offenses when trailing arguments to `Utils.popen_read` are unseparated" do expect_no_offenses(<<~RUBY) class Foo < Formula def install @@ -1626,7 +1523,7 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do RUBY end - it "Utils.popen_read arguments should be separated following a shell variable" do + it "reports and corrects an offense when `Utils.popen_read` arguments are unseparated after a shell variable" do expect_offense(<<~RUBY) class Foo < Formula def install @@ -1635,111 +1532,14 @@ describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do end end RUBY - end - it "separates shell commands in system" do - source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula def install - system "foo bar" + Utils.popen_read({ "SHELL" => "bash"}, "foo", "bar") end end RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - system "foo", "bar" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands with string interpolation in system" do - source = <<~RUBY - class Foo < Formula - def install - system "\#{foo}/bar baz" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - system "\#{foo}/bar", "baz" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands in Utils.popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("foo bar") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("foo", "bar") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands with string interpolation in Utils.popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("\#{foo}/bar baz") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("\#{foo}/bar", "baz") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands following a shell variable in Utils.popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read({ "SHELL" => "bash" }, "foo bar") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read({ "SHELL" => "bash" }, "foo", "bar") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end end From 9046d778e0a17a40006b423da7cdef607d0d8b97 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 11:05:37 +1100 Subject: [PATCH 03/19] rubocops/checksum: use rubocop v1 API --- Library/Homebrew/rubocops/checksum.rb | 18 +++++------ .../rubocops/shared/helper_functions.rb | 1 + .../Homebrew/test/rubocops/checksum_spec.rb | 30 +++++-------------- 3 files changed, 16 insertions(+), 33 deletions(-) diff --git a/Library/Homebrew/rubocops/checksum.rb b/Library/Homebrew/rubocops/checksum.rb index daeab70415..e8035b8ee3 100644 --- a/Library/Homebrew/rubocops/checksum.rb +++ b/Library/Homebrew/rubocops/checksum.rb @@ -38,7 +38,7 @@ module RuboCop return unless regex_match_group(checksum, /[^a-f0-9]+/i) - problem "sha256 contains invalid characters" + add_offense(@offensive_source_range, message: "sha256 contains invalid characters") end end @@ -46,6 +46,8 @@ module RuboCop # # @api private class ChecksumCase < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) return if body_node.nil? @@ -55,15 +57,11 @@ module RuboCop next if checksum.nil? next unless regex_match_group(checksum, /[A-F]+/) - problem "sha256 should be lowercase" - end - end - - def autocorrect(node) - lambda do |corrector| - correction = node.source.downcase - corrector.insert_before(node.source_range, correction) - corrector.remove(node.source_range) + add_offense(@offensive_source_range, message: "sha256 should be lowercase") do |corrector| + correction = @offensive_node.source.downcase + corrector.insert_before(@offensive_node.source_range, correction) + corrector.remove(@offensive_node.source_range) + end end end end diff --git a/Library/Homebrew/rubocops/shared/helper_functions.rb b/Library/Homebrew/rubocops/shared/helper_functions.rb index a2270efa47..c9330051d2 100644 --- a/Library/Homebrew/rubocops/shared/helper_functions.rb +++ b/Library/Homebrew/rubocops/shared/helper_functions.rb @@ -29,6 +29,7 @@ module RuboCop @line_no = line_number(node) @source_buf = source_buffer(node) @offensive_node = node + @offensive_source_range = source_range(@source_buf, @line_no, @column, @length) match_object end diff --git a/Library/Homebrew/test/rubocops/checksum_spec.rb b/Library/Homebrew/test/rubocops/checksum_spec.rb index 4546c4f37a..0372161d7e 100644 --- a/Library/Homebrew/test/rubocops/checksum_spec.rb +++ b/Library/Homebrew/test/rubocops/checksum_spec.rb @@ -33,12 +33,12 @@ describe RuboCop::Cop::FormulaAudit::Checksum do stable do url "https://github.com/foo-lang/foo-compiler/archive/0.18.0.tar.gz" sha256 "5cf6e1ae0a645b426c0474cc7cd3f7d1605ffa1ac5756a39a8b2268ddc7ea0e9ad" - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sha256 should be 64 characters + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sha256 should be 64 characters resource "foo-package" do url "https://github.com/foo-lang/foo-package/archive/0.18.0.tar.gz" sha256 "5cf6e1ae0a645b426c047aaa4cc7cd3f7d1605ffa1ac5756a39a8b2268ddc7ea0e9" - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sha256 should be 64 characters + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sha256 should be 64 characters end end end @@ -89,7 +89,7 @@ describe RuboCop::Cop::FormulaAudit::ChecksumCase do RUBY end - it "reports an offense if a checksum outside a `stable` block contains uppercase letters" do + it "reports and corrects an offense if a checksum outside a `stable` block contains uppercase letters" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -109,27 +109,14 @@ describe RuboCop::Cop::FormulaAudit::ChecksumCase do end end RUBY - end - it "auto-corrects checksums that contain uppercase letters" do - source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' - stable do - url "https://github.com/foo-lang/foo-compiler/archive/0.18.0.tar.gz" - sha256 "5cf6e1ae0A645b426c0a7cc7cd3f7d1605ffa1ac5756a39a8b2268ddc7ea0e9a" - - resource "foo-package" do - url "https://github.com/foo-lang/foo-package/archive/0.18.0.tar.gz" - sha256 "5cf6e1Ae0a645b426b047aa4cc7cd3f7d1605ffa1ac5756a39a8b2268ddc7ea9" - end + resource "foo-outside" do + url "https://github.com/foo-lang/foo-outside/archive/0.18.0.tar.gz" + sha256 "a4cc7cd3f7d1605ffa1ac5755cf6e1ae0a645b426b047a6a39a8b2268ddc7ea9" end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' stable do url "https://github.com/foo-lang/foo-compiler/archive/0.18.0.tar.gz" sha256 "5cf6e1ae0a645b426c0a7cc7cd3f7d1605ffa1ac5756a39a8b2268ddc7ea0e9a" @@ -141,9 +128,6 @@ describe RuboCop::Cop::FormulaAudit::ChecksumCase do end end RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end end From c18ffa84ca8d1500ebd403955da61530db50f4f3 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 11:14:12 +1100 Subject: [PATCH 04/19] rubocops/deprecate_disable: use rubocop v1 API --- .../Homebrew/rubocops/deprecate_disable.rb | 38 ++- .../test/rubocops/deprecate_disable_spec.rb | 276 ++++++++---------- 2 files changed, 135 insertions(+), 179 deletions(-) diff --git a/Library/Homebrew/rubocops/deprecate_disable.rb b/Library/Homebrew/rubocops/deprecate_disable.rb index 2c8cc4b7db..140e75e244 100644 --- a/Library/Homebrew/rubocops/deprecate_disable.rb +++ b/Library/Homebrew/rubocops/deprecate_disable.rb @@ -8,6 +8,8 @@ module RuboCop module FormulaAudit # This cop audits `deprecate!` and `disable!` dates. class DeprecateDisableDate < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) [:deprecate!, :disable!].each do |method| node = find_node_method_by_name(body_node, method) @@ -19,18 +21,13 @@ module RuboCop rescue ArgumentError fixed_date_string = Date.parse(string_content(date_node)).iso8601 offending_node(date_node) - problem "Use `#{fixed_date_string}` to comply with ISO 8601" + problem "Use `#{fixed_date_string}` to comply with ISO 8601" do |corrector| + corrector.replace(date_node.source_range, "\"#{fixed_date_string}\"") + end end end end - def autocorrect(node) - lambda do |corrector| - fixed_fixed_date_string = Date.parse(string_content(node)).iso8601 - corrector.replace(node.source_range, "\"#{fixed_fixed_date_string}\"") - end - end - def_node_search :date, <<~EOS (pair (sym :date) $str) EOS @@ -38,6 +35,8 @@ module RuboCop # This cop audits `deprecate!` and `disable!` reasons. class DeprecateDisableReason < FormulaCop + extend AutoCorrector + PUNCTUATION_MARKS = %w[. ! ?].freeze def audit_formula(_node, _class_node, _parent_class_node, body_node) @@ -54,9 +53,17 @@ module RuboCop offending_node(reason_node) reason_string = string_content(reason_node) - problem "Do not start the reason with `it`" if reason_string.start_with?("it ") + if reason_string.start_with?("it ") + problem "Do not start the reason with `it`" do |corrector| + corrector.replace(@offensive_node.source_range, "\"#{reason_string[3..]}\"") + end + end - problem "Do not end the reason with a punctuation mark" if PUNCTUATION_MARKS.include?(reason_string[-1]) + if PUNCTUATION_MARKS.include?(reason_string[-1]) + problem "Do not end the reason with a punctuation mark" do |corrector| + corrector.replace(@offensive_node.source_range, "\"#{reason_string.chop}\"") + end + end end next if reason_found @@ -70,17 +77,6 @@ module RuboCop end end - def autocorrect(node) - return unless node.str_type? - - lambda do |corrector| - reason = string_content(node) - reason = reason[3..] if reason.start_with?("it ") - reason.chop! if PUNCTUATION_MARKS.include?(reason[-1]) - corrector.replace(node.source_range, "\"#{reason}\"") - end - end - def_node_search :reason, <<~EOS (pair (sym :because) ${str sym}) EOS diff --git a/Library/Homebrew/test/rubocops/deprecate_disable_spec.rb b/Library/Homebrew/test/rubocops/deprecate_disable_spec.rb index 58572a91b1..59c40133d7 100644 --- a/Library/Homebrew/test/rubocops/deprecate_disable_spec.rb +++ b/Library/Homebrew/test/rubocops/deprecate_disable_spec.rb @@ -7,7 +7,7 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do subject(:cop) { described_class.new } context "when auditing `deprecate!`" do - it "reports an offense if `date` is not ISO 8601 compliant" do + it "reports and corrects an offense if `date` is not ISO 8601 compliant" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -15,9 +15,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do ^^^^^^^^^^^^^^^ Use `2020-06-25` to comply with ISO 8601 end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! date: "2020-06-25" + end + RUBY end - it "reports an offense if `date` is not ISO 8601 compliant (with `reason`)" do + it "reports and corrects an offense if `date` is not ISO 8601 compliant (with `reason`)" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -25,6 +32,13 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do ^^^^^^^^^^^^^^^ Use `2020-06-25` to comply with ISO 8601 end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! because: "is broken", date: "2020-06-25" + end + RUBY end it "reports no offenses if `date` is ISO 8601 compliant" do @@ -62,48 +76,10 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do end RUBY end - - it "auto-corrects `date` to ISO 8601 format" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! date: "June 25, 2020" - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! date: "2020-06-25" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "auto-corrects `date` to ISO 8601 format (with `reason`)" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! because: "is broken", date: "June 25, 2020" - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! because: "is broken", date: "2020-06-25" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end end context "when auditing `disable!`" do - it "reports an offense if `date` is not ISO 8601 compliant" do + it "reports and corrects an offense if `date` is not ISO 8601 compliant" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -111,9 +87,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do ^^^^^^^^^^^^^^^ Use `2020-06-25` to comply with ISO 8601 end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! date: "2020-06-25" + end + RUBY end - it "reports an offense if `date` is not ISO 8601 compliant (with `reason`)" do + it "reports and corrects an offense if `date` is not ISO 8601 compliant (with `reason`)" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -121,6 +104,13 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do ^^^^^^^^^^^^^^^ Use `2020-06-25` to comply with ISO 8601 end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! because: "is broken", date: "2020-06-25" + end + RUBY end it "reports no offenses if `date` is ISO 8601 compliant" do @@ -158,44 +148,6 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableDate do end RUBY end - - it "auto-corrects `date` to ISO 8601 format" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! date: "June 25, 2020" - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! date: "2020-06-25" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "auto-corrects `date` to ISO 8601 format (with `reason`)" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! because: "is broken", date: "June 25, 2020" - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! because: "is broken", date: "2020-06-25" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end end end @@ -259,7 +211,7 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do RUBY end - it "reports an offense if `reason` starts with 'it'" do + it "reports and corrects an offense if `reason` starts with 'it'" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -267,9 +219,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^^^ Do not start the reason with `it` end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! because: "is broken" + end + RUBY end - it "reports an offense if `reason` starts with 'it' (with `date`)" do + it "reports and corrects an offense if `reason` starts with 'it' (with `date`)" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -277,9 +236,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^^^ Do not start the reason with `it` end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! date: "2020-08-28", because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with a period" do + it "reports and corrects an offense if `reason` ends with a period" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -287,9 +253,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with an exclamation point" do + it "reports and corrects an offense if `reason` ends with an exclamation point" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -297,9 +270,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with a question mark" do + it "reports and corrects an offense if `reason` ends with a question mark" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -307,9 +287,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with a period (with `date`)" do + it "reports and corrects an offense if `reason` ends with a period (with `date`)" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -317,44 +304,13 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY - end - it "auto-corrects `reason` to remove 'it'" do - source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' - deprecate! because: "it is broken" + deprecate! date: "2020-08-28", because: "is broken" end RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! because: "is broken" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "auto-corrects `reason` to remove punctuation" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! because: "is broken." - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - deprecate! because: "is broken" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end @@ -415,7 +371,7 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do RUBY end - it "reports an offense if `reason` starts with 'it'" do + it "reports and corrects an offense if `reason` starts with 'it'" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -423,9 +379,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^^^ Do not start the reason with `it` end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! because: "is broken" + end + RUBY end - it "reports an offense if `reason` starts with 'it' (with `date`)" do + it "reports and corrects an offense if `reason` starts with 'it' (with `date`)" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -433,9 +396,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^^^ Do not start the reason with `it` end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! date: "2020-08-28", because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with a period" do + it "reports and corrects an offense if `reason` ends with a period" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -443,9 +413,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with an exclamation point" do + it "reports and corrects an offense if `reason` ends with an exclamation point" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -453,9 +430,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with a question mark" do + it "reports and corrects an offense if `reason` ends with a question mark" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -463,9 +447,16 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + disable! because: "is broken" + end + RUBY end - it "reports an offense if `reason` ends with a period (with `date`)" do + it "reports and corrects an offense if `reason` ends with a period (with `date`)" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -473,44 +464,13 @@ describe RuboCop::Cop::FormulaAudit::DeprecateDisableReason do ^^^^^^^^^^^^ Do not end the reason with a punctuation mark end RUBY - end - it "auto-corrects to remove 'it'" do - source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' - disable! because: "it is broken" + disable! date: "2020-08-28", because: "is broken" end RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! because: "is broken" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "auto-corrects to remove punctuation" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! because: "is broken." - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - disable! because: "is broken" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end end end From 3bda457478239813d0de59343038ee8871e09e7d Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 12:01:29 +1100 Subject: [PATCH 05/19] rubocops/text: use rubocop v1 API --- Library/Homebrew/rubocops/text.rb | 19 +++++++++---------- Library/Homebrew/test/rubocops/text_spec.rb | 7 +++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/rubocops/text.rb b/Library/Homebrew/rubocops/text.rb index 443eab7a58..82df0a41ab 100644 --- a/Library/Homebrew/rubocops/text.rb +++ b/Library/Homebrew/rubocops/text.rb @@ -10,17 +10,16 @@ module RuboCop # # @api private class Text < FormulaCop - def audit_formula(node, _class_node, _parent_class_node, body_node) - @full_source_content = source_buffer(node).source + extend AutoCorrector - if match = @full_source_content.match(/^require ['"]formula['"]$/) - @offensive_node = node - @source_buf = source_buffer(node) - @line_no = match.pre_match.count("\n") + 1 - @column = 0 - @length = match[0].length - @offense_source_range = source_range(@source_buf, @line_no, @column, @length) - problem "`#{match}` is now unnecessary" + def audit_formula(node, _class_node, _parent_class_node, body_node) + full_source_content = source_buffer(node).source + + if match = full_source_content.match(/^require ['"]formula['"]$/) + range = source_range(source_buffer(node), match.pre_match.count("\n") + 1, 0, match[0].length) + add_offense(range, message: "`#{match}` is now unnecessary") do |corrector| + corrector.remove(range_with_surrounding_space(range: range)) + end end if !find_node_method_by_name(body_node, :plist_options) && diff --git a/Library/Homebrew/test/rubocops/text_spec.rb b/Library/Homebrew/test/rubocops/text_spec.rb index 63c49df4f8..b7730e37db 100644 --- a/Library/Homebrew/test/rubocops/text_spec.rb +++ b/Library/Homebrew/test/rubocops/text_spec.rb @@ -16,6 +16,13 @@ describe RuboCop::Cop::FormulaAudit::Text do homepage "https://brew.sh" end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + homepage "https://brew.sh" + end + RUBY end it "with both openssl and libressl optional dependencies" do From a778dc3bdbc6e182e9ab7be733bcf08793d78853 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 13:33:23 +1100 Subject: [PATCH 06/19] rubocops/class: use rubocop v1 API --- Library/Homebrew/rubocops/class.rb | 40 ++++------ Library/Homebrew/test/rubocops/class_spec.rb | 81 ++++++++------------ 2 files changed, 47 insertions(+), 74 deletions(-) diff --git a/Library/Homebrew/rubocops/class.rb b/Library/Homebrew/rubocops/class.rb index 4a2a408ac8..5e1fa9ddf7 100644 --- a/Library/Homebrew/rubocops/class.rb +++ b/Library/Homebrew/rubocops/class.rb @@ -10,6 +10,8 @@ module RuboCop # # @api private class ClassName < FormulaCop + extend AutoCorrector + DEPRECATED_CLASSES = %w[ GithubGistFormula ScriptFileFormula @@ -20,12 +22,8 @@ module RuboCop parent_class = class_name(parent_class_node) return unless DEPRECATED_CLASSES.include?(parent_class) - problem "#{parent_class} is deprecated, use Formula instead" - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.source_range, "Formula") + problem "#{parent_class} is deprecated, use Formula instead" do |corrector| + corrector.replace(parent_class_node.source_range, "Formula") end end end @@ -34,6 +32,8 @@ module RuboCop # # @api private class Test < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) test = find_block(body_node, :test) return unless test @@ -49,31 +49,17 @@ module RuboCop p1, p2 = params if match = string_content(p1).match(%r{(/usr/local/(s?bin))}) offending_node(p1) - problem "use \#{#{match[2]}} instead of #{match[1]} in #{node}" + problem "use \#{#{match[2]}} instead of #{match[1]} in #{node}" do |corrector| + corrector.replace(p1.source_range, p1.source.sub(match[1], "\#{#{match[2]}}")) + end end if node == :shell_output && node_equals?(p2, 0) offending_node(p2) - problem "Passing 0 to shell_output() is redundant" - end - end - end - - def autocorrect(node) - lambda do |corrector| - case node.type - when :str, :dstr - # Rubocop: intentionally outputted non-interpolated strings - corrector.replace(node.source_range, - node.source.to_s.sub(%r{(/usr/local/(s?bin))}, - '#{\2}')) # rubocop:disable Lint/InterpolationCheck - when :int - corrector.remove( - range_with_surrounding_comma( - range_with_surrounding_space(range: node.source_range, - side: :left), - ), - ) + problem "Passing 0 to shell_output() is redundant" do |corrector| + corrector.remove(range_with_surrounding_comma(range_with_surrounding_space(range: p2.source_range, + side: :left))) + end end end end diff --git a/Library/Homebrew/test/rubocops/class_spec.rb b/Library/Homebrew/test/rubocops/class_spec.rb index 1f0c84ab27..15c793b229 100644 --- a/Library/Homebrew/test/rubocops/class_spec.rb +++ b/Library/Homebrew/test/rubocops/class_spec.rb @@ -6,55 +6,47 @@ require "rubocops/class" describe RuboCop::Cop::FormulaAudit::ClassName do subject(:cop) { described_class.new } - it "reports an offense when using ScriptFileFormula" do + corrected_source = <<~RUBY + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + end + RUBY + + it "reports and corrects an offense when using ScriptFileFormula" do expect_offense(<<~RUBY) class Foo < ScriptFileFormula ^^^^^^^^^^^^^^^^^ ScriptFileFormula is deprecated, use Formula instead url 'https://brew.sh/foo-1.0.tgz' end RUBY + expect_correction(corrected_source) end - it "reports an offense when using GithubGistFormula" do + it "reports and corrects an offense when using GithubGistFormula" do expect_offense(<<~RUBY) class Foo < GithubGistFormula ^^^^^^^^^^^^^^^^^ GithubGistFormula is deprecated, use Formula instead url 'https://brew.sh/foo-1.0.tgz' end RUBY + expect_correction(corrected_source) end - it "reports an offense when using AmazonWebServicesFormula" do + it "reports and corrects an offense when using AmazonWebServicesFormula" do expect_offense(<<~RUBY) class Foo < AmazonWebServicesFormula ^^^^^^^^^^^^^^^^^^^^^^^^ AmazonWebServicesFormula is deprecated, use Formula instead url 'https://brew.sh/foo-1.0.tgz' end RUBY - end - - it "supports auto-correcting deprecated parent classes" do - source = <<~RUBY - class Foo < AmazonWebServicesFormula - url 'https://brew.sh/foo-1.0.tgz' - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) + expect_correction(corrected_source) end end describe RuboCop::Cop::FormulaAudit::Test do subject(:cop) { described_class.new } - it "reports an offense when /usr/local/bin is found in test calls" do + it "reports and corrects an offense when /usr/local/bin is found in test calls" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -65,9 +57,19 @@ describe RuboCop::Cop::FormulaAudit::Test do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + + test do + system "\#{bin}/test" + end + end + RUBY end - it "reports an offense when passing 0 as the second parameter to shell_output" do + it "reports and corrects an offense when passing 0 as the second parameter to shell_output" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -78,6 +80,16 @@ describe RuboCop::Cop::FormulaAudit::Test do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + + test do + shell_output("\#{bin}/test") + end + end + RUBY end it "reports an offense when there is an empty test block" do @@ -104,31 +116,6 @@ describe RuboCop::Cop::FormulaAudit::Test do end RUBY end - - it "supports auto-correcting test calls" do - source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - - test do - shell_output("/usr/local/sbin/test", 0) - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url 'https://brew.sh/foo-1.0.tgz' - - test do - shell_output("\#{sbin}/test") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end end describe RuboCop::Cop::FormulaAuditStrict::TestPresent do From 986f5dac0d2cffd688b3cad2fc151d1e11c12ac7 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 13:58:53 +1100 Subject: [PATCH 07/19] rubocops/livecheck: use rubocop v1 API --- Library/Homebrew/rubocops/livecheck.rb | 60 +++++++++++--------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/Library/Homebrew/rubocops/livecheck.rb b/Library/Homebrew/rubocops/livecheck.rb index fe3a76ec12..3b8717a6b9 100644 --- a/Library/Homebrew/rubocops/livecheck.rb +++ b/Library/Homebrew/rubocops/livecheck.rb @@ -11,6 +11,8 @@ module RuboCop # # @api private class LivecheckSkip < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) livecheck_node = find_block(body_node, :livecheck) return if livecheck_node.blank? @@ -21,16 +23,12 @@ module RuboCop return if find_every_method_call_by_name(livecheck_node).length < 3 offending_node(livecheck_node) - problem "Skipped formulae must not contain other livecheck information." - end - - def autocorrect(node) - lambda do |corrector| - skip = find_every_method_call_by_name(node, :skip).first + problem "Skipped formulae must not contain other livecheck information." do |corrector| + skip = find_every_method_call_by_name(livecheck_node, :skip).first skip = find_strings(skip).first skip = string_content(skip) if skip.present? corrector.replace( - node.source_range, + livecheck_node.source_range, <<~EOS.strip, livecheck do skip#{" \"#{skip}\"" if skip.present?} @@ -65,7 +63,7 @@ module RuboCop # # @api private class LivecheckUrlSymbol < FormulaCop - @offense = nil + extend AutoCorrector def audit_formula(_node, _class_node, _parent_class_node, body_node) livecheck_node = find_block(body_node, :livecheck) @@ -110,24 +108,20 @@ module RuboCop next if url != livecheck_url && url != "#{livecheck_url}/" && "#{url}/" != livecheck_url offending_node(livecheck_url_node) - @offense = symbol - problem "Use `url :#{symbol}`" + problem "Use `url :#{symbol}`" do |corrector| + corrector.replace(livecheck_url_node.source_range, "url :#{symbol}") + end break end end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.source_range, "url :#{@offense}") - @offense = nil - end - end end # This cop ensures that the `regex` call in the `livecheck` block uses parentheses. # # @api private class LivecheckRegexParentheses < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) livecheck_node = find_block(body_node, :livecheck) return if livecheck_node.blank? @@ -141,13 +135,9 @@ module RuboCop return if parentheses?(livecheck_regex_node) offending_node(livecheck_regex_node) - problem "The `regex` call should always use parentheses." - end - - def autocorrect(node) - lambda do |corrector| - pattern = node.source.split[1..].join - corrector.replace(node.source_range, "regex(#{pattern})") + problem "The `regex` call should always use parentheses." do |corrector| + pattern = livecheck_regex_node.source.split[1..].join + corrector.replace(livecheck_regex_node.source_range, "regex(#{pattern})") end end end @@ -157,6 +147,8 @@ module RuboCop # # @api private class LivecheckRegexExtension < FormulaCop + extend AutoCorrector + TAR_PATTERN = /\\?\.t(ar|(g|l|x)z$|[bz2]{2,4}$)(\\?\.((g|l|x)z)|[bz2]{2,4}|Z)?$/i.freeze def audit_formula(_node, _class_node, _parent_class_node, body_node) @@ -175,12 +167,8 @@ module RuboCop return if match.blank? offending_node(regex_node) - problem "Use `\\.t` instead of `#{match}`" - end - - def autocorrect(node) - lambda do |corrector| - node = find_strings(node).first + problem "Use `\\.t` instead of `#{match}`" do |corrector| + node = find_strings(regex_node).first correct = node.source.gsub(TAR_PATTERN, "\\.t") corrector.replace(node.source_range, correct) end @@ -218,6 +206,10 @@ module RuboCop # # @api private class LivecheckRegexCaseInsensitive < FormulaCop + extend AutoCorrector + + MSG = "Regexes should be case-insensitive unless sensitivity is explicitly required for proper matching." + def audit_formula(_node, _class_node, _parent_class_node, body_node) return if tap_style_exception? :regex_case_sensitive_allowlist @@ -235,12 +227,8 @@ module RuboCop return if options_node.source.include?("i") offending_node(regex_node) - problem "Regexes should be case-insensitive unless sensitivity is explicitly required for proper matching." - end - - def autocorrect(node) - lambda do |corrector| - node = node.regopt + problem MSG do |corrector| + node = regex_node.regopt corrector.replace(node.source_range, "i#{node.source}".chars.sort.join) end end From 098ade13192f42c73d6d7f133d83d6650d4ce48b Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 14:32:34 +1100 Subject: [PATCH 08/19] rubocops/urls: use rubocop v1 API --- Library/Homebrew/test/rubocops/urls_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/test/rubocops/urls_spec.rb b/Library/Homebrew/test/rubocops/urls_spec.rb index e55298094f..1207ebc531 100644 --- a/Library/Homebrew/test/rubocops/urls_spec.rb +++ b/Library/Homebrew/test/rubocops/urls_spec.rb @@ -200,9 +200,9 @@ describe RuboCop::Cop::FormulaAudit::Urls do column: formula["col"], source: source }] - inspect_source(source) + offenses = inspect_source(source) - expected_offenses.zip(cop.offenses.reverse).each do |expected, actual| + expected_offenses.zip(offenses.reverse).each do |expected, actual| expect(actual.message).to eq(expected[:message]) expect(actual.severity).to eq(expected[:severity]) expect(actual.line).to eq(expected[:line]) From 63bebf542fdbe16be2d49533fe5a5e687a596a53 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 14:52:47 +1100 Subject: [PATCH 09/19] rubocops/dependency_order: use rubocop v1 API --- Library/Homebrew/rubocops/dependency_order.rb | 54 +++---- .../test/rubocops/dependency_order_spec.rb | 146 +++++++++++------- 2 files changed, 110 insertions(+), 90 deletions(-) diff --git a/Library/Homebrew/rubocops/dependency_order.rb b/Library/Homebrew/rubocops/dependency_order.rb index 79943949a8..6d92839ca4 100644 --- a/Library/Homebrew/rubocops/dependency_order.rb +++ b/Library/Homebrew/rubocops/dependency_order.rb @@ -11,6 +11,8 @@ module RuboCop # precedence order: # build-time > test > normal > recommended > optional class DependencyOrder < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) check_dependency_nodes_order(body_node) check_uses_from_macos_nodes_order(body_node) @@ -89,17 +91,26 @@ module RuboCop # Verify actual order of sorted `depends_on` nodes in source code; # raise RuboCop problem otherwise. def verify_order_in_source(ordered) - ordered.each_with_index do |dependency_node_1, idx| - l1 = line_number(dependency_node_1) - dependency_node_2 = nil - ordered.drop(idx+1).each do |node2| - l2 = line_number(node2) - dependency_node_2 = node2 if l2 < l1 + ordered.each_with_index do |node_1, idx| + l1 = line_number(node_1) + l2 = nil + node_2 = nil + ordered.drop(idx + 1).each do |test_node| + l2 = line_number(test_node) + node_2 = test_node if l2 < l1 end - next unless dependency_node_2 + next unless node_2 - @offensive_nodes = [dependency_node_1, dependency_node_2] - component_problem dependency_node_1, dependency_node_2 + offending_node(node_1) + + problem "dependency \"#{dependency_name(node_1)}\" (line #{l1}) should be put before dependency "\ + "\"#{dependency_name(node_2)}\" (line #{l2})" do |corrector| + indentation = " " * (start_column(node_2) - line_start_column(node_2)) + line_breaks = "\n" + corrector.insert_before(node_2.source_range, + node_1.source + line_breaks + indentation) + corrector.remove(range_with_surrounding_space(range: node_1.source_range, side: :left)) + end end end @@ -150,31 +161,6 @@ module RuboCop match_node = dependency_name_node(dependency_node).to_a.first string_content(match_node) if match_node end - - def autocorrect(_node) - succeeding_node = @offensive_nodes[0] - preceding_node = @offensive_nodes[1] - lambda do |corrector| - reorder_components(corrector, succeeding_node, preceding_node) - end - end - - private - - def component_problem(c1, c2) - offending_node(c1) - problem "dependency \"#{dependency_name(c1)}\" " \ - "(line #{line_number(c1)}) should be put before dependency "\ - "\"#{dependency_name(c2)}\" (line #{line_number(c2)})" - end - - # Reorder two nodes in the source, using the corrector instance in the {autocorrect} method. - def reorder_components(corrector, node1, node2) - indentation = " " * (start_column(node2) - line_start_column(node2)) - line_breaks = "\n" - corrector.insert_before(node2.source_range, node1.source + line_breaks + indentation) - corrector.remove(range_with_surrounding_space(range: node1.source_range, side: :left)) - end end end end diff --git a/Library/Homebrew/test/rubocops/dependency_order_spec.rb b/Library/Homebrew/test/rubocops/dependency_order_spec.rb index c4ab30fdb8..b080d55ef7 100644 --- a/Library/Homebrew/test/rubocops/dependency_order_spec.rb +++ b/Library/Homebrew/test/rubocops/dependency_order_spec.rb @@ -7,7 +7,7 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do subject(:cop) { described_class.new } context "when auditing `uses_from_macos`" do - it "reports an offense if wrong conditional order" do + it "reports and corrects incorrectly ordered conditional dependencies" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -17,9 +17,18 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 5) should be put before dependency "apple" (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "foo" => :optional + uses_from_macos "apple" if build.with? "foo" + end + RUBY end - it "reports an offense if wrong alphabetical order" do + it "reports and corrects incorrectly ordered alphabetical dependencies" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -29,9 +38,18 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "foo" (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "bar" + uses_from_macos "foo" + end + RUBY end - it "supports requirement constants" do + it "reports and corrects incorrectly ordered dependencies that are Requirements" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -41,9 +59,18 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "FooRequirement" (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "bar" + uses_from_macos FooRequirement + end + RUBY end - it "reports an offense if wrong conditional order with block" do + it "reports and corrects wrong conditional order within a spec block" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -60,6 +87,20 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 10) should be put before dependency "apple" (line 9) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + head do + uses_from_macos "bar" + uses_from_macos "foo" => :optional + uses_from_macos "apple" if build.with? "foo" + end + uses_from_macos "foo" => :optional + uses_from_macos "apple" if build.with? "foo" + end + RUBY end it "reports no offenses if correct order for multiple tags" do @@ -76,7 +117,7 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do end context "when auditing `depends_on`" do - it "reports an offense if wrong conditional order" do + it "reports and corrects incorrectly ordered conditional dependencies" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -86,9 +127,18 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 5) should be put before dependency "apple" (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + depends_on "foo" => :optional + depends_on "apple" if build.with? "foo" + end + RUBY end - it "reports an offense if wrong alphabetical order" do + it "reports and corrects incorrectly ordered alphabetical dependencies" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -98,9 +148,18 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "foo" (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + depends_on "bar" + depends_on "foo" + end + RUBY end - it "supports requirement constants" do + it "reports and corrects incorrectly ordered dependencies that are Requirements" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -110,9 +169,18 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "FooRequirement" (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + depends_on "bar" + depends_on FooRequirement + end + RUBY end - it "reports an offense if wrong conditional order with block" do + it "reports and corrects wrong conditional order within a spec block" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -129,6 +197,20 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 10) should be put before dependency "apple" (line 9) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + head do + depends_on "bar" + depends_on "foo" => :optional + depends_on "apple" if build.with? "foo" + end + depends_on "foo" => :optional + depends_on "apple" if build.with? "foo" + end + RUBY end it "reports no offenses if correct order for multiple tags" do @@ -143,52 +225,4 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do RUBY end end - - context "when auto-correcting" do - it "supports wrong conditional `uses_from_macos` order" do - source = <<~RUBY - class Foo < Formula - homepage "https://brew.sh" - url "https://brew.sh/foo-1.0.tgz" - uses_from_macos "apple" if build.with? "foo" - uses_from_macos "foo" => :optional - end - RUBY - - correct_source = <<~RUBY - class Foo < Formula - homepage "https://brew.sh" - url "https://brew.sh/foo-1.0.tgz" - uses_from_macos "foo" => :optional - uses_from_macos "apple" if build.with? "foo" - end - RUBY - - corrected_source = autocorrect_source(source) - expect(corrected_source).to eq(correct_source) - end - - it "supports wrong conditional `depends_on` order" do - source = <<~RUBY - class Foo < Formula - homepage "https://brew.sh" - url "https://brew.sh/foo-1.0.tgz" - depends_on "apple" if build.with? "foo" - depends_on "foo" => :optional - end - RUBY - - correct_source = <<~RUBY - class Foo < Formula - homepage "https://brew.sh" - url "https://brew.sh/foo-1.0.tgz" - depends_on "foo" => :optional - depends_on "apple" if build.with? "foo" - end - RUBY - - corrected_source = autocorrect_source(source) - expect(corrected_source).to eq(correct_source) - end - end end From 92e07f5c0b0c2b7f3200e30097472b56acbcf46e Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 15:58:52 +1100 Subject: [PATCH 10/19] rubocops/homepage: use rubocop v1 API --- Library/Homebrew/rubocops/homepage.rb | 62 ++--- .../Homebrew/test/rubocops/homepage_spec.rb | 225 ++++++++++-------- 2 files changed, 164 insertions(+), 123 deletions(-) diff --git a/Library/Homebrew/rubocops/homepage.rb b/Library/Homebrew/rubocops/homepage.rb index da27f11f70..928e30e717 100644 --- a/Library/Homebrew/rubocops/homepage.rb +++ b/Library/Homebrew/rubocops/homepage.rb @@ -8,19 +8,23 @@ module RuboCop module FormulaAudit # This cop audits the `homepage` URL in formulae. class Homepage < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) homepage_node = find_node_method_by_name(body_node, :homepage) - homepage = if homepage_node - string_content(parameters(homepage_node).first) - else - "" + + if homepage_node.nil? + problem "Formula should have a homepage." + return end - problem "Formula should have a homepage." if homepage_node.nil? || homepage.empty? + homepage_parameter_node = parameters(homepage_node).first + offending_node(homepage_parameter_node) + homepage = string_content(homepage_parameter_node) - unless homepage.match?(%r{^https?://}) - problem "The homepage should start with http or https (URL is #{homepage})." - end + problem "Formula should have a homepage." if homepage.empty? + + problem "The homepage should start with http or https." unless homepage.match?(%r{^https?://}) case homepage # Freedesktop is complicated to handle - It has SSL/TLS, but only on certain subdomains. @@ -29,25 +33,34 @@ module RuboCop # "Software" is redirected to https://wiki.freedesktop.org/www/Software/project_name when %r{^http://((?:www|nice|libopenraw|liboil|telepathy|xorg)\.)?freedesktop\.org/(?:wiki/)?} if homepage.include?("Software") - problem "#{homepage} should be styled `https://wiki.freedesktop.org/www/Software/project_name`" + problem "Freedesktop homepages should be styled "\ + "`https://wiki.freedesktop.org/www/Software/project_name`" else - problem "#{homepage} should be styled `https://wiki.freedesktop.org/project_name`" + problem "Freedesktop homepages should be styled `https://wiki.freedesktop.org/project_name`" end # Google Code homepages should end in a slash when %r{^https?://code\.google\.com/p/[^/]+[^/]$} - problem "#{homepage} should end with a slash" + problem "Google Code homepages should end with a slash" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{homepage}/\"") + end when %r{^http://([^/]*)\.(sf|sourceforge)\.net(/|$)} - problem "#{homepage} should be `https://#{Regexp.last_match(1)}.sourceforge.io/`" + fixed = "https://#{Regexp.last_match(1)}.sourceforge.io/" + problem "Sourceforge homepages should be `#{fixed}`" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{fixed}\"") + end when /readthedocs\.org/ - offending_node(parameters(homepage_node).first) - problem "#{homepage} should be `#{homepage.sub("readthedocs.org", "readthedocs.io")}`" + fixed = homepage.sub("readthedocs.org", "readthedocs.io") + problem "Readthedocs homepages should be `#{fixed}`" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{fixed}\"") + end when %r{^https://github.com.*\.git} - offending_node(parameters(homepage_node).first) - problem "GitHub homepages (`#{homepage}`) should not end with .git" + problem "GitHub homepages should not end with .git" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{homepage.delete_suffix(".git")}\"") + end # People will run into mixed content sometimes, but we should enforce and then add # exemptions as they are discovered. Treat mixed content on homepages as a bug. @@ -81,20 +94,9 @@ module RuboCop %r{^http://code\.google\.com/}, %r{^http://bitbucket\.org/}, %r{^http://(?:[^/]*\.)?archive\.org} - problem "Please use https:// for #{homepage}" - end - end - - def autocorrect(node) - lambda do |corrector| - return if node.nil? - - homepage = string_content(node).dup - return if homepage.nil? || homepage.empty? - - homepage.sub!("readthedocs.org", "readthedocs.io") - homepage.delete_suffix!(".git") if homepage.start_with?("https://github.com") - corrector.replace(node.source_range, "\"#{homepage}\"") + problem "Please use https:// for #{homepage}" do |corrector| + corrector.replace(homepage_parameter_node.source_range, "\"#{homepage.sub("http", "https")}\"") + end end end end diff --git a/Library/Homebrew/test/rubocops/homepage_spec.rb b/Library/Homebrew/test/rubocops/homepage_spec.rb index cfabcb0921..1458673a9c 100644 --- a/Library/Homebrew/test/rubocops/homepage_spec.rb +++ b/Library/Homebrew/test/rubocops/homepage_spec.rb @@ -7,64 +7,149 @@ describe RuboCop::Cop::FormulaAudit::Homepage do subject(:cop) { described_class.new } context "When auditing homepage" do - it "When there is no homepage" do - source = <<~RUBY + it "reports an offense when there is no homepage" do + expect_offense(<<~RUBY) class Foo < Formula + ^^^^^^^^^^^^^^^^^^^ Formula should have a homepage. url 'https://brew.sh/foo-1.0.tgz' end RUBY - - expected_offenses = [{ message: "Formula should have a homepage.", - severity: :convention, - line: 1, - column: 0, - source: source }] - - inspect_source(source) - - expected_offenses.zip(cop.offenses).each do |expected, actual| - expect_offense(expected, actual) - end end - it "Homepage with ftp" do - source = <<~RUBY + it "reports an offense when the homepage is not HTTP or HTTPS" do + expect_offense(<<~RUBY) class Foo < Formula homepage "ftp://brew.sh/foo" + ^^^^^^^^^^^^^^^^^^^ The homepage should start with http or https. + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports an offense for freedesktop.org wiki pages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://www.freedesktop.org/wiki/bar" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Freedesktop homepages should be styled `https://wiki.freedesktop.org/project_name` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports an offense for freedesktop.org software wiki pages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://www.freedesktop.org/wiki/Software/baz" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Freedesktop homepages should be styled `https://wiki.freedesktop.org/www/Software/project_name` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports and corrects Google Code homepages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://code.google.com/p/qux" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Google Code homepages should end with a slash url "https://brew.sh/foo-1.0.tgz" end RUBY - expected_offenses = [{ message: "The homepage should start with http or " \ - "https (URL is ftp://brew.sh/foo).", - severity: :convention, - line: 2, - column: 2, - source: source }] + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://code.google.com/p/qux/" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end - inspect_source(source) + it "reports and corrects GitHub homepages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://github.com/foo/bar.git" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ GitHub homepages should not end with .git + url "https://brew.sh/foo-1.0.tgz" + end + RUBY - expected_offenses.zip(cop.offenses).each do |expected, actual| - expect_offense(expected, actual) + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://github.com/foo/bar" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + context "for Sourceforge" do + correct_formula = <<~RUBY + class Foo < Formula + homepage "https://foo.sourceforge.io/" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + it "reports and corrects [1]" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://foo.sourceforge.net/" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sourceforge homepages should be `https://foo.sourceforge.io/` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(correct_formula) + end + + it "reports and corrects [2]" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://foo.sourceforge.net" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sourceforge homepages should be `https://foo.sourceforge.io/` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(correct_formula) + end + + it "reports and corrects [3]" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "http://foo.sf.net/" + ^^^^^^^^^^^^^^^^^^^^ Sourceforge homepages should be `https://foo.sourceforge.io/` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(correct_formula) end end - it "Homepage URLs" do + it "reports and corrects readthedocs.org pages" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://foo.readthedocs.org" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Readthedocs homepages should be `https://foo.readthedocs.io` + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://foo.readthedocs.io" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY + end + + it "reports an offense for HTTP homepages" do formula_homepages = { - "bar" => "http://www.freedesktop.org/wiki/bar", - "baz" => "http://www.freedesktop.org/wiki/Software/baz", - "qux" => "https://code.google.com/p/qux", - "quux" => "http://github.com/quux", + "sf" => "http://foo.sourceforge.io/", "corge" => "http://savannah.nongnu.org/corge", "grault" => "http://grault.github.io/", "garply" => "http://www.gnome.org/garply", - "sf1" => "http://foo.sourceforge.net/", - "sf2" => "http://foo.sourceforge.net", - "sf3" => "http://foo.sf.net/", - "sf4" => "http://foo.sourceforge.io/", "waldo" => "http://www.gnu.org/waldo", - "dotgit" => "https://github.com/foo/bar.git", - "rtd" => "https://foo.readthedocs.org", + "dotgit" => "http://github.com/quux", } formula_homepages.each do |name, homepage| @@ -75,65 +160,19 @@ describe RuboCop::Cop::FormulaAudit::Homepage do end RUBY - inspect_source(source) - if homepage.include?("http://www.freedesktop.org") - if homepage.include?("Software") - expected_offenses = [{ message: "#{homepage} should be styled " \ - "`https://wiki.freedesktop.org/www/Software/project_name`", - severity: :convention, - line: 2, - column: 2, - source: source }] - else - expected_offenses = [{ message: "#{homepage} should be styled " \ - "`https://wiki.freedesktop.org/project_name`", - severity: :convention, - line: 2, - column: 2, - source: source }] - end - elsif homepage.include?("https://code.google.com") - expected_offenses = [{ message: "#{homepage} should end with a slash", - severity: :convention, - line: 2, - column: 2, - source: source }] - elsif homepage.match?(/foo\.(sf|sourceforge)\.net/) - expected_offenses = [{ message: "#{homepage} should be `https://foo.sourceforge.io/`", - severity: :convention, - line: 2, - column: 2, - source: source }] - elsif homepage.match?("https://github.com/foo/bar.git") - expected_offenses = [{ message: "GitHub homepages (`#{homepage}`) should not end with .git", - severity: :convention, - line: 2, - column: 11, - source: source }] - elsif homepage.match?("https://foo.readthedocs.org") - expected_offenses = [{ message: "#{homepage} should be `https://foo.readthedocs.io`", - severity: :convention, - line: 2, - column: 11, - source: source }] - else - expected_offenses = [{ message: "Please use https:// for #{homepage}", - severity: :convention, - line: 2, - column: 2, - source: source }] - end - expected_offenses.zip([cop.offenses.last]).each do |expected, actual| - expect_offense(expected, actual) + expected_offenses = [{ message: "Please use https:// for #{homepage}", + severity: :convention, + line: 2, + column: 11, + source: source }] + + expected_offenses.zip([inspect_source(source).last]).each do |expected, actual| + expect(actual.message).to eq(expected[:message]) + expect(actual.severity).to eq(expected[:severity]) + expect(actual.line).to eq(expected[:line]) + expect(actual.column).to eq(expected[:column]) end end end - - def expect_offense(expected, actual) - expect(actual.message).to eq(expected[:message]) - expect(actual.severity).to eq(expected[:severity]) - expect(actual.line).to eq(expected[:line]) - expect(actual.column).to eq(expected[:column]) - end end end From f57c96465ce0ebff36ac18b58accaf21a022dae5 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 16:15:52 +1100 Subject: [PATCH 11/19] rubocops/conflicts: use rubocop v1 API --- Library/Homebrew/rubocops/conflicts.rb | 36 ++++++------ .../Homebrew/test/rubocops/conflicts_spec.rb | 57 ++++++------------- 2 files changed, 35 insertions(+), 58 deletions(-) diff --git a/Library/Homebrew/rubocops/conflicts.rb b/Library/Homebrew/rubocops/conflicts.rb index dd8e524e14..fab244784c 100644 --- a/Library/Homebrew/rubocops/conflicts.rb +++ b/Library/Homebrew/rubocops/conflicts.rb @@ -9,6 +9,8 @@ module RuboCop module FormulaAudit # This cop audits versioned formulae for `conflicts_with`. class Conflicts < FormulaCop + extend AutoCorrector + MSG = "Versioned formulae should not use `conflicts_with`. " \ "Use `keg_only :versioned_formula` instead." @@ -19,31 +21,29 @@ module RuboCop reason = parameters(conflicts_with_call).last.values.first offending_node(reason) name = Regexp.new(@formula_name, Regexp::IGNORECASE) - reason = string_content(reason).sub(name, "") - first_word = reason.split.first + reason_text = string_content(reason).sub(name, "") + first_word = reason_text.split.first - if reason.match?(/\A[A-Z]/) - problem "'#{first_word}' from the `conflicts_with` reason should be '#{first_word.downcase}'." + if reason_text.match?(/\A[A-Z]/) + problem "'#{first_word}' from the `conflicts_with` reason "\ + "should be '#{first_word.downcase}'." do |corrector| + reason_text[0] = reason_text[0].downcase + corrector.replace(reason.source_range, "\"#{reason_text}\"") + end end + next unless reason_text.end_with?(".") - problem "`conflicts_with` reason should not end with a period." if reason.end_with?(".") + problem "`conflicts_with` reason should not end with a period." do |corrector| + corrector.replace(reason.source_range, "\"#{reason_text.chop}\"") + end end return unless versioned_formula? - problem MSG if !tap_style_exception?(:versioned_formulae_conflicts_allowlist) && - method_called_ever?(body_node, :conflicts_with) - end - - def autocorrect(node) - lambda do |corrector| - if versioned_formula? - corrector.replace(node.source_range, "keg_only :versioned_formula") - else - reason = string_content(node) - reason[0] = reason[0].downcase - reason = reason.delete_suffix(".") - corrector.replace(node.source_range, "\"#{reason}\"") + if !tap_style_exception?(:versioned_formulae_conflicts_allowlist) && method_called_ever?(body_node, + :conflicts_with) + problem MSG do |corrector| + corrector.replace(@offensive_node.source_range, "keg_only :versioned_formula") end end end diff --git a/Library/Homebrew/test/rubocops/conflicts_spec.rb b/Library/Homebrew/test/rubocops/conflicts_spec.rb index 88f940c42c..c772b02642 100644 --- a/Library/Homebrew/test/rubocops/conflicts_spec.rb +++ b/Library/Homebrew/test/rubocops/conflicts_spec.rb @@ -7,7 +7,7 @@ describe RuboCop::Cop::FormulaAudit::Conflicts do subject(:cop) { described_class.new } context "when auditing `conflicts_with`" do - it "reports an offense if reason is capitalized" do + it "reports and corrects an offense if reason is capitalized" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -16,9 +16,17 @@ describe RuboCop::Cop::FormulaAudit::Conflicts do conflicts_with "baz", :because => "Foo is the formula name which does not require downcasing" end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + conflicts_with "bar", :because => "reason" + conflicts_with "baz", :because => "Foo is the formula name which does not require downcasing" + end + RUBY end - it "reports an offense if reason ends with a period" do + it "reports and corrects an offense if reason ends with a period" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -26,6 +34,13 @@ describe RuboCop::Cop::FormulaAudit::Conflicts do ^^^^^^^^^ `conflicts_with` reason should not end with a period. end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + conflicts_with "bar", "baz", :because => "reason" + end + RUBY end it "reports an offense if it is present in a versioned formula" do @@ -46,43 +61,5 @@ describe RuboCop::Cop::FormulaAudit::Conflicts do end RUBY end - - it "auto-corrects capitalized reason" do - source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - conflicts_with "bar", :because => "Reason" - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - conflicts_with "bar", :because => "reason" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "auto-corrects trailing period" do - source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - conflicts_with "bar", :because => "reason." - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - conflicts_with "bar", :because => "reason" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end end end From c3f797d9ac2f5c375f9b7c7c82d427d71f58a04c Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 16:36:06 +1100 Subject: [PATCH 12/19] rubocops/components_order: use rubocop v1 API --- Library/Homebrew/rubocops/components_order.rb | 21 +- .../test/rubocops/components_order_spec.rb | 787 ++++++++++-------- 2 files changed, 443 insertions(+), 365 deletions(-) diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index eea8093512..03f62c39c2 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -12,6 +12,8 @@ module RuboCop # - `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 + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) @present_components, @offensive_nodes = check_order(FORMULA_COMPONENT_PRECEDENCE_LIST, body_node) @@ -126,17 +128,6 @@ module RuboCop end end - # {autocorrect} gets called just after {component_problem}. - def autocorrect(_node) - return if @offensive_nodes.nil? - - succeeding_node = @offensive_nodes[0] - preceding_node = @offensive_nodes[1] - lambda do |corrector| - reorder_components(corrector, succeeding_node, preceding_node) - end - end - # Reorder two nodes in the source, using the corrector instance in autocorrect method. # Components of same type are grouped together when rewriting the source. # Linebreaks are introduced if components are of two different methods/blocks/multilines. @@ -197,13 +188,15 @@ module RuboCop nil end - # Method to format message for reporting component precedence violations. + # Method to report and correct component precedence violations. def component_problem(c1, c2) return if tap_style_exception? :components_order_exceptions problem "`#{format_component(c1)}` (line #{line_number(c1)}) " \ - "should be put before `#{format_component(c2)}` " \ - "(line #{line_number(c2)})" + "should be put before `#{format_component(c2)}` " \ + "(line #{line_number(c2)})" do |corrector| + reorder_components(corrector, c1, c2) + end end # Node pattern method to match diff --git a/Library/Homebrew/test/rubocops/components_order_spec.rb b/Library/Homebrew/test/rubocops/components_order_spec.rb index 862b639def..5c7a26e0b8 100644 --- a/Library/Homebrew/test/rubocops/components_order_spec.rb +++ b/Library/Homebrew/test/rubocops/components_order_spec.rb @@ -7,7 +7,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do subject(:cop) { described_class.new } context "When auditing formula components order" do - it "When uses_from_macos precedes depends_on" do + it "reports and corrects an offense when `uses_from_macos` precedes `depends_on`" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -18,9 +18,20 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^ `depends_on` (line 6) should be put before `uses_from_macos` (line 5) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + + depends_on "foo" + + uses_from_macos "apple" + end + RUBY end - it "When license precedes sha256" do + it "reports and corrects an offense when `license` precedes `sha256`" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -30,9 +41,18 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^^ `sha256` (line 5) should be put before `license` (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + sha256 "samplesha256" + license "0BSD" + end + RUBY end - it "When `bottle` precedes `livecheck`" do + it "reports and corrects an offense when `bottle` precedes `livecheck`" do expect_offense(<<~RUBY) class Foo < Formula homepage "https://brew.sh" @@ -47,9 +67,23 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + + livecheck do + url "https://brew.sh/foo/versions/" + regex(/href=.+?foo-(\d+(?:\.\d+)+)\.t/) + end + + bottle :unneeded + end + RUBY end - it "When url precedes homepage" do + it "reports and corrects an offense when `url` precedes `homepage`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -57,9 +91,16 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^^^^^^^ `homepage` (line 3) should be put before `url` (line 2) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + end + RUBY end - it "When `resource` precedes `depends_on`" do + it "reports and corrects an offense when `resource` precedes `depends_on`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -72,9 +113,21 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^ `depends_on` (line 8) should be put before `resource` (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + depends_on "openssl" + + resource "foo2" do + url "https://brew.sh/foo-2.0.tgz" + end + end + RUBY end - it "When `test` precedes `plist`" do + it "reports and corrects an offense when `test` precedes `plist`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -88,9 +141,22 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + def plist + end + + test do + expect(shell_output("./dogs")).to match("Dogs are terrific") + end + end + RUBY end - it "When `install` precedes `depends_on`" do + it "reports and corrects an offense when `install` precedes `depends_on`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -102,9 +168,20 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^ `depends_on` (line 7) should be put before `install` (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + depends_on "openssl" + + def install + end + end + RUBY end - it "When `test` precedes `depends_on`" do + it "reports and corrects an offense when `test` precedes `depends_on`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -119,9 +196,23 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^ `depends_on` (line 10) should be put before `install` (line 4) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + depends_on "openssl" + + def install + end + + def test + end + end + RUBY end - it "When only one of many `depends_on` precedes `conflicts_with`" do + it "reports and corrects an offense when only one of many `depends_on` precedes `conflicts_with`" do expect_offense(<<~RUBY) class Foo < Formula depends_on "autoconf" => :build @@ -133,9 +224,20 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do depends_on "gettext" end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + depends_on "autoconf" => :build + depends_on "automake" => :build + depends_on "libtool" => :build + depends_on "pkg-config" => :build + depends_on "gettext" + conflicts_with "visionmedia-watch" + end + RUBY end - it "the on_macos block is not after uses_from_macos" do + it "reports and corrects an offense when the `on_macos` block precedes `uses_from_macos`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -146,9 +248,20 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^^ `uses_from_macos` (line 6) should be put before `on_macos` (line 3) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "bar" + + on_macos do + depends_on "readline" + end + end + RUBY end - it "the on_linux block is not after uses_from_macos" do + it "reports and corrects an offense when the `on_linux` block precedes `uses_from_macos`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -159,9 +272,20 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do ^^^^^^^^^^^^^^^^^^^^^ `uses_from_macos` (line 6) should be put before `on_linux` (line 3) end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "bar" + + on_linux do + depends_on "readline" + end + end + RUBY end - it "the on_linux block is not after the on_macos block" do + it "reports and corrects an offense when the `on_linux` block precedes the `on_macos` block" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -174,82 +298,43 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do end end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + on_macos do + depends_on "readline" + end + + on_linux do + depends_on "vim" + end + end + RUBY end end - context "When auditing formula components order with autocorrect" do - it "When url precedes homepage" do - source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - homepage "https://brew.sh" - end - RUBY + it "reports and corrects an offense when `depends_on` precedes `deprecate!`" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" - correct_source = <<~RUBY - class Foo < Formula - homepage "https://brew.sh" - url "https://brew.sh/foo-1.0.tgz" - end - RUBY + depends_on "openssl" - corrected_source = autocorrect_source(source) - expect(corrected_source).to eq(correct_source) - end + deprecate! because: "has been replaced by bar" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `deprecate!` (line 6) should be put before `depends_on` (line 4) + end + RUBY - it "When `resource` precedes `depends_on`" do - source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" + expect_correction(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" - resource "foo2" do - url "https://brew.sh/foo-2.0.tgz" - end + deprecate! because: "has been replaced by bar" - depends_on "openssl" - end - RUBY - - correct_source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - depends_on "openssl" - - resource "foo2" do - url "https://brew.sh/foo-2.0.tgz" - end - end - RUBY - - corrected_source = autocorrect_source(source) - expect(corrected_source).to eq(correct_source) - end - - it "When `depends_on` precedes `deprecate!`" do - source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - depends_on "openssl" - - deprecate! because: "has been replaced by bar" - end - RUBY - - correct_source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - deprecate! because: "has been replaced by bar" - - depends_on "openssl" - end - RUBY - - corrected_source = autocorrect_source(source) - expect(corrected_source).to eq(correct_source) - end + depends_on "openssl" + end + RUBY end context "no on_os_block" do @@ -351,321 +436,321 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do end RUBY end - end - it "there can only be one on_macos block" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - on_macos do - depends_on "readline" - end - - on_macos do - ^^^^^^^^^^^ there can only be one `on_macos` block in a formula. - depends_on "foo" - end - end - RUBY - end - - it "there can only be one on_linux block" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - on_linux do - depends_on "readline" - end - - on_linux do - ^^^^^^^^^^^ there can only be one `on_linux` block in a formula. - depends_on "foo" - end - end - RUBY - end - - it "the on_macos block can only contain depends_on, patch and resource nodes" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - on_macos do - depends_on "readline" - uses_from_macos "ncurses" - ^^^^^^^^^^^^^^^^^^^^^^^^^ `on_macos` cannot include `uses_from_macos`. [...] - end - end - RUBY - end - - it "the on_linux block can only contain depends_on, patch and resource nodes" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - on_linux do - depends_on "readline" - uses_from_macos "ncurses" - ^^^^^^^^^^^^^^^^^^^^^^^^^ `on_linux` cannot include `uses_from_macos`. [...] - end - end - RUBY - end - - context "in a resource block" do - it "reports no offenses for a valid on_macos and on_linux block" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - homepage "https://brew.sh" - - resource do - on_macos do - url "https://brew.sh/resource1.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_linux do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - end - end - RUBY - end - - it "reports no offenses for a valid on_macos and on_linux block with versions" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - homepage "https://brew.sh" - - resource do - on_macos do - url "https://brew.sh/resource1.tar.gz" - version "1.2.3" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_linux do - url "https://brew.sh/resource2.tar.gz" - version "1.2.3" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - end - end - RUBY - end - - it "reports an offense if there are two on_macos blocks" do + it "reports an offense when there are multiple `on_macos` blocks" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" + on_macos do + depends_on "readline" + end - resource do - ^^^^^^^^^^^ there can only be one `on_macos` block in a resource block. - on_macos do - url "https://brew.sh/resource1.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_macos do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end + on_macos do + ^^^^^^^^^^^ there can only be one `on_macos` block in a formula. + depends_on "foo" end end RUBY end - it "reports an offense if there are two on_linux blocks" do + it "reports an offense when there are multiple `on_linux` blocks" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" + on_linux do + depends_on "readline" + end - resource do - ^^^^^^^^^^^ there can only be one `on_linux` block in a resource block. - on_linux do - url "https://brew.sh/resource1.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_linux do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end + on_linux do + ^^^^^^^^^^^ there can only be one `on_linux` block in a formula. + depends_on "foo" end end RUBY end - it "reports no offenses if there is an on_macos block but no on_linux block" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - resource do - on_macos do - url "https://brew.sh/resource1.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - end - end - RUBY - end - - it "reports no offenses if there is an on_linux block but no on_macos block" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - resource do - on_linux do - url "https://brew.sh/resource1.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - end - end - RUBY - end - - it "reports an offense if the content of an on_macos block is improperly formatted" do + it "reports an offense when the `on_macos` block contains nodes other than `depends_on`, `patch` or `resource`" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" - - resource do - ^^^^^^^^^^^ `on_macos` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). - on_macos do - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - url "https://brew.sh/resource2.tar.gz" - end - - on_linux do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end + on_macos do + depends_on "readline" + uses_from_macos "ncurses" + ^^^^^^^^^^^^^^^^^^^^^^^^^ `on_macos` cannot include `uses_from_macos`. [...] end end RUBY end - it "reports no offenses if an on_macos block has if-else branches that are properly formatted" do - expect_no_offenses(<<~RUBY) + it "reports an offense when the `on_linux` block contains nodes other than `depends_on`, `patch` or `resource`" do + expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" + on_linux do + depends_on "readline" + uses_from_macos "ncurses" + ^^^^^^^^^^^^^^^^^^^^^^^^^ `on_linux` cannot include `uses_from_macos`. [...] + end + end + RUBY + end - resource do - on_macos do - if foo == :bar - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - else + context "in a resource block" do + it "reports no offenses for a valid `on_macos` and `on_linux` block" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + + resource do + on_macos do url "https://brew.sh/resource1.tar.gz" - sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" end - end - on_linux do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - end - end - RUBY - end - - it "reports an offense if an on_macos block has if-else branches that aren't properly formatted" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - resource do - ^^^^^^^^^^^ `on_macos` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). - on_macos do - if foo == :bar + on_linux do url "https://brew.sh/resource2.tar.gz" sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - else - sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - url "https://brew.sh/resource1.tar.gz" - end - end - - on_linux do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - end - end - RUBY - end - - it "reports an offense if the content of an on_linux block is improperly formatted" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - resource do - ^^^^^^^^^^^ `on_linux` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). - on_macos do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_linux do - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - url "https://brew.sh/resource2.tar.gz" - end - end - end - RUBY - end - - it "reports no offenses if an on_linux block has if-else branches that are properly formatted" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - resource do - on_macos do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_linux do - if foo == :bar - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - else - url "https://brew.sh/resource1.tar.gz" - sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" end end end - end - RUBY - end + RUBY + end - it "reports an offense if an on_linux block has if-else branches that aren't properly formatted" do - expect_offense(<<~RUBY) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" + it "reports no offenses for a valid `on_macos` and `on_linux` block with versions" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" - resource do - ^^^^^^^^^^^ `on_linux` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). - on_macos do - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - end - - on_linux do - if foo == :bar - url "https://brew.sh/resource2.tar.gz" - sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" - else - sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + resource do + on_macos do url "https://brew.sh/resource1.tar.gz" + version "1.2.3" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + + on_linux do + url "https://brew.sh/resource2.tar.gz" + version "1.2.3" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" end end end - end - RUBY + RUBY + end + + it "reports an offense if there are two `on_macos` blocks" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + ^^^^^^^^^^^ there can only be one `on_macos` block in a resource block. + on_macos do + url "https://brew.sh/resource1.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + + on_macos do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports an offense if there are two `on_linux` blocks" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + ^^^^^^^^^^^ there can only be one `on_linux` block in a resource block. + on_linux do + url "https://brew.sh/resource1.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + + on_linux do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports no offenses if there is an `on_macos` block but no `on_linux` block" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + resource do + on_macos do + url "https://brew.sh/resource1.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports no offenses if there is an `on_linux` block but no `on_macos` block" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + resource do + on_linux do + url "https://brew.sh/resource1.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports an offense if the content of an `on_macos` block is improperly formatted" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + ^^^^^^^^^^^ `on_macos` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). + on_macos do + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + url "https://brew.sh/resource2.tar.gz" + end + + on_linux do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports no offenses if an `on_macos` block has if-else branches that are properly formatted" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + on_macos do + if foo == :bar + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + else + url "https://brew.sh/resource1.tar.gz" + sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + + on_linux do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports an offense if an `on_macos` block has if-else branches that aren't properly formatted" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + ^^^^^^^^^^^ `on_macos` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). + on_macos do + if foo == :bar + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + else + sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + url "https://brew.sh/resource1.tar.gz" + end + end + + on_linux do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + RUBY + end + + it "reports an offense if the content of an `on_linux` block is improperly formatted" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + ^^^^^^^^^^^ `on_linux` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). + on_macos do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + + on_linux do + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + url "https://brew.sh/resource2.tar.gz" + end + end + end + RUBY + end + + it "reports no offenses if an `on_linux` block has if-else branches that are properly formatted" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + on_macos do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + + on_linux do + if foo == :bar + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + else + url "https://brew.sh/resource1.tar.gz" + sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + end + RUBY + end + + it "reports an offense if an `on_linux` block has if-else branches that aren't properly formatted" do + expect_offense(<<~RUBY) + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + + resource do + ^^^^^^^^^^^ `on_linux` blocks within resource blocks must contain only a url and sha256 or a url, version, and sha256 (in those orders). + on_macos do + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + + on_linux do + if foo == :bar + url "https://brew.sh/resource2.tar.gz" + sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + else + sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + url "https://brew.sh/resource1.tar.gz" + end + end + end + end + RUBY + end end end end From bb497ebfe0a066126129f768edc12710689aa59c Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 16:38:15 +1100 Subject: [PATCH 13/19] rubocops/caveats: use rubocop v1 API --- Library/Homebrew/test/rubocops/caveats_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/test/rubocops/caveats_spec.rb b/Library/Homebrew/test/rubocops/caveats_spec.rb index e59a04a2e8..1d161b6901 100644 --- a/Library/Homebrew/test/rubocops/caveats_spec.rb +++ b/Library/Homebrew/test/rubocops/caveats_spec.rb @@ -14,7 +14,7 @@ describe RuboCop::Cop::FormulaAudit::Caveats do url "https://brew.sh/foo-1.0.tgz" def caveats "setuid" - ^^^^^^ Don\'t recommend setuid in the caveats, suggest sudo instead. + ^^^^^^^^ Don\'t recommend setuid in the caveats, suggest sudo instead. end end RUBY From df8e03017489f521d3dc39696708f502c254ba69 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 16:54:53 +1100 Subject: [PATCH 14/19] rubocops/patches: use rubocop v1 API --- Library/Homebrew/rubocops/patches.rb | 3 +- .../Homebrew/test/rubocops/patches_spec.rb | 52 +++++++++---------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/Library/Homebrew/rubocops/patches.rb b/Library/Homebrew/rubocops/patches.rb index 4471a18566..10692a497f 100644 --- a/Library/Homebrew/rubocops/patches.rb +++ b/Library/Homebrew/rubocops/patches.rb @@ -8,6 +8,7 @@ module RuboCop module Cop module FormulaAudit # This cop audits `patch`es in formulae. + # TODO: Many of these could be auto-corrected. class Patches < FormulaCop extend T::Sig @@ -26,7 +27,7 @@ module RuboCop if inline_patches.empty? && patch_end? offending_patch_end_node(node) - problem "patch is missing 'DATA'" + add_offense(@offense_source_range, message: "patch is missing 'DATA'") end patches_node = find_method_def(body, :patches) diff --git a/Library/Homebrew/test/rubocops/patches_spec.rb b/Library/Homebrew/test/rubocops/patches_spec.rb index 6dc31fa92f..df785e9a25 100644 --- a/Library/Homebrew/test/rubocops/patches_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_spec.rb @@ -7,7 +7,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do subject(:cop) { described_class.new } context "When auditing legacy patches" do - it "When there is no legacy patch" do + it "reports no offenses when there is no legacy patch" do expect_no_offenses(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -15,7 +15,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do RUBY end - it "Formula with `def patches`" do + it "reports an offense if `def patches` is present" do expect_offense(<<~RUBY) class Foo < Formula homepage "ftp://brew.sh/foo" @@ -28,7 +28,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do RUBY end - it "Patch URLs" do + it "reports an offense for various patch URLs" do patch_urls = [ "https://raw.github.com/mogaal/sendemail", "https://mirrors.ustc.edu.cn/macports/trunk/", @@ -48,7 +48,6 @@ describe RuboCop::Cop::FormulaAudit::Patches do end EOS - inspect_source(source) expected_offense = if patch_url.include?("/raw.github.com/") [{ message: <<~EOS.chomp, @@ -57,7 +56,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 12, + column: 4, source: source }] elsif patch_url.include?("macports/trunk") [{ message: @@ -67,7 +66,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 33, + column: 4, source: source }] elsif patch_url.start_with?("http://trac.macports.org") [{ message: @@ -77,7 +76,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 5, + column: 4, source: source }] elsif patch_url.start_with?("http://bugs.debian.org") [{ message: @@ -87,7 +86,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 5, + column: 4, source: source }] # rubocop:disable Layout/LineLength elsif patch_url.match?(%r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}) @@ -95,7 +94,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do [{ message: "Use a commit hash URL rather than patch-diff: #{patch_url}", severity: :convention, line: 5, - column: 5, + column: 4, source: source }] elsif patch_url.match?(%r{https?://github\.com/.+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)}) [{ message: @@ -105,10 +104,10 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 5, + column: 4, source: source }] end - expected_offense.zip([cop.offenses.last]).each do |expected, actual| + expected_offense.zip([inspect_source(source).last]).each do |expected, actual| expect(actual.message).to eq(expected[:message]) expect(actual.severity).to eq(expected[:severity]) expect(actual.line).to eq(expected[:line]) @@ -117,7 +116,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do end end - it "Formula with nested `def patches`" do + it "reports an offense with nested `def patches`" do source = <<~RUBY class Foo < Formula homepage "ftp://brew.sh/foo" @@ -144,12 +143,10 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 8, - column: 26, + column: 25, source: source }] - inspect_source(source) - - expected_offenses.zip(cop.offenses).each do |expected, actual| + expected_offenses.zip(inspect_source(source)).each do |expected, actual| expect(actual.message).to eq(expected[:message]) expect(actual.severity).to eq(expected[:severity]) expect(actual.line).to eq(expected[:line]) @@ -206,7 +203,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do end context "When auditing external patches" do - it "Patch URLs" do + it "reports an offense for various patch URLs" do patch_urls = [ "https://raw.github.com/mogaal/sendemail", "https://mirrors.ustc.edu.cn/macports/trunk/", @@ -230,7 +227,6 @@ describe RuboCop::Cop::FormulaAudit::Patches do end RUBY - inspect_source(source) expected_offense = if patch_url.include?("/raw.github.com/") [{ message: <<~EOS.chomp, @@ -239,7 +235,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 16, + column: 8, source: source }] elsif patch_url.include?("macports/trunk") [{ message: @@ -249,7 +245,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 37, + column: 8, source: source }] elsif patch_url.start_with?("http://trac.macports.org") [{ message: @@ -259,7 +255,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 9, + column: 8, source: source }] elsif patch_url.start_with?("http://bugs.debian.org") [{ message: @@ -269,19 +265,19 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 9, + column: 8, source: source }] elsif patch_url.match?(%r{https://github.com/[^/]*/[^/]*/pull}) [{ message: "Use a commit hash URL rather than an unstable pull request URL: #{patch_url}", severity: :convention, line: 5, - column: 9, + column: 8, source: source }] elsif patch_url.match?(%r{.*gitlab.*/merge_request.*}) [{ message: "Use a commit hash URL rather than an unstable merge request URL: #{patch_url}", severity: :convention, line: 5, - column: 9, + column: 8, source: source }] elsif patch_url.match?(%r{https://github.com/[^/]*/[^/]*/commit/}) [{ message: @@ -291,7 +287,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 9, + column: 8, source: source }] elsif patch_url.match?(%r{.*gitlab.*/commit/}) [{ message: @@ -301,7 +297,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS severity: :convention, line: 5, - column: 9, + column: 8, source: source }] # rubocop:disable Layout/LineLength elsif patch_url.match?(%r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}) @@ -309,10 +305,10 @@ describe RuboCop::Cop::FormulaAudit::Patches do [{ message: "Use a commit hash URL rather than patch-diff: #{patch_url}", severity: :convention, line: 5, - column: 9, + column: 8, source: source }] end - expected_offense.zip([cop.offenses.last]).each do |expected, actual| + expected_offense.zip([inspect_source(source).last]).each do |expected, actual| expect(actual.message).to eq(expected[:message]) expect(actual.severity).to eq(expected[:severity]) expect(actual.line).to eq(expected[:line]) From 929e481dce5366dcf7314cb92479511605615be2 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 17:02:48 +1100 Subject: [PATCH 15/19] rubocops/keg_only: use rubocop v1 API --- Library/Homebrew/rubocops/keg_only.rb | 11 ++- .../Homebrew/test/rubocops/keg_only_spec.rb | 72 ++++++------------- 2 files changed, 31 insertions(+), 52 deletions(-) diff --git a/Library/Homebrew/rubocops/keg_only.rb b/Library/Homebrew/rubocops/keg_only.rb index 3b811f07b8..ea2bd9131d 100644 --- a/Library/Homebrew/rubocops/keg_only.rb +++ b/Library/Homebrew/rubocops/keg_only.rb @@ -10,6 +10,8 @@ module RuboCop # # @api private class KegOnly < FormulaCop + extend AutoCorrector + def audit_formula(_node, _class_node, _parent_class_node, body_node) keg_only_node = find_node_method_by_name(body_node, :keg_only) return unless keg_only_node @@ -33,12 +35,17 @@ module RuboCop first_word = reason.split.first if reason =~ /\A[A-Z]/ && !reason.start_with?(*allowlist) - problem "'#{first_word}' from the `keg_only` reason should be '#{first_word.downcase}'." + problem "'#{first_word}' from the `keg_only` reason should be '#{first_word.downcase}'." do |corrector| + reason[0] = reason[0].downcase + corrector.replace(@offensive_node.source_range, "\"#{reason}\"") + end end return unless reason.end_with?(".") - problem "`keg_only` reason should not end with a period." + problem "`keg_only` reason should not end with a period." do |corrector| + corrector.replace(@offensive_node.source_range, "\"#{reason.chop}\"") + end end def autocorrect(node) diff --git a/Library/Homebrew/test/rubocops/keg_only_spec.rb b/Library/Homebrew/test/rubocops/keg_only_spec.rb index 16bb4c6234..c98d1ef599 100644 --- a/Library/Homebrew/test/rubocops/keg_only_spec.rb +++ b/Library/Homebrew/test/rubocops/keg_only_spec.rb @@ -6,7 +6,7 @@ require "rubocops/keg_only" describe RuboCop::Cop::FormulaAudit::KegOnly do subject(:cop) { described_class.new } - specify "keg_only_needs_downcasing" do + it "reports and corrects an offense when the `keg_only` reason is capitalized" do expect_offense(<<~RUBY) class Foo < Formula @@ -17,9 +17,19 @@ describe RuboCop::Cop::FormulaAudit::KegOnly do ^^^^^^^^^^^^^^^^^ 'Because' from the `keg_only` reason should be 'because'. end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + + url "https://brew.sh/foo-1.0.tgz" + homepage "https://brew.sh" + + keg_only "because why not" + end + RUBY end - specify "keg_only_redundant_period" do + it "reports and corrects an offense when the `keg_only` reason ends with a period" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -29,51 +39,18 @@ describe RuboCop::Cop::FormulaAudit::KegOnly do ^^^^^^^^^^^^^^^^^^^^^^^ `keg_only` reason should not end with a period. end RUBY - end - specify "keg_only_autocorrects_downcasing" do - source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" homepage "https://brew.sh" - keg_only "Because why not" - end - RUBY - corrected_source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - homepage "https://brew.sh" - keg_only "because why not" - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - specify "keg_only_autocorrects_redundant_period" do - source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - homepage "https://brew.sh" - keg_only "ending with a period." - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - homepage "https://brew.sh" keg_only "ending with a period" end RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) end - specify "keg_only_handles_block_correctly" do + it "reports no offenses when a `keg_only` reason is a block" do expect_no_offenses(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -89,7 +66,7 @@ describe RuboCop::Cop::FormulaAudit::KegOnly do RUBY end - specify "keg_only_handles_allowlist_correctly" do + it "reports no offenses if a capitalized `keg-only` reason is an exempt proper noun" do expect_no_offenses(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" @@ -100,18 +77,13 @@ describe RuboCop::Cop::FormulaAudit::KegOnly do RUBY end - specify "keg_only does not need downcasing of formula name in reason" do - filename = Formulary.core_path("foo") - File.open(filename, "w") do |file| - FileUtils.chmod "-rwx", filename + it "reports no offenses if a capitalized `keg_only` reason is the formula's name" do + expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb") + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" - expect_no_offenses(<<~RUBY, file) - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - - keg_only "Foo is the formula name hence downcasing is not required" - end - RUBY - end + keg_only "Foo is the formula name hence downcasing is not required" + end + RUBY end end From 23b8d0ccb8af07d38a67e9befbcbf5058f2e1997 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 18:12:56 +1100 Subject: [PATCH 16/19] rubocops/desc: use rubocop v1 API --- Library/Homebrew/rubocops/cask/desc.rb | 12 ++--- Library/Homebrew/rubocops/formula_desc.rb | 8 ++-- .../Homebrew/rubocops/shared/desc_helper.rb | 29 ++++++------ .../test/rubocops/formula_desc_spec.rb | 44 +++++++++---------- 4 files changed, 45 insertions(+), 48 deletions(-) diff --git a/Library/Homebrew/rubocops/cask/desc.rb b/Library/Homebrew/rubocops/cask/desc.rb index 98b6c6bbad..9b33d476d7 100644 --- a/Library/Homebrew/rubocops/cask/desc.rb +++ b/Library/Homebrew/rubocops/cask/desc.rb @@ -11,19 +11,15 @@ module RuboCop module Cask # This cop audits `desc` in casks. # See the {DescHelper} module for details of the checks. - class Desc < Cop + class Desc < Base include OnDescStanza include DescHelper + extend AutoCorrector def on_desc_stanza(stanza) - name = cask_block.header.cask_token + @name = cask_block.header.cask_token desc_call = stanza.stanza_node - audit_desc(:cask, name, desc_call) - end - - def autocorrect(node) - name = cask_block.header.cask_token - autocorrect_desc(node, name) + audit_desc(:cask, @name, desc_call) end end end diff --git a/Library/Homebrew/rubocops/formula_desc.rb b/Library/Homebrew/rubocops/formula_desc.rb index 7ce26eec6b..11dfefd3b0 100644 --- a/Library/Homebrew/rubocops/formula_desc.rb +++ b/Library/Homebrew/rubocops/formula_desc.rb @@ -12,14 +12,12 @@ module RuboCop # See the {DescHelper} module for details of the checks. class Desc < FormulaCop include DescHelper + extend AutoCorrector def audit_formula(_node, _class_node, _parent_class_node, body_node) + @name = @formula_name desc_call = find_node_method_by_name(body_node, :desc) - audit_desc(:formula, @formula_name, desc_call) - end - - def autocorrect(node) - autocorrect_desc(node, @formula_name) + audit_desc(:formula, @name, desc_call) end end end diff --git a/Library/Homebrew/rubocops/shared/desc_helper.rb b/Library/Homebrew/rubocops/shared/desc_helper.rb index f39269503c..5e14114b70 100644 --- a/Library/Homebrew/rubocops/shared/desc_helper.rb +++ b/Library/Homebrew/rubocops/shared/desc_helper.rb @@ -38,39 +38,41 @@ module RuboCop end # Check the desc for leading whitespace. - problem "Description shouldn't have leading spaces." if regex_match_group(desc, /^\s+/) + desc_problem "Description shouldn't have leading spaces." if regex_match_group(desc, /^\s+/) # Check the desc for trailing whitespace. - problem "Description shouldn't have trailing spaces." if regex_match_group(desc, /\s+$/) + desc_problem "Description shouldn't have trailing spaces." if regex_match_group(desc, /\s+$/) # Check if "command-line" is spelled incorrectly in the desc. if match = regex_match_group(desc, /(command ?line)/i) c = match.to_s[0] - problem "Description should use \"#{c}ommand-line\" instead of \"#{match}\"." + desc_problem "Description should use \"#{c}ommand-line\" instead of \"#{match}\"." end # Check if the desc starts with an article. - problem "Description shouldn't start with an article." if regex_match_group(desc, /^(the|an?)(?=\s)/i) + desc_problem "Description shouldn't start with an article." if regex_match_group(desc, /^(the|an?)(?=\s)/i) # Check if invalid lowercase words are at the start of a desc. if !VALID_LOWERCASE_WORDS.include?(string_content(desc).split.first) && regex_match_group(desc, /^[a-z]/) - problem "Description should start with a capital letter." + desc_problem "Description should start with a capital letter." end # Check if the desc starts with the formula's or cask's name. name_regex = name.delete("-").split("").join('[\s\-]?') - problem "Description shouldn't start with the #{type} name." if regex_match_group(desc, /^#{name_regex}\b/i) + if regex_match_group(desc, /^#{name_regex}\b/i) + desc_problem "Description shouldn't start with the #{type} name." + end if type == :cask && (match = regex_match_group(desc, /\b(macOS|Mac( ?OS( ?X)?)?|OS ?X)(?! virtual machines?)\b/i)) && match[1] != "MAC" - problem "Description shouldn't contain the platform." + desc_problem "Description shouldn't contain the platform." end # Check if a full stop is used at the end of a desc (apart from in the case of "etc."). if regex_match_group(desc, /\.$/) && !string_content(desc).end_with?("etc.") - problem "Description shouldn't end with a full stop." + desc_problem "Description shouldn't end with a full stop." end # Check if the desc length exceeds maximum length. @@ -80,9 +82,10 @@ module RuboCop "The current length is #{desc_length}." end - def autocorrect_desc(node, name) - lambda do |corrector| - /\A(?["'])(?.*)(?:\k)\Z/ =~ node.source + # Auto correct desc problems. `regex_match_group` must be called before this to populate @offense_source_range. + def desc_problem(message) + add_offense(@offensive_source_range, message: message) do |corrector| + /\A(?["'])(?.*)(?:\k)\Z/ =~ @offensive_node.source next if correction.nil? @@ -98,12 +101,12 @@ module RuboCop end correction.gsub!(/(ommand ?line)/i, "ommand-line") - correction.gsub!(/(^|[^a-z])#{name}([^a-z]|$)/i, "\\1\\2") + correction.gsub!(/(^|[^a-z])#{@name}([^a-z]|$)/i, "\\1\\2") correction.gsub!(/^\s+/, "") correction.gsub!(/\s+$/, "") correction.gsub!(/\.$/, "") - corrector.replace(node.source_range, "#{quote}#{correction}#{quote}") + corrector.replace(@offensive_node.source_range, "#{quote}#{correction}#{quote}") end end end diff --git a/Library/Homebrew/test/rubocops/formula_desc_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_spec.rb index 47c518537f..f0b4f1a140 100644 --- a/Library/Homebrew/test/rubocops/formula_desc_spec.rb +++ b/Library/Homebrew/test/rubocops/formula_desc_spec.rb @@ -6,8 +6,8 @@ require "rubocops/formula_desc" describe RuboCop::Cop::FormulaAudit::Desc do subject(:cop) { described_class.new } - context "When auditing formula desc" do - it "When there is no desc" do + context "When auditing formula `desc` methods" do + it "reports an offense when there is no `desc`" do expect_offense(<<~RUBY) class Foo < Formula ^^^^^^^^^^^^^^^^^^^ Formula should have a desc (Description). @@ -16,7 +16,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "When desc is an empty string" do + it "reports an offense when `desc` is an empty string" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -26,7 +26,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "When desc is too long" do + it "reports an offense when `desc` is too long" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -36,7 +36,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "When desc is a multiline string" do + it "reports an offense when `desc` is a multiline string" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -48,39 +48,39 @@ describe RuboCop::Cop::FormulaAudit::Desc do end end - context "When auditing formula desc" do - it "When the description starts with a leading space" do + context "When auditing formula description texts" do + it "reports an offense when the description starts with a leading space" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' desc ' Description with a leading space' - ^ Description shouldn\'t have leading spaces. + ^ Description shouldn't have leading spaces. end RUBY end - it "When the description ends with a trailing space" do + it "reports an offense when the description ends with a trailing space" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' desc 'Description with a trailing space ' - ^ Description shouldn\'t have trailing spaces. + ^ Description shouldn't have trailing spaces. end RUBY end - it "When \"command-line\" is incorrectly spelled in the desc" do + it "reports an offense when \"command-line\" is incorrectly spelled in the description" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' desc 'command line' ^ Description should start with a capital letter. - ^^^^^^^^^^^^ Description should use \"command-line\" instead of \"command line\". + ^^^^^^^^^^^^ Description should use "command-line" instead of "command line". end RUBY end - it "When an article is used in the desc" do + it "reports an offense when an article is used in the description" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -98,7 +98,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "When the desc starts with a lowercase letter" do + it "reports an offense when the description starts with a lowercase letter" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -108,7 +108,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "When the desc starts with the formula name" do + it "reports an offense when the description starts with the formula name" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -118,7 +118,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "When the description ends with a full stop" do + it "reports an offense when the description ends with a full stop" do expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -128,23 +128,23 @@ describe RuboCop::Cop::FormulaAudit::Desc do RUBY end - it "autocorrects all rules" do - source = <<~RUBY + it "reports and corrects all rules for description text" do + expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' desc ' an bar: commandline foo ' + ^ Description shouldn't have trailing spaces. + ^^^^^^^^^^^ Description should use "command-line" instead of "commandline". + ^ Description shouldn't have leading spaces. end RUBY - correct_source = <<~RUBY + expect_correction(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' desc 'Bar: command-line' end RUBY - - corrected_source = autocorrect_source(source, "/homebrew-core/Formula/foo.rb") - expect(corrected_source).to eq(correct_source) end end end From 0366e2f7f7e2a3809bc6ac18b44746d618b13228 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 18:16:26 +1100 Subject: [PATCH 17/19] rubocops/options: use rubocop v1 API --- Library/Homebrew/test/rubocops/options_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/test/rubocops/options_spec.rb b/Library/Homebrew/test/rubocops/options_spec.rb index e1d9981980..f532083d55 100644 --- a/Library/Homebrew/test/rubocops/options_spec.rb +++ b/Library/Homebrew/test/rubocops/options_spec.rb @@ -12,12 +12,12 @@ describe RuboCop::Cop::FormulaAudit::Options do class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' option "with-32-bit" - ^^^^^^ macOS has been 64-bit only since 10.6 so 32-bit options are deprecated. + ^^^^^^^^^^^^^ macOS has been 64-bit only since 10.6 so 32-bit options are deprecated. end RUBY end - it "with universal" do + it "reports an offense when using `:universal`" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -27,7 +27,7 @@ describe RuboCop::Cop::FormulaAudit::Options do RUBY end - it "with bad option names" do + it "reports an offense when using bad option names" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -38,7 +38,7 @@ describe RuboCop::Cop::FormulaAudit::Options do RUBY end - it "with without-check option name" do + it "reports an offense when using `without-check` option names" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -48,7 +48,7 @@ describe RuboCop::Cop::FormulaAudit::Options do RUBY end - it "with deprecated_optionss" do + it "reports an offense when using `deprecated_option` in homebrew/core" do expect_offense(<<~RUBY, "/homebrew-core/") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -58,7 +58,7 @@ describe RuboCop::Cop::FormulaAudit::Options do RUBY end - it "with options" do + it "reports an offense when using `option` in homebrew/core" do expect_offense(<<~RUBY, "/homebrew-core/") class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' From 30f7a872b50814564887d59556fe5dc6a0186170 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 18:20:30 +1100 Subject: [PATCH 18/19] rubocops/unless_multiple_conditions: use rubocop v1 API --- .../Homebrew/rubocops/unless_multiple_conditions.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/rubocops/unless_multiple_conditions.rb b/Library/Homebrew/rubocops/unless_multiple_conditions.rb index 0ad8a15602..4e4d673e47 100644 --- a/Library/Homebrew/rubocops/unless_multiple_conditions.rb +++ b/Library/Homebrew/rubocops/unless_multiple_conditions.rb @@ -7,8 +7,9 @@ module RuboCop # This cop checks that `unless` is not used with multiple conditions. # # @api private - class UnlessMultipleConditions < Cop + class UnlessMultipleConditions < Base extend T::Sig + extend AutoCorrector MSG = "Avoid using `unless` with multiple conditions." @@ -16,12 +17,7 @@ module RuboCop 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| + add_offense(node.condition.source_range.with(begin_pos: node.loc.keyword.begin_pos)) 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| From ac8eb00443be063832a4971c182cbda7454095bb Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Tue, 12 Jan 2021 18:25:05 +1100 Subject: [PATCH 19/19] rubocops/cask/*: use rubocop v1 API --- .../cask/homepage_url_trailing_slash.rb | 16 ++++------- .../Homebrew/rubocops/cask/no_dsl_version.rb | 15 ++++------ .../Homebrew/rubocops/cask/stanza_grouping.rb | 28 ++++++++----------- .../Homebrew/rubocops/cask/stanza_order.rb | 20 ++++++------- .../rubocops/cask/shared_examples/cask_cop.rb | 9 +++--- 5 files changed, 34 insertions(+), 54 deletions(-) diff --git a/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb b/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb index a3a18a9caf..313f9ef04f 100644 --- a/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb +++ b/Library/Homebrew/rubocops/cask/homepage_url_trailing_slash.rb @@ -9,8 +9,9 @@ module RuboCop module Cask # This cop checks that a cask's homepage ends with a slash # if it does not have a path component. - class HomepageUrlTrailingSlash < Cop + class HomepageUrlTrailingSlash < Base include OnHomepageStanza + extend AutoCorrector MSG_NO_SLASH = "'%s' must have a slash after the domain." @@ -26,19 +27,14 @@ module RuboCop return unless url&.match?(%r{^.+://[^/]+$}) - add_offense(url_node, location: :expression, - message: format(MSG_NO_SLASH, url: url)) - end - - def autocorrect(node) - domain = URI(node.str_content).host + domain = URI(url_node.str_content).host # This also takes URLs like 'https://example.org?path' # and 'https://example.org#path' into account. - corrected_source = node.source.sub("://#{domain}", "://#{domain}/") + corrected_source = url_node.source.sub("://#{domain}", "://#{domain}/") - lambda do |corrector| - corrector.replace(node.source_range, corrected_source) + add_offense(url_node.loc.expression, message: format(MSG_NO_SLASH, url: url)) do |corrector| + corrector.replace(url_node.source_range, corrected_source) end end end diff --git a/Library/Homebrew/rubocops/cask/no_dsl_version.rb b/Library/Homebrew/rubocops/cask/no_dsl_version.rb index 7866e52fe6..63dbda85e6 100644 --- a/Library/Homebrew/rubocops/cask/no_dsl_version.rb +++ b/Library/Homebrew/rubocops/cask/no_dsl_version.rb @@ -18,10 +18,11 @@ module RuboCop # cask 'foo' do # ... # end - class NoDslVersion < Cop + class NoDslVersion < Base extend T::Sig extend Forwardable + extend AutoCorrector include CaskHelp MESSAGE = "Use `%s` instead of `%s`" @@ -33,13 +34,6 @@ module RuboCop offense end - def autocorrect(method_node) - @cask_header = cask_header(method_node) - lambda do |corrector| - corrector.replace(header_range, preferred_header_str) - end - end - private def_delegator :@cask_header, :source_range, :header_range @@ -54,8 +48,9 @@ module RuboCop end def offense - add_offense(@cask_header.method_node, location: header_range, - message: error_msg) + add_offense(header_range, message: error_msg) do |corrector| + corrector.replace(header_range, preferred_header_str) + end end sig { returns(String) } diff --git a/Library/Homebrew/rubocops/cask/stanza_grouping.rb b/Library/Homebrew/rubocops/cask/stanza_grouping.rb index 6d17bbb9df..0e68b230d0 100644 --- a/Library/Homebrew/rubocops/cask/stanza_grouping.rb +++ b/Library/Homebrew/rubocops/cask/stanza_grouping.rb @@ -8,8 +8,9 @@ module RuboCop module Cask # This cop checks that a cask's stanzas are grouped correctly. # @see https://github.com/Homebrew/homebrew-cask/blob/HEAD/doc/cask_language_reference/readme.md#stanza-order - class StanzaGrouping < Cop + class StanzaGrouping < Base extend Forwardable + extend AutoCorrector include CaskHelp include RangeHelp @@ -25,17 +26,6 @@ module RuboCop add_offenses end - def autocorrect(range) - lambda do |corrector| - case line_ops[range.line - 1] - when :insert - corrector.insert_before(range, "\n") - when :remove - corrector.remove(range) - end - end - end - private attr_reader :cask_block, :line_ops @@ -79,20 +69,24 @@ module RuboCop def add_offense_missing_line(stanza) line_index = index_of_line_after(stanza) line_ops[line_index] = :insert - add_offense(line_index, message: MISSING_LINE_MSG) + add_offense(line_index, message: MISSING_LINE_MSG) do |corrector| + corrector.insert_before(@range, "\n") + end end def add_offense_extra_line(stanza) line_index = index_of_line_after(stanza) line_ops[line_index] = :remove - add_offense(line_index, message: EXTRA_LINE_MSG) + add_offense(line_index, message: EXTRA_LINE_MSG) do |corrector| + corrector.remove(@range) + end end def add_offense(line_index, message:) line_length = [processed_source[line_index].size, 1].max - range = source_range(processed_source.buffer, line_index + 1, 0, - line_length) - super(range, location: range, message: message) + @range = source_range(processed_source.buffer, line_index + 1, 0, + line_length) + super(@range, message: message) end end end diff --git a/Library/Homebrew/rubocops/cask/stanza_order.rb b/Library/Homebrew/rubocops/cask/stanza_order.rb index af044d8b75..96f1b057c7 100644 --- a/Library/Homebrew/rubocops/cask/stanza_order.rb +++ b/Library/Homebrew/rubocops/cask/stanza_order.rb @@ -8,8 +8,9 @@ module RuboCop module Cask # This cop checks that a cask's stanzas are ordered correctly. # @see https://github.com/Homebrew/homebrew-cask/blob/HEAD/doc/cask_language_reference/readme.md#stanza-order - class StanzaOrder < Cop + class StanzaOrder < Base extend Forwardable + extend AutoCorrector include CaskHelp MESSAGE = "`%s` stanza out of order" @@ -19,15 +20,6 @@ module RuboCop add_offenses end - def autocorrect(stanza) - lambda do |corrector| - correct_stanza_index = toplevel_stanzas.index(stanza) - correct_stanza = sorted_toplevel_stanzas[correct_stanza_index] - corrector.replace(stanza.source_range_with_comments, - correct_stanza.source_with_comments) - end - end - private attr_reader :cask_block @@ -38,8 +30,12 @@ module RuboCop def add_offenses offending_stanzas.each do |stanza| message = format(MESSAGE, stanza: stanza.stanza_name) - add_offense(stanza, location: stanza.source_range_with_comments, - message: message) + add_offense(stanza.source_range_with_comments, message: message) do |corrector| + correct_stanza_index = toplevel_stanzas.index(stanza) + correct_stanza = sorted_toplevel_stanzas[correct_stanza_index] + corrector.replace(stanza.source_range_with_comments, + correct_stanza.source_with_comments) + end end end diff --git a/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb b/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb index 8654a49606..49b09a0d83 100644 --- a/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb +++ b/Library/Homebrew/test/rubocops/cask/shared_examples/cask_cop.rb @@ -21,14 +21,13 @@ module CaskCop end def expect_no_offenses(source) - inspect_source(source) - expect(cop.offenses).to be_empty + expect(inspect_source(source)).to be_empty end def expect_reported_offenses(source, expected_offenses) - inspect_source(source) - expect(cop.offenses.size).to eq(expected_offenses.size) - expected_offenses.zip(cop.offenses).each do |expected, actual| + offenses = inspect_source(source) + expect(offenses.size).to eq(expected_offenses.size) + expected_offenses.zip(offenses).each do |expected, actual| expect_offense(expected, actual) end end