From 02696248399e6e0bfc041737ebc4c5081c399282 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Wed, 29 Jan 2020 09:46:39 -0800
Subject: [PATCH 01/17] find variables used in strings that should probably be
 fstrings

---
 python/deadcode/missing-fstring.yaml     | 10 ++++++++++
 tests/python/deadcode/missing-fstring.py |  3 +++
 2 files changed, 13 insertions(+)
 create mode 100644 python/deadcode/missing-fstring.yaml
 create mode 100644 tests/python/deadcode/missing-fstring.py

diff --git a/python/deadcode/missing-fstring.yaml b/python/deadcode/missing-fstring.yaml
new file mode 100644
index 0000000000..2e8fd3627c
--- /dev/null
+++ b/python/deadcode/missing-fstring.yaml
@@ -0,0 +1,10 @@
+rules:
+  - id: should-be-fstring
+    patterns:
+      - pattern: |
+          $X = $Y
+          ...
+          print('...{$X}...')
+    message: "possibly missing an f-string specifier for string containing variable $X"
+    languages: [python]
+    severity: WARNING
diff --git a/tests/python/deadcode/missing-fstring.py b/tests/python/deadcode/missing-fstring.py
new file mode 100644
index 0000000000..9ef142862d
--- /dev/null
+++ b/tests/python/deadcode/missing-fstring.py
@@ -0,0 +1,3 @@
+x = y
+print('hi')
+print('now {x} is {y}')

From 64922d467c20e802668a8608204c62449f286b5c Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 16:06:57 -0800
Subject: [PATCH 02/17] add a whole bunch of dead code checks

---
 python/deadcode/return.yaml                   | 18 ++++++++++
 python/deadcode/useless-assign.py             | 23 ++++++++++++
 python/deadcode/useless-assign.yaml           | 21 +++++++++++
 python/deadcode/useless-eqeq.yaml             |  2 +-
 python/deadcode/useless-ifelse.yaml           | 35 +++++++++++++++++++
 python/deadcode/useless-innerfunction.yaml    | 19 ++++++++++
 python/exceptions/exceptions.yaml             | 16 +++++++++
 python/smells/sleep.yaml                      | 20 +++++++++++
 python/tempfile/flush.yaml                    | 26 ++++++++++++--
 python/tempfile/mktemp.yaml                   |  6 ++++
 tests/python/deadcode/missing-fstring.py      |  3 --
 .../test_avoid_app_run_with_bad_host.py       |  3 --
 .../test_avoid_app_run_with_bad_host_value.py |  1 -
 .../test_app_run_with_guard.py                |  7 ----
 .../test_app_run_with_guard_no_option.py      |  8 -----
 .../test_avoid_using_app_run_directly.py      |  4 ---
 16 files changed, 182 insertions(+), 30 deletions(-)
 create mode 100644 python/deadcode/return.yaml
 create mode 100644 python/deadcode/useless-assign.py
 create mode 100644 python/deadcode/useless-assign.yaml
 create mode 100644 python/deadcode/useless-ifelse.yaml
 create mode 100644 python/deadcode/useless-innerfunction.yaml
 create mode 100644 python/exceptions/exceptions.yaml
 create mode 100644 python/smells/sleep.yaml
 create mode 100644 python/tempfile/mktemp.yaml
 delete mode 100644 tests/python/deadcode/missing-fstring.py
 delete mode 100644 tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py
 delete mode 100644 tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py
 delete mode 100644 tests/python/flask/app_run_security_config/test_app_run_with_guard.py
 delete mode 100644 tests/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py
 delete mode 100644 tests/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py

diff --git a/python/deadcode/return.yaml b/python/deadcode/return.yaml
new file mode 100644
index 0000000000..3dc2507a8a
--- /dev/null
+++ b/python/deadcode/return.yaml
@@ -0,0 +1,18 @@
+rules:
+  - id: code-after-unconditional-return
+    patterns:
+      - pattern: |
+          return $X
+          $S
+    message: "code after return statement will not be executed"
+    languages: [python]
+    severity: WARNING
+  - id: return-not-in-function
+    patterns:
+      - pattern-not-inside: |
+          def $F(...):
+              ...
+      - pattern: return $X
+    message: "`return` only makes sense inside a function"
+    languages: [python]
+    severity: WARNING
diff --git a/python/deadcode/useless-assign.py b/python/deadcode/useless-assign.py
new file mode 100644
index 0000000000..4287702b84
--- /dev/null
+++ b/python/deadcode/useless-assign.py
@@ -0,0 +1,23 @@
+d = {}
+z = {}
+a = {}
+for i in xrange(100):
+    # ruleid: useless-assignment-keyed
+    d[i] = z[i]
+    d[i] = z[i]
+    d[i+1] = z[i]
+
+    for i in xrange(100):
+        # ruleid: useless-assignment-keyed
+        da[i*1][j] = z[i]
+        da[i*1][j] = z[i]
+        da[i*4] = z[i]
+
+# ruleid: useless-assignment
+x = 5
+x = 5
+
+x = y
+x = y()
+
+y() = y()
diff --git a/python/deadcode/useless-assign.yaml b/python/deadcode/useless-assign.yaml
new file mode 100644
index 0000000000..52db8f6a70
--- /dev/null
+++ b/python/deadcode/useless-assign.yaml
@@ -0,0 +1,21 @@
+rules:
+  - id: useless-assignment-keyed
+    patterns:
+      - pattern-either:
+          - pattern: |
+              $X[$Y] = ...
+              $X[$Y] = ...
+          - pattern: |
+              $X[$Y][$Z] = ...
+              $X[$Y][$Z] = ...
+    message: "key `$Y` in `$X` is uselessly assigned the same value twice"
+    languages: [python]
+    severity: WARNING
+  - id: useless-assignment
+    patterns:
+      - pattern: |
+          $X = $Y
+          $X = $Y
+    message: "`$X` is uselessly assigned to the same value (`$Y`) twice"
+    languages: [python]
+    severity: WARNING
diff --git a/python/deadcode/useless-eqeq.yaml b/python/deadcode/useless-eqeq.yaml
index 21eb06b95d..08cae81bfd 100644
--- a/python/deadcode/useless-eqeq.yaml
+++ b/python/deadcode/useless-eqeq.yaml
@@ -11,6 +11,6 @@ rules:
           - pattern: $X == $X
           - pattern: $X != $X
       - pattern-not: 1 == 1
-    message: "useless comparison operation `$X == $X` or `$X != $X`; possible bug?"
+    message: "useless comparison operation `$X == $X` or `$X != $X`; if testing for floating point NaN, use `math.isnan`, or `cmath.isnan` if the number is complex."
     languages: [python]
     severity: ERROR
