check-code: detect "missing _() in ui message" more exactly

Before this patch, "missing _() in ui message" rule overlooks
translatable message, which starts with other than alphabet.

To detect "missing _() in ui message" more exactly, this patch
improves the regexp with assumptions below.

  - sequence consisting of below might precede "translatable message"
    in same string token

    - formatting string, which starts with '%'
    - escaped character, which starts with 'b' (as replacement of '\\'), or
    - characters other than '%', 'b' and 'x' (as replacement of alphabet)

  - any string tokens might precede a string token, which contains
    "translatable message"

This patch builds an input file, which is used to examine "missing _()
in ui message" detection, before '"$check_code" stringjoin.py' in
test-contrib-check-code.t, because this reduces amount of change churn
in subsequent patch.

This patch also applies "()" instead of "_()" on messages below to
hide false-positives:

  - messages for ui.debug() or debug commands/tools
    - contrib/debugshell.py
    - hgext/win32mbcs.py (ui.write() is used, though)
    - mercurial/commands.py
      - _debugchangegroup
      - debugindex
      - debuglocks
      - debugrevlog
      - debugrevspec
      - debugtemplate

  - untranslatable messages
    - doc/gendoc.py (ReST specific text)
    - hgext/hgk.py (permission string)
    - hgext/keyword.py (text written into configuration file)
    - mercurial/cmdutil.py (formatting strings for JSON)

contrib/check-code.py

@@ -329,7 +329,20 b' pypats = ['
     # rules depending on implementation of repquote()
     (r' x+[xpqo%APM][\'"]\n\s+[\'"]x',
      'string join across lines with no space'),
-    (r'ui\.(status|progress|write|note|warn)\([\'\"]x',
+    (r'''(?x)ui\.(status|progress|write|note|warn)\(
+         [ \t\n#]*
+         (?# any strings/comments might precede a string, which
+           # contains translatable message)
+         ((['"]|\'\'\'|""")[ \npq%bAPMxno]*(['"]|\'\'\'|""")[ \t\n#]+)*
+         (?# sequence consisting of below might precede translatable message
+           # - formatting string: "% 10s", "%05d", "% -3.2f", "%*s", "%%" ...
+           # - escaped character: "\\", "\n", "\0" ...
+           # - character other than '%', 'b' as '\', and 'x' as alphabet)
+         (['"]|\'\'\'|""")
+         ((%([ n]?[PM]?([np]+|A))?x)|%%|b[bnx]|[ \nnpqAPMo])*x
+         (?# this regexp can't use [^...] style,
+           # because _preparepats forcibly adds "\n" into [^...],
+           # even though this regexp wants match it against "\n")''',
      "missing _() in ui message (use () to hide false-positives)"),
   ],
   # warnings

contrib/debugshell.py

@@ -52,7 +52,7 b' def debugshell(ui, repo, **opts):'
         with demandimport.deactivated():
             __import__(pdbmap[debugger])
     except ImportError:
