Fix lint rule logic

Author: khiga8

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

@@ -10,31 +10,7 @@ class LandmarkHasLabel < Linter
           include ERBLint::Linters::CustomHelpers
           include LinterRegistry
 
-          LANDMARK_ROLES = %w[complementary navigation region search].freeze
-          LANDMARK_TAGS = %w[aside nav section].freeze
-          MESSAGE = "Landmark elements should have an aria-label attribute, or aria-labelledby if a heading elements exists in the landmark."
-          ROLE_TAG_MAPPING = { "complementary" => "aside", "navigation" => "nav", "region" => "section" }.freeze
-
-          def get_additional_message(tag, roles)
-            role_matched = (roles & ROLE_TAG_MAPPING.keys).first
-            if role_matched
-              tag_matched = ROLE_TAG_MAPPING[role_matched]
-
-              if tag.name == tag_matched
-                "The <#{tag_matched}> element will automatically communicate a role of '#{role_matched}'. You can safely drop the role attribute."
-              else
-                replace_message = if tag.name == "div"
-                                    "If possible replace this tag with a <#{tag_matched}>."
-                                  else
-                                    "Wrapping this element in a <#{tag_matched}> and setting a label on it is reccomended."
-                                  end
-
-                "The <#{tag_matched}> element will automatically communicate a role of '#{role_matched}'. #{replace_message}"
-              end
-            elsif roles.include?("search") && tag.name != "form"
-              "The 'search' role works best when applied to a <form> element. If possible replace this tag with a <form>."
-            end
-          end
+          MESSAGE = "The navigation landmark should have a unique accessible name via `aria-label` or `aria-labelledby`."
 
           class ConfigSchema < LinterConfig
             property :counter_enabled, accepts: [true, false], default: false, reader: :counter_enabled?
@@ -44,17 +20,15 @@ class ConfigSchema < LinterConfig
           def run(processed_source)
             tags(processed_source).each do |tag|
               next if tag.closing?
-
-              possible_roles = possible_attribute_values(tag, "role")
-              next unless LANDMARK_TAGS.include?(tag.name) && (possible_roles & LANDMARK_ROLES).empty?
-              next if tag.attributes["aria-label"]&.value&.present? || tag.attributes["aria-labelledby"]&.value&.present?
-
-              message = get_additional_message(tag, possible_roles)
-              if message
-                generate_offense(self.class, processed_source, tag, "#{MESSAGE}\n#{message}")
-              else
-                generate_offense(self.class, processed_source, tag)
+              next unless possible_attribute_values(tag, "role").include?("navigation") || tag.name == "nav"
+              if possible_attribute_values(tag, "aria-label").empty? && possible_attribute_values(tag, "aria-labelledby").empty?
+                message = MESSAGE
+                if tag.name != "nav"
+                  message += "Additionally, you can safely drop the `role='navigation'` and replace it with the native HTML `nav` element."
+                end
+                generate_offense(self.class, processed_source, tag, message)
               end
+
             end
 
             if @config.counter_enabled?

test/linters/accessibility/landmark_has_label_test.rb

@@ -7,24 +7,42 @@ def linter_class
     ERBLint::Linters::GitHub::Accessibility::LandmarkHasLabel
   end
 
-  def test_warns_if_landmark_has_no_label
+  def test_warns_if_navigation_landmark_has_no_label
     @file = <<~ERB
-      <section>
-        <h1>This is a text</h1>
-      </section>
+      <nav>
+      </nav>
     ERB
     @linter.run(processed_source)
 
     assert_equal(1, @linter.offenses.count)
-    error_messages = @linter.offenses.map(&:message).sort
-    assert_match(/Landmark elements should have an aria-label attribute, or aria-labelledby if a heading elements exists in the landmark./, error_messages.last)
+    assert_match(/The navigation landmark should have a unique accessible name via `aria-label` or `aria-labelledby`./, @linter.offenses.first.message)
   end
 
-  def test_does_not_warn_if_landmark_has_label
+  def test_warns_if_navigation_role_landmark_has_no_label
     @file = <<~ERB
-      <section aria-labelledby="title_id"t>
-        <h1 id="title_id">This is a text</h1>
-      </section>
+      <div role="navigation">
+      </div>
+    ERB
+    @linter.run(processed_source)
+
+    assert_equal(1, @linter.offenses.count)
+    assert_match(/The navigation landmark should have a unique accessible name via `aria-label` or `aria-labelledby`./, @linter.offenses.first.message)
+  end
+
+  def test_does_not_warn_if_navigation_landmark_has_aria_labelled_by
+    @file = <<~ERB
+      <nav aria-labelledby="title_id"t>
+      </nav>
+    ERB
+    @linter.run(processed_source)
+
+    assert_empty @linter.offenses
+  end
+
+  def test_does_not_warn_if_navigation_landmark_has_aria_label
+    @file = <<~ERB
+      <nav aria-label="Repos"t>
+      </nav>
     ERB
     @linter.run(processed_source)