Merge pull request #23 from gregsdennis/improvements

fix missed match when multiple dependencies are present

README.md

@@ -28,7 +28,9 @@ Also supports custom domains for use with GitHub Enterprise!
 Just add the following to a `.yml` file in your `.github/workflows/` folder.
 
 ```yaml
-on: [pull_request]
+on:
+  pull_request_target: 
+    types: [opened, edited, closed, reopened]
 
 jobs:
   check_dependencies:

evaluate-dependencies.js

@@ -0,0 +1,141 @@
+const core = require('@actions/core');
+const github = require('@actions/github');
+
+var customDomains = core.getInput('custom-domains')?.split(/(\s+)/) ?? [];
+
+const keyPhrases = 'depends on|blocked by';
+const issueTypes = 'issues|pull';
+const domainsList = ['github.com'].concat(customDomains); // add others from parameter
+const domainsString = combineDomains(domainsList);
+
+const quickLinkRegex = new RegExp(`(${keyPhrases}) #(\\d+)`, 'gmi');
+const partialLinkRegex = new RegExp(`(${keyPhrases}) ([-_\\w]+)\\/([-._a-z0-9]+)(#)(\\d+)`, 'gmi');
+const partialUrlRegex = new RegExp(`(${keyPhrases}) ([-_\\w]+)\\/([-._a-z0-9]+)\\/(${issueTypes})\\/(\\d+)`, 'gmi');
+const fullUrlRegex = new RegExp(`(${keyPhrases}) https?:\\/\\/(?:${domainsString})\\/([-_\\w]+)\\/([-._a-z0-9]+)\\/(${issueTypes})\\/(\\d+)`, 'gmi');
+const markdownRegex = new RegExp(`(${keyPhrases}) \\[.*\\]\\(https?:\\/\\/(?:${domainsString})\\/([-_\\w]+)\\/([-._a-z0-9]+)\\/(${issueTypes})\\/(\\d+)\\)`, 'gmi');
+
+function escapeDomainForRegex(domain) {
+    return domain.replace('.', '\\.');
+}
+
+function combineDomains(domains) {
+    return domains.map(x => escapeDomainForRegex(x)).join("|");
+}
+
+function extractFromMatch(match) {
+    return {
+        owner: match[2],
+        repo: match[3],
+        pull_number: parseInt(match[5], 10)
+    };
+}
+
+function getAllDependencies(body) {
+    var allMatches = [];
+
+    var quickLinkMatches = [...body.matchAll(quickLinkRegex)];
+    if (quickLinkMatches.length !== 0) {
+        quickLinkMatches.forEach(match => {
+            core.info(`  Found number-referenced dependency in '${match}'`);
+            allMatches.push({
+                owner: github.context.repo.owner,
+                repo: github.context.repo.repo,
+                pull_number: parseInt(match[2], 10)
+            });
+        });
+    }
+
+    var extractableMatches = [...body.matchAll(partialLinkRegex)]
+        .concat([...body.matchAll(partialUrlRegex)])
+        .concat([...body.matchAll(fullUrlRegex)])
+        .concat([...body.matchAll(markdownRegex)]);
+    if (extractableMatches.length !== 0) {
+        extractableMatches.forEach(match => {
+            core.info(`  Found number-referenced dependency in '${match}'`);
+            allMatches.push(extractFromMatch(match));
+        });
+    }
+
+    return allMatches;
+}
+
+async function evaluate() {
+    try {
+        core.info('Initializing...');
+        const myToken = process.env.GITHUB_TOKEN;
+        const octokit = github.getOctokit(myToken);
+
+        const { data: pullRequest } = await octokit.pulls.get({
+            owner: github.context.repo.owner,
+            repo: github.context.repo.repo,
+            pull_number: github.context.issue.number,
+        });
+
+        if (!pullRequest.body){
+            core.info('body empty')
+            return;
+        }
+
+        core.info('\nReading PR body...');
+        var dependencies = getAllDependencies(pullRequest.body);
+
+        core.info('\nAnalyzing lines...');
+        var dependencyIssues = [];
+        for (var d of dependencies) {
+            core.info(`  Fetching '${JSON.stringify(d)}'`);
+            var isPr = true;
+            var response = await octokit.pulls.get(d).catch(error => core.error(error));
+            if (response === undefined) {
+                isPr = false;
+                d = {
+                    owner: d.owner,
+                    repo: d.repo,
+                    issue_number: d.pull_number,
+                };
+                core.info(`  Fetching '${JSON.stringify(d)}'`);
+                response = await octokit.issues.get(d).catch(error => core.error(error));
+                if (response === undefined) {
+                    core.info('    Could not locate this dependency.  Will need to verify manually.');
+                    continue;
+                }
+            }
+            if (isPr) {
+                const { data: pr } = response;
+                if (!pr) continue;
+                if (!pr.merged && !pr.closed_at) {
+                    core.info('    PR is still open.');
+                    dependencyIssues.push(pr);
+                } else {
+                    core.info('    PR has been closed.');
+                }
+            } else {
+                const { data: issue } = response;
+                if (!issue) continue;
+                if (!issue.closed_at) {
+                    core.info('    Issue is still open.');
+                    dependencyIssues.push(issue);
+                } else {
+                    core.info('    Issue has been closed.');
+                }
+            }
+        }
+
+        if (dependencyIssues.length !== 0) {
+            var msg = '\nThe following issues need to be resolved before this PR can be merged:\n';
+            for (var pr of dependencyIssues) {
+                msg += `\n#${pr.number} - ${pr.title}`;
+            }
+            core.setFailed(msg);
+        } else {
+            core.info("\nAll dependencies have been resolved!")
+        }
+    } catch (error) {
+        core.setFailed(error.message);
+        throw error;
+    }
+}
+
+module.exports = {
+    evaluate: evaluate,
+    getAllDependencies: getAllDependencies
+}

