Skip to content

Commit 2523fb9

Browse files
richardlaurichardlau
committed
rules: stricter Fixes: URL check
Only allow GitHub issue URLs, or comments or discussions from pull request URLs.
1 parent a82ac92 commit 2523fb9

File tree

2 files changed

+177
-16
lines changed

2 files changed

+177
-16
lines changed

‎lib/rules/fixes-url.js

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict'
22

33
const id = 'fixes-url'
4+
const github = new RegExp('^https://github\.com/\\w+\/\\w+/' +
5+
'(issues|pull)/\\d+(#issuecomment-\\d+|#discussion_r\\d+)?$'
6+
)
47

58
module.exports = {
69
id: id
@@ -21,28 +24,53 @@ module.exports = {
2124
})
2225
return
2326
}
27+
// Allow GitHub issues with optional comment.
28+
// GitHub pull requests must reference a comment or discussion.
2429
for (const url of parsed.fixes) {
30+
const match = github.exec(url)
2531
if (url[0] === '#') {
2632
// See nodejs/node#2aa376914b621018c5784104b82c13e78ee51307
2733
// for an example
2834
const { line, column } = findLineAndColumn(context.body, url)
2935
context.report({
3036
id: id
31-
, message: 'Fixes must be a url, not an issue number.'
37+
, message: 'Fixes must be a URL, not an issue number.'
3238
, string: url
3339
, line: line
3440
, column: column
3541
, level: 'fail'
3642
})
43+
} else if (match) {
44+
if (match[1] === 'pull' && match[2] === undefined) {
45+
const { line, column } = findLineAndColumn(context.body, url)
46+
context.report({
47+
id: id
48+
, message: 'Pull request URL must reference a comment or discussion.'
49+
, string: url
50+
, line: line
51+
, column: column
52+
, level: 'fail'
53+
})
54+
} else {
55+
const { line, column } = findLineAndColumn(context.body, url)
56+
context.report({
57+
id: id
58+
, message: 'Valid fixes URL.'
59+
, string: url
60+
, line: line
61+
, column: column
62+
, level: 'pass'
63+
})
64+
}
3765
} else {
3866
const { line, column } = findLineAndColumn(context.body, url)
3967
context.report({
4068
id: id
41-
, message: 'Valid fixes url'
69+
, message: 'Fixes must be a GitHub URL.'
4270
, string: url
4371
, line: line
4472
, column: column
45-
, level: 'pass'
73+
, level: 'fail'
4674
})
4775
}
4876
}

‎test/rules/fixes-url.js

Lines changed: 146 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,36 @@ const test = require('tap').test
44
const Rule = require('../../lib/rules/fixes-url')
55
const Commit = require('gitlint-parser-node')
66
const Validator = require('../../')
7-
const INVALID_FIXES_URL = 'Fixes must be a url, not an issue number.'
7+
8+
const INVALID_PRURL = 'Pull request URL must reference a comment or discussion.'
9+
const NOT_AN_ISSUE_NUMBER = 'Fixes must be a URL, not an issue number.'
10+
const NOT_A_GITHUB_URL = 'Fixes must be a GitHub URL.'
11+
const VALID_FIXES_URL = 'Valid fixes URL.'
12+
13+
const makeCommit = (msg) => {
14+
return new Commit({
15+
sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea'
16+
, author: {
17+
name: 'Evan Lucas'
18+
, email: '[email protected]'
19+
, date: '2016-04-12T19:42:23Z'
20+
}
21+
, message: msg
22+
}, new Validator())
23+
}
824

