Patchwork D2864: contrib: open a hole in the open().read() ban for open().close()

login
register
mail settings
Submitter phabricator
Date March 14, 2018, 7:53 p.m.
Message ID <differential-rev-PHID-DREV-4nusuzzz2ztjco2n7rdi-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29511/
State Superseded
Headers show

Comments

phabricator - March 14, 2018, 7:53 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It turns out open().close() is both fine and something we occasionally
  do to verify something can be written. The few cases in the codebase
  were getting missed due to a regular expression bug (which I discussed
  in my previous change), but since I'm about to fix the bug, I need to
  fix the patterns.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2864

AFFECTED FILES
  contrib/check-code.py

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -319,9 +319,9 @@ 
      "use util.readfile() instead"),
     (r'[\s\(](open|file)\([^)]*\)\.write\(',
      "use util.writefile() instead"),
-    (r'^[\s\(]*(open(er)?|file)\([^)]*\)',
+    (r'^[\s\(]*(open(er)?|file)\([^)]*\)(?!\.close\(\))',
      "always assign an opened file to a variable, and close it afterwards"),
-    (r'[\s\(](open|file)\([^)]*\)\.',
+    (r'[\s\(](open|file)\([^)]*\)\.(?!close\(\))',
      "always assign an opened file to a variable, and close it afterwards"),
     (r'(?i)descend[e]nt', "the proper spelling is descendAnt"),
     (r'\.debug\(\_', "don't mark debug messages for translation"),