From 654e239b25e5ed49dd0340132a74e4617aa185c8 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Date: Thu, 15 Oct 2020 15:45:13 +0100
Subject: [PATCH] Add option to scripts-dev/lint.sh to only lint files changed
 since the last git commit (#8472)

This PR makes several changes to the `./scripts-dev/lint.sh` script, which lints the codebase with a number of tools:

* Adds usage information, with `-h` flag to show it. Otherwise it will show when providing an unknown flag.
* Adds option `-d` which will check both staged and unstaged files that have changed since the last commit and add them to the list of files to lint.
  - Note that only files without an extension, or with a `.py` extension will be allowed. This prevents editing bash scripts causing the linters to break on non-python files.
* Improves the print-out of which files/directories are being linted.
---
 CONTRIBUTING.md       |  4 ++
 changelog.d/8472.misc |  1 +
 scripts-dev/lint.sh   | 95 ++++++++++++++++++++++++++++++++++++++-----
 setup.py              |  4 +-
 4 files changed, 91 insertions(+), 13 deletions(-)
 create mode 100644 changelog.d/8472.misc

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 524f82433d..c17e3b2399 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -63,6 +63,10 @@ run-time:
 ./scripts-dev/lint.sh path/to/file1.py path/to/file2.py path/to/folder
 ```
 
+You can also provided the `-d` option, which will lint the files that have been
+changed since the last git commit. This will often be significantly faster than
+linting the whole codebase.
+
 Before pushing new changes, ensure they don't produce linting errors. Commit any
 files that were corrected.
 
diff --git a/changelog.d/8472.misc b/changelog.d/8472.misc
new file mode 100644
index 0000000000..880f3f5e14
--- /dev/null
+++ b/changelog.d/8472.misc
@@ -0,0 +1 @@
+Add `-d` option to `./scripts-dev/lint.sh` to lint files that have changed since the last git commit.
\ No newline at end of file
diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh
index 0647993658..f2b65a2105 100755
--- a/scripts-dev/lint.sh
+++ b/scripts-dev/lint.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 #
 # Runs linting scripts over the local Synapse checkout
 # isort - sorts import statements
@@ -7,15 +7,90 @@
 
 set -e
 
-if [ $# -ge 1 ]
-then
-  files=$*
-else
-  files="synapse tests scripts-dev scripts contrib synctl"
+usage() {
+  echo
+  echo "Usage: $0 [-h] [-d] [paths...]"
+  echo
+  echo "-d"
+  echo "  Lint files that have changed since the last git commit."
+  echo
+  echo "  If paths are provided and this option is set, both provided paths and those"
+  echo "  that have changed since the last commit will be linted."
+  echo
+  echo "  If no paths are provided and this option is not set, all files will be linted."
+  echo
+  echo "  Note that paths with a file extension that is not '.py' will be excluded."
+  echo "-h"
+  echo "  Display this help text."
+}
+
+USING_DIFF=0
+files=()
+
+while getopts ":dh" opt; do
+  case $opt in
+    d)
+      USING_DIFF=1
+      ;;
+    h)
+      usage
+      exit
+      ;;
+    \?)
+      echo "ERROR: Invalid option: -$OPTARG" >&2
+      usage
+      exit
+      ;;
+  esac
+done
+
+# Strip any options from the command line arguments now that
+# we've finished processing them
+shift "$((OPTIND-1))"
+
+if [ $USING_DIFF -eq 1 ]; then
+  # Check both staged and non-staged changes
+  for path in $(git diff HEAD --name-only); do
+    filename=$(basename "$path")
+    file_extension="${filename##*.}"
+
+    # If an extension is present, and it's something other than 'py',
+    # then ignore this file
+    if [[ -n ${file_extension+x} && $file_extension != "py" ]]; then
+      continue
+    fi
+
+    # Append this path to our list of files to lint
+    files+=("$path")
+  done
 fi
 
-echo "Linting these locations: $files"
-isort $files
-python3 -m black $files
+# Append any remaining arguments as files to lint
+files+=("$@")
+
+if [[ $USING_DIFF -eq 1 ]]; then
+  # If we were asked to lint changed files, and no paths were found as a result...
+  if [ ${#files[@]} -eq 0 ]; then
+    # Then print and exit
+    echo "No files found to lint."
+    exit 0
+  fi
+else
+  # If we were not asked to lint changed files, and no paths were found as a result,
+  # then lint everything!
+  if [[ -z ${files+x} ]]; then
+    # Lint all source code files and directories
+    files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl" "setup.py")
+  fi
+fi
+
+echo "Linting these paths: ${files[*]}"
+echo
+
+# Print out the commands being run
+set -x
+
+isort "${files[@]}"
+python3 -m black "${files[@]}"
 ./scripts-dev/config-lint.sh
-flake8 $files
+flake8 "${files[@]}"
diff --git a/setup.py b/setup.py
index 926b1bc86f..08843fe2a3 100755
--- a/setup.py
+++ b/setup.py
@@ -15,12 +15,10 @@
 # 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.
-
 import glob
 import os
-from setuptools import setup, find_packages, Command
-import sys
 
+from setuptools import Command, find_packages, setup
 
 here = os.path.abspath(os.path.dirname(__file__))