Add lint rule to make sure <details> elements have a <summary> element

README.md

@@ -48,6 +48,7 @@ linters:
 - [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)
+- [GitHub::Accessibility::DetailsHasSummary](./docs/rules/accessibility/details-has-summary.md)
 
 ## Testing
 

docs/rules/accessibility/details-has-summary.md

@@ -0,0 +1,35 @@
+# Details has summary
+
+## Rule Details
+`<details>` should have a `<summary>` element hinting to the user what they'll be expanding by interfacing with the element. `<summary>` elements are helpful for screen reader users navigating the website. They are also a good UI pattern since a user agent adds their own `<summary>` if a developer omitted it with no context on what UI the details element will expand.
+
+### 👎 Examples of **incorrect** code for this rule:
+
+```erb
+<details>
+  I don't have a summary tag!
+</details>
+```
+
+```erb
+<details>
+  <div><summary>Expand me!</summary></div>
+  I have a invalid summary tag! The summary tag needs to be a direct child of the details tag.
+</details>
+```
+
+### 👍 Examples of **correct** code for this rule:
+
+```erb
+<details>
+  <summary>Expand me!</summary>
+  I do have a summary tag!
+</details>
+````
+
+```erb
+<details>
+  I do have a summary tag!
+  <summary>Expand me!</summary>
+</details>
+````

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

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require_relative "../../custom_helpers"
+require_relative "../../tag_tree_helpers"
+
+module ERBLint
+  module Linters
+    module GitHub
+      module Accessibility
+        class DetailsHasSummary < Linter
+          include ERBLint::Linters::CustomHelpers
+          include ERBLint::Linters::TagTreeHelpers
+          include LinterRegistry
+
+          MESSAGE = "<details> elements need to have explict <summary> elements"
+
+          def run(processed_source)
+            has_summary = false
+
+            (tags, tag_tree) = build_tag_tree(processed_source)
+
+            tags.each do |tag|
+              next if tag.name != "details" || tag.closing?
+
+              details = tag_tree[tag]
+
+              details[:children].each do |child|
+                if child && child[:tag].name == "summary"
+                  has_summary = true
+                  break
+                end
+              end
+
+              generate_offense(self.class, processed_source, details[:tag]) unless has_summary
+
+              has_summary = false
+            end
+
+            rule_disabled?(processed_source)
+          end
+        end
+      end
+    end
+  end
+end

lib/erblint-github/linters/tag_tree_helpers.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+module ERBLint
+  module Linters
+    # Helpers used by linters to organize HTML tags into abstract syntax trees.
+    module TagTreeHelpers
+      # from https://github.com/Shopify/erb-lint/blob/6179ee2d9d681a6ec4dd02351a1e30eefa748d3d/lib/erb_lint/linters/self_closing_tag.rb
+      SELF_CLOSING_TAGS = %w[
+        area base br col command embed hr input keygen
+        link menuitem meta param source track wbr img
+      ].freeze
+
+      # This assumes that the AST provided represents valid HTML, where each tag has a corresponding closing tag.
+      # From the tags, we build a structured tree which represents the tag hierarchy.
+      # With this, we are able to know where the tags start and end.
+      def build_tag_tree(processed_source)
+        nodes = processed_source.ast.children
+        tag_tree = {}
+        tags = []
+        current_opened_tag = nil
+
+        nodes.each do |node|
+          tag = BetterHtml::Tree::Tag.from_node(node)
+
+          if node.type == :tag
+            # get the tag from previously calculated list so the references are the same
+            tags << tag
+
+            if tag.closing?
+              if current_opened_tag && tag.name == current_opened_tag.name
+                tag_tree[current_opened_tag][:closing] = tag
+                current_opened_tag = tag_tree[current_opened_tag][:parent]
+              end
+
+              next
+            end
+
+            self_closing = self_closing?(tag)
+
+            tag_tree[tag] = {
+              tag: tag,
+              closing: self_closing ? tag : nil,
+              parent: current_opened_tag,
+              children: []
+            }
+
+            tag_tree[current_opened_tag][:children] << tag_tree[tag] if current_opened_tag
+            current_opened_tag = tag unless self_closing
+          elsif current_opened_tag
+            tag_tree[current_opened_tag][:children] << {
+              tag: tag,
+              closing: tag,
+              parent: current_opened_tag,
+              children: []
+            }
+          end
+        end
+
+        [tags, tag_tree]
+      end
+
+      def self_closing?(tag)
+        tag.self_closing? || SELF_CLOSING_TAGS.include?(tag.name)
+      end
+    end
+  end
+end

test/linters/accessibility/details_has_summary_test.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class DetailsHasSummary < LinterTestCase
+  def linter_class
+    ERBLint::Linters::GitHub::Accessibility::DetailsHasSummary
+  end
+
+  def test_warns_if_details_doesnt_have_a_summary
+    @file = "<details>I don't have a summary element</details>"
+    @linter.run(processed_source)
+
+    assert_equal @linter.offenses.count, 1
+  end
+
+  def test_does_not_warn_if_details_has_a_summary_in_a_weird_place
+    @file = "<details><p>Surprise text!</p><summary>Expand me!</summary></details>"
+    @linter.run(processed_source)
+
+    assert_empty @linter.offenses
+  end
+
+  def test_does_not_warn_if_details_has_a_summary
+    @file = "<details><summary>Expand me!</summary><button>Surprise button!</button></details>"
+    @linter.run(processed_source)
+
+    assert_empty @linter.offenses
+  end
+
+  def test_does_warns_if_details_has_a_summary_as_an_inner_child
+    @file = "<details><p><summary>Expand me!</summary></p><button>Surprise button!</button></details>"
+    @linter.run(processed_source)
+
+    assert_equal @linter.offenses.count, 1
+  end
+end