From ff13d58c1443aeb3a59e224f73ab82b52230c6ff Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 8 Sep 2017 16:43:30 -0700 Subject: [PATCH] Remove use of unicode_literals as it is an anti-pattern from __future__ unicode_literals leads to developer confusion as developers no longer can tell whether a bare literal string is a byte string or a unicode string. Explicit marking as u"" or b"" is the way to solve the same problem in the Ansbile codebase. --- .../testing/sanity/no-unicode-literals.rst | 16 ++++++++ lib/ansible/plugins/lookup/mongodb.py | 37 +++++++++---------- test/sanity/code-smell/no-unicode-literals.sh | 16 ++++++++ 3 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 docs/docsite/rst/dev_guide/testing/sanity/no-unicode-literals.rst create mode 100755 test/sanity/code-smell/no-unicode-literals.sh diff --git a/docs/docsite/rst/dev_guide/testing/sanity/no-unicode-literals.rst b/docs/docsite/rst/dev_guide/testing/sanity/no-unicode-literals.rst new file mode 100644 index 0000000000..fefa72c307 --- /dev/null +++ b/docs/docsite/rst/dev_guide/testing/sanity/no-unicode-literals.rst @@ -0,0 +1,16 @@ +Sanity Tests ยป no-unicode_literals +================================== + +The use of :code:`from __future__ import unicode_literals` has been deemed an anti-pattern. The +problems with it are: + +* It makes it so one can't jump into the middle of a file and know whether a bare literal string is + a byte string or text string. The programmer has to first check the top of the file to see if the + import is there. +* It removes the ability to define native strings (a string which should be a byte string on python2 + and a text string on python3) via a string literal. +* It makes for more context switching. A programmer could be reading one file which has + `unicode_literals` and know that bare string literals are text strings but then switch to another + file (perhaps tracing program execution into a third party library) and have to switch their + understanding of what bare string literals are. + diff --git a/lib/ansible/plugins/lookup/mongodb.py b/lib/ansible/plugins/lookup/mongodb.py index dff4a7bbec..6f410aeb53 100644 --- a/lib/ansible/plugins/lookup/mongodb.py +++ b/lib/ansible/plugins/lookup/mongodb.py @@ -88,7 +88,6 @@ EXAMPLES: ''' from __future__ import (absolute_import, division, print_function) -from __future__ import unicode_literals from ansible.module_utils.six import string_types, integer_types import datetime @@ -120,7 +119,7 @@ class LookupModule(LookupBase): return sort_parameter if not isinstance(sort_parameter, list): - raise AnsibleError("Error. Sort parameters must be a list, not [ {} ]".format(sort_parameter)) + raise AnsibleError(u"Error. Sort parameters must be a list, not [ {} ]".format(sort_parameter)) for item in sort_parameter: self._convert_sort_string_to_constant(item) @@ -130,9 +129,9 @@ class LookupModule(LookupBase): def _convert_sort_string_to_constant(self, item): original_sort_order = item[1] sort_order = original_sort_order.upper() - if sort_order == "ASCENDING": + if sort_order == u"ASCENDING": item[1] = ASCENDING - elif sort_order == "DESCENDING": + elif sort_order == u"DESCENDING": item[1] = DESCENDING # else the user knows what s/he is doing and we won't predict. PyMongo will return an error if necessary @@ -159,13 +158,13 @@ class LookupModule(LookupBase): return (result - datetime.datetime(1970, 1, 1)). total_seconds() else: # failsafe - return "{}".format(result) + return u"{}".format(result) def run(self, terms, variables, **kwargs): ret = [] for term in terms: - ''' + u''' Makes a MongoDB query and returns the output as a valid list of json. Timestamps are converted to epoch integers/longs. @@ -206,20 +205,20 @@ class LookupModule(LookupBase): ------------------------------------------------------------------------------- ''' - connection_string = term.get('connection_string', "mongodb://localhost") - database = term["database"] - collection = term['collection'] - extra_connection_parameters = term.get('extra_connection_parameters', {}) + connection_string = term.get(u'connection_string', u"mongodb://localhost") + database = term[u"database"] + collection = term[u'collection'] + extra_connection_parameters = term.get(u'extra_connection_parameters', {}) - if "extra_connection_parameters" in term: - del term["extra_connection_parameters"] - if "connection_string" in term: - del term["connection_string"] - del term["database"] - del term["collection"] + if u"extra_connection_parameters" in term: + del term[u"extra_connection_parameters"] + if u"connection_string" in term: + del term[u"connection_string"] + del term[u"database"] + del term[u"collection"] - if "sort" in term: - term["sort"] = self._fix_sort_parameter(term["sort"]) + if u"sort" in term: + term[u"sort"] = self._fix_sort_parameter(term[u"sort"]) # all other parameters are sent to mongo, so we are future and past proof @@ -232,6 +231,6 @@ class LookupModule(LookupBase): ret.append(result) except ConnectionFailure as e: - raise AnsibleError('unable to connect to database: %s' % str(e)) + raise AnsibleError(u'unable to connect to database: %s' % str(e)) return ret diff --git a/test/sanity/code-smell/no-unicode-literals.sh b/test/sanity/code-smell/no-unicode-literals.sh new file mode 100755 index 0000000000..b474a99564 --- /dev/null +++ b/test/sanity/code-smell/no-unicode-literals.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +UNICODE_LITERALS_USERS=$(grep -r unicode_literals . \ + --exclude-dir .git \ + --exclude-dir .tox \ + --exclude no-unicode-literals.sh \ + --exclude no-unicode-literals.rst | + grep -v ./test/results \ + ) + +if [ "${UNICODE_LITERALS_USERS}" ]; then + echo "${UNICODE_LITERALS_USERS}" + exit 1 +fi + +exit 0