From 6de61e4994fd889fd95c697c75aa969fa576f70a Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 12 Apr 2023 23:14:43 +0100 Subject: [PATCH] Ensure that stanza grouping works for nested stanzas with comments - Since moving `comments_hash` to `Stanza`, we've been using the wrong kind of "comments": the comments for the _stanza_, not the comments for the entire Cask. - Add a test to ensure this actually works. There was previously an infinite loop here due to the bad `comments`, visible in a `StanzaOrder` cop test, which I speculatively added a failing test for. Turns out that supporting nested stanza _ordering_ (vs. just grouping) is a whole separate piece of work (there are multiple TODOs there already), so I've backed that out and will do that separately. --- .../Homebrew/rubocops/cask/ast/cask_block.rb | 2 +- Library/Homebrew/rubocops/cask/ast/stanza.rb | 10 +++--- .../Homebrew/rubocops/cask/stanza_grouping.rb | 2 +- .../rubocops/cask/stanza_grouping_spec.rb | 35 +++++++++++++++++++ 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/rubocops/cask/ast/cask_block.rb b/Library/Homebrew/rubocops/cask/ast/cask_block.rb index 8b8d1eca8e..1b4090ab49 100644 --- a/Library/Homebrew/rubocops/cask/ast/cask_block.rb +++ b/Library/Homebrew/rubocops/cask/ast/cask_block.rb @@ -32,7 +32,7 @@ module RuboCop @stanzas ||= cask_body.each_node .select(&:stanza?) - .map { |node| Stanza.new(node) } + .map { |node| Stanza.new(node, comments) } end def toplevel_stanzas diff --git a/Library/Homebrew/rubocops/cask/ast/stanza.rb b/Library/Homebrew/rubocops/cask/ast/stanza.rb index f2aca3d242..68b6de8218 100644 --- a/Library/Homebrew/rubocops/cask/ast/stanza.rb +++ b/Library/Homebrew/rubocops/cask/ast/stanza.rb @@ -12,11 +12,12 @@ module RuboCop class Stanza extend Forwardable - def initialize(method_node) + def initialize(method_node, all_comments) @method_node = method_node + @all_comments = all_comments end - attr_reader :method_node + attr_reader :method_node, :all_comments alias stanza_node method_node @@ -58,10 +59,7 @@ module RuboCop end def comments_hash - @comments_hash ||= Parser::Source::Comment.associate_locations( - stanza_node.parent, - comments, - ) + @comments_hash ||= Parser::Source::Comment.associate_locations(stanza_node.parent, all_comments) end def ==(other) diff --git a/Library/Homebrew/rubocops/cask/stanza_grouping.rb b/Library/Homebrew/rubocops/cask/stanza_grouping.rb index 6338e0fa6b..146a2a636f 100644 --- a/Library/Homebrew/rubocops/cask/stanza_grouping.rb +++ b/Library/Homebrew/rubocops/cask/stanza_grouping.rb @@ -32,7 +32,7 @@ module RuboCop block_contents = on_block.child_nodes.select(&:begin_type?) inner_nodes = block_contents.map(&:child_nodes).flatten.select(&:send_type?) - inner_stanzas = inner_nodes.map { |node| RuboCop::Cask::AST::Stanza.new(node) } + inner_stanzas = inner_nodes.map { |node| RuboCop::Cask::AST::Stanza.new(node, processed_source.comments) } add_offenses(inner_stanzas) end diff --git a/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb b/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb index 395e2bd502..46e8bc6973 100644 --- a/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb +++ b/Library/Homebrew/test/rubocops/cask/stanza_grouping_spec.rb @@ -545,6 +545,41 @@ describe RuboCop::Cop::Cask::StanzaGrouping do include_examples "autocorrects source" end + describe "nested `on_*` blocks with comments" do + let(:source) do + <<~CASK + cask 'foo' do + on_arm do + version "1.0.2" + + sha256 :no_check # comment on same line + end + on_intel do + version "0.9.8" + sha256 :no_check + end + end + CASK + end + + let(:correct_source) do + <<~CASK + cask 'foo' do + on_arm do + version "1.0.2" + sha256 :no_check # comment on same line + end + on_intel do + version "0.9.8" + sha256 :no_check + end + end + CASK + end + + include_examples "autocorrects source" + end + # TODO: Maybe this should be fixed too? describe "inner erroneously grouped nested livecheck block contents are ignored" do let(:source) do