rules: stricter PR-URL: check

Only allow GitHub pull request URLs.

Author: richardlau

lib/rules/pr-url.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const id = 'pr-url'
+const prUrl = /^https:\/\/github\.com\/\w+\/\w+\/pull\/\d+$/
 
 module.exports = {
   id: id
@@ -22,33 +23,42 @@ module.exports = {
       })
       return
     }
-    if (context.prUrl) {
-      if (context.prUrl[0] === '#') {
-        // see nodejs/node#7d3a7ea0d7df9b6f11df723dec370f49f4f87e99
-        // for an example
-        var line = -1
-        var column = -1
-        for (var i = 0; i < context.body.length; i++) {
-          const l = context.body[i]
-          if (~l.indexOf('PR-URL') && ~l.indexOf(context.prUrl)) {
-            line = i
-            column = l.indexOf(context.prUrl)
-          }
-        }
-        context.report({
-          id: id
-        , message: 'PR-URL must be a url, not a pr number.'
-        , string: context.prUrl
-        , line: line
-        , column: column
-        , level: 'fail'
-        })
+    var line = -1
+    var column = -1
+    for (var i = 0; i < context.body.length; i++) {
+      const l = context.body[i]
+      if (~l.indexOf('PR-URL') && ~l.indexOf(context.prUrl)) {
+        line = i
+        column = l.indexOf(context.prUrl)
       }
+    }
+    if (context.prUrl[0] === '#') {
+      // see nodejs/node#7d3a7ea0d7df9b6f11df723dec370f49f4f87e99
+      // for an example
+      context.report({
+        id: id
+      , message: 'PR-URL must be a URL, not a pull request number.'
+      , string: context.prUrl
+      , line: line
+      , column: column
+      , level: 'fail'
+      })
+    } else if (!prUrl.test(context.prUrl)) {
+      context.report({
+        id: id
+      , message: 'PR-URL must be a GitHub pull request URL.'
+      , string: context.prUrl
+      , line: line
+      , column: column
+      , level: 'fail'
+      })
     } else {
       context.report({
         id: id
-      , message: 'PR-URL is valid'
+      , message: 'PR-URL is valid.'
       , string: context.prUrl
+      , line: line
+      , column: column
       , level: 'pass'
       })
     }

test/rules/pr-url.js

@@ -3,7 +3,9 @@
 const test = require('tap').test
 const Rule = require('../../lib/rules/pr-url')
 const MISSING_PR_URL = 'Commit must have a PR-URL.'
-const INVALID_PR_URL = 'PR-URL must be a url, not a pr number.'
+const INVALID_PR_URL = 'PR-URL must be a GitHub pull request URL.'
+const NUMERIC_PR_URL = 'PR-URL must be a URL, not a pull request number.'
+const VALID_PR_URL = 'PR-URL is valid.'
 
 test('rule: pr-url', (t) => {
   t.test('missing', (tt) => {
@@ -24,7 +26,8 @@ test('rule: pr-url', (t) => {
     Rule.validate(context)
   })
 
-  t.test('invalid', (tt) => {
+
+  t.test('invalid numeric', (tt) => {
     tt.plan(7)
     const context = {
       prUrl: '#1234'
@@ -35,7 +38,7 @@ test('rule: pr-url', (t) => {
     , report: (opts) => {
         tt.pass('called report')
         tt.equal(opts.id, 'pr-url', 'id')
-        tt.equal(opts.message, INVALID_PR_URL, 'message')
+        tt.equal(opts.message, NUMERIC_PR_URL, 'message')
         tt.equal(opts.string, '#1234', 'string')
         tt.equal(opts.line, 1, 'line')
         tt.equal(opts.column, 8, 'column')
@@ -46,5 +49,51 @@ test('rule: pr-url', (t) => {
     Rule.validate(context)
   })
 
+  t.test('invalid', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/issues/1234'
+    const context = {
+      prUrl: url
+    , body: [
+        ''
+      , `PR-URL: ${url}`
+      ]
+    , report: (opts) => {
+        tt.pass('called report')
+        tt.equal(opts.id, 'pr-url', 'id')
+        tt.equal(opts.message, INVALID_PR_URL, 'message')
+        tt.equal(opts.string, url, 'string')
+        tt.equal(opts.line, 1, 'line')
+        tt.equal(opts.column, 8, 'column')
+        tt.equal(opts.level, 'fail', 'level')
+      }
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('valid', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/pull/1234'
+    const context = {
+      prUrl: url
+    , body: [
+        ''
+      , `PR-URL: ${url}`
+      ]
+    , report: (opts) => {
+        tt.pass('called report')
+        tt.equal(opts.id, 'pr-url', 'id')
+        tt.equal(opts.message, VALID_PR_URL, 'message')
+        tt.equal(opts.string, url, 'string')
+        tt.equal(opts.line, 1, 'line')
+        tt.equal(opts.column, 8, 'column')
+        tt.equal(opts.level, 'pass', 'level')
+      }
+    }
+
+    Rule.validate(context)
+  })
+
   t.end()
 })