-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Add no-generic-link-text rule #10
Changes from all commits
b64b572
7f778c1
def43d6
24071c4
b908be6
550896e
aa094d7
0f42a59
85b876e
b63e5e2
4ce66a8
cce4a05
f5b1e48
7ebcdee
91d4422
1bf18ae
4269133
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/* Downcase and strip extra whitespaces and punctuation */ | ||
function stripAndDowncaseText(text) { | ||
return text | ||
.toLowerCase() | ||
.replace(/[.,/#!$%^&*;:{}=\-_`~()]/g, "") | ||
.replace(/\s+/g, " ") | ||
.trim(); | ||
} | ||
|
||
module.exports = { stripAndDowncaseText }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
const { stripAndDowncaseText } = require("./helpers/strip-and-downcase-text"); | ||
|
||
const bannedLinkText = [ | ||
"read more", | ||
"learn more", | ||
"more", | ||
"here", | ||
"click here", | ||
"link", | ||
]; | ||
|
||
module.exports = { | ||
names: ["GH002", "no-generic-link-text"], | ||
description: | ||
"Avoid using generic link text like `Learn more` or `Click here`", | ||
information: new URL( | ||
"https://primer.style/design/accessibility/links#writing-link-text" | ||
), | ||
tags: ["accessibility", "links"], | ||
function: function GH002(params, onError) { | ||
// markdown syntax | ||
const allBannedLinkTexts = bannedLinkText.concat( | ||
params.config.additional_banned_texts || [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm using snake case to follow the convention set in |
||
); | ||
const inlineTokens = params.tokens.filter((t) => t.type === "inline"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found markdown-it demo and using the Additionally, I referenced: md042. |
||
for (const token of inlineTokens) { | ||
const { children } = token; | ||
let inLink = false; | ||
let linkText = ""; | ||
|
||
for (const child of children) { | ||
const { content, type } = child; | ||
if (type === "link_open") { | ||
inLink = true; | ||
linkText = ""; | ||
} else if (type === "link_close") { | ||
inLink = false; | ||
if (allBannedLinkTexts.includes(stripAndDowncaseText(linkText))) { | ||
onError({ | ||
lineNumber: child.lineNumber, | ||
detail: `For link: ${linkText}`, | ||
}); | ||
} | ||
} else if (inLink) { | ||
linkText += content; | ||
} | ||
} | ||
} | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
const { | ||
stripAndDowncaseText, | ||
} = require("../../helpers/strip-and-downcase-text"); | ||
|
||
describe("stripAndDowncaseText", () => { | ||
test("strips extra whitespace", () => { | ||
expect(stripAndDowncaseText(" read more ")).toBe("read more"); | ||
expect(stripAndDowncaseText(" learn ")).toBe("learn"); | ||
}); | ||
|
||
test("strips punctuation", () => { | ||
expect(stripAndDowncaseText("learn more!!!!")).toBe("learn more"); | ||
expect(stripAndDowncaseText("I like dogs...")).toBe("i like dogs"); | ||
}); | ||
|
||
test("downcases text", () => { | ||
expect(stripAndDowncaseText("HeRe")).toBe("here"); | ||
expect(stripAndDowncaseText("CLICK HERE")).toBe("click here"); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
const noGenericLinkTextRule = require("../no-generic-link-text"); | ||
const runTest = require("./utils/run-test").runTest; | ||
|
||
describe("GH002: No Generic Link Text", () => { | ||
describe("successes", () => { | ||
test("inline", async () => { | ||
const strings = [ | ||
"[GitHub](https://www.github.com)", | ||
"[Read more about GitHub](https://www.github.com/about)", | ||
"[](www.github.com)", | ||
"![Image](www.github.com)", | ||
` | ||
## Hello | ||
I am not a link, and unrelated. | ||
![GitHub](some_image.png) | ||
`, | ||
]; | ||
|
||
const results = await runTest(strings, noGenericLinkTextRule); | ||
|
||
for (const result of results) { | ||
expect(result).not.toBeDefined(); | ||
} | ||
}); | ||
}); | ||
describe("failures", () => { | ||
test("inline", async () => { | ||
const strings = [ | ||
"[Click here](www.github.com)", | ||
"[here](www.github.com)", | ||
"Please [read more](www.github.com)", | ||
"[more](www.github.com)", | ||
"[link](www.github.com)", | ||
"You may [learn more](www.github.com) at GitHub", | ||
"[learn more.](www.github.com)", | ||
"[click here!](www.github.com)", | ||
]; | ||
|
||
const results = await runTest(strings, noGenericLinkTextRule); | ||
|
||
const failedRules = results | ||
.map((result) => result.ruleNames) | ||
.flat() | ||
.filter((name) => !name.includes("GH")); | ||
|
||
expect(failedRules).toHaveLength(8); | ||
for (const rule of failedRules) { | ||
expect(rule).toBe("no-generic-link-text"); | ||
} | ||
}); | ||
|
||
test("error message", async () => { | ||
const strings = ["[Click here](www.github.com)"]; | ||
|
||
const results = await runTest(strings, noGenericLinkTextRule); | ||
|
||
expect(results[0].ruleDescription).toMatch( | ||
/Avoid using generic link text like `Learn more` or `Click here`/ | ||
); | ||
expect(results[0].errorDetail).toBe("For link: Click here"); | ||
}); | ||
|
||
test("additional words can be configured", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💖 |
||
const results = await runTest( | ||
["[something](www.github.com)"], | ||
noGenericLinkTextRule, | ||
// eslint-disable-next-line camelcase | ||
{ additional_banned_texts: ["something"] } | ||
); | ||
|
||
const failedRules = results | ||
.map((result) => result.ruleNames) | ||
.flat() | ||
.filter((name) => !name.includes("GH")); | ||
|
||
expect(failedRules).toHaveLength(1); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
const markdownlint = require("markdownlint"); | ||
khiga8 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
async function runTest(strings, rule, ruleConfig) { | ||
const thisRuleName = rule.names[1]; | ||
|
||
const config = { | ||
config: { | ||
default: false, | ||
[thisRuleName]: ruleConfig || true, | ||
}, | ||
customRules: [rule], | ||
}; | ||
|
||
return await Promise.all( | ||
strings.map((variation) => { | ||
const thisTestConfig = { | ||
...config, | ||
strings: [variation], | ||
}; | ||
|
||
return new Promise((resolve, reject) => { | ||
markdownlint(thisTestConfig, (err, result) => { | ||
if (err) reject(err); | ||
resolve(result[0][0]); | ||
}); | ||
}); | ||
}) | ||
); | ||
} | ||
|
||
exports.runTest = runTest; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible or worth making this work for various languages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This plugin doesn't currently have i18n support, but I added support so people can additionally configure text so they can through that!
Related: #10 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will want to address i18n in a separate issue! I'll make an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i18n Issue!