Check PR links in every version the changed page supports (#63993) · github/docs@5d8713d · GitHub
Skip to content

Commit 5d8713d

Browse files
heiskrCopilot
andauthored
Check PR links in every version the changed page supports (#63993)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bf5940de-658e-4a1a-bffa-3018fb78b138
1 parent 6622cb9 commit 5d8713d

5 files changed

Lines changed: 189 additions & 42 deletions

File tree

‎src/links/lib/extract-links.ts‎

Lines changed: 5 additions & 0 deletions

‎src/links/lib/link-report.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,47 @@ export function mergeInternalLinkReports(
433433
return { ...report, versionsChecked, summary: report.summary + scope }
434434
}
435435

436+
// One href can redirect in one version and miss in another, so a missing target outranks a
437+
// redirect warning and the PR check still fails on it.
438+
export function mergeVersionedLinkFlaws(flaws: BrokenLink[]): {
439+
brokenLinks: BrokenLink[]
440+
redirectLinks: BrokenLink[]
441+
} {
442+
const merged = new Map<string, BrokenLink>()
443+
444+
for (const flaw of flaws) {
445+
const key = `${flaw.href}\u0000${flaw.file}`
446+
const existing = merged.get(key)
447+
const lines = [...new Set([...(existing?.lines ?? []), ...flaw.lines])].sort((a, b) => a - b)
448+
if (!existing) {
449+
merged.set(key, { ...flaw, lines })
450+
} else if (existing.isRedirect && !flaw.isRedirect) {
451+
const { href, file, text, isAutotitle } = existing
452+
merged.set(key, { href, file, lines, text, isAutotitle })
453+
} else {
454+
existing.lines = lines
455+
if (existing.isRedirect && flaw.isRedirect) {
456+
existing.requiresVersionContext =
457+
existing.requiresVersionContext || flaw.requiresVersionContext
458+
if (
459+
existing.redirectTarget &&
460+
flaw.redirectTarget &&
461+
!sameDestination(existing.redirectTarget, flaw.redirectTarget)
462+
) {
463+
existing.hasConflictingRedirectTargets = true
464+
}
465+
existing.redirectTarget = existing.redirectTarget ?? flaw.redirectTarget
466+
}
467+
}
468+
}
469+
470+
const all = [...merged.values()]
471+
return {
472+
brokenLinks: all.filter((flaw) => !flaw.isRedirect),
473+
redirectLinks: all.filter((flaw) => flaw.isRedirect),
474+
}
475+
}
476+
436477
export function generateInternalLinkReport(
437478
brokenLinks: BrokenLink[],
438479
options: {

‎src/links/scripts/check-links-pr.ts‎

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {
1919
checkAssetLink,
2020
isAssetLink,
2121
getRelativePath,
22+
getLinkCheckVersions,
23+
type ExtractedLink,
2224
} from '@/links/lib/extract-links'
2325
import {
2426
buildRenderContext,
@@ -33,6 +35,7 @@ import {
3335
type CrossPageAnchorFlaw,
3436
generatePRComment,
3537
groupBrokenLinks,
38+
mergeVersionedLinkFlaws,
3639
} from '@/links/lib/link-report'
3740
import { uploadArtifact } from '@/links/scripts/upload-artifact'
3841
import github from '@/workflows/github'
@@ -50,67 +53,72 @@ interface CheckResult {
5053

5154
async function checkFile(
5255
filePath: string,
56+
sourcePage: Page | undefined,
5357
pageMap: Record<string, Page>,
5458
redirects: Record<string, string>,
55-
version: string = 'free-pro-team@latest',
5659
language: string = 'en',
5760
): Promise<CheckResult> {
58-
const brokenLinks: BrokenLink[] = []
59-
const redirectLinks: BrokenLink[] = []
6061
let totalLinksChecked = 0
6162

6263
let content: string
6364
try {
6465
content = fs.readFileSync(filePath, 'utf-8')
6566
} catch {
6667
console.warn(`Could not read file: ${filePath}`)
67-
return { file: filePath, brokenLinks, redirectLinks, totalLinksChecked }
68+
return { file: filePath, brokenLinks: [], redirectLinks: [], totalLinksChecked }
6869
}
6970

70-
const context = createLiquidContext(version, language)
71+
const file = getRelativePath(filePath)
72+
const flaws: BrokenLink[] = []
73+
// Liquid shifts rendered lines, so use raw source lines unless a Liquid-built href has none.
74+
const flawFor = (link: ExtractedLink): BrokenLink => {
75+
const lines = findLinkLines(
76+
content,
77+
link.fragment ? `${link.href}#${link.fragment}` : link.href,
78+
)
79+
return {
80+
href: link.href,
81+
file,
82+
lines: lines.length ? lines : [link.line],
83+
text: link.text,
84+
isAutotitle: link.isAutotitle,
85+
}
86+
}
7187

72-
const { internalLinks } = await extractLinksWithLiquid(content, context)
88+
// ifversion blocks and versionless links depend on the version, so check each one.
89+
for (const version of getLinkCheckVersions(sourcePage)) {
90+
// Pages use the anchor check's context so version names and feature flags evaluate like the site.
91+
const context = sourcePage
92+
? buildRenderContext(sourcePage, version, pageMap, redirects)
93+
: createLiquidContext(version, language)
7394

74-
// imageLinks are excluded: they're static assets, not docs pages.
75-
totalLinksChecked = internalLinks.length
95+
const { internalLinks } = await extractLinksWithLiquid(content, context)
7696

77-
for (const link of internalLinks) {
78-
if (isAssetLink(link.href)) {
79-
if (!checkAssetLink(link.href)) {
80-
brokenLinks.push({
81-
href: link.href,
82-
file: getRelativePath(filePath),
83-
lines: [link.line],
84-
text: link.text,
85-
isAutotitle: link.isAutotitle,
86-
})
97+
// imageLinks are excluded: they're static assets, not docs pages.
98+
totalLinksChecked = Math.max(totalLinksChecked, internalLinks.length)
99+
100+
for (const link of internalLinks) {
101+
if (isAssetLink(link.href)) {
102+
if (!checkAssetLink(link.href)) flaws.push(flawFor(link))
103+
continue
87104
}
88-
continue
89-
}
90105

91-
const result = checkInternalLink(link.href, pageMap, redirects)
92-
93-
if (!result.exists) {
94-
brokenLinks.push({
95-
href: link.href,
96-
file: getRelativePath(filePath),
97-
lines: [link.line],
98-
text: link.text,
99-
isAutotitle: link.isAutotitle,
100-
})
101-
} else if (result.isRedirect) {
102-
redirectLinks.push({
103-
href: link.href,
104-
file: getRelativePath(filePath),
105-
lines: [link.line],
106-
text: link.text,
107-
isAutotitle: link.isAutotitle,
108-
isRedirect: true,
109-
redirectTarget: result.redirectTarget,
110-
})
106+
const result = checkInternalLink(link.href, pageMap, redirects, version, language)
107+
108+
if (!result.exists) {
109+
flaws.push(flawFor(link))
110+
} else if (result.isRedirect) {
111+
flaws.push({
112+
...flawFor(link),
113+
isRedirect: true,
114+
redirectTarget: result.redirectTarget,
115+
requiresVersionContext: result.requiresVersionContext,
116+
})
117+
}
111118
}
112119
}
113120

121+
const { brokenLinks, redirectLinks } = mergeVersionedLinkFlaws(flaws)
114122
return { file: filePath, brokenLinks, redirectLinks, totalLinksChecked }
115123
}
116124

@@ -369,13 +377,13 @@ async function main() {
369377
console.log(` Checking: ${file}`)
370378
}
371379

372-
const result = await checkFile(filePath, pageMap, redirects)
380+
const sourcePage = pageByRelativePath.get(getRelativePath(filePath))
381+
const result = await checkFile(filePath, sourcePage, pageMap, redirects)
373382
allBrokenLinks.push(...result.brokenLinks)
374383
allRedirectLinks.push(...result.redirectLinks)
375384
totalLinksChecked += result.totalLinksChecked
376385

377386
// Skip changed files outside pageByRelativePath because data and reusables have no versions of their own.
378-
const sourcePage = pageByRelativePath.get(getRelativePath(filePath))
379387
if (checkAnchors && sourcePage) {
380388
allBrokenAnchors.push(
381389
...(await checkFileAnchors(filePath, sourcePage, pageMap, redirects, headingCache)),

‎src/links/tests/extract-links.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import {
66
resolveInternalLinkKey,
77
checkAssetLink,
88
isAssetLink,
9+
getLinkCheckVersions,
910
createLiquidContext,
1011
extractLinksWithLiquid,
1112
} from '../lib/extract-links'
1213
import { latest, latestStable } from '@/versions/lib/enterprise-server-releases'
14+
import { allVersions } from '@/versions/lib/all-versions'
1315
import { getFeaturesByVersion } from '@/versions/middleware/features'
1416
import type { Page } from '@/types'
1517

@@ -973,6 +975,33 @@ describe('resolveInternalLinkKey version precedence', () => {
973975
})
974976
})
975977

978+
describe('getLinkCheckVersions', () => {
979+
test('returns every version the page applies to', () => {
980+
const versions = ['free-pro-team@latest', 'enterprise-cloud@latest']
981+
expect(getLinkCheckVersions({ applicableVersions: versions })).toEqual(versions)
982+
})
983+
984+
test('returns every current version for files without a page', () => {
985+
expect(getLinkCheckVersions(undefined)).toEqual(Object.keys(allVersions))
986+
expect(getLinkCheckVersions({ applicableVersions: [] })).toEqual(Object.keys(allVersions))
987+
})
988+
989+
test('resolves versionless links on GHEC-only pages without a redirect', () => {
990+
// Pages without FPT get a versionless fallback redirect to their first version.
991+
const pageMap = {
992+
'/en/enterprise-cloud@latest/admin/create-account': {} as unknown as Page,
993+
}
994+
const redirects = {
995+
'/admin/create-account': '/enterprise-cloud@latest/admin/create-account',
996+
}
997+
const versions = getLinkCheckVersions({ applicableVersions: ['enterprise-cloud@latest'] })
998+
for (const version of versions) {
999+
const result = checkInternalLink('/admin/create-account', pageMap, redirects, version)
1000+
expect(result).toEqual({ exists: true, isRedirect: false })
1001+
}
1002+
})
1003+
})
1004+
9761005
describe('extractLinksWithLiquid with createLiquidContext', () => {
9771006
const fptFeatures = getFeaturesByVersion('free-pro-team@latest')
9781007
const fptFeature = Object.keys(fptFeatures).find((name) => fptFeatures[name])

‎src/links/tests/link-report.ts‎

Lines changed: 64 additions & 0 deletions

0 commit comments

Comments
 (0)