Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ea1958d

Browse filesBrowse files
khiga8khiga8
authored
Add no-generic-link-text rule (#10)
* Move to helper * Add test file * Add no-generic-link-text rule * Add tests with punctuation * Update tests and account for blank link - We don't want anything to break when link is blank. - Tests should account for links with punctuation text. * Ensure rule is configured * Add rule to accessibility * Fix tests and run linter * More updates * Make tests more robust * Allow additional words to be configured * Run lint * Allow rule params to be configured * Add test * Rebase conflicts * Clean up code * Move helper out and add tests
1 parent 18247e8 commit ea1958d
Copy full SHA for ea1958d
Expand file treeCollapse file tree

9 files changed

+203
-38
lines changed

‎helpers/strip-and-downcase-text.js

Copy file name to clipboard
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/* Downcase and strip extra whitespaces and punctuation */
2+
function stripAndDowncaseText(text) {
3+
return text
4+
.toLowerCase()
5+
.replace(/[.,/#!$%^&*;:{}=\-_`~()]/g, "")
6+
.replace(/\s+/g, " ")
7+
.trim();
8+
}
9+
10+
module.exports = { stripAndDowncaseText };

‎index.js

Copy file name to clipboardExpand all lines: index.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ const _ = require("lodash");
33
const accessibilityRules = require("./style/accessibility.json");
44
const base = require("./style/base.json");
55
const noDefaultAltText = require("./no-default-alt-text");
6+
const noGenericLinkText = require("./no-generic-link-text");
67

7-
const customRules = [noDefaultAltText];
8+
const customRules = [noDefaultAltText, noGenericLinkText];
89

910
module.exports = [...customRules];
1011

‎no-generic-link-text.js

Copy file name to clipboard
+50Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
const { stripAndDowncaseText } = require("./helpers/strip-and-downcase-text");
2+
3+
const bannedLinkText = [
4+
"read more",
5+
"learn more",
6+
"more",
7+
"here",
8+
"click here",
9+
"link",
10+
];
11+
12+
module.exports = {
13+
names: ["GH002", "no-generic-link-text"],
14+
description:
15+
"Avoid using generic link text like `Learn more` or `Click here`",
16+
information: new URL(
17+
"https://primer.style/design/accessibility/links#writing-link-text"
18+
),
19+
tags: ["accessibility", "links"],
20+
function: function GH002(params, onError) {
21+
// markdown syntax
22+
const allBannedLinkTexts = bannedLinkText.concat(
23+
params.config.additional_banned_texts || []
24+
);
25+
const inlineTokens = params.tokens.filter((t) => t.type === "inline");
26+
for (const token of inlineTokens) {
27+
const { children } = token;
28+
let inLink = false;
29+
let linkText = "";
30+
31+
for (const child of children) {
32+
const { content, type } = child;
33+
if (type === "link_open") {
34+
inLink = true;
35+
linkText = "";
36+
} else if (type === "link_close") {
37+
inLink = false;
38+
if (allBannedLinkTexts.includes(stripAndDowncaseText(linkText))) {
39+
onError({
40+
lineNumber: child.lineNumber,
41+
detail: `For link: ${linkText}`,
42+
});
43+
}
44+
} else if (inLink) {
45+
linkText += content;
46+
}
47+
}
48+
}
49+
},
50+
};

‎style/accessibility.json

Copy file name to clipboardExpand all lines: style/accessibility.json
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"no-default-alt-text": true,
33
"no-duplicate-header": true,
44
"no-emphasis-as-header": true,
5+
"no-generic-link-text": true,
56
"no-space-in-links": false,
67
"ol-prefix": "ordered",
78
"single-h1": true,

‎test/helpers/strip-and-downcase-text.test.js

Copy file name to clipboard
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
const {
2+
stripAndDowncaseText,
3+
} = require("../../helpers/strip-and-downcase-text");
4+
5+
describe("stripAndDowncaseText", () => {
6+
test("strips extra whitespace", () => {
7+
expect(stripAndDowncaseText(" read more ")).toBe("read more");
8+
expect(stripAndDowncaseText(" learn ")).toBe("learn");
9+
});
10+
11+
test("strips punctuation", () => {
12+
expect(stripAndDowncaseText("learn more!!!!")).toBe("learn more");
13+
expect(stripAndDowncaseText("I like dogs...")).toBe("i like dogs");
14+
});
15+
16+
test("downcases text", () => {
17+
expect(stripAndDowncaseText("HeRe")).toBe("here");
18+
expect(stripAndDowncaseText("CLICK HERE")).toBe("click here");
19+
});
20+
});

‎test/no-default-alt-text.test.js

Copy file name to clipboardExpand all lines: test/no-default-alt-text.test.js
+8-36Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,5 @@
1-
const markdownlint = require("markdownlint");
21
const altTextRule = require("../no-default-alt-text");
3-
4-
const thisRuleName = altTextRule.names[1];
5-
6-
const config = {
7-
config: {
8-
default: false,
9-
[thisRuleName]: true,
10-
},
11-
customRules: [altTextRule],
12-
};
13-
14-
async function runTest(strings) {
15-
return await Promise.all(
16-
strings.map((variation) => {
17-
const thisTestConfig = {
18-
...config,
19-
strings: [variation],
20-
};
21-
22-
return new Promise((resolve, reject) => {
23-
markdownlint(thisTestConfig, (err, result) => {
24-
if (err) reject(err);
25-
resolve(result[0][0]);
26-
});
27-
});
28-
})
29-
);
30-
}
2+
const runTest = require("./utils/run-test").runTest;
313

324
describe("GH001: No Default Alt Text", () => {
335
describe("successes", () => {
@@ -36,7 +8,7 @@ describe("GH001: No Default Alt Text", () => {
368
"![Chart with a single root node reading 'Example'](https://user-images.githubusercontent.com/abcdef.png)",
379
];
3810

39-
const results = await runTest(strings);
11+
const results = await runTest(strings, altTextRule);
4012

4113
for (const result of results) {
4214
expect(result).not.toBeDefined();
@@ -47,7 +19,7 @@ describe("GH001: No Default Alt Text", () => {
4719
'<img alt="A helpful description" src="https://user-images.githubusercontent.com/abcdef.png">',
4820
];
4921

50-
const results = await runTest(strings);
22+
const results = await runTest(strings, altTextRule);
5123

5224
for (const result of results) {
5325
expect(result).not.toBeDefined();
@@ -63,7 +35,7 @@ describe("GH001: No Default Alt Text", () => {
6335
"![Screenshot 2022-06-26 at 7 41 30 PM](https://user-images.githubusercontent.com/abcdef.png)",
6436
];
6537

66-
const results = await runTest(strings);
38+
const results = await runTest(strings, altTextRule);
6739

6840
const failedRules = results
6941
.map((result) => result.ruleNames)
@@ -72,7 +44,7 @@ describe("GH001: No Default Alt Text", () => {
7244

7345
expect(failedRules).toHaveLength(4);
7446
for (const rule of failedRules) {
75-
expect(rule).toBe(thisRuleName);
47+
expect(rule).toBe("no-default-alt-text");
7648
}
7749
});
7850

@@ -84,7 +56,7 @@ describe("GH001: No Default Alt Text", () => {
8456
'<img alt="Screenshot 2022-06-26 at 7 41 30 PM" src="https://user-images.githubusercontent.com/abcdef.png">',
8557
];
8658

87-
const results = await runTest(strings);
59+
const results = await runTest(strings, altTextRule);
8860

8961
const failedRules = results
9062
.map((result) => result.ruleNames)
@@ -93,7 +65,7 @@ describe("GH001: No Default Alt Text", () => {
9365

9466
expect(failedRules).toHaveLength(4);
9567
for (const rule of failedRules) {
96-
expect(rule).toBe(thisRuleName);
68+
expect(rule).toBe("no-default-alt-text");
9769
}
9870
});
9971

@@ -103,7 +75,7 @@ describe("GH001: No Default Alt Text", () => {
10375
'<img alt="Screen Shot 2022-06-26 at 7 41 30 PM" src="https://user-images.githubusercontent.com/abcdef.png">',
10476
];
10577

106-
const results = await runTest(strings);
78+
const results = await runTest(strings, altTextRule);
10779

10880
expect(results[0].ruleDescription).toMatch(
10981
/Images should not use the MacOS default screenshot filename as alternate text/

‎test/no-generic-link-text.test.js

Copy file name to clipboard
+79Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
const noGenericLinkTextRule = require("../no-generic-link-text");
2+
const runTest = require("./utils/run-test").runTest;
3+
4+
describe("GH002: No Generic Link Text", () => {
5+
describe("successes", () => {
6+
test("inline", async () => {
7+
const strings = [
8+
"[GitHub](https://www.github.com)",
9+
"[Read more about GitHub](https://www.github.com/about)",
10+
"[](www.github.com)",
11+
"![Image](www.github.com)",
12+
`
13+
## Hello
14+
I am not a link, and unrelated.
15+
![GitHub](some_image.png)
16+
`,
17+
];
18+
19+
const results = await runTest(strings, noGenericLinkTextRule);
20+
21+
for (const result of results) {
22+
expect(result).not.toBeDefined();
23+
}
24+
});
25+
});
26+
describe("failures", () => {
27+
test("inline", async () => {
28+
const strings = [
29+
"[Click here](www.github.com)",
30+
"[here](www.github.com)",
31+
"Please [read more](www.github.com)",
32+
"[more](www.github.com)",
33+
"[link](www.github.com)",
34+
"You may [learn more](www.github.com) at GitHub",
35+
"[learn more.](www.github.com)",
36+
"[click here!](www.github.com)",
37+
];
38+
39+
const results = await runTest(strings, noGenericLinkTextRule);
40+
41+
const failedRules = results
42+
.map((result) => result.ruleNames)
43+
.flat()
44+
.filter((name) => !name.includes("GH"));
45+
46+
expect(failedRules).toHaveLength(8);
47+
for (const rule of failedRules) {
48+
expect(rule).toBe("no-generic-link-text");
49+
}
50+
});
51+
52+
test("error message", async () => {
53+
const strings = ["[Click here](www.github.com)"];
54+
55+
const results = await runTest(strings, noGenericLinkTextRule);
56+
57+
expect(results[0].ruleDescription).toMatch(
58+
/Avoid using generic link text like `Learn more` or `Click here`/
59+
);
60+
expect(results[0].errorDetail).toBe("For link: Click here");
61+
});
62+
63+
test("additional words can be configured", async () => {
64+
const results = await runTest(
65+
["[something](www.github.com)"],
66+
noGenericLinkTextRule,
67+
// eslint-disable-next-line camelcase
68+
{ additional_banned_texts: ["something"] }
69+
);
70+
71+
const failedRules = results
72+
.map((result) => result.ruleNames)
73+
.flat()
74+
.filter((name) => !name.includes("GH"));
75+
76+
expect(failedRules).toHaveLength(1);
77+
});
78+
});
79+
});

‎test/usage.test.js

Copy file name to clipboardExpand all lines: test/usage.test.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ describe("usage", () => {
44
describe("default export", () => {
55
test("custom rules on default export", () => {
66
const rules = githubMarkdownLint;
7-
expect(rules).toHaveLength(1);
7+
expect(rules).toHaveLength(2);
88
expect(rules[0].names).toEqual(["GH001", "no-default-alt-text"]);
99
});
1010
});
@@ -17,6 +17,7 @@ describe("usage", () => {
1717
"no-space-in-links": false,
1818
"single-h1": true,
1919
"no-emphasis-as-header": true,
20+
"no-generic-link-text": true,
2021
"ul-style": true,
2122
default: true,
2223
"no-inline-html": false,

‎test/utils/run-test.js

Copy file name to clipboard
+31Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
const markdownlint = require("markdownlint");
2+
3+
async function runTest(strings, rule, ruleConfig) {
4+
const thisRuleName = rule.names[1];
5+
6+
const config = {
7+
config: {
8+
default: false,
9+
[thisRuleName]: ruleConfig || true,
10+
},
11+
customRules: [rule],
12+
};
13+
14+
return await Promise.all(
15+
strings.map((variation) => {
16+
const thisTestConfig = {
17+
...config,
18+
strings: [variation],
19+
};
20+
21+
return new Promise((resolve, reject) => {
22+
markdownlint(thisTestConfig, (err, result) => {
23+
if (err) reject(err);
24+
resolve(result[0][0]);
25+
});
26+
});
27+
})
28+
);
29+
}
30+
31+
exports.runTest = runTest;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.