From 63977720e421d06942320c79345536a52ff4f9e2 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Mon, 27 Jun 2022 13:20:11 -0700 Subject: [PATCH] Add an ActionList linter (#1198) --- .changeset/thick-suns-perform.md | 5 ++ docs/content/system-arguments.md | 1 - .../linters/disallow_action_list.rb | 73 +++++++++++++++++++ test/linter_test_case.rb | 3 +- test/linters/disallow_action_list_test.rb | 56 ++++++++++++++ 5 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 .changeset/thick-suns-perform.md create mode 100644 lib/primer/view_components/linters/disallow_action_list.rb create mode 100644 test/linters/disallow_action_list_test.rb diff --git a/.changeset/thick-suns-perform.md b/.changeset/thick-suns-perform.md new file mode 100644 index 0000000000..a8feb42163 --- /dev/null +++ b/.changeset/thick-suns-perform.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Add ActionList linter diff --git a/docs/content/system-arguments.md b/docs/content/system-arguments.md index 5b9bcb333a..b892ed736e 100644 --- a/docs/content/system-arguments.md +++ b/docs/content/system-arguments.md @@ -39,7 +39,6 @@ System arguments include most HTML attributes. For example: | `style` | `String` | Inline styles. | | `title` | `String` | The `title` attribute. | | `width` | `Integer` | Width. | -| `role` | `String` | Roles from the ARIA spec. [MDN docs](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles). | ## Animation diff --git a/lib/primer/view_components/linters/disallow_action_list.rb b/lib/primer/view_components/linters/disallow_action_list.rb new file mode 100644 index 0000000000..9e6a97980c --- /dev/null +++ b/lib/primer/view_components/linters/disallow_action_list.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require "pathname" +require_relative "helpers/rubocop_helpers" + +module ERBLint + module Linters + # Finds usages of ActionList CSS classes. + class DisallowActionList < Linter + include ERBLint::LinterRegistry + include TagTreeHelpers + + class ConfigSchema < LinterConfig + property :ignore_files, accepts: array_of?(String), default: -> { [] } + end + self.config_schema = ConfigSchema + + def run(processed_source) + return if ignored?(processed_source.filename) + + class_regex = /ActionList[\w-]*/ + tags, * = build_tag_tree(processed_source) + + tags.each do |tag| + next if tag.closing? + + classes = + if (class_attrib = tag.attributes["class"]) + loc = class_attrib.value_node.loc + loc.source_buffer.source[loc.begin_pos...loc.end_pos] + else + "" + end + + indices = [].tap do |results| + classes.scan(class_regex) do + results << Regexp.last_match.offset(0) + end + end + + next if indices.empty? + + indices.each do |(start_idx, end_idx)| + new_loc = class_attrib.value_node.loc.with( + begin_pos: class_attrib.value_node.loc.begin_pos + start_idx, + end_pos: class_attrib.value_node.loc.begin_pos + end_idx + ) + + add_offense( + new_loc, + "ActionList classes are only designed to be used by Primer View Components and " \ + "should be considered private. Please reach out in the #primer-rails Slack channel." + ) + end + end + end + + private + + def ignored?(filename) + filename = Pathname(filename) + + begin + filename = filename.relative_path_from(Pathname(Dir.getwd)) + rescue ArgumentError + # raised if the filename does not have Dir.getwd as a prefix + end + + @config.ignore_files.any? { |pattern| filename.fnmatch?(pattern) } + end + end + end +end diff --git a/test/linter_test_case.rb b/test/linter_test_case.rb index 63c7d1b84b..b8075d5b67 100644 --- a/test/linter_test_case.rb +++ b/test/linter_test_case.rb @@ -7,6 +7,7 @@ class LinterTestCase < Minitest::Test def setup @linter = linter_class&.new(file_loader, linter_class.config_schema.new) + @filename = "file.rb" end private @@ -38,7 +39,7 @@ def file_loader end def processed_source - ERBLint::ProcessedSource.new("file.rb", @file) + ERBLint::ProcessedSource.new(@filename, @file) end def tags diff --git a/test/linters/disallow_action_list_test.rb b/test/linters/disallow_action_list_test.rb new file mode 100644 index 0000000000..1e74b1ca4b --- /dev/null +++ b/test/linters/disallow_action_list_test.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require "linter_test_case" + +class DisallowActionListTest < LinterTestCase + def test_identifies_action_list_class + @file = "
fooo
" + @linter.run(processed_source) + + assert @linter.offenses.size == 1 + + offense = @linter.offenses.first + assert offense.source_range.begin_pos == 12 + assert offense.source_range.end_pos == 22 + end + + def test_identifies_two_action_list_classes + @file = "
fooo
" + @linter.run(processed_source) + + assert @linter.offenses.size == 2 + + offense = @linter.offenses[0] + assert offense.source_range.begin_pos == 12 + assert offense.source_range.end_pos == 22 + + offense = @linter.offenses[1] + assert offense.source_range.begin_pos == 23 + assert offense.source_range.end_pos == 38 + end + + def test_identifies_action_list_class_nested + @file = "
fooo
" + @linter.run(processed_source) + + assert @linter.offenses.size == 1 + + offense = @linter.offenses.first + assert offense.source_range.begin_pos == 17 + assert offense.source_range.end_pos == 27 + end + + def test_does_not_identify_action_list_class_in_ignored_files + @file = "
fooo
" + @filename = "app/components/primer/component_template.html.erb" + config = linter_class.config_schema.new(ignore_files: ["app/components/primer/*.html.erb"]) + @linter = linter_class.new(file_loader, config) + @linter.run(processed_source) + + assert_empty @linter.offenses + end + + def linter_class + ERBLint::Linters::DisallowActionList + end +end