check-code: ignore naked excepts with a "re-raise" comment
authorBrodie Rao <brodie@sf.io>
Sun, 13 May 2012 13:18:06 +0200
changeset 16705 c2d9ef43ff6c
parent 16704 1f3acc30bdfe
child 16706 a270ec977ba6
check-code: ignore naked excepts with a "re-raise" comment This also promotes the naked except check from a warning to an error.
contrib/check-code.py
contrib/shrink-revlog.py
hgext/mq.py
mercurial/commands.py
mercurial/dispatch.py
mercurial/hg.py
mercurial/keepalive.py
mercurial/localrepo.py
mercurial/patch.py
mercurial/repair.py
mercurial/util.py
tests/test-check-code-hg.t
--- a/contrib/check-code.py	Sun May 13 13:17:50 2012 +0200
+++ b/contrib/check-code.py	Sun May 13 13:18:06 2012 +0200
@@ -203,10 +203,10 @@
     (r'(?i)descendent', "the proper spelling is descendAnt"),
     (r'\.debug\(\_', "don't mark debug messages for translation"),
     (r'\.strip\(\)\.split\(\)', "no need to strip before splitting"),
+    (r'^\s*except\s*:', "warning: naked except clause", r'#.*re-raises'),
   ],
   # warnings
   [
-    (r'^\s*except\s*:', "warning: naked except clause"),
     (r'ui\.(status|progress|write|note|warn)\([\'\"]x',
      "warning: unwrapped ui message"),
   ]
@@ -355,7 +355,13 @@
 
         prelines = None
         errors = []
-        for p, msg in pats:
+        for pat in pats:
+            if len(pat) == 3:
+                p, msg, ignore = pat
+            else:
+                p, msg = pat
+                ignore = None
+
             # fix-up regexes for multiline searches
             po = p
             # \s doesn't match \n
@@ -386,6 +392,8 @@
                         print "Skipping %s for %s:%s (check-code -ignore)" % (
                             name, f, n)
                     continue
+                elif ignore and re.search(ignore, l, re.MULTILINE):
+                    continue
                 bd = ""
                 if blame:
                     bd = 'working directory'
--- a/contrib/shrink-revlog.py	Sun May 13 13:17:50 2012 +0200
+++ b/contrib/shrink-revlog.py	Sun May 13 13:18:06 2012 +0200
@@ -240,7 +240,7 @@
             writerevs(ui, r1, r2, order, tr)
             report(ui, r1, r2)
             tr.close()
-        except:
+        except: # re-raises
             # Abort transaction first, so we truncate the files before
             # deleting them.
             tr.abort()
--- a/hgext/mq.py	Sun May 13 13:17:50 2012 +0200
+++ b/hgext/mq.py	Sun May 13 13:18:06 2012 +0200
@@ -712,7 +712,7 @@
                 tr.close()
                 self.savedirty()
                 return 2, repo.dirstate.p1()
-            except:
+            except: # re-raises
                 try:
                     tr.abort()
                 finally:
@@ -1077,7 +1077,7 @@
                     r = self.qrepo()
                     if r:
                         r[None].add([patchfn])
-                except:
+                except: # re-raises
                     repo.rollback()
                     raise
             except Exception:
@@ -1303,7 +1303,7 @@
                 else:
                     ret = self.apply(repo, s, list, all_files=all_files,
                                      tobackup=tobackup, check=check)
-            except:
+            except: # re-raises
                 self.ui.warn(_('cleaning up working directory...'))
                 node = repo.dirstate.p1()
                 hg.revert(repo, node, None)
@@ -1629,7 +1629,7 @@
                 self.applieddirty = True
                 self.strip(repo, [top], update=False,
                            backup='strip')
-            except:
+            except: # re-raises
                 repo.dirstate.invalidate()
                 raise
 
@@ -1643,7 +1643,7 @@
                 # only write patch after a successful commit
                 patchf.close()
                 self.applied.append(statusentry(n, patchfn))
-            except:
+            except: # re-raises
                 ctx = repo[cparents[0]]
                 repo.dirstate.rebuild(ctx.node(), ctx.manifest())
                 self.savedirty()
--- a/mercurial/commands.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/commands.py	Sun May 13 13:18:06 2012 +0200
@@ -3775,7 +3775,7 @@
                 tr.close()
             if msgs:
                 repo.savecommitmessage('\n* * *\n'.join(msgs))
-        except:
+        except: # re-raises
             # wlock.release() indirectly calls dirstate.write(): since
             # we're crashing, we do not want to change the working dir
             # parent after all, so make sure it writes nothing
--- a/mercurial/dispatch.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/dispatch.py	Sun May 13 13:18:06 2012 +0200
@@ -88,7 +88,7 @@
                 return _dispatch(req)
             finally:
                 ui.flush()
-        except:
+        except: # re-raises
             # enter the debugger when we hit an exception
             if '--debugger' in req.args:
                 traceback.print_exc()
@@ -204,7 +204,7 @@
         return inst.code
     except socket.error, inst:
         ui.warn(_("abort: %s\n") % inst.args[-1])
-    except:
+    except: # re-raises
         ui.warn(_("** unknown exception encountered,"
                   " please report by visiting\n"))
         ui.warn(_("**  http://mercurial.selenic.com/wiki/BugTracker\n"))
