Skip to content

Commit

Permalink
Add additional search-replace rules and tests (#42826)
Browse files Browse the repository at this point in the history
Co-authored-by: Peter Bengtsson <[email protected]>
  • Loading branch information
rachmari and peterbe authored Sep 22, 2023
1 parent 72c5f0c commit 4819fa2
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/content-linter/lib/default-markdownlint-options.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export function testOptions(rule, module, { strings, files }) {
export function testOptions(rule, module, { strings, files, testConfig }) {
const config = {
default: false,
[rule]: true,
[rule]: testConfig || true,
}

const options = {
Expand Down
4 changes: 2 additions & 2 deletions src/content-linter/lib/helpers/get-rules.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import rules from '../../../../node_modules/markdownlint/lib/rules.js'
import { gitHubDocsMarkdownlint } from '../linting-rules/index.js'
import { baseConfig } from '../../style/base.js'
import { githubDocsConfig } from '../../style/github-docs.js'
import { githubDocsConfig, searchReplaceConfig } from '../../style/github-docs.js'

export const customRules = gitHubDocsMarkdownlint.rules
export const allRules = [...rules, ...gitHubDocsMarkdownlint.rules]
export const allConfig = { ...baseConfig, ...githubDocsConfig }
export const allConfig = { ...baseConfig, ...githubDocsConfig, ...searchReplaceConfig }
4 changes: 2 additions & 2 deletions src/content-linter/lib/init-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import markdownlint from 'markdownlint'

import { testOptions } from './default-markdownlint-options.js'

export async function runRule(module, { strings, files } = {}) {
export async function runRule(module, { strings, files, testConfig }) {
if ((!strings && !files) || (strings && files))
throw new Error('Must provide either Markdown strings or files to run a rule')

const options = testOptions(module.names[0], module, { strings, files })
const options = testOptions(module.names[0], module, { strings, files, testConfig })
return await markdownlint.promises.markdownlint(options)
}
63 changes: 60 additions & 3 deletions src/content-linter/style/github-docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,15 @@ export const githubDocsConfig = {
severity: 'error',
'partial-markdown-files': true,
},
}

export const searchReplaceConfig = {
'search-replace': {
rules: [
{
name: 'todocs-placeholder',
message: 'Catch occurrences of TODOCS placeholder.',
search: 'TODOCS',
searchPattern: '/todocs/gi',
searchScope: 'all',
severity: 'error',
'severity-local': 'warning',
Expand All @@ -62,7 +65,7 @@ export const githubDocsConfig = {
{
name: 'docs-domain',
message: 'Catch occurrences of docs.gitub.com domain.',
search: 'docs.gitub.com',
search: 'docs.github.com',
searchScope: 'all',
severity: 'error',
'partial-markdown-files': true,
Expand Down Expand Up @@ -90,11 +93,65 @@ export const githubDocsConfig = {
// developer.github.com/enterprise/[0-9] or
// developer.github.com/enterprise/{{something}} (e.g. liquid).
// There are occurences that will likely always remain in the content.
searchPattern: '/developer.github.com(?!/(changes|enterprise/([0-9]|{))).*/g',
searchPattern: '/developer\\.github\\.com(?!\\/(changes|enterprise\\/([0-9]|{))).*/g',
searchScope: 'all',
severity: 'error',
'partial-markdown-files': true,
},
{
// Catches usage of old liquid data reusable syntax. For example:
// {{ site.data.variables.product_releases }}
name: 'deprecated liquid syntax: site.data',
message: 'Catch occurrences of deprecated liquid data syntax.',
searchPattern: '/{{\\s*?site\\.data\\.([a-zA-Z0-9-_]+(?:\\.[a-zA-Z0-9-_]+)+)\\s*?}}/g',
replace: '{% data $1 %}',
},
{
// Catches usage of old octicon variable syntax. For example:
// - {{ octicon-plus }}
// - {{ octicon-plus An example label }}
name: 'deprecated liquid syntax: octicon-<icon-name>',
message:
'The octicon liquid syntax used is deprecated. Use this format instead {% octicon "<octicon-name>" aria-label="<Octicon aria label>" %}',
searchPattern: '/{{\\s*?octicon-([a-z-]+)(\\s[\\w\\s\\d-]+)?\\s*?}}/g',
},
{
// Catches usage of string personal access token, which should
// be replaced with a reusable data variable.
name: 'personal access token reusable',
message:
'The string "personal access token" can be replaced with a variable. You should use one of the variables from data/variables/product.yml instead of the literal phrase(s):',
searchPattern: '/personal access tokens?/gi',
},
{
// Catches usage of GitHub-owned actions that don't use a
// resuable.
// GitHub-owned actions (e.g. actions/checkout@v2) should use a
// reusable in examples.
//
// - actions/checkout@v2
// - actions/delete-package-versions@v2
// - actions/download-artifact@v2
// - actions/upload-artifact@v2
// - actions/github-script@v2
// - actions/setup-dotnet@v2
// - actions/setup-go@v2
// - actions/setup-java@v2
// - actions/setup-node@v2
// - actions/setup-python@v2
// - actions/stale@v2
// - actions/cache@v2
// - github/codeql-action/init@v2
// - github/codeql-action/analyze@v2
// - github/codeql-action/autobuild@v2
// - github/codeql-action/upload-sarif@v2
//
name: 'GitHub-owned action references should use a reusable',
message:
'A GitHub-owned action is referenced, but should be replaced with a reusable from data/reusables/actions.',
searchPattern:
'/(actions\\/(checkout|delete-package-versions|download-artifact|upload-artifact|github-script|setup-dotnet|setup-go|setup-java|setup-node|setup-python|stale|cache)|github\\/codeql-action[/a-zA-Z-]*)/g',
},
],
},
}
93 changes: 93 additions & 0 deletions src/content-linter/tests/unit/search-replace.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import searchReplace from 'markdownlint-rule-search-replace'

import { runRule } from '../../lib/init-test.js'
import { searchReplaceConfig } from '../../style/github-docs.js'

describe(searchReplace.names.join(' - '), () => {
test('TODOCS placeholder occurrences cause errors', async () => {
const markdown = [
'## TODOCS',
'- Todocs',
'[ToDOCS](/todocs)',
'![TODOCS](./ToDocs.png)',
'HelloTODOCS',
].join('\n')
const result = await runRule(searchReplace, {
strings: { markdown },
testConfig: searchReplaceConfig['search-replace'],
})
const errors = result.markdown
expect(errors.length).toBe(7)
})

test('docs domain occurrences cause error', async () => {
const markdown = [
'These are not ok:',
'docs.github.com',
'- help.github.com',
'[help.github.com](//developer.github.com)',
'![developer.github.com](//preview.ghdocs.com)',
' docs.github.com',
'developer.github.com/enterprise',
'developer.github.com/enterprise/',
'',
'These are ok:',
'developer.github.com/changes',
'developer.github.com/changes/',
'developer.github.com/changes/changes',
'developer.github.com/enterprise/1',
].join('\n')
const result = await runRule(searchReplace, {
strings: { markdown },
testConfig: searchReplaceConfig['search-replace'],
})
const errors = result.markdown
expect(errors.length).toBe(9)
})

test('Deprecated Liquid syntax causes error', async () => {
const markdown = [
'{{ site.data.thing1.thing2 }}',
'{{site.data.thing1.thing2}}',
'{{ octicon-plus An example label }}',
'{{octicon-icon}}',
].join('\n')
const result = await runRule(searchReplace, {
strings: { markdown },
testConfig: searchReplaceConfig['search-replace'],
})
const errors = result.markdown
expect(errors.length).toBe(4)
})

test('Using hardcoded personal access token string causes error', async () => {
const markdown = [
'Hello personal access token for apps.',
'A Personal access token for apps.',
'Lots of PERSONAL ACCESS TOKENS for apps.',
'access tokens for apps.',
].join('\n')
const result = await runRule(searchReplace, {
strings: { markdown },
testConfig: searchReplaceConfig['search-replace'],
})
const errors = result.markdown
expect(errors.length).toBe(3)
})

test('Using hardcoded personal access token string causes error', async () => {
const markdown = [
'Hello actions/checkout@v2 apps.',
'A actions/delete-package-versions@v2 for apps.',
'Hello actions/download-artifact@v2.',
'actions/cache@432433423423',
'actions/cache@',
].join('\n')
const result = await runRule(searchReplace, {
strings: { markdown },
testConfig: searchReplaceConfig['search-replace'],
})
const errors = result.markdown
expect(errors.length).toBe(5)
})
})

0 comments on commit 4819fa2

Please sign in to comment.