From d5f949e60b6d78aa50f0409a2dfb50a63eaa554e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?fn=20=E2=8C=83=20=E2=8C=A5?= <70830482+FnControlOption@users.noreply.github.com> Date: Tue, 2 Aug 2022 21:56:45 -0700 Subject: [PATCH 1/2] Check dependency order in on_system methods --- Library/Homebrew/rubocops/components_order.rb | 6 ------ Library/Homebrew/rubocops/dependency_order.rb | 2 +- Library/Homebrew/rubocops/extend/formula.rb | 6 ++++++ 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index 002ce1f9cf..5d28a172a1 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -14,12 +14,6 @@ module RuboCop class ComponentsOrder < FormulaCop extend AutoCorrector - def on_system_methods - @on_system_methods ||= [:intel, :arm, :macos, :linux, :system, *MacOSVersions::SYMBOLS.keys].map do |m| - :"on_#{m}" - end - end - def audit_formula(_node, _class_node, _parent_class_node, body_node) @present_components, @offensive_nodes = check_order(FORMULA_COMPONENT_PRECEDENCE_LIST, body_node) diff --git a/Library/Homebrew/rubocops/dependency_order.rb b/Library/Homebrew/rubocops/dependency_order.rb index fc12fe4119..34775de930 100644 --- a/Library/Homebrew/rubocops/dependency_order.rb +++ b/Library/Homebrew/rubocops/dependency_order.rb @@ -16,7 +16,7 @@ module RuboCop 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) - [:head, :stable].each do |block_name| + ([:head, :stable] + on_system_methods).each do |block_name| block = find_block(body_node, block_name) next unless block diff --git a/Library/Homebrew/rubocops/extend/formula.rb b/Library/Homebrew/rubocops/extend/formula.rb index e6cc658350..d4b09bab73 100644 --- a/Library/Homebrew/rubocops/extend/formula.rb +++ b/Library/Homebrew/rubocops/extend/formula.rb @@ -198,6 +198,12 @@ module RuboCop @file_path !~ Regexp.union(paths_to_exclude) end + + def on_system_methods + @on_system_methods ||= [:intel, :arm, :macos, :linux, :system, *MacOSVersions::SYMBOLS.keys].map do |m| + :"on_#{m}" + end + end end end end From b5a6f90cd82e746f67fa5d90d5146084fcc20982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?fn=20=E2=8C=83=20=E2=8C=A5?= <70830482+FnControlOption@users.noreply.github.com> Date: Tue, 2 Aug 2022 23:04:28 -0700 Subject: [PATCH 2/2] Add tests --- .../test/rubocops/dependency_order_spec.rb | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/Library/Homebrew/test/rubocops/dependency_order_spec.rb b/Library/Homebrew/test/rubocops/dependency_order_spec.rb index b080d55ef7..465477dac0 100644 --- a/Library/Homebrew/test/rubocops/dependency_order_spec.rb +++ b/Library/Homebrew/test/rubocops/dependency_order_spec.rb @@ -114,6 +114,34 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do end RUBY end + + it "reports and corrects wrong conditional order within a system block" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + on_arm do + uses_from_macos "apple" if build.with? "foo" + uses_from_macos "bar" + ^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 6) should be put before dependency "apple" (line 5) + uses_from_macos "foo" => :optional + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 7) should be put before dependency "apple" (line 5) + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + on_arm do + uses_from_macos "bar" + uses_from_macos "foo" => :optional + uses_from_macos "apple" if build.with? "foo" + end + end + RUBY + end end context "when auditing `depends_on`" do @@ -224,5 +252,33 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do end RUBY end + + it "reports and corrects wrong conditional order within a system block" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + on_linux do + depends_on "apple" if build.with? "foo" + depends_on "bar" + ^^^^^^^^^^^^^^^^ dependency "bar" (line 6) should be put before dependency "apple" (line 5) + depends_on "foo" => :optional + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 7) should be put before dependency "apple" (line 5) + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + on_linux do + depends_on "bar" + depends_on "foo" => :optional + depends_on "apple" if build.with? "foo" + end + end + RUBY + end end end