Flag code analysis issues once per converter used (facebook#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 facebook#21492 for an example of such a case, where `analysis-bot` left a spammy review at facebook#21492 (review).
Pull Request resolved: facebook#21503

Differential Revision: D10219439

Pulled By: hramos

fbshipit-source-id: 75d32ef3bfeaa91ab614763a19494659ad1be0dd

Author: hramos

.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"
-              sudo apt-get install -y shellcheck
-              yarn add @octokit/rest@15.10.0
-              echo -e "\\x1B[36mAnalyzing shell scripts\\x1B[0m"
-              GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_scripts.sh
-            else
-              echo "Skipping shell script analysis."
-            fi
+            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
+            sudo apt-get install -y shellcheck
+            yarn add @octokit/rest@15.10.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" \
+            GITHUB_REPO="$CIRCLE_PROJECT_REPONAME" \
+            GITHUB_PR_NUMBER="$CIRCLE_PR_NUMBER" \
+            ./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[36mAnalyzing code\\x1B[0m"; GITHUB_TOKEN="$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_A""$PUBLIC_ANALYSISBOT_GITHUB_TOKEN_B" ./scripts/circleci/analyze_code.sh
-            else
-              echo "Skipping code analysis."
-            fi
+            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"; yarn add @octokit/rest@15.10.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" \
+            GITHUB_REPO="$CIRCLE_PROJECT_REPONAME" \
+            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
-              cd bots
-              yarn install --non-interactive --cache-folder ~/.cache/yarn
-              DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" yarn danger
-            else
-              echo "Skipping pull request analysis."
-            fi
+            echo -e "\\x1B[36mInstalling additional dependencies\\x1B[0m"
+            cd bots
+            yarn install --non-interactive --cache-folder ~/.cache/yarn
+            echo -e "\\x1B[36mAnalyzing pull request\\x1B[0m"; \
+            DANGER_GITHUB_API_TOKEN="$PUBLIC_PULLBOT_GITHUB_TOKEN_A""$PUBLIC_PULLBOT_GITHUB_TOKEN_B" \
+            yarn danger
           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) {
-  console.error('Missing CIRCLE_PROJECT_USERNAME. Example: facebook');
+if (!process.env.GITHUB_OWNER) {
+  console.error('Missing GITHUB_OWNER. Example: facebook');
   process.exit(1);
 }
-if (!process.env.CIRCLE_PROJECT_REPONAME) {
-  console.error('Missing CIRCLE_PROJECT_REPONAME. Example: react-native');
+if (!process.env.GITHUB_REPO) {
+  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) {
-  if (comments.length === 0) {
-    // Do not leave an empty review.
-    return;
-  }
-
-  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,
-  };
+function sendReview(owner, repo, number, commit_id, body, comments) {
+  if (process.env.GITHUB_TOKEN) {
+    if (comments.length === 0) {
+      // Do not leave an empty review.
+      return;
+    }
 
-  octokit.pullRequests.createReview(opts, function(error, res) {
-    if (error) {
-      console.error(error);
+    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) {
-    console.error(
-      'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e',
+  if (process.env.GITHUB_TOKEN) {
+    octokit.authenticate({
+      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 repo = process.env.CIRCLE_PROJECT_REPONAME;
+  const owner = process.env.GITHUB_OWNER;
+  const repo = process.env.GITHUB_REPO;
 
-  if (!process.env.CIRCLE_PR_NUMBER) {
-    console.error('Missing CIRCLE_PR_NUMBER. Example: 4687');
+  if (!process.env.GITHUB_PR_NUMBER) {
+    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);