feat(snippet-bot): add a summary of the region tag changes on PRs

packages/snippet-bot/__snapshots__/snippet-bot.js

@@ -21,3 +21,8 @@ exports[
   body:
     '<!-- probot comment [11359653]-->\n\n## snippet-bot scan result\nFailed running the full scan: Error: Failed to scan files: unexpected response Forbidden.\n\n---\nReport generated by [snippet-bot](https://github.com/apps/snippet-bot).\nIf you find problems with this result, please file an issue at:\nhttps://github.com/googleapis/repo-automation-bots/issues.\n',
 };
+
+exports['snippet-bot responds to PR sets a "failure" context on PR 2'] = {
+  body:
+    '<!-- probot comment [11237253]-->\nHere is the summary of changes.\n<details>\n  <summary>You added 3 region tags.</summary>\n\n  - [`datastore_incomplete_key2` in `datastore/cloud-client/snippets.py`](https://github.com/tmatsuo/repo-automation-bots/blob/ce03c1b7977aadefb5f6afc09901f106ee6ece6a/datastore/cloud-client/snippets.py#L22)\n- [`datastore_named_key2` in `datastore/cloud-client/snippets.py`](https://github.com/tmatsuo/repo-automation-bots/blob/ce03c1b7977aadefb5f6afc09901f106ee6ece6a/datastore/cloud-client/snippets.py#L27)\n- [`datastore_key_with_parent2` in `datastore/cloud-client/snippets.py`](https://github.com/tmatsuo/repo-automation-bots/blob/ce03c1b7977aadefb5f6afc09901f106ee6ece6a/datastore/cloud-client/snippets.py#L32)\n\n</details>\n\n<details>\n  <summary>You deleted 3 region tags.\n</summary>\n\n  - [`datastore_incomplete_key` in `datastore/cloud-client/snippets.py`](https://github.com/tmatsuo/repo-automation-bots/blob/48d47a91300728008c9712d6e793a6ed5d86e01d/datastore/cloud-client/snippets.py#L22)\n- [`datastore_named_key` in `datastore/cloud-client/snippets.py`](https://github.com/tmatsuo/repo-automation-bots/blob/48d47a91300728008c9712d6e793a6ed5d86e01d/datastore/cloud-client/snippets.py#L27)\n- [`datastore_key_with_parent` in `datastore/cloud-client/snippets.py`](https://github.com/tmatsuo/repo-automation-bots/blob/48d47a91300728008c9712d6e793a6ed5d86e01d/datastore/cloud-client/snippets.py#L32)\n\n</details>\n\n',
+};

packages/snippet-bot/package.json

@@ -30,6 +30,7 @@
   "dependencies": {
     "gcf-utils": "^6.1.1",
     "node-fetch": "^2.6.0",
+    "parse-diff": "^0.7.1",
     "tar": "^6.0.5",
     "tmp-promise": "^3.0.2"
   },

packages/snippet-bot/src/region-tag-parser.ts

@@ -12,16 +12,98 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// We'll add error details later.
+import parseDiff from 'parse-diff';
+
+/**
+ * The result for unmatched region tag checks.
+ */
 export interface ParseResult {
   result: boolean;
   messages: string[];
   tagsFound: boolean;
 }
 
-const START_TAG_REGEX = /\[START ([^\]]*)\]/;
+type ChangeTypes = 'add' | 'del';
+
+/**
+ * A single region tag change in a pull request.
+ */
+export interface Change {
+  type: ChangeTypes;
+  regionTag: string;
+  owner: string;
+  repo: string;
+  file?: string;
+  sha: string;
+  line: number;
+}
+
+/**
+ * The summary of the region tag changes in a pull request.
+ */
+export interface ChangesInPullRequest {
+  changes: Change[];
+  added: number;
+  deleted: number;
+}
+
+export const START_TAG_REGEX = /\[START ([^\]]*)\]/;
 const END_TAG_REGEX = /\[END ([^\]]*)\]/;
 
