Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Configure severity with unified-lint-rule #67

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Configure severity with unified-lint-rule #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0521f16
Configure severity with unified-lint-rule
jablko Mar 3, 2022
8afd265
Cherry pick https://github.com/remarkjs/remark-lint/pull/283
jablko Mar 3, 2022
a914756
Work around optional parameters without void
jablko Mar 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ jobs:
with:
node-version: ${{matrix.node}}
- run: npm install
- name: Cherry pick https://github.com/remarkjs/remark-lint/pull/283
run: |
curl https://patch-diff.githubusercontent.com/raw/remarkjs/remark-lint/pull/283.patch |
git apply -p 3 --ignore-space-change --directory node_modules/unified-lint-rule
- run: npm test
- uses: codecov/codecov-action@v1
strategy:
Expand Down
48 changes: 32 additions & 16 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
* Otherwise, use `urlConfig` to specify this manually.
*/

import {lintRule} from 'unified-lint-rule'
import {check} from './check/index.js'
import {find} from './find/index.js'
import {constants} from './constants.js'
Expand All @@ -51,30 +52,45 @@ cliCompleter.pluginId = constants.sourceId
* Plugin to validate that Markdown links and images reference existing local
* files and headings.
*
* @type {import('unified').Plugin<[Options?, FileSet?]|[Options?]|void[], Root>}
* @typedef {import('unified-lint-rule').Label|import('unified-lint-rule').Severity} Severity
* @type {import('unified').Plugin<[(Options|[Severity, Options?])?, FileSet?], Root>}
* https://github.com/microsoft/TypeScript/pull/48132
* @param [rawOptions]
* @param [fileSet]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The typedef should not be in this block: that makes the description belong to it. The description belongs to the plugin. You can move the typedef up.
  • Severity and label are different things as types, severity does not encompass both, so I think it’s better to not import both as one name, but import them separately under their own names
  • Add type for rules' callback parameter remark-lint#283 (comment)

*/
export default function remarkValidateLinks(options, fileSet) {
export default function remarkValidateLinks(rawOptions, fileSet) {
// Attach a `completer`.
if (fileSet) {
fileSet.use(cliCompleter)
}

// Find references and landmarks.
return (tree, file, next) => {
find.run(
{tree, file, fileSet, options: {...options}},
/** @type {Callback} */
(error) => {
if (error) {
next(error)
} else if (fileSet) {
next()
} else {
checkAll([file], next)
return lintRule(
{
origin: 'validate-links',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This most likely prevents ruleIds from being set. We currently use different ruleIds, and this overwrites them:

remark-validate-links/lib/check/validate.js

Lines 73 to 88 in 78871b0

if (hash) {
reason = 'Link to unknown heading'
ruleId = constants.headingRuleId
if (base && path.join(base, filePath) !== absolute) {
reason += ' in `' + filePath + '`'
ruleId = constants.headingInFileRuleId
}
reason += ': `' + hash + '`'
} else {
reason = 'Link to unknown file: `' + filePath + '`'
ruleId = constants.fileRuleId
}
const origin = [constants.sourceId, ruleId].join(':')
.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s likely not correct to use use lintRule. That’s meant for one rule (one reason for emitting things). This package supports different reasons.
Perhaps coerce should be used, and exposed from unified-lint-rule?

url: 'https://github.com/remarkjs/remark-validate-links#readme'
},
/** @type {import('unified-lint-rule').Rule<Root, Options>} */ (
tree,
file,
options,
next
) => {
find.run(
{tree, file, fileSet, options: {...options}},
/** @type {Callback} */
(error) => {
if (error) {
next(error)
} else if (fileSet) {
next()
} else {
checkAll([file], next)
}
}
}
)
}
)
}
).call(this, rawOptions)
}

/**
Expand Down
71 changes: 29 additions & 42 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,50 +447,37 @@ test('remark-validate-links', async (t) => {
rimraf.sync('./.git')

fs.renameSync('../../.git', '../../.git.bak')

try {
await exec(
[
bin,
'--no-config',
'--no-ignore',
'--use',
'../../index.js',
'--use',
'../sort.js',
'github.md'
].join(' ')
)
} catch (error) {
const exception = /** @type {ExecError} */ (error)
fs.renameSync('../../.git.bak', '../../.git')
t.ok(
/not a git repository/i.test(String(exception)),
'should fail w/o Git repository'
)
}
;({stderr} = await exec(
[
bin,
'--no-config',
'--no-ignore',
'--use',
'../../index.js',
'--use',
'../sort.js',
'github.md'
].join(' ')
))
fs.renameSync('../../.git.bak', '../../.git')
t.ok(/not a git repository/i.test(stderr), 'should fail w/o Git repository')

await exec('git init')

try {
await exec(
[
bin,
'--no-config',
'--no-ignore',
'--use',
'../../index.js',
'github.md'
].join(' ')
)
} catch (error) {
const exception = /** @type {ExecError} */ (error)
rimraf.sync('./.git')
t.ok(
/Could not find remote origin/.test(String(exception)),
'should fail w/o Git repository w/o remote'
)
}
;({stderr} = await exec(
[
bin,
'--no-config',
'--no-ignore',
'--use',
'../../index.js',
'github.md'
].join(' ')
))
rimraf.sync('./.git')
t.ok(
/Could not find remote origin/.test(stderr),
'should fail w/o Git repository w/o remote'
)

fs.renameSync('../../.git', '../../.git.bak')
;({stderr} = await exec(
Expand Down