Use ShellCheck (#19681)

Summary:
As recommended in https://circleci.com/docs/2.0/using-shell-scripts/#use-shellcheck

It will only run on PRs for now.
Pull Request resolved: https://github.com/facebook/react-native/pull/19681

Differential Revision: D10111711

Pulled By: hramos

fbshipit-source-id: e980a526561dced79e5197a11cfb41a3eba9be8b

.circleci/config.yml

@@ -585,9 +585,23 @@ jobs:
       - checkout
       - run: *setup-artifacts
 
-      - restore-cache: *restore-yarn-cache
+      - restore-cache: *restore-cache-analysis
       - run: *yarn
 
+      - 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
+          when: always
+
       - run:
           name: Analyze Code
           command: |
@@ -599,7 +613,6 @@ jobs:
             fi
           when: always
 
-      - restore-cache: *restore-cache-analysis
       - run:
           name: Analyze Pull Request
           command: |

ContainerShip/scripts/run-android-docker-instrumentation-tests.sh

@@ -6,7 +6,7 @@ mount -o remount,exec /dev/shm
 AVD_UUID=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 8 | head -n 1)
 
 # create virtual device
-echo no | android create avd -n $AVD_UUID -f -t android-19 --abi default/armeabi-v7a
+echo no | android create avd -n "$AVD_UUID" -f -t android-19 --abi default/armeabi-v7a
 
 # emulator setup
 emulator64-arm -avd $AVD_UUID -no-skin -no-audio -no-window -no-boot-anim &
@@ -28,8 +28,9 @@ watchman shutdown-server
 node local-cli/cli.js bundle --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js
 
 # build test APK
+# shellcheck disable=SC1091
 source ./scripts/android-setup.sh && NO_BUCKD=1 retry3 buck install ReactAndroid/src/androidTest/buck-runner:instrumentation-tests --config build.threads=1
 
 # run installed apk with tests
-node ./ContainerShip/scripts/run-android-ci-instrumentation-tests.js $*
+node ./ContainerShip/scripts/run-android-ci-instrumentation-tests.js "$*"
 exit $?

ContainerShip/scripts/run-android-docker-unit-tests.sh

@@ -9,4 +9,4 @@ mount -o remount,exec /dev/shm
 set -x
 
 # run unit tests
-buck test ReactAndroid/src/test/... --config build.threads=$UNIT_TESTS_BUILD_THREADS
+buck test ReactAndroid/src/test/... --config build.threads="$UNIT_TESTS_BUILD_THREADS"

ContainerShip/scripts/run-ci-e2e-tests.sh

