diff --git a/.eslintrc.js b/.eslintrc.js index bfe514c1b20..4856ed4bcdb 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -31,9 +31,11 @@ const config = { }, ], }, + plugins: ['@sourcegraph/sourcegraph'], rules: { // Rules that are specific to this repo // All other rules should go into https://github.com/sourcegraph/eslint-config + '@sourcegraph/sourcegraph/check-help-links': 'error', 'no-restricted-imports': [ 'error', { diff --git a/client/packages/@sourcegraph/eslint-plugin-sourcegraph/.gitignore b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/.gitignore new file mode 100644 index 00000000000..8ee01d321b7 --- /dev/null +++ b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/.gitignore @@ -0,0 +1 @@ +yarn.lock diff --git a/client/packages/@sourcegraph/eslint-plugin-sourcegraph/README.md b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/README.md new file mode 100644 index 00000000000..21f35788c25 --- /dev/null +++ b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/README.md @@ -0,0 +1,30 @@ +# eslint-plugin-sourcegraph + +Custom ESLint rules for Sourcegraph. This package should only be used within +the main Sourcegraph project, and isn't intended for reuse by other packages in +the Sourcegraph organisation. + +## Rules + +Rules are defined in `lib/rules`. At present, one rule is available. + +### `check-help-links` + +This rule parses `Link` and `a` elements in JSX/TSX files. If a list of valid +docsite pages is provided, elements that point to a `/help/*` link are checked +against that list: if they don't exist, a linting error is raised. + +The list of docsite pages is provided either via the `DOCSITE_LIST` environment +variable, which should be a newline separated list of pages as outputted by +`docsite ls`, or via the `docsiteList` rule option, which is the same data as +an array. + +If neither of these are set, then the rule will silently succeed. + +## Testing + +Unit tests can be run with: + +```sh +yarn test +``` diff --git a/client/packages/@sourcegraph/eslint-plugin-sourcegraph/lib/index.js b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/lib/index.js new file mode 100644 index 00000000000..986df6dbd61 --- /dev/null +++ b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/lib/index.js @@ -0,0 +1,14 @@ +'use strict' + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var requireIndex = require('requireindex') + +//------------------------------------------------------------------------------ +// Plugin Definition +//------------------------------------------------------------------------------ + +// import all rules in lib/rules +module.exports.rules = requireIndex(__dirname + '/rules') diff --git a/client/packages/@sourcegraph/eslint-plugin-sourcegraph/lib/rules/check-help-links.js b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/lib/rules/check-help-links.js new file mode 100644 index 00000000000..8b07e67b06c --- /dev/null +++ b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/lib/rules/check-help-links.js @@ -0,0 +1,101 @@ +'use strict' + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Check that /help links point to real, non-redirected pages', + category: 'Best Practices', + recommended: false, + }, + schema: [ + { + type: 'object', + properties: { + docsiteList: { + type: 'array', + items: { + type: 'string', + }, + }, + }, + additionalProperties: false, + }, + ], + }, + + create: function (context) { + // Build the set of valid pages. In order, we'll try to get this from: + // + // 1. The DOCSITE_LIST environment variable, which should be a newline + // separated list of pages, as outputted by `docsite ls`. + // 2. The docsiteList rule option, which should be an array of pages. + // + // If neither of these are set, this rule will silently pass, so as not to + // require docsite to be run when a user wants to run eslint in general. + const pages = new Set() + if (process.env.DOCSITE_LIST) { + process.env.DOCSITE_LIST.split('\n').forEach(page => pages.add(page)) + } else if (context.options.length > 0) { + context.options[0].docsiteList.forEach(page => pages.add(page)) + } + + // No pages were provided, so we'll return an empty object and do nothing. + if (pages.size === 0) { + return {} + } + + // Return the object that will install the listeners we want. In this case, + // we only need to look at JSX opening elements. + // + // Note that we could use AST selectors below, but the structure of the AST + // makes that tricky: the identifer (Link or a) and attribute (to or href) + // we use to identify an element of interest are siblings, so we'd probably + // have to select on the identifier and have some ugly traversal code below + // to check the attribute. It feels cleaner to do it this way with the + // opening element as the context. + return { + JSXOpeningElement: node => { + // Figure out what kind of element we have and therefore what attribute + // we'd want to look for. + let attrName + if (node.name.name === 'Link') { + attrName = 'to' + } else if (node.name.name === 'a') { + attrName = 'href' + } else { + // Anything that's not a link is uninteresting. + return + } + + // Go find the link target in the attribute array. + const target = node.attributes.reduce( + (target, attr) => target ?? (attr.name && attr.name.name === attrName ? attr.value.value : undefined), + undefined + ) + + // Make sure the target points to a help link; if not, we don't need to + // go any further. + if (!target || !target.startsWith('/help/')) { + return + } + + // Strip off the /help/ prefix, any anchor, and any trailing slash, then + // look up the resultant page in the pages set, bearing in mind that it + // might point to a directory and we also need to look for any index + // page that might exist. + const destination = target.substring(6).split('#')[0].replace(/\/+$/, '') + if (!pages.has(destination + '.md') && !pages.has(destination + '/index.md')) { + context.report({ + node, + message: 'Help link to non-existent page: {{ destination }}', + data: { destination }, + }) + } + }, + } + }, +} diff --git a/client/packages/@sourcegraph/eslint-plugin-sourcegraph/package.json b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/package.json new file mode 100644 index 00000000000..4915ffcbe16 --- /dev/null +++ b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/package.json @@ -0,0 +1,22 @@ +{ + "name": "eslint-plugin-sourcegraph", + "version": "0.0.0", + "description": "Custom ESLint rules for Sourcegraph", + "keywords": ["eslint", "eslintplugin", "eslint-plugin"], + "main": "lib/index.js", + "scripts": { + "eslint": "../../../../node_modules/.bin/eslint", + "test": "mocha tests --recursive" + }, + "dependencies": { + "requireindex": "~1.1.0" + }, + "devDependencies": { + "eslint": "^7.1.0", + "mocha": "^7.2.0" + }, + "engines": { + "node": ">=0.10.0" + }, + "license": "Apache-2.0" +} diff --git a/client/packages/@sourcegraph/eslint-plugin-sourcegraph/tests/lib/rules/check-help-links.test.js b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/tests/lib/rules/check-help-links.test.js new file mode 100644 index 00000000000..81956370e8e --- /dev/null +++ b/client/packages/@sourcegraph/eslint-plugin-sourcegraph/tests/lib/rules/check-help-links.test.js @@ -0,0 +1,81 @@ +'use strict' + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require('../../../lib/rules/check-help-links'), + RuleTester = require('eslint').RuleTester + +// Set up the configuration such that JSX is valid. +RuleTester.setDefaultConfig({ + parserOptions: { + ecmaVersion: 6, + ecmaFeatures: { + jsx: true, + }, + }, +}) + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester() +const invalidLinkError = path => { + return { message: 'Help link to non-existent page: ' + path, type: 'JSXOpeningElement' } +} +const options = [{ docsiteList: ['a.md', 'b/c.md', 'd/index.md'] }] + +// Build up the test cases given the various combinations we need to support. +const cases = { valid: [], invalid: [] } +for (const [element, attribute] of [ + ['a', 'href'], + ['Link', 'to'], +]) { + for (const anchor of ['', '#anchor', '#anchor#double']) { + for (const content of ['', 'link content']) { + const code = target => + content + ? `<${element} ${attribute}="${target}${anchor}">${content}` + : `<${element} ${attribute}="${target}${anchor}" />` + + cases.valid.push( + ...[ + '/help/a', + '/help/b/c', + '/help/d', + '/help/d/', + 'not-a-help-link', + 'help/but-not-absolute', + '/help-but-not-a-directory', + ].map(target => { + return { + code: code(target), + options, + } + }) + ) + + cases.invalid.push( + ...['/help/', '/help/b', '/help/does/not/exist'].map(target => { + return { + code: code(target), + errors: [invalidLinkError(target.substring(6))], + options, + } + }) + ) + } + } +} + +// Every case should be valid if the options are empty. +cases.valid.push( + ...[...cases.invalid, ...cases.valid].map(({ code }) => { + return { code } + }) +) + +// Actually run the tests. +ruleTester.run('check-help-links', rule, cases) diff --git a/dev/foreach-ts-project.sh b/dev/foreach-ts-project.sh index 2e18fe3603d..33f17c06567 100755 --- a/dev/foreach-ts-project.sh +++ b/dev/foreach-ts-project.sh @@ -16,6 +16,7 @@ DIRS=( client/branded client/browser client/packages/sourcegraph-extension-api + client/packages/@sourcegraph/eslint-plugin-sourcegraph client/packages/@sourcegraph/extension-api-types dev/release dev/ts-morph diff --git a/package.json b/package.json index bc5c9e9d01b..fe8a468f55a 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "scripts": { "prettier": "prettier '**/{*.{js?(on),ts?(x),graphql,md,scss},.*.js?(on)}' --write --list-different --config prettier.config.js", "prettier-check": "yarn -s run prettier --write=false", - "all:eslint": "dev/foreach-ts-project.sh yarn -s run eslint --quiet", + "all:eslint": "DOCSITE_LIST=\"$(./dev/docsite.sh -config doc/docsite.json ls)\" dev/foreach-ts-project.sh yarn -s run eslint --quiet", "all:stylelint": "yarn --cwd client/web run stylelint && yarn --cwd client/shared run stylelint && yarn --cwd client/branded run stylelint && yarn --cwd client/browser run stylelint", "all:tsgql": "yarn --cwd client/web run tsgql validate -p . --exitOnWarn && yarn --cwd client/shared run tsgql validate -p . --exitOnWarn && yarn --cwd client/browser run tsgql validate -p . --exitOnWarn", "build-ts": "tsc -b tsconfig.all.json", @@ -95,6 +95,7 @@ "@slack/web-api": "^5.10.0", "@sourcegraph/babel-plugin-transform-react-hot-loader-wrapper": "^1.0.0", "@sourcegraph/eslint-config": "^0.20.19", + "@sourcegraph/eslint-plugin-sourcegraph": "link:client/packages/@sourcegraph/eslint-plugin-sourcegraph", "@sourcegraph/prettierrc": "^3.0.3", "@sourcegraph/stylelint-config": "^1.1.9", "@sourcegraph/tsconfig": "^4.0.1", diff --git a/yarn.lock b/yarn.lock index 6fc86ff3a01..67a440683b0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2680,6 +2680,10 @@ eslint-plugin-rxjs "^2.1.5" eslint-plugin-unicorn "^21.0.0" +"@sourcegraph/eslint-plugin-sourcegraph@link:client/packages/@sourcegraph/eslint-plugin-sourcegraph": + version "0.0.0" + uid "" + "@sourcegraph/event-positions@^1.0.4": version "1.0.4" resolved "https://registry.npmjs.org/@sourcegraph/event-positions/-/event-positions-1.0.4.tgz#031541374b725cfb0cbefaa07add020b1a119a60" @@ -18779,6 +18783,11 @@ require-uncached@^1.0.2: caller-path "^0.1.0" resolve-from "^1.0.0" +requireindex@~1.1.0: + version "1.1.0" + resolved "https://registry.npmjs.org/requireindex/-/requireindex-1.1.0.tgz#e5404b81557ef75db6e49c5a72004893fe03e162" + integrity sha1-5UBLgVV+91225JxacgBIk/4D4WI= + requireindex@~1.2.0: version "1.2.0" resolved "https://registry.npmjs.org/requireindex/-/requireindex-1.2.0.tgz#3463cdb22ee151902635aa6c9535d4de9c2ef1ef"