evaluate-dependencies.test.js

@@ -0,0 +1,70 @@
+const evaluate = require('./evaluate-dependencies');
+
+process.env.GITHUB_REPOSITORY = 'owner/repo';
+
+const shorthand = 'Depends on #14'
+test('Shorthand', () => {
+    expect(evaluate.getAllDependencies(shorthand))
+        .toStrictEqual([{
+            owner: 'owner',
+            repo: 'repo',
+            pull_number: 14
+        }]);
+});
+
+const partialLink = 'Depends on gregsdennis/dependencies-action#5'
+test('partialLink', () => {
+    expect(evaluate.getAllDependencies(partialLink))
+        .toStrictEqual([{
+            owner: 'gregsdennis',
+            repo: 'dependencies-action',
+            pull_number: 5
+        }]);
+});
+
+const shorthandAndPartialLink = `Depends on #14
+Depends on gregsdennis/dependencies-action#5`
+test('shorthandAndPartialLink', () => {
+    expect(evaluate.getAllDependencies(shorthandAndPartialLink))
+        .toStrictEqual([{
+            owner: 'owner',
+            repo: 'repo',
+            pull_number: 14
+        },{
+            owner: 'gregsdennis',
+            repo: 'dependencies-action',
+            pull_number: 5
+        }]);
+});
+
+const shorthandAndPartialLinkWithBlankLineAtEnd = `Depends on #14
+Depends on gregsdennis/dependencies-action#5
+`
+test('shorthandAndPartialLinkWithBlankLineAtEnd', () => {
+    expect(evaluate.getAllDependencies(shorthandAndPartialLinkWithBlankLineAtEnd))
+        .toStrictEqual([{
+            owner: 'owner',
+            repo: 'repo',
+            pull_number: 14
+        },{
+            owner: 'gregsdennis',
+            repo: 'dependencies-action',
+            pull_number: 5
+        }]);
+});
+
+const shorthandAndPartialLinkWithBlankLineInMiddle = `Depends on #14
+
+Depends on gregsdennis/dependencies-action#5`
+test('shorthandAndPartialLinkWithBlankLineInMiddle', () => {
+    expect(evaluate.getAllDependencies(shorthandAndPartialLinkWithBlankLineInMiddle))
+        .toStrictEqual([{
+            owner: 'owner',
+            repo: 'repo',
+            pull_number: 14
+        },{
+            owner: 'gregsdennis',
+            repo: 'dependencies-action',
+            pull_number: 5
+        }]);
+});

index.js

