Skip to content

Commit 6ee53c2

Browse files
khiga8khiga8
committed
Do not evaluate link_to helpers with ARIA label attributes
1 parent 1b233fe commit 6ee53c2

File tree

2 files changed

+130
-127
lines changed

2 files changed

+130
-127
lines changed

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

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def run(processed_source)
2525
next unless node.methods.include?(:type) && node.type == :text
2626

2727
text = node.children.join.strip
28+
2829
# Checks HTML tags
2930
if banned_text?(text)
3031
prev_node = processed_source.ast.children[index - 1]
@@ -39,10 +40,12 @@ def run(processed_source)
3940
aria_label = possible_attribute_values(prev_node_tag, "aria-label")
4041
aria_labelledby = possible_attribute_values(prev_node_tag, "aria-labelledby")
4142

42-
# We only report if the text is nested between two link tags.
43+
# Checks if nested between two link tags.
4344
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+
# We skip because we cannot reliably check accessible name given aria-labelledby, or an aria-label that is set to a variable
46+
# with static code analysis.
4547
next if aria_labelledby.present? || (aria_label.present? && aria_label.join.include?("<%="))
48+
# We skip because aria-label contains visible text. Relates to Success Criterion 2.5.3: Label in Name
4649
next if aria_label.present? && valid_accessible_name?(aria_label.join, text)
4750

4851
range = prev_node_tag.loc.begin_pos...text_node_tag.loc.end_pos
@@ -71,19 +74,16 @@ def run(processed_source)
7174
child_node.descendants(:pair).each do |pair_node|
7275
next unless pair_node.children.first.type?(:sym)
7376

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
77+
if pair_node.children.first.children.join == "aria"
78+
pair_node.children[1].descendants(:sym).each do |sym_node|
79+
banned_text = nil if sym_node.children.join == "label" || sym_node.children.join == "labelledby"
80+
end
8681
end
82+
83+
# Skip if `link_to` has `aria-labelledby` or `aria-label` which we cannot be evaluated accurately with ERB lint alone.
84+
# ERB lint removes Ruby string interpolation so the `aria-label` for "<%= link_to 'Learn more', "aria-label": "Learn #{@some_variable}" %>" will
85+
# be `Learn` which is unreliable so we have to skip entirely unlike with HTML :(
86+
banned_text = nil if pair_node.children.first.children.join == "aria-labelledby" || pair_node.children.first.children.join == "aria-label"
8787
end
8888
end
8989
if banned_text.present?
@@ -114,10 +114,13 @@ def banned_text?(text)
114114
BANNED_GENERIC_TEXT.map(&:downcase).include?(text.downcase)
115115
end
116116

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)
117+
def valid_accessible_name?(aria_label, text)
118+
aria_label.start_with?(text)
119+
end
120+
121+
# Checks if Ruby node contains `aria-label` or `aria-labelledby`
122+
def ruby_node_contains_aria_label_attributes?(pair_node)
123+
pair_node.children.first.children.join == "aria-labelledby" || pair_node.children.first.children.join == "aria-label"
121124
end
122125

123126
def extract_ruby_node(source)

‎test/linters/accessibility/avoid_generic_link_text_counter_test.rb‎

Lines changed: 109 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -7,106 +7,106 @@ 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_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)
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)
5757

58-
# assert_empty @linter.offenses
59-
# end
58+
assert_empty @linter.offenses
59+
end
6060

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)
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)
6666

67-
# refute_empty @linter.offenses
68-
# end
67+
refute_empty @linter.offenses
68+
end
6969

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)
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)
7575

76-
# assert_empty @linter.offenses
77-
# end
76+
assert_empty @linter.offenses
77+
end
7878

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)
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)
8282

83-
# assert_empty @linter.offenses
84-
# end
83+
assert_empty @linter.offenses
84+
end
8585

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)
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)
8989

90-
# refute_empty @linter.offenses
91-
# end
90+
refute_empty @linter.offenses
91+
end
9292

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)
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)
9696

97-
# assert_empty @linter.offenses
98-
# end
97+
assert_empty @linter.offenses
98+
end
9999

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)
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)
105105

106-
# assert_empty @linter.offenses
107-
# end
106+
assert_empty @linter.offenses
107+
end
108108

109-
def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label_that_cannot_be_parsed_by_linter_because_interpolated
109+
def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label_since_cannot_be_parsed_accurately_by_linter
110110
@file = <<~ERB
111111
<%= link_to('learn more', 'aria-label': "Learn #{@variable}", id: 'redirect') %>
112112
ERB
@@ -115,48 +115,48 @@ def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label_that
115115
assert_empty @linter.offenses
116116
end
117117

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)
118+
def test_ignores_when_link_rails_helper_text_is_banned_text_with_aria_label
119+
@file = "<%= link_to('learn more', 'aria-label': 'learn more about GitHub', id: 'redirect') %>"
120+
@linter.run(processed_source)
121121

122-
# assert_empty @linter.offenses
123-
# end
122+
assert_empty @linter.offenses
123+
end
124124

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)
125+
def test_ignores_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)
128128

129-
# assert_empty @linter.offenses
130-
# end
129+
assert_empty @linter.offenses
130+
end
131131

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)
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)
135135

136-
# assert_empty @linter.offenses
137-
# end
136+
assert_empty @linter.offenses
137+
end
138138

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)
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)
145145

146-
# assert_equal 0, @linter.offenses.count
147-
# end
146+
assert_equal 0, @linter.offenses.count
147+
end
148148

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
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
155155

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
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
162162
end

0 commit comments

Comments
 (0)