Merge branch 'main' into custom-fallback-counter

primer · Jul 22, 2023 · 0e42489 · 0e42489
2 parents 54adf67 + cbd5c84
commit 0e42489
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 7 deletions.
diff --git a/.changeset/dry-dryers-thank.md b/.changeset/dry-dryers-thank.md
@@ -0,0 +1,7 @@
+---
+'@primer/view-components': minor
+---
+
+Add a linter discouraging use of <details-menu> in favor of Primer::Alpha::ActionMenu
+
+<!-- Changed components: _none_ -->
diff --git a/lib/primer/view_components/linters/accessibility.yml b/lib/primer/view_components/linters/accessibility.yml
@@ -5,3 +5,5 @@ linters:
     enabled: true
   Primer::Accessibility::TooltippedMigration:
     enabled: true
+  Primer::Accessibility::DetailsMenuMigration:
+    enabled: true
diff --git a/lib/primer/view_components/linters/argument_mappers/label.rb b/lib/primer/view_components/linters/argument_mappers/label.rb
@@ -7,24 +7,24 @@ module Linters
     module ArgumentMappers
       # Maps classes in a label element to arguments for the Label component.
       class Label < Base
-        SCHEME_MAPPINGS = Primer::ViewComponents::Constants.get(
+        SCHEME_MAPPINGS = ::Primer::ViewComponents::Constants.get(
           component: "Primer::Beta::Label",
           constant: "SCHEME_MAPPINGS",
           symbolize: true
         ).freeze
 
-        SIZE_MAPPINGS = Primer::ViewComponents::Constants.get(
+        SIZE_MAPPINGS = ::Primer::ViewComponents::Constants.get(
           component: "Primer::Beta::Label",
           constant: "SIZE_MAPPINGS",
           symbolize: true
         ).freeze
 
-        DEFAULT_TAG = Primer::ViewComponents::Constants.get(
+        DEFAULT_TAG = ::Primer::ViewComponents::Constants.get(
           component: "Primer::Beta::Label",
           constant: "DEFAULT_TAG"
         ).freeze
 
-        INLINE_CLASS = Primer::ViewComponents::Constants.get(
+        INLINE_CLASS = ::Primer::ViewComponents::Constants.get(
           component: "Primer::Beta::Label",
           constant: "INLINE_CLASS"
         ).freeze

diff --git a/lib/primer/view_components/linters/details_menu_migration.rb b/lib/primer/view_components/linters/details_menu_migration.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require_relative "helpers/rule_helpers"
+module ERBLint
+  module Linters
+    module Primer
+      module Accessibility
+        # Flag when `<details-menu>` is being used and offer alternatives.
+        class DetailsMenuMigration < Linter
+          include LinterRegistry
+          include Helpers::RuleHelpers
+
+          MIGRATE_FROM_DETAILS_MENU = "<details-menu> has been deprecated. Please instead use Primer::Alpha::ActionMenu" \
+            "https://primer.style/design/components/action-menu/rails/alpha"
+          DETAILS_MENU_RUBY_PATTERN = /tag:?\s+:"details-menu"/.freeze
+
+          def run(processed_source)
+            # HTML tags
+            tags(processed_source).each do |tag|
+              next if tag.closing?
+
+              generate_offense(self.class, processed_source, tag, MIGRATE_FROM_DETAILS_MENU) if tag.name == "details-menu"
+            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_FROM_DETAILS_MENU) if code.match?(DETAILS_MENU_RUBY_PATTERN)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/primer/view_components/linters/label_component_migration_counter.rb b/lib/primer/view_components/linters/label_component_migration_counter.rb
@@ -10,7 +10,7 @@ module Linters
     class LabelComponentMigrationCounter < BaseLinter
       include Autocorrectable
 
-      TAGS = Primer::ViewComponents::Constants.get(
+      TAGS = ::Primer::ViewComponents::Constants.get(
         component: "Primer::Beta::Label",
         constant: "TAG_OPTIONS"
       ).freeze

diff --git a/test/lib/erblint/details_menu_migration_test.rb b/test/lib/erblint/details_menu_migration_test.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require "lib/erblint_test_case"
+
+class DetailsMenuMigrationTest < ErblintTestCase
+  def linter_class
+    ERBLint::Linters::Primer::Accessibility::DetailsMenuMigration
+  end
+
+  def test_warns_if_details_menu_tag_is_used
+    @file = "<details-menu class='SelectMenu' role='menu'></details-menu>"
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.<details-menu> has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_warns_if_details_menu_content_tag_is_rendered
+    @file = <<~HTML
+      <%= content_tag :"details-menu",
+      class: "SelectMenu" do %>
+    HTML
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.<details-menu> has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_warns_if_details_menu_view_component_is_rendered
+    @file = '<%= render SomeComponent.new(tag: :"details-menu") do %>'
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.<details-menu> has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_warns_if_details_menu_view_component_slot_is_rendered
+    @file = '<% component.with_body(tag: :"details-menu") do %>'
+    @linter.run(processed_source)
+    assert_equal 1, @linter.offenses.count
+    assert_match(/.<details-menu> has been deprecated./, @linter.offenses.first.message)
+  end
+
+  def test_does_not_warn_if_inline_disable_comment
+    @file = <<~HTML
+      <%= render SomeComponent.new(tag: :"details-menu") do %><%# erblint:disable Primer::Accessibility::DetailsMenuMigration %>
+    HTML
+    @linter.run_and_update_offense_status(processed_source)
+    offenses = @linter.offenses.reject(&:disabled?)
+    assert_equal 0, offenses.count
+  end
+end
diff --git a/test/lib/erblint/linter_config_works_test.rb b/test/lib/erblint/linter_config_works_test.rb
@@ -14,7 +14,7 @@ def test_asserts_recommended_setup_works
     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
+    assert_equal 2, rules_enabled_in_accessibility_config
+    assert_equal 2, known_linter_names_count
   end
 end