From cbd5c84d8d290b0d018f15c5ee84cd94afcd4b0b Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Fri, 21 Jul 2023 18:16:45 +0100 Subject: [PATCH] add details_menu_migration linter (#2148) --- .changeset/dry-dryers-thank.md | 7 +++ .../view_components/linters/accessibility.yml | 2 + .../linters/argument_mappers/label.rb | 8 +-- .../linters/details_menu_migration.rb | 35 +++++++++++++ .../label_component_migration_counter.rb | 2 +- .../erblint/details_menu_migration_test.rb | 49 +++++++++++++++++++ test/lib/erblint/linter_config_works_test.rb | 4 +- 7 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 .changeset/dry-dryers-thank.md create mode 100644 lib/primer/view_components/linters/details_menu_migration.rb create mode 100644 test/lib/erblint/details_menu_migration_test.rb diff --git a/.changeset/dry-dryers-thank.md b/.changeset/dry-dryers-thank.md new file mode 100644 index 0000000000..6f456bf70b --- /dev/null +++ b/.changeset/dry-dryers-thank.md @@ -0,0 +1,7 @@ +--- +'@primer/view-components': minor +--- + +Add a linter discouraging use of in favor of Primer::Alpha::ActionMenu + + diff --git a/lib/primer/view_components/linters/accessibility.yml b/lib/primer/view_components/linters/accessibility.yml index bf0b7d9ee7..c3614bda39 100644 --- 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 index a97c2d5cc6..97864d4e7f 100644 --- 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 new file mode 100644 index 0000000000..cc65313df1 --- /dev/null +++ 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 `` is being used and offer alternatives. + class DetailsMenuMigration < Linter + include LinterRegistry + include Helpers::RuleHelpers + + MIGRATE_FROM_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 index 50662d2c0c..579e2eb4d1 100644 --- 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 new file mode 100644 index 0000000000..e4cc7ac32d --- /dev/null +++ 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 = "" + @linter.run(processed_source) + assert_equal 1, @linter.offenses.count + assert_match(/. 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(/. 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(/. 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(/. 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 index 4d8f8fb8a1..07d0deec17 100644 --- 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