Skip to content

Commit 1b233fe

Browse files
committed
Add logic to consider ARIA label attributes
1 parent acb1d6f commit 1b233fe

File tree

2 files changed

+169
-60
lines changed

2 files changed

+169
-60
lines changed

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

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,15 @@ def run(processed_source)
3636
prev_node_tag = BetterHtml::Tree::Tag.from_node(prev_node)
3737
next_node_tag = BetterHtml::Tree::Tag.from_node(next_node)
3838

39+
aria_label = possible_attribute_values(prev_node_tag, "aria-label")
40+
aria_labelledby = possible_attribute_values(prev_node_tag, "aria-labelledby")
41+
3942
# We only report if the text is nested between two link tags.
4043
if link_tag?(prev_node_tag) && link_tag?(next_node_tag) && next_node_tag.closing?
44+
# Cannot statically check label from aria-labelledby or label from variable aria-label so we skip
45+
next if aria_labelledby.present? || (aria_label.present? && aria_label.join.include?("<%="))
46+
next if aria_label.present? && valid_accessible_name?(aria_label.join, text)
47+
4148
range = prev_node_tag.loc.begin_pos...text_node_tag.loc.end_pos
4249
source_range = processed_source.to_source_range(range)
4350
generate_offense_from_source_range(self.class, source_range)
@@ -54,11 +61,34 @@ def run(processed_source)
5461
send_node = ruby_node&.descendants(:send)&.first
5562
next unless send_node.methods.include?(:method_name) && send_node.method_name == :link_to
5663

64+
banned_text = nil
65+
5766
send_node.child_nodes.each do |child_node|
58-
if child_node.methods.include?(:type) && child_node.type == :str && banned_text?(child_node.children.join)
59-
tag = BetterHtml::Tree::Tag.from_node(code_node)
60-
generate_offense(self.class, processed_source, tag)
67+
banned_text = child_node.children.join if child_node.methods.include?(:type) && child_node.type == :str && banned_text?(child_node.children.join)
68+
69+
next unless banned_text.present? && child_node.methods.include?(:type) && child_node.type == :hash
70+
71+
child_node.descendants(:pair).each do |pair_node|
72+
next unless pair_node.children.first.type?(:sym)
73+
74+
# Don't flag if link has aria-labelledby which we cannot evaluate with ERB alone.
75+
banned_text = nil if pair_node.children.first.children.join == "aria-labelledby" || pair_node.children.first.children.join == "aria-label"
76+
next unless pair_node.children.first.children.join == "aria-label"
77+
78+
aria_label_value_node = pair_node.children[1]
79+
if aria_label_value_node.type == :str
80+
binding.irb
81+
aria_label_text = aria_label_value_node.children.join
82+
banned_text = nil if valid_accessible_name?(aria_label_text, banned_text)
83+
else
84+
# Don't flag if aria-label is not string (e.g. is a variable) since we cannot evaluate with ERB alone.
85+
banned_text = nil
6186
end
87+
end
88+
end
89+
if banned_text.present?
90+
tag = BetterHtml::Tree::Tag.from_node(code_node)
91+
generate_offense(self.class, processed_source, tag)
6292
end
6393
end
6494
counter_correct?(processed_source)
@@ -84,6 +114,12 @@ def banned_text?(text)
84114
BANNED_GENERIC_TEXT.map(&:downcase).include?(text.downcase)
85115
end
86116

117+
# We flag if accessible name does not start with visible text.
118+
# Related: Success Criteria 2.5.3
119+
def valid_accessible_name?(accessible_name, visible_text)
120+
accessible_name.downcase.start_with?(visible_text.downcase)
121+
end
122+
87123
def extract_ruby_node(source)
88124
BetterHtml::TestHelper::RubyNode.parse(source)
89125
rescue ::Parser::SyntaxError

test/linters/accessibility/avoid_generic_link_text_counter_test.rb