925
test('rule: fixes-url', (t) => {
10-
t.test('invalid', (tt) => {
26+
t.test('issue number', (tt) => {
1127
tt.plan(7)
12-
const v = new Validator()
13-
const context = new Commit({
14-
sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea'
15-
, author: {
16-
name: 'Evan Lucas'
17-
, email: '[email protected]'
18-
, date: '2016-04-12T19:42:23Z'
19-
}
20-
, message: `test: fix something
28+
const context = makeCommit(`test: fix something
2129
2230
Fixes: #1234`
23-
}, v)
31+
)
2432

2533
context.report = (opts) => {
2634
tt.pass('called report')
2735
tt.equal(opts.id, 'fixes-url', 'id')
28-
tt.equal(opts.message, INVALID_FIXES_URL, 'message')
36+
tt.equal(opts.message, NOT_AN_ISSUE_NUMBER, 'message')
2937
tt.equal(opts.string, '#1234', 'string')
3038
tt.equal(opts.line, 1, 'line')
3139
tt.equal(opts.column, 7, 'column')
@@ -35,5 +43,130 @@ Fixes: #1234`
3543
Rule.validate(context)
3644
})
3745

46+
t.test('GitHub issue URL', (tt) => {
47+
tt.plan(7)
48+
const url = 'https://github.com/nodejs/node/issues/1234'
49+
const context = makeCommit(`test: fix something
50+
51+
Fixes: ${url}`
52+
)
53+
54+
context.report = (opts) => {
55+
tt.pass('called report')
56+
tt.equal(opts.id, 'fixes-url', 'id')
57+
tt.equal(opts.message, VALID_FIXES_URL, 'message')
58+
tt.equal(opts.string, url, 'string')
59+
tt.equal(opts.line, 1, 'line')
60+
tt.equal(opts.column, 7, 'column')
61+
tt.equal(opts.level, 'pass', 'level')
62+
}
63+
64+
Rule.validate(context)
65+
})
66+
67+
t.test('GitHub issue URL with comment', (tt) => {
68+
tt.plan(7)
69+
const url = 'https://github.com/nodejs/node/issues/1234#issuecomment-1234'
70+
const context = makeCommit(`test: fix something
71+
72+
Fixes: ${url}`
73+
)
74+
75+
context.report = (opts) => {
76+
tt.pass('called report')
77+
tt.equal(opts.id, 'fixes-url', 'id')
78+
tt.equal(opts.message, VALID_FIXES_URL, 'message')
79+
tt.equal(opts.string, url, 'string')
80+
tt.equal(opts.line, 1, 'line')
81+
tt.equal(opts.column, 7, 'column')
82+
tt.equal(opts.level, 'pass', 'level')
83+
}
84+
85+
Rule.validate(context)
86+
})
87+
88+
t.test('GitHub PR URL', (tt) => {
89+
tt.plan(7)
90+
const url = 'https://github.com/nodejs/node/pull/1234'
91+
const context = makeCommit(`test: fix something
92+
93+
Fixes: ${url}`
94+
)
95+
96+
context.report = (opts) => {
97+
tt.pass('called report')
98+
tt.equal(opts.id, 'fixes-url', 'id')
99+
tt.equal(opts.message, INVALID_PRURL, 'message')
100+
tt.equal(opts.string, url, 'string')
101+
tt.equal(opts.line, 1, 'line')
102+
tt.equal(opts.column, 7, 'column')
103+
tt.equal(opts.level, 'fail', 'level')
104+
}
105+
106+
Rule.validate(context)
107+
})
108+
109+
t.test('GitHub PR URL with comment', (tt) => {
110+
tt.plan(7)
111+
const url = 'https://github.com/nodejs/node/pull/1234#issuecomment-1234'
112+
const context = makeCommit(`test: fix something
113+
114+
Fixes: ${url}`
115+
)
116+
117+
context.report = (opts) => {
118+
tt.pass('called report')
119+
tt.equal(opts.id, 'fixes-url', 'id')
120+
tt.equal(opts.message, VALID_FIXES_URL, 'message')
121+
tt.equal(opts.string, url, 'string')
122+
tt.equal(opts.line, 1, 'line')
123+
tt.equal(opts.column, 7, 'column')
124+
tt.equal(opts.level, 'pass', 'level')
125+
}
126+
127+
Rule.validate(context)
128+
})
129+
130+
t.test('GitHub PR URL with discussion comment', (tt) => {
131+
tt.plan(7)
132+
const url = 'https://github.com/nodejs/node/pull/1234#discussion_r1234'
133+
const context = makeCommit(`test: fix something
134+
135+
Fixes: ${url}`
136+
)
137+
138+
context.report = (opts) => {
139+
tt.pass('called report')
140+
tt.equal(opts.id, 'fixes-url', 'id')
141+
tt.equal(opts.message, VALID_FIXES_URL, 'message')
142+
tt.equal(opts.string, url, 'string')
143+
tt.equal(opts.line, 1, 'line')
144+
tt.equal(opts.column, 7, 'column')
145+
tt.equal(opts.level, 'pass', 'level')
146+
}
147+
148+
Rule.validate(context)
149+
})
150+
151+
t.test('non-GitHub URL', (tt) => {
152+
tt.plan(7)
153+
const context = makeCommit(`test: fix something
154+
155+
Fixes: https://nodejs.org`
156+
)
157+
158+
context.report = (opts) => {
159+
tt.pass('called report')
160+
tt.equal(opts.id, 'fixes-url', 'id')
161+
tt.equal(opts.message, NOT_A_GITHUB_URL, 'message')
162+
tt.equal(opts.string, 'https://nodejs.org', 'string')
163+
tt.equal(opts.line, 1, 'line')
164+
tt.equal(opts.column, 7, 'column')
165+
tt.equal(opts.level, 'fail', 'level')
166+
}
167+
168+
Rule.validate(context)
169+
})
170+
38171
t.end()
39172
})

0 commit comments

Comments
 (0)