Flag code analysis issues once per converter used (#21503)

Summary:
Fixes issue where the bot would leave multiple lines in the top review comment if, say, eslint found 30 issues, there would be 30 mentions of eslint having found issues.

See https://github.com/facebook/react-native/pull/21492 for an example of such a case, where `analysis-bot` left a spammy review at https://github.com/facebook/react-native/pull/21492#pullrequestreview-161837975.
Pull Request resolved: https://github.com/facebook/react-native/pull/21503

Differential Revision: D10219439

Pulled By: hramos

fbshipit-source-id: 75d32ef3bfeaa91ab614763a19494659ad1be0dd

.circleci/config.yml

@@ -591,38 +591,38 @@ jobs:
       - run:
           name: Analyze Shell Scripts
           command: |
-            if [ -n "$CIRCLE_PR_NUMBER" ]; then
+            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
-              echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
+            sudo apt-get install -y shellcheck
-              sudo apt-get install -y shellcheck
+            yarn add @octokit/rest@15.10.0
-              yarn add @octokit/rest@15.10.0
+            echo -e "\\x1B[36mAnalyzing shell scripts\\x1B[0m"; \
-              echo -e "\\x1B[36mAnalyzing shell scripts\\x1B[0m"
+            GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" \
-              GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_scripts.sh
+            GITHUB_OWNER="$CIRCLE_PROJECT_USERNAME" \
-            else
+            GITHUB_REPO="$CIRCLE_PROJECT_REPONAME" \
-              echo "Skipping shell script analysis."
+            GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" \
-            fi
+            ./scripts/circleci/analyze_scripts.sh
           when: always
 
       - run:
           name: Analyze Code
           command: |
-            if [ -n "$CIRCLE_PR_NUMBER" ]; then
+            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.10.0
+            echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; \
-              echo -e "\\x1B[36mAnalyzing code\\x1B[0m"; GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_code.sh
+            GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" \
-            else
+            GITHUB_OWNER="$CIRCLE_PROJECT_USERNAME" \
-              echo "Skipping code analysis."
+            GITHUB_REPO="$CIRCLE_PROJECT_REPONAME" \
-            fi
+            GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" \
+            ./scripts/circleci/analyze_code.sh
           when: always
 
       - run:
           name: Analyze Pull Request
           command: |
-            if [ -n "$CIRCLE_PR_NUMBER" ]; then
+            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
-              cd bots
+            cd bots
-              yarn install --non-interactive --cache-folder ~/.cache/yarn
+            yarn install --non-interactive --cache-folder ~/.cache/yarn
-              DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" yarn danger
+            echo -e "\\x1B[36mAnalyzing pull request\\x1B[0m"; \
-            else
+            DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" \
-              echo "Skipping pull request analysis."
+            yarn danger
-            fi
           when: always
       - save-cache: *save-cache-analysis
 
@@ -730,6 +730,8 @@ workflows:
             tags:
               only: /v[0-9]+(\.[0-9]+)*(\-rc(\.[0-9]+)?)?/
 
-      # Run code checks
+      # Run code checks on PRs from forks
       - analyze_pr:
-          filters: *filter-ignore-master-stable
+          filters:
+            branches:
+              only: /^pull\/.*$/

scripts/circleci/analyze_code.sh

@@ -9,7 +9,7 @@ cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run fl
 # check status
 STATUS=$?
 if [ $STATUS == 0 ]; then
-  echo "Code analyzed successfully"
+  echo "Code analyzed successfully."
 else
-  echo "Code analysis failed, error status $STATUS"
+  echo "Code analysis failed, error status $STATUS."
 fi

scripts/circleci/analyze_scripts.sh

@@ -13,7 +13,7 @@ cat <(echo shellcheck; printf '%s\n' "${results[@]}" | jq .,[] | jq -s . | jq --
 # check status
 STATUS=$?
 if [ $STATUS == 0 ]; then
-  echo "Shell scripts analyzed successfully"
+  echo "Shell scripts analyzed successfully."
 else
-  echo "Shell script analysis failed, error status $STATUS"
+  echo "Shell script analysis failed, error status $STATUS."
 fi

scripts/circleci/code-analysis-bot.js

@@ -9,12 +9,12 @@
 
 'use strict';
 
-if (!process.env.CIRCLE_PROJECT_USERNAME) {
+if (!process.env.GITHUB_OWNER) {
-  console.error('Missing CIRCLE_PROJECT_USERNAME. Example: facebook');
+  console.error('Missing GITHUB_OWNER. Example: facebook');
   process.exit(1);
 }
-if (!process.env.CIRCLE_PROJECT_REPONAME) {
+if (!process.env.GITHUB_REPO) {
-  console.error('Missing CIRCLE_PROJECT_REPONAME. Example: react-native');
+  console.error('Missing GITHUB_REPO. Example: react-native');
   process.exit(1);
 }
 
@@ -162,35 +162,51 @@ function getLineMapFromPatch(patchString) {
   return lineMap;
 }
 
-function sendReview(owner, repo, number, commit_id, comments, convertersUsed) {
+function sendReview(owner, repo, number, commit_id, body, comments) {
-  if (comments.length === 0) {
+  if (process.env.GITHUB_TOKEN) {
-    // Do not leave an empty review.
+    if (comments.length === 0) {
-    return;
+      // Do not leave an empty review.
-  }
-
-  let body = '**Code analysis results:**\n\n';
-  convertersUsed.forEach(converter => {
-    body += '* `' + converter + '` found some issues.\n';
-  });
-
-  const event = 'REQUEST_CHANGES';
-
-  const opts = {
-    owner,
-    repo,
-    number,
-    commit_id,
-    body,
-    event,
-    comments,
-  };
-
-  octokit.pullRequests.createReview(opts, function(error, res) {
-    if (error) {
-      console.error(error);
       return;
     }
-  });
+
+    const event = 'REQUEST_CHANGES';
+
+    const opts = {
+      owner,
+      repo,
+      number,
+      commit_id,
+      body,
+      event,
+      comments,
+    };
+
+    octokit.pullRequests.createReview(opts, function(error, res) {
+      if (error) {
+        console.error(error);
+        return;
+      }
+    });
+  } else {
+    if (comments.length === 0) {
+      console.log('No issues found.');
+      return;
+    }
+
+    if (process.env.CIRCLE_CI) {
+      console.error(
+        'Code analysis found issues, but the review cannot be posted to GitHub without an access token.',
+      );
+      process.exit(1);
+    }
+
+    let results = body + '\n';
+    comments.forEach(comment => {
+      results +=
+        comment.path + ':' + comment.position + ': ' + comment.body + '\n';
+    });
+    console.log(results);
+  }
 }
 
 function main(messages, owner, repo, number) {
@@ -199,18 +215,17 @@ function main(messages, owner, repo, number) {
     return;
   }
 
-  if (!process.env.GITHUB_TOKEN) {
+  if (process.env.GITHUB_TOKEN) {
-    console.error(
+    octokit.authenticate({
-      'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e',
+      type: 'oauth',
+      token: process.env.GITHUB_TOKEN,
+    });
+  } else {
+    console.log(
+      'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e. Review feedback with code analysis results will not be provided on GitHub.',
     );
-    process.exit(1);
   }
 
-  octokit.authenticate({
-    type: 'oauth',
-    token: process.env.GITHUB_TOKEN,
-  });
-
   getShaFromPullRequest(owner, repo, number, sha => {
     getFilesFromCommit(owner, repo, sha, files => {
       let comments = [];
@@ -234,7 +249,13 @@ function main(messages, owner, repo, number) {
         }); // forEach
       }); // filter
 
-      sendReview(owner, repo, number, sha, comments, convertersUsed);
+      let body = '**Code analysis results:**\n\n';
+      const uniqueconvertersUsed = [...new Set(convertersUsed)];
+      uniqueconvertersUsed.forEach(converter => {
+        body += '* `' + converter + '` found some issues.\n';
+      });
+
+      sendReview(owner, repo, number, sha, body, comments);
     }); // getFilesFromCommit
   }); // getShaFromPullRequest
 }
@@ -287,16 +308,16 @@ process.stdin.on('end', function() {
     delete messages[absolutePath];
   }
 
-  const owner = process.env.CIRCLE_PROJECT_USERNAME;
+  const owner = process.env.GITHUB_OWNER;
-  const repo = process.env.CIRCLE_PROJECT_REPONAME;
+  const repo = process.env.GITHUB_REPO;
 
-  if (!process.env.CIRCLE_PR_NUMBER) {
+  if (!process.env.GITHUB_PR_NUMBER) {
-    console.error('Missing CIRCLE_PR_NUMBER. Example: 4687');
+    console.error('Missing GITHUB_PR_NUMBER. Example: 4687');
     // for master branch, don't throw an error
     process.exit(0);
   }
 
-  const number = process.env.CIRCLE_PR_NUMBER;
+  const number = process.env.GITHUB_PR_NUMBER;
 
   // intentional lint warning to make sure that the bot is working :)
   main(messages, owner, repo, number);