From 27ce5754c969cbd3d3684680b07ed956ce11eebc Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Wed, 23 Dec 2020 14:03:37 -0500 Subject: [PATCH 1/3] style: fix on_macos/on_linux resource block checks --- Library/Homebrew/rubocops/components_order.rb | 19 +++-- .../test/rubocops/components_order_spec.rb | 78 ++++++++++++++++++- 2 files changed, 89 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index 568d447bbb..8c19eee17c 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -106,9 +106,12 @@ module RuboCop if on_macos_blocks.length == 1 on_macos_block = on_macos_blocks.first child_nodes = on_macos_block.body.child_nodes - if child_nodes[0].method_name.to_s != "url" && child_nodes[1].method_name.to_s != "sha256" - problem "only an url and a sha256 (in the right order) are allowed in a `on_macos` " \ - "block within a resource block." + unless child_nodes[0].method?("url") && ( + child_nodes[1].method?("sha256") || + (child_nodes[1].method?("version") && child_nodes[2].method?("sha256")) + ) + problem "`on_macos` blocks within resource blocks must contain only a " \ + "url and sha256 or a url, version, and sha256 (in those orders)." next end end @@ -116,9 +119,13 @@ module RuboCop if on_linux_blocks.length == 1 on_linux_block = on_linux_blocks.first child_nodes = on_linux_block.body.child_nodes - if child_nodes[0].method_name.to_s != "url" && child_nodes[1].method_name.to_s != "sha256" - problem "only an url and a sha256 (in the right order) are allowed in a `on_linux` " \ - "block within a resource block." + unless child_nodes[0].method?("url") && ( + child_nodes[1].method?("sha256") || + (child_nodes[1].method?("version") && child_nodes[2].method?("sha256")) + ) + problem "`on_linux` blocks within resource blocks must contain only a " \ + "url and sha256 or a url, version, and sha256 (in those orders)." + next end end diff --git a/Library/Homebrew/test/rubocops/components_order_spec.rb b/Library/Homebrew/test/rubocops/components_order_spec.rb index 0798d09c4d..4daf77fcfd 100644 --- a/Library/Homebrew/test/rubocops/components_order_spec.rb +++ b/Library/Homebrew/test/rubocops/components_order_spec.rb @@ -432,6 +432,28 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end + it "correctly uses an 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 "there are two on_macos blocks, which is not allowed" do expect_offense(<<~RUBY) class Foo < Formula @@ -508,7 +530,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do url "https://brew.sh/foo-1.0.tgz" resource do - ^^^^^^^^^^^ only an url and a sha256 (in the right order) are allowed in a `on_macos` block within a resource block. + ^^^^^^^^^^^ `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" @@ -523,13 +545,39 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end + it "the content of the on_macos block is wrong and not a method" 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 + 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 "the content of the on_linux block is wrong in a resource block" do expect_offense(<<~RUBY) class Foo < Formula url "https://brew.sh/foo-1.0.tgz" resource do - ^^^^^^^^^^^ only an url and a sha256 (in the right order) are allowed in a `on_linux` block within a resource block. + ^^^^^^^^^^^ `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" @@ -543,5 +591,31 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do end RUBY end + + it "the content of the on_linux block is wrong and not a method" 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 + url "https://brew.sh/resource1.tar.gz" + sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + end + end + end + end + RUBY + end end end From 4924b7633c69594d1d724c1a0976b564841b06ef Mon Sep 17 00:00:00 2001 From: Seeker Date: Wed, 23 Dec 2020 15:00:28 -0800 Subject: [PATCH 2/3] components_order: allow if-else statements inside on_os blocks Co-Authored-By: Rylan Polster --- Library/Homebrew/rubocops/components_order.rb | 47 ++++++++------- .../test/rubocops/components_order_spec.rb | 58 +++++++++++++++++-- 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index 8c19eee17c..f197379730 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -103,30 +103,37 @@ module RuboCop next if on_macos_blocks.length.zero? && on_linux_blocks.length.zero? - if on_macos_blocks.length == 1 - on_macos_block = on_macos_blocks.first - child_nodes = on_macos_block.body.child_nodes - unless child_nodes[0].method?("url") && ( - child_nodes[1].method?("sha256") || - (child_nodes[1].method?("version") && child_nodes[2].method?("sha256")) - ) - problem "`on_macos` blocks within resource blocks must contain only a " \ - "url and sha256 or a url, version, and sha256 (in those orders)." - next + on_os_bodies = [] + + (on_macos_blocks + on_linux_blocks).each do |on_os_block| + on_os_body = on_os_block.body + if on_os_body.if_type? + on_os_bodies += on_os_body.branches.map { |branch| [on_os_block.method_name, branch] } + else + on_os_bodies << [on_os_block.method_name, on_os_body] end end - if on_linux_blocks.length == 1 - on_linux_block = on_linux_blocks.first - child_nodes = on_linux_block.body.child_nodes - unless child_nodes[0].method?("url") && ( - child_nodes[1].method?("sha256") || - (child_nodes[1].method?("version") && child_nodes[2].method?("sha256")) - ) - problem "`on_linux` blocks within resource blocks must contain only a " \ - "url and sha256 or a url, version, and sha256 (in those orders)." - next + message = nil + allowed_methods = [ + [:url, :sha256], + [:url, :version, :sha256], + ] + + on_os_bodies.each do |method_name, on_os_body| + child_nodes = on_os_body.begin_type? ? on_os_body.child_nodes : [on_os_body] + if child_nodes.all? { |n| n.send_type? || n.block_type? } + method_names = child_nodes.map(&:method_name) + next if allowed_methods.include? method_names end + message = "`#{method_name}` blocks within resource blocks must contain only a " \ + "url and sha256 or a url, version, and sha256 (in those orders)." + break + end + + if message.present? + problem message + next end if on_macos_blocks.length > 1 diff --git a/Library/Homebrew/test/rubocops/components_order_spec.rb b/Library/Homebrew/test/rubocops/components_order_spec.rb index 4daf77fcfd..dc6ae63a33 100644 --- a/Library/Homebrew/test/rubocops/components_order_spec.rb +++ b/Library/Homebrew/test/rubocops/components_order_spec.rb @@ -545,6 +545,31 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end + it "reports no offenses if the on_macos block has if-else branches and they 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 "the content of the on_macos block is wrong and not a method" do expect_offense(<<~RUBY) class Foo < Formula @@ -557,8 +582,8 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do url "https://brew.sh/resource2.tar.gz" sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" else - url "https://brew.sh/resource1.tar.gz" sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35" + url "https://brew.sh/resource1.tar.gz" end end @@ -592,13 +617,12 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "the content of the on_linux block is wrong and not a method" do - expect_offense(<<~RUBY) + it "reports no offenses if the on_linux block has if-else branches and they are properly formatted" do + expect_no_offenses(<<~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" @@ -617,5 +641,31 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do end RUBY end + + it "the content of the on_linux block is wrong and not a method" 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 From b5cc723d3338399adac252630bad3739ea1f7da5 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Wed, 23 Dec 2020 21:05:50 -0500 Subject: [PATCH 3/3] tests/rubocops/components_order: fix test descriptions --- .../test/rubocops/components_order_spec.rb | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/test/rubocops/components_order_spec.rb b/Library/Homebrew/test/rubocops/components_order_spec.rb index dc6ae63a33..862b639def 100644 --- a/Library/Homebrew/test/rubocops/components_order_spec.rb +++ b/Library/Homebrew/test/rubocops/components_order_spec.rb @@ -411,8 +411,8 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - context "resource" do - it "correctly uses an on_macos and on_linux block" do + 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" @@ -432,7 +432,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "correctly uses an on_macos and on_linux block with versions" do + 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" @@ -454,7 +454,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "there are two on_macos blocks, which is not allowed" do + 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" @@ -475,7 +475,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "there are two on_linux blocks, which is not allowed" do + 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" @@ -496,7 +496,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "there is a on_macos block but no on_linux block" do + 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" @@ -510,7 +510,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "there is a on_linux block but no on_macos block" do + 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" @@ -524,7 +524,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "the content of the on_macos block is wrong in a resource block" do + 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" @@ -545,7 +545,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "reports no offenses if the on_macos block has if-else branches and they are properly formatted" do + 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" @@ -570,7 +570,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "the content of the on_macos block is wrong and not a method" do + 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" @@ -596,7 +596,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "the content of the on_linux block is wrong in a resource block" do + 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" @@ -617,7 +617,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "reports no offenses if the on_linux block has if-else branches and they are properly formatted" do + 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" @@ -642,7 +642,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do RUBY end - it "the content of the on_linux block is wrong and not a method" do + 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"