@@ -128,6 +128,7 @@ function e2e_suite() {
         emulator64-arm -avd "$AVD_UUID" -no-skin -no-audio -no-window -no-boot-anim &
 
         bootanim=""
+        # shellcheck disable=SC2076
         until [[ "$bootanim" =~ "stopped" ]]; do
             sleep 5
             bootanim=$(adb -e shell getprop init.svc.bootanim 2>&1)
@@ -210,15 +211,15 @@ function e2e_suite() {
       fi
 
       # kill packager process
-      if kill -0 $SERVER_PID; then
+      if kill -0 "$SERVER_PID"; then
         echo "Killing packager $SERVER_PID"
-        kill -9 $SERVER_PID
+        kill -9 "$SERVER_PID"
       fi
 
       # kill appium process
-      if kill -0 $APPIUM_PID; then
+      if kill -0 "$APPIUM_PID"; then
         echo "Killing appium $APPIUM_PID"
-        kill -9 $APPIUM_PID
+        kill -9 "$APPIUM_PID"
       fi
 
     fi

ContainerShip/scripts/run-instrumentation-tests-via-adb-shell.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-
+# shellcheck disable=SC1117
 # Python script to run instrumentation tests, copied from https://github.com/circleci/circle-dummy-android
 # Example: ./scripts/run-android-instrumentation-tests.sh com.facebook.react.tests com.facebook.react.tests.ReactPickerTestCase
 #
@@ -9,7 +9,7 @@ export PATH="$ANDROID_HOME/platform-tools:$ANDROID_HOME/tools:$PATH"
 adb logcat -c
 
 # run tests and check output
-python - $1 $2 << END
+python - "$1" "$2" << END
 
 import re
 import subprocess as sp
@@ -24,7 +24,7 @@ test_class = None
 
 if len(sys.argv) > 2:
   test_class = sys.argv[2]
-  
+
 def update():
   # prevent CircleCI from killing the process for inactivity
   while not done:
@@ -38,10 +38,10 @@ t.start()
 def run():
   sp.Popen(['adb', 'wait-for-device']).communicate()
   if (test_class != None):
-    p = sp.Popen('adb shell am instrument -w -e class %s %s/android.support.test.runner.AndroidJUnitRunner' 
+    p = sp.Popen('adb shell am instrument -w -e class %s %s/android.support.test.runner.AndroidJUnitRunner'
       % (test_class, test_app), shell=True, stdout=sp.PIPE, stderr=sp.PIPE, stdin=sp.PIPE)
   else :
-    p = sp.Popen('adb shell am instrument -w %s/android.support.test.runner.AndroidJUnitRunner' 
+    p = sp.Popen('adb shell am instrument -w %s/android.support.test.runner.AndroidJUnitRunner'
       % (test_app), shell=True, stdout=sp.PIPE, stderr=sp.PIPE, stdin=sp.PIPE)
   return p.communicate()
 

scripts/circleci/analyze_scripts.sh

@@ -0,0 +1,19 @@
+#!/bin/bash
+# Copyright (c) Facebook, Inc. and its affiliates.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+IFS=$'\n'
+
+results=( "$(find . -type f -not -path "*node_modules*" -not -path "*third-party*" -name '*.sh' -exec sh -c  'shellcheck "$1" -f json' -- {} \;)" )
+
+cat <(echo shellcheck; printf '%s\n' "${results[@]}" | jq .,[] | jq -s . | jq --compact-output --raw-output '[ (.[] | .[] | . ) ]') | node bots/code-analysis-bot.js
+
+# check status
+STATUS=$?
+if [ $STATUS == 0 ]; then
+  echo "Shell scripts analyzed successfully"
+else
+  echo "Shell script analysis failed, error status $STATUS"
+fi

scripts/circleci/code-analysis-bot.js

@@ -17,27 +17,11 @@ if (!process.env.CIRCLE_PROJECT_REPONAME) {
   console.error('Missing CIRCLE_PROJECT_REPONAME. Example: react-native');
   process.exit(1);
 }
-if (!process.env.GITHUB_TOKEN) {
-  console.error(
-    'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e',
-  );
-  process.exit(1);
-}
-if (!process.env.CIRCLE_PR_NUMBER) {
-  console.error('Missing CIRCLE_PR_NUMBER. Example: 4687');
-  // for master branch, don't throw an error
-  process.exit(0);
-}
 
 // https://octokit.github.io/rest.js/
 const octokit = require('@octokit/rest')();
 
-var path = require('path');
-
-octokit.authenticate({
-  type: 'oauth',
-  token: process.env.GITHUB_TOKEN,
-});
+const path = require('path');
 
 function push(arr, key, value) {
   if (!arr[key]) {
@@ -58,9 +42,9 @@ function push(arr, key, value) {
  * This is an object where the keys are the path of the files and values
  * is an array of objects of the shape message and line.
  */
-var converters = {
+const converters = {
   raw: function(output, input) {
-    for (var key in input) {
+    for (let key in input) {
       input[key].forEach(function(message) {
         push(output, key, message);
       });
@@ -76,6 +60,7 @@ var converters = {
       push(output, error.message[0].path, {
         message: error.message.map(message => message.descr).join(' '),
         line: error.message[0].line,
+        converter: 'flow',
       });
     });
   },
@@ -90,10 +75,36 @@ var converters = {
         push(output, file.filePath, {
           message: message.ruleId + ': ' + message.message,
           line: message.line,
+          converter: 'eslint',
         });
       });
     });
   },
+
+  shellcheck: function(output, input) {
+    if (!input) {
+      return;
+    }
+
+    input.forEach(function(report) {
+      push(output, report.file, {
+        message:
+          '**[SC' +
+          report.code +
+          '](https://github.com/koalaman/shellcheck/wiki/SC' +
+          report.code +
+          '):** (' +
+          report.level +
+          ') ' +
+          report.message,
+        line: report.line,
+        endLine: report.endLine,
+        column: report.column,
+        endColumn: report.endColumn,
+        converter: 'shellcheck',
+      });
+    });
+  },
 };
 
 function getShaFromPullRequest(owner, repo, number, callback) {
@@ -129,9 +140,9 @@ function getFilesFromCommit(owner, repo, sha, callback) {
  * in the patch file
  */
 function getLineMapFromPatch(patchString) {
-  var diffLineIndex = 0;
-  var fileLineIndex = 0;
-  var lineMap = {};
+  let diffLineIndex = 0;
+  let fileLineIndex = 0;
+  let lineMap = {};
 
   patchString.split('\n').forEach(line => {
     if (line.match(/^@@/)) {
@@ -151,14 +162,17 @@ function getLineMapFromPatch(patchString) {
   return lineMap;
 }
 
-function sendReview(owner, repo, number, commit_id, comments) {
+function sendReview(owner, repo, number, commit_id, comments, convertersUsed) {
   if (comments.length === 0) {
     // Do not leave an empty review.
     return;
   }
 
-  const body =
-    '`eslint` found some issues. You may run `yarn prettier` or `npm run prettier` to fix these.';
+  let body = '**Code analysis results:**\n\n';
+  convertersUsed.forEach(converter => {
+    body += '* `' + converter + '` found some issues.\n';
+  });
+
   const event = 'REQUEST_CHANGES';
 
   const opts = {
@@ -179,71 +193,59 @@ function sendReview(owner, repo, number, commit_id, comments) {
   });
 }
 
-function sendComment(owner, repo, number, sha, filename, lineMap, message) {
-  if (!lineMap[message.line]) {
-    // Do not send messages on lines that did not change
-    return;
-  }
-
-  var opts = {
-    owner,
-    repo,
-    number,
-    sha,
-    commit_id: sha,
-    path: filename,
-    position: lineMap[message.line],
-    body: message.message,
-  };
-  octokit.pullRequests.createComment(opts, function(error, res) {
-    if (error) {
-      console.error(error);
-      return;
-    }
-  });
-  console.log('Sending comment', opts);
-}
-
 function main(messages, owner, repo, number) {
   // No message, we don't need to do anything :)
   if (Object.keys(messages).length === 0) {
     return;
   }
 
+  if (!process.env.GITHUB_TOKEN) {
+    console.error(
+      'Missing GITHUB_TOKEN. Example: 5fd88b964fa214c4be2b144dc5af5d486a2f8c1e',
+    );
+    process.exit(1);
+  }
+
+  octokit.authenticate({
+    type: 'oauth',
+    token: process.env.GITHUB_TOKEN,
+  });
+
   getShaFromPullRequest(owner, repo, number, sha => {
     getFilesFromCommit(owner, repo, sha, files => {
-      var comments = [];
+      let comments = [];
+      let convertersUsed = [];
       files.filter(file => messages[file.filename]).forEach(file => {
         // github api sometimes does not return a patch on large commits
         if (!file.patch) {
           return;
         }
-        var lineMap = getLineMapFromPatch(file.patch);
+        const lineMap = getLineMapFromPatch(file.patch);
         messages[file.filename].forEach(message => {
           if (lineMap[message.line]) {
-            var comment = {
+            const comment = {
               path: file.filename,
               position: lineMap[message.line],
               body: message.message,
             };
-
+            convertersUsed.push(message.converter);
             comments.push(comment);
           }
         }); // forEach
       }); // filter
 
-      sendReview(owner, repo, number, sha, comments);
+      sendReview(owner, repo, number, sha, comments, convertersUsed);
     }); // getFilesFromCommit
   }); // getShaFromPullRequest
 }
 
-var content = '';
+let content = '';
 process.stdin.resume();
 process.stdin.on('data', function(buf) {
   content += buf.toString();
 });
 process.stdin.on('end', function() {
-  var messages = {};
+  let messages = {};
 
   // Since we send a few http requests to setup the process, we don't want
   // to run this file one time per code analysis tool. Instead, we write all
@@ -259,13 +261,13 @@ process.stdin.on('end', function() {
   //
   //   cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; flow --json) | node code-analysis-bot.js
 
-  var lines = content.trim().split('\n');
-  for (var i = 0; i < Math.ceil(lines.length / 2); ++i) {
-    var converter = converters[lines[i * 2]];
+  const lines = content.trim().split('\n');
+  for (let i = 0; i < Math.ceil(lines.length / 2); ++i) {
+    const converter = converters[lines[i * 2]];
     if (!converter) {
       throw new Error('Unknown converter ' + lines[i * 2]);
     }
-    var json;
+    let json;
     try {
       json = JSON.parse(lines[i * 2 + 1]);
     } catch (e) {}
@@ -275,9 +277,9 @@ process.stdin.on('end', function() {
 
   // The paths are returned in absolute from code analysis tools but github works
   // on paths relative from the root of the project. Doing the normalization here.
-  var pwd = path.resolve('.');
-  for (var absolutePath in messages) {
-    var relativePath = path.relative(pwd, absolutePath);
+  const pwd = path.resolve('.');
+  for (let absolutePath in messages) {
+    const relativePath = path.relative(pwd, absolutePath);
     if (relativePath === absolutePath) {
       continue;
     }
@@ -285,9 +287,16 @@ process.stdin.on('end', function() {
     delete messages[absolutePath];
   }
 
-  var owner = process.env.CIRCLE_PROJECT_USERNAME;
-  var repo = process.env.CIRCLE_PROJECT_REPONAME;
-  var number = process.env.CIRCLE_PR_NUMBER;
+  const owner = process.env.CIRCLE_PROJECT_USERNAME;
+  const repo = process.env.CIRCLE_PROJECT_REPONAME;
+
+  if (!process.env.CIRCLE_PR_NUMBER) {
+    console.error('Missing CIRCLE_PR_NUMBER. Example: 4687');
+    // for master branch, don't throw an error
+    process.exit(0);
+  }
+
+  const number = process.env.CIRCLE_PR_NUMBER;
 
   // intentional lint warning to make sure that the bot is working :)
   main(messages, owner, repo, number);

scripts/test-manual-e2e.sh

@@ -8,16 +8,16 @@ BLUE="\033[0;35m"
 ENDCOLOR="\033[0m"
 
 error() {
-    echo -e $RED"$@"$ENDCOLOR
+    echo -e "$RED""$*""$ENDCOLOR"
     exit 1
 }
 
 success() {
-    echo -e $GREEN"$@"$ENDCOLOR
+    echo -e "$GREEN""$*""$ENDCOLOR"
 }
 
 info() {
-    echo -e $BLUE"$@"$ENDCOLOR
+    echo -e "$BLUE""$*""$ENDCOLOR"
 }
 
 PACKAGE_VERSION=$(cat package.json \

scripts/validate-android-device-env.sh

@@ -28,8 +28,9 @@ fi
 
 while :
 do
-    BOOTANIM=`adb -e shell getprop init.svc.bootanim`
-    if [ -n `echo $BOOTANIM | grep stopped` ]; then
+    BOOTANIM=$(adb -e shell getprop init.svc.bootanim)
+    # shellcheck disable=SC2143
+    if [[ -n $(echo "$BOOTANIM" | grep stopped) ]]; then
         break
     fi
     echo "Waiting for the emulator to finish booting..."