-        ui.warn("%s debugger specified but %s module was not found\n"
+        ui.warn(("%s debugger specified but %s module was not found\n")
                 % (debugger, pdbmap[debugger]))
         debugger = 'pdb'
 

doc/gendoc.py

@@ -117,11 +117,11 b' def showdoc(ui):'
     ui.write(_("This section contains help for extensions that are "
                "distributed together with Mercurial. Help for other "
                "extensions is available in the help system."))
-    ui.write("\n\n"
+    ui.write(("\n\n"
              ".. contents::\n"
              "   :class: htmlonly\n"
              "   :local:\n"
-             "   :depth: 1\n\n")
+             "   :depth: 1\n\n"))
 
     for extensionname in sorted(allextensionnames()):
         mod = extensions.load(ui, extensionname, None)

hgext/hgk.py

@@ -81,13 +81,13 b' def difftree(ui, repo, node1=None, node2'
 
         for f in modified:
             # TODO get file permissions
-            ui.write(":100664 100664 %s %s M\t%s\t%s\n" %
+            ui.write((":100664 100664 %s %s M\t%s\t%s\n") %
                      (short(mmap[f]), short(mmap2[f]), f, f))
         for f in added:
-            ui.write(":000000 100664 %s %s N\t%s\t%s\n" %
+            ui.write((":000000 100664 %s %s N\t%s\t%s\n") %
                      (empty, short(mmap2[f]), f, f))
         for f in removed:
-            ui.write(":100664 000000 %s %s D\t%s\t%s\n" %
+            ui.write((":100664 000000 %s %s D\t%s\t%s\n") %
                      (short(mmap[f]), empty, f, f))
     ##
 

hgext/keyword.py

@@ -455,7 +455,7 b' def demo(ui, repo, *args, **opts):'
 
     uisetup(ui)
     reposetup(ui, repo)
-    ui.write('[extensions]\nkeyword =\n')
+    ui.write(('[extensions]\nkeyword =\n'))
     demoitems('keyword', ui.configitems('keyword'))
     demoitems('keywordset', ui.configitems('keywordset'))
     demoitems('keywordmaps', kwmaps.iteritems())

hgext/win32mbcs.py

@@ -192,5 +192,5 b' def extsetup(ui):'
         # command line options is not yet applied when
         # extensions.loadall() is called.
         if '--debug' in sys.argv:
-            ui.write("[win32mbcs] activated with encoding: %s\n"
+            ui.write(("[win32mbcs] activated with encoding: %s\n")
                      % _encoding)

mercurial/cmdutil.py

@@ -1405,24 +1405,24 b' class jsonchangeset(changeset_printer):'
             self.ui.write(",\n {")
 
         if self.ui.quiet:
-            self.ui.write('\n  "rev": %s' % jrev)
-            self.ui.write(',\n  "node": %s' % jnode)
+            self.ui.write(('\n  "rev": %s') % jrev)
+            self.ui.write((',\n  "node": %s') % jnode)
             self.ui.write('\n }')
             return
 
-        self.ui.write('\n  "rev": %s' % jrev)
-        self.ui.write(',\n  "node": %s' % jnode)
-        self.ui.write(',\n  "branch": "%s"' % j(ctx.branch()))
-        self.ui.write(',\n  "phase": "%s"' % ctx.phasestr())
-        self.ui.write(',\n  "user": "%s"' % j(ctx.user()))
-        self.ui.write(',\n  "date": [%d, %d]' % ctx.date())
-        self.ui.write(',\n  "desc": "%s"' % j(ctx.description()))
-
-        self.ui.write(',\n  "bookmarks": [%s]' %
+        self.ui.write(('\n  "rev": %s') % jrev)
+        self.ui.write((',\n  "node": %s') % jnode)
+        self.ui.write((',\n  "branch": "%s"') % j(ctx.branch()))
+        self.ui.write((',\n  "phase": "%s"') % ctx.phasestr())
+        self.ui.write((',\n  "user": "%s"') % j(ctx.user()))
+        self.ui.write((',\n  "date": [%d, %d]') % ctx.date())
+        self.ui.write((',\n  "desc": "%s"') % j(ctx.description()))
+
+        self.ui.write((',\n  "bookmarks": [%s]') %
                       ", ".join('"%s"' % j(b) for b in ctx.bookmarks()))
-        self.ui.write(',\n  "tags": [%s]' %
+        self.ui.write((',\n  "tags": [%s]') %
                       ", ".join('"%s"' % j(t) for t in ctx.tags()))
-        self.ui.write(',\n  "parents": [%s]' %
+        self.ui.write((',\n  "parents": [%s]') %
                       ", ".join('"%s"' % c.hex() for c in ctx.parents()))
 
         if self.ui.debugflag:
@@ -1430,26 +1430,26 b' class jsonchangeset(changeset_printer):'
                 jmanifestnode = 'null'
             else:
                 jmanifestnode = '"%s"' % hex(ctx.manifestnode())
-            self.ui.write(',\n  "manifest": %s' % jmanifestnode)
-
-            self.ui.write(',\n  "extra": {%s}' %
+            self.ui.write((',\n  "manifest": %s') % jmanifestnode)
+
+            self.ui.write((',\n  "extra": {%s}') %
                           ", ".join('"%s": "%s"' % (j(k), j(v))
                                     for k, v in ctx.extra().items()))
 
             files = ctx.p1().status(ctx)
-            self.ui.write(',\n  "modified": [%s]' %
+            self.ui.write((',\n  "modified": [%s]') %
                           ", ".join('"%s"' % j(f) for f in files[0]))
-            self.ui.write(',\n  "added": [%s]' %
+            self.ui.write((',\n  "added": [%s]') %
                           ", ".join('"%s"' % j(f) for f in files[1]))
-            self.ui.write(',\n  "removed": [%s]' %
+            self.ui.write((',\n  "removed": [%s]') %
                           ", ".join('"%s"' % j(f) for f in files[2]))
 
         elif self.ui.verbose:
-            self.ui.write(',\n  "files": [%s]' %
+            self.ui.write((',\n  "files": [%s]') %
                           ", ".join('"%s"' % j(f) for f in ctx.files()))
 
             if copies:
-                self.ui.write(',\n  "copies": {%s}' %
+                self.ui.write((',\n  "copies": {%s}') %
                               ", ".join('"%s": "%s"' % (j(k), j(v))
                                                         for k, v in copies))
 
@@ -1463,12 +1463,13 b' class jsonchangeset(changeset_printer):'
                 self.ui.pushbuffer()
                 diffordiffstat(self.ui, self.repo, diffopts, prev, node,
                                match=matchfn, stat=True)
-                self.ui.write(',\n  "diffstat": "%s"' % j(self.ui.popbuffer()))
+                self.ui.write((',\n  "diffstat": "%s"')
+                              % j(self.ui.popbuffer()))
             if diff:
                 self.ui.pushbuffer()
                 diffordiffstat(self.ui, self.repo, diffopts, prev, node,
                                match=matchfn, stat=False)
-                self.ui.write(',\n  "diff": "%s"' % j(self.ui.popbuffer()))
+                self.ui.write((',\n  "diff": "%s"') % j(self.ui.popbuffer()))
 
         self.ui.write("\n }")
 

mercurial/commands.py

@@ -2095,7 +2095,7 b' def debugbundle(ui, bundlepath, all=None'
 def _debugchangegroup(ui, gen, all=None, indent=0, **opts):
     indent_string = ' ' * indent
     if all:
-        ui.write("%sformat: id, p1, p2, cset, delta base, len(delta)\n"
+        ui.write(("%sformat: id, p1, p2, cset, delta base, len(delta)\n")
                  % indent_string)
 
         def showchunks(named):
@@ -2562,12 +2562,12 b' def debugindex(ui, repo, file_=None, **o'
         break
 
     if format == 0:
-        ui.write("   rev    offset  length " + basehdr + " linkrev"
-                 " %s %s p2\n" % ("nodeid".ljust(idlen), "p1".ljust(idlen)))
+        ui.write(("   rev    offset  length " + basehdr + " linkrev"
+                 " %s %s p2\n") % ("nodeid".ljust(idlen), "p1".ljust(idlen)))
     elif format == 1:
-        ui.write("   rev flag   offset   length"
+        ui.write(("   rev flag   offset   length"
                  "     size " + basehdr + "   link     p1     p2"
-                 " %s\n" % "nodeid".rjust(idlen))
+                 " %s\n") % "nodeid".rjust(idlen))
 
     for i in r:
         node = r.node(i)
@@ -3030,13 +3030,13 b' def debuglocks(ui, repo, **opts):'
                     else:
                         locker = 'user %s, process %s, host %s' \
                                  % (user, pid, host)
-                ui.write("%-6s %s (%ds)\n" % (name + ":", locker, age))
+                ui.write(("%-6s %s (%ds)\n") % (name + ":", locker, age))
                 return 1
             except OSError as e:
                 if e.errno != errno.ENOENT:
                     raise
 
-        ui.write("%-6s free\n" % (name + ":"))
+        ui.write(("%-6s free\n") % (name + ":"))
         return 0
 
     held += report(repo.svfs, "lock", repo.lock)
@@ -3329,8 +3329,8 b' def debugrevlog(ui, repo, file_=None, **'
 
     if opts.get("dump"):
         numrevs = len(r)
-        ui.write("# rev p1rev p2rev start   end deltastart base   p1   p2"
-                 " rawsize totalsize compression heads chainlen\n")
+        ui.write(("# rev p1rev p2rev start   end deltastart base   p1   p2"
+                 " rawsize totalsize compression heads chainlen\n"))
         ts = 0
         heads = set()
 
@@ -3519,18 +3519,19 b' def debugrevspec(ui, repo, expr, **opts)'
         ui.note(revset.prettyformat(tree), "\n")
         newtree = revset.expandaliases(ui, tree)
         if newtree != tree:
-            ui.note("* expanded:\n", revset.prettyformat(newtree), "\n")
+            ui.note(("* expanded:\n"), revset.prettyformat(newtree), "\n")
         tree = newtree
         newtree = revset.foldconcat(tree)
         if newtree != tree:
-            ui.note("* concatenated:\n", revset.prettyformat(newtree), "\n")
+            ui.note(("* concatenated:\n"), revset.prettyformat(newtree), "\n")
         if opts["optimize"]:
             optimizedtree = revset.optimize(newtree)
-            ui.note("* optimized:\n", revset.prettyformat(optimizedtree), "\n")
+            ui.note(("* optimized:\n"),
+                    revset.prettyformat(optimizedtree), "\n")
     func = revset.match(ui, expr, repo)
     revs = func(repo)
     if ui.verbose:
-        ui.note("* set:\n", revset.prettyformatset(revs), "\n")
+        ui.note(("* set:\n"), revset.prettyformatset(revs), "\n")
     for c in revs:
         ui.write("%s\n" % c)
 
@@ -3685,7 +3686,7 b' def debugtemplate(ui, repo, tmpl, **opts'
         ui.note(templater.prettyformat(tree), '\n')
         newtree = templater.expandaliases(tree, aliases)
         if newtree != tree:
-            ui.note("* expanded:\n", templater.prettyformat(newtree), '\n')
+            ui.note(("* expanded:\n"), templater.prettyformat(newtree), '\n')
 
     mapfile = None
     if revs is None:

tests/test-contrib-check-code.t

@@ -262,6 +262,20 b' web templates'
   >        'bar foo-'
   >        'bar')
   > EOF
+
+'missing _() in ui message' detection
+
+  $ cat > uigettext.py <<EOF
+  > ui.status("% 10s %05d % -3.2f %*s %%"
+  >           # this use '\\\\' instead of '\\', because the latter in
+  >           # heredoc on shell becomes just '\'
+  >           '\\\\ \n \t \0'
+  >           """12345
+  >           """
+  >           '''.:*+-=
+  >           ''' "%-6d \n 123456 .:*+-= foobar")
+  > EOF
+
   $ "$check_code" stringjoin.py
   stringjoin.py:1:
    > foo = (' foo'
@@ -288,3 +302,9 b' web templates'
    >        'bar foo-'
    string join across lines with no space
   [1]
+
+  $ "$check_code" uigettext.py
+  uigettext.py:1:
+   > ui.status("% 10s %05d % -3.2f %*s %%"
+   missing _() in ui message (use () to hide false-positives)
+  [1]