Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 68 additions & 13 deletions .github/scripts/check-markdown.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
* Current checks:
* - H1 headings (# ...): the H1 is already generated from the `title` field
* in the frontmatter, so adding a `#` heading manually creates a duplicate.
* - Absolute URLs to the published site: links beginning with
* https://design-history.prevention-services.nhs.uk/ should use
* relative URLs instead, so that they work in previews and in case the URL
* changes in future.
*
* When run directly, scans all markdown files under app/:
* npm run check:markdown
Expand All @@ -19,10 +23,61 @@ import { readFileSync, readdirSync } from 'fs'
import { join } from 'path'
import { fileURLToPath } from 'url'

const H1_MESSAGE =
'The page title H1 is already generated from the `title` field in the frontmatter. ' +
'If this heading duplicates the title, remove it. ' +
'If it is a different heading, change it to an H2 using `##`.'
/**
* Checks if a line contains an H1 heading.
*
* @param {string} line
* @returns {{ message: string } | null}
*/
function checkH1Heading(line) {
const message =
'The page title H1 is already generated from the `title` field in the frontmatter. ' +
'If this heading duplicates the title, remove it. ' +
'If it is a different heading, change it to an H2 using `##`.'

if (/^# /.test(line)) {
return { message }
}
return null
}

/**
* Checks if a line contains an absolute URL to the published site.
*
* @param {string} line
* @returns {{ message: string, suggestion?: string } | null}
*/
function checkAbsoluteUrl(line) {
const siteUrlRe = /(https?:\/\/)?design-history\.prevention-services\.nhs\.uk\//i

const message =
'Use a relative URL instead of a full URL for links to other posts on the site.\n\n' +
'This means that the links will work in previews, and in case the site domain name changes in future.\n\n' +
'For example, replace `https://design-history.prevention-services.nhs.uk/some/path/` with `/some/path/`.'

if (siteUrlRe.test(line)) {
return {
message,
suggestion: line.replaceAll(siteUrlRe, '/')
}
}
return null
}

const checks = [checkH1Heading, checkAbsoluteUrl]

/**
* Runs all checks against a single line and returns any mistakes found.
*
* @param {string} line
* @returns {{ message: string, suggestion?: string }[]}
*/
function checkLine(line) {
return checks.flatMap((check) => {
const result = check(line)
return result ? [result] : []
})
}

/**
* Recursively finds all .md files under the given directory.
Expand Down Expand Up @@ -53,8 +108,8 @@ export function scanAllFiles() {
for (const filePath of files) {
const lines = readFileSync(filePath, 'utf8').split('\n')
for (let i = 0; i < lines.length; i++) {
if (/^# /.test(lines[i])) {
mistakes.push({ path: filePath, line: i + 1, message: H1_MESSAGE })
for (const result of checkLine(lines[i])) {
mistakes.push({ path: filePath, line: i + 1, ...result })
}
Comment thread
frankieroberto marked this conversation as resolved.
}
}
Expand All @@ -70,6 +125,10 @@ export function scanAllFiles() {
* @returns {{ path: string, line: number, message: string }[]}
*/
export function getMistakes(baseRef) {
// Reject unexpected characters to prevent shell command injection
if (!/^[\w\/.\-]+$/.test(baseRef)) {
throw new Error(`Invalid baseRef: ${baseRef}`)
}
const diff = execSync(`git diff ${baseRef}...HEAD`, { encoding: 'utf8' })
const mistakes = []
let currentFile = null
Expand Down Expand Up @@ -98,13 +157,9 @@ export function getMistakes(baseRef) {

if (rawLine.startsWith('+')) {
lineNumber++
// Added line that is an H1 (single `#` followed by a space)
if (/^\+# /.test(rawLine)) {
mistakes.push({
path: currentFile,
line: lineNumber,
message: H1_MESSAGE
})
const lineContent = rawLine.slice(1)
for (const result of checkLine(lineContent)) {
mistakes.push({ path: currentFile, line: lineNumber, ...result })
}
} else if (!rawLine.startsWith('-')) {
// Context line -- still advances the new-file line number
Expand Down
26 changes: 19 additions & 7 deletions .github/scripts/pr-review.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ const { GITHUB_TOKEN, REPO, BASE_REF, PR_NUMBER, HEAD_SHA } = process.env

const BOT_USER = 'github-actions[bot]'

/**
* Builds the comment body for a mistake. If the mistake includes a suggestion,
* appends a GitHub suggestion block so the author can apply the fix in one click.
*
* @param {{ message: string, suggestion?: string }} mistake
* @returns {string}
*/
function commentBody({ message, suggestion }) {
if (!suggestion) return message
return `${message}\n\n\`\`\`suggestion\n${suggestion}\n\`\`\``
}

async function githubFetch(path, options = {}) {
const response = await fetch(`https://api.github.com${path}`, {
...options,
Expand Down Expand Up @@ -56,7 +68,7 @@ const botComments = existingComments
const staleComments = botComments.filter(
(c) =>
!mistakes.some(
(m) => m.path === c.path && m.line === c.line && m.message === c.body
(m) => m.path === c.path && m.line === c.line && commentBody(m) === c.body
)
)

Expand Down Expand Up @@ -107,16 +119,16 @@ if (mistakes.length === 0) {
// Post new comments for mistakes that don't already have a comment
const newComments = mistakes
.filter(
({ path, line, message }) =>
(m) =>
!botComments.some(
(c) => c.path === path && c.line === line && c.body === message
(c) => c.path === m.path && c.line === m.line && c.body === commentBody(m)
)
)
.map(({ path, line, message }) => ({
path,
line,
.map((m) => ({
path: m.path,
line: m.line,
side: 'RIGHT',
body: message
body: commentBody(m)
}))

if (newComments.length === 0) {
Expand Down