From f1ed752f191415800071ac416047d439e25c47a9 Mon Sep 17 00:00:00 2001 From: Neal Lindsay Date: Fri, 3 Feb 2023 15:18:29 -0500 Subject: [PATCH 1/5] Linter to disallow use of private HTML classes In order to preserve our ability to add, remove, and change styles for the HTML classes we create for Primer ViewComponents, this linter will complain if it sees those classes being used in ERB templates. --- .../disallow_view_component_html_classes.rb | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 lib/primer/view_components/linters/disallow_view_component_html_classes.rb diff --git a/lib/primer/view_components/linters/disallow_view_component_html_classes.rb b/lib/primer/view_components/linters/disallow_view_component_html_classes.rb new file mode 100644 index 0000000000..4ba6e67315 --- /dev/null +++ b/lib/primer/view_components/linters/disallow_view_component_html_classes.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'better_html' + +require_relative "helpers/rubocop_helpers" + +module ERBLint + module Linters + # Finds usages of ViewComponent-reserved HTML classes + class DisallowViewComponentHtmlClasses < Linter + include LinterRegistry + + RESERVED_HTML_CLASSES = %w( + ActionListItem-visual + ).freeze + + def run(processed_source) + processed_source + .parser + .nodes_with_type(:tag) + .each do |node| + tag = BetterHtml::Tree::Tag.from_node(node) + + tag.attributes["class"]&.value&.split(/\s+/)&.each do |class_name| + if RESERVED_HTML_CLASSES.include? class_name + add_offense( + processed_source.to_source_range(tag.loc), + "HTML class \"#{class_name}\" is reserved for Primer ViewComponents. It might disappear or have different styles in the future." + ) + end + end + end + end + end + end +end From f3d717ce054ae72587c244e1899fe1f8a6d2c40d Mon Sep 17 00:00:00 2001 From: Neal Lindsay Date: Wed, 8 Feb 2023 15:43:01 -0500 Subject: [PATCH 2/5] Compile list of classes to restrict with linter The `DisallowViewComponentHtmlClasses` linter will scan ERB files for use of classes that we want to reserve for internal use by Primer ViewComponents. This commit augments the `export-css-selectors` script to also generate a list of such classes that we want to restrict. Those classes are written out to `static/classes.json` and read into a constant when the linter is loaded. The script will also add classes to the individual `*.css.json` files that live alongside our CSS files for test purposes. Most of these json files are not committed. --- .../disallow_view_component_html_classes.rb | 6 +- package-lock.json | 8 +- package.json | 3 +- script/export-css-selectors | 100 ++++-- static/classes.json | 314 ++++++++++++++++++ 5 files changed, 396 insertions(+), 35 deletions(-) create mode 100644 static/classes.json diff --git a/lib/primer/view_components/linters/disallow_view_component_html_classes.rb b/lib/primer/view_components/linters/disallow_view_component_html_classes.rb index 4ba6e67315..68bd9319c8 100644 --- a/lib/primer/view_components/linters/disallow_view_component_html_classes.rb +++ b/lib/primer/view_components/linters/disallow_view_component_html_classes.rb @@ -10,8 +10,10 @@ module Linters class DisallowViewComponentHtmlClasses < Linter include LinterRegistry - RESERVED_HTML_CLASSES = %w( - ActionListItem-visual + RESERVED_HTML_CLASSES = JSON.parse( + File.read( + File.join(__dir__, "..", "..", "..", "..", "static", "classes.json") + ) ).freeze def run(processed_source) diff --git a/package-lock.json b/package-lock.json index ed9227825c..0a9da52d6f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,7 +18,8 @@ "@github/mini-throttle": "^2.1.0", "@github/relative-time-element": "^4.0.0", "@github/tab-container-element": "^3.1.2", - "@primer/behaviors": "^1.2.0" + "@primer/behaviors": "^1.2.0", + "css-what": "^6.1.0" }, "devDependencies": { "@changesets/changelog-github": "^0.4.6", @@ -2469,8 +2470,6 @@ "version": "6.1.0", "resolved": "https://registry.npmjs.org/css-what/-/css-what-6.1.0.tgz", "integrity": "sha512-HTUrgRJ7r4dsZKU6GjmpfRK1O76h97Z8MfS1G0FozR+oF2kG6Vfe8JE6zwrkbxigziPHinCJ+gCPjA9EaBDtRw==", - "dev": true, - "license": "BSD-2-Clause", "engines": { "node": ">= 6" }, @@ -11017,8 +11016,7 @@ "css-what": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/css-what/-/css-what-6.1.0.tgz", - "integrity": "sha512-HTUrgRJ7r4dsZKU6GjmpfRK1O76h97Z8MfS1G0FozR+oF2kG6Vfe8JE6zwrkbxigziPHinCJ+gCPjA9EaBDtRw==", - "dev": true + "integrity": "sha512-HTUrgRJ7r4dsZKU6GjmpfRK1O76h97Z8MfS1G0FozR+oF2kG6Vfe8JE6zwrkbxigziPHinCJ+gCPjA9EaBDtRw==" }, "cssdb": { "version": "7.0.0", diff --git a/package.json b/package.json index 6c2bb245ad..bdef4d6cb1 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,8 @@ "@github/mini-throttle": "^2.1.0", "@github/relative-time-element": "^4.0.0", "@github/tab-container-element": "^3.1.2", - "@primer/behaviors": "^1.2.0" + "@primer/behaviors": "^1.2.0", + "css-what": "^6.1.0" }, "devDependencies": { "@changesets/changelog-github": "^0.4.6", diff --git a/script/export-css-selectors b/script/export-css-selectors index 5a3f4702c7..142efedc86 100755 --- a/script/export-css-selectors +++ b/script/export-css-selectors @@ -1,37 +1,83 @@ #!/usr/bin/env node -const postcss = require('postcss'); -const glob = require('glob'); -const fs = require('fs'); +const postcss = require('postcss') +const { readFile, writeFile } = require('node:fs/promises') +const { promisify } = require('util') +const glob = promisify(require('glob')) +const CSSwhat = require('css-what') -console.log('Exporting CSS selectors...'); +console.log('Exporting CSS selectors...') -const export_selectors = (folder) => { +const exportSelectors = (folder) => { const folder_glob = folder + '/**/*.css' + const componentNameRegex = new RegExp(`${folder.replace('/','\\/')}\\/(.*).css`) - glob(folder_glob, (_err, files) => { - files.forEach(file => { - console.log(`Processing ${file}`) - const css = fs.readFileSync(file, 'utf8') - const root = postcss.parse(css) - const componentNameRegex = new RegExp(`${folder.replace('/','\\/')}\\/(.*).css`) - const componentName = componentNameRegex.exec(file)[1] - const selectors = [] - - root.walkRules(rule => { - if (rule.parent.type === 'atrule' && rule.parent.name === 'keyframes') { - return - } - - rule.selectors.forEach(ruleSelector => { - selectors.push(ruleSelector) + return glob(folder_glob).then(files => + Promise.all( + files.map(async (file) => { + console.log(`Processing ${file}`) + const css = await readFile(file, 'utf8') + const root = postcss.parse(css) + const componentName = componentNameRegex.exec(file)[1] + + const selectors = [] + const classes = [] + + root.walkRules(rule => { + // @keyframes at-rules have children that look like they have normal + // CSS selectors, but they're each just "from", "to", or a percentage. + // Either way, we don't have to worry about them as selectors and they + // don't include any classes. + if (rule.parent?.type === 'atrule' && rule.parent?.name === 'keyframes') { + return + } + + rule.selectors.forEach(ruleSelector => { + selectors.push(ruleSelector) + CSSwhat.parse(ruleSelector)[0].forEach(ruleObj => { + if (ruleObj.type === 'attribute' && ruleObj.name === 'class') { + classes.push(ruleObj.value) + } + }) + }) }) - }) - fs.writeFileSync(`${file}.json`, JSON.stringify({ 'name': componentName, 'selectors': [...new Set(selectors)] }, null, 2)) - }) - }) + console.log(`Writing ${file}.json`) + writeFile( + `${file}.json`, + JSON.stringify({ + name: componentName, + selectors: [...new Set(selectors)], + classes: [...new Set(classes)] + }, null, 2) + ).catch(error => console.error(`Failed to write ${file}.json`, { error })) + + return classes + }) + ) + ) } -export_selectors('app/components/primer') -export_selectors('app/lib/primer/css') +// stylesheets under app/lib/primer/css need their individual +// json files generated +exportSelectors('app/lib/primer/css') + +// class names referenced under app/components/primer might need +// to be reserved in addition to getting individual json files +const classShouldBeReserved = className => + (className[0].toUpperCase() === className[0]) +exportSelectors('app/components/primer') + .then(classLists => { + console.log(`Writing static/classes.json`) + return writeFile( + 'static/classes.json', + JSON.stringify( + [...new Set(classLists.reduce((a, b) => a.concat(b)))] + .filter(classShouldBeReserved) + .sort(), + null, + 2 + ) + ) + }) + .catch(error => console.error("failed to write static/classes.json", { error })) diff --git a/static/classes.json b/static/classes.json new file mode 100644 index 0000000000..88684045c8 --- /dev/null +++ b/static/classes.json @@ -0,0 +1,314 @@ +[ + "ActionList--subGroup", + "ActionList-sectionDivider", + "ActionList-sectionDivider--filled", + "ActionList-sectionDivider-title", + "ActionListContent", + "ActionListContent--blockDescription", + "ActionListContent--hasActiveSubItem", + "ActionListContent--sizeLarge", + "ActionListContent--sizeXLarge", + "ActionListContent--visual16", + "ActionListContent--visual20", + "ActionListContent--visual24", + "ActionListItem", + "ActionListItem--danger", + "ActionListItem--hasSubItem", + "ActionListItem--navActive", + "ActionListItem--subItem", + "ActionListItem--trailingActionHover", + "ActionListItem--withActions", + "ActionListItem-action", + "ActionListItem-action--leading", + "ActionListItem-action--trailing", + "ActionListItem-collapseIcon", + "ActionListItem-description", + "ActionListItem-descriptionWrap", + "ActionListItem-descriptionWrap--inline", + "ActionListItem-label", + "ActionListItem-label--truncate", + "ActionListItem-multiSelectCheckmark", + "ActionListItem-multiSelectIcon", + "ActionListItem-multiSelectIconRect", + "ActionListItem-singleSelectCheckmark", + "ActionListItem-trailingAction", + "ActionListItem-visual", + "ActionListItem-visual--leading", + "ActionListItem-visual--trailing", + "ActionListWrap", + "ActionListWrap--divided", + "ActionListWrap--inset", + "AvatarStack", + "AvatarStack--right", + "AvatarStack--three-plus", + "AvatarStack--two", + "AvatarStack-body", + "Banner", + "Banner--error", + "Banner--full", + "Banner--full-whenNarrow", + "Banner--success", + "Banner--warning", + "Banner-actions", + "Banner-close", + "Banner-message", + "Banner-title", + "Banner-visual", + "Box", + "Box--blue", + "Box--condensed", + "Box--danger", + "Box--scrollable", + "Box--spacious", + "Box-body", + "Box-btn-octicon", + "Box-footer", + "Box-header", + "Box-header--blue", + "Box-row", + "Box-row--blue", + "Box-row--drag-button", + "Box-row--drag-hide", + "Box-row--focus-blue", + "Box-row--focus-gray", + "Box-row--gray", + "Box-row--hover-blue", + "Box-row--hover-gray", + "Box-row--unread", + "Box-row--yellow", + "Box-row-link", + "Box-title", + "BtnGroup", + "BtnGroup-item", + "BtnGroup-parent", + "Button", + "Button--danger", + "Button--fullWidth", + "Button--iconOnly", + "Button--invisible", + "Button--invisible-noVisuals", + "Button--large", + "Button--link", + "Button--medium", + "Button--primary", + "Button--secondary", + "Button--small", + "Button-content", + "Button-content--alignStart", + "Button-label", + "Button-leadingVisual", + "Button-trailingAction", + "Button-trailingVisual", + "Button-visual", + "Button-withTooltip", + "Counter", + "Counter--primary", + "Counter--secondary", + "FormControl", + "FormControl--fullWidth", + "FormControl-caption", + "FormControl-check-group-wrap", + "FormControl-checkbox", + "FormControl-checkbox-labelWrap", + "FormControl-checkbox-wrap", + "FormControl-error", + "FormControl-horizontalGroup", + "FormControl-inlineValidation", + "FormControl-input", + "FormControl-input-leadingVisual", + "FormControl-input-leadingVisualWrap", + "FormControl-input-trailingAction", + "FormControl-input-trailingAction--divider", + "FormControl-input-wrap", + "FormControl-input-wrap--leadingVisual", + "FormControl-input-wrap--trailingAction", + "FormControl-input-wrap-trailingAction--divider", + "FormControl-inset", + "FormControl-label", + "FormControl-large", + "FormControl-medium", + "FormControl-monospace", + "FormControl-radio", + "FormControl-radio-group-wrap", + "FormControl-radio-labelWrap", + "FormControl-radio-wrap", + "FormControl-select", + "FormControl-select-wrap", + "FormControl-small", + "FormControl-spacingWrapper", + "FormControl-success", + "FormControl-textarea", + "FormControl-toggleSwitchInput", + "FormControl-warning", + "Label", + "Label--accent", + "Label--attention", + "Label--closed", + "Label--danger", + "Label--done", + "Label--info", + "Label--inline", + "Label--large", + "Label--open", + "Label--primary", + "Label--secondary", + "Label--severe", + "Label--sponsors", + "Label--success", + "Label--warning", + "Layout", + "Layout--divided", + "Layout--flowRow-until-lg", + "Layout--flowRow-until-md", + "Layout--gutter-condensed", + "Layout--gutter-none", + "Layout--gutter-spacious", + "Layout--sidebar-narrow", + "Layout--sidebar-wide", + "Layout--sidebarPosition-end", + "Layout--sidebarPosition-flowRow-end", + "Layout--sidebarPosition-flowRow-none", + "Layout--sidebarPosition-flowRow-start", + "Layout--sidebarPosition-start", + "Layout-divider", + "Layout-divider--flowRow-hidden", + "Layout-divider--flowRow-shallow", + "Layout-main", + "Layout-main-centered-lg", + "Layout-main-centered-md", + "Layout-main-centered-xl", + "Layout-sidebar", + "Link", + "Link--muted", + "Link--onHover", + "Link--primary", + "Link--secondary", + "Overlay", + "Overlay--height-auto", + "Overlay--height-large", + "Overlay--height-medium", + "Overlay--height-small", + "Overlay--height-xlarge", + "Overlay--height-xsmall", + "Overlay--hidden", + "Overlay--motion-scaleFade", + "Overlay--size-auto", + "Overlay--size-full", + "Overlay--size-large", + "Overlay--size-medium", + "Overlay--size-medium-portrait", + "Overlay--size-small", + "Overlay--size-small-portrait", + "Overlay--size-xlarge", + "Overlay--size-xsmall", + "Overlay--visibilityHidden", + "Overlay--width-auto", + "Overlay--width-large", + "Overlay--width-medium", + "Overlay--width-small", + "Overlay--width-xlarge", + "Overlay--width-xxlarge", + "Overlay-actionWrap", + "Overlay-backdrop--anchor", + "Overlay-backdrop--anchor-whenNarrow", + "Overlay-backdrop--center", + "Overlay-backdrop--center-whenNarrow", + "Overlay-backdrop--full", + "Overlay-backdrop--full-whenNarrow", + "Overlay-backdrop--placement-bottom", + "Overlay-backdrop--placement-bottom-whenNarrow", + "Overlay-backdrop--placement-left", + "Overlay-backdrop--placement-left-whenNarrow", + "Overlay-backdrop--placement-right", + "Overlay-backdrop--placement-right-whenNarrow", + "Overlay-backdrop--placement-top", + "Overlay-backdrop--placement-top-whenNarrow", + "Overlay-backdrop--side", + "Overlay-backdrop--side-whenNarrow", + "Overlay-body", + "Overlay-body--paddingCondensed", + "Overlay-body--paddingNone", + "Overlay-closeButton", + "Overlay-description", + "Overlay-footer", + "Overlay-footer--alignCenter", + "Overlay-footer--alignEnd", + "Overlay-footer--alignStart", + "Overlay-footer--divided", + "Overlay-form", + "Overlay-header", + "Overlay-header--divided", + "Overlay-header--large", + "Overlay-headerContentWrap", + "Overlay-title", + "Overlay-titleWrap", + "Overlay-whenNarrow", + "Popover", + "Popover-message", + "Popover-message--bottom", + "Popover-message--bottom-left", + "Popover-message--bottom-right", + "Popover-message--large", + "Popover-message--left", + "Popover-message--left-bottom", + "Popover-message--left-top", + "Popover-message--no-caret", + "Popover-message--right", + "Popover-message--right-bottom", + "Popover-message--right-top", + "Popover-message--top-left", + "Popover-message--top-right", + "Progress", + "Progress--large", + "Progress--small", + "Progress-item", + "SegmentedControl", + "SegmentedControl--fullWidth", + "SegmentedControl-item", + "SegmentedControl-item--selected", + "State", + "State--closed", + "State--draft", + "State--merged", + "State--open", + "State--small", + "Subhead", + "Subhead--spacious", + "Subhead-actions", + "Subhead-description", + "Subhead-heading", + "Subhead-heading--danger", + "TimelineItem", + "TimelineItem--condensed", + "TimelineItem-avatar", + "TimelineItem-badge", + "TimelineItem-badge--success", + "TimelineItem-body", + "TimelineItem-break", + "ToggleSwitch", + "ToggleSwitch--checked", + "ToggleSwitch--disabled", + "ToggleSwitch--small", + "ToggleSwitch--statusAtEnd", + "ToggleSwitch-circleIcon", + "ToggleSwitch-icons", + "ToggleSwitch-knob", + "ToggleSwitch-lineIcon", + "ToggleSwitch-status", + "ToggleSwitch-statusIcon", + "ToggleSwitch-statusOff", + "ToggleSwitch-statusOn", + "ToggleSwitch-track", + "Truncate", + "Truncate-text", + "Truncate-text--expandable", + "Truncate-text--primary", + "UnderlineNav", + "UnderlineNav--full", + "UnderlineNav--right", + "UnderlineNav-actions", + "UnderlineNav-body", + "UnderlineNav-container", + "UnderlineNav-item", + "UnderlineNav-octicon" +] \ No newline at end of file From 53ec54a16aac6ce98202e77053d704f267a94127 Mon Sep 17 00:00:00 2001 From: Neal Lindsay Date: Fri, 10 Feb 2023 11:00:35 -0500 Subject: [PATCH 3/5] Inherit from BaseLinter for HTML class checking BaseLinter already has class checking and exception count functionality. We did have to modify BaseLinter to allow an empty `TAGS` array constant, thereby checking all tags. --- .../view_components/linters/base_linter.rb | 2 +- .../disallow_view_component_html_classes.rb | 38 ---------------- .../view_component_html_classes_counter.rb | 30 +++++++++++++ package-lock.json | 8 ++-- package.json | 5 +-- ...iew_component_html_classes_counter_test.rb | 44 +++++++++++++++++++ 6 files changed, 82 insertions(+), 45 deletions(-) delete mode 100644 lib/primer/view_components/linters/disallow_view_component_html_classes.rb create mode 100644 lib/primer/view_components/linters/view_component_html_classes_counter.rb create mode 100644 test/lib/erblint/view_component_html_classes_counter_test.rb diff --git a/lib/primer/view_components/linters/base_linter.rb b/lib/primer/view_components/linters/base_linter.rb index 8cf669d510..7fad24eb07 100644 --- a/lib/primer/view_components/linters/base_linter.rb +++ b/lib/primer/view_components/linters/base_linter.rb @@ -40,7 +40,7 @@ def run(processed_source) tags.each do |tag| next if tag.closing? - next unless self.class::TAGS&.include?(tag.name) + next if self.class::TAGS&.none?(tag.name) classes = tag.attributes["class"]&.value&.split(" ") || [] tag_tree[tag][:offense] = false diff --git a/lib/primer/view_components/linters/disallow_view_component_html_classes.rb b/lib/primer/view_components/linters/disallow_view_component_html_classes.rb deleted file mode 100644 index 68bd9319c8..0000000000 --- a/lib/primer/view_components/linters/disallow_view_component_html_classes.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'better_html' - -require_relative "helpers/rubocop_helpers" - -module ERBLint - module Linters - # Finds usages of ViewComponent-reserved HTML classes - class DisallowViewComponentHtmlClasses < Linter - include LinterRegistry - - RESERVED_HTML_CLASSES = JSON.parse( - File.read( - File.join(__dir__, "..", "..", "..", "..", "static", "classes.json") - ) - ).freeze - - def run(processed_source) - processed_source - .parser - .nodes_with_type(:tag) - .each do |node| - tag = BetterHtml::Tree::Tag.from_node(node) - - tag.attributes["class"]&.value&.split(/\s+/)&.each do |class_name| - if RESERVED_HTML_CLASSES.include? class_name - add_offense( - processed_source.to_source_range(tag.loc), - "HTML class \"#{class_name}\" is reserved for Primer ViewComponents. It might disappear or have different styles in the future." - ) - end - end - end - end - end - end -end diff --git a/lib/primer/view_components/linters/view_component_html_classes_counter.rb b/lib/primer/view_components/linters/view_component_html_classes_counter.rb new file mode 100644 index 0000000000..e312570b22 --- /dev/null +++ b/lib/primer/view_components/linters/view_component_html_classes_counter.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require_relative "base_linter" + +# Load all the other linters so we can filter out their restricted +# CLASSES—they will be responsible for complaining about the use of +# those HTML classes. +Dir[File.join(__dir__, "*.rb")].sort.each do |file| + require file unless file == __FILE__ +end + +module ERBLint + module Linters + # Counts the number of times a class reserved for ViewComponents is used + class ViewComponentHtmlClassesCounter < BaseLinter + CLASSES = ( + JSON.parse( + File.read( + File.join(__dir__, "..", "..", "..", "..", "static", "classes.json") + ) + ) - BaseLinter.subclasses.reduce([]) do |html_classes, klass| + html_classes.concat(klass.const_get(:CLASSES)) + end + ).freeze + + TAGS = nil + MESSAGE = "Primer ViewComponents defines some HTML classes with associated styles that should not be used outside those components. (These classes might have their styles changed or even disappear in the future.) Instead of using this class directly, please use its component if appropriate or define the styles you need some other way." + end + end +end diff --git a/package-lock.json b/package-lock.json index 0a9da52d6f..ed9227825c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,8 +18,7 @@ "@github/mini-throttle": "^2.1.0", "@github/relative-time-element": "^4.0.0", "@github/tab-container-element": "^3.1.2", - "@primer/behaviors": "^1.2.0", - "css-what": "^6.1.0" + "@primer/behaviors": "^1.2.0" }, "devDependencies": { "@changesets/changelog-github": "^0.4.6", @@ -2470,6 +2469,8 @@ "version": "6.1.0", "resolved": "https://registry.npmjs.org/css-what/-/css-what-6.1.0.tgz", "integrity": "sha512-HTUrgRJ7r4dsZKU6GjmpfRK1O76h97Z8MfS1G0FozR+oF2kG6Vfe8JE6zwrkbxigziPHinCJ+gCPjA9EaBDtRw==", + "dev": true, + "license": "BSD-2-Clause", "engines": { "node": ">= 6" }, @@ -11016,7 +11017,8 @@ "css-what": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/css-what/-/css-what-6.1.0.tgz", - "integrity": "sha512-HTUrgRJ7r4dsZKU6GjmpfRK1O76h97Z8MfS1G0FozR+oF2kG6Vfe8JE6zwrkbxigziPHinCJ+gCPjA9EaBDtRw==" + "integrity": "sha512-HTUrgRJ7r4dsZKU6GjmpfRK1O76h97Z8MfS1G0FozR+oF2kG6Vfe8JE6zwrkbxigziPHinCJ+gCPjA9EaBDtRw==", + "dev": true }, "cssdb": { "version": "7.0.0", diff --git a/package.json b/package.json index bdef4d6cb1..2037ab4a00 100644 --- a/package.json +++ b/package.json @@ -53,15 +53,14 @@ "@github/mini-throttle": "^2.1.0", "@github/relative-time-element": "^4.0.0", "@github/tab-container-element": "^3.1.2", - "@primer/behaviors": "^1.2.0", - "css-what": "^6.1.0" + "@primer/behaviors": "^1.2.0" }, "devDependencies": { "@changesets/changelog-github": "^0.4.6", "@changesets/cli": "^2.24.1", "@github/browserslist-config": "^1.0.0", - "@github/markdownlint-github": "^0.2.2", "@github/prettier-config": "0.0.4", + "@github/markdownlint-github": "^0.2.2", "@playwright/test": "^1.27.1", "@primer/css": "20.4.7", "@primer/primitives": "^7.9.0", diff --git a/test/lib/erblint/view_component_html_classes_counter_test.rb b/test/lib/erblint/view_component_html_classes_counter_test.rb new file mode 100644 index 0000000000..7b41de0157 --- /dev/null +++ b/test/lib/erblint/view_component_html_classes_counter_test.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "lib/erblint_test_case" + +class ViewComponentHtmlClassesCounterTest < ErblintTestCase + include Primer::BasicLinterSharedTests + + def test_no_warning_on_unrestricted_class + @file = "
Hello
" + @linter.run(processed_source) + assert_empty offenses + end + + def test_warns_on_restricted_class + @file = "
Reserved for ActionList
" + @linter.run(processed_source) + refute_empty offenses + end + + def test_no_warning_on_class_covered_by_others + @file = "
Covered by other linter
" + @linter.run(processed_source) + assert_empty offenses + end + + def test_does_not_warn_if_wrong_tag + # this test defined in BasicLinterSharedTests does not apply + # to this linter (this linter is designed to check all tags) + end + + private + + def linter_class + ERBLint::Linters::ViewComponentHtmlClassesCounter + end + + def default_tag + "div" + end + + def default_class + "ActionListContent--sizeLarge" + end +end From bacc554bd0baebd2e4f48b33aa250b783f385801 Mon Sep 17 00:00:00 2001 From: Neal Lindsay Date: Wed, 15 Feb 2023 10:02:30 -0500 Subject: [PATCH 4/5] Rename general class-checking counter `DisallowComponentCssCounter` hints at the purpose of this linter better than `ViewComponentHtmlClassesCounter`. --- ...l_classes_counter.rb => disallow_component_css_counter.rb} | 2 +- ...counter_test.rb => disallow_component_css_counter_test.rb} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename lib/primer/view_components/linters/{view_component_html_classes_counter.rb => disallow_component_css_counter.rb} (95%) rename test/lib/erblint/{view_component_html_classes_counter_test.rb => disallow_component_css_counter_test.rb} (89%) diff --git a/lib/primer/view_components/linters/view_component_html_classes_counter.rb b/lib/primer/view_components/linters/disallow_component_css_counter.rb similarity index 95% rename from lib/primer/view_components/linters/view_component_html_classes_counter.rb rename to lib/primer/view_components/linters/disallow_component_css_counter.rb index e312570b22..049ea97bf9 100644 --- a/lib/primer/view_components/linters/view_component_html_classes_counter.rb +++ b/lib/primer/view_components/linters/disallow_component_css_counter.rb @@ -12,7 +12,7 @@ module ERBLint module Linters # Counts the number of times a class reserved for ViewComponents is used - class ViewComponentHtmlClassesCounter < BaseLinter + class DisallowComponentCssCounter < BaseLinter CLASSES = ( JSON.parse( File.read( diff --git a/test/lib/erblint/view_component_html_classes_counter_test.rb b/test/lib/erblint/disallow_component_css_counter_test.rb similarity index 89% rename from test/lib/erblint/view_component_html_classes_counter_test.rb rename to test/lib/erblint/disallow_component_css_counter_test.rb index 7b41de0157..9816fb51e9 100644 --- a/test/lib/erblint/view_component_html_classes_counter_test.rb +++ b/test/lib/erblint/disallow_component_css_counter_test.rb @@ -2,7 +2,7 @@ require "lib/erblint_test_case" -class ViewComponentHtmlClassesCounterTest < ErblintTestCase +class DisallowComponentCssCounterTest < ErblintTestCase include Primer::BasicLinterSharedTests def test_no_warning_on_unrestricted_class @@ -31,7 +31,7 @@ def test_does_not_warn_if_wrong_tag private def linter_class - ERBLint::Linters::ViewComponentHtmlClassesCounter + ERBLint::Linters::DisallowComponentCssCounter end def default_tag From b9ed9a191530a2e9928c756e78628fc22fa5ea67 Mon Sep 17 00:00:00 2001 From: Neal Lindsay Date: Wed, 15 Feb 2023 10:05:54 -0500 Subject: [PATCH 5/5] Add changeset description --- .changeset/ten-ravens-camp.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ten-ravens-camp.md diff --git a/.changeset/ten-ravens-camp.md b/.changeset/ten-ravens-camp.md new file mode 100644 index 0000000000..3f1eb08cc3 --- /dev/null +++ b/.changeset/ten-ravens-camp.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Add general reserved-class-checking linter