Merge pull request #19 from github/kh-add_no_title_counter

Port over counter implementation and introduce linter against title attribute

Author: khiga8

README.md

@@ -35,6 +35,8 @@ linters:
     enabled: true
   GitHub::Accessibility::NoRedundantImageAlt:
     enabled: true
+  GitHub::Accessibility::NoTitleAttributeCounter:
+    enabled: true
 ```
 
 ## Rules
@@ -45,6 +47,7 @@ linters:
 - [GitHub::Accessibility::NoAriaLabelMisuse](./docs/rules/accessibility/no-aria-label-misuse.md)
 - [GitHub::Accessibility::NoPositiveTabIndex](./docs/rules/accessibility/no-positive-tab-index.md)
 - [GitHub::Accessibility::NoRedundantImageAlt](./docs/rules/accessibility/no-redundant-image-alt.md)
+- [GitHub::Accessibility::NoTitleAttributeCounter](./docs/rules/accessibility/no-title-attribute-counter.md)
 
 ## Testing
 

docs/rules/accessibility/no-title-attribute-counter.md

@@ -0,0 +1,39 @@
+# No title attribute counter
+
+## Rule Details
+
+The `title` attribute is strongly discouraged. The only exception is on an `<iframe>` element. It is hardly useful and cannot be accessed by multiple groups of users including keyboard-only users and mobile users.
+
+The `title` attribute is commonly seen set on links, matching the link text. This is redundant and unnecessary so it can be simply be removed.
+
+If you are considering the`title` attribute to provide supplementary description, consider whether the text in question can be persisted in the design. Alternatively, if it's important to display supplementary text that is hidden by default, consider using an **accessible** tooltip implementation that uses the `aria-labelledby` or `aria-describedby` semantics. Even so, proceed with caution: tooltips should only be used on interactive elements like links or buttons.
+
+### Should I use the `title` attribute to provide an accessible name for an `<svg>`?
+
+Use a `<title>` element instead of the `title` attribute, or an `aria-label`.
+
+### Resources
+
+- [TPGI: Using the HTML title attribute ](https://www.tpgi.com/using-the-html-title-attribute/)
+- [The Trials and Tribulations of the Title Attribute](https://www.24a11y.com/2017/the-trials-and-tribulations-of-the-title-attribute/)
+
+### 👎 Examples of **incorrect** code for this rule:
+
+```erb
+<a title="A home for all developers" href="github.com">GitHub</a>
+```
+
+```erb
+<a href="/" title="github.com">GitHub</a>
+```
+
+### 👍 Examples of **correct** code for this rule:
+
+```erb
+<a href="github.com" aria-describedby="description">GitHub</a>
+<p id="description" class="tooltip js-tooltip">A home for all developers</p>
+```
+
+```erb
+<a href="github.com">GitHub</a>
+```

lib/erblint-github/linters/custom_helpers.rb

@@ -24,6 +24,39 @@ def rule_disabled?(processed_source)
         end
       end
 
+      def counter_correct?(processed_source)
+        comment_node = nil
+        expected_count = 0
+        rule_name = simple_class_name
+        offenses_count = @offenses.length
+
+        processed_source.parser.ast.descendants(:erb).each do |node|
+          indicator_node, _, code_node, = *node
+          indicator = indicator_node&.loc&.source
+          comment = code_node&.loc&.source&.strip
+
+          if indicator == "#" && comment.start_with?("erblint:counter") && comment.match(rule_name)
+            comment_node = node
+            expected_count = comment.match(/\s(\d+)\s?$/)[1].to_i
+          end
+        end
+
+        if offenses_count.zero?
+          # have to adjust to get `\n` so we delete the whole line
+          add_offense(processed_source.to_source_range(comment_node.loc.adjust(end_pos: 1)), "Unused erblint:counter comment for #{rule_name}", "") if comment_node
+          return
+        end
+
+        first_offense = @offenses[0]
+
+        if comment_node.nil?
+          add_offense(processed_source.to_source_range(first_offense.source_range), "#{rule_name}: If you must, add <%# erblint:counter #{rule_name} #{offenses_count} %> to bypass this check.", "<%# erblint:counter #{rule_name} #{offenses_count} %>")
+        else
+          clear_offenses
+          add_offense(processed_source.to_source_range(comment_node.loc), "Incorrect erblint:counter number for #{rule_name}. Expected: #{expected_count}, actual: #{offenses_count}.", "<%# erblint:counter #{rule_name} #{offenses_count} %>") if expected_count != offenses_count
+        end
+      end
+
       def generate_offense(klass, processed_source, tag, message = nil, replacement = nil)
         message ||= klass::MESSAGE
         message += "\nLearn more at https://github.com/github/erblint-github#rules.\n"

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

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require_relative "../../custom_helpers"
+
+module ERBLint
+  module Linters
+    module GitHub
+      module Accessibility
+        class NoTitleAttributeCounter < Linter
+          include ERBLint::Linters::CustomHelpers
+          include LinterRegistry
+
+          MESSAGE = "The title attribute should never be used unless for an `<iframe>` as it is inaccessible for several groups of users."
+
+          def run(processed_source)
+            tags(processed_source).each do |tag|
+              next if tag.name == "iframe"
+              next if tag.closing?
+
+              title = possible_attribute_values(tag, "title")
+              generate_offense(self.class, processed_source, tag) if title.present?
+            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
+        end
+      end
+    end
+  end
+end

test/custom_helpers_test.rb

@@ -38,8 +38,36 @@ def test_rule_disabled_adds_offense_if_disable_comment_is_present_but_no_offense
     assert_empty @linter.offenses
 
     extended_linter.rule_disabled?(processed_source)
+    assert_equal "Unused erblint:disable comment for CustomHelpersTest::FakeLinter", @linter.offenses.first.message
+  end
 
-    assert_equal @linter.offenses.length, 1
+  def test_counter_correct_does_not_add_offense_if_counter_matches_offense_count
+    @file = <<~HTML
+      <%# erblint:counter CustomHelpersTest::FakeLinter 1 %>
+    HTML
+    @linter.offenses = ["fake offense"]
+
+    extended_linter.counter_correct?(processed_source)
+    assert_empty @linter.offenses
+  end
+
+  def test_counter_correct_add_offense_if_counter_comment_is_unused
+    @file = <<~HTML
+      <%# erblint:counter CustomHelpersTest::FakeLinter 1 %>
+    HTML
+
+    extended_linter.counter_correct?(processed_source)
+    assert_equal "Unused erblint:counter comment for CustomHelpersTest::FakeLinter", @linter.offenses.first.message
+  end
+
+  def test_counter_correct_add_offense_if_counter_comment_count_is_incorrect
+    @file = <<~HTML
+      <%# erblint:counter CustomHelpersTest::FakeLinter 2 %>
+    HTML
+    @linter.offenses = ["fake offense"]
+
+    extended_linter.counter_correct?(processed_source)
+    assert_equal "Incorrect erblint:counter number for CustomHelpersTest::FakeLinter. Expected: 2, actual: 1.", @linter.offenses.first.message
   end
 
   def test_generate_offense_with_message_defined_in_linter_class

test/linter_test_case.rb

@@ -24,4 +24,15 @@ def processed_source
   def tags
     processed_source.parser.nodes_with_type(:tag).map { |tag_node| BetterHtml::Tree::Tag.from_node(tag_node) }
   end
+
+  def corrected_content
+    @corrected_content ||= begin
+      source = processed_source
+
+      @linter.run(source)
+      corrector = ERBLint::Corrector.new(source, offenses)
+
+      corrector.corrected_content
+    end
+  end
 end

test/linters/accessibility/no_title_attribute_counter_test.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class NoTitleAttributeCounterTest < LinterTestCase
+  def linter_class
+    ERBLint::Linters::GitHub::Accessibility::NoTitleAttributeCounter
+  end
+
+  def test_warns_if_element_sets_title_and_has_no_counter_comment
+    @file = "<img title='octopus'></img>"
+    @linter.run(processed_source)
+
+    assert_equal(2, @linter.offenses.count)
+    error_messages = @linter.offenses.map(&:message).sort
+    assert_match(/If you must, add <%# erblint:counter GitHub::Accessibility::NoTitleAttributeCounter 1 %> to bypass this check./, error_messages.first)
+    assert_match(/The title attribute should never be used unless for an `<iframe>` as it is inaccessible for several groups of users./, error_messages.last)
+  end
+
+  def test_does_not_warns_if_element_sets_title_and_has_correct_counter_comment
+    @file = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::NoTitleAttributeCounter 1 %>
+      <a href="/" title="bad">some website</a>
+    ERB
+    @linter.run(processed_source)
+
+    assert_equal 0, @linter.offenses.count
+  end
+
+  def test_does_not_warn_if_iframe_sets_title
+    @file = "<iframe title='Introduction video'></iframe>"
+    @linter.run(processed_source)
+
+    assert_empty @linter.offenses
+  end
+
+  def test_does_not_autocorrect_when_ignores_are_correct
+    @file = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::NoTitleAttributeCounter 1 %>
+      <a href="/" title="bad">some website</a>
+    ERB
+
+    assert_equal @file, corrected_content
+  end
+
+  def test_does_autocorrect_when_ignores_are_not_correct
+    @file = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::NoTitleAttributeCounter 3 %>
+      <a href="/" title="bad">some website</a>
+    ERB
+    refute_equal @file, corrected_content
+
+    expected_content = <<~ERB
+      <%# erblint:counter GitHub::Accessibility::NoTitleAttributeCounter 1 %>
+      <a href="/" title="bad">some website</a>
+    ERB
+    assert_equal expected_content, corrected_content
+  end
+end