diff --git a/src/content-linter/lib/helpers/utils.js b/src/content-linter/lib/helpers/utils.js index 879b36462b..3f0649d4d6 100644 --- a/src/content-linter/lib/helpers/utils.js +++ b/src/content-linter/lib/helpers/utils.js @@ -1,14 +1,11 @@ -import { addError, newLineRe } from 'markdownlint-rule-helpers' +import { addError } from 'markdownlint-rule-helpers' // Adds an error object with details conditionally via the onError callback export function addFixErrorDetail(onError, lineNumber, expected, actual, range, fixInfo) { addError(onError, lineNumber, `Expected: ${expected}`, ` Actual: ${actual}`, range, fixInfo) } -export function getCodeFenceTokens(params) { - return params.tokens.filter((t) => t.type === 'fence') -} - -export function getCodeFenceLines(token) { - return token.content.split(newLineRe) +export function getRange(line, content) { + const startColumnIndex = line.indexOf(content) + return startColumnIndex ? [startColumnIndex + 1, content.length] : null } diff --git a/src/content-linter/lib/linting-rules/code-fence-line-length.js b/src/content-linter/lib/linting-rules/code-fence-line-length.js index 5adc0f1df8..aee37037a4 100644 --- a/src/content-linter/lib/linting-rules/code-fence-line-length.js +++ b/src/content-linter/lib/linting-rules/code-fence-line-length.js @@ -1,6 +1,4 @@ -import { addError, ellipsify } from 'markdownlint-rule-helpers' - -import { getCodeFenceTokens, getCodeFenceLines } from '../helpers/utils.js' +import { addError, ellipsify, filterTokens, newLineRe } from 'markdownlint-rule-helpers' export const codeFenceLineLength = { names: ['GHD001', 'code-fence-line-length'], @@ -10,9 +8,8 @@ export const codeFenceLineLength = { information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'), function: function GHD001(params, onError) { const MAX_LINE_LENGTH = String(params.config.maxLength || 60) - const codeFenceTokens = getCodeFenceTokens(params) - codeFenceTokens.forEach((token) => { - const lines = getCodeFenceLines(token) + filterTokens(params, 'fence', (token) => { + const lines = token.content.split(newLineRe) lines.forEach((line, index) => { if (line.length > MAX_LINE_LENGTH) { // The token line number is the line number of the first line of the diff --git a/src/content-linter/lib/linting-rules/image-alt-text-end-punctuation.js b/src/content-linter/lib/linting-rules/image-alt-text-end-punctuation.js index b3d126f94e..f8a7b7ca8c 100644 --- a/src/content-linter/lib/linting-rules/image-alt-text-end-punctuation.js +++ b/src/content-linter/lib/linting-rules/image-alt-text-end-punctuation.js @@ -1,6 +1,6 @@ import { forEachInlineChild } from 'markdownlint-rule-helpers' -import { addFixErrorDetail } from '../helpers/utils.js' +import { addFixErrorDetail, getRange } from '../helpers/utils.js' export const imageAltTextEndPunctuation = { names: ['GHD002', 'image-alt-text-end-punctuation'], @@ -13,8 +13,7 @@ export const imageAltTextEndPunctuation = { const quoteRegex = /[.?!]['"]$/ const endRegex = /[.?!]$/ const imageAltText = token.content.trim() - const startColumnIndex = token.line.indexOf(imageAltText) - const range = startColumnIndex ? [startColumnIndex + 1, imageAltText.length] : null + const range = getRange(token.line, imageAltText) if ( (!imageAltText.endsWith('"') && !imageAltText.slice(-1).match(endRegex)) || (imageAltText.endsWith('"') && !imageAltText.slice(-2).match(quoteRegex)) diff --git a/src/content-linter/lib/linting-rules/image-alt-text-exclude-start-words.js b/src/content-linter/lib/linting-rules/image-alt-text-exclude-start-words.js new file mode 100644 index 0000000000..9221248769 --- /dev/null +++ b/src/content-linter/lib/linting-rules/image-alt-text-exclude-start-words.js @@ -0,0 +1,35 @@ +import { addError, forEachInlineChild } from 'markdownlint-rule-helpers' + +import { getRange } from '../helpers/utils.js' + +const excludeStartWords = ['image', 'graphic'] + +/* + Images should have meaningful alternative text (alt text) + and should not begin with words like "image" or "graphic". + */ +export const imageAltTextExcludeStartWords = { + names: ['GHD007', 'image-alt-text-exclude-words'], + description: 'Alternate text for images should not begin with words like "image" or "graphic".', + severity: 'error', + tags: ['accessibility', 'images'], + information: new URL('https://github.com/github/docs/blob/main/src/content-linter/README.md'), + function: function GHD007(params, onError) { + forEachInlineChild(params, 'image', function forToken(token) { + const imageAltText = token.content.trim() + const range = getRange(token.line, imageAltText) + if ( + excludeStartWords.some((excludeWord) => imageAltText.toLowerCase().startsWith(excludeWord)) + ) { + addError( + onError, + token.lineNumber, + `Image alternate text should not start with "image" or "graphic".`, + imageAltText, + range, + null, // No fix possible + ) + } + }) + }, +} diff --git a/src/content-linter/lib/linting-rules/image-alt-text-length.js b/src/content-linter/lib/linting-rules/image-alt-text-length.js index c83c8609f7..3e3784c0fb 100644 --- a/src/content-linter/lib/linting-rules/image-alt-text-length.js +++ b/src/content-linter/lib/linting-rules/image-alt-text-length.js @@ -1,6 +1,8 @@ import { addError, forEachInlineChild } from 'markdownlint-rule-helpers' + import { liquid } from '#src/content-render/index.js' import { allVersions } from '../../../../lib/all-versions.js' +import { getRange } from '../helpers/utils.js' export const incorrectAltTextLength = { names: ['GHD003', 'incorrect-alt-text-length'], @@ -11,6 +13,7 @@ export const incorrectAltTextLength = { function: function GHD004(params, onError) { forEachInlineChild(params, 'image', async function forToken(token) { let renderedString = token.content + const range = getRange(token.line, token.content) if (token.content.includes('{%') || token.content.includes('{{')) { const context = { currentLanguage: 'en', @@ -25,7 +28,7 @@ export const incorrectAltTextLength = { token.lineNumber, `Image alternate text is ${renderedString.length} characters long.`, renderedString, - null, // No range + range, null, // No fix possible ) } diff --git a/src/content-linter/lib/linting-rules/index.js b/src/content-linter/lib/linting-rules/index.js index f402fde205..754f957f73 100644 --- a/src/content-linter/lib/linting-rules/index.js +++ b/src/content-linter/lib/linting-rules/index.js @@ -6,6 +6,7 @@ import { imageFileKebab } from './image-file-kebab.js' import { incorrectAltTextLength } from './image-alt-text-length.js' import { internalLinksLang } from './internal-links-lang.js' import { internalLinksSlash } from './internal-links-slash.js' +import { imageAltTextExcludeStartWords } from './image-alt-text-exclude-start-words.js' export const gitHubDocsMarkdownlint = { rules: [ @@ -16,5 +17,6 @@ export const gitHubDocsMarkdownlint = { incorrectAltTextLength, internalLinksLang, internalLinksSlash, + imageAltTextExcludeStartWords, ], } diff --git a/src/content-linter/lib/linting-rules/internal-links-lang.js b/src/content-linter/lib/linting-rules/internal-links-lang.js index c9b0e60ee9..2c728b4400 100644 --- a/src/content-linter/lib/linting-rules/internal-links-lang.js +++ b/src/content-linter/lib/linting-rules/internal-links-lang.js @@ -1,6 +1,6 @@ import { filterTokens } from 'markdownlint-rule-helpers' -import { addFixErrorDetail } from '../helpers/utils.js' +import { addFixErrorDetail, getRange } from '../helpers/utils.js' import { languageKeys } from '../../../../lib/languages.js' export const internalLinksLang = { @@ -27,13 +27,14 @@ export const internalLinksLang = { } } } else if (child.type === 'link_close') { + const range = getRange(token.line, child.content) if (internalLinkHasLang) { addFixErrorDetail( onError, child.lineNumber, linkHref.replace(/(\/)?[a-z]{2}/, ''), linkHref, - undefined, // Todo add range + range, { lineNumber: child.lineNumber, editColumn: token.line.indexOf('(') + 2, diff --git a/src/content-linter/lib/linting-rules/internal-links-slash.js b/src/content-linter/lib/linting-rules/internal-links-slash.js index 9b12a3d03f..adfaf59491 100644 --- a/src/content-linter/lib/linting-rules/internal-links-slash.js +++ b/src/content-linter/lib/linting-rules/internal-links-slash.js @@ -1,6 +1,6 @@ import { filterTokens } from 'markdownlint-rule-helpers' -import { addFixErrorDetail } from '../helpers/utils.js' +import { addFixErrorDetail, getRange } from '../helpers/utils.js' export const internalLinksSlash = { names: ['GHD006', 'internal-links-slash'], @@ -28,8 +28,9 @@ export const internalLinksSlash = { } } } else if (child.type === 'link_close') { + const range = getRange(token.line, child.content) if (!internalLinkHasSlash) { - addFixErrorDetail(onError, child.lineNumber, `/${linkHref}`, linkHref, undefined, { + addFixErrorDetail(onError, child.lineNumber, `/${linkHref}`, linkHref, range, { lineNumber: child.lineNumber, editColumn: token.line.indexOf('(') + 2, deleteCount: 0, diff --git a/src/content-linter/style/github-docs.js b/src/content-linter/style/github-docs.js index eca647f70e..d9cd87274f 100644 --- a/src/content-linter/style/github-docs.js +++ b/src/content-linter/style/github-docs.js @@ -29,6 +29,11 @@ export const githubDocsConfig = { severity: 'error', 'partial-markdown-files': true, }, + 'image-alt-text-exclude-words': { + // GHD007 + severity: 'error', + 'partial-markdown-files': true, + }, 'search-replace': { severity: 'error', 'severity-local-env': 'warning', diff --git a/src/content-linter/tests/fixtures/image-alt-text-exclude-start-words.md b/src/content-linter/tests/fixtures/image-alt-text-exclude-start-words.md new file mode 100644 index 0000000000..03d7372c01 --- /dev/null +++ b/src/content-linter/tests/fixtures/image-alt-text-exclude-start-words.md @@ -0,0 +1,6 @@ +![This is ok image](/images/this-is-ok.png) +![Image with alt text](/images/image-with-alt-text.png) +![image with alt text](/images/image-with-alt-text.png) +![Graphic with alt text](/images/graphic-with-alt-text.png) +![graphic with alt text](/images/graphic-with-alt-text.png) +![This is ok grapic](/images/this-is-ok.png) diff --git a/src/content-linter/tests/unit/image-alt-text-exclude-start-words.js b/src/content-linter/tests/unit/image-alt-text-exclude-start-words.js new file mode 100644 index 0000000000..241b45c004 --- /dev/null +++ b/src/content-linter/tests/unit/image-alt-text-exclude-start-words.js @@ -0,0 +1,18 @@ +import { jest } from '@jest/globals' + +import { runRule } from '../../lib/init-test.js' +import { imageAltTextExcludeStartWords } from '../../lib/linting-rules/image-alt-text-exclude-start-words.js' + +jest.setTimeout(60 * 1000) + +const fixtureFile = 'src/content-linter/tests/fixtures/image-alt-text-exclude-start-words.md' +const result = await runRule(imageAltTextExcludeStartWords, fixtureFile) +const errors = result[fixtureFile] + +describe(imageAltTextExcludeStartWords.names.join(' - '), () => { + test('image alt text does not start with exclude words', () => { + expect(Object.keys(result).length).toBe(1) + expect(errors.length).toBe(4) + expect(errors.map((error) => error.lineNumber)).toEqual([2, 3, 4, 5]) + }) +})