Skip to content

Commit

Permalink
Warn on legacy (ambiguous) issue references
Browse files Browse the repository at this point in the history
  • Loading branch information
CyberShadow committed Jun 24, 2023
1 parent 197fb27 commit 835295f
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 11 deletions.
5 changes: 2 additions & 3 deletions source/dlangbot/app.d
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,8 @@ void handlePR(string action, PullRequest* _pr)
refs = getIssueRefs(commits);
descs = getDescriptions(refs);
if (action == "opened" || action == "synchronize" || action == "ready_for_review")
{
msgs = pr.checkForWarnings(descs, refs);
}
msgs ~= pr.checkForWarnings(descs, refs);
msgs ~= checkLegacyIssueRefs(commits);

pr.updateGithubComment(comment, action, refs, descs, msgs);

Expand Down
48 changes: 40 additions & 8 deletions source/dlangbot/bugzilla.d
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,23 @@ static immutable bugzillaProjectSlugs = ["dlang/dmd", "dlang/druntime", "dlang/p
import std.algorithm, std.conv, std.range, std.string;
import std.exception : enforce;
import std.format : format;
import std.regex : ctRegex, matchFirst, matchAll;

import dlangbot.warnings : UserMessage;

//==============================================================================
// Bugzilla
//==============================================================================

auto matchIssueRefs(string message)
{
import std.regex;
private enum issueRE = ctRegex!(
`(?:^fix(?:es)?(?:\s+bugzilla)(?:\s+(?:issues?|bugs?))?\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))|` ~
`(?:bugzilla\s+(?:(?:issues?|bugs?)\s+)?(#?\d+(?:[\s,\+&and]+#?\d+)*))`, "im");
private enum oldIssueRE = ctRegex!(
`(?:^fix(?:es)?(?:\s+(?:issues?|bugs?))?\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))|` ~
`(?:(?:issues?|bugs?)\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))`, "im");

auto matchIssueRefs(RE)(string message, RE re = issueRE)
{
static auto matchToRefs(M)(M m)
{
enum splitRE = ctRegex!(`(\d+)`);
Expand All @@ -31,9 +39,7 @@ auto matchIssueRefs(string message)
.map!(match => IssueRef(match.hit.to!int, closed));
}

enum issueRE = ctRegex!(`(?:^fix(?:es)?(?:\s+bugzilla)(?:\s+(?:issues?|bugs?))?\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))|` ~
`(?:bugzilla\s+(?:(?:issues?|bugs?)\s+)?(#?\d+(?:[\s,\+&and]+#?\d+)*))`, "im");
return matchToRefs(message.matchFirst(issueRE));
return matchToRefs(message.matchFirst(re));
}

unittest
Expand Down Expand Up @@ -87,13 +93,13 @@ unittest

struct IssueRef { int id; bool fixed; Json[] commits; }
// get all issues mentioned in a commit
IssueRef[] getIssueRefs(Json[] commits)
IssueRef[] getIssueRefs(RE)(Json[] commits, RE re = issueRE)
{
return commits
// Collect all issue references (range of ranges per commit)
.map!(c => c["commit"]["message"]
.get!string
.matchIssueRefs
.matchIssueRefs(re)
.map!((r) { r.commits = [c]; return r; })
)
// Join to flat list
Expand Down Expand Up @@ -122,6 +128,32 @@ unittest
assert(getIssueRefs([mention(1), fix(2), fix(1)]) == [IssueRef(1, true, [mention(1), fix(1)]), IssueRef(2, true, [fix(2)])]);
}

UserMessage[] checkLegacyIssueRefs(Json[] commits)
{
auto oldHits = commits.getIssueRefs(oldIssueRE).map!(r => r.id);
auto newHits = commits.getIssueRefs(issueRE).map!(r => r.id);
auto onlyOld = oldHits.filter!(id => !newHits.canFind(id));
if (!onlyOld.empty)
return [UserMessage(UserMessage.Type.Warning,
"In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. " ~
"Please add the word \"Bugzilla\" to issue references. For example, `Fix Bugzilla Issue 12345` or `Fix Bugzilla 12345`." ~
"(Reminder: the edit needs to be done in the Git *commit message*, not the GitHub *pull request*.)"
)];
return null;
}

unittest
{
Json fixOld(int id) { return ["commit":["message":"Fix Issue %d".format(id).Json].Json].Json; }
Json fixNew(int id) { return ["commit":["message":"Fix Bugzilla %d".format(id).Json].Json].Json; }

assert( checkLegacyIssueRefs([]).empty);
assert( checkLegacyIssueRefs([fixNew(1)]).empty);
assert( checkLegacyIssueRefs([fixOld(1), fixNew(1)]).empty);
assert(!checkLegacyIssueRefs([fixOld(1)]).empty);
assert(!checkLegacyIssueRefs([fixOld(1), fixNew(2)]).empty);
}

struct Issue
{
int id;
Expand Down

0 comments on commit 835295f

Please sign in to comment.