diff --git a/python/deadcode/useless-ifelse.yaml b/python/deadcode/useless-ifelse.yaml
new file mode 100644
index 0000000000..1f5fa5fabe
--- /dev/null
+++ b/python/deadcode/useless-ifelse.yaml
@@ -0,0 +1,35 @@
+rules:
+  - id: useless-if-conditional
+    patterns:
+      - pattern-either:
+          - pattern: |
+              if $X:
+                  ...
+              elif $X:
+                  ...
+    message: "if block checks for the same condition on both branches (`$X`)"
+    languages: [python]
+    severity: WARNING
+  - id: useless-if-body
+    patterns:
+      - pattern-either:
+          - pattern: |
+              if $X:
+                  $S
+              elif $Y:
+                  $S
+          - pattern: |
+              if $X:
+                  $S
+              else:
+                  $S
+          - pattern: |
+              if $X:
+                  $S
+              elif $Z:
+                ...
+              elif $Y:
+                 $S
+    message: "useless if statment; both blocks have the same body"
+    languages: [python]
+    severity: WARNING
diff --git a/python/deadcode/useless-innerfunction.yaml b/python/deadcode/useless-innerfunction.yaml
new file mode 100644
index 0000000000..f85fea6455
--- /dev/null
+++ b/python/deadcode/useless-innerfunction.yaml
@@ -0,0 +1,19 @@
+rules:
+  - id: useless-inner-function
+    patterns:
+      - pattern-not-inside: |
+          def $F(...):
+              ...
+              def $F2(...):
+                 ...
+              ...
+              $F2
+      - pattern: |
+          def $F(...):
+              ...
+              def $F2(...):
+                 ...
+              ...
+    message: "function `$F2` is defined inside a function but never used"
+    languages: [python]
+    severity: ERROR
diff --git a/python/exceptions/exceptions.yaml b/python/exceptions/exceptions.yaml
new file mode 100644
index 0000000000..d1271d79d4
--- /dev/null
+++ b/python/exceptions/exceptions.yaml
@@ -0,0 +1,16 @@
+rules:
+  - id: raise-not-base-exception
+    patterns:
+      - pattern-either:
+          - pattern: raise "..."
+          #- pattern: |
+          #    raise $X: int
+          #- pattern: |
+          #    raise $X: float
+          # TODO, the second pattern requires sgrep typing support
+          - pattern: |
+              $X: BaseException
+              raise $X(...)
+    message: "In Python3, a runtime `TypeError` will be thrown if you attempt to raise an object or class which does not inherit from `BaseException`"
+    languages: [python]
+    severity: ERROR
diff --git a/python/smells/sleep.yaml b/python/smells/sleep.yaml
new file mode 100644
index 0000000000..31a37732b7
--- /dev/null
+++ b/python/smells/sleep.yaml
@@ -0,0 +1,20 @@
+rules:
+  - id: arbitrary-sleep
+    patterns:
+      - pattern-not: time.sleep($F(...))
+      - pattern: time.sleep(...)
+    message: "time.sleep() call; did you mean to leave this in?"
+    languages: [python]
+    severity: ERROR
+# Feature request:
+#
+#    - id: sleep
+#    patterns:
+#      - pattern-either:
+#          - pattern: time.sleep(=~/[0-9]/)
+#          - pattern: time.sleep("=~/([0-9])+/")
+#          - pattern: time.sleep("=~/([0-9])+\.([0-9])+/")
+#    message: "time.sleep() call with an arbitrary number; did you mean to leave this in?"
+#    languages: [python]
+#    severity: WARNING
+
diff --git a/python/tempfile/flush.yaml b/python/tempfile/flush.yaml
index 02a2c4a482..392612c126 100644
--- a/python/tempfile/flush.yaml
+++ b/python/tempfile/flush.yaml
@@ -1,6 +1,26 @@
 rules:
-  - id: requests-verify-false
-    pattern: "requests.get(..., verify=False)"
-    message: "don't make requests with disabled ssl verification"
+  - id: tempfile-without-flush
+    patterns:
+      - pattern-not: |
+          $F = tempfile.NamedTemporaryFile(...)
+          ...
+          $F.write(...)
+          ...
+          $F.flush()
+          ...
+          $F.name
+      - pattern-not: |
+          $F = tempfile.NamedTemporaryFile(...)
+          ...
+          $F.write(...)
+          ...
+          $F.close()
+          ...
+          $F.name
+      - pattern: |
+          $F = tempfile.NamedTemporaryFile(...)
+          ...
+          $F.name
+    message: "possibly missing a .flush() or .close() call to temporary file $F; file may or may not exist when $F.name is used"
     languages: [python]
     severity: ERROR
diff --git a/python/tempfile/mktemp.yaml b/python/tempfile/mktemp.yaml
new file mode 100644
index 0000000000..f68b2a87f7
--- /dev/null
+++ b/python/tempfile/mktemp.yaml
@@ -0,0 +1,6 @@
+rules:
+  - id: tempfile-insecure
+    pattern: tempfile.mktemp(...)
+    message: "Use tempfile.NamedTemporaryFile instead. From the official Python documentation: THIS FUNCTION IS UNSAFE AND SHOULD NOT BE USED. The file name may refer to a file that did not exist at some point, but by the time you get around to creating it, someone else may have beaten you to the punch."
+    languages: [python]
+    severity: ERROR
diff --git a/tests/python/deadcode/missing-fstring.py b/tests/python/deadcode/missing-fstring.py
deleted file mode 100644
index 9ef142862d..0000000000
--- a/tests/python/deadcode/missing-fstring.py
+++ /dev/null
@@ -1,3 +0,0 @@
-x = y
-print('hi')
-print('now {x} is {y}')
diff --git a/tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py b/tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py
deleted file mode 100644
index f6b16ce281..0000000000
--- a/tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py
+++ /dev/null
@@ -1,3 +0,0 @@
-app.run("0.0.0.0")
-
-foo.run("0.0.0.0")
\ No newline at end of file
diff --git a/tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py b/tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py
deleted file mode 100644
index c7316f121f..0000000000
--- a/tests/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py
+++ /dev/null
@@ -1 +0,0 @@
-app.run(host="0.0.0.0")
\ No newline at end of file
diff --git a/tests/python/flask/app_run_security_config/test_app_run_with_guard.py b/tests/python/flask/app_run_security_config/test_app_run_with_guard.py
deleted file mode 100644
index 07e0d05586..0000000000
--- a/tests/python/flask/app_run_security_config/test_app_run_with_guard.py
+++ /dev/null
@@ -1,7 +0,0 @@
-app = None
-if __name__ == '__main__':
-    app.run(foo=None)
-
-if __name__ == '__main__':
-    x = None
-    app.run(foo=None)
\ No newline at end of file
diff --git a/tests/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py b/tests/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py
deleted file mode 100644
index 8ae054fda7..0000000000
--- a/tests/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py
+++ /dev/null
@@ -1,8 +0,0 @@
-
-app = None
-if __name__ == '__main__':
-    app.run()
-
-if __name__ == '__main__':
-    x = None
-    app.run()
\ No newline at end of file
diff --git a/tests/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py b/tests/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py
deleted file mode 100644
index 9da24d4c6f..0000000000
--- a/tests/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py
+++ /dev/null
@@ -1,4 +0,0 @@
-app.run()
-
-
-app.run(debug=True)

From 2bcd4da3eb663d84b9b2d6838b92df0bbd4b9d5e Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 16:07:10 -0800
Subject: [PATCH 03/17] add a whole bunch of dead code checks

