PR feedback, more error-checking

Author: dmalan

src/cs50/flask.py

@@ -19,7 +19,7 @@
     try:
         import flask.logging
         flask.logging.default_handler.formatter.formatException = lambda exc_info: formatException(*exc_info)
-    except:
+    except Exception:
         pass
 
     # Enable logging when Flask is in use,
@@ -28,21 +28,19 @@
     try:
         import flask
         from .sql import SQL
+    except ImportError:
+        pass
+    else:
         _before = SQL.execute
         def _after(*args, **kwargs):
             disabled = logging.getLogger("cs50").disabled
             if flask.current_app:
                 logging.getLogger("cs50").disabled = False
             try:
-                ret = _before(*args, **kwargs)
-                logging.getLogger("cs50").disabled = disabled
-                return ret
-            except:
+                return _before(*args, **kwargs)
+            finally:
                 logging.getLogger("cs50").disabled = disabled
-                raise
         SQL.execute = _after
-    except:
-        pass
 
     # Add support for Cloud9 proxy so that flask.redirect doesn't redirect from HTTPS to HTTP
     # http://stackoverflow.com/a/23504684/5156190
@@ -58,6 +56,5 @@ def _after(*args, **kwargs):
         except:
             pass
 
-except Exception as e:
-    print(e)
+except Exception:
     pass

src/cs50/sql.py

@@ -100,33 +100,54 @@ def execute(self, sql, *args, **kwargs):
                 # Determine paramstyle, name
                 _paramstyle, name = _parse_placeholder(token)
 
-                # Ensure paramstyle is consistent
-                if paramstyle is not None and _paramstyle != paramstyle:
-                    raise RuntimeError("inconsistent paramstyle")
-
                 # Remember paramstyle
-                if paramstyle is None:
+                if not paramstyle:
                     paramstyle = _paramstyle
 
+                # Ensure paramstyle is consistent
+                elif _paramstyle != paramstyle:
+                    raise RuntimeError("inconsistent paramstyle")
+
                 # Remember placeholder's index, name
                 placeholders[index] = name
 
-        # In case user passes args in list or tuple
-        if len(args) == 1 and (isinstance(args[0], list) or isinstance(args[0], tuple)) and len(placeholders) != 1:
-            args = args[0]
+        # If more placeholders than arguments
+        if len(args) == 1 and len(placeholders) > 1:
+
+            # If user passed args as list or tuple, explode values into args
+            if isinstance(args[0], (list, tuple)):
+                args = args[0]
+
+            # If user passed kwargs as dict, migrate values from args to kwargs
+            elif len(kwargs) == 0 and isinstance(args[0], dict):
+                kwargs = args[0]
+                args = []
+
+        # If no placeholders
+        if not paramstyle:
+
+            # Error-check like qmark if args
+            if args:
+                paramstyle = "qmark"
+
+            # Error-check like named if kwargs
+            elif kwargs:
+                paramstyle = "named"
 
-        # In case user passes kwargs in dict
-        if len(args) == 1 and len(kwargs) == 0 and isinstance(args[0], dict) and len(placeholders) != 1:
-            kwargs = args[0]
+        # In case of errors
+        _placeholders = ", ".join([str(tokens[index]) for index in placeholders])
+        _args = ", ".join([str(self._escape(arg)) for arg in args])
+        #_kwargs = ", ".join([str(self._escape(arg)) for arg in args])
 
         # qmark
         if paramstyle == "qmark":
 
             # Validate number of placeholders
-            if len(placeholders) < len(args):
-                raise RuntimeError("too few placeholders")
-            elif len(placeholders) > len(args):
-                raise RuntimeError("too many placeholders")
+            if len(placeholders) != len(args):
+                if len(placeholders) < len(args):
+                    raise RuntimeError("fewer placeholders ({}) than values ({})".format(_placeholders, _args))
+                else:
+                    raise RuntimeError("more placeholders ({}) than values ({})".format(_placeholders, _args))
 
             # Escape values
             for i, index in enumerate(placeholders.keys()):
@@ -138,8 +159,8 @@ def execute(self, sql, *args, **kwargs):
             # Escape values
             for index, name in placeholders.items():
                 i = int(name) - 1
-                if i < 0 or i >= len(args):
-                    raise RuntimeError("placeholder out of range")
+                if i >= len(args):
+                    raise RuntimeError("placeholder (:{}) greater than number of values ({})".format(name, _args))
                 tokens[index] = self._escape(args[i])
 
         # named
@@ -148,17 +169,18 @@ def execute(self, sql, *args, **kwargs):
             # Escape values
             for index, name in placeholders.items():
                 if name not in kwargs:
-                    raise RuntimeError("missing value for placeholder")
+                    raise RuntimeError("missing value for placeholder (:{})".format(name))
                 tokens[index] = self._escape(kwargs[name])
 
         # format
         elif paramstyle == "format":
 
             # Validate number of placeholders
-            if len(placeholders) < len(args):
-                raise RuntimeError("too few placeholders")
-            elif len(placeholders) > len(args):
-                raise RuntimeError("too many placeholders")
+            if len(placeholders) != len(args):
+                if len(placeholders) < len(args):
+                    raise RuntimeError("fewer placeholders ({}) than values ({})".format(_placeholders, _args))
+                else:
+                    raise RuntimeError("more placeholders ({}) than values ({})".format(_placeholders, _args))
 
             # Escape values
             for i, index in enumerate(placeholders.keys()):
@@ -170,7 +192,7 @@ def execute(self, sql, *args, **kwargs):
             # Escape values
             for index, name in placeholders.items():
                 if name not in kwargs:
-                    raise RuntimeError("missing value for placeholder")
+                    raise RuntimeError("missing value for placeholder (:{})".format(name))
                 tokens[index] = self._escape(kwargs[name])
 
         # Join tokens into statement

tests/sqlite.py

@@ -17,6 +17,8 @@
 db.execute("SELECT * FROM Employee WHERE FirstName IN (?)", ("Andrew",))
 db.execute("SELECT * FROM Employee WHERE FirstName IN (?)", ["Andrew", "Nancy"])
 db.execute("SELECT * FROM Employee WHERE FirstName IN (?)", ("Andrew", "Nancy"))
+db.execute("SELECT * FROM Employee WHERE FirstName IN (?)", [])
+db.execute("SELECT * FROM Employee WHERE FirstName IN (?)", ())
 
 db.execute("SELECT * FROM Employee WHERE FirstName = ? AND LastName = ?", "Andrew", "Adams")
 db.execute("SELECT * FROM Employee WHERE FirstName = ? AND LastName = ?", ["Andrew", "Adams"])