From af1859478afd7e120b1b40e71891560ec428c365 Mon Sep 17 00:00:00 2001 From: HiDeoo <494699+HiDeoo@users.noreply.github.com> Date: Tue, 12 Dec 2023 20:03:58 +0100 Subject: [PATCH] feat!: add errors for relative internal links --- docs/src/content/docs/configuration.md | 24 +++++++++++++++++++ packages/starlight-links-validator/index.ts | 6 +++++ .../starlight-links-validator/libs/remark.ts | 2 +- .../libs/validation.ts | 10 +++++++- .../tests/basics.test.ts | 13 ++++++++-- .../src/content/docs/relative.md | 9 +++++++ .../fixtures/relative-ignore/astro.config.ts | 13 ++++++++++ .../src/content/docs/guides/example.md | 7 ++++++ .../src/content/docs/relative.md | 9 +++++++ .../relative-ignore/src/content/docs/test.md | 19 +++++++++++++++ .../tests/relative.test.ts | 21 ++++++++++++++++ .../tests/trailing.test.ts | 4 ++-- 12 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 packages/starlight-links-validator/tests/fixtures/basics-invalid-links/src/content/docs/relative.md create mode 100644 packages/starlight-links-validator/tests/fixtures/relative-ignore/astro.config.ts create mode 100644 packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/guides/example.md create mode 100644 packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/relative.md create mode 100644 packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/test.md create mode 100644 packages/starlight-links-validator/tests/relative.test.ts diff --git a/docs/src/content/docs/configuration.md b/docs/src/content/docs/configuration.md index c7eef12..d972859 100644 --- a/docs/src/content/docs/configuration.md +++ b/docs/src/content/docs/configuration.md @@ -51,3 +51,27 @@ export default defineConfig({ ], }) ``` + +### `errorOnRelativeLinks` + +**Type:** `boolean` +**Default:** `true` + +Relative internal links, such as `./test` or `../test`, are usually considered confusing as they can be difficult to reason about, figure out where they point to and require more maintenance when a page is moved. + +By default, the Starlight Links Validator plugin will error if a relative internal link is found. +If you want to allow relative internal links, you can set this option to `false` but note that theses links will not be validated. + +```js {6} +export default defineConfig({ + integrations: [ + starlight({ + plugins: [ + starlightLinksValidator({ + errorOnRelativeLinks: false, + }), + ], + }), + ], +}) +``` diff --git a/packages/starlight-links-validator/index.ts b/packages/starlight-links-validator/index.ts index 3ec1edc..0224a33 100644 --- a/packages/starlight-links-validator/index.ts +++ b/packages/starlight-links-validator/index.ts @@ -17,6 +17,12 @@ const starlightLinksValidatorOptionsSchema = z * @see https://starlight.astro.build/guides/i18n/#fallback-content */ errorOnFallbackPages: z.boolean().default(true), + /** + * Defines whether the plugin should error on internal relative links. + * + * When set to `false`, the plugin will ignore relative links (e.g. `./foo` or `../bar`). + */ + errorOnRelativeLinks: z.boolean().default(true), }) .default({}) diff --git a/packages/starlight-links-validator/libs/remark.ts b/packages/starlight-links-validator/libs/remark.ts index 92b2bec..6fccc77 100644 --- a/packages/starlight-links-validator/libs/remark.ts +++ b/packages/starlight-links-validator/libs/remark.ts @@ -131,7 +131,7 @@ export function getValidationData() { } function isInternalLink(link: string) { - return nodePath.isAbsolute(link) || link.startsWith('#') + return nodePath.isAbsolute(link) || link.startsWith('#') || link.startsWith('.') } function normalizeFilePath(filePath?: string) { diff --git a/packages/starlight-links-validator/libs/validation.ts b/packages/starlight-links-validator/libs/validation.ts index a21a097..e7ecaa8 100644 --- a/packages/starlight-links-validator/libs/validation.ts +++ b/packages/starlight-links-validator/libs/validation.ts @@ -84,7 +84,7 @@ export function logErrors(errors: ValidationErrors) { * Validate a link to another internal page that may or may not have a hash. */ function validateLink(context: ValidationContext) { - const { errors, filePath, link, outputDir, pages } = context + const { errors, filePath, link, options, outputDir, pages } = context const sanitizedLink = link.replace(/^\//, '') const segments = sanitizedLink.split('#') @@ -96,6 +96,14 @@ function validateLink(context: ValidationContext) { throw new Error('Failed to validate a link with no path.') } + if (path.startsWith('.')) { + if (options.errorOnRelativeLinks) { + addError(errors, filePath, link) + } + + return + } + if (isValidAsset(path, outputDir)) { return } diff --git a/packages/starlight-links-validator/tests/basics.test.ts b/packages/starlight-links-validator/tests/basics.test.ts index e659335..4367bef 100644 --- a/packages/starlight-links-validator/tests/basics.test.ts +++ b/packages/starlight-links-validator/tests/basics.test.ts @@ -11,12 +11,12 @@ test('should build with valid links', async () => { }) test('should not build with invalid links', async () => { - expect.assertions(4) + expect.assertions(5) try { await loadFixture('basics-invalid-links') } catch (error) { - expect(error).toMatch(/Found 20 invalid links in 3 files./) + expect(error).toMatch(/Found 25 invalid links in 4 files./) expect(error).toMatch( new RegExp(`▶ test/ @@ -49,5 +49,14 @@ test('should not build with invalid links', async () => { ├─ #some-other-content └─ /guides/namespacetest/#another-content`), ) + + expect(error).toMatch( + new RegExp(`▶ relative/ + ├─ . + ├─ ./relative + ├─ ./test + ├─ ./guides/example + └─ ../test`), + ) } }) diff --git a/packages/starlight-links-validator/tests/fixtures/basics-invalid-links/src/content/docs/relative.md b/packages/starlight-links-validator/tests/fixtures/basics-invalid-links/src/content/docs/relative.md new file mode 100644 index 0000000..af5f41b --- /dev/null +++ b/packages/starlight-links-validator/tests/fixtures/basics-invalid-links/src/content/docs/relative.md @@ -0,0 +1,9 @@ +--- +title: Relative +--- + +- [Link to the same page](.) +- [Link to the same page with its name](./relative) +- [Link to another page in the same directory](./test) +- [Link to another page in another directory](./guides/example) +- [Link to another page in a parent directory](../test) diff --git a/packages/starlight-links-validator/tests/fixtures/relative-ignore/astro.config.ts b/packages/starlight-links-validator/tests/fixtures/relative-ignore/astro.config.ts new file mode 100644 index 0000000..3625663 --- /dev/null +++ b/packages/starlight-links-validator/tests/fixtures/relative-ignore/astro.config.ts @@ -0,0 +1,13 @@ +import starlight from '@astrojs/starlight' +import { defineConfig } from 'astro/config' + +import starlightLinksValidator from '../..' + +export default defineConfig({ + integrations: [ + starlight({ + plugins: [starlightLinksValidator({ errorOnRelativeLinks: false })], + title: 'Starlight Links Validator Tests - relative ignore', + }), + ], +}) diff --git a/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/guides/example.md b/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/guides/example.md new file mode 100644 index 0000000..e78ca68 --- /dev/null +++ b/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/guides/example.md @@ -0,0 +1,7 @@ +--- +title: Example +--- + +## Description + +This is an example page. diff --git a/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/relative.md b/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/relative.md new file mode 100644 index 0000000..af5f41b --- /dev/null +++ b/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/relative.md @@ -0,0 +1,9 @@ +--- +title: Relative +--- + +- [Link to the same page](.) +- [Link to the same page with its name](./relative) +- [Link to another page in the same directory](./test) +- [Link to another page in another directory](./guides/example) +- [Link to another page in a parent directory](../test) diff --git a/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/test.md b/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/test.md new file mode 100644 index 0000000..e3a9cf4 --- /dev/null +++ b/packages/starlight-links-validator/tests/fixtures/relative-ignore/src/content/docs/test.md @@ -0,0 +1,19 @@ +--- +title: Test +--- + +# Some links + +- [External link](https://starlight.astro.build/) + +- [Example page](/guides/example) +- [Example page](/guides/example/) + +- [Example page with hash](/guides/example#description) +- [Example page with hash](/guides/example/#description) + +- [Unknown page](/unknown) +- [Unknown page](/unknown/) + +- [Example page with unknown hash](/guides/example#unknown) +- [Example page with unknown hash](/guides/example/#unknown) diff --git a/packages/starlight-links-validator/tests/relative.test.ts b/packages/starlight-links-validator/tests/relative.test.ts new file mode 100644 index 0000000..c8c2a5d --- /dev/null +++ b/packages/starlight-links-validator/tests/relative.test.ts @@ -0,0 +1,21 @@ +import { expect, test } from 'vitest' + +import { loadFixture } from './utils' + +test('should ignore relative links when the `errorOnRelativeLinks` option is set to `false`', async () => { + expect.assertions(2) + + try { + await loadFixture('relative-ignore') + } catch (error) { + expect(error).toMatch(/Found 4 invalid links in 1 file./) + + expect(error).toMatch( + new RegExp(`▶ test/ + ├─ /unknown + ├─ /unknown/ + ├─ /guides/example#unknown + └─ /guides/example/#unknown`), + ) + } +}) diff --git a/packages/starlight-links-validator/tests/trailing.test.ts b/packages/starlight-links-validator/tests/trailing.test.ts index 98ad477..7f696e0 100644 --- a/packages/starlight-links-validator/tests/trailing.test.ts +++ b/packages/starlight-links-validator/tests/trailing.test.ts @@ -2,7 +2,7 @@ import { expect, test } from 'vitest' import { loadFixture } from './utils' -test('should validate links when the `trailingSlash` option is set to `never`', async () => { +test('should validate links when the `trailingSlash` Astro option is set to `never`', async () => { expect.assertions(2) try { @@ -20,7 +20,7 @@ test('should validate links when the `trailingSlash` option is set to `never`', } }) -test('should validate links when the `trailingSlash` option is set to `always`', async () => { +test('should validate links when the `trailingSlash` Astro option is set to `always`', async () => { expect.assertions(2) try {