Merge pull request #32 from github/kh-avoid-generic-link

Add linter against generic link text

Author: khiga8

README.md

@@ -42,6 +42,7 @@ linters:
 ## Rules
 
 - [GitHub::Accessibility::AvoidBothDisabledAndAriaDisabled](./docs/rules/accessibility/avoid-both-disabled-and-aria-disabled.md)
+- [GitHub::Accessibility::AvoidGenericLinkTextCounter](./docs/rules/accessibility/avoid-generic-link-text-counter.md)
 - [GitHub::Accessibility::IframeHasTitle](./docs/rules/accessibility/iframe-has-title.md)
 - [GitHub::Accessibility::ImageHasAlt](./docs/rules/accessibility/image-has-alt.md)
 - [GitHub::Accessibility::NoAriaLabelMisuseCounter](./docs/rules/accessibility/no-aria-label-misuse-counter.md)

docs/rules/accessibility/avoid-generic-link-text-counter.md

@@ -0,0 +1,54 @@
+# Avoid generic link text
+
+## Rule Details
+
+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. While it is possible to provide a more specific link text by setting the `aria-label`, this results in divergence between the label and the text and is not an ideal, future-proof solution.
+
+Additionally, generic link text can also problematic for heavy zoom users where the link context is out of view.
+
+Ensure that your link text is descriptive and the purpose of the link is clear even when read out of context of surrounding text. 
+Learn more about how to write descriptive link text at [Access Guide: Write descriptive link text](https://www.accessguide.io/guide/descriptive-link-text)
+
+## Resources
+
+- [Primer: Links](https://primer.style/design/accessibility/links)
+- [Understanding Success Criterion 2.4.4: Link Purpose (In Context)](https://www.w3.org/WAI/WCAG21/Understanding/link-purpose-in-context.html)
+- [WebAim: Links and Hypertext](https://webaim.org/techniques/hypertext/)
+- [Deque: Use link text that make sense when read out of context](https://dequeuniversity.com/tips/link-text)
+
+## Examples
+
+### **Incorrect** code for this rule 👎
+
+```erb
+<a href="github.com/about">Learn more</a>
+```
+
+```erb
+<!-- also bad -->
+<a href="github.com/about">Read more</a>
+```
+
+```erb
+<!-- also bad -->
+<span>
+  <a href="github.com/new">Click here</a> to create a new repository.
+</span>
+```
+
+```erb
+<!-- also bad -->
+<%= link_to "Learn more", "#" %>
+```
+
+### **Correct** code for this rule  👍
+
+```erb
+<a href="github.com/about">Learn more about GitHub</a>
+```
+
+```erb
+<a href="github.com/new">Create a new repository</a>
+```

lib/erblint-github/linters/custom_helpers.rb

@@ -64,6 +64,13 @@ def generate_offense(klass, processed_source, tag, message = nil, replacement =
         add_offense(processed_source.to_source_range(tag.loc), offense, replacement)
       end
 
+      def generate_offense_from_source_range(klass, source_range, message = nil, replacement = nil)
+        message ||= klass::MESSAGE
+        message += "\nLearn more at https://github.com/github/erblint-github#rules.\n"
+        offense = ["#{simple_class_name}:#{message}", source_range.source].join("\n")
+        add_offense(source_range, offense, replacement)
+      end
+
       def possible_attribute_values(tag, attr_name)
         value = tag.attributes[attr_name]&.value || nil
         basic_conditional_code_check(value || "") || [value].compact

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

@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+require_relative "../../custom_helpers"
+
+module ERBLint
+  module Linters
+    module GitHub
+      module Accessibility
+        class AvoidGenericLinkTextCounter < Linter
+          include ERBLint::Linters::CustomHelpers
+          include LinterRegistry
+
+          BANNED_GENERIC_TEXT = [
+            "Read more",
+            "Learn more",
+            "Click here",
+            "More",
+            "Link",
+            "Here"
+          ].freeze
+          MESSAGE = "Avoid using generic link text such as #{BANNED_GENERIC_TEXT.join(', ')} which do not make sense in isolation."
+
+          def run(processed_source)
+            processed_source.ast.children.each_with_index do |node, index|
+              next unless node.methods.include?(:type) && node.type == :text
+
+              text = node.children.join.strip
+              # Checks HTML tags
+              if banned_text?(text)
+                prev_node = processed_source.ast.children[index - 1]
+                next_node = processed_source.ast.children[index + 1]
+
+                next unless tag_type?(prev_node) && tag_type?(next_node)
+
+                text_node_tag = BetterHtml::Tree::Tag.from_node(node)
+                prev_node_tag = BetterHtml::Tree::Tag.from_node(prev_node)
+                next_node_tag = BetterHtml::Tree::Tag.from_node(next_node)
+
+                # We only report if the text is nested between two link tags.
+                if link_tag?(prev_node_tag) && link_tag?(next_node_tag) && next_node_tag.closing?
+                  range = prev_node_tag.loc.begin_pos...text_node_tag.loc.end_pos
+                  source_range = processed_source.to_source_range(range)
+                  generate_offense_from_source_range(self.class, source_range)
+                end
+              end
+
+              # Checks Rails link helpers like `link_to`
+              erb_node = node.type == :erb ? node : node.descendants(:erb).first
+              next unless erb_node
+
+              _, _, code_node = *erb_node
+              source = code_node.loc.source
+              ruby_node = extract_ruby_node(source)
+              send_node = ruby_node&.descendants(:send)&.first
+              next unless send_node.methods.include?(:method_name) && send_node.method_name == :link_to
+
+              send_node.child_nodes.each do |child_node|
+                if child_node.methods.include?(:type) && child_node.type == :str && banned_text?(child_node.children.join)
+                  tag = BetterHtml::Tree::Tag.from_node(code_node)
+                  generate_offense(self.class, processed_source, tag)
+                  end
+              end
+            end
+            counter_correct?(processed_source)
+          end
+
+          def autocorrect(processed_source, offense)
+            return unless offense.context
+
+            lambda do |corrector|
+              if processed_source.file_content.include?("erblint:counter #{simple_class_name}")
+                # update the counter if exists
+                corrector.replace(offense.source_range, offense.context)
+              else
+                # add comment with counter if none
+                corrector.insert_before(processed_source.source_buffer.source_range, "#{offense.context}\n")
+              end
+            end
+          end
+
+          private
+
+          def banned_text?(text)
+            BANNED_GENERIC_TEXT.map(&:downcase).include?(text.downcase)
+          end
+
+          def extract_ruby_node(source)
+            BetterHtml::TestHelper::RubyNode.parse(source)
+          rescue ::Parser::SyntaxError
+            nil
+          end
+
+          def link_tag?(tag_node)
+            tag_node.name == "a"
+          end
+
+          def tag_type?(node)
+            node.methods.include?(:type) && node.type == :tag
+          end
+        end
+      end
+    end
+  end
+end

test/linters/accessibility/avoid_generic_link_text_counter_test.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class AvoidGenericLinkTextCounterTest < LinterTestCase
+  def linter_class
+    ERBLint::Linters::GitHub::Accessibility::AvoidGenericLinkTextCounter
+  end
+
+  def test_warns_when_link_text_is_click_here
+    @file = "<a>Click here</a>"
+    @linter.run(processed_source)
+
+    refute_empty @linter.offenses
+  end
+
+  def test_warns_when_link_text_is_learn_more
+    @file = "<a>Learn more</a>"
+    @linter.run(processed_source)
+
+    refute_empty @linter.offenses
+  end
+
+  def test_warns_when_link_text_is_read_more
+    @file = "<a>Read more</a>"
+    @linter.run(processed_source)
+
+    refute_empty @linter.offenses
+  end
+
+  def test_warns_when_link_text_is_more
+    @file = "<a>More</a>"
+    @linter.run(processed_source)
+
+    refute_empty @linter.offenses
+  end
+
+  def test_warns_when_link_text_is_link
+    @file = "<a>Link</a>"
+    @linter.run(processed_source)
+
+    refute_empty @linter.offenses
+  end
+
+  def test_does_not_warn_when_banned_text_is_part_of_more_text
+    @file = "<a>Learn more about GitHub Stars</a>"
+    @linter.run(processed_source)
+
+    assert_empty @linter.offenses
+  end
+
+  def test_warns_when_link_rails_helper_text_is_banned_text
+    @file = "<%= link_to('click here', redirect_url, id: 'redirect') %>"
+    @linter.run(processed_source)
+
+    refute_empty @linter.offenses
+  end
+
+  def test_does_not_warn_when_generic_text_is_link_rails_helper_sub_text
+    @file = "<%= link_to('click here to learn about github', redirect_url, id: 'redirect') %>"
+    @linter.run(processed_source)
+
+    assert_empty @linter.offenses
+  end
+
+  def test_does_not_warns_if_element_has_correct_counter_comment
+    @file = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
+      <a>Link</a>
+    ERB
+    @linter.run(processed_source)
+
+    assert_equal 0, @linter.offenses.count
+  end
+
+  def test_autocorrects_when_ignores_are_not_correct
+    @file = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 2 %>
+      <a>Link</a>
+    ERB
+    refute_equal @file, corrected_content
+
+    expected_content = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::AvoidGenericLinkTextCounter 1 %>
+      <a>Link</a>
+    ERB
+    assert_equal expected_content, corrected_content
+  end
+end