Lines changed: 130 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -7,83 +7,156 @@ def linter_class
77
ERBLint::Linters::GitHub::Accessibility::AvoidGenericLinkTextCounter
88
end
99

10-
def test_warns_when_link_text_is_click_here
11-
@file = "<a>Click here</a>"
12-
@linter.run(processed_source)
10+
# def test_warns_when_link_text_is_click_here
11+
# @file = "<a>Click here</a>"
12+
# @linter.run(processed_source)
1313

14-
refute_empty @linter.offenses
15-
end
14+
# refute_empty @linter.offenses
15+
# end
1616

17-
def test_warns_when_link_text_is_learn_more
18-
@file = "<a>Learn more</a>"
19-
@linter.run(processed_source)
17+
# def test_warns_when_link_text_is_learn_more
18+
# @file = "<a>Learn more</a>"
19+
# @linter.run(processed_source)
2020

21-
refute_empty @linter.offenses
22-
end
21+
# refute_empty @linter.offenses
22+
# end
2323

24-
def test_warns_when_link_text_is_read_more
25-
@file = "<a>Read more</a>"
26-
@linter.run(processed_source)
24+
# def test_warns_when_link_text_is_read_more
25+
# @file = "<a>Read more</a>"
26+
# @linter.run(processed_source)
2727

28-
refute_empty @linter.offenses
29-
end
28+
# refute_empty @linter.offenses
29+
# end
3030

31-
def test_warns_when_link_text_is_more
32-
@file = "<a>More</a>"
33-
@linter.run(processed_source)
31+
# def test_warns_when_link_text_is_more
32+
# @file = "<a>More</a>"
33+
# @linter.run(processed_source)
3434

35-
refute_empty @linter.offenses
36-
end
35+
# refute_empty @linter.offenses
36+
# end
3737

38-
def test_warns_when_link_text_is_link
39-
@file = "<a>Link</a>"
40-
@linter.run(processed_source)
38+
# def test_warns_when_link_text_is_link
39+
# @file = "<a>Link</a>"
40+
# @linter.run(processed_source)
4141

42-
refute_empty @linter.offenses
43-
end
42+
# refute_empty @linter.offenses
43+
# end
4444

45-
def test_does_not_warn_when_banned_text_is_part_of_more_text
46-
@file = "<a>Learn more about GitHub Stars</a>"
47-
@linter.run(processed_source)
45+
# def test_does_not_warn_when_banned_text_is_part_of_more_text
46+
# @file = "<a>Learn more about GitHub Stars</a>"
47+
# @linter.run(processed_source)
4848

49-
assert_empty @linter.offenses
50-
end
49+
# assert_empty @linter.offenses
50+
# end
5151

52-
def test_warns_when_link_rails_helper_text_is_banned_text
53-
@file = "<%= link_to('click here', redirect_url, id: 'redirect') %>"
54-
@linter.run(processed_source)
52+
# def test_ignores_when_aria_label_with_variable_is_set_on_link_tag
53+
# @file = <<~ERB
54+
# <a aria-label="<%= tooltip_text %>">Learn more</a>
55+
# ERB
56+
# @linter.run(processed_source)
5557

56-
refute_empty @linter.offenses
57-
end
58+
# assert_empty @linter.offenses
59+
# end
5860

59-
def test_does_not_warn_when_generic_text_is_link_rails_helper_sub_text
60-
@file = "<%= link_to('click here to learn about github', redirect_url, id: 'redirect') %>"
61-
@linter.run(processed_source)
61+
# def test_flags_when_aria_label_does_not_include_visible_link_text
62+
# @file = <<~ERB
63+
# <a aria-label="GitHub Sponsors">Learn more</a>
64+
# ERB
65+
# @linter.run(processed_source)
6266

63-
assert_empty @linter.offenses
64-
end
67+
# refute_empty @linter.offenses
68+
# end
6569