---
 .../deadcode/baseclass-attribute-override.py  | 21 +++++++
 .../baseclass-attribute-override.yaml         | 17 ++++++
 python/deadcode/missing-fstring.py            |  3 +
 python/deadcode/return.py                     | 19 ++++++
 python/deadcode/useless-ifelse.py             | 39 +++++++++++++
 python/deadcode/useless-innerfunction.py      | 21 +++++++
 python/deadcode/useless-literal.py            | 12 ++++
 python/deadcode/useless-literal.yaml          | 18 ++++++
 python/exceptions/exceptions.py               | 24 ++++++++
 .../test_avoid_app_run_with_bad_host.py       |  3 +
 .../test_avoid_app_run_with_bad_host_value.py |  1 +
 .../test_app_run_with_guard.py                |  7 +++
 .../test_app_run_with_guard_no_option.py      |  8 +++
 .../test_avoid_using_app_run_directly.py      |  4 ++
 python/smells/missing-hash-with-eq.py         | 13 +++++
 python/smells/missing-hash-with-eq.yaml       | 19 ++++++
 python/smells/return-in-init.py               | 18 ++++++
 python/smells/return-in-init.yaml             | 25 ++++++++
 python/smells/sleep.py                        | 19 ++++++
 python/tempfile/flush.py                      | 58 +++++++++++++++++++
 python/tempfile/mktemp.py                     |  6 ++
 21 files changed, 355 insertions(+)
 create mode 100644 python/deadcode/baseclass-attribute-override.py
 create mode 100644 python/deadcode/baseclass-attribute-override.yaml
 create mode 100644 python/deadcode/missing-fstring.py
 create mode 100644 python/deadcode/return.py
 create mode 100644 python/deadcode/useless-ifelse.py
 create mode 100644 python/deadcode/useless-innerfunction.py
 create mode 100644 python/deadcode/useless-literal.py
 create mode 100644 python/deadcode/useless-literal.yaml
 create mode 100644 python/exceptions/exceptions.py
 create mode 100644 python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py
 create mode 100644 python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py
 create mode 100644 python/flask/app_run_security_config/test_app_run_with_guard.py
 create mode 100644 python/flask/app_run_security_config/test_app_run_with_guard_no_option.py
 create mode 100644 python/flask/app_run_security_config/test_avoid_using_app_run_directly.py
 create mode 100644 python/smells/missing-hash-with-eq.py
 create mode 100644 python/smells/missing-hash-with-eq.yaml
 create mode 100644 python/smells/return-in-init.py
 create mode 100644 python/smells/return-in-init.yaml
 create mode 100644 python/smells/sleep.py
 create mode 100644 python/tempfile/flush.py
 create mode 100644 python/tempfile/mktemp.py