+/**
+ * Detects region tag changes in a pull request and return the summary.
+ */
+export function parseRegionTagsInPullRequest(
+  diff: string,
+  owner: string,
+  repo: string,
+  sha: string,
+  headOwner: string,
+  headRepo: string,
+  headSha: string
+): ChangesInPullRequest {
+  const changes: Change[] = [];
+  const ret = {
+    changes: changes,
+    added: 0,
+    deleted: 0,
+  };
+
+  const diffResult = parseDiff(diff);
+  for (const file of diffResult) {
+    for (const chunk of file.chunks) {
+      for (const change of chunk.changes) {
+        if (change.type === 'normal') {
+          continue;
+        }
+        // We only track add/deletion of start tags.
+        const startMatch = change.content.match(START_TAG_REGEX);
+        if (startMatch) {
+          if (change.type === 'add') {
+            ret.added += 1;
+          }
+          if (change.type === 'del') {
+            ret.deleted += 1;
+          }
+          ret.changes.push({
+            type: change.type === 'del' ? 'del' : 'add',
+            regionTag: startMatch[1],
+            owner: change.type === 'del' ? owner : headOwner,
+            repo: change.type === 'del' ? repo : headRepo,
+            file: change.type === 'del' ? file.from : file.to,
+            sha: change.type === 'del' ? sha : headSha,
+            line: change.ln,
+          });
+        }
+      }
+    }
+  }
+  return ret;
+}
+
+/**
+ * Parses a single file and checks unmatched region tags.
+ */
 export function parseRegionTags(
   contents: string,
   filename: string

packages/snippet-bot/src/snippet-bot.ts

@@ -17,6 +17,8 @@
 
 import {Application} from 'probot';
 import {parseRegionTags} from './region-tag-parser';
+import {parseRegionTagsInPullRequest} from './region-tag-parser';
+import {Change} from './region-tag-parser';
 import {logger} from 'gcf-utils';
 import fetch from 'node-fetch';
 import tmp from 'tmp-promise';
@@ -70,6 +72,10 @@ class Configuration {
   }
 }
 
+/**
+ * Formats the full scan report with the comment mark, so that it can
+ * preserve original contents.
+ */
 function formatBody(
   originalBody: string,
   commentMark: string,
@@ -91,6 +97,27 @@ https://github.com/googleapis/repo-automation-bots/issues.
 `;
 }
 
+/**
+ * It formats the summary and detail as an expandable UI in the markdown.
+ */
+function formatExpandable(summary: string, detail: string): string {
+  return `<details>
+  <summary>${summary}</summary>
+
+  ${detail}
+</details>
+
+`;
+}
+
+/**
+ * It formats a region tag change as a markdown with a permalink to the code.
+ */
+function formatChangedFile(change: Change): string {
+  const url = `https://github.com/${change.owner}/${change.repo}/blob/${change.sha}/${change.file}#L${change.line}`;
+  return `[\`${change.regionTag}\` in \`${change.file}\`](${url})`;
+}
+
 async function downloadFile(url: string, file: string) {
   const response = await fetch(url);
   if (response.ok) {
@@ -149,13 +176,14 @@ export = (app: Application) => {
         ...configOptions,
       });
       logger.info({config: configuration});
+      const installationId = context.payload.installation.id;
+      const commentMark = `<!-- probot comment [${installationId}]-->`;
+      const owner = context.payload.repository.owner.login;
+      const repo = context.payload.repository.name;
+
       if (context.payload.issue?.title.includes(FULL_SCAN_ISSUE_TITLE)) {
         // full scan start
-        const installationId = context.payload.installation.id;
-        const commentMark = `<!-- probot comment [${installationId}]-->`;
         const issueNumber = context.payload.issue.number;
-        const owner = context.payload.repository.owner.login;
-        const repo = context.payload.repository.name;
 
         const url = `https://github.com/${owner}/${repo}/tarball/${defaultBranch}`;
         const tmpDir = tmp.dirSync();
@@ -310,7 +338,6 @@ ${bodyDetail}`
           'utf8'
         );
 
-        logger.info({fileContents: fileContents});
         const parseResult = parseRegionTags(fileContents, file.filename);
         if (!parseResult.result) {
           mismatchedTags = true;
@@ -346,6 +373,78 @@ ${bodyDetail}`
       if (tagsFound) {
         await context.github.checks.create(checkParams);
       }
+
+      // Parse the PR diff and recognize added/deleted region tags.
+      const response = await fetch(context.payload.pull_request.diff_url);
+      const diff = await response.text();
+
+      const result = parseRegionTagsInPullRequest(
+        diff,
+        context.payload.pull_request.base.repo.owner.login,
+        context.payload.pull_request.base.repo.name,
+        context.payload.pull_request.base.sha,
+        context.payload.pull_request.head.repo.owner.login,
+        context.payload.pull_request.head.repo.name,
+        context.payload.pull_request.head.sha
+      );
+
+      if (result.changes.length === 0) {
+        return;
+      }
+
+      // Add or update a comment on the PR.
+      const prNumber = context.payload.pull_request.number;
+      let commentBody = 'Here is the summary of changes.\n';
+      if (result.added > 0) {
+        const plural = result.added === 1 ? '' : 's';
+        const summary = `You added ${result.added} region tag${plural}.`;
+        let detail = '';
+        for (const change of result.changes) {
+          if (change.type === 'add') {
+            detail += `- ${formatChangedFile(change)}\n`;
+          }
+        }
+        commentBody += formatExpandable(summary, detail);
+      }
+      if (result.deleted > 0) {
+        const plural = result.deleted === 1 ? '' : 's';
+        const summary = `You deleted ${result.deleted} region tag${plural}.\n`;
+        let detail = '';
+        for (const change of result.changes) {
+          if (change.type === 'del') {
+            detail += `- ${formatChangedFile(change)}\n`;
+          }
+        }
+        commentBody += formatExpandable(summary, detail);
+      }
+
+      const listCommentsResponse = await context.github.issues.listComments({
+        owner: owner,
+        repo: repo,
+        per_page: 50,
+        issue_number: prNumber,
+      });
+      let found = false;
+      for (const comment of listCommentsResponse.data) {
+        if (comment.body.includes(commentMark)) {
+          // We found the existing comment, so updating it
+          await context.github.issues.updateComment({
+            owner: owner,
+            repo: repo,
+            comment_id: comment.id,
+            body: `${commentMark}\n${commentBody}`,
+          });
+          found = true;
+        }
+      }
+      if (!found) {
+        await context.github.issues.createComment({
+          owner: owner,
+          repo: repo,
+          issue_number: prNumber,
+          body: `${commentMark}\n${commentBody}`,
+        });
+      }
     }
   );
 };