66-
def test_does_not_warns_if_element_has_correct_counter_comment
67-
@file = <<~ERB
68-
<%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
69-
<a>Link</a>
70-
ERB
71-
@linter.run(processed_source)
70+
# def test_does_not_flag_when_aria_label_includes_visible_link_text
71+
# @file = <<~ERB
72+
# <a aria-label="Learn more about GitHub Sponsors">Learn more</a>
73+
# ERB
74+
# @linter.run(processed_source)
7275

73-
assert_equal 0, @linter.offenses.count
74-
end
76+
# assert_empty @linter.offenses
77+
# end
78+
79+
# def test_ignores_when_aria_labelledby_is_set_on_link_tag
80+
# @file = "<a aria-labelledby='someElement'>Click here</a>"
81+
# @linter.run(processed_source)
82+
83+
# assert_empty @linter.offenses
84+
# end
85+
86+
# def test_warns_when_link_rails_helper_text_is_banned_text
87+
# @file = "<%= link_to('click here', redirect_url, id: 'redirect') %>"
88+
# @linter.run(processed_source)
89+
90+
# refute_empty @linter.offenses
91+
# end
7592

76-
def test_autocorrects_when_ignores_are_not_correct
93+
# def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_labelled_by
94+
# @file = "<%= link_to('learn more', 'aria-labelledby': 'element1234', id: 'redirect') %>"
95+
# @linter.run(processed_source)
96+
97+
# assert_empty @linter.offenses
98+
# end
99+
100+
# def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label_that_cannot_be_parsed_by_linter
101+
# @file = <<~ERB
102+
# <%= link_to('learn more', 'aria-label': some_variable, id: 'redirect') %>
103+
# ERB
104+
# @linter.run(processed_source)
105+
106+
# assert_empty @linter.offenses
107+
# end
108+
109+
def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label_that_cannot_be_parsed_by_linter_because_interpolated
77110
@file = <<~ERB
78-
<%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 2 %>
79-
<a>Link</a>
111+
<%= link_to('learn more', 'aria-label': "Learn #{@variable}", id: 'redirect') %>
80112
ERB
81-
refute_equal @file, corrected_content
113+
@linter.run(processed_source)
82114

83-
expected_content = <<~ERB
84-
<%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
85-
<a>Link</a>
86-
ERB
87-
assert_equal expected_content, corrected_content
115+
assert_empty @linter.offenses
88116
end
117+
118+
# def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label_that_contains_visible_text
119+
# @file = "<%= link_to('learn more', 'aria-label': 'learn more about GitHub', id: 'redirect') %>"
120+
# @linter.run(processed_source)
121+
122+
# assert_empty @linter.offenses
123+
# end
124+
125+
# def test_warns_when_link_rails_helper_text_is_banned_text_with_aria_label_hash
126+
# @file = "<%= link_to('learn more', aria: { label: 'learn more about GitHub' }, id: 'redirect') %>"
127+
# @linter.run(processed_source)
128+
129+
# assert_empty @linter.offenses
130+
# end
131+
132+
# def test_does_not_warn_when_generic_text_is_link_rails_helper_sub_text
133+
# @file = "<%= link_to('click here to learn about github', redirect_url, id: 'redirect') %>"
134+
# @linter.run(processed_source)
135+
136+
# assert_empty @linter.offenses
137+
# end
138+
139+
# def test_does_not_warns_if_element_has_correct_counter_comment
140+
# @file = <<~ERB
141+
# <%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
142+
# <a>Link</a>
143+
# ERB
144+
# @linter.run(processed_source)
145+
146+
# assert_equal 0, @linter.offenses.count
147+
# end
148+
149+
# def test_autocorrects_when_ignores_are_not_correct
150+
# @file = <<~ERB
151+
# <%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 2 %>
152+
# <a>Link</a>
153+
# ERB
154+
# refute_equal @file, corrected_content
155+
156+
# expected_content = <<~ERB
157+
# <%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
158+
# <a>Link</a>
159+
# ERB
160+
# assert_equal expected_content, corrected_content
161+
# end
89162
end

0 commit comments

Comments
 (0)