@@ -1,155 +1,7 @@
-const core = require('@actions/core');
-const github = require('@actions/github');
-
-var customDomains = core.getInput('custom-domains')?.split(/(\s+)/) ?? [];
-
-const keyPhrases = 'depends on|blocked by';
-const issueTypes = 'issues|pull';
-const domainsList = ['github.com'].concat(customDomains); // add others from parameter
-const domainsString = combineDomains(domainsList);
-
-const quickLinkRegex = new RegExp(`(${keyPhrases}) #(\\d+)`, 'gmi');
-const partialLinkRegex = new RegExp(`(${keyPhrases}) ([-_\\w]+)\\/([-._a-z0-9]+)(#)(\\d+)`, 'gmi');
-const partialUrlRegex = new RegExp(`(${keyPhrases}) ([-_\\w]+)\\/([-._a-z0-9]+)\\/(${issueTypes})\\/(\\d+)`, 'gmi');
-const fullUrlRegex = new RegExp(`(${keyPhrases}) https?:\\/\\/(?:${domainsString})\\/([-_\\w]+)\\/([-._a-z0-9]+)\\/(${issueTypes})\\/(\\d+)`, 'gmi');
-const markdownRegex = new RegExp(`(${keyPhrases}) \\[.*\\]\\(https?:\\/\\/(?:${domainsString})\\/([-_\\w]+)\\/([-._a-z0-9]+)\\/(${issueTypes})\\/(\\d+)\\)`, 'gmi');
-
-function escapeDomainForRegex(domain) {
-    return domain.replace('.', '\\.');
-}
-
-function combineDomains(domains) {
-    return domains.map(x => escapeDomainForRegex(x)).join("|");
-}
-
-function extractFromMatch(match) {
-    return {
-        owner: match[2],
-        repo: match[3],
-        pull_number: parseInt(match[5], 10)
-    };
-}
-
-function getDependency(line) {
-    var match = quickLinkRegex.exec(line);
-    if (match !== null) {
-        core.info(`  Found number-referenced dependency in '${line}'`);
-        return {
-            owner: github.context.repo.owner,
-            repo: github.context.repo.repo,
-            pull_number: parseInt(match[2], 10)
-        };
-    }
-
-    match = partialLinkRegex.exec(line);
-    if (match !== null) {
-        core.info(`  Found partial-link dependency in '${line}'`);
-        return extractFromMatch(match);
-    }
-
-    match = partialUrlRegex.exec(line);
-    if (match !== null) {
-        core.info(`  Found partial-url dependency in '${line}'`);
-        return extractFromMatch(match);
-    }
-
-    match = fullUrlRegex.exec(line);
-    if (match !== null) {
-        core.info(`  Found full-url dependency in '${line}'`);
-        return extractFromMatch(match);
-    }
-
-    match = markdownRegex.exec(line);
-    if (match !== null) {
-        core.info(`  Found markdown dependency in '${line}'`);
-        return extractFromMatch(match);
-    }
-
-    core.info(`  Found no dependency in '${line}'`);
-    return null;
-};
+const evaluate = require('./evaluate-dependencies');
 
 async function run() {
-    try {
-        core.info('Initializing...');
-        const myToken = process.env.GITHUB_TOKEN;
-        const octokit = github.getOctokit(myToken);
-
-        const { data: pullRequest } = await octokit.pulls.get({
-            owner: github.context.repo.owner,
-            repo: github.context.repo.repo,
-            pull_number: github.context.issue.number,
-        });
-
-        if (!pullRequest.body){
-            core.info('body empty')
-            return;
-        }
-
-        core.info('\nReading PR body...');
-        const lines = pullRequest.body.split(/\r\n|\r|\n/);
-
-        var dependencies = [];
-        lines.forEach(l => {
-            var dependency = getDependency(l);
-            if (dependency !== null)
-                dependencies.push(dependency);
-        });
-
-        core.info('\nAnalyzing lines...');
-        var dependencyIssues = [];
-        for (var d of dependencies) {
-            core.info(`  Fetching '${JSON.stringify(d)}'`);
-            var isPr = true;
-            var response = await octokit.pulls.get(d).catch(error => core.error(error));
-            if (response === undefined) {
-                isPr = false;
-                d = {
-                    owner: d.owner,
-                    repo: d.repo,
-                    issue_number: d.pull_number,
-                };
-                core.info(`  Fetching '${JSON.stringify(d)}'`);
-                response = await octokit.issues.get(d).catch(error => core.error(error));
-                if (response === undefined) {
-                    core.info('    Could not locate this dependency.  Will need to verify manually.');
-                    continue;
-                }
-            }
-            if (isPr) {
-                const { data: pr } = response;
-                if (!pr) continue;
-                if (!pr.merged && !pr.closed_at) {
-                    core.info('    PR is still open.');
-                    dependencyIssues.push(pr);
-                } else {
-                    core.info('    PR has been closed.');
-                }
-            } else {
-                const { data: issue } = response;
-                if (!issue) continue;
-                if (!issue.closed_at) {
-                    core.info('    Issue is still open.');
-                    dependencyIssues.push(issue);
-                } else {
-                    core.info('    Issue has been closed.');
-                }
-            }
-        }
-
-        if (dependencyIssues.length !== 0) {
-            var msg = '\nThe following issues need to be resolved before this PR can be merged:\n';
-            for (var pr of dependencyIssues) {
-                msg += `\n#${pr.number} - ${pr.title}`;
-            }
-            core.setFailed(msg);
-        } else {
-            core.info("\nAll dependencies have been resolved!")
-        }
-    } catch (error) {
-        core.setFailed(error.message);
-        throw error;
-    }
+    await evaluate.evaluate();
 }
 
-run();
+run();