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