Skip to content

Commit 3480742

Browse files
committed
Introduce generic link text linter
1 parent 2c50cc7 commit 3480742

File tree

4 files changed

+237
-0
lines changed

4 files changed

+237
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ linters:
4242
## Rules
4343
4444
- [GitHub::Accessibility::AvoidBothDisabledAndAriaDisabled](./docs/rules/accessibility/avoid-both-disabled-and-aria-disabled.md)
45+
- [GitHub::Accessibility::AvoidGenericLinkTextCounter](./docs/rules/accessibility/avoid-generic-link-text-counter.md)
4546
- [GitHub::Accessibility::IframeHasTitle](./docs/rules/accessibility/iframe-has-title.md)
4647
- [GitHub::Accessibility::ImageHasAlt](./docs/rules/accessibility/image-has-alt.md)
4748
- [GitHub::Accessibility::NoAriaLabelMisuseCounter](./docs/rules/accessibility/no-aria-label-misuse-counter.md)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Avoid generic link text
2+
3+
## Rule Details
4+
5+
Avoid setting generic link text like, "Click here", "Read more", and "Learn more" which do not make sense when read out of context. Screen reader users often tab through links on a page to quickly find content without needing to listen to the full page. When link text is too generic, it becomes difficult to quickly identify the destination of the link.
6+
7+
## Resources
8+
9+
- [Primer: Links](https://primer.style/design/accessibility/link)
10+
- [Understanding Success Criterion 2.4.4: Link Purpose (In Context)](https://www.w3.org/WAI/WCAG21/Understanding/link-purpose-in-context.html)
11+
- [WebAim: Links and Hypertext](https://webaim.org/techniques/hypertext/)
12+
- [Deque: Use link text that make sense when read out of context](https://dequeuniversity.com/tips/link-text)
13+
14+
## Examples
15+
16+
### **Incorrect** code for this rule 👎
17+
18+
```erb
19+
<a href="github.com/about">Learn more</a>
20+
```
21+
22+
```erb
23+
<!-- also bad -->
24+
<a href="github.com/about">Read more</a>
25+
```
26+
27+
```erb
28+
<!-- also bad -->
29+
<span>
30+
<a href="github.com/new">Click here</a> to create a new repository.
31+
</span>
32+
```
33+
34+
```erb
35+
<!-- also bad -->
36+
<%= link_to "Learn more", "#" %>
37+
```
38+
39+
### **Correct** code for this rule 👍
40+
41+
```erb
42+
<a href="github.com/about">Learn more about GitHub</a>
43+
```
44+
45+
```erb
46+
<a href="github.com/new">Create a new repository</a>
47+
```
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# frozen_string_literal: true
2+
3+
require_relative "../../custom_helpers"
4+
5+
module ERBLint
6+
module Linters
7+
module GitHub
8+
module Accessibility
9+
class AvoidGenericLinkTextCounter < Linter
10+
include ERBLint::Linters::CustomHelpers
11+
include LinterRegistry
12+
13+
BANNED_GENERIC_TEXT = [
14+
"Read more",
15+
"Learn more",
16+
"Click here",
17+
"More",
18+
"Link"
19+
].freeze
20+
MESSAGE = "Avoid using generic link text such as #{BANNED_GENERIC_TEXT.join(', ')} which do not make sense in isolation."
21+
22+
def run(processed_source)
23+
processed_source.ast.children.each_with_index do |node, index|
24+
next unless node.methods.include?(:type) && node.type == :text
25+
text = node.children.join.strip
26+
# Checks HTML tags
27+
if banned_text?(text)
28+
prev_node = processed_source.ast.children[index-1]
29+
next_node = processed_source.ast.children[index+1]
30+
31+
next unless tag?(prev_node) && tag?(next_node)
32+
33+
prev_node_tag = BetterHtml::Tree::Tag.from_node(prev_node)
34+
next_node_tag = BetterHtml::Tree::Tag.from_node(next_node)
35+
36+
# We only report if the text is nested between two link tags.
37+
if link_tag?(prev_node_tag) && link_tag?(next_node_tag) && next_node_tag.closing?
38+
generate_offense(self.class, processed_source, prev_node_tag)
39+
end
40+
end
41+
42+
# Checks Rails link helpers like `link_to`
43+
erb_node = node.type == :erb ? node : node.descendants(:erb).first
44+
next unless erb_node
45+
_, _, code_node = *erb_node
46+
source = code_node.loc.source
47+
ruby_node = extract_ruby_node(source)
48+
send_node = ruby_node&.descendants(:send)&.first
49+
if send_node.methods.include?(:method_name) && send_node.method_name == :link_to
50+
send_node.child_nodes.each do |child_node|
51+
if child_node.methods.include?(:type) && child_node.type == :str
52+
if banned_text?(child_node.children.join)
53+
tag = BetterHtml::Tree::Tag.from_node(code_node)
54+
generate_offense(self.class, processed_source, tag)
55+
end
56+
end
57+
end
58+
end
59+
end
60+
counter_correct?(processed_source)
61+
end
62+
63+
def autocorrect(processed_source, offense)
64+
return unless offense.context
65+
66+
lambda do |corrector|
67+
if processed_source.file_content.include?("erblint:counter #{simple_class_name}")
68+
# update the counter if exists
69+
corrector.replace(offense.source_range, offense.context)
70+
else
71+
# add comment with counter if none
72+
corrector.insert_before(processed_source.source_buffer.source_range, "#{offense.context}\n")
73+
end
74+
end
75+
end
76+
77+
private
78+
79+
def banned_text?(text)
80+
BANNED_GENERIC_TEXT.map(&:downcase).include?(text.downcase)
81+
end
82+
83+
def extract_ruby_node(source)
84+
BetterHtml::TestHelper::RubyNode.parse(source)
85+
rescue ::Parser::SyntaxError
86+
nil
87+
end
88+
89+
def link_tag?(tag_node)
90+
tag_node.name == "a"
91+
end
92+
93+
def tag?(node)
94+
node.methods.include?(:type) && node.type == :tag
95+
end
96+
end
97+
end
98+
end
99+
end
100+
end
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# frozen_string_literal: true
2+
3+
require "test_helper"
4+
5+
class AvoidGenericLinkTextCounterTest < LinterTestCase
6+
def linter_class
7+
ERBLint::Linters::GitHub::Accessibility::AvoidGenericLinkTextCounter
8+
end
9+
10+
def test_warns_when_link_text_is_click_here
11+
@file = "<a>Click here</a>"
12+
@linter.run(processed_source)
13+
14+
refute_empty @linter.offenses
15+
end
16+
17+
def test_warns_when_link_text_is_learn_more
18+
@file = "<a>Learn more</a>"
19+
@linter.run(processed_source)
20+
21+
refute_empty @linter.offenses
22+
end
23+
24+
def test_warns_when_link_text_is_read_more
25+
@file = "<a>Read more</a>"
26+
@linter.run(processed_source)
27+
28+
refute_empty @linter.offenses
29+
end
30+
31+
def test_warns_when_link_text_is_more
32+
@file = "<a>More</a>"
33+
@linter.run(processed_source)
34+
35+
refute_empty @linter.offenses
36+
end
37+
38+
def test_warns_when_link_text_is_link
39+
@file = "<a>Link</a>"
40+
@linter.run(processed_source)
41+
42+
refute_empty @linter.offenses
43+
end
44+
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)
48+
49+
assert_empty @linter.offenses
50+
end
51+
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)
55+
56+
refute_empty @linter.offenses
57+
end
58+
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)
62+
63+
assert_empty @linter.offenses
64+
end
65+
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)
72+
73+
assert_equal 0, @linter.offenses.count
74+
end
75+
76+
def test_autocorrects_when_ignores_are_not_correct
77+
@file = <<~ERB
78+
<%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 2 %>
79+
<a>Link</a>
80+
ERB
81+
refute_equal @file, corrected_content
82+
83+
expected_content = <<~ERB
84+
<%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
85+
<a>Link</a>
86+
ERB
87+
assert_equal expected_content, corrected_content
88+
end
89+
end

0 commit comments

Comments
 (0)