From 44878ea9bc6b582e5a1318a85e0b76c23c906f33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=A9ctor=20Ramos?= <hramos@fb.com>
Date: Wed, 5 Dec 2018 13:12:50 -0800
Subject: [PATCH] Fix code analysis bot failure to post lint warnings on pull
 requests

Summary:
The code analysis script takes the results of `eslint .` and filters out any messages for filepaths outside of what is modified in a given pull request. This diff fixes an issue where the bot will fail to post warnings if a pull request contains multiple commits, where the most recent commit is a rebase (e.g. https://github.com/facebook/react-native/pull/22470/commits/63c00f20a7f43db436f061c478b12a4f9aa2bd5a in https://github.com/facebook/react-native/pull/22470). This happens because the script looks for files changed in the most recent commit in a PR.

In this diff, we switch to a new GitHub API that returns the list of all files changed by a PR, obviating the need to go through individual commits in a PR to look for changed files.

Reviewed By: TheSavior

Differential Revision: D13324154

fbshipit-source-id: f9f50028439d1969b0feea65f0b3e8bf75ac1a33
---
 .circleci/config.yml                  |  4 ++--
 scripts/circleci/code-analysis-bot.js | 28 +++++++++++++--------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 3438e2279..44da460f0 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -574,7 +574,7 @@ jobs:
           command: |
             echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
             sudo apt-get install -y shellcheck
-            yarn add @octokit/rest@15.10.0
+            yarn add @octokit/rest@15.18.0
             echo -e "\\x1B[36mAnalyzing shell scripts\\x1B[0m"; \
             GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" \
             GITHUB_OWNER="$CIRCLE_PROJECT_USERNAME" \
@@ -586,7 +586,7 @@ jobs:
       - run:
           name: Analyze Code
           command: |
-            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.0
+            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.18.0
             echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; \
             GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" \
             GITHUB_OWNER="$CIRCLE_PROJECT_USERNAME" \
diff --git a/scripts/circleci/code-analysis-bot.js b/scripts/circleci/code-analysis-bot.js
index 86f21ab69..eb003f5fb 100644
--- a/scripts/circleci/code-analysis-bot.js
+++ b/scripts/circleci/code-analysis-bot.js
@@ -118,19 +118,17 @@ function getShaFromPullRequest(owner, repo, number, callback) {
   });
 }
 
-function getFilesFromCommit(owner, repo, sha, callback) {
-  octokit.repos.getCommit({owner, repo, sha}, (error, res) => {
-    if (error) {
-      console.error(error);
-      return;
-    }
-    // A merge commit should not have any new changes to report
-    if (res.parents && res.parents.length > 1) {
-      return;
-    }
-
-    callback(res.data.files);
-  });
+function getFilesFromPullRequest(owner, repo, number, callback) {
+  octokit.pullRequests.listFiles(
+    {owner, repo, number, per_page: 100},
+    (error, res) => {
+      if (error) {
+        console.error(error);
+        return;
+      }
+      callback(res.data);
+    },
+  );
 }
 
 /**
@@ -227,7 +225,7 @@ function main(messages, owner, repo, number) {
   }
 
   getShaFromPullRequest(owner, repo, number, sha => {
-    getFilesFromCommit(owner, repo, sha, files => {
+    getFilesFromPullRequest(owner, repo, number, files => {
       let comments = [];
       let convertersUsed = [];
       files.filter(file => messages[file.filename]).forEach(file => {
@@ -256,7 +254,7 @@ function main(messages, owner, repo, number) {
       });
 
       sendReview(owner, repo, number, sha, body, comments);
-    }); // getFilesFromCommit
+    }); // getFilesFromPullRequest
   }); // getShaFromPullRequest
 }