Hacking rule to check i18n usage

* Detect neutron.i18n import (neutron._i18n is recommended)
* Check builtins _ usage
* 'builtins = _' in tox.ini is no longer required.
* Introduce hacking rule doctest framework.
  Newly added check_builtins_gettext() hacking check takes
  token as argument. It is not a good idea to pass a tokenized
  line manually. Instead it is reasonable to use docstring based
  tests used in hacking repo.

Change-Id: Ib7464658fc4c8a6f1b03af6ab46f0bd3ee0bfb18

HACKING.rst

@@ -25,6 +25,8 @@ Neutron Specific Commandments
          assertEqual(observed_http_code, expected_http_code).
 - [N333] Validate that LOG.warning is used instead of LOG.warn. The latter
   is deprecated.
+- [N340] Check usage of <module>.i18n (and neutron.i18n)
+- [N341] Check usage of _ from python builtins
 
 Creating Unit Tests
 -------------------

neutron/__init__.py

@@ -25,6 +25,7 @@
     gettext.install('neutron')
 
 
+# flake8: noqa
 six.moves.builtins.__dict__['_'] = removals.remove(
     message='Builtin _ translation function is deprecated in OpenStack; '
             'use the function from _i18n module for your project.')(_)

neutron/agent/l3/dvr_local_router.py

@@ -71,7 +71,7 @@ def floating_ip_added_dist(self, fip, fip_cidr):
         if self.rtr_fip_subnet is None:
             self.rtr_fip_subnet = self.fip_ns.local_subnets.allocate(
                 self.router_id)
