Data structs declared with constructor replaced with literal

Changes:
  - list() declarations replaced with []
  - dict() declarations replaced with {}
  - Added N351 to hacking for literal rule along with checks, test hacking, and to readme
  - Minor fix to incorrect path in hacking guidelines pointing to test_hacking.py

Change-Id: Ie7b9201d61b6f2024217b6f4117605eb52fa916a
Closes-Bug: #1437515
This commit is contained in:
Hugh Ma 2015-04-14 09:12:11 -07:00
parent b3b126dd77
commit 6a3f210375
5 changed files with 41 additions and 5 deletions

View File

@ -143,7 +143,7 @@ class TaskTestCase(unittest.TestCase):
rally = utils.Rally() rally = utils.Rally()
cfg = self._get_sample_task_config() cfg = self._get_sample_task_config()
config = utils.TaskConfig(cfg) config = utils.TaskConfig(cfg)
task_uuids = list() task_uuids = []
for i in range(3): for i in range(3):
res = rally("task start --task %s" % config.filename) res = rally("task start --task %s" % config.filename)
for line in res.splitlines(): for line in res.splitlines():
@ -158,7 +158,7 @@ class TaskTestCase(unittest.TestCase):
rally = utils.Rally() rally = utils.Rally()
cfg = self._get_sample_task_config() cfg = self._get_sample_task_config()
config = utils.TaskConfig(cfg) config = utils.TaskConfig(cfg)
files = list() files = []
for i in range(3): for i in range(3):
rally("task start --task %s" % config.filename) rally("task start --task %s" % config.filename)
path = "/tmp/task_%d.json" % i path = "/tmp/task_%d.json" % i
@ -241,7 +241,7 @@ class TaskTestCase(unittest.TestCase):
# Validate against a single task # Validate against a single task
res = rally("task start --task %s" % config.filename) res = rally("task start --task %s" % config.filename)
task_uuids = list() task_uuids = []
for line in res.splitlines(): for line in res.splitlines():
if "finished" in line: if "finished" in line:
task_uuids.append(line.split(" ")[1][:-1]) task_uuids.append(line.split(" ")[1][:-1])

View File

@ -97,7 +97,7 @@ class Rally(object):
self.reports_root = os.environ.get("REPORTS_ROOT", self.reports_root = os.environ.get("REPORTS_ROOT",
"rally-cli-output-files") "rally-cli-output-files")
self._created_files = list() self._created_files = []
self("deployment create --file %s --name MAIN" % DEPLOYMENT_FILE, self("deployment create --file %s --name MAIN" % DEPLOYMENT_FILE,
write_report=False) write_report=False)

View File

@ -24,3 +24,4 @@ Rally Specific Commandments
* [N340] - Ensure that we are importing always ``from rally import objects`` * [N340] - Ensure that we are importing always ``from rally import objects``
* [N341] - Ensure that we are importing oslo_xyz packages instead of deprecated oslo.xyz ones * [N341] - Ensure that we are importing oslo_xyz packages instead of deprecated oslo.xyz ones
* [N350] - Ensure that single quotes are not used * [N350] - Ensure that single quotes are not used
* [N351] - Ensure that data structs (i.e Lists and Dicts) are declared literally rather than using constructors

View File

@ -20,7 +20,7 @@ Guidelines for writing new hacking checks
- Keep the test method code in the source file ordered based - Keep the test method code in the source file ordered based
on the N3xx value. on the N3xx value.
- List the new rule in the top level HACKING.rst file - List the new rule in the top level HACKING.rst file
- Add test cases for each new rule to tests/test_hacking.py - Add test cases for each new rule to tests/unit/test_hacking.py
""" """
@ -46,6 +46,10 @@ re_assert_equal_in_end_with_true_or_false = re.compile(
r"assertEqual\((\w|[][.'\"])+( not)? in (\w|[][.'\", ])+, (True|False)\)") r"assertEqual\((\w|[][.'\"])+( not)? in (\w|[][.'\", ])+, (True|False)\)")
re_assert_equal_in_start_with_true_or_false = re.compile( re_assert_equal_in_start_with_true_or_false = re.compile(
r"assertEqual\((True|False), (\w|[][.'\"])+( not)? in (\w|[][.'\", ])+\)") r"assertEqual\((True|False), (\w|[][.'\"])+( not)? in (\w|[][.'\", ])+\)")
re_no_construct_dict = re.compile(
r"=\sdict\(\)")
re_no_construct_list = re.compile(
r"=\slist\(\)")
def skip_ignored_lines(func): def skip_ignored_lines(func):
@ -333,6 +337,21 @@ def check_quotes(logical_line, filename):
yield (i, "N350 Remove Single quotes") yield (i, "N350 Remove Single quotes")
@skip_ignored_lines
def check_no_constructor_data_struct(logical_line, filename):
"""Check that data structs (lists, dicts) are declared using literals
N351
"""
match = re_no_construct_dict.search(logical_line)
if match:
yield (0, "N351 Remove dict() construct and use literal {}")
match = re_no_construct_list.search(logical_line)
if match:
yield (0, "N351 Remove list() construct and use literal []")
def factory(register): def factory(register):
register(check_assert_methods_from_mock) register(check_assert_methods_from_mock)
register(check_import_of_logging) register(check_import_of_logging)
@ -346,3 +365,4 @@ def factory(register):
register(check_no_direct_rally_objects_import) register(check_no_direct_rally_objects_import)
register(check_no_oslo_deprecated_import) register(check_no_oslo_deprecated_import)
register(check_quotes) register(check_quotes)
register(check_no_constructor_data_struct)

View File

@ -245,3 +245,18 @@ class HackingTestCase(test.TestCase):
"a = '' # noqa " "a = '' # noqa "
] ]
self._assert_good_samples(checks.check_quotes, good_lines) self._assert_good_samples(checks.check_quotes, good_lines)
def test_check_no_constructor_data_struct(self):
bad_struct = [
"= dict()",
"= list()"
]
self._assert_bad_samples(checks.check_no_constructor_data_struct,
bad_struct)
good_struct = [
"= []",
"= {}",
]
self._assert_good_samples(checks.check_no_constructor_data_struct,
good_struct)