Skip to content

Commit 0db9781

Browse files
authored
Add table column integrity checker content linter rule (GHD047) (#56099)
1 parent 0dcc58d commit 0db9781

File tree

5 files changed

+206
-0
lines changed

5 files changed

+206
-0
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
| GHD038 | expired-content | Expired content must be remediated. | warning | expired |
6565
| GHD039 | expiring-soon | Content that expires soon should be proactively addressed. | warning | expired |
6666
| [GHD040](https://github.com/github/docs/blob/main/src/content-linter/README.md) | table-liquid-versioning | Tables must use the correct liquid versioning format | error | tables |
67+
| GHD047 | table-column-integrity | Tables must have consistent column counts across all rows | warning | tables, accessibility, formatting |
6768
| GHD041 | third-party-action-pinning | Code examples that use third-party actions must always pin to a full length commit SHA | error | feature, actions |
6869
| 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 |
6970
| GHD043 | link-quotation | Internal link titles must not be surrounded by quotations | error | links, url |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { raiReusableUsage } from './rai-reusable-usage.js'
3131
import { imageNoGif } from './image-no-gif.js'
3232
import { expiredContent, expiringSoon } from './expired-content.js'
3333
import { tableLiquidVersioning } from './table-liquid-versioning.js'
34+
import { tableColumnIntegrity } from './table-column-integrity.js'
3435
import { thirdPartyActionPinning } from './third-party-action-pinning.js'
3536
import { liquidTagWhitespace } from './liquid-tag-whitespace.js'
3637
import { linkQuotation } from './link-quotation.js'
@@ -85,6 +86,7 @@ export const gitHubDocsMarkdownlint = {
8586
expiredContent,
8687
expiringSoon,
8788
tableLiquidVersioning,
89+
tableColumnIntegrity,
8890
thirdPartyActionPinning,
8991
liquidTagWhitespace,
9092
linkQuotation,
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { addError } from 'markdownlint-rule-helpers'
2+
import { getRange } from '../helpers/utils.js'
3+
import frontmatter from '#src/frame/lib/read-frontmatter.js'
4+
5+
// Regex to detect table rows (must start with |, contain at least one more |, and end with optional whitespace)
6+
const TABLE_ROW_REGEX = /^\s*\|.*\|\s*$/
7+
// Regex to detect table separator rows (contains only |, :, -, and whitespace)
8+
const TABLE_SEPARATOR_REGEX = /^\s*\|[\s\-:|\s]*\|\s*$/
9+
// Regex to detect Liquid-only cells (whitespace, liquid tag, whitespace)
10+
const LIQUID_ONLY_CELL_REGEX = /^\s*{%\s*(ifversion|else|endif|elsif).*%}\s*$/
11+
12+
/**
13+
* Counts the number of columns in a table row by splitting on | and handling edge cases
14+
*/
15+
function countColumns(row) {
16+
// Remove leading and trailing whitespace
17+
const trimmed = row.trim()
18+
19+
// Handle empty rows
20+
if (!trimmed || !trimmed.includes('|')) {
21+
return 0
22+
}
23+
24+
// Split by | and filter out empty cells at start/end (from leading/trailing |)
25+
const cells = trimmed.split('|')
26+
27+
// Remove first and last elements if they're empty (from leading/trailing |)
28+
if (cells.length > 0 && cells[0].trim() === '') {
29+
cells.shift()
30+
}
31+
if (cells.length > 0 && cells[cells.length - 1].trim() === '') {
32+
cells.pop()
33+
}
34+
35+
return cells.length
36+
}
37+
38+
/**
39+
* Checks if a table row contains only Liquid conditionals
40+
*/
41+
function isLiquidOnlyRow(row) {
42+
const trimmed = row.trim()
43+
if (!trimmed.includes('|')) return false
44+
45+
const cells = trimmed.split('|')
46+
// Remove empty cells from leading/trailing |
47+
const filteredCells = cells.filter((cell, index) => {
48+
if (index === 0 && cell.trim() === '') return false
49+
if (index === cells.length - 1 && cell.trim() === '') return false
50+
return true
51+
})
52+
53+
// Check if all cells contain only Liquid tags
54+
return (
55+
filteredCells.length > 0 && filteredCells.every((cell) => LIQUID_ONLY_CELL_REGEX.test(cell))
56+
)
57+
}
58+
59+
export const tableColumnIntegrity = {
60+
names: ['GHD047', 'table-column-integrity'],
61+
description: 'Tables must have consistent column counts across all rows',
62+
tags: ['tables', 'accessibility', 'formatting'],
63+
severity: 'error',
64+
function: (params, onError) => {
65+
// Skip autogenerated files
66+
const frontmatterString = params.frontMatterLines.join('\n')
67+
const fm = frontmatter(frontmatterString).data
68+
if (fm && fm.autogenerated) return
69+
70+
const lines = params.lines
71+
let inTable = false
72+
let expectedColumnCount = null
73+
let tableStartLine = null
74+
let headerRow = null
75+
76+
for (let i = 0; i < lines.length; i++) {
77+
const line = lines[i]
78+
const isTableRow = TABLE_ROW_REGEX.test(line)
79+
const isSeparatorRow = TABLE_SEPARATOR_REGEX.test(line)
80+
81+
// Check if we're starting a new table
82+
if (!inTable && isTableRow) {
83+
// Look ahead to see if next line is a separator (confirming this is a table)
84+
const nextLine = lines[i + 1]
85+
if (nextLine && TABLE_SEPARATOR_REGEX.test(nextLine)) {
86+
inTable = true
87+
tableStartLine = i + 1
88+
headerRow = line
89+
expectedColumnCount = countColumns(line)
90+
continue
91+
}
92+
}
93+
94+
// Check if we're ending a table
95+
if (inTable && !isTableRow) {
96+
inTable = false
97+
expectedColumnCount = null
98+
tableStartLine = null
99+
headerRow = null
100+
continue
101+
}
102+
103+
// If we're in a table, validate column count
104+
if (inTable && isTableRow && !isSeparatorRow) {
105+
// Skip Liquid-only rows as they're allowed to have different column counts
106+
if (isLiquidOnlyRow(line)) {
107+
continue
108+
}
109+
110+
const actualColumnCount = countColumns(line)
111+
112+
if (actualColumnCount !== expectedColumnCount) {
113+
const range = getRange(line, line.trim())
114+
let errorMessage
115+
116+
if (actualColumnCount > expectedColumnCount) {
117+
errorMessage = `Table row has ${actualColumnCount} columns but header has ${expectedColumnCount}. Add ${actualColumnCount - expectedColumnCount} more column(s) to the header row to match this row.`
118+
} else {
119+
errorMessage = `Table row has ${actualColumnCount} columns but header has ${expectedColumnCount}. Add ${expectedColumnCount - actualColumnCount} missing column(s) to this row.`
120+
}
121+
122+
addError(
123+
onError,
124+
i + 1,
125+
errorMessage,
126+
line,
127+
range,
128+
null, // No auto-fix available due to complexity
129+
)
130+
}
131+
}
132+
}
133+
},
134+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,12 @@ const githubDocsConfig = {
186186
'partial-markdown-files': true,
187187
'yml-files': true,
188188
},
189+
'table-column-integrity': {
190+
// GHD047
191+
severity: 'warning',
192+
'partial-markdown-files': true,
193+
'yml-files': true,
194+
},
189195
'british-english-quotes': {
190196
// GHD048
191197
severity: 'warning',
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { describe, expect, test } from 'vitest'
2+
3+
import { runRule } from '../../lib/init-test.js'
4+
import { tableColumnIntegrity } from '../../lib/linting-rules/table-column-integrity.js'
5+
6+
describe(tableColumnIntegrity.names.join(' - '), () => {
7+
test('Valid table with consistent columns passes', async () => {
8+
const markdown = [
9+
'| Artist | Album | Year |',
10+
'|--------|-------|------|',
11+
'| Beyoncé | Lemonade | 2016 |',
12+
'| Kendrick Lamar | DAMN. | 2017 |',
13+
].join('\n')
14+
const result = await runRule(tableColumnIntegrity, { strings: { markdown } })
15+
const errors = result.markdown
16+
expect(errors.length).toBe(0)
17+
})
18+
19+
test('Table with extra columns causes error', async () => {
20+
const markdown = ['| Name | Age |', '|------|-----|', '| Alice | 25 | Extra |'].join('\n')
21+
const result = await runRule(tableColumnIntegrity, { strings: { markdown } })
22+
const errors = result.markdown
23+
expect(errors.length).toBe(1)
24+
expect(errors[0].lineNumber).toBe(3)
25+
if (errors[0].detail) {
26+
expect(errors[0].detail).toContain('Table row has 3 columns but header has 2')
27+
} else if (errors[0].errorDetail) {
28+
expect(errors[0].errorDetail).toContain('Table row has 3 columns but header has 2')
29+
} else {
30+
console.log('Error structure:', JSON.stringify(errors[0], null, 2))
31+
expect(errors[0]).toHaveProperty('detail')
32+
}
33+
})
34+
35+
test('Table with missing columns causes error', async () => {
36+
const markdown = ['| Name | Age | City |', '|------|-----|------|', '| Bob | 30 |'].join('\n')
37+
const result = await runRule(tableColumnIntegrity, { strings: { markdown } })
38+
const errors = result.markdown
39+
expect(errors.length).toBe(1)
40+
expect(errors[0].lineNumber).toBe(3)
41+
if (errors[0].detail) {
42+
expect(errors[0].detail).toContain('Table row has 2 columns but header has 3')
43+
} else if (errors[0].errorDetail) {
44+
expect(errors[0].errorDetail).toContain('Table row has 2 columns but header has 3')
45+
} else {
46+
console.log('Error structure:', JSON.stringify(errors[0], null, 2))
47+
expect(errors[0]).toHaveProperty('detail')
48+
}
49+
})
50+
51+
test('Liquid-only rows are ignored', async () => {
52+
const markdown = [
53+
'| Feature | Status |',
54+
'|---------|--------|',
55+
'| {% ifversion ghes %} |',
56+
'| Advanced | Enabled |',
57+
'| {% endif %} |',
58+
].join('\n')
59+
const result = await runRule(tableColumnIntegrity, { strings: { markdown } })
60+
const errors = result.markdown
61+
expect(errors.length).toBe(0)
62+
})
63+
})

0 commit comments

Comments
 (0)