packages/snippet-bot/test/fixtures/diff.txt

@@ -0,0 +1,38 @@
+diff --git a/datastore/cloud-client/snippets.py b/datastore/cloud-client/snippets.py
+index 198d0257595..6048151ca9f 100644
+--- a/datastore/cloud-client/snippets.py
++++ b/datastore/cloud-client/snippets.py
+@@ -21,28 +21,28 @@
+
+
+ def incomplete_key(client):
+-    # [START datastore_incomplete_key]
++    # [START datastore_incomplete_key2]
+     key = client.key('Task')
+-    # [END datastore_incomplete_key]
++    # [END datastore_incomplete_key2]
+
+     return key
+
+
+ def named_key(client):
+-    # [START datastore_named_key]
++    # [START datastore_named_key2]
+     key = client.key('Task', 'sample_task')
+-    # [END datastore_named_key]
++    # [END datastore_named_key2]
+
+     return key
+
+
+ def key_with_parent(client):
+-    # [START datastore_key_with_parent]
++    # [START datastore_key_with_parent2]
+     key = client.key('TaskList', 'default', 'Task', 'sample_task')
+     # Alternatively
+     parent_key = client.key('TaskList', 'default')
+     key = client.key('Task', 'sample_task', parent=parent_key)
+-    # [END datastore_key_with_parent]
++    # [END datastore_key_with_parent2]
+
+     return key

packages/snippet-bot/test/region-tag-parser.ts

@@ -0,0 +1,51 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+/* eslint-disable @typescript-eslint/no-var-requires */
+/* eslint-disable node/no-extraneous-import */
+
+import {parseRegionTagsInPullRequest} from '../src/region-tag-parser';
+
+import {resolve} from 'path';
+import * as fs from 'fs';
+import assert from 'assert';
+import {describe, it} from 'mocha';
+
+const fixturesPath = resolve(__dirname, '../../test/fixtures');
+
+describe('region-tag-parser', () => {
+  const diff = fs.readFileSync(resolve(fixturesPath, 'diff.txt'), 'utf8');
+  describe('parses a diff', () => {
+    it('returns a correct result', () => {
+      const result = parseRegionTagsInPullRequest(
+        diff,
+        'owner',
+        'repo',
+        'sha',
+        'headOwner',
+        'headRepo',
+        'headSha'
+      );
+      assert.strictEqual(3, result.added);
+      assert.strictEqual(3, result.deleted);
+      assert.strictEqual('owner', result.changes[0].owner);
+      assert.strictEqual('repo', result.changes[0].repo);
+      assert.strictEqual('sha', result.changes[0].sha);
+      assert.strictEqual('headOwner', result.changes[1].owner);
+      assert.strictEqual('headRepo', result.changes[1].repo);
+      assert.strictEqual('headSha', result.changes[1].sha);
+    });
+  });
+});