diff --git a/python/deadcode/baseclass-attribute-override.py b/python/deadcode/baseclass-attribute-override.py
new file mode 100644
index 0000000000..c0dd863e3d
--- /dev/null
+++ b/python/deadcode/baseclass-attribute-override.py
@@ -0,0 +1,21 @@
+
+class A:
+    def method1(self, args):
+        pass
+
+
+class A2:
+    def method2(self, args):
+        pass
+
+
+class B:
+    def method1(self, args):
+        print('hello there')
+
+# ruleid: baseclass-attribute-override
+
+
+class C(A, B):
+    def __init__():
+        print('initialized')
diff --git a/python/deadcode/baseclass-attribute-override.yaml b/python/deadcode/baseclass-attribute-override.yaml
new file mode 100644
index 0000000000..3a9ada5172
--- /dev/null
+++ b/python/deadcode/baseclass-attribute-override.yaml
@@ -0,0 +1,17 @@
+rules:
+  - id: baseclass-attribute-override
+    patterns:
+      - pattern: |
+          class $A(...):
+            def $F(...):
+             ...
+          ...    
+          class $B(...):
+            def $F(...):
+             ...
+          ...
+          class $C(..., $A, $B, ...):
+             ...
+    message: "Class $C inherits from both `$A` and `$B` which both have a method named `$F`; one of these methods will be overwritten"
+    languages: [python]
+    severity: WARNING
diff --git a/python/deadcode/missing-fstring.py b/python/deadcode/missing-fstring.py
new file mode 100644
index 0000000000..9ef142862d
--- /dev/null
+++ b/python/deadcode/missing-fstring.py
@@ -0,0 +1,3 @@
+x = y
+print('hi')
+print('now {x} is {y}')
diff --git a/python/deadcode/return.py b/python/deadcode/return.py
new file mode 100644
index 0000000000..05a804eaab
--- /dev/null
+++ b/python/deadcode/return.py
@@ -0,0 +1,19 @@
+
+# ruleid: return-not-in-function
+return 5
+
+
+def alwaysblue():
+    if isblue():
+        return 'blue'
+    # ruleid: code-after-unconditional-return
+    return 'red'
+    return 'green'
+
+
+def alwaysblue():
+    if isblue():
+        return 'blue'
+    # ruleid: code-after-unconditional-return
+    return 'red'
+    x = 5
diff --git a/python/deadcode/useless-ifelse.py b/python/deadcode/useless-ifelse.py
new file mode 100644
index 0000000000..778274ffec
--- /dev/null
+++ b/python/deadcode/useless-ifelse.py
@@ -0,0 +1,39 @@
+a, b, c = 1
+
+# ruleid: useless-if-conditional
+if a:
+    print('1')
+elif a:
+    print('2')
+
+# ruleid: useless-if-body
+if a:
+    print('1')
+else:
+    print('1')
+
+# ruleid: useless-if-body
+if a:
+    print('1')
+elif b:
+    print('1')
+
+
+# ruleid: useless-if-body
+if a:
+    print('this is a')
+elif b:
+    print('this is b')
+elif c:
+    print('this is c')
+elif d:
+    print('this is d')
+
+
+if a:
+    print('this is a')
+# ruleid: useless-if-body
+elif b:
+    print('this is b')
+elif c:
+    print('this is b')
diff --git a/python/deadcode/useless-innerfunction.py b/python/deadcode/useless-innerfunction.py
new file mode 100644
index 0000000000..0e607a40e4
--- /dev/null
+++ b/python/deadcode/useless-innerfunction.py
@@ -0,0 +1,21 @@
+# ruleid:uselses-inner-function
+def A():
+    print_error('test')
+
+    def B():
+        print_error('again')
+
+    def C():
+        print_error('another')
+    return None
+
+# ruleid:uselses-inner-function
+def A():
+    print_error('test')
+
+    def B():
+        print_error('again')
+
+    def C():
+        print_error('another')
+    return B(), C()
diff --git a/python/deadcode/useless-literal.py b/python/deadcode/useless-literal.py
new file mode 100644
index 0000000000..4ba056ab1a
--- /dev/null
+++ b/python/deadcode/useless-literal.py
@@ -0,0 +1,12 @@
+# ruleid: useless-literal-dict
+d = dict(1: 'a', 2: 'b', 1: 'a')
+# ruleid: useless-literal-set
+d = set(1: 'a', 2: 'b', 1: 'a')
+
+# ruleid: useless-literal-dict
+d = {1: 'a', 2: 'b', 1: 'a'}
+# ruleid: useless-literal-dict
+d = {'a': 1, 'a': 1}
+
+# OK
+d = {1: 'a', 2: 'b', 3: 'a'}
diff --git a/python/deadcode/useless-literal.yaml b/python/deadcode/useless-literal.yaml
new file mode 100644
index 0000000000..f4fa8fc5c7
--- /dev/null
+++ b/python/deadcode/useless-literal.yaml
@@ -0,0 +1,18 @@
+rules:
+  - id: useless-literal-dict
+    patterns:
+      - pattern-either:
+#          - pattern: |
+#              dict(..., $X: $A, ..., $X: $B, ...)
+          - pattern: |
+              {..., $X: $A, ..., $X: $B, ...}
+    message: "key `$X` is uselessly assigned twice"
+    languages: [python]
+    severity: WARNING
+  - id: useless-literal-set
+    patterns:
+#      - pattern: |
+#            set(..., $X: $A, ..., $X: $B, ...)
+    message: "`$X` is uselessly assigned twice inside the creation of the set"
+    languages: [python]
+    severity: ERROR
diff --git a/python/exceptions/exceptions.py b/python/exceptions/exceptions.py
new file mode 100644
index 0000000000..6861e4b6d0
--- /dev/null
+++ b/python/exceptions/exceptions.py
@@ -0,0 +1,24 @@
+# ruleid:raise-not-base-exception
+raise "error here"
+
+# ruleid:raise-not-base-exception
+raise 5
+
+
+class Foobar:
+    x = 5
+
+
+# ruleid:raise-not-base-exception
+raise Foobar()
+
+
+class Foobar2(BaseException):
+    x = 5
+
+
+# OK
+raise Foobar2()
+
+# OK
+raise Exception()
diff --git a/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py b/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py
new file mode 100644
index 0000000000..f6b16ce281
--- /dev/null
+++ b/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host.py
@@ -0,0 +1,3 @@
+app.run("0.0.0.0")
+
+foo.run("0.0.0.0")
\ No newline at end of file
diff --git a/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py b/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py
new file mode 100644
index 0000000000..c7316f121f
--- /dev/null
+++ b/python/flask/app_run_param_config/test_avoid_app_run_with_bad_host_value.py
@@ -0,0 +1 @@
+app.run(host="0.0.0.0")
\ No newline at end of file
diff --git a/python/flask/app_run_security_config/test_app_run_with_guard.py b/python/flask/app_run_security_config/test_app_run_with_guard.py
new file mode 100644
index 0000000000..07e0d05586
--- /dev/null
+++ b/python/flask/app_run_security_config/test_app_run_with_guard.py
@@ -0,0 +1,7 @@
+app = None
+if __name__ == '__main__':
+    app.run(foo=None)
+
+if __name__ == '__main__':
+    x = None
+    app.run(foo=None)
\ No newline at end of file
diff --git a/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py b/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py
new file mode 100644
index 0000000000..8ae054fda7
--- /dev/null
+++ b/python/flask/app_run_security_config/test_app_run_with_guard_no_option.py
@@ -0,0 +1,8 @@
+
+app = None
+if __name__ == '__main__':
+    app.run()
+
+if __name__ == '__main__':
+    x = None
+    app.run()
\ No newline at end of file
diff --git a/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py b/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py
new file mode 100644
index 0000000000..9da24d4c6f
--- /dev/null
+++ b/python/flask/app_run_security_config/test_avoid_using_app_run_directly.py
@@ -0,0 +1,4 @@
+app.run()
+
+
+app.run(debug=True)
diff --git a/python/smells/missing-hash-with-eq.py b/python/smells/missing-hash-with-eq.py
new file mode 100644
index 0000000000..fce9273ae7
--- /dev/null
+++ b/python/smells/missing-hash-with-eq.py
@@ -0,0 +1,13 @@
+
+# ruleid:missing-hash-with-eq
+class A:
+    def __eq__(self, someother):
+        pass
+
+
+class A2:
+    def __eq__(self, someother):
+        pass
+
+    def __hash__(self):
+        pass
diff --git a/python/smells/missing-hash-with-eq.yaml b/python/smells/missing-hash-with-eq.yaml
new file mode 100644
index 0000000000..16d8c246a1
--- /dev/null
+++ b/python/smells/missing-hash-with-eq.yaml
@@ -0,0 +1,19 @@
+rules:
+  - id: missing-hash-with-eq
+    patterns:
+      - pattern-not-inside: |
+          class A(...):
+              ...
+              def __hash__(self):
+                  ...
+              ...
+              def __eq__(self, $O):
+                  ...
+      - pattern: |
+          class A(...):
+            ...
+            def __eq__(self, $O): ...
+            ...
+    message: "Class `$A` has defined `__eq__` which means it should also have defined `__hash__`; "
+    languages: [python]
+    severity: WARNING
diff --git a/python/smells/return-in-init.py b/python/smells/return-in-init.py
new file mode 100644
index 0000000000..ad40b730ba
--- /dev/null
+++ b/python/smells/return-in-init.py
@@ -0,0 +1,18 @@
+class A:
+
+    def __init__(a, b, c):
+        # ruleid:return-in-init
+        return A(a, b, c)
+
+
+class B:
+
+    def __init__(a, b, c):
+        # ruleid:yield-in-init
+        yield
+
+class C:
+
+    def __init__():
+        # ruleid:yield-in-init
+        yield 5
diff --git a/python/smells/return-in-init.yaml b/python/smells/return-in-init.yaml
new file mode 100644
index 0000000000..d51abaa7fa
--- /dev/null
+++ b/python/smells/return-in-init.yaml
@@ -0,0 +1,25 @@
+rules:
+    - id: return-in-init
+      patterns:
+        - pattern-inside: |
+            class A(...): 
+                ...
+        - pattern-inside: |
+            def __init__(...):
+                ...
+        - pattern: return $X
+      message: "`return` should never appear inside a class __init__ function"
+      languages: [python]
+      severity: ERROR
+    - id: return-in-init
+      patterns:
+        - pattern-inside: |
+            class A(...): 
+                ...
+        - pattern-inside: |
+            def __init__(...):
+                ...
+        - pattern: yield
+      message: "`yield` should never appear inside a class __init__ function"
+      languages: [python]
+      severity: ERROR
diff --git a/python/smells/sleep.py b/python/smells/sleep.py
new file mode 100644
index 0000000000..9015694a02
--- /dev/null
+++ b/python/smells/sleep.py
@@ -0,0 +1,19 @@
+import time as t
+
+
+def a():
+    return 10
+
+
+# OK
+t.sleep
+
+# ruleid:arbitrary-sleep
+t.sleep(5)
+# ruleid:arbitrary-sleep
+t.sleep(0.1)
+# ruleid:arbitrary-sleep
+time.sleep("bad")
+
+# OK
+t.sleep(a())
diff --git a/python/tempfile/flush.py b/python/tempfile/flush.py
new file mode 100644
index 0000000000..6ac6d22ad0
--- /dev/null
+++ b/python/tempfile/flush.py
@@ -0,0 +1,58 @@
+import tempfile
+
+import at
+import tf
+
+
+def main():
+    with tempfile.NamedTemporaryFile("w") as fout:
+        debug_print(astr)
+        fout.write(astr)
+        # OK
+        fout.flush()
+        cmd = [binary_name, fout.name, *[str(path) for path in targets]]
+
+
+def main_b():
+    with tempfile.NamedTemporaryFile("w") as fout:
+        debug_print(astr)
+        fout.write(astr)
+        # OK
+        fout.close()
+        cmd = [binary_name, fout.name, *[str(path) for path in targets]]
+
+
+def main_c():
+    with tempfile.NamedTemporaryFile("w") as fout:
+        debug_print(astr)
+        fout.write(astr)
+
+    # OK
+    cmd = [binary_name, fout.name, *[str(path) for path in targets]]
+
+
+def main_c():
+    with tempfile.NamedTemporaryFile("w") as fout:
+        debug_print(astr)
+        fout.write(astr)
+        debug_print('wrote file')
+        # ruleid:tempfile-without-flush
+        cmd = [binary_name, fout.name, *[str(path) for path in targets]]
+
+
+def main_d():
+    fout = tempfile.NamedTemporaryFile('w')
+    debug_print(astr)
+    fout.write(astr)
+    # ruleid:tempfile-without-flush
+    fout.name
+    cmd = [binary_name, fout.name, *[str(path) for path in targets]]
+
+
+def main_e():
+    fout = tempfile.NamedTemporaryFile('w')
+    debug_print(astr)
+    fout.write(astr)
+    # ruleid:tempfile-without-flush
+    print(fout.name)
+    cmd = [binary_name, fout.name, *[str(path) for path in targets]]
diff --git a/python/tempfile/mktemp.py b/python/tempfile/mktemp.py
new file mode 100644
index 0000000000..f157c87ea4
--- /dev/null
+++ b/python/tempfile/mktemp.py
@@ -0,0 +1,6 @@
+import tempfile as tf
+
+# ruleid: tempfile-insecure
+x = tempfile.mktemp()
+# ruleid: tempfile-insecure
+x = tempfile.mktemp(dir="/tmp")

