From 70018aa683f93998b1b263cb0ddb5ec13ee4cfdf Mon Sep 17 00:00:00 2001
From: Dheepak Krishnamurthy <kdheepak89@gmail.com>
Date: Tue, 9 Oct 2018 06:09:22 -0600
Subject: [PATCH] Add checks for except: body blocks (#9191)

---
 compiler/parser.nim                 |  1 -
 compiler/semstmts.nim               | 21 ++++++++++++++++++++-
 tests/errmsgs/tgeneral_excepts.nim  | 10 ++++++++++
 tests/errmsgs/tmultiple_finally.nim | 12 ++++++++++++
 4 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 tests/errmsgs/tgeneral_excepts.nim
 create mode 100644 tests/errmsgs/tmultiple_finally.nim

diff --git a/compiler/parser.nim b/compiler/parser.nim
index 69e372e4e1..54aaf81941 100644
--- a/compiler/parser.nim
+++ b/compiler/parser.nim
@@ -1578,7 +1578,6 @@ proc parseTry(p: var TParser; isExpr: bool): PNode =
     colcom(p, b)
     addSon(b, parseStmt(p))
     addSon(result, b)
-    if b.kind == nkFinally: break
   if b == nil: parMessage(p, "expected 'except'")
 
 proc parseExceptBlock(p: var TParser, kind: TNodeKind): PNode =
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index f2cb2dcb3b..2817b5ef95 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -207,6 +207,8 @@ proc semTry(c: PContext, n: PNode; flags: TExprFlags): PNode =
   typ = commonType(typ, n[0].typ)
 
   var last = sonsLen(n) - 1
+  var catchAllExcepts = 0
+
   for i in countup(1, last):
     let a = n.sons[i]
     checkMinSonsLen(a, 1, c.config)
@@ -227,8 +229,16 @@ proc semTry(c: PContext, n: PNode; flags: TExprFlags): PNode =
         # Overwrite symbol in AST with the symbol in the symbol table.
         a[0][2] = newSymNode(symbol, a[0][2].info)
 
+      elif a.len == 1:
+          # count number of ``except: body`` blocks
+          inc catchAllExcepts
+
       else:
         # support ``except KeyError, ValueError, ... : body``
+        if catchAllExcepts > 0:
+          # if ``except: body`` already encountered,
+          # cannot be followed by a ``except KeyError, ... : body`` block
+          inc catchAllExcepts
         var is_native, is_imported: bool
         for j in 0..a.len-2:
           let tmp = semExceptBranchType(a[j])
@@ -238,9 +248,18 @@ proc semTry(c: PContext, n: PNode; flags: TExprFlags): PNode =
         if is_native and is_imported:
           localError(c.config, a[0].info, "Mix of imported and native exception types is not allowed in one except branch")
 
-    elif a.kind != nkFinally:
+    elif a.kind == nkFinally:
+      if i != n.len-1:
+        localError(c.config, a.info, "Only one finally is allowed after all other branches")
+
+    else:
       illFormedAst(n, c.config)
 
+    if catchAllExcepts > 1:
+      # if number of ``except: body`` blocks is greater than 1
+      # or more specific exception follows a general except block, it is invalid
+      localError(c.config, a.info, "Only one general except clause is allowed after more specific exceptions")
+
     # last child of an nkExcept/nkFinally branch is a statement:
     a[^1] = semExprBranchScope(c, a[^1])
     if a.kind != nkFinally: typ = commonType(typ, a[^1])
diff --git a/tests/errmsgs/tgeneral_excepts.nim b/tests/errmsgs/tgeneral_excepts.nim
new file mode 100644
index 0000000000..8f8be797fd
--- /dev/null
+++ b/tests/errmsgs/tgeneral_excepts.nim
@@ -0,0 +1,10 @@
+discard """
+errormsg: "Only one general except clause is allowed after more specific exceptions"
+"""
+
+try:
+  discard
+except:
+  discard
+except:
+  discard
diff --git a/tests/errmsgs/tmultiple_finally.nim b/tests/errmsgs/tmultiple_finally.nim
new file mode 100644
index 0000000000..519a801452
--- /dev/null
+++ b/tests/errmsgs/tmultiple_finally.nim
@@ -0,0 +1,12 @@
+discard """
+errormsg: "Only one finally is allowed after all other branches"
+"""
+
+try:
+  discard
+finally:
+  discard
+finally:
+  discard
+
+