-        rtr_2_fip, _ = self.rtr_fip_subnet.get_pair()
+        rtr_2_fip, __ = self.rtr_fip_subnet.get_pair()
         device = ip_lib.IPDevice(fip_2_rtr_name, namespace=fip_ns_name)
         device.route.add_route(fip_cidr, str(rtr_2_fip.ip))
         interface_name = (

neutron/db/bgp_db.py

@@ -25,6 +25,7 @@
 
 from neutron_lib import constants as lib_consts
 
+from neutron._i18n import _
 from neutron.api.v2 import attributes as attr
 from neutron.common import exceptions as n_exc
 from neutron.db import address_scope_db

neutron/db/external_net_db.py

@@ -19,6 +19,7 @@
 from sqlalchemy import sql
 from sqlalchemy.sql import expression as expr
 
+from neutron._i18n import _
 from neutron.api.v2 import attributes
 from neutron.callbacks import events
 from neutron.callbacks import exceptions as c_exc

neutron/db/sqlalchemytypes.py

@@ -15,6 +15,8 @@
 import netaddr
 from sqlalchemy import types
 
+from neutron._i18n import _
+
 
 class IPAddress(types.TypeDecorator):
 

neutron/hacking/checks.py

@@ -12,11 +12,23 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import os
 import re
 
 import pep8
 import six
 
+
+def flake8ext(f):
+    """Decorator to indicate flake8 extension.
+
+    This is borrowed from hacking.core.flake8ext(), but at now it is used
+    only for unit tests to know which are neutron flake8 extensions.
+    """
+    f.name = __name__
+    return f
+
+
 # Guidelines for writing new hacking checks
 #
 #  - Use only for Neutron specific tests. OpenStack general tests
@@ -59,6 +71,7 @@ def _regex_for_level(level, hint):
 contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
 
 
+@flake8ext
 def validate_log_translations(logical_line, physical_line, filename):
     # Translations are not required in the test directory
     if "neutron/tests" in filename:
@@ -71,6 +84,7 @@ def validate_log_translations(logical_line, physical_line, filename):
         yield (0, msg)
 
 
+@flake8ext
 def use_jsonutils(logical_line, filename):
     msg = "N321: jsonutils.%(fun)s must be used instead of json.%(fun)s"
 
@@ -93,6 +107,7 @@ def use_jsonutils(logical_line, filename):
                 yield (pos, msg % {'fun': f[:-1]})
 
 
+@flake8ext
 def no_translate_debug_logs(logical_line, filename):
     """Check for 'LOG.debug(_(' and 'LOG.debug(_Lx('
 
@@ -108,6 +123,7 @@ def no_translate_debug_logs(logical_line, filename):
             yield(0, "N319 Don't translate debug level logs")
 
 
+@flake8ext
 def check_assert_called_once_with(logical_line, filename):
     # Try to detect unintended calls of nonexistent mock methods like:
     #    assert_called_once
@@ -131,6 +147,7 @@ def check_assert_called_once_with(logical_line, filename):
             yield (0, msg)
 
 
+@flake8ext
 def check_no_contextlib_nested(logical_line, filename):
     msg = ("N324: contextlib.nested is deprecated. With Python 2.7 and later "
            "the with-statement supports multiple nested objects. See https://"
@@ -141,25 +158,29 @@ def check_no_contextlib_nested(logical_line, filename):
         yield(0, msg)
 
 
+@flake8ext
 def check_python3_xrange(logical_line):
     if re.search(r"\bxrange\s*\(", logical_line):
         yield(0, "N325: Do not use xrange. Use range, or six.moves.range for "
                  "large loops.")
 
 
+@flake8ext
 def check_no_basestring(logical_line):
     if re.search(r"\bbasestring\b", logical_line):
         msg = ("N326: basestring is not Python3-compatible, use "
                "six.string_types instead.")
         yield(0, msg)
 
 
+@flake8ext
 def check_python3_no_iteritems(logical_line):
     if re.search(r".*\.iteritems\(\)", logical_line):
         msg = ("N327: Use six.iteritems() instead of dict.iteritems().")
         yield(0, msg)
 
 
+@flake8ext
 def check_asserttrue(logical_line, filename):
     if 'neutron/tests/' in filename:
         if re.search(r"assertEqual\(\s*True,[^,]*(,[^,]*)?\)", logical_line):
@@ -172,12 +193,14 @@ def check_asserttrue(logical_line, filename):
             yield (0, msg)
 
 
+@flake8ext
 def no_mutable_default_args(logical_line):
     msg = "N329: Method's default argument shouldn't be mutable!"
     if mutable_default_args.match(logical_line):
         yield (0, msg)
 
 
+@flake8ext
 def check_assertfalse(logical_line, filename):
     if 'neutron/tests/' in filename:
         if re.search(r"assertEqual\(\s*False,[^,]*(,[^,]*)?\)", logical_line):
@@ -190,6 +213,7 @@ def check_assertfalse(logical_line, filename):
             yield (0, msg)
 
 
+@flake8ext
 def check_assertempty(logical_line, filename):
     if 'neutron/tests/' in filename:
         msg = ("N330: Use assertEqual(*empty*, observed) instead of "
@@ -201,6 +225,7 @@ def check_assertempty(logical_line, filename):
             yield (0, msg)
 
 
+@flake8ext
 def check_assertisinstance(logical_line, filename):
     if 'neutron/tests/' in filename:
         if re.search(r"assertTrue\(\s*isinstance\(\s*[^,]*,\s*[^,]*\)\)",
@@ -210,6 +235,7 @@ def check_assertisinstance(logical_line, filename):
             yield (0, msg)
 
 
+@flake8ext
 def check_assertequal_for_httpcode(logical_line, filename):
     msg = ("N332: Use assertEqual(expected_http_code, observed_http_code) "
            "instead of assertEqual(observed_http_code, expected_http_code)")
@@ -219,12 +245,82 @@ def check_assertequal_for_httpcode(logical_line, filename):
             yield (0, msg)
 
 
+@flake8ext
 def check_log_warn_deprecated(logical_line, filename):
     msg = "N333: Use LOG.warning due to compatibility with py3"
     if log_warn.match(logical_line):
         yield (0, msg)
 
 
+@flake8ext
+def check_oslo_i18n_wrapper(logical_line, filename, noqa):
+    """Check for neutron.i18n usage.
+
+    Okay(neutron/foo/bar.py): from neutron._i18n import _
+    Okay(neutron_lbaas/foo/bar.py): from neutron_lbaas._i18n import _
+    N340(neutron/foo/bar.py): from neutron.i18n import _
+    N340(neutron_lbaas/foo/bar.py): from neutron_lbaas.i18n import _
+    N340(neutron_lbaas/foo/bar.py): from neutron.i18n import _
+    N340(neutron_lbaas/foo/bar.py): from neutron._i18n import _
+    Okay(neutron/foo/bar.py): from neutron.i18n import _  # noqa
+    """
+
+    if noqa:
+        return
+
+    split_line = logical_line.split()
+    modulename = os.path.normpath(filename).split('/')[0]
+    bad_i18n_module = '%s.i18n' % modulename
+
+    if (len(split_line) > 1 and split_line[0] in ('import', 'from')):
+        if (split_line[1] == bad_i18n_module or
+            modulename != 'neutron' and split_line[1] in ('neutron.i18n',
+                                                          'neutron._i18n')):
+            msg = ("N340: %(found)s is found. Use %(module)s._i18n instead."
+                   % {'found': split_line[1], 'module': modulename})
+            yield (0, msg)
+
+
+@flake8ext
+def check_builtins_gettext(logical_line, tokens, filename, lines, noqa):
+    """Check usage of builtins gettext _().
+
+    Okay(neutron/foo.py): from neutron._i18n import _\n_('foo')
+    N341(neutron/foo.py): _('foo')
+    Okay(neutron/_i18n.py): _('foo')
+    Okay(neutron/i18n.py): _('foo')
+    Okay(neutron/foo.py): _('foo')  # noqa
+    """
+
+    if noqa:
+        return
+
+    modulename = os.path.normpath(filename).split('/')[0]
+
+    if '%s/tests' % modulename in filename:
+        return
+
+    if os.path.basename(filename) in ('i18n.py', '_i18n.py'):
+        return
+
+    token_values = [t[1] for t in tokens]
+    i18n_wrapper = '%s._i18n' % modulename
+
+    if '_' in token_values:
+        i18n_import_line_found = False
+        for line in lines:
+            split_line = [elm.rstrip(',') for elm in line.split()]
+            if (len(split_line) > 1 and split_line[0] == 'from' and
+                    split_line[1] == i18n_wrapper and
+                    '_' in split_line):
+                i18n_import_line_found = True
+                break
+        if not i18n_import_line_found:
+            msg = ("N341: _ from python builtins module is used. "
+                   "Use _ from %s instead." % i18n_wrapper)
+            yield (0, msg)
+
+
 def factory(register):
     register(validate_log_translations)
     register(use_jsonutils)
@@ -241,3 +337,5 @@ def factory(register):
     register(check_assertisinstance)
     register(check_assertequal_for_httpcode)
     register(check_log_warn_deprecated)
+    register(check_oslo_i18n_wrapper)
+    register(check_builtins_gettext)

neutron/pecan_wsgi/controllers/extensions.py

@@ -14,6 +14,7 @@
 
 import pecan
 
+from neutron._i18n import _
 from neutron.api import extensions
 from neutron.pecan_wsgi.controllers import utils
 

neutron/pecan_wsgi/controllers/resource.py

@@ -16,8 +16,8 @@
 import pecan
 from pecan import request
 
+from neutron._i18n import _LW
 from neutron.api import api_common
-from neutron.i18n import _LW
 from neutron import manager
 from neutron.pecan_wsgi.controllers import utils
 

neutron/plugins/ml2/drivers/agent/config.py

@@ -16,6 +16,7 @@
 
 from oslo_config import cfg
 
+from neutron._i18n import _
 from neutron.agent.common import config
 
 agent_opts = [

neutron/plugins/ml2/drivers/l2pop/mech_driver.py

@@ -185,7 +185,7 @@ def _create_agent_fdb(self, session, agent, segment, network_id):
 
     def _get_tunnels(self, tunnel_network_ports, exclude_host):
         agents = {}
-        for _, agent in tunnel_network_ports:
+        for __, agent in tunnel_network_ports:
             if agent.host == exclude_host:
                 continue
 

neutron/plugins/ml2/drivers/macvtap/agent/config.py

@@ -16,6 +16,8 @@
 
 from oslo_config import cfg
 
+from neutron._i18n import _
+
 DEFAULT_INTERFACE_MAPPINGS = []
 
 macvtap_opts = [