API: created linter ensuring octicons have an aria label (#54798)

Co-authored-by: Kevin Heis <heiskr@users.noreply.github.com>

data/reusables/contributing/content-linter-rules.md

@@ -65,4 +65,4 @@
 | GHD041 | third-party-action-pinning | Code examples that use third-party actions must always pin to a full length commit SHA | error | feature, actions |
 | GHD042 | liquid-tag-whitespace | Liquid tags should start and end with one whitespace. Liquid tag arguments should be separated by only one whitespace. | error | liquid, format |
 | GHD043 | link-quotation | Internal link titles must not be surrounded by quotations | error | links, url |
-| GHD022 | liquid-ifversion-versions | Liquid `ifversion`, `elsif`, and `else` tags should be valid and not contain unsupported versions. | error | liquid, versioning |
+| GHD044 | octicon-aria-labels | Octicons should always have an aria-label attribute even if aria-hidden. | warning | accessibility, octicons |

src/content-linter/lib/linting-rules/index.js

@@ -33,6 +33,7 @@ import { tableLiquidVersioning } from './table-liquid-versioning.js'
 import { thirdPartyActionPinning } from './third-party-action-pinning.js'
 import { liquidTagWhitespace } from './liquid-tag-whitespace.js'
 import { linkQuotation } from './link-quotation.js'
+import { octiconAriaLabels } from './octicon-aria-labels.js'
 import { liquidIfversionVersions } from './liquid-ifversion-versions.js'
 
 const noDefaultAltText = markdownlintGitHub.find((elem) =>
@@ -82,6 +83,6 @@ export const gitHubDocsMarkdownlint = {
     thirdPartyActionPinning,
     liquidTagWhitespace,
     linkQuotation,
-    liquidIfversionVersions,
+    octiconAriaLabels,
   ],
 }

src/content-linter/lib/linting-rules/octicon-aria-labels.js

@@ -0,0 +1,56 @@
+import { TokenKind } from 'liquidjs'
+import { getLiquidTokens, getPositionData } from '../helpers/liquid-utils.js'
+import { addFixErrorDetail } from '../helpers/utils.js'
+/*
+Octicons should always have an aria-label attribute even if aria hidden. For example:
+
+  DO use aria-label
+  {% octicon "alert" aria-label="alert" %}
+  {% octicon "alert" aria-label="alert" aria-hidden="true" %}
+  {% octicon "alert" aria-label="alert" aria-hidden="true" class="foo" %}
+
+  This is necessary for copilot to be able to recognize the svgs correctly when using our API. 
+
+*/
+
+export const octiconAriaLabels = {
+  names: ['GHD044', 'octicon-aria-labels'],
+  description: 'Octicons should always have an aria-label attribute even if aria-hidden.',
+  tags: ['accessibility', 'octicons'],
+  parser: 'markdownit',
+  function: (params, onError) => {
+    const content = params.lines.join('\n')
+    const tokens = getLiquidTokens(content)
+      .filter((token) => token.kind === TokenKind.Tag)
+      .filter((token) => token.name === 'octicon')
+
+    for (const token of tokens) {
+      const { lineNumber, column, length } = getPositionData(token, params.lines)
+
+      const hasAriaLabel = token.args.includes('aria-label=')
+
+      if (!hasAriaLabel) {
+        const range = [column, length]
+
+        const octiconNameMatch = token.args.match(/["']([^"']+)["']/)
+        const octiconName = octiconNameMatch ? octiconNameMatch[1] : 'icon'
+        const originalContent = token.content
+        const fixedContent = originalContent + ` aria-label="${octiconName}"`
+
+        addFixErrorDetail(
+          onError,
+          lineNumber,
+          `octicon should have an aria-label even if aria hidden. Try using 'aria-label=${octiconName}'`,
+          token.content,
+          range,
+          {
+            lineNumber,
+            editColumn: column,
+            deleteCount: length,
+            insertText: `{% ${fixedContent} %}`,
+          },
+        )
+      }
+    }
+  },
+}

src/content-linter/style/github-docs.js

@@ -168,6 +168,12 @@ const githubDocsConfig = {
     'partial-markdown-files': true,
     'yml-files': true,
   },
+  'octicon-aria-labels': {
+    // GHD044
+    severity: 'warning',
+    'partial-markdown-files': true,
+    'yml-files': true,
+  },
 }
 
 export const githubDocsFrontmatterConfig = {

src/content-linter/tests/unit/octicon-aria-labels.js

@@ -0,0 +1,160 @@
+import { describe, expect, test } from 'vitest'
+import { octiconAriaLabels } from '../../lib/linting-rules/octicon-aria-labels.js'
+
+describe('octicon-aria-labels', () => {
+  const rule = octiconAriaLabels
+
+  test('detects octicon without aria-label', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = ['This is a test with an octicon:', '{% octicon "alert" %}', 'Some more text.']
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].detail).toContain('aria-label=alert')
+    expect(errors[0].fixInfo.insertText).toContain('aria-label="alert"')
+  })
+
+  test('ignores octicons with aria-label', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with a proper octicon:',
+      '{% octicon "alert" aria-label="alert" %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(0)
+  })
+
+  test('detects multiple octicons without aria-label', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with multiple octicons:',
+      '{% octicon "alert" %}',
+      'Some text in between.',
+      '{% octicon "check" %}',
+      'More text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(2)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].detail).toContain('aria-label=alert')
+    expect(errors[1].lineNumber).toBe(4)
+    expect(errors[1].detail).toContain('aria-label=check')
+  })
+
+  test('ignores non-octicon liquid tags', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with non-octicon tags:',
+      '{% data foo.bar %}',
+      '{% ifversion fpt %}',
+      'Some text.',
+      '{% endif %}',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(0)
+  })
+
+  test('suggests correct fix for octicon with other attributes', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with an octicon with other attributes:',
+      '{% octicon "plus" aria-hidden="true" class="foo" %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].fixInfo.insertText).toContain('aria-label="plus"')
+    expect(errors[0].fixInfo.insertText).toContain('aria-hidden="true"')
+    expect(errors[0].fixInfo.insertText).toContain('class="foo"')
+  })
+
+  test('handles octicons with unusual spacing', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with unusual spacing:',
+      '{%  octicon   "x"    %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].detail).toContain('aria-label=x')
+  })
+
+  test('handles octicons split across multiple lines', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with a multi-line octicon:',
+      '{% octicon "chevron-down"',
+      '   class="dropdown-menu-icon"',
+      '%}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].detail).toContain('aria-label=chevron-down')
+  })
+
+  test('falls back to "icon" when octicon name cannot be determined', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with a malformed octicon:',
+      '{% octicon variable %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].detail).toContain('aria-label=icon')
+    expect(errors[0].fixInfo.insertText).toContain('aria-label="icon"')
+  })
+})