diff --git a/Library/Homebrew/rubocops/cask/ast/stanza.rb b/Library/Homebrew/rubocops/cask/ast/stanza.rb index e17e849e74..2088a43655 100644 --- a/Library/Homebrew/rubocops/cask/ast/stanza.rb +++ b/Library/Homebrew/rubocops/cask/ast/stanza.rb @@ -21,11 +21,11 @@ module RuboCop alias stanza_node method_node - def_delegator :stanza_node, :method_name, :stanza_name def_delegator :stanza_node, :parent, :parent_node + def_delegator :stanza_node, :arch_variable? def source_range - stanza_node.expression + stanza_node.location_expression end def source_range_with_comments @@ -38,6 +38,12 @@ module RuboCop def_delegator :source_range_with_comments, :source, :source_with_comments + def stanza_name + return :on_arch_conditional if arch_variable? + + stanza_node.method_name + end + def stanza_group Constants::STANZA_GROUP_HASH[stanza_name] end diff --git a/Library/Homebrew/rubocops/cask/constants/stanza.rb b/Library/Homebrew/rubocops/cask/constants/stanza.rb index 93a9b67d55..5294a1c45f 100644 --- a/Library/Homebrew/rubocops/cask/constants/stanza.rb +++ b/Library/Homebrew/rubocops/cask/constants/stanza.rb @@ -6,6 +6,7 @@ module RuboCop # Constants available globally for use in all cask cops. module Constants STANZA_GROUPS = [ + [:arch, :on_arch_conditional], [:version, :sha256], [:language], [:url, :appcast, :name, :desc, :homepage], diff --git a/Library/Homebrew/rubocops/cask/extend/node.rb b/Library/Homebrew/rubocops/cask/extend/node.rb index 906518db49..c0c4f5f993 100644 --- a/Library/Homebrew/rubocops/cask/extend/node.rb +++ b/Library/Homebrew/rubocops/cask/extend/node.rb @@ -15,8 +15,11 @@ module RuboCop def_node_matcher :val_node, "{(pair _ $_) (hash (pair _ $_) ...)}" def_node_matcher :cask_block?, "(block (send nil? :cask _) args ...)" + def_node_matcher :arch_variable?, "(lvasgn _ (send nil? :on_arch_conditional ...))" def stanza? + return true if arch_variable? + (send_type? || block_type?) && STANZA_ORDER.include?(method_name) end @@ -24,7 +27,7 @@ module RuboCop loc.is_a?(Parser::Source::Map::Heredoc) end - def expression + def location_expression base_expression = loc.expression descendants.select(&:heredoc?).reduce(base_expression) do |expr, node| expr.join(node.loc.heredoc_end) diff --git a/Library/Homebrew/rubocops/cask/variables.rb b/Library/Homebrew/rubocops/cask/variables.rb new file mode 100644 index 0000000000..cf434e1b65 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/variables.rb @@ -0,0 +1,76 @@ +# typed: false +# frozen_string_literal: true + +require "forwardable" + +module RuboCop + module Cop + module Cask + # This cop audits variables in casks. + # + # @example + # # bad + # cask do + # arch = Hardware::CPU.intel? ? "darwin" : "darwin-arm64" + # end + # + # # good + # cask 'foo' do + # arch arm: "darwin-arm64", intel: "darwin" + # end + class Variables < Base + extend Forwardable + extend AutoCorrector + include CaskHelp + + def on_cask(cask_block) + @cask_block = cask_block + add_offenses + end + + private + + def_delegator :@cask_block, :cask_node + + def add_offenses + variable_assignment(cask_node) do |node, var_name, arch_condition, true_node, false_node| + arm_node, intel_node = if arch_condition == :arm? + [true_node, false_node] + else + [false_node, true_node] + end + + replacement_string = if var_name == :arch + "arch " + else + "#{var_name} = on_arch_conditional " + end + replacement_parameters = [] + replacement_parameters << "arm: #{arm_node.source}" unless blank_node?(arm_node) + replacement_parameters << "intel: #{intel_node.source}" unless blank_node?(intel_node) + replacement_string += replacement_parameters.join(", ") + + add_offense(node, message: "Use `#{replacement_string}` instead of `#{node.source}`") do |corrector| + corrector.replace(node, replacement_string) + end + end + end + + def blank_node?(node) + case node.type + when :str + node.value.empty? + when :nil + true + else + false + end + end + + def_node_search :variable_assignment, <<~PATTERN + $(lvasgn $_ (if (send (const (const nil? :Hardware) :CPU) ${:arm? :intel?}) $_ $_)) + PATTERN + end + end + end +end diff --git a/Library/Homebrew/rubocops/rubocop-cask.rb b/Library/Homebrew/rubocops/rubocop-cask.rb index 63b02b0e19..673b6a5231 100644 --- a/Library/Homebrew/rubocops/rubocop-cask.rb +++ b/Library/Homebrew/rubocops/rubocop-cask.rb @@ -19,3 +19,4 @@ require_relative "cask/no_dsl_version" require_relative "cask/stanza_order" require_relative "cask/stanza_grouping" require_relative "cask/url_legacy_comma_separators" +require_relative "cask/variables" diff --git a/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb b/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb index 3cce050983..4d806bdef7 100644 --- a/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb @@ -41,6 +41,22 @@ describe RuboCop::Cop::Cask::StanzaGrouping do include_examples "does not report any offenses" end + context "when no stanzas or variable assignments are incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + + version :latest + sha256 :no_check + end + CASK + end + + include_examples "does not report any offenses" + end + context "when one stanza is incorrectly grouped" do let(:source) do <<-CASK.undent @@ -74,6 +90,78 @@ describe RuboCop::Cop::Cask::StanzaGrouping do include_examples "autocorrects source" end + context "when the arch stanza is incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + version :latest + sha256 :no_check + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: missing_line_msg, + severity: :convention, + line: 3, + column: 0, + source: " version :latest", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when one variable assignment is incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + version :latest + sha256 :no_check + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: missing_line_msg, + severity: :convention, + line: 4, + column: 0, + source: " version :latest", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + context "when many stanzas are incorrectly grouped" do let(:source) do <<-CASK.undent @@ -142,6 +230,94 @@ describe RuboCop::Cop::Cask::StanzaGrouping do include_examples "autocorrects source" end + context "when many stanzas and variable assignments are incorrectly grouped" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + + platform = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + version :latest + sha256 :no_check + url 'https://foo.brew.sh/foo.zip' + + name 'Foo' + + homepage 'https://foo.brew.sh' + + app 'Foo.app' + uninstall :quit => 'com.example.foo', + :kext => 'com.example.foo.kextextension' + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + platform = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + + version :latest + sha256 :no_check + + url 'https://foo.brew.sh/foo.zip' + name 'Foo' + homepage 'https://foo.brew.sh' + + app 'Foo.app' + + uninstall :quit => 'com.example.foo', + :kext => 'com.example.foo.kextextension' + end + CASK + end + let(:expected_offenses) do + [{ + message: extra_line_msg, + severity: :convention, + line: 4, + column: 0, + source: "\n", + }, { + message: missing_line_msg, + severity: :convention, + line: 6, + column: 0, + source: " version :latest", + }, { + message: missing_line_msg, + severity: :convention, + line: 8, + column: 0, + source: " url 'https://foo.brew.sh/foo.zip'", + }, { + message: extra_line_msg, + severity: :convention, + line: 9, + column: 0, + source: "\n", + }, { + message: extra_line_msg, + severity: :convention, + line: 11, + column: 0, + source: "\n", + }, { + message: missing_line_msg, + severity: :convention, + line: 15, + column: 0, + source: " uninstall :quit => 'com.example.foo',", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + context "when caveats stanza is incorrectly grouped" do let(:source) do format(<<-CASK.undent, caveats: caveats.strip) @@ -284,6 +460,52 @@ describe RuboCop::Cop::Cask::StanzaGrouping do include_examples "autocorrects source" end + context "when a stanza has a comment and there is a variable assignment" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + # comment with an empty line between + version :latest + sha256 :no_check + + # comment directly above + postflight do + puts 'We have liftoff!' + end + url 'https://foo.brew.sh/foo.zip' + name 'Foo' + app 'Foo.app' + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + + # comment with an empty line between + version :latest + sha256 :no_check + + # comment directly above + postflight do + puts 'We have liftoff!' + end + + url 'https://foo.brew.sh/foo.zip' + name 'Foo' + + app 'Foo.app' + end + CASK + end + + include_examples "autocorrects source" + end + # TODO: detect incorrectly grouped stanzas in nested expressions context "when stanzas are nested in a conditional expression" do let(:source) do diff --git a/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb b/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb index c5cc7594a2..8e512193a6 100644 --- a/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/stanza_order_spec.rb @@ -25,8 +25,11 @@ describe RuboCop::Cop::Cask::StanzaOrder do let(:source) do <<-CASK.undent cask 'foo' do + arch arm: "arm", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" version :latest sha256 :no_check + foo = "bar" end CASK end @@ -72,6 +75,136 @@ describe RuboCop::Cop::Cask::StanzaOrder do include_examples "autocorrects source" end + context "when the arch stanza is out of order" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + arch arm: "arm", intel: "x86_64" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm", intel: "x86_64" + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: "`version` stanza out of order", + severity: :convention, + line: 2, + column: 2, + source: "version :latest", + }, { + message: "`sha256` stanza out of order", + severity: :convention, + line: 3, + column: 2, + source: "sha256 :no_check", + }, { + message: "`arch` stanza out of order", + severity: :convention, + line: 4, + column: 2, + source: 'arch arm: "arm", intel: "x86_64"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when an arch variable assignment is out of order" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm", intel: "x86_64" + sha256 :no_check + version :latest + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: "`sha256` stanza out of order", + severity: :convention, + line: 3, + column: 2, + source: "sha256 :no_check", + }, { + message: "`on_arch_conditional` stanza out of order", + severity: :convention, + line: 5, + column: 2, + source: 'folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when an arch variable assignment is above the arch stanza" do + let(:source) do + <<-CASK.undent + cask 'foo' do + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + arch arm: "arm", intel: "x86_64" + version :latest + sha256 :no_check + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm", intel: "x86_64" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + version :latest + sha256 :no_check + end + CASK + end + let(:expected_offenses) do + [{ + message: "`on_arch_conditional` stanza out of order", + severity: :convention, + line: 2, + column: 2, + source: 'folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin"', + }, { + message: "`arch` stanza out of order", + severity: :convention, + line: 3, + column: 2, + source: 'arch arm: "arm", intel: "x86_64"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + context "when many stanzas are out of order" do let(:source) do <<-CASK.undent @@ -197,6 +330,41 @@ describe RuboCop::Cop::Cask::StanzaOrder do include_examples "autocorrects source" end + context "when a variable assignment is out of order with a comment" do + let(:source) do + <<-CASK.undent + cask 'foo' do + version :latest + sha256 :no_check + # comment with an empty line between + + # comment directly above + postflight do + puts 'We have liftoff!' + end + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" # comment on same line + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" # comment on same line + version :latest + sha256 :no_check + # comment with an empty line between + + # comment directly above + postflight do + puts 'We have liftoff!' + end + end + CASK + end + + include_examples "autocorrects source" + end + context "when the caveats stanza is out of order" do let(:source) do format(<<-CASK.undent, caveats: caveats.strip) diff --git a/Library/Homebrew/test/rubocops/cask/variables_spec.rb b/Library/Homebrew/test/rubocops/cask/variables_spec.rb new file mode 100644 index 0000000000..96f44ef162 --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/variables_spec.rb @@ -0,0 +1,282 @@ +# typed: false +# frozen_string_literal: true + +require "rubocops/rubocop-cask" +require "test/rubocops/cask/shared_examples/cask_cop" + +describe RuboCop::Cop::Cask::Variables do + include CaskCop + + subject(:cop) { described_class.new } + + context "when there are no variables" do + let(:source) do + <<-CASK.undent + cask "foo" do + version :latest + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there is an arch stanza" do + let(:source) do + <<-CASK.undent + cask "foo" do + arch arm: "darwin-arm64", intel: "darwin" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there is a non-arch variable that uses the arch conditional" do + let(:source) do + <<-CASK.undent + cask "foo" do + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + end + CASK + end + + include_examples "does not report any offenses" + end + + context "when there is an arch variable" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch = Hardware::CPU.intel? ? "darwin" : "darwin-arm64" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "darwin-arm64", intel: "darwin" + end + CASK + end + let(:expected_offenses) do + [{ + message: 'Use `arch arm: "darwin-arm64", intel: "darwin"` instead of ' \ + '`arch = Hardware::CPU.intel? ? "darwin" : "darwin-arm64"`', + severity: :convention, + line: 2, + column: 2, + source: 'arch = Hardware::CPU.intel? ? "darwin" : "darwin-arm64"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when there is an arch variable that doesn't use strings" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch = Hardware::CPU.intel? ? :darwin : :darwin_arm64 + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: :darwin_arm64, intel: :darwin + end + CASK + end + let(:expected_offenses) do + [{ + message: "Use `arch arm: :darwin_arm64, intel: :darwin` instead of " \ + "`arch = Hardware::CPU.intel? ? :darwin : :darwin_arm64`", + severity: :convention, + line: 2, + column: 2, + source: "arch = Hardware::CPU.intel? ? :darwin : :darwin_arm64", + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when there is an arch with an empty string" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch = Hardware::CPU.intel? ? "" : "arm64" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "arm64" + end + CASK + end + let(:expected_offenses) do + [{ + message: 'Use `arch arm: "arm64"` instead of ' \ + '`arch = Hardware::CPU.intel? ? "" : "arm64"`', + severity: :convention, + line: 2, + column: 2, + source: 'arch = Hardware::CPU.intel? ? "" : "arm64"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when there is a non-arch variable" do + let(:source) do + <<-CASK.undent + cask 'foo' do + folder = Hardware::CPU.intel? ? "darwin" : "darwin-arm64" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + end + CASK + end + let(:expected_offenses) do + [{ + message: 'Use `folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin"` instead of ' \ + '`folder = Hardware::CPU.intel? ? "darwin" : "darwin-arm64"`', + severity: :convention, + line: 2, + column: 2, + source: 'folder = Hardware::CPU.intel? ? "darwin" : "darwin-arm64"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when there is a non-arch variable with an empty string" do + let(:source) do + <<-CASK.undent + cask 'foo' do + folder = Hardware::CPU.intel? ? "amd64" : "" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + folder = on_arch_conditional intel: "amd64" + end + CASK + end + let(:expected_offenses) do + [{ + message: 'Use `folder = on_arch_conditional intel: "amd64"` instead of ' \ + '`folder = Hardware::CPU.intel? ? "amd64" : ""`', + severity: :convention, + line: 2, + column: 2, + source: 'folder = Hardware::CPU.intel? ? "amd64" : ""', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when there is an arch and a non-arch variable" do + let(:source) do + <<-CASK.undent + cask 'foo' do + arch = Hardware::CPU.arm? ? "darwin-arm64" : "darwin" + folder = Hardware::CPU.arm? ? "darwin-arm64" : "darwin" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + arch arm: "darwin-arm64", intel: "darwin" + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + end + CASK + end + let(:expected_offenses) do + [{ + message: 'Use `arch arm: "darwin-arm64", intel: "darwin"` instead of ' \ + '`arch = Hardware::CPU.arm? ? "darwin-arm64" : "darwin"`', + severity: :convention, + line: 2, + column: 2, + source: 'arch = Hardware::CPU.arm? ? "darwin-arm64" : "darwin"', + }, { + message: 'Use `folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin"` instead of ' \ + '`folder = Hardware::CPU.arm? ? "darwin-arm64" : "darwin"`', + severity: :convention, + line: 3, + column: 2, + source: 'folder = Hardware::CPU.arm? ? "darwin-arm64" : "darwin"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end + + context "when there are two non-arch variables" do + let(:source) do + <<-CASK.undent + cask 'foo' do + folder = Hardware::CPU.arm? ? "darwin-arm64" : "darwin" + platform = Hardware::CPU.intel? ? "darwin": "darwin-arm64" + end + CASK + end + let(:correct_source) do + <<-CASK.undent + cask 'foo' do + folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + platform = on_arch_conditional arm: "darwin-arm64", intel: "darwin" + end + CASK + end + let(:expected_offenses) do + [{ + message: 'Use `folder = on_arch_conditional arm: "darwin-arm64", intel: "darwin"` instead of ' \ + '`folder = Hardware::CPU.arm? ? "darwin-arm64" : "darwin"`', + severity: :convention, + line: 2, + column: 2, + source: 'folder = Hardware::CPU.arm? ? "darwin-arm64" : "darwin"', + }, { + message: 'Use `platform = on_arch_conditional arm: "darwin-arm64", intel: "darwin"` instead of ' \ + '`platform = Hardware::CPU.intel? ? "darwin": "darwin-arm64"`', + severity: :convention, + line: 3, + column: 2, + source: 'platform = Hardware::CPU.intel? ? "darwin": "darwin-arm64"', + }] + end + + include_examples "reports offenses" + + include_examples "autocorrects source" + end +end