Add lint rule against .tooltipped and introduce accessibility.yml con…

…fig (#2086)

.changeset/cuddly-laws-peel.md

@@ -0,0 +1,5 @@
+---
+"@primer/view-components": minor
+---
+
+Add lint rule against .tooltipped and introduce accessibility.yml config

.erb-lint.yml

@@ -1,9 +1,13 @@
 ---
 EnableDefaultLinters: true
+inherit_from:
+  - lib/primer/view_components/linters/accessibility.yml
 inherit_gem:
   erblint-github:
     - config/accessibility.yml
 linters:
+  NoUnusedDisable:
+    enabled: true
   DeprecatedComponentsCounter:
     enabled: true
   ButtonComponentMigrationCounter:

lib/primer/view_components/linters/accessibility.yml

@@ -0,0 +1,7 @@
+# Accessibility-focused lint rule configurations, tracked by accessibility scorecard.
+---
+linters:
+  NoUnusedDisable:
+    enabled: true
+  Primer::Accessibility::TooltippedMigration:
+    enabled: true

lib/primer/view_components/linters/argument_mappers/base.rb

@@ -53,7 +53,7 @@ def map_classes(classes_node)
           system_arguments = system_arguments_to_args(classes_node.value)
           args = classes_to_args(system_arguments[:classes]&.split || [])
 
-          invalid_classes = args[:classes].select { |class_name| Primer::Classify::Validation.invalid?(class_name) }
+          invalid_classes = args[:classes].select { |class_name| ::Primer::Classify::Validation.invalid?(class_name) }
 
           raise ConversionError, "Cannot convert #{'class'.pluralize(invalid_classes.size)} #{invalid_classes.join(',')}" if invalid_classes.present?
 
@@ -81,7 +81,7 @@ def classes_to_args(classes)
         end
 
         def system_arguments_to_args(classes)
-          system_arguments = Primer::Classify::Utilities.classes_to_hash(classes)
+          system_arguments = ::Primer::Classify::Utilities.classes_to_hash(classes)
 
           # need to transform symbols to strings with leading `:`
           system_arguments.transform_values do |v|

lib/primer/view_components/linters/base_linter.rb

@@ -5,7 +5,7 @@
 require "primer/view_components/constants"
 
 require_relative "tag_tree_helpers"
-
+require_relative "helpers/rule_helpers"
 # :nocov:
 
 module ERBLint
@@ -17,6 +17,7 @@ module Linters
     # * `REQUIRED_ARGUMENTS` - optional - A list of HTML attributes that are required by the component.
     class BaseLinter < Linter
       include TagTreeHelpers
+      include Helpers::RuleHelpers
 
       DUMP_FILE = ".erblint-counter-ignore.json"
       DISALLOWED_CLASSES = [].freeze
@@ -134,10 +135,6 @@ def correct_counter(corrector, processed_source, offense)
         end
       end
 
-      def tags(processed_source)
-        processed_source.parser.nodes_with_type(:tag).map { |tag_node| BetterHtml::Tree::Tag.from_node(tag_node) }
-      end
-
       def counter_correct?(processed_source)
         comment_node = nil
         expected_count = 0
@@ -178,13 +175,6 @@ def counter_correct?(processed_source)
         end
       end
 
-      def generate_offense(klass, processed_source, tag, message = nil, replacement = nil)
-        message ||= klass::MESSAGE
-        klass_name = klass.name.demodulize
-        offense = ["#{klass_name}:#{message}", tag.node.loc.source].join("\n")
-        add_offense(processed_source.to_source_range(tag.loc), offense, replacement)
-      end
-
       def dump_data(processed_source)
         return if @total_offenses.zero?
 

lib/primer/view_components/linters/helpers/deprecated_components_helpers.rb

@@ -8,11 +8,11 @@ module Helpers
       # Helpers to share between DeprecatedComponents ERB lint and Rubocop cop
       module DeprecatedComponentsHelpers
         def message(component_name)
-          Primer::Deprecations.deprecation_message(component_name)
+          ::Primer::Deprecations.deprecation_message(component_name)
         end
 
         def deprecated_components
-          Primer::Deprecations.deprecated_components
+          ::Primer::Deprecations.deprecated_components
         end
       end
     end

lib/primer/view_components/linters/helpers/rule_helpers.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module ERBLint
+  module Linters
+    module Helpers
+      # Helpers for writing rules
+      module RuleHelpers
+        def tags(processed_source)
+          processed_source.parser.nodes_with_type(:tag).map { |tag_node| BetterHtml::Tree::Tag.from_node(tag_node) }
+        end
+
+        def generate_offense(klass, processed_source, tag, message = nil, replacement = nil)
+          message ||= klass::MESSAGE
+          klass_name = klass.name.demodulize
+          offense = ["#{klass_name}:#{message}", tag.node.loc.source].join("\n")
+          add_offense(processed_source.to_source_range(tag.loc), offense, replacement)
+        end
+
+        # Generate offense for ERB node tag
+        def generate_node_offense(klass, processed_source, node, message = nil)
+          message ||= klass::MESSAGE
+          offense = ["#{klass.name}:#{message}", node.loc.source].join("\n")
+          add_offense(processed_source.to_source_range(node.loc), offense)
+        end
+
+        def erb_nodes(processed_source)
+          processed_source.parser.ast.descendants(:erb)
+        end
+
+        def extract_ruby_from_erb_node(erb_node)
+          return unless erb_node.type == :erb
+
+          _, _, code_node = *erb_node
+          code_node.loc.source.strip
+        end
+      end
+    end
+  end
+end

lib/primer/view_components/linters/tooltipped_migration.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require_relative "helpers/rule_helpers"
+module ERBLint
+  module Linters
+    module Primer
+      module Accessibility
+        # Flag when `.tooltipped` is being used and offer alternatives.
+        class TooltippedMigration < Linter
+          include LinterRegistry
+          include Helpers::RuleHelpers
+
+          MIGRATE_TO_NEWER_TOOLTIP = ".tooltipped has been deprecated. Tooltip should never be set on a non-interactive element (e.g. <span>, <div>, <li>). " \
+            "Find alternatives in https://primer.style/design/guides/accessibility/tooltip-alternatives/. " \
+            "If you're setting a tooltip on an interactive element, use the latest tooltip in https://primer.style/view-components."
+          TOOLTIPPED_RUBY_PATTERN = /classes:.*tooltipped|class:.*tooltipped/.freeze
+
+          def run(processed_source)
+            # HTML tags
+            tags(processed_source).each do |tag|
+              next if tag.closing?
+
+              classes = tag.attributes["class"]&.value
+              generate_offense(self.class, processed_source, tag, MIGRATE_TO_NEWER_TOOLTIP) if classes&.include?("tooltipped")
+            end
+
+            # ERB nodes
+            erb_nodes(processed_source).each do |node|
+              code = extract_ruby_from_erb_node(node)
+              generate_node_offense(self.class, processed_source, node, MIGRATE_TO_NEWER_TOOLTIP) if code.match?(TOOLTIPPED_RUBY_PATTERN)
+            end
+          end
+        end
+      end
+    end
+  end
+end

test/lib/erblint/linter_config_works_test.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require "lib/erblint_test_case"
+
+class LinterConfigWorksTest < ErblintTestCase
+  # The ability to share rules and configs from other gems in erb_lint is not well-documented.
+  # This test validates that our recommended setup works.
+  def test_asserts_recommended_setup_works
+    erb_lint_config = ERBLint::RunnerConfig.new(file_loader.yaml(".erb-lint.yml"), file_loader)
+
+    rules_enabled_in_accessibility_config = 0
+    erb_lint_config.to_hash["linters"].each do |linter|
+      rules_enabled_in_accessibility_config += 1 if linter[0].include?("Primer::Accessibility") && linter[1]["enabled"] == true
+    end
+    known_linter_names ||= ERBLint::LinterRegistry.linters.map(&:simple_name)
+    known_linter_names_count = known_linter_names.count { |linter| linter.include?("Primer::Accessibility") }
+    assert_equal 1, rules_enabled_in_accessibility_config
+    assert_equal 1, known_linter_names_count
+  end
+end

test/lib/erblint/tooltipped_migration_test.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require "lib/erblint_test_case"
+
+class TooltippedMigrationTest < ErblintTestCase
+  def linter_class
+    ERBLint::Linters::Primer::Accessibility::TooltippedMigration
+  end
+
+  def test_warns_if_tooltipped_class_is_used_on_html_tag
+    @file = "<li class='tooltipped'>List item</li>"
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.tooltipped has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_warns_if_tooltipped_class_is_used_on_view_component
+    @file = <<~HTML
+      <%= content_tag :span,
+      class: "tooltipped tooltipped-nw cursor-pointer" do %>
+    HTML
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.tooltipped has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_warns_if_tooltipped_class_is_used_on_ruby
+    @file = '<%= render SomeComponent.new(classes: "tooltipped tooltipped-multiline tooltipped-s") do %>'
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.tooltipped has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_does_not_warn_if_tooltipped_is_not_used
+    @file = "<%= render SomeComponent.new do %>"
+    @linter.run(processed_source)
+    assert_equal 0, @linter.offenses.count
+  end
+
+  def test_does_not_warn_if_inline_disable_comment
+    @file = <<~HTML
+      <%= render SomeComponent.new(classes: "tooltipped") do %><%# erblint:disable Primer::Accessibility::TooltippedMigration %>
+    HTML
+    @linter.run_and_update_offense_status(processed_source)
+    offenses = @linter.offenses.reject(&:disabled?) # Reject offenses marked as disabled. (This happens at the runner level so we repliace it here)
+    assert_equal 0, offenses.count
+  end
+end