From 688a4a263deb4be70763b9ba39cae854896b44ce Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 5 Dec 2024 10:37:13 -0800 Subject: [PATCH] Remove SystemArgumentInsteadOfClass linter and fix bug with whitespace in rendered class (#3220) --- .changeset/wet-houses-own.md | 5 ++ .rubocop.yml | 3 - docs/contributors/linting.md | 2 +- lib/primer/classify.rb | 2 +- lib/rubocop/config/default.yml | 3 - .../system_argument_instead_of_class.rb | 57 ------------------ test/components/base_component_test.rb | 6 ++ .../system_arguments_instead_of_class_test.rb | 60 ------------------- 8 files changed, 13 insertions(+), 125 deletions(-) create mode 100644 .changeset/wet-houses-own.md delete mode 100644 lib/rubocop/cop/primer/system_argument_instead_of_class.rb delete mode 100644 test/lib/rubocop/system_arguments_instead_of_class_test.rb diff --git a/.changeset/wet-houses-own.md b/.changeset/wet-houses-own.md new file mode 100644 index 0000000000..512f5291f5 --- /dev/null +++ b/.changeset/wet-houses-own.md @@ -0,0 +1,5 @@ +--- +"@primer/view-components": patch +--- + +Remove SystemArgumentInsteadOfClass linter and fix bug with whitespace in rendered class diff --git a/.rubocop.yml b/.rubocop.yml index c3371ed86d..4900ebfdb8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -57,9 +57,6 @@ Primer/NoTagMemoize: Include: - "app/components/**/*" -Primer/SystemArgumentInsteadOfClass: - Enabled: true - Lint/MissingSuper: Enabled: false diff --git a/docs/contributors/linting.md b/docs/contributors/linting.md index 7e3742a6c7..7cd3de47a1 100644 --- a/docs/contributors/linting.md +++ b/docs/contributors/linting.md @@ -18,7 +18,7 @@ inherit_gem: You can also modify that configuration enabling/disabling the cops you want: ```yml -Primer/SystemArgumentInsteadOfClass: +Primer/NoTagMemoize: Enabled: false ``` diff --git a/lib/primer/classify.rb b/lib/primer/classify.rb index 7c00476f19..acc8e1a920 100644 --- a/lib/primer/classify.rb +++ b/lib/primer/classify.rb @@ -49,7 +49,7 @@ def call(args = {}) case key when :classes # insert :classes first to avoid huge doc diffs - result.unshift(val) + result.unshift(val) unless val.blank? next when :style style = val diff --git a/lib/rubocop/config/default.yml b/lib/rubocop/config/default.yml index 65fee4cdbd..e51130db04 100644 --- a/lib/rubocop/config/default.yml +++ b/lib/rubocop/config/default.yml @@ -1,9 +1,6 @@ require: - rubocop/cop/primer -Primer/SystemArgumentInsteadOfClass: - Enabled: true - Primer/NoTagMemoize: Enabled: false diff --git a/lib/rubocop/cop/primer/system_argument_instead_of_class.rb b/lib/rubocop/cop/primer/system_argument_instead_of_class.rb deleted file mode 100644 index c2ebdc937d..0000000000 --- a/lib/rubocop/cop/primer/system_argument_instead_of_class.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -require "rubocop" -require "primer/classify/utilities" - -# :nocov: -module RuboCop - module Cop - module Primer - # This cop ensures that components use System Arguments instead of CSS classes. - # - # bad - # Component.new(classes: "mr-1") - # - # good - # Component.new(mr: 1) - class SystemArgumentInsteadOfClass < BaseCop - INVALID_MESSAGE = <<~STR - Avoid using CSS classes when you can use System Arguments: https://primer.style/view-components/system-arguments. - STR - - def on_send(node) - return unless valid_node?(node) - return unless node.arguments? - - # we are looking for hash arguments and they are always last - kwargs = node.arguments.last - - return unless kwargs.type == :hash - - # find classes pair - classes_arg = kwargs.pairs.find { |kwarg| kwarg.key.value == :classes } - - return if classes_arg.nil? - return unless classes_arg.value.type == :str - - # get actual classes - classes = classes_arg.value.value - - system_arguments = ::Primer::Classify::Utilities.classes_to_hash(classes) - - # no classes are fixable - return if system_arguments[:classes] == classes - - add_offense(classes_arg, message: INVALID_MESSAGE) - end - - def autocorrect(node) - lambda do |corrector| - args = ::Primer::Classify::Utilities.classes_to_args(node.value.value) - corrector.replace(node.loc.expression, args) - end - end - end - end - end -end diff --git a/test/components/base_component_test.rb b/test/components/base_component_test.rb index b87efd44d2..b581cdd8fa 100644 --- a/test/components/base_component_test.rb +++ b/test/components/base_component_test.rb @@ -73,6 +73,12 @@ def test_does_not_render_class_attribute_if_none_is_set refute_selector("div[class='']") end + def test_renders_system_argument_class_with_no_whitespace + render_inline(Primer::BaseComponent.new(tag: :div, ml: 3)) + + assert_selector("div[class='ml-3']") + end + def test_does_not_render_primer_layout_classes_as_attributes render_inline(Primer::BaseComponent.new(tag: :div, my: 4)) diff --git a/test/lib/rubocop/system_arguments_instead_of_class_test.rb b/test/lib/rubocop/system_arguments_instead_of_class_test.rb deleted file mode 100644 index b0a56be2ee..0000000000 --- a/test/lib/rubocop/system_arguments_instead_of_class_test.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -require "lib/cop_test_case" - -class RubocopSystemArgumentInsteadOfClassTest < CopTestCase - def cop_class - RuboCop::Cop::Primer::SystemArgumentInsteadOfClass - end - - def test_non_primer_component - investigate(cop, <<-RUBY) - Component.new(classes: "mr-1") - RUBY - - assert_empty cop.offenses - end - - def test_primer_component - investigate(cop, <<-RUBY) - Primer::BaseComponent.new(classes: "mr-1") - RUBY - - assert_equal 1, cop.offenses.count - assert_equal "Primer/SystemArgumentInsteadOfClass: Avoid using CSS classes when you can use System Arguments: https://primer.style/view-components/system-arguments.\n", cop.offenses.first.message - end - - def test_non_primer_view_helper - investigate(cop, <<-RUBY) - octicon(classes: "mr-1") - RUBY - - assert_empty cop.offenses - end - - def test_primer_view_helper - investigate(cop, <<-RUBY) - primer_octicon(classes: "mr-1") - RUBY - - assert_equal 1, cop.offenses.count - assert_equal "Primer/SystemArgumentInsteadOfClass: Avoid using CSS classes when you can use System Arguments: https://primer.style/view-components/system-arguments.\n", cop.offenses.first.message - end - - def test_custom_class - investigate(cop, <<-RUBY) - Primer::BaseComponent.new(classes: "mr-1 custom") - RUBY - - assert_equal 1, cop.offenses.count - assert_equal "Primer/SystemArgumentInsteadOfClass: Avoid using CSS classes when you can use System Arguments: https://primer.style/view-components/system-arguments.\n", cop.offenses.first.message - end - - def test_no_classes - investigate(cop, <<-RUBY) - Primer::BaseComponent.new - RUBY - - assert_empty cop.offenses - end -end