Improve umdp3 checks I : Fixing the easy to deal with false failure flags.

.github/linters/.ruff.toml

@@ -1,6 +1,7 @@
 cache-dir = "/tmp/.ruff_cache"
 line-length = 88
 output-format = "grouped"
+exclude = ["*.F90"]
 
 [lint]
 # Allow unused variables when underscore-prefixed.

script_umdp3_checker/checker_dispatch_tables.py

@@ -9,19 +9,16 @@
 Python translation of the original Perl module
 """
 
-from typing import Dict, Callable
+from typing import Callable
 from umdp3_checker_rules import UMDP3Checker
 
 """
-TODO : This list was checked to ensure it had something for each
-       test in the original.
-TODO : This needs to be re-checked.
-TODO : And the tests need to be compared to the original Perl tests
-       to ensure they are equivalent.
-"""
+TODO : This module has lost it's way. It uses a class to define methods which just
+       return lists of functions. I don't think a class is required for this.
+       As the functions themselves are shifted, renamed and hopefully improved, this
+       class should eventually get emptied out and the file removed.
 
-# Declare version
-VERSION = "13.5.0"
+"""
 
 
 class CheckerDispatchTables:
@@ -30,86 +27,66 @@ class CheckerDispatchTables:
     def __init__(self):
         self.umdp3_checker = UMDP3Checker()
 
-    def get_diff_dispatch_table_fortran(self) -> Dict[str, Callable]:
+    def get_diff_dispatch_table_fortran(self) -> list[Callable]:
         """Get dispatch table for Fortran diff tests"""
-        return {
-            # 'Captain Daves doomed test of destruction':
-            # self.umdp3_checker.capitulated_keywords,
-            "Lowercase Fortran keywords not permitted": self.umdp3_checker.capitalised_keywords,
-            "OpenMP sentinels not in column one": self.umdp3_checker.openmp_sentinels_in_column_one,
-            "Omitted optional space in keywords": self.umdp3_checker.unseparated_keywords,
-            "GO TO other than 9999": self.umdp3_checker.go_to_other_than_9999,
-            "WRITE without format": self.umdp3_checker.write_using_default_format,
-            "Lowercase or CamelCase variable names only": self.umdp3_checker.lowercase_variable_names,
-            "Use of dimension attribute": self.umdp3_checker.dimension_forbidden,
-            "Continuation lines shouldn't start with &": self.umdp3_checker.ampersand_continuation,
-            "Use of EQUIVALENCE or PAUSE": self.umdp3_checker.forbidden_keywords,
-            "Use of older form of relational operator (.GT. etc.)": self.umdp3_checker.forbidden_operators,
-            "Line longer than 80 characters": self.umdp3_checker.line_over_80chars,
-            "Line includes tab character": self.umdp3_checker.tab_detection,
-            "USEd printstatus_mod instead of umPrintMgr": self.umdp3_checker.printstatus_mod,
-            "Used PRINT rather than umMessage and umPrint": self.umdp3_checker.printstar,
-            "Used WRITE(6) rather than umMessage and umPrint": self.umdp3_checker.write6,
-            "Used um_fort_flush rather than umPrintFlush": self.umdp3_checker.um_fort_flush,
-            "Used Subversion keyword substitution which is prohibited": self.umdp3_checker.svn_keyword_subst,
-            "Used !OMP instead of !$OMP": self.umdp3_checker.omp_missing_dollar,
-            "Used #ifdef/#ifndef rather than #if defined() or #if !defined()": self.umdp3_checker.cpp_ifdef,
-            "Presence of fortran comment in CPP directive": self.umdp3_checker.cpp_comment,
-            "Used an archaic fortran intrinsic function": self.umdp3_checker.obsolescent_fortran_intrinsic,
-            "EXIT statements should be labelled": self.umdp3_checker.exit_stmt_label,
-            "Intrinsic modules must be USEd with an INTRINSIC "
-            + "keyword specifier": self.umdp3_checker.intrinsic_modules,
-            "READ statements should have an explicit UNIT= as "
-            + "their first argument": self.umdp3_checker.read_unit_args,
-        }
+        return [
+            self.umdp3_checker.openmp_sentinels_in_column_one,
+            self.umdp3_checker.unseparated_keywords,
+            self.umdp3_checker.go_to_other_than_9999,
+            self.umdp3_checker.write_using_default_format,
+            self.umdp3_checker.dimension_forbidden,
+            self.umdp3_checker.ampersand_continuation,
+            self.umdp3_checker.forbidden_keywords,
+            self.umdp3_checker.forbidden_operators,
+            self.umdp3_checker.tab_detection,
+            self.umdp3_checker.printstatus_mod,
+            self.umdp3_checker.printstar,
+            self.umdp3_checker.write6,
+            self.umdp3_checker.um_fort_flush,
+            self.umdp3_checker.svn_keyword_subst,
+            self.umdp3_checker.omp_missing_dollar,
+            self.umdp3_checker.cpp_ifdef,
+            self.umdp3_checker.cpp_comment,
+            self.umdp3_checker.obsolescent_fortran_intrinsic,
+            self.umdp3_checker.exit_stmt_label,
+            self.umdp3_checker.intrinsic_modules,
+            self.umdp3_checker.read_unit_args,
+        ]
 
-    def get_file_dispatch_table_fortran(
-        self, filename: str = ""
-    ) -> Dict[str, Callable]:
+    def get_file_dispatch_table_fortran(self, filename: str = "") -> list[Callable]:
         """Get dispatch table for Fortran file tests"""
-        return {
-            "Warning - used an if-def due for retirement": self.umdp3_checker.retire_if_def,
-            "File is missing at least one IMPLICIT NONE": self.umdp3_checker.implicit_none,
-            "Never use STOP or CALL abort": self.umdp3_checker.forbidden_stop,
-            "Use of Fortran function as a variable name": self.umdp3_checker.intrinsic_as_variable,
-            "File missing crown copyright statement or agreement reference": self.umdp3_checker.check_crown_copyright,
-            "File missing correct code owner comment": self.umdp3_checker.check_code_owner,
-            "Used (/ 1,2,3 /) form of array initialisation, rather than "
-            + "[1,2,3] form": self.umdp3_checker.array_init_form,
-        }
+        return [
+            self.umdp3_checker.retire_if_def,
+            self.umdp3_checker.implicit_none,
+            self.umdp3_checker.forbidden_stop,
+            self.umdp3_checker.intrinsic_as_variable,
+            self.umdp3_checker.check_code_owner,
+            self.umdp3_checker.array_init_form,
+        ]
 
-    def get_diff_dispatch_table_c(self) -> Dict[str, Callable]:
+    def get_diff_dispatch_table_c(self) -> list[Callable]:
         """Get dispatch table for C diff tests"""
-        return {
-            "Line longer than 80 characters": self.umdp3_checker.line_over_80chars,
-            "Line includes tab character": self.umdp3_checker.tab_detection,
-            "Fixed-width Integer format specifiers must have a space "
-            + 'between themselves and the string delimiter (the " character)': self.umdp3_checker.c_integral_format_specifiers,
-        }
+        return [
+            self.umdp3_checker.tab_detection,
+            self.umdp3_checker.c_integral_format_specifiers,
+        ]
 
-    def get_file_dispatch_table_c(self) -> Dict[str, Callable]:
+    def get_file_dispatch_table_c(self) -> list[Callable]:
         """Get dispatch table for C file tests"""
-        return {
-            "Warning - used an if-def due for retirement": self.umdp3_checker.retire_if_def,
-            "Used a deprecated C identifier": self.umdp3_checker.c_deprecated,
-            "File missing crown copyright statement or agreement reference": self.umdp3_checker.check_crown_copyright,
-            "File missing correct code owner comment": self.umdp3_checker.check_code_owner,
-            "Used an _OPENMP if-def without also testing against "
-            + "SHUM_USE_C_OPENMP_VIA_THREAD_UTILS. (Or _OPENMP does "
-            + "not come first in the test.)": self.umdp3_checker.c_openmp_define_pair_thread_utils,
-            "Used an _OPENMP && SHUM_USE_C_OPENMP_VIA_THREAD_UTILS if-def "
-            + "test in a logical combination with a third macro": self.umdp3_checker.c_openmp_define_no_combine,
-            "Used !defined(_OPENMP) rather than defined(_OPENMP) "
-            + "with #else branch": self.umdp3_checker.c_openmp_define_not,
-            "Used an omp #pragma (or #include <omp.h>) without "
-            + "protecting it with an _OPENMP if-def": self.umdp3_checker.c_protect_omp_pragma,
-            "Used the #ifdef style of if-def, rather than the #if "
-            + "defined() style": self.umdp3_checker.c_ifdef_defines,
-            "C Unit does not end with a final newline character": self.umdp3_checker.c_final_newline,
-        }
+        return [
+            self.umdp3_checker.retire_if_def,
+            self.umdp3_checker.c_deprecated,
+            self.umdp3_checker.check_code_owner,
+            self.umdp3_checker.c_openmp_define_pair_thread_utils,
+            self.umdp3_checker.c_openmp_define_no_combine,
+            self.umdp3_checker.c_openmp_define_not,
+            self.umdp3_checker.c_protect_omp_pragma,
+            self.umdp3_checker.c_ifdef_defines,
+            self.umdp3_checker.c_final_newline,
+        ]
 
-    def get_file_dispatch_table_all(self) -> Dict[str, Callable]:
+    def get_file_dispatch_table_all(self) -> list[Callable]:
         """Get dispatch table for universal file tests"""
-        return {
-            "Line includes trailing whitespace character(s)": self.umdp3_checker.line_trail_whitespace,
-        }
+        return [
+            self.umdp3_checker.line_trail_whitespace,
+        ]

script_umdp3_checker/fortran_keywords.py

@@ -14,6 +14,8 @@
 """
 TODO: Current order may not be perfect, and could possibly be reviewed.
 However, it is probably 'good enough' for now.
+TODO: Document method used to decide on order. I think it was a grep and count for the
+frequency the keywords appear in the source code.
 """
 
 fortran_keywords = (

script_umdp3_checker/tests/conftest.py

@@ -0,0 +1,9 @@
+from pathlib import Path
+import pytest
+
+
+@pytest.fixture(scope="session")
+def example_fortran_lines() -> list[str]:
+    """Return the example Fortran source as a list of lines for tests."""
+    test_dir = Path(__file__).resolve().parent
+    return (test_dir / "example_fortran_code.F90").read_text().splitlines()

script_umdp3_checker/tests/example_fortran_code.F90

@@ -0,0 +1,121 @@
+! fortls: ignore file
+! fortitude: ignore file
+! *****************************COPYRIGHT*******************************
+! (C) Crown copyright Met Office. All rights reserved.
+! For further details please refer to the file COPYRIGHT.txt
+! which you should have received as part of this distribution.
+! *****************************COPYRIGHT*******************************
+!
+! An example routine depicting how one should construct new code
+! to meet the UMDP3 coding standards.
+!
+MODULE example_mod
+IMPLICIT NONE
+! Description:
+!   A noddy routine that illustrates the way to apply the UMDP3
+!   coding standards to new code to help code developers
+!   pass code reviews.
+!
+! Method:
+!   In this routine we apply many of the UMDP3 features
+!   to construct a simple routine. The references on the RHS take the reader
+!   to the appropriate section of the UMDP3 guide with further details.
+!
+! Code Owner: Please refer to the UM file CodeOwners.txt
+! This file belongs in section: Control
+!
+! Code description:
+!   Language: Fortran 2003.
+!   This code is written to UMDP3 standards.
+CHARACTER(LEN=*), PARAMETER, PRIVATE :: ModuleName="EXAMPLE_MOD"
+CONTAINS
+! Subroutine Interface:
+SUBROUTINE example (xlen,ylen,l_unscale,input1,input2,   &
+                    output, l_loud_opt)
+! Description:
+! Nothing further to add to module description.
+USE atmos_constants_mod, ONLY: r
+USE ereport_mod, ONLY: ereport
+USE parkind1, ONLY: jpim, jprb
+USE umprintMgr, ONLY: umprint,ummessage,PrNorm
+USE errormessagelength_mod, ONLY: errormessagelength
+USE yomhook, ONLY: lhook, dr_hook
+IMPLICIT NONE
+! Subroutine arguments
+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) ! 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
+INTEGER(KIND=jpim), PARAMETER :: zhook_out = 1 ! DrHook tracing exit
+INTEGER :: i ! Loop counter
+INTEGER :: j ! Loop counter
+INTEGER :: icode ! error code for EReport
+LOGICAL :: l_loud ! debug flag (default false unless l_loud_opt is used)
+REAL, ALLOCATABLE :: field(:,:) ! Scaling array to fill.
+REAL, ALLOCATABLE :: field2(:,:) ! Scaling array to fill.
+REAL(KIND=jprb) :: zhook_handle ! DrHook tracing
+CHARACTER(LEN=*), PARAMETER :: RoutineName="EXAMPLE"
+CHARACTER(LEN=errormessagelength) :: Cmessage ! used for EReport
+CHARACTER(LEN=256) :: my_char ! string for output
+! End of header
+IF (lhook) CALL dr_hook(ModuleName//":"//RoutineName,zhook_in,zhook_handle)
+! Set debug flag if argument is present
+l_loud = .FALSE.
+IF (PRESENT(l_loud_opt)) THEN
+    l_loud = l_loud_opt
+END IF
+my_char                                                                        &
+    = "This is a very very very very very very very "                          &
+    // "long character assignment" ! A pointless long character example.
+icode=0
+! verbosity choice, output some numbers to aid with debugging
+! protected by printstatus>=PrNorm and pe=0
+WRITE(ummessage,"(A,I0)")"xlen=",xlen
+CALL umprint(ummessage,level=PrNorm,pe=0,src="example_mod")
+WRITE(ummessage,"(A,I0)")"ylen=",ylen
+CALL umprint(ummessage,level=PrNorm,pe=0,src="example_mod")
+IF (l_loud) CALL umprint(my_char,level=PrNormal,src="example_mod")
+! Allocate and initialise scaling array
+! Noddy code warns user when scalling is not employed.
+IF ( l_unscale ) THEN
+    icode = -100 ! set up WARNING message
+    ALLOCATE(field( 1,1 ) )
+    ALLOCATE(field2( 1,1 ) )
+    cmessage="Scaling is switched off in run!"
+    CALL ereport(RoutineName,icode,cmessage)
+ELSE
+    ALLOCATE(field( xlen, ylen ) )
+    ALLOCATE(field2( xlen, ylen ) )
+    DO j=1,ylen
+        DO i=1,xlen
+            field(i, j) = (1.0*i) + (2.0*j)
+            input2(i, j) = input2(i, j) * field(i, j)
+            field2(i, j) = (1.0*i) - (2.0*j)                                   &
+                + (3.0*i*j) + (4.0*i**2) + field(i, j)*2
+        END DO
+    END DO
+END IF
+! The main calculation of the routine, using OpenMP.
+!$OMP PARALLEL DEFAULT(NONE) &
+!$OMP SHARED(xlen,ylen,input1,input2,field,output) &
+!$OMP PRIVATE(i, j )
+!$OMP DO SCHEDULE(STATIC)
+DO j = 1, ylen
+    i_loop: DO i = 1, xlen
+    ! Calculate the Output value:
+        output(i, j) = (input1(i, j) * input2(i, j))
+    END DO i_loop
+END DO ! j loop
+!$OMP END DO
+!$OMP END PARALLEL
+DEALLOCATE (field)
+IF (lhook) CALL dr_hook(ModuleName//":"//RoutineName,zhook_out,zhook_handle)
+RETURN
+END SUBROUTINE example
+END MODULE example_mod