diff --git a/script_umdp3_checker/tests/example_fortran_code.F90 b/script_umdp3_checker/tests/example_fortran_code.F90 index 9525ae5c..9e78967c 100644 --- a/script_umdp3_checker/tests/example_fortran_code.F90 +++ b/script_umdp3_checker/tests/example_fortran_code.F90 @@ -45,9 +45,10 @@ SUBROUTINE example (xlen,ylen,l_unscale,input1,input2, & INTEGER, INTENT(IN) :: xlen !Length of first dimension of the arrays. INTEGER, INTENT(IN) :: ylen !Length of second dimension of the arrays. LOGICAL, INTENT(IN) :: l_unscale ! switch scaling off. -REAL, INTENT(IN) :: input1(xlen, ylen) !First input array -REAL, INTENT(IN OUT) :: input2(xlen, ylen) !Second input array -REAL, INTENT(OUT) :: output(xlen, ylen) !Contains the result +REAL, INTENT(IN) :: input1(xlen, ylen) !First input array +REAL, INTENT(IN OUT) :: input2(xlen, ylen) ! INOUT Second input array +REAL, INTENT(OUT) :: output(xlen, ylen) !Contains the result +REAL, PARAMETER :: b_pollonator(4) = [ 0.0, 11.2, 6.6, 3.6] LOGICAL, INTENT(IN), OPTIONAL :: l_loud_opt !optional debug flag ! Local variables INTEGER(KIND=jpim), PARAMETER :: zhook_in = 0 ! DrHook tracing entry diff --git a/script_umdp3_checker/tests/test_fortran_checks.py b/script_umdp3_checker/tests/test_fortran_checks.py index 0700bdd7..0360e0bf 100644 --- a/script_umdp3_checker/tests/test_fortran_checks.py +++ b/script_umdp3_checker/tests/test_fortran_checks.py @@ -84,6 +84,10 @@ def test_openmp_sentinels_in_column_one(lines, expected_result): (["i=0", "i=i+1", "PRINT*,i"], 0, "No keywords"), (["PROGRAM test", "i=0", "ENDIF"], 1, "One keyword unseparated"), (["i=0", "ENDPARALLELDO", "END DO"], 1, "One keyword unseparated in middle"), + (["REAL, INTENT(IN OUT) :: lambda_pole !INOUT Longitude of pole", + "REAL, INTENT(OUT) :: Dave", "DO", "nothing", "END DO"], 0, + "unseparated keyword in comment"), + (["#endif", "i=i+1", "PRINT*,i"], 0, "Safely ignore cpp commands"), ] diff --git a/script_umdp3_checker/tests/test_umdp3_rules_S3.py b/script_umdp3_checker/tests/test_umdp3_rules_S3.py index 256ff927..5ecc0606 100644 --- a/script_umdp3_checker/tests/test_umdp3_rules_S3.py +++ b/script_umdp3_checker/tests/test_umdp3_rules_S3.py @@ -148,8 +148,6 @@ def test_concatenate_lines(example_fortran_lines): @pytest.mark.parametrize( "changes_list, expected_passed, expected_failure_count, expected_errors", [ - # Valid: MODULE example_mod ... END MODULE example_mod (no changes) - ([], True, 0, {}), # Invalid: No program unit found (delete MODULE line) ( [["replace", 12, ["! No module declaration here"]]], @@ -162,7 +160,7 @@ def test_concatenate_lines(example_fortran_lines): ), # Invalid: Mismatched END statement ( - [["replace", 120, ["END MODULE wrong_mod_name"]]], + [["replace", 121, ["END MODULE wrong_mod_name"]]], False, 1, { @@ -173,7 +171,7 @@ def test_concatenate_lines(example_fortran_lines): ), # Invalid: No END statement found ( - [["replace", 120, ["! Missing END MODULE"]]], + [["replace", 121, ["! Missing END MODULE"]]], False, 1, { @@ -182,12 +180,22 @@ def test_concatenate_lines(example_fortran_lines): ] }, ), + # Pass ccp directives #if !defined(MCT) + ( + [["replace", 1, ["#if !defined(MCT)"]], ["add", 121, ["#endif"]]], + True, + 0, + {}, + ), + # Valid: MODULE example_mod ... END MODULE example_mod (no changes) + ([], True, 0, {}), ], ids=[ - "Valid module with matching END", "No program unit found", "Mismatched END statement", "No END statement", + "Valid checking cpp directives pass", + "Valid module with matching END", ], ) def test_r3_1_1_there_can_be_only_one( @@ -199,15 +207,15 @@ def test_r3_1_1_there_can_be_only_one( ): modified_fortran_lines = modify_fortran_lines(example_fortran_lines, changes_list) result = r3_1_1_there_can_be_only_one(modified_fortran_lines) - assert result.passed == expected_passed - assert result.failure_count == expected_failure_count errors = result.errors - assert len(errors) == len(expected_errors) for error, lines_list in errors.items(): assert error in expected_errors - assert len(lines_list) == len(expected_errors[error]) for line_no in lines_list: assert line_no in expected_errors[error] + assert len(lines_list) == len(expected_errors[error]) + assert len(errors) == len(expected_errors) + assert result.failure_count == expected_failure_count + assert result.passed == expected_passed # ================================================================= @@ -238,14 +246,14 @@ def test_r3_2_1_check_crown_copyright( modified_fortran_lines = modify_fortran_lines(example_fortran_lines, changes_list) result = r3_2_1_check_crown_copyright(modified_fortran_lines) failure_count = result.failure_count - assert failure_count == expected_result errors = result.errors - assert len(errors) == len(expected_errors) for error, lines_list in errors.items(): assert error in expected_errors - assert len(lines_list) == len(expected_errors[error]) for line_no in lines_list: assert line_no in expected_errors[error] + assert len(lines_list) == len(expected_errors[error]) + assert len(errors) == len(expected_errors) + assert failure_count == expected_result # ================================================================= @@ -272,7 +280,6 @@ def test_r3_2_1_check_crown_copyright( + '":" // RoutineName, zhook_out, zhook_handle) ! extra comment' ], ], - ["replace", 42, ["use yomhook, ONLY: lhook, dr_hook"]], ], 2, {"line too long": [73, 117]}, @@ -288,14 +295,14 @@ def test_r3_3_2_line_too_long( modified_fortran_lines = modify_fortran_lines(example_fortran_lines, changes_list) result = r3_3_2_line_too_long(modified_fortran_lines) failure_count = result.failure_count - assert failure_count == expected_result errors = result.errors - assert len(errors) == len(expected_errors) for error, lines_list in errors.items(): assert error in expected_errors - assert len(lines_list) == len(expected_errors[error]) for line_no in lines_list: assert line_no in expected_errors[error] + assert len(lines_list) == len(expected_errors[error]) + assert len(errors) == len(expected_errors) + assert failure_count == expected_result # ================================================================= @@ -313,9 +320,19 @@ def test_r3_3_2_line_too_long( 3, {"lowercase keyword: Module": [12], "lowercase keyword: use": [39, 42]}, ), + ( + [ + ["add", 44, ["#if defined(SOMETHING)"]], + ["add", 47, ["#else", "INTEGER, INTENT(INOUT) :: xlen", + "INTEGER, INTENT(OUT) :: ylen", + "LOGICAL, INTENT(IN) :: l_WhoopSies = .FALSE.", "#endif"]], + ], + 0, + {}, + ), ([], 0, {}), # No changes, expect no errors ], - ids=["3 Lowercase Errors", "No Lowercase Errors"], + ids=["3 Lowercase Errors", "Pass cpp directives", "No Lowercase Errors"], ) def test_r3_4_1_capitalised_keywords( example_fortran_lines, changes_list, expected_result, expected_errors @@ -323,14 +340,14 @@ def test_r3_4_1_capitalised_keywords( modified_fortran_lines = modify_fortran_lines(example_fortran_lines, changes_list) result = r3_4_1_capitalised_keywords(modified_fortran_lines) failure_count = result.failure_count - assert failure_count == expected_result errors = result.errors - assert len(errors) == len(expected_errors) for error, lines_list in errors.items(): assert error in expected_errors - assert len(lines_list) == len(expected_errors[error]) for line_no in lines_list: assert line_no in expected_errors[error] + assert len(lines_list) == len(expected_errors[error]) + assert len(errors) == len(expected_errors) + assert failure_count == expected_result # ================================================================= @@ -347,19 +364,19 @@ def test_r3_4_1_capitalised_keywords( 45, ["INTEGER, INTENT(IN) :: XLEN !Length of first dim of the arrays."], ], - ["replace", 60, ["REAL :: var1, DAVE_2, HiPPo"]], + ["replace", 61, ["REAL :: var1, DAVE_2, HiPPo"]], ], 2, { - "Found UPPERCASE variable name in declaration at line 45: XLEN": [45], - "Found UPPERCASE variable name in declaration at line 60: DAVE_2": [60], + "Found UPPERCASE variable name in declaration at line 45: \"XLEN\"": [45], + "Found UPPERCASE variable name in declaration at line 61: \"DAVE_2\"": [61], }, ), ( [ [ "add", - 58, + 59, ["LOGICAL :: l_whizz_bang = .FALSE. ! optimisation flag"], ], ], @@ -370,7 +387,7 @@ def test_r3_4_1_capitalised_keywords( [ [ "add", - 60, + 61, [ "REAL :: VARIaBLE_1, variable_2, &", " VARIABLE_3, Hot_Potato, Baked Potato &", @@ -380,7 +397,7 @@ def test_r3_4_1_capitalised_keywords( ], [ "replace", - 56, + 57, [ "INTEGER :: j ! Loop counter &", "INTEGER :: k ! Loop counter &", @@ -398,25 +415,59 @@ def test_r3_4_1_capitalised_keywords( ], 5, { - "Found UPPERCASE variable name in declaration at line 45: XLEN": [45], - "Found UPPERCASE variable name in declaration at line 58: IJ": [58], - "Found UPPERCASE variable name in declaration at line 62: CASPVAR": [ - 62 + "Found UPPERCASE variable name in declaration at line 45: \"XLEN\"": [45], + "Found UPPERCASE variable name in declaration at line 59: \"IJ\"": [59], + "Found UPPERCASE variable name in declaration at line 63: \"CASPVAR\"": [ + 63 ], - "Found UPPERCASE variable name in declaration at line 62: VARIABLE_3": [ - 62 + "Found UPPERCASE variable name in declaration at line 63: \"VARIABLE_3\"": [ + 63 ], - "Found UPPERCASE variable name in declaration at line 62: CAPS_VAR": [ - 62 + "Found UPPERCASE variable name in declaration at line 63: \"CAPS_VAR\"": [ + 63 ], }, ), + ( + [ + ["delete", 48, None], + [ + "replace", + 49, + [ + "REAL, INTENT(IN) :: input1(xlen, ylen), & !First input array", + " input2(XLEN, ylen), !Second input array", + ], + ], + # [], + ], + 0, + {}, + ), + ( + [ + ["delete", 48, None], + [ + "replace", + 49, + [ + "REAL, INTENT(IN) :: input1(xlen, ylen), & !First input array", + " INPUT2(XLEN, ylen), !Second input array", + ], + ], + # [], + ], + 1, + {"Found UPPERCASE variable name in declaration at line 48: \"INPUT2\"" : [48]}, + ), ([], 0, {}), # No changes, expect no errors ], ids=[ "2 UpperCase Var Errors", "False FALSE error", "5 UpperCase Var Errors on extended lines", + "Array dimnensions, twice on an extended line, but no failures", + "Array dimnensions, twice on an extended line, with one failure", "No UpperCase Var Errors", ], ) @@ -426,11 +477,16 @@ def test_r3_4_2_no_full_uppercase_variable_names( modified_fortran_lines = modify_fortran_lines(example_fortran_lines, changes_list) result = r3_4_2_no_full_uppercase_variable_names(modified_fortran_lines) failure_count = result.failure_count - assert failure_count == expected_result errors = result.errors - assert len(errors) == len(expected_errors) for error, lines_list in errors.items(): assert error in expected_errors - assert len(lines_list) == len(expected_errors[error]) for line_no in lines_list: assert line_no in expected_errors[error] + assert len(lines_list) == len(expected_errors[error]) + assert len(errors) == len(expected_errors) + assert failure_count == expected_result + + +"""TODO: When testing for 'unseparated keywords' remember to test/discard false +ves on +"#endif" or similar. Current (old) test flags these in the wild, but the example file +has no cpp directives...""" \ No newline at end of file diff --git a/script_umdp3_checker/umdp3_checker_rules.py b/script_umdp3_checker/umdp3_checker_rules.py index ea469565..eddc9140 100644 --- a/script_umdp3_checker/umdp3_checker_rules.py +++ b/script_umdp3_checker/umdp3_checker_rules.py @@ -134,7 +134,9 @@ def unseparated_keywords(self, lines: List[str]) -> TestResult: if line.lstrip(" ").startswith("!"): continue clean_line = self.remove_quoted(line) - for pattern in [f"\\b{kw}\\b" for kw in unseparated_keywords_list]: + clean_line = self.comment_line.sub("", clean_line).strip() + # The [^#] is to avoid false positives on #endif and similar. + for pattern in [f"([^#]|^)\\b{kw}\\b" for kw in unseparated_keywords_list]: if re.search(pattern, clean_line, re.IGNORECASE): failures += 1 error_log = self.add_error_log( @@ -158,7 +160,7 @@ def go_to_other_than_9999(self, lines: List[str]) -> TestResult: count = -1 for count, line in enumerate(lines): clean_line = self.remove_quoted(line) - clean_line = re.sub(r"!.*$", "", clean_line) + clean_line = self.comment_line.sub("", clean_line).strip() if match := re.search(r"\bGO\s*TO\s+(\d+)", clean_line, re.IGNORECASE): label = match.group(1) diff --git a/script_umdp3_checker/umdp3_rules_S3.py b/script_umdp3_checker/umdp3_rules_S3.py index b7f03dfa..f59f41e2 100644 --- a/script_umdp3_checker/umdp3_rules_S3.py +++ b/script_umdp3_checker/umdp3_rules_S3.py @@ -24,6 +24,7 @@ # from script_umdp3_checker import fortran_keywords comment_line = re.compile(r"!.*$") +cpp_command_line = re.compile(r"^#.*$") word_splitter = re.compile(r"\b\w+\b") @@ -77,9 +78,16 @@ def create_unique_random_string(storage_set, length: int = 7) -> str: def remove_comments(line: str) -> str: """Remove comments from the lines : - There is a bit of an assumption here that quoted text has already been removed, so that we don't accidentally remove text after an "!" in a string.""" + There is a bit of an assumption here that quoted text has already been removed, so + that we don't accidentally remove text after an "!" in a string.""" return comment_line.sub("", line).rstrip() +def remove_cpp_commands(line: str) -> str: + """Remove cpp commands from the lines : + There is a bit of an assumption here that quoted text has already been removed, so that we don't accidentally remove text after an "#" in a string. + Also that cpp commands have the '#' in col 1 of the line.""" + return cpp_command_line.sub("", line).rstrip() + def concatenate_lines(lines: List[str], line_no: int) -> str: """Concatenate the continuation lines into a single string""" @@ -118,6 +126,7 @@ def r3_1_1_there_can_be_only_one( def find_first(lines: List[str]) -> tuple[bool, str]: for line in lines: executable_line = remove_comments(line).strip() + executable_line = remove_cpp_commands(executable_line).strip() if not executable_line: continue # Skip empty lines for keyword in program_unit_keywords: @@ -136,6 +145,7 @@ def find_first(lines: List[str]) -> tuple[bool, str]: def find_last(lines: List[str], unit_type: str, unit_name: str) -> tuple[bool, str]: for line in reversed(lines): executable_line = remove_comments(line).strip() + executable_line = remove_cpp_commands(executable_line).strip() if not executable_line: continue # Skip empty lines unit_name_search = re.search(rf"END\s+{unit_type}\s+(\w+)", executable_line) @@ -285,6 +295,7 @@ def r3_4_1_capitalised_keywords(lines: List[str]) -> TestResult: continue clean_line = remove_quoted(line) clean_line = comment_line.sub("", clean_line) + clean_line = remove_cpp_commands(clean_line) # Check for lowercase keywords for word in word_splitter.findall(clean_line): upcase = word.upper() @@ -323,6 +334,9 @@ def r3_4_2_no_full_uppercase_variable_names(lines: List[str]) -> TestResult: declaration_search = re.compile( r"^\s*(INTEGER|REAL|LOGICAL|CHARACTER|TYPE)\s*.*::\s*", re.IGNORECASE ) + digit_search = re.compile(r"([+-]?\d+\.\d+|[+-]?\d+)") + array_dimensions_search = re.compile(r"\([^)]*\)") # finds array dimensions in declaration + array_assignment_search = re.compile(r"=\s*\[[^\]]*\]\s*") # finds array assignments for count, line in enumerate(lines, 1): clean_line = remove_quoted(line) clean_line = remove_comments(clean_line) @@ -330,15 +344,19 @@ def r3_4_2_no_full_uppercase_variable_names(lines: List[str]) -> TestResult: if declaration_search.search(clean_line): full_line = concatenate_lines(lines, count) clean_line = full_line.split("::", 1)[1].strip() - clean_line = re.sub(r"\([^)]*\)", "", clean_line) + clean_line = array_dimensions_search.sub("", clean_line).strip() + clean_line = array_assignment_search.sub("", clean_line) variables = [var.strip() for var in clean_line.split(",")] for var in variables: + if digit_search.fullmatch(var): + continue # Skip if it's just a number var = var.split(r"=", 1)[0].strip() # Remove any assignment part - if var.upper() == var: + if var and var.upper() == var: failures += 1 error_log = add_error_log( error_log, - f"Found UPPERCASE variable name in declaration at line {count}: {var}", + f"Found UPPERCASE variable name in declaration at line {count}:" + + f" \"{var}\"", count, ) return TestResult(