--- a/mercurial/hg.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/hg.py	Sun May 13 13:18:06 2012 +0200
@@ -203,7 +203,7 @@
         else:
             ui.debug("copied %d files\n" % num)
         return destlock
-    except:
+    except: # re-raises
         release(destlock)
         raise
 
--- a/mercurial/keepalive.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/keepalive.py	Sun May 13 13:18:06 2012 +0200
@@ -290,7 +290,7 @@
             # worked.  We'll check the version below, too.
         except (socket.error, httplib.HTTPException):
             r = None
-        except:
+        except: # re-raises
             # adding this block just in case we've missed
             # something we will still raise the exception, but
             # lets try and close the connection and remove it
--- a/mercurial/localrepo.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/localrepo.py	Sun May 13 13:18:06 2012 +0200
@@ -1180,7 +1180,7 @@
                 self.hook("precommit", throw=True, parent1=hookp1,
                           parent2=hookp2)
                 ret = self.commitctx(cctx, True)
-            except:
+            except: # re-raises
                 if edited:
                     self.ui.write(
                         _('note: commit message saved in %s\n') % msgfn)
--- a/mercurial/patch.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/patch.py	Sun May 13 13:18:06 2012 +0200
@@ -245,7 +245,7 @@
                         tmpfp.write('\n')
             elif not diffs_seen and message and content_type == 'text/plain':
                 message += '\n' + payload
-    except:
+    except: # re-raises
         tmpfp.close()
         os.unlink(tmpname)
         raise
--- a/mercurial/repair.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/repair.py	Sun May 13 13:18:06 2012 +0200
@@ -131,7 +131,7 @@
                 file, troffset, ignore = tr.entries[i]
                 repo.sopener(file, 'a').truncate(troffset)
             tr.close()
-        except:
+        except: # re-raises
             tr.abort()
             raise
 
@@ -160,7 +160,7 @@
         for m in updatebm:
             bm[m] = repo['.'].node()
         bookmarks.write(repo)
-    except:
+    except: # re-raises
         if backupfile:
             ui.warn(_("strip failed, full bundle stored in '%s'\n")
                     % backupfile)
--- a/mercurial/util.py	Sun May 13 13:17:50 2012 +0200
+++ b/mercurial/util.py	Sun May 13 13:18:06 2012 +0200
@@ -760,7 +760,7 @@
             ofp.write(chunk)
         ifp.close()
         ofp.close()
-    except:
+    except: # re-raises
         try: os.unlink(temp)
         except OSError: pass
         raise
--- a/tests/test-check-code-hg.t	Sun May 13 13:17:50 2012 +0200
+++ b/tests/test-check-code-hg.t	Sun May 13 13:18:06 2012 +0200
@@ -8,9 +8,6 @@
   $ hg manifest | xargs "$check_code" || echo 'FAILURE IS NOT AN OPTION!!!'
 
   $ hg manifest | xargs "$check_code" --warnings --nolineno --per-file=0 || true
-  contrib/shrink-revlog.py:0:
-   >         except:
-   warning: naked except clause
   hgext/convert/cvsps.py:0:
    >                     ui.write('Ancestors: %s\n' % (','.join(r)))
    warning: unwrapped ui message
@@ -69,15 +66,6 @@
    >     ui.note("hg ci -m '%s'\n" % msg)
    warning: unwrapped ui message
   hgext/mq.py:0:
-   >                 except:
-   warning: naked except clause
-  hgext/mq.py:0:
-   >             except:
-   warning: naked except clause
-   warning: naked except clause
-   warning: naked except clause
-   warning: naked except clause
-  hgext/mq.py:0:
    >         ui.write("mq:     %s\n" % ', '.join(m))
    warning: unwrapped ui message
   hgext/patchbomb.py:0:
@@ -117,9 +105,6 @@
    >             ui.write('deltas against p2    : ' + fmt % pcfmt(nump2, numdeltas))
    warning: unwrapped ui message
   mercurial/commands.py:0:
-   >         except:
-   warning: naked except clause
-  mercurial/commands.py:0:
    >         ui.write("common heads: %s\n" % " ".join([short(n) for n in common]))
    warning: unwrapped ui message
   mercurial/commands.py:0:
@@ -177,33 +162,6 @@
   mercurial/commands.py:0:
    >     ui.write('symlink: %s\n' % (util.checklink(path) and 'yes' or 'no'))
    warning: unwrapped ui message
-  mercurial/dispatch.py:0:
-   >         except:
-   warning: naked except clause
-  mercurial/dispatch.py:0:
-   >     except:
-   warning: naked except clause
-  mercurial/hg.py:0:
-   >     except:
-   warning: naked except clause
-  mercurial/keepalive.py:0:
-   >         except:
-   warning: naked except clause
-  mercurial/localrepo.py:0:
-   >             except:
-   warning: naked except clause
-  mercurial/patch.py:0:
-   >     except:
-   warning: naked except clause
-  mercurial/repair.py:0:
-   >         except:
-   warning: naked except clause
-  mercurial/repair.py:0:
-   >     except:
-   warning: naked except clause
-  mercurial/util.py:0:
-   >     except:
-   warning: naked except clause
   tests/autodiff.py:0:
    >         ui.write('data lost for: %s\n' % fn)
    warning: unwrapped ui message