From ef07bcca02fd6977e45a765b91861851c3e1397a Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 18:25:39 -0800
Subject: [PATCH 04/17] tests

---
 .circleci/config.yml |   1 +
 test.py              | 168 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100755 test.py

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 17efbe8bbc..627ca66588 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -7,6 +7,7 @@ jobs:
       - checkout
       - run: sgrep-lint --validate python .
       - run: sgrep-lint --validate c .
+      - run: ./test.py --ignore-todo .
 workflows:
   version: 2.1
   build_and_test:
diff --git a/test.py b/test.py
new file mode 100755
index 0000000000..80a4ae15dc
--- /dev/null
+++ b/test.py
@@ -0,0 +1,168 @@
+#!/usr/bin/env python3
+
+import argparse
+import collections
+import json
+import subprocess
+import sys
+from pathlib import Path
+from typing import List
+
+"""
+For each directory containing YAML rules, run those rules on the file in the same directory with the same name but different extension. 
+E.g. eqeq.yaml runs on eqeq.py.
+Validate that the output is annotated in the source file with by looking for a comment like:
+ 
+ ```
+ # ruleid:eqeq-is-bad
+ ```
+ On the preceeding line.
+
+ """
+YML_EXTENSIONS = {".yml", ".yaml"}
+
+def normalize_rule_id(line):
+    """
+    given a line like `     # ruleid:foobar` 
+    return `foobar`
+    """
+    return line.strip().split(':')[1].strip()
+
+def compute_confusion_matrix(reported, expected):
+    true_positives = len(expected.intersection(reported))
+    false_positives = len(reported - expected)
+    true_negatives = 0 # we have no way to label "ok"
+    false_negatives = len(expected - reported)
+
+    return [true_positives, true_negatives, false_positives, false_negatives]
+
+def _test_compute_confusion_matrix():
+    tp, tn, fp, fn = compute_confusion_matrix(set([1, 2, 3, 4]), set([1]))
+    assert tp == 1
+    assert tn == 0
+    assert fp == 3
+    assert fn == 0
+
+    tp, tn, fp, fn = compute_confusion_matrix(set([1, 2, 3, 4]), set([1, 2, 3, 4]))
+    assert tp == 4
+    assert tn == 0
+    assert fp == 0
+    assert fn == 0
+
+    tp, tn, fp, fn = compute_confusion_matrix(set([2, 3]), set([1, 2, 3, 4]))
+    assert tp == 2
+    assert tn == 0
+    assert fp == 0
+    assert fn == 2
+
+def score_output_json(json_out, test_files: List[str], ignore_todo: bool):
+    comment_lines = collections.defaultdict(lambda: collections.defaultdict(list))
+    reported_lines = collections.defaultdict(lambda: collections.defaultdict(list))
+    score_by_checkid = collections.defaultdict(lambda: [0, 0, 0, 0])
+    num_todo = 0
+
+    for test_file in test_files:
+        test_file = str(test_file.resolve())
+        with open(test_file) as fin:            
+            all_lines = fin.readlines()            
+            for i, line in enumerate(all_lines):
+                todo_in_line = ('#todoruleid:' in line or '# todoruleid' in line)
+                if todo_in_line:
+                    num_todo += 1
+                if (not ignore_todo and todo_in_line) or \
+                    ('#ruleid:' in line or "# ruleid:" in line):
+                    # +1 because we are 0 based and sgrep output is not, plus skip the comment line
+                    comment_lines[test_file][normalize_rule_id(line)].append(i + 2)
+            
+    for result in json_out['results']:
+        reported_lines[str(Path(result['path']).resolve())][result['check_id']].append(int(result['start']['line']))
+        
+    def join_keys(a, b):
+        return set(a.keys()).union(set(b.keys()))
+
+    for file_path in join_keys(comment_lines, reported_lines):
+        print(comment_lines)
+        print(reported_lines)
+        for check_id in join_keys(comment_lines[file_path], reported_lines[file_path]):
+            assert len(set(reported_lines[file_path][check_id])) == len(reported_lines[file_path][check_id]), f"for testing, please don't make rules that fire multiple times on the same line ({check_id} in {file_path})"
+            reported = set(reported_lines[file_path][check_id])
+            expected = set(comment_lines[file_path][check_id])
+            new_cm = compute_confusion_matrix(reported, expected)
+            print(reported, expected, new_cm)
+            old_cm = score_by_checkid[check_id]
+            score_by_checkid[check_id] = [old_cm[i] + new_cm[i] for i in range(len(new_cm))]
+
+    return (score_by_checkid, num_todo)
+
+def generate_file_pairs(location: Path, ignore_todo: bool):
+    filenames = list(location.rglob("*"))
+    no_tests = []
+    tested = []
+    sgrep_error = []
+    for filename in filenames:
+        if filename.suffix in YML_EXTENSIONS:
+            # find all filenames that have the same name but not extension
+            test_files = [path for path in filenames if (path.suffix not in YML_EXTENSIONS and path.with_suffix('') == filename.with_suffix(''))]
+            if not len(test_files):
+                no_tests.append(filename)
+                continue
+            # invoke sgrep
+            cmd = ['sgrep-lint', '--no-rewrite-rule-ids', '-f', str(filename)] + [str(t) for t in test_files]
+            print(cmd)
+            try:
+                output = subprocess.check_output(cmd, shell=False)
+                output_json = json.loads((output.decode("utf-8")))
+                print(output_json)
+                tested.append((filename, score_output_json(output_json, test_files, ignore_todo)))
+            except subprocess.CalledProcessError as ex:
+                print(f'sgrep error running {cmd}: {ex}')
+                sgrep_error.append(cmd)
+    
+    print(f"{len(no_tests)} yaml files missing tests")
+    print(f"{len(tested)} yaml files tested")
+    print('check id scoring follows')
+    count_failures = 0
+    for (filename, (output, num_todo)) in tested:
+        print('='*120)
+        print(filename)
+        if not len(output.items()):
+            print(f' no checks fired (TODOs: {num_todo})')
+        for check_id, (tp, tn, fp, fn) in output.items():
+            good = (fp == 0) and (fn == 0)
+            if not good:
+                count_failures += 1
+            status = '✔' if good else '⚠'
+            todo_text = f"(TODOs: {num_todo})" if num_todo > 0 else ''
+            print(f'{status} - {check_id.ljust(60)}TP: {tp}\tTN:{tn}\t FP: {fp}\t FN: {fn} {todo_text}')
+    if count_failures > 0:
+        sys.exit(1)
+
+def main(location: Path, ignore_todo: bool, verbose: bool):
+    global DEBUG
+    DEBUG = verbose
+    generate_file_pairs(location, ignore_todo)
+
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser(
+        description="run tests for sgrep rule yaml files in specified directory",
+    )
+
+    # input
+    parser.add_argument(
+        "directory",
+        default=["."],
+        help="Folder to collect tests from (by default, entire current working directory searched).",
+    )
+    parser.add_argument(
+        '--ignore-todo', 
+        help='ignore rules marked as #todoruleid: in test files',
+        action='store_true'
+    )
+    parser.add_argument(
+        '-v', '--verbose', 
+        help='debug output',
+        action='store_true'
+    )
+    args = parser.parse_args()
+    _test_compute_confusion_matrix()
+    main(Path(args.directory), args.ignore_todo, args.verbose)

