From 440072339988d8a5791e621b0a3efffef3754a5c Mon Sep 17 00:00:00 2001
From: "Ivan A. Melnikov" <imelnikov@griddynamics.com>
Date: Fri, 22 Nov 2013 15:12:27 +0400
Subject: [PATCH] Improve is_valid_attribute_name utility function

- don't allow python keywords;
- use ascii symbols only, no national letters or digits;
- code cleanup.

Change-Id: Ibf863edb6fd06257df05ca3c490f87f1c097144f
---
 taskflow/tests/unit/test_utils.py | 31 +++++++++++++++++++
 taskflow/utils/misc.py            | 50 +++++++++++++------------------
 2 files changed, 52 insertions(+), 29 deletions(-)

diff --git a/taskflow/tests/unit/test_utils.py b/taskflow/tests/unit/test_utils.py
index 82708e3f..97287b76 100644
--- a/taskflow/tests/unit/test_utils.py
+++ b/taskflow/tests/unit/test_utils.py
@@ -260,6 +260,37 @@ class AttrDictTest(test.TestCase):
         self.assertEqual('c', obj['_b'])
 
 
+class IsValidAttributeNameTestCase(test.TestCase):
+    def test_a_is_ok(self):
+        self.assertTrue(misc.is_valid_attribute_name('a'))
+
+    def test_name_can_be_longer(self):
+        self.assertTrue(misc.is_valid_attribute_name('foobarbaz'))
+
+    def test_name_can_have_digits(self):
+        self.assertTrue(misc.is_valid_attribute_name('fo12'))
+
+    def test_name_cannot_start_with_digit(self):
+        self.assertFalse(misc.is_valid_attribute_name('1z'))
+
+    def test_hidden_names_are_forbidden(self):
+        self.assertFalse(misc.is_valid_attribute_name('_z'))
+
+    def test_hidden_names_can_be_allowed(self):
+        self.assertTrue(
+            misc.is_valid_attribute_name('_z', allow_hidden=True))
+
+    def test_self_is_forbidden(self):
+        self.assertFalse(misc.is_valid_attribute_name('self'))
+
+    def test_self_can_be_allowed(self):
+        self.assertTrue(
+            misc.is_valid_attribute_name('self', allow_self=True))
+
+    def test_no_unicode_please(self):
+        self.assertFalse(misc.is_valid_attribute_name('mañana'))
+
+
 class ExcInfoUtilsTest(test.TestCase):
 
     def _make_ex_info(self):
diff --git a/taskflow/utils/misc.py b/taskflow/utils/misc.py
index ea046daa..5b5c9abe 100644
--- a/taskflow/utils/misc.py
+++ b/taskflow/utils/misc.py
@@ -21,9 +21,11 @@ import collections
 import copy
 import errno
 import functools
+import keyword
 import logging
 import os
 import six
+import string
 import sys
 import traceback
 
@@ -32,7 +34,7 @@ from taskflow.utils import reflection
 
 
 LOG = logging.getLogger(__name__)
-NUMERIC_TYPES = tuple(list(six.integer_types) + [float])
+NUMERIC_TYPES = six.integer_types + (float,)
 
 
 def wraps(fn):
@@ -76,36 +78,26 @@ def get_duplicate_keys(iterable, key=None):
     return duplicates
 
 
+# NOTE(imelnikov): we should not use str.isalpha or str.isdigit
+# as they are locale-dependant
+_ASCII_WORD_SYMBOLS = frozenset(string.ascii_letters + string.digits + '_')
+
+
 def is_valid_attribute_name(name, allow_self=False, allow_hidden=False):
     """Validates that a string name is a valid/invalid python attribute name"""
-    if not isinstance(name, six.string_types) or len(name) == 0:
-        return False
-    # Make the name just be a simple string in latin-1 encoding in python3
-    name = six.b(name)
-    if not allow_self and name.lower().startswith(six.b('self')):
-        return False
-    if not allow_hidden and name.startswith(six.b("_")):
-        return False
-    # See: http://docs.python.org/release/2.5.2/ref/grammar.txt (or newer)
-    #
-    # Python identifiers should start with a letter.
-    if isinstance(name[0], six.integer_types):
-        if not chr(name[0]).isalpha():
-            return False
-    else:
-        if not name[0].isalpha():
-            return False
-    for i in range(1, len(name)):
-        symbol = name[i]
-        # The rest of an attribute name follows: (letter | digit | "_")*
-        if isinstance(symbol, six.integer_types):
-            symbol = chr(symbol)
-            if not (symbol.isalpha() or symbol.isdigit() or symbol == "_"):
-                return False
-        else:
-            if not (symbol.isalpha() or symbol.isdigit() or symbol == "_"):
-                return False
-    return True
+    return all((
+        isinstance(name, six.string_types),
+        len(name) > 0,
+        (allow_self or not name.lower().startswith('self')),
+        (allow_hidden or not name.lower().startswith('_')),
+
+        # NOTE(imelnikov): keywords should be forbidden
+        not keyword.iskeyword(name),
+
+        # See: http://docs.python.org/release/2.5.2/ref/grammar.txt
+        not (name[0] in string.digits),
+        all(symbol in _ASCII_WORD_SYMBOLS for symbol in name)
+    ))
 
 
 class AttrDict(dict):