From b5dea7eb204765c55c5d49c3cad7ec1c8555cd58 Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 09:55:05 +0100 Subject: [PATCH 1/7] Added test for rule syntax --- tests/test_rules.py | 110 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/test_rules.py diff --git a/tests/test_rules.py b/tests/test_rules.py new file mode 100644 index 000000000..5505447b0 --- /dev/null +++ b/tests/test_rules.py @@ -0,0 +1,110 @@ +import re +import glob +import unittest +import yaml + +PROTO_FILES = glob.glob("osi_environment.proto") + +class TestRules(unittest.TestCase): + """ Test class for units documentation. """ + + def test_rules_compliance(self): + ''' Test rule compliance syntax of proto files. ''' + + lineruleCount = 0 + foundruleCount = 0 + + with open(r'rules.yml') as rules_file: + RULES_DICT = yaml.load(rules_file, Loader=yaml.FullLoader) + + for file in PROTO_FILES: + with open(file, "rt") as fin, self.subTest(file=file): + line_number = 0 + noMessage = 0 + saveStatement = "" + + for line in fin: + line_number += 1 + + # Divide statement and comment. Concatenate multi line statements. + # Search for comment ("//"). + matchComment = re.search("//", line) + if matchComment is not None: + statement = line[:matchComment.start()] + comment = line[matchComment.end():] + else: + statement = line + comment = "" + + # Add part of the statement from last line. + statement = saveStatement + " " + statement + saveStatement = "" + + # New line is not necessary. Remove for a better output. + statement = statement.replace("\n", "") + comment = comment.replace("\n", "") + + # Is statement complete + matchSep = re.search(r"[{};]", statement) + if matchSep is None: + saveStatement = statement + statement = "" + else: + saveStatement = statement[matchSep.end():] + statement = statement[:matchSep.end()] + + # Search for "message". + matchMessage = re.search(r"\bmessage\b", statement) + if matchMessage is not None: + # a new message or a new nested message + noMessage += 1 + endOfLine = statement[matchMessage.end():] + matchName = re.search(r"\b\w[\S]*\b", endOfLine) + + elif re.search(r"\bextend\b", statement) is not None: + # treat extend as message + noMessage += 1 + else: + # Check field names + if noMessage > 0: + matchName = re.search(r"\b\w[\S]*\b\s*=", statement) + if matchName is not None: + checkName = statement[matchName.start():matchName.end()-1] + # Check field message type (remove field name) + type = statement.replace(checkName, "") + matchName = re.search(r"\b\w[\S\.]*\s*=", type) + + # Search for a closing brace. + matchClosingBrace = re.search("}", statement) + if noMessage > 0 and matchClosingBrace is not None: + noMessage -= 1 + + if matchComment is not None: + if comment.find("\\endrules") != -1: + endRule = True + + if comment.find("\\rules") != -1: + hasRule = True + lineruleCount = -1 + foundruleCount = -1 + + if not endRule and comment != '': + for rulename, ruleregex in RULES_DICT.items(): + if re.search(ruleregex, comment): + foundruleCount += 1 + + elif len(saveStatement) == 0: + if noMessage > 0: + if statement.find(";") != -1: + statement = statement.strip() + self.assertFalse(hasRule and lineruleCount != foundruleCount and endRule and lineruleCount-foundruleCount-1>0, file + " in line " + str(line_number) + ": "+str(lineruleCount-foundruleCount-1)+" defined rule(s) does not exists for: '"+statement+"'") + self.assertFalse(hasRule and lineruleCount > foundruleCount and not endRule, file + " in line " + str(line_number) + ": endrules statement does not exists for: '"+statement+"'") + + hasRule = False + endRule = False + + if hasRule and not endRule: + lineruleCount += 1 + +if __name__ == '__main__': + unittest.main() \ No newline at end of file From f6f8ec3c442eac91410a40a170cee4bdb1dd7805 Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 11:13:59 +0100 Subject: [PATCH 2/7] Added not enclosed rule --- tests/test_rules.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/tests/test_rules.py b/tests/test_rules.py index 5505447b0..73bdd5731 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -11,17 +11,18 @@ class TestRules(unittest.TestCase): def test_rules_compliance(self): ''' Test rule compliance syntax of proto files. ''' - lineruleCount = 0 - foundruleCount = 0 - with open(r'rules.yml') as rules_file: RULES_DICT = yaml.load(rules_file, Loader=yaml.FullLoader) for file in PROTO_FILES: with open(file, "rt") as fin, self.subTest(file=file): line_number = 0 - noMessage = 0 + numMessage = 0 + lineruleCount = 0 + foundruleCount = 0 saveStatement = "" + hasRule = False + endRule = False for line in fin: line_number += 1 @@ -57,16 +58,16 @@ def test_rules_compliance(self): matchMessage = re.search(r"\bmessage\b", statement) if matchMessage is not None: # a new message or a new nested message - noMessage += 1 + numMessage += 1 endOfLine = statement[matchMessage.end():] matchName = re.search(r"\b\w[\S]*\b", endOfLine) elif re.search(r"\bextend\b", statement) is not None: # treat extend as message - noMessage += 1 + numMessage += 1 else: # Check field names - if noMessage > 0: + if numMessage > 0: matchName = re.search(r"\b\w[\S]*\b\s*=", statement) if matchName is not None: checkName = statement[matchName.start():matchName.end()-1] @@ -76,8 +77,8 @@ def test_rules_compliance(self): # Search for a closing brace. matchClosingBrace = re.search("}", statement) - if noMessage > 0 and matchClosingBrace is not None: - noMessage -= 1 + if numMessage > 0 and matchClosingBrace is not None: + numMessage -= 1 if matchComment is not None: if comment.find("\\endrules") != -1: @@ -94,16 +95,22 @@ def test_rules_compliance(self): foundruleCount += 1 elif len(saveStatement) == 0: - if noMessage > 0: + if numMessage > 0: if statement.find(";") != -1: statement = statement.strip() self.assertFalse(hasRule and lineruleCount != foundruleCount and endRule and lineruleCount-foundruleCount-1>0, file + " in line " + str(line_number) + ": "+str(lineruleCount-foundruleCount-1)+" defined rule(s) does not exists for: '"+statement+"'") - self.assertFalse(hasRule and lineruleCount > foundruleCount and not endRule, file + " in line " + str(line_number) + ": endrules statement does not exists for: '"+statement+"'") - + self.assertFalse(hasRule and lineruleCount > foundruleCount and not endRule, file + " in line " + str(line_number) + ": \\endrules statement does not exists for: '"+statement+"'") + self.assertFalse(not hasRule and lineruleCount < foundruleCount and endRule, file + " in line " + str(line_number) + ": \\rules statement does not exists for: '"+statement+"'") + self.assertFalse(not hasRule and not endRule and lineruleCount < foundruleCount, file + " in line " + str(line_number) + ": rules found but no statements (\\rules and \\endrules) around it for: '"+statement+"'") + + # elif numMessage == 0 and hasRule: + # statement = statement.strip() + # self.assertFalse(hasRule and lineruleCount != foundruleCount and endRule and lineruleCount-foundruleCount-1>0, file + " in line " + str(line_number) + ": "+str(lineruleCount-foundruleCount-1)+" defined rule(s) does not exists for: '"+statement+"'") + hasRule = False endRule = False - if hasRule and not endRule: + if hasRule and not endRule or not hasRule and endRule: lineruleCount += 1 if __name__ == '__main__': From 5c2d5ec4b501ad624a6b179ea7203f26a237580c Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 12:20:13 +0100 Subject: [PATCH 3/7] Added test no rules for message --- tests/test_rules.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_rules.py b/tests/test_rules.py index 73bdd5731..1cdcad1e2 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -59,6 +59,7 @@ def test_rules_compliance(self): if matchMessage is not None: # a new message or a new nested message numMessage += 1 + self.assertFalse(foundruleCount > 0, file + f" in line {str(line_number-1)}: message should not have rules for '{statement.strip()}'") endOfLine = statement[matchMessage.end():] matchName = re.search(r"\b\w[\S]*\b", endOfLine) @@ -99,9 +100,11 @@ def test_rules_compliance(self): if statement.find(";") != -1: statement = statement.strip() self.assertFalse(hasRule and lineruleCount != foundruleCount and endRule and lineruleCount-foundruleCount-1>0, file + " in line " + str(line_number) + ": "+str(lineruleCount-foundruleCount-1)+" defined rule(s) does not exists for: '"+statement+"'") - self.assertFalse(hasRule and lineruleCount > foundruleCount and not endRule, file + " in line " + str(line_number) + ": \\endrules statement does not exists for: '"+statement+"'") - self.assertFalse(not hasRule and lineruleCount < foundruleCount and endRule, file + " in line " + str(line_number) + ": \\rules statement does not exists for: '"+statement+"'") + self.assertFalse(hasRule and not endRule, file + " in line " + str(line_number) + ": \\endrules statement does not exists for: '"+statement+"'") + self.assertFalse(not hasRule and endRule, file + " in line " + str(line_number) + ": \\rules statement does not exists for: '"+statement+"'") self.assertFalse(not hasRule and not endRule and lineruleCount < foundruleCount, file + " in line " + str(line_number) + ": rules found but no statements (\\rules and \\endrules) around it for: '"+statement+"'") + lineruleCount = -1 + foundruleCount = -1 # elif numMessage == 0 and hasRule: # statement = statement.strip() From 8fb28cbe512a6b4c399cf1148cbbed2bd9e9585f Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 16:39:22 +0100 Subject: [PATCH 4/7] Added test no rules for enums --- tests/test_rules.py | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/test_rules.py b/tests/test_rules.py index 1cdcad1e2..590bc0201 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -3,7 +3,7 @@ import unittest import yaml -PROTO_FILES = glob.glob("osi_environment.proto") +PROTO_FILES = glob.glob("*.proto") class TestRules(unittest.TestCase): """ Test class for units documentation. """ @@ -21,6 +21,7 @@ def test_rules_compliance(self): lineruleCount = 0 foundruleCount = 0 saveStatement = "" + isEnum = False hasRule = False endRule = False @@ -59,7 +60,7 @@ def test_rules_compliance(self): if matchMessage is not None: # a new message or a new nested message numMessage += 1 - self.assertFalse(foundruleCount > 0, file + f" in line {str(line_number-1)}: message should not have rules for '{statement.strip()}'") + self.assertFalse(foundruleCount > 0 or lineruleCount > 0, file + f" in line {str(line_number-1)}: message should not have rules for '{statement.strip()}'") endOfLine = statement[matchMessage.end():] matchName = re.search(r"\b\w[\S]*\b", endOfLine) @@ -76,19 +77,38 @@ def test_rules_compliance(self): type = statement.replace(checkName, "") matchName = re.search(r"\b\w[\S\.]*\s*=", type) + if isEnum: + matchName = re.search(r"\b\w[\S:]+\b", statement) + if matchName is not None: + checkName = statement[matchName.start():matchName.end()] + self.assertFalse(foundruleCount > 0 or lineruleCount > 0, file + f" in line {str(line_number-1)}: enum field should not have rules for '{statement.strip()}'") + + # Search for "enum". + matchEnum = re.search(r"\benum\b", statement) + if matchEnum is not None: + isEnum = True + endOfLine = statement[matchEnum.end():] + matchName = re.search(r"\b\w[\S]*\b", endOfLine) + if matchName is not None: + self.assertFalse(foundruleCount > 0 or lineruleCount > 0, file + f" in line {str(line_number-1)}: enum should not have rules for '{statement.strip()}'") + # Search for a closing brace. matchClosingBrace = re.search("}", statement) if numMessage > 0 and matchClosingBrace is not None: numMessage -= 1 + if isEnum is True and matchClosingBrace is not None: + isEnum = False + enumName = "" + if matchComment is not None: if comment.find("\\endrules") != -1: endRule = True if comment.find("\\rules") != -1: hasRule = True - lineruleCount = -1 - foundruleCount = -1 + lineruleCount = 0 + foundruleCount = 0 if not endRule and comment != '': for rulename, ruleregex in RULES_DICT.items(): @@ -103,8 +123,8 @@ def test_rules_compliance(self): self.assertFalse(hasRule and not endRule, file + " in line " + str(line_number) + ": \\endrules statement does not exists for: '"+statement+"'") self.assertFalse(not hasRule and endRule, file + " in line " + str(line_number) + ": \\rules statement does not exists for: '"+statement+"'") self.assertFalse(not hasRule and not endRule and lineruleCount < foundruleCount, file + " in line " + str(line_number) + ": rules found but no statements (\\rules and \\endrules) around it for: '"+statement+"'") - lineruleCount = -1 - foundruleCount = -1 + lineruleCount = 0 + foundruleCount = 0 # elif numMessage == 0 and hasRule: # statement = statement.strip() From e9b930186a7836a610bc7359e403d0a5af3392d5 Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 17:17:52 +0100 Subject: [PATCH 5/7] Update rule search syntax --- rules.yml | 24 ++++++++++++------------ tests/test_rules.py | 14 ++++---------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/rules.yml b/rules.yml index 99c8ff5be..473bf51b2 100644 --- a/rules.yml +++ b/rules.yml @@ -1,12 +1,12 @@ -is_greater_than: '\b(is_greater_than)\b: ' -is_greater_than_or_equal_to: '\b(is_greater_than_or_equal_to)\b: ' -is_less_than_or_equal_to: '\b(is_less_than_or_equal_to)\b: ' -is_less_than: '\b(is_less_than)\b: ' -is_equal_to: '\b(is_equal_to)\b: ' -is_different_to: '\b(is_different_to)\b: ' -is_globally_unique: '\b(is_globally_unique)\b' -refers_to: '\b(refers_to)\b' -is_iso_country_code: '\b(is_iso_country_code)\b' -first_element: '\b(first_element)\b' -last_element: '\b(last_element)\b' -check_if: '(\bcheck_if\b)(.*\belse do_check\b)' \ No newline at end of file +is_greater_than: '^[ ]\b(is_greater_than)\b: ([\s\d]+)$' +is_greater_than_or_equal_to: '^[ ]\b(is_greater_than_or_equal_to)\b: ([\s\d]+)$' +is_less_than_or_equal_to: '^[ ]\b(is_less_than_or_equal_to)\b: ([\s\d]+)$' +is_less_than: '^[ ]\b(is_less_than)\b: ([\s\d]+)$' +is_equal_to: '^[ ]\b(is_equal_to)\b: ([\s\d]+)$' +is_different_to: '^[ ]\b(is_different_to)\b: ([\s\d]+)$' +is_globally_unique: '^[ ]\b(is_globally_unique)\b' +refers_to: '^[ ]\b(refers_to)\b' +is_iso_country_code: '^[ ]\b(is_iso_country_code)\b' +first_element: '^[ ]\b(first_element)\b' +last_element: '^[ ]\b(last_element)\b' +check_if: '^[ ](\bcheck_if\b)(.*\belse do_check\b)' \ No newline at end of file diff --git a/tests/test_rules.py b/tests/test_rules.py index 590bc0201..1ddde908a 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -22,8 +22,6 @@ def test_rules_compliance(self): foundruleCount = 0 saveStatement = "" isEnum = False - hasRule = False - endRule = False for line in fin: line_number += 1 @@ -102,10 +100,10 @@ def test_rules_compliance(self): enumName = "" if matchComment is not None: - if comment.find("\\endrules") != -1: + if re.search(r"^[ ]\\\bendrules\b$", comment) is not None: endRule = True - if comment.find("\\rules") != -1: + if re.search(r"^[ ]\\\brules\b$", comment) is not None: hasRule = True lineruleCount = 0 foundruleCount = 0 @@ -120,16 +118,12 @@ def test_rules_compliance(self): if statement.find(";") != -1: statement = statement.strip() self.assertFalse(hasRule and lineruleCount != foundruleCount and endRule and lineruleCount-foundruleCount-1>0, file + " in line " + str(line_number) + ": "+str(lineruleCount-foundruleCount-1)+" defined rule(s) does not exists for: '"+statement+"'") - self.assertFalse(hasRule and not endRule, file + " in line " + str(line_number) + ": \\endrules statement does not exists for: '"+statement+"'") - self.assertFalse(not hasRule and endRule, file + " in line " + str(line_number) + ": \\rules statement does not exists for: '"+statement+"'") + self.assertFalse(hasRule and not endRule, file + " in line " + str(line_number) + ": \\endrules statement does not exists for: '"+statement+"'. Check spacing and rule syntax.") + self.assertFalse(not hasRule and endRule, file + " in line " + str(line_number) + ": \\rules statement does not exists for: '"+statement+"'. Check spacing and rule syntax.") self.assertFalse(not hasRule and not endRule and lineruleCount < foundruleCount, file + " in line " + str(line_number) + ": rules found but no statements (\\rules and \\endrules) around it for: '"+statement+"'") lineruleCount = 0 foundruleCount = 0 - # elif numMessage == 0 and hasRule: - # statement = statement.strip() - # self.assertFalse(hasRule and lineruleCount != foundruleCount and endRule and lineruleCount-foundruleCount-1>0, file + " in line " + str(line_number) + ": "+str(lineruleCount-foundruleCount-1)+" defined rule(s) does not exists for: '"+statement+"'") - hasRule = False endRule = False From 3b8aba71a00d950c1f53e615855fe075da5566b3 Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 18:48:28 +0100 Subject: [PATCH 6/7] Changed fullloader to baseloader --- tests/test_rules.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_rules.py b/tests/test_rules.py index 1ddde908a..c5c0036f7 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -12,7 +12,7 @@ def test_rules_compliance(self): ''' Test rule compliance syntax of proto files. ''' with open(r'rules.yml') as rules_file: - RULES_DICT = yaml.load(rules_file, Loader=yaml.FullLoader) + RULES_DICT = yaml.load(rules_file, Loader=yaml.BaseLoader) for file in PROTO_FILES: with open(file, "rt") as fin, self.subTest(file=file): @@ -97,7 +97,6 @@ def test_rules_compliance(self): if isEnum is True and matchClosingBrace is not None: isEnum = False - enumName = "" if matchComment is not None: if re.search(r"^[ ]\\\bendrules\b$", comment) is not None: From cb32bd71443db2768b43e637b2835139d9855874 Mon Sep 17 00:00:00 2001 From: Viktor Kreschenski Date: Mon, 17 Feb 2020 19:35:00 +0100 Subject: [PATCH 7/7] Updated documentation --- doc/commenting.rst | 54 +++++++++++++--------------------------------- 1 file changed, 15 insertions(+), 39 deletions(-) diff --git a/doc/commenting.rst b/doc/commenting.rst index cb1ec68a9..572bbe5e4 100644 --- a/doc/commenting.rst +++ b/doc/commenting.rst @@ -288,45 +288,21 @@ The rule definition must follow the syntax which is defined by a regex search wh .. code-block:: python - 'is_greater_than': r'\b(is_greater_than)\b: \d+(\.\d+)?' # is_greater_than: 1 - 'is_greater_than_or_equal_to': r'\b(is_greater_than_or_equal_to)\b: \d+(\.\d+)?' # is_greater_than_or_equal_to: 1 - 'is_less_than_or_equal_to': r'\b(is_less_than_or_equal_to)\b: \d+(\.\d+)?' # is_less_than_or_equal_to: 10 - 'is_less_than': r'\b(is_less_than)\b: \d+(\.\d+)?' # is_less_than: 2 - 'is_equal': r'\b(is_equal)\b: \d+(\.\d+)?' # is_equal: 1 - 'is_different': r'\b(is_different)\b: \d+(\.\d+)?' # is_different: 2 - 'is_global_unique': r'\b(is_global_unique)\b' # is_global_unique - 'refers': r'\b(refers)\b' # refers - 'is_iso_country_code': r'\b(is_iso_country_code)\b' # is_iso_country_code - 'first_element': r'\b(first_element)\b: \{.*: \d+\.\d+\}' # first_element: {is_equal: 0.13, is_greater_than: 0.13} - 'last_element': r'\b(last_element)\b: \{.*: \d+\.\d+\}' # last_element: {is_equal: 0.13, is_greater_than: 0.13} - 'is_optional': r'\b(is_optional)\b' # is_optional - 'check_if': r'\b(check_if)\b: \[\{.*: \d+(\.\d+)?, target: .*}, \{do_check: \{.*: \d+(\.\d+)?}}]' # check_if: [{is_equal: 2, is_greater_than: 3, target: this.y}, {do_check: {is_equal: 1, is_less_than: 3}}] - -You can check the correctness of these regular expression on `regex101 `_. - - -.. is_greater_than: 2 -.. is_greater_than: 2.23 -.. is_greater_than_or_equal_to: 1 -.. is_greater_than_or_equal_to: 1.12 -.. is_less_than_or_equal_to: 10 -.. is_less_than_or_equal_to: 10.123 -.. is_less_than: 2 -.. is_less_than: 2.321 -.. is_equal: 1 -.. is_equal: 1.312 -.. is_different: 2 -.. is_different: 2.2122 -.. is_global_unique -.. refers -.. is_iso_country_code -.. first_element: {is_equal: 3, is_greater: 2} -.. first_element: {is_equal: 0.13, is_greater: 0.13} -.. last_element: {is_equal: 3, is_greater: 2} -.. last_element: {is_equal: 0.13, is_greater: 0.13} -.. check_if: [{is_equal: 2, is_greater_than: 3, target: this.y}, {do_check: {is_equal: 1, is_less_than: 3}}] -.. is_set - + 'is_greater_than': r'^[ ]\b(is_greater_than)\b: ([\s\d]+)$' # is_greater_than: 1 + 'is_greater_than_or_equal_to': r'^[ ]\b(is_greater_than_or_equal_to)\b: ([\s\d]+)$' # is_greater_than_or_equal_to: 1 + 'is_less_than_or_equal_to': r'^[ ]\b(is_less_than_or_equal_to)\b: ([\s\d]+)$' # is_less_than_or_equal_to: 10 + 'is_less_than': r'^[ ]\b(is_less_than)\b: ([\s\d]+)$' # is_less_than: 2 + 'is_equal': r'^[ ]\b(is_equal_to)\b: ([\s\d]+)$' # is_equal_to: 1 + 'is_different': r'^[ ]\b(is_different_to)\b: ([\s\d]+)$' # is_different_to: 2 + 'is_global_unique': r'^[ ]\b(is_globally_unique)\b' # is_globally_unique + 'refers': r'^[ ]\b(refers_to)\b' # refers_to: DetectedObject + 'is_iso_country_code': r'^[ ]\b(is_iso_country_code)\b' # is_iso_country_code + 'first_element': r'^[ ]\b(first_element)\b' # first_element height is_equal_to 0.13 + 'last_element': r'^[ ]\b(last_element)\b' # last_element width is_equal_to 0.13 + 'check_if': r'^[ ](\bcheck_if\b)(.*\belse do_check\b)' # check_if this.type is_equal_to 2 else do_check is_set + +You can check the correctness of these regular expression on `regex101 `_. + Commenting with doxygen references ------------------------------------