diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index a506bccaf..7a7775d9a 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -9,12 +9,14 @@ This page contains documentation for all Herb Linter rules. - [`a11y-no-accesskey-attribute`](./a11y-no-accesskey-attribute.md) - Prevent usage of the `accesskey` attribute - [`a11y-no-aria-unsupported-elements`](./a11y-no-aria-unsupported-elements.md) - Prevent usage of ARIA on unsupported elements - [`a11y-no-autofocus-attribute`](./a11y-no-autofocus-attribute.md) - Prevent usage of the `autofocus` attribute +- [`a11y-no-redundant-image-alt`](./a11y-no-redundant-image-alt.md) - Prevent redundant words in `` `alt` attributes #### Action View - [`actionview-no-silent-helper`](./actionview-no-silent-helper.md) - Disallow silent ERB tags for Action View helpers - [`actionview-no-silent-render`](./actionview-no-silent-render.md) - Disallow calling `render` without outputting the result +- [`actionview-no-unnecessary-tag-attributes`](./actionview-no-unnecessary-tag-attributes.md) - Disallow unnecessary attributes on Action View tag helpers - [`actionview-no-void-element-content`](./actionview-no-void-element-content.md) - Disallow content arguments for void Action View elements - [`actionview-strict-locals-first-line`](./actionview-strict-locals-first-line.md) - Require strict locals on the first line of partials with a blank line after. - [`actionview-strict-locals-partial-only`](./actionview-strict-locals-partial-only.md) - Only allow strict local definitions in partial files. @@ -26,6 +28,7 @@ This page contains documentation for all Herb Linter rules. - [`erb-no-case-node-children`](./erb-no-case-node-children.md) - Don't use `children` for `case/when` and `case/in` nodes - [`erb-no-debug-output`](./erb-no-debug-output.md) - Disallow debug output methods (`p`, `pp`, `puts`, `print`, `debug`) in ERB templates - [`erb-no-conditional-html-element`](./erb-no-conditional-html-element.md) - Disallow conditional HTML elements +- [`erb-no-conditional-open-tag`](./erb-no-conditional-open-tag.md) - Disallow conditional HTML open tags - [`erb-no-duplicate-branch-elements`](./erb-no-duplicate-branch-elements.md) - Disallow duplicate elements across conditional branches - [`erb-no-empty-control-flow`](./erb-no-empty-control-flow.md) - Disallow empty ERB control flow blocks - [`erb-no-empty-tags`](./erb-no-empty-tags.md) - Disallow empty ERB tags @@ -97,6 +100,7 @@ This page contains documentation for all Herb Linter rules. - [`html-no-duplicate-ids`](./html-no-duplicate-ids.md) - Prevents duplicate IDs within a document - [`html-no-duplicate-meta-names`](./html-no-duplicate-meta-names.md) - Duplicate `` name attributes are not allowed. - [`html-no-empty-attributes`](./html-no-empty-attributes.md) - Attributes must not have empty values +- [`html-no-empty-headings`](./html-no-empty-headings.md) - Disallow empty heading elements - [`html-no-nested-links`](./html-no-nested-links.md) - Prevents nested anchor tags - [`html-no-positive-tab-index`](./html-no-positive-tab-index.md) - Avoid positive `tabindex` values - [`html-no-self-closing`](./html-no-self-closing.md) - Disallow self closing tags @@ -105,6 +109,7 @@ This page contains documentation for all Herb Linter rules. - [`html-no-space-in-tag`](./html-no-space-in-tag.md) - Disallow spaces in HTML tags - [`html-no-title-attribute`](./html-no-title-attribute.md) - Avoid using the `title` attribute - [`html-no-underscores-in-attribute-names`](./html-no-underscores-in-attribute-names.md) - Disallow underscores in HTML attribute names +- [`html-require-closing-tags`](./html-require-closing-tags.md) - Require closing tags for non-void HTML elements - [`html-require-script-nonce`](./html-require-script-nonce.md) - Require `nonce` attribute on script tags and helpers - [`html-tag-name-lowercase`](./html-tag-name-lowercase.md) - Enforces lowercase tag names in HTML diff --git a/javascript/packages/linter/docs/rules/a11y-no-redundant-image-alt.md b/javascript/packages/linter/docs/rules/a11y-no-redundant-image-alt.md new file mode 100644 index 000000000..4d576be8a --- /dev/null +++ b/javascript/packages/linter/docs/rules/a11y-no-redundant-image-alt.md @@ -0,0 +1,42 @@ +# Linter Rule: No redundant `alt` text on `` tags + +**Rule:** `a11y-no-redundant-image-alt` + +## Description + +Enforce that `` `alt` attributes do not contain the words "image" or "picture". + +## Rationale + +Screen readers already announce `` elements as images. Including words like "image" or "picture" in the `alt` attribute is redundant and creates a repetitive experience for users relying on assistive technologies (e.g., "image, image of a sunset"). + +## Examples + +### ✅ Good + +```erb +A sunset over the ocean +``` + +```erb +Company logo +``` + +```erb + +``` + +### 🚫 Bad + +```erb +image of a sunset +``` + +```erb +picture of the company logo +``` + +## References + +- [WCAG 2.1: Non-text Content](https://www.w3.org/WAI/WCAG22/quickref/?versions=2.1#non-text-content) +- [Deque: Alt text should not use the word "image", "picture", or "photo"](https://dequeuniversity.com/rules/axe/4.10/image-redundant-alt) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 7a4f935b3..7e02f77c0 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -3,6 +3,7 @@ import type { RuleClass } from "./types.js" import { A11yNoAccesskeyAttributeRule } from "./rules/a11y-no-accesskey-attribute.js" import { A11yNoAriaUnsupportedElementsRule } from "./rules/a11y-no-aria-unsupported-elements.js" import { A11yNoAutofocusAttributeRule } from "./rules/a11y-no-autofocus-attribute.js" +import { A11yNoRedundantImageAltRule } from "./rules/a11y-no-redundant-image-alt.js" import { ActionViewNoSilentHelperRule } from "./rules/actionview-no-silent-helper.js" import { ActionViewNoSilentRenderRule } from "./rules/actionview-no-silent-render.js" @@ -106,6 +107,7 @@ export const rules: RuleClass[] = [ A11yNoAccesskeyAttributeRule, A11yNoAriaUnsupportedElementsRule, A11yNoAutofocusAttributeRule, + A11yNoRedundantImageAltRule, ActionViewNoSilentHelperRule, ActionViewNoSilentRenderRule, diff --git a/javascript/packages/linter/src/rules/a11y-no-redundant-image-alt.ts b/javascript/packages/linter/src/rules/a11y-no-redundant-image-alt.ts new file mode 100644 index 000000000..814b20410 --- /dev/null +++ b/javascript/packages/linter/src/rules/a11y-no-redundant-image-alt.ts @@ -0,0 +1,78 @@ +import { BaseRuleVisitor } from "./rule-utils.js" +import { getAttribute, getAttributeValue, hasAttributeValue, getTagLocalName, isHTMLOpenTagNode, isERBOpenTagNode, filterHTMLAttributeNodes, findAttributeByName, hasDynamicAttributeValue } from "@herb-tools/core" + +import { ParserRule } from "../types.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" +import type { HTMLElementNode, HTMLAttributeNode, ParseResult, ParserOptions } from "@herb-tools/core" + +const REDUNDANT_ALT_WORDS = new Set(["image", "picture"]) + +class NoRedundantImageAltVisitor extends BaseRuleVisitor { + visitHTMLElementNode(node: HTMLElementNode): void { + this.checkImgTag(node) + super.visitHTMLElementNode(node) + } + + private checkImgTag(node: HTMLElementNode): void { + const tagName = getTagLocalName(node) + + if (tagName !== "img") { + return + } + + const altAttribute = this.getAltAttribute(node) + + if (!altAttribute) return + if (!hasAttributeValue(altAttribute)) return + if (hasDynamicAttributeValue(altAttribute)) return + + const altValue = getAttributeValue(altAttribute) + + if (!altValue) return + + const words = altValue.toLowerCase().split(/\s+/) + + if (words.some(word => REDUNDANT_ALT_WORDS.has(word))) { + this.addOffense( + "`` `alt` prop should not contain \"image\" or \"picture\" as screen readers already announce the element as an image.", + altAttribute.location, + ) + } + } + + private getAltAttribute(node: HTMLElementNode): HTMLAttributeNode | null { + const openTag = node.open_tag + + if (isHTMLOpenTagNode(openTag)) { + return getAttribute(openTag, "alt") + } + + if (isERBOpenTagNode(openTag)) { + return findAttributeByName(filterHTMLAttributeNodes(openTag.children), "alt") + } + + return null + } +} + +export class A11yNoRedundantImageAltRule extends ParserRule { + static ruleName = "a11y-no-redundant-image-alt" + static introducedIn = this.version("unreleased") + + get defaultConfig(): FullRuleConfig { + return { + enabled: false, + severity: "warning" + } + } + + get parserOptions(): Partial { + return { action_view_helpers: true } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new NoRedundantImageAltVisitor(this.ruleName, context) + visitor.visit(result.value) + return visitor.offenses + } +} diff --git a/javascript/packages/linter/src/rules/index.ts b/javascript/packages/linter/src/rules/index.ts index 815093506..08c4e1b6d 100644 --- a/javascript/packages/linter/src/rules/index.ts +++ b/javascript/packages/linter/src/rules/index.ts @@ -1,6 +1,7 @@ export * from "./a11y-no-accesskey-attribute.js" export * from "./a11y-no-aria-unsupported-elements.js" export * from "./a11y-no-autofocus-attribute.js" +export * from "./a11y-no-redundant-image-alt.js" export * from "./rule-utils.js" export * from "./prism-rule-utils.js" diff --git a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap index 1b38e8234..04fb24b07 100644 --- a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap +++ b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap @@ -111,7 +111,7 @@ test/fixtures/ignored.html.erb:8:8 Offenses 5 errors | 2 warnings (7 offenses across 1 file) Note 3 additional offenses reported (would have been ignored) Fixable 7 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > Excluded Files > skips excluded file in subdirectory with README.md project indicator 1`] = ` @@ -177,7 +177,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 1`] = ` @@ -202,7 +202,7 @@ test/fixtures/no-trailing-newline.html.erb:1:29 Checked 1 file Offenses 1 error | 0 warnings (1 offense across 1 file) Fixable 1 offense | 1 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 2`] = ` @@ -266,7 +266,7 @@ test/fixtures/erb-no-extra-whitespace-inside-tags.html.erb:1:4 Checked 1 file Offenses 4 errors | 0 warnings (4 offenses across 1 file) Fixable 4 offenses | 3 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > Ignores disabled rules 1`] = ` @@ -307,7 +307,7 @@ test/fixtures/ignored.html.erb:6:14 Checked 1 file Offenses 2 errors | 0 warnings | 3 ignored (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > allows tag.attributes in attribute position 1`] = ` @@ -346,7 +346,7 @@ test/fixtures/tag-attributes.html.erb:5:0 Checked 1 file Offenses 2 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > diplays only parsers errors if one is present 1`] = ` @@ -372,7 +372,7 @@ test/fixtures/parser-errors.html.erb:2:16 Checked 1 file Offenses 1 error | 0 warnings (1 offense across 1 file) Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > displays most violated rules with multiple offenses 1`] = ` @@ -587,7 +587,7 @@ test/fixtures/multiple-rule-offenses.html.erb:4:2 Checked 1 file Offenses 8 errors | 6 warnings (14 offenses across 1 file) Fixable 14 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > displays rule offenses when showing all rules 1`] = ` @@ -684,7 +684,7 @@ test/fixtures/few-rule-offenses.html.erb:6:0 Checked 1 file Offenses 4 errors | 2 warnings (6 offenses across 1 file) Fixable 6 offenses | 3 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for bad file 1`] = ` @@ -723,7 +723,7 @@ test/fixtures/bad-file.html.erb:1:16 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for clean file 1`] = ` @@ -735,7 +735,7 @@ exports[`CLI Output Formatting > formats GitHub Actions output correctly for cle Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for file with errors 1`] = ` @@ -795,7 +795,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output with --format=github option 1`] = ` @@ -834,7 +834,7 @@ test/fixtures/test-file-simple.html.erb:2:22 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats JSON output correctly for bad file 1`] = ` @@ -1038,7 +1038,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats simple output correctly 1`] = ` @@ -1057,7 +1057,7 @@ test/fixtures/test-file-simple.html.erb: Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats simple output for bad-file correctly 1`] = ` @@ -1076,7 +1076,7 @@ test/fixtures/bad-file.html.erb: Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats success output correctly 1`] = ` @@ -1088,7 +1088,7 @@ exports[`CLI Output Formatting > formats success output correctly 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > handles boolean attributes 1`] = ` @@ -1100,7 +1100,7 @@ exports[`CLI Output Formatting > handles boolean attributes 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > handles multiple errors correctly 1`] = ` @@ -1135,7 +1135,7 @@ test/fixtures/bad-file.html.erb:1:16 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > herb:disable rules 1`] = ` @@ -1267,7 +1267,7 @@ test/fixtures/disabled-1.html.erb:14:19 Checked 1 file Offenses 2 errors | 6 warnings | 8 ignored (8 offenses across 1 file) Fixable 8 offenses | 1 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > herb:disable rules 2`] = ` @@ -1470,7 +1470,7 @@ test/fixtures/disabled-2.html.erb:2:44 Checked 1 file Offenses 6 errors | 7 warnings | 5 ignored (13 offenses across 1 file) Fixable 13 offenses | 6 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > uses GitHub Actions format by default when GITHUB_ACTIONS is true 1`] = ` @@ -1530,5 +1530,5 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; diff --git a/javascript/packages/linter/test/rule-docs.test.ts b/javascript/packages/linter/test/rule-docs.test.ts new file mode 100644 index 000000000..dd73aa785 --- /dev/null +++ b/javascript/packages/linter/test/rule-docs.test.ts @@ -0,0 +1,22 @@ +import fs from "node:fs" +import path from "node:path" +import { describe, test, expect } from "vitest" + +import { rules } from "../src/rules.js" + +const docsDir = path.resolve(__dirname, "../docs/rules") +const readmePath = path.join(docsDir, "README.md") +const readmeContent = fs.readFileSync(readmePath, "utf-8") + +describe("rule documentation completeness", () => { + const ruleNames = rules.map((r) => r.ruleName) + + test.each(ruleNames)("%s has a docs page", (ruleName) => { + const docPath = path.join(docsDir, `${ruleName}.md`) + expect(fs.existsSync(docPath), `Missing docs page: docs/rules/${ruleName}.md`).toBe(true) + }) + + test.each(ruleNames)("%s is linked from the README", (ruleName) => { + expect(readmeContent, `Missing README entry for ${ruleName}`).toContain(`(./${ruleName}.md)`) + }) +}) diff --git a/javascript/packages/linter/test/rules/a11y-no-redundant-image-alt.test.ts b/javascript/packages/linter/test/rules/a11y-no-redundant-image-alt.test.ts new file mode 100644 index 000000000..e39b09f5b --- /dev/null +++ b/javascript/packages/linter/test/rules/a11y-no-redundant-image-alt.test.ts @@ -0,0 +1,104 @@ +import { describe, test } from "vitest" +import { A11yNoRedundantImageAltRule } from "../../src/rules/a11y-no-redundant-image-alt.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" +import dedent from "dedent" + +const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(A11yNoRedundantImageAltRule) + +const MESSAGE = '`` `alt` prop should not contain "image" or "picture" as screen readers already announce the element as an image.' + +describe("a11y-no-redundant-image-alt", () => { + describe("valid cases", () => { + test("passes for img with descriptive alt", () => { + expectNoOffenses('Mona Lisa') + }) + + test("passes for img with detailed descriptive alt", () => { + expectNoOffenses('The original painting of Mona Lisa') + }) + + test("passes for img with screenshot in alt", () => { + expectNoOffenses('Screenshot of the Settings page') + }) + + test("passes for img with photograph in alt", () => { + expectNoOffenses('Photograph of a sunset') + }) + + test("passes for img with painting in alt", () => { + expectNoOffenses('Painting of the night sky') + }) + + test("passes for img with no alt attribute", () => { + expectNoOffenses('') + }) + + test("passes for img with empty alt attribute", () => { + expectNoOffenses('') + }) + + test("passes for img with ERB dynamic alt value", () => { + expectNoOffenses('<%= description %>') + }) + + test("passes for non-img elements", () => { + expectNoOffenses('
') + }) + + test("passes for img with word containing image as substring", () => { + expectNoOffenses('imagery of the forest') + }) + }) + + describe("invalid cases", () => { + test("fails for alt containing 'picture'", () => { + expectWarning(MESSAGE) + + assertOffenses('picture of Mona Lisa') + }) + + test("fails for alt containing 'image'", () => { + expectWarning(MESSAGE) + + assertOffenses('image of a fluffy dog') + }) + + test("fails for alt containing 'Image' (case insensitive)", () => { + expectWarning(MESSAGE) + + assertOffenses('Image of a fluffy dog') + }) + + test("fails for alt containing 'PICTURE' (case insensitive)", () => { + expectWarning(MESSAGE) + + assertOffenses('PICTURE of a sunset') + }) + + test("fails for alt containing 'image' among other words", () => { + expectWarning(MESSAGE) + + assertOffenses('an image showing the dashboard') + }) + + test("fails for alt that is just 'image'", () => { + expectWarning(MESSAGE) + + assertOffenses('image') + }) + + test("fails for alt that is just 'picture'", () => { + expectWarning(MESSAGE) + + assertOffenses('picture') + }) + }) + + describe("ERB open tag", () => { + test("passes for image_tag with descriptive alt", () => { + expectNoOffenses(dedent` + <%= image_tag "monalisa.png", alt: "Mona Lisa" %> + `) + }) + }) +})