Skip to content

Commit e5adc3b

Browse files
committed
Make sure we're iterating through all ERB nodes
1 parent 29dc516 commit e5adc3b

File tree

2 files changed

+45
-27
lines changed

2 files changed

+45
-27
lines changed

lib/erblint-github/linters/github/accessibility/avoid_generic_link_text_counter.rb

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -55,41 +55,41 @@ def run(processed_source)
5555
end
5656

5757
# Checks Rails link helpers like `link_to`
58-
erb_node = node.type == :erb ? node : node.descendants(:erb).first
59-
next unless erb_node
58+
node.descendants(:erb).each do |erb_node|
59+
_, _, code_node = *erb_node
60+
source = code_node.loc.source
61+
ruby_node = extract_ruby_node(source)
62+
send_node = ruby_node&.descendants(:send)&.first
63+
next unless send_node.methods.include?(:method_name) && send_node.method_name == :link_to
6064

61-
_, _, code_node = *erb_node
62-
source = code_node.loc.source
63-
ruby_node = extract_ruby_node(source)
64-
send_node = ruby_node&.descendants(:send)&.first
65-
next unless send_node.methods.include?(:method_name) && send_node.method_name == :link_to
65+
banned_text = nil
6666

67-
banned_text = nil
67+
send_node.child_nodes.each do |child_node|
68+
banned_text = child_node.children.join if child_node.methods.include?(:type) && child_node.type == :str && banned_text?(child_node.children.join)
69+
next if banned_text.blank?
6870

69-
send_node.child_nodes.each do |child_node|
70-
banned_text = child_node.children.join if child_node.methods.include?(:type) && child_node.type == :str && banned_text?(child_node.children.join)
71-
next if banned_text.blank?
71+
next unless child_node.methods.include?(:type) && child_node.type == :hash
7272

73-
next unless child_node.methods.include?(:type) && child_node.type == :hash
73+
child_node.descendants(:pair).each do |pair_node|
74+
next unless pair_node.children.first.type?(:sym)
7475

75-
child_node.descendants(:pair).each do |pair_node|
76-
next unless pair_node.children.first.type?(:sym)
77-
78-
if pair_node.children.first.children.join == "aria"
79-
pair_node.children[1].descendants(:sym).each do |sym_node|
80-
banned_text = nil if sym_node.children.join == "label" || sym_node.children.join == "labelledby"
76+
if pair_node.children.first.children.join == "aria"
77+
pair_node.children[1].descendants(:sym).each do |sym_node|
78+
banned_text = nil if sym_node.children.join == "label" || sym_node.children.join == "labelledby"
79+
end
8180
end
82-
end
8381

84-
# Skips if `link_to` has `aria-labelledby` or `aria-label` which we cannot be evaluated accurately with ERB lint alone.
85-
# ERB lint removes Ruby string interpolation so the `aria-label` for "<%= link_to 'Learn more', "aria-label": "Learn #{@some_variable}" %>" will
86-
# only be `Learn` which is unreliable so we can't do checks :(
87-
banned_text = nil if pair_node.children.first.children.join == "aria-labelledby" || pair_node.children.first.children.join == "aria-label"
82+
# Skips if `link_to` has `aria-labelledby` or `aria-label` which we cannot be evaluated accurately with ERB lint alone.
83+
# ERB lint removes Ruby string interpolation so the `aria-label` for "<%= link_to 'Learn more', "aria-label": "Learn #{@some_variable}" %>" will
84+
# only be `Learn` which is unreliable so we can't do checks :(
85+
banned_text = nil if pair_node.children.first.children.join == "aria-labelledby" || pair_node.children.first.children.join == "aria-label"
86+
end
8887
end
89-
end
90-
if banned_text.present?
91-
tag = BetterHtml::Tree::Tag.from_node(code_node)
92-
generate_offense(self.class, processed_source, tag)
88+
if banned_text.present?
89+
tag = BetterHtml::Tree::Tag.from_node(code_node)
90+
generate_offense(self.class, processed_source, tag)
91+
end
92+
banned_text = nil
9393
end
9494
end
9595
counter_correct?(processed_source)

test/linters/accessibility/avoid_generic_link_text_counter_test.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,24 @@ def test_does_not_warn_when_generic_text_is_link_rails_helper_sub_text
145145
assert_empty @linter.offenses
146146
end
147147

148+
def test_handles_files_with_various_links
149+
@file = <<~ERB
150+
<p>
151+
<a aria-labelledby='someElement'>Click here</a>
152+
</p>
153+
<p>
154+
<%= link_to "learn more", billing_path, "aria-label": "something" %>
155+
<%= link_to "learn more", billing_path, aria: { label: "something" } %>
156+
<%= link_to "learn more", billing_path, aria: { describedby: "element123" } %>
157+
<%= link_to "learn more", billing_path, "aria-describedby": "element123" %>
158+
</p>
159+
ERB
160+
@linter.run(processed_source)
161+
162+
refute_empty @linter.offenses
163+
assert_equal 3, @linter.offenses.count
164+
end
165+
148166
def test_does_not_warns_if_element_has_correct_counter_comment
149167
@file = <<~ERB
150168
<%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>

0 commit comments

Comments
 (0)