From cf0072f6b877c7c3a50eabbea91f4a4ca1e5dd98 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 18:25:49 -0800
Subject: [PATCH 05/17] update tests to pass

---
 python/deadcode/baseclass-attribute-override.py | 4 +---
 python/deadcode/return.py                       | 4 ++--
 python/deadcode/useless-assign.py               | 4 ++--
 python/deadcode/useless-ifelse.py               | 2 +-
 python/deadcode/useless-innerfunction.py        | 4 ++--
 python/deadcode/useless-literal.py              | 8 ++++----
 python/exceptions/exceptions.py                 | 4 ++--
 python/smells/missing-hash-with-eq.py           | 2 +-
 python/tempfile/flush.py                        | 9 ++++++---
 9 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/python/deadcode/baseclass-attribute-override.py b/python/deadcode/baseclass-attribute-override.py
index c0dd863e3d..391f314065 100644
--- a/python/deadcode/baseclass-attribute-override.py
+++ b/python/deadcode/baseclass-attribute-override.py
@@ -13,9 +13,7 @@ class B:
     def method1(self, args):
         print('hello there')
 
-# ruleid: baseclass-attribute-override
-
-
+# todoruleid: baseclass-attribute-override
 class C(A, B):
     def __init__():
         print('initialized')
diff --git a/python/deadcode/return.py b/python/deadcode/return.py
index 05a804eaab..165356c221 100644
--- a/python/deadcode/return.py
+++ b/python/deadcode/return.py
@@ -6,7 +6,7 @@
 def alwaysblue():
     if isblue():
         return 'blue'
-    # ruleid: code-after-unconditional-return
+    # todoruleid: code-after-unconditional-return
     return 'red'
     return 'green'
 
@@ -14,6 +14,6 @@ def alwaysblue():
 def alwaysblue():
     if isblue():
         return 'blue'
-    # ruleid: code-after-unconditional-return
+    # todoruleid: code-after-unconditional-return
     return 'red'
     x = 5
diff --git a/python/deadcode/useless-assign.py b/python/deadcode/useless-assign.py
index 4287702b84..fa13279a53 100644
--- a/python/deadcode/useless-assign.py
+++ b/python/deadcode/useless-assign.py
@@ -8,12 +8,12 @@
     d[i+1] = z[i]
 
     for i in xrange(100):
-        # ruleid: useless-assignment-keyed
+        # todoruleid: useless-assignment-keyed
         da[i*1][j] = z[i]
         da[i*1][j] = z[i]
         da[i*4] = z[i]
 
-# ruleid: useless-assignment
+# todoruleid: useless-assignment
 x = 5
 x = 5
 
diff --git a/python/deadcode/useless-ifelse.py b/python/deadcode/useless-ifelse.py
index 778274ffec..03ad5b7639 100644
--- a/python/deadcode/useless-ifelse.py
+++ b/python/deadcode/useless-ifelse.py
@@ -19,7 +19,7 @@
     print('1')
 
 
-# ruleid: useless-if-body
+# todoruleid: useless-if-body
 if a:
     print('this is a')
 elif b:
diff --git a/python/deadcode/useless-innerfunction.py b/python/deadcode/useless-innerfunction.py
index 0e607a40e4..a685c15584 100644
--- a/python/deadcode/useless-innerfunction.py
+++ b/python/deadcode/useless-innerfunction.py
@@ -1,4 +1,4 @@
-# ruleid:uselses-inner-function
+# todoruleid:useless-inner-function
 def A():
     print_error('test')
 
@@ -9,7 +9,7 @@ def C():
         print_error('another')
     return None
 
-# ruleid:uselses-inner-function
+# todoruleid:useless-inner-function
 def A():
     print_error('test')
 
diff --git a/python/deadcode/useless-literal.py b/python/deadcode/useless-literal.py
index 4ba056ab1a..6e3bb2ff8d 100644
--- a/python/deadcode/useless-literal.py
+++ b/python/deadcode/useless-literal.py
@@ -1,11 +1,11 @@
-# ruleid: useless-literal-dict
+# todoruleid: useless-literal-dict
 d = dict(1: 'a', 2: 'b', 1: 'a')
-# ruleid: useless-literal-set
+# todoruleid: useless-literal-set
 d = set(1: 'a', 2: 'b', 1: 'a')
 
-# ruleid: useless-literal-dict
+# todoruleid: useless-literal-dict
 d = {1: 'a', 2: 'b', 1: 'a'}
-# ruleid: useless-literal-dict
+# todoruleid: useless-literal-dict
 d = {'a': 1, 'a': 1}
 
 # OK
diff --git a/python/exceptions/exceptions.py b/python/exceptions/exceptions.py
index 6861e4b6d0..3c414d15aa 100644
--- a/python/exceptions/exceptions.py
+++ b/python/exceptions/exceptions.py
@@ -1,7 +1,7 @@
 # ruleid:raise-not-base-exception
 raise "error here"
 
-# ruleid:raise-not-base-exception
+# todoruleid:raise-not-base-exception
 raise 5
 
 
@@ -9,7 +9,7 @@ class Foobar:
     x = 5
 
 
-# ruleid:raise-not-base-exception
+# todoruleid:raise-not-base-exception
 raise Foobar()
 
 
diff --git a/python/smells/missing-hash-with-eq.py b/python/smells/missing-hash-with-eq.py
index fce9273ae7..b3cd8bb7d1 100644
--- a/python/smells/missing-hash-with-eq.py
+++ b/python/smells/missing-hash-with-eq.py
@@ -1,5 +1,5 @@
 
-# ruleid:missing-hash-with-eq
+# todoruleid:missing-hash-with-eq
 class A:
     def __eq__(self, someother):
         pass
diff --git a/python/tempfile/flush.py b/python/tempfile/flush.py
index 6ac6d22ad0..45a8210d38 100644
--- a/python/tempfile/flush.py
+++ b/python/tempfile/flush.py
@@ -32,27 +32,30 @@ def main_c():
 
 
 def main_c():
+    # todoruleid:tempfile-without-flush
     with tempfile.NamedTemporaryFile("w") as fout:
         debug_print(astr)
         fout.write(astr)
         debug_print('wrote file')
-        # ruleid:tempfile-without-flush
+        
         cmd = [binary_name, fout.name, *[str(path) for path in targets]]
 
 
 def main_d():
+    # ruleid:tempfile-without-flush
     fout = tempfile.NamedTemporaryFile('w')
     debug_print(astr)
     fout.write(astr)
-    # ruleid:tempfile-without-flush
+    
     fout.name
     cmd = [binary_name, fout.name, *[str(path) for path in targets]]
 
 
 def main_e():
+    # totoruleid:tempfile-without-flush
     fout = tempfile.NamedTemporaryFile('w')
     debug_print(astr)
     fout.write(astr)
-    # ruleid:tempfile-without-flush
+    
     print(fout.name)
     cmd = [binary_name, fout.name, *[str(path) for path in targets]]

From c7593f44eedc461894da3089b9661c592c879170 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 19:42:04 -0800
Subject: [PATCH 06/17] passing tests

---
 test.py | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/test.py b/test.py
index 80a4ae15dc..b8ad8ddba7 100755
--- a/test.py
+++ b/test.py
@@ -21,6 +21,11 @@
  """
 YML_EXTENSIONS = {".yml", ".yaml"}
 
+def print_debug(msg):
+    global DEBUG
+    if DEBUG:
+        print(msg, file=sys.stderr)
+
 def normalize_rule_id(line):
     """
     given a line like `     # ruleid:foobar` 
@@ -81,14 +86,14 @@ def join_keys(a, b):
         return set(a.keys()).union(set(b.keys()))
 
     for file_path in join_keys(comment_lines, reported_lines):
-        print(comment_lines)
-        print(reported_lines)
+        print_debug(comment_lines)
+        print_debug(reported_lines)
         for check_id in join_keys(comment_lines[file_path], reported_lines[file_path]):
             assert len(set(reported_lines[file_path][check_id])) == len(reported_lines[file_path][check_id]), f"for testing, please don't make rules that fire multiple times on the same line ({check_id} in {file_path})"
             reported = set(reported_lines[file_path][check_id])
             expected = set(comment_lines[file_path][check_id])
             new_cm = compute_confusion_matrix(reported, expected)
-            print(reported, expected, new_cm)
+            print_debug(f"reported: {reported}, expected: {expected}, confusion matrix: {new_cm}")
             old_cm = score_by_checkid[check_id]
             score_by_checkid[check_id] = [old_cm[i] + new_cm[i] for i in range(len(new_cm))]
 
@@ -108,11 +113,11 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
                 continue
             # invoke sgrep
             cmd = ['sgrep-lint', '--no-rewrite-rule-ids', '-f', str(filename)] + [str(t) for t in test_files]
-            print(cmd)
+            print_debug(cmd)
             try:
                 output = subprocess.check_output(cmd, shell=False)
                 output_json = json.loads((output.decode("utf-8")))
-                print(output_json)
+                print_debug(output_json)
                 tested.append((filename, score_output_json(output_json, test_files, ignore_todo)))
             except subprocess.CalledProcessError as ex:
                 print(f'sgrep error running {cmd}: {ex}')
@@ -120,7 +125,7 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
     
     print(f"{len(no_tests)} yaml files missing tests")
     print(f"{len(tested)} yaml files tested")
-    print('check id scoring follows')
+    print('check id scoring:')
     count_failures = 0
     for (filename, (output, num_todo)) in tested:
         print('='*120)
@@ -134,8 +139,13 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
             status = '✔' if good else '⚠'
             todo_text = f"(TODOs: {num_todo})" if num_todo > 0 else ''
             print(f'{status} - {check_id.ljust(60)}TP: {tp}\tTN:{tn}\t FP: {fp}\t FN: {fn} {todo_text}')
+
     if count_failures > 0:
+        print("{count_failures} checks failed tests")
         sys.exit(1)
+    else:
+        print("all tests passed")
+        sys.exit(0)
 
 def main(location: Path, ignore_todo: bool, verbose: bool):
     global DEBUG

From 2bab34acca2ce31c5373adfbdfa16e4d3f5e5eda Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 20:11:45 -0800
Subject: [PATCH 07/17] makefile

---
 .circleci/config.yml | 4 +---
 Makefile             | 4 ++++
 2 files changed, 5 insertions(+), 3 deletions(-)
 create mode 100644 Makefile

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 627ca66588..a14f6d77cc 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -5,9 +5,7 @@ jobs:
       - image: returntocorp/sgrep
     steps:
       - checkout
-      - run: sgrep-lint --validate python .
-      - run: sgrep-lint --validate c .
-      - run: ./test.py --ignore-todo .
+      - run: make test
 workflows:
   version: 2.1
   build_and_test:
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000000..74e030def4
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,4 @@
+test:
+	sgrep-lint --validate python .
+    sgrep-lint --validate c .
+    ./test.py --ignore-todo .

From ce622958076697cb4dfd02b3f7dfa976509dfa84 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 20:47:14 -0800
Subject: [PATCH 08/17] correct python37

---
 python/compatibility/python37.yaml | 2 +-
 test.py                            | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/python/compatibility/python37.yaml b/python/compatibility/python37.yaml
index 637c0c4908..5b8aea17b0 100644
--- a/python/compatibility/python37.yaml
+++ b/python/compatibility/python37.yaml
@@ -86,7 +86,7 @@ rules:
   - id: python37-compatibility-os2-ok2
     patterns:
       - pattern-not-inside: |
-          if hasattr(os, 'pwritev):
+          if hasattr(os, 'pwritev'):
               ...
       - pattern: os.pwritev(...)
     message: "this function is only available on Python 3.7+"
diff --git a/test.py b/test.py
index b8ad8ddba7..660f2f9c20 100755
--- a/test.py
+++ b/test.py
@@ -86,8 +86,6 @@ def join_keys(a, b):
         return set(a.keys()).union(set(b.keys()))
 
     for file_path in join_keys(comment_lines, reported_lines):
-        print_debug(comment_lines)
-        print_debug(reported_lines)
         for check_id in join_keys(comment_lines[file_path], reported_lines[file_path]):
             assert len(set(reported_lines[file_path][check_id])) == len(reported_lines[file_path][check_id]), f"for testing, please don't make rules that fire multiple times on the same line ({check_id} in {file_path})"
             reported = set(reported_lines[file_path][check_id])
@@ -104,6 +102,7 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
     no_tests = []
     tested = []
     sgrep_error = []
+    print('starting tests...')
     for filename in filenames:
         if filename.suffix in YML_EXTENSIONS:
             # find all filenames that have the same name but not extension

From d3b0a089c8a01828cdb34124278758195697f248 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Fri, 31 Jan 2020 20:47:36 -0800
Subject: [PATCH 09/17] correct python37

---
 python/compatibility/python37.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 python/compatibility/python37.py

diff --git a/python/compatibility/python37.py b/python/compatibility/python37.py
new file mode 100644
index 0000000000..ac34fc3b6e
--- /dev/null
+++ b/python/compatibility/python37.py
@@ -0,0 +1,17 @@
+import os
+
+# ruleid:python37-compatability-os-module
+os.pwrite('a')
+
+if hasattr(os, 'pwrite'):
+    # OK
+    os.pwrite('a')
+
+
+if hasattr(os, 'pwritev'):
+    # OK
+    os.pwritev('a')    
+
+
+# ruleid:python37-compatibility-os2-ok2
+os.pwritev('b')

From ad0ebc2a9b07c664ec9918c6b68ebf25e24ddf03 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Sat, 1 Feb 2020 00:04:00 -0500
Subject: [PATCH 10/17] typo

---
 test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test.py b/test.py
index 660f2f9c20..66afd25c31 100755
--- a/test.py
+++ b/test.py
@@ -140,7 +140,7 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
             print(f'{status} - {check_id.ljust(60)}TP: {tp}\tTN:{tn}\t FP: {fp}\t FN: {fn} {todo_text}')
 
     if count_failures > 0:
-        print("{count_failures} checks failed tests")
+        print(f"{count_failures} checks failed tests")
         sys.exit(1)
     else:
         print("all tests passed")

From ec2d98ef51879b24cc67c40fe97a75098b5333e9 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Sat, 1 Feb 2020 00:06:26 -0500
Subject: [PATCH 11/17] update circleci config

---
 .circleci/config.yml | 2 +-
 Makefile             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index a14f6d77cc..2cb2bac49c 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -2,7 +2,7 @@ version: 2.1
 jobs:
   test:
     docker:
-      - image: returntocorp/sgrep
+      - image: returntocorp/sgrep-build:2.3
     steps:
       - checkout
       - run: make test
diff --git a/Makefile b/Makefile
index 74e030def4..6bae4c05b2 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
 test:
-	sgrep-lint --validate python .
-    sgrep-lint --validate c .
+	sgrep-lint --validate --config=./python .
+    sgrep-lint --validate --config=./c .
     ./test.py --ignore-todo .

From 0a4526b3bd587a2ce04860ffbcba2b6ff07ecc3f Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Sat, 1 Feb 2020 00:18:17 -0500
Subject: [PATCH 12/17] tabs in makefile

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 6bae4c05b2..e85f2ba780 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
 test:
 	sgrep-lint --validate --config=./python .
-    sgrep-lint --validate --config=./c .
-    ./test.py --ignore-todo .
+	sgrep-lint --validate --config=./c .
+	./test.py --ignore-todo .

From 2917595c6091be96dd3c71a892ae366894cd81d1 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Sat, 1 Feb 2020 00:20:18 -0500
Subject: [PATCH 13/17] need the built image

---
 .circleci/config.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 2cb2bac49c..a14f6d77cc 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -2,7 +2,7 @@ version: 2.1
 jobs:
   test:
     docker:
-      - image: returntocorp/sgrep-build:2.3
+      - image: returntocorp/sgrep
     steps:
       - checkout
       - run: make test

From 2f5374063630804b611722c354ebb6b37343bddb Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Sat, 1 Feb 2020 00:32:44 -0500
Subject: [PATCH 14/17] work around no makefile

---
 .circleci/config.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index a14f6d77cc..314eb83bcd 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -5,7 +5,9 @@ jobs:
       - image: returntocorp/sgrep
     steps:
       - checkout
-      - run: make test
+      - run: sgrep-lint --validate --config=./python .
+      - run: sgrep-lint --validate --config=./c .
+      - run: ./test.py --ignore-todo .
 workflows:
   version: 2.1
   build_and_test:

From 73e169dadb86b61f696f35733fdf4581838a34f0 Mon Sep 17 00:00:00 2001
From: Isaac Evans <ine@mit.edu>
Date: Sat, 1 Feb 2020 00:50:07 -0500
Subject: [PATCH 15/17] add strict flag

---
 test.py | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/test.py b/test.py
index 66afd25c31..89175bf802 100755
--- a/test.py
+++ b/test.py
@@ -97,7 +97,7 @@ def join_keys(a, b):
 
     return (score_by_checkid, num_todo)
 
-def generate_file_pairs(location: Path, ignore_todo: bool):
+def generate_file_pairs(location: Path, ignore_todo: bool, strict: bool):
     filenames = list(location.rglob("*"))
     no_tests = []
     tested = []
@@ -111,7 +111,7 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
                 no_tests.append(filename)
                 continue
             # invoke sgrep
-            cmd = ['sgrep-lint', '--no-rewrite-rule-ids', '-f', str(filename)] + [str(t) for t in test_files]
+            cmd = ['sgrep-lint', '--strict', '--no-rewrite-rule-ids', '-f', str(filename)] + [str(t) for t in test_files]
             print_debug(cmd)
             try:
                 output = subprocess.check_output(cmd, shell=False)
@@ -121,7 +121,11 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
             except subprocess.CalledProcessError as ex:
                 print(f'sgrep error running {cmd}: {ex}')
                 sgrep_error.append(cmd)
-    
+
+    if len(sgrep_error) and strict:
+        print('exiting due to sgrep/config errors and strict flag')
+        sys.exit(1)
+
     print(f"{len(no_tests)} yaml files missing tests")
     print(f"{len(tested)} yaml files tested")
     print('check id scoring:')
@@ -146,10 +150,10 @@ def generate_file_pairs(location: Path, ignore_todo: bool):
         print("all tests passed")
         sys.exit(0)
 
-def main(location: Path, ignore_todo: bool, verbose: bool):
+def main(location: Path, ignore_todo: bool, verbose: bool, strict: bool):
     global DEBUG
     DEBUG = verbose
-    generate_file_pairs(location, ignore_todo)
+    generate_file_pairs(location, ignore_todo, strict)
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(
@@ -167,6 +171,11 @@ def main(location: Path, ignore_todo: bool, verbose: bool):
         help='ignore rules marked as #todoruleid: in test files',
         action='store_true'
     )
+    parser.add_argument(
+        '-s', '--strict', 
+        help='fail on any error',
+        action='store_true'
+    )
     parser.add_argument(
         '-v', '--verbose', 
         help='debug output',
@@ -174,4 +183,4 @@ def main(location: Path, ignore_todo: bool, verbose: bool):
     )
     args = parser.parse_args()
     _test_compute_confusion_matrix()
-    main(Path(args.directory), args.ignore_todo, args.verbose)
+    main(Path(args.directory), args.ignore_todo, args.verbose, args.strict)

From e22a0037ec9838206aca47afd39f4f2b40e6c444 Mon Sep 17 00:00:00 2001
From: Drew Dennison <drew@returntocorp.com>
Date: Sat, 1 Feb 2020 11:41:46 -0800
Subject: [PATCH 16/17] add make

---
 .circleci/config.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 314eb83bcd..5aa56b5c99 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -4,6 +4,7 @@ jobs:
     docker:
       - image: returntocorp/sgrep
     steps:
+      - run: apk add make
       - checkout
       - run: sgrep-lint --validate --config=./python .
       - run: sgrep-lint --validate --config=./c .

From 1d82b5fed1d0bf6bd64cb6313321de22f5ca9874 Mon Sep 17 00:00:00 2001
From: Drew Dennison <drew@returntocorp.com>
Date: Sat, 1 Feb 2020 11:50:15 -0800
Subject: [PATCH 17/17] switch to make test

---
 .circleci/config.yml | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 5aa56b5c99..ec12c3e8a4 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -6,9 +6,7 @@ jobs:
     steps:
       - run: apk add make
       - checkout
-      - run: sgrep-lint --validate --config=./python .
-      - run: sgrep-lint --validate --config=./c .
-      - run: ./test.py --ignore-todo .
+      - run: make test
 workflows:
   version: 2.1
   build_and_test: