Linter debug rule fixes

lint_rules/lint_rules/debug_rules.py

@@ -10,7 +10,7 @@
      FindNodes, CallStatement, Assignment, Scalar, RangeIndex, resolve_associates,
      simplify, Sum, Product, IntLiteral, as_tuple, SubstituteExpressions, Array,
      symbolic_op, StringLiteral, is_constant, LogicLiteral, VariableDeclaration, flatten,
-     FindInlineCalls, Conditional, Transformer, FindExpressions, Comparison
+     FindInlineCalls, Conditional, FindExpressions, Comparison
 )
 from loki.lint import GenericRule, RuleType
 
@@ -21,6 +21,10 @@ class ArgSizeMismatchRule(GenericRule):
 
     type = RuleType.WARN
 
+    config = {
+        'max_indirections': 2,
+    }
+
     @staticmethod
     def range_to_sum(lower, upper):
         """
@@ -106,6 +110,8 @@ def check_subroutine(cls, subroutine, rule_report, config, **kwargs):
         :any:`Subroutine`.
         """
 
+        max_indirections = config['max_indirections']
+
         # first resolve associates
         resolve_associates(subroutine)
 
@@ -183,9 +189,22 @@ def check_subroutine(cls, subroutine, rule_report, config, **kwargs):
                 dummy_size = Product(dummy_arg_size)
                 stat = cls.compare_sizes(arg_size, alt_arg_size, dummy_size)
 
-                # if necessary, update dimension names and check
-                if not stat:
-                    dummy_size = Product(SubstituteExpressions(assign_map).visit(dummy_arg_size))
+                # we check for a configurable number of indirections for the dummy and arg dimension names
+                for _ in range(max_indirections):
+                    if stat:
+                        break
+
+                    # if necessary, update dummy arg dimension names and check
+                    dummy_arg_size = SubstituteExpressions(assign_map).visit(dummy_arg_size)
+                    dummy_size = Product(dummy_arg_size)
+                    stat = cls.compare_sizes(arg_size, alt_arg_size, dummy_size)
+
+                    if stat:
+                        break
+
+                    # if necessary, update arg dimension names and check
+                    arg_size = SubstituteExpressions(assign_map).visit(arg_size)
+                    alt_arg_size = SubstituteExpressions(assign_map).visit(alt_arg_size)
                     stat = cls.compare_sizes(arg_size, alt_arg_size, dummy_size)
 
                 if not stat:
@@ -254,8 +273,8 @@ def fix_subroutine(cls, subroutine, rule_report, config):
         ubound_checks = cls.get_ubound_checks(subroutine)
         args = cls.get_assumed_shape_args(subroutine)
 
-        new_vars = ()
         node_map = {}
+        var_map = {}
 
         for arg in args:
             checks = [c for c in ubound_checks if arg.name in c.arguments]
@@ -281,16 +300,18 @@ def fix_subroutine(cls, subroutine, rule_report, config):
                     else:
                         new_shape += as_tuple(cond.left)
 
-                vtype = arg.type.clone(shape=new_shape, scope=subroutine)
-                new_vars += as_tuple(arg.clone(type=vtype, dimensions=new_shape, scope=subroutine))
+                vtype = arg.type.clone(shape=new_shape)
+                var_map.update({arg: arg.clone(type=vtype, dimensions=new_shape)})
 
-        #TODO: add 'VariableDeclaration.symbols' should be of type 'Variable' rather than 'Expression'
-        # to enable case-insensitive search here
-        new_var_names = [v.name.lower() for v in new_vars]
-        subroutine.variables = [var for var in subroutine.variables if not var.name.lower() in new_var_names]
+        # update variable declarations
+        subroutine.spec = SubstituteExpressions(var_map).visit(subroutine.spec)
+        for decl in FindNodes(VariableDeclaration).visit(subroutine.spec):
+            if decl.dimensions:
+                if not all(sym.shape == decl.dimensions for sym in decl.symbols):
+                    new_decls = as_tuple(VariableDeclaration(as_tuple(sym)) for sym in decl.symbols)
+                    node_map.update({decl: new_decls})
 
-        subroutine.body = Transformer(node_map).visit(subroutine.body)
-        subroutine.variables += new_vars
+        return node_map
 
 # Create the __all__ property of the module to contain only the rule names
 __all__ = tuple(name for name in dir() if name.endswith('Rule') and name != 'GenericRule')

lint_rules/lint_rules/ifs_coding_standards_2011.py

@@ -472,15 +472,6 @@ class Fortran90OperatorsRule(GenericRule):  # Coding standards 4.15
         '<': re.compile(r'(?P<f77>\.lt\.)|(?P<f90><(?!=))', re.I),
     }
 
-    _op_map = {
-        '==': '.eq.',
-        '/=': '.ne.',
-        '>=': '.ge.',
-        '<=': '.le.',
-        '>': '.gt.',
-        '<': '.lt.'
-    }
-
     @classmethod
     def check_subroutine(cls, subroutine, rule_report, config, **kwargs):
         '''Check for the use of Fortran 90 comparison operators.'''
@@ -513,8 +504,16 @@ def check_subroutine(cls, subroutine, rule_report, config, **kwargs):
                     op_str = op if op != '!=' else '/='
                     line = [line for line in lines if op_str in strip_inline_comments(line.string)]
                     if not line:
+                        _op_map = {
+                            '==': '.eq.',
+                            '/=': '.ne.',
+                            '>=': '.ge.',
+                            '<=': '.le.',
+                            '>': '.gt.',
+                            '<': '.lt.'
+                        }
                         line = [line for line in lines
-                                if op_str in strip_inline_comments(line.string.replace(cls._op_map[op_str], op_str))]
+                                if op_str in strip_inline_comments(line.string.replace(_op_map[op_str], op_str))]
 
                     source_string = strip_inline_comments(line[0].string)
                     matches = cls._op_patterns[op].findall(source_string)

lint_rules/tests/test_debug_rules.py

@@ -11,7 +11,7 @@
 import pytest
 
 from conftest import run_linter, available_frontends
-from loki import Sourcefile, FindInlineCalls
+from loki import Sourcefile, FindInlineCalls, FindNodes, VariableDeclaration
 from loki.lint import DefaultHandler
 
 
@@ -42,14 +42,16 @@ def test_arg_size_array_slices(rules, frontend):
 real, intent(in) :: var6(:,:), var7(:,:)
 real, intent(inout) :: var0(klon, nblk), var1(klon, 138, nblk)
 real(kind=jphook) :: zhook_handle
-integer :: klev, ibl
+integer :: klev, ibl, iproma, iend
 
 if(lhook) call dr_hook('driver', 0, zhook_handle)
 
 associate(nlev => klev)
 nlev = 137
 do ibl = 1, nblk
-   call kernel(klon, nlev, var0(:,ibl), var1(:,:,ibl), var2(1:klon, 1:nlev), &
+   iproma = klon
+   iend = iproma
+   call kernel(klon, nlev, var0(:,ibl), var1(:,:,ibl), var2(1:iend, 1:nlev), &
                var3, var4(1:klon, 1:nlev+1), var5(:, 1:nlev+1), &
                var6_d=var6, var7_d=var7(:,1:nlev))
 enddo
@@ -84,7 +86,8 @@ def test_arg_size_array_slices(rules, frontend):
 
     messages = []
     handler = DefaultHandler(target=messages.append)
-    _ = run_linter(driver_source, [rules.ArgSizeMismatchRule], handlers=[handler], targets=['kernel',])
+    _ = run_linter(driver_source, [rules.ArgSizeMismatchRule], config={'ArgSizeMismatchRule': {'max_indirections': 3}},
+                   handlers=[handler], targets=['kernel',])
 
     assert len(messages) == 3
     keyword = 'ArgSizeMismatchRule'
@@ -113,13 +116,15 @@ def test_arg_size_array_sequence(rules, frontend):
 real(kind=jphook) :: zhook_handle
 real, dimension(klon, 137) :: var4, var5
 real :: var6
-integer :: klev, ibl
+integer :: klev, ibl, iproma, iend
 
 if(lhook) call dr_hook('driver', 0, zhook_handle)
 
 klev = 137
 do ibl = 1, nblk
-   call kernel(klon, klev, var0(1,ibl), var1(1,1,ibl), var2(1, 1), var3(1), &
+   iproma = klon
+   iend = iproma
+   call kernel(klon, klev, var0(1,ibl), var1(1,1,ibl), var2(1:iend, 1), var3(1), &
                var4(1, 1), var5, var6, 1, .true.)
 enddo
 
@@ -173,12 +178,13 @@ def test_dynamic_ubound_checks(rules, frontend):
     """
 
     fcode = """
-subroutine kernel(klon, klev, nblk, var0, var1, var2)
+subroutine kernel(klon, klev, nblk, var0, var1, var2, var3, var4)
 use abort_mod
 implicit none
 integer, intent(in) :: klon, klev, nblk
 real, dimension(:,:,:), intent(inout) :: var0, var1
 real, dimension(:,:,:), intent(inout) :: var2
+real, intent(inout) :: var3(:,:), var4(:,:,:)
 
 if(ubound(var0, 1) < klon)then
   call abort('kernel: first dimension of var0 too short')
@@ -198,7 +204,11 @@ def test_dynamic_ubound_checks(rules, frontend):
   call abort('kernel: dimensions of var2 too short')
 endif
 
-call some_other_kernel(klon, klen, nblk, var0, var1, var2)
+if(ubound(var4, 1) < klon .and. ubound(var4, 2) < klev .and. ubound(var4, 3) < nblk)then
+  call abort('kernel: dimensions of var4 too short')
+endif
+
+call some_other_kernel(klon, klen, nblk, var0, var1, var2, var3, var4)
 
 end subroutine kernel
     """.strip()
@@ -211,11 +221,12 @@ def test_dynamic_ubound_checks(rules, frontend):
     _ = run_linter(kernel, [rules.DynamicUboundCheckRule], config={'fix': True}, handlers=[handler])
 
     # check rule violations
-    assert len(messages) == 2
+    assert len(messages) == 3
     assert all('DynamicUboundCheckRule' in msg for msg in messages)
 
     assert 'var0' in messages[0]
     assert 'var2' in messages[1]
+    assert 'var4' in messages[2]
 
     # check fixed subroutine
     routine = kernel['kernel']
@@ -228,5 +239,22 @@ def test_dynamic_ubound_checks(rules, frontend):
 
     assert all(s.name == d for s, d in zip(routine.variable_map['var0'].shape, shape))
     assert all(s.name == d for s, d in zip(routine.variable_map['var2'].shape, shape))
+    assert all(s.name == d for s, d in zip(routine.variable_map['var4'].shape, shape))
+
+    arg_names = ['klon', 'klev', 'nblk', 'var0', 'var1', 'var2', 'var3', 'var4']
+    assert [arg.name.lower() for arg in routine.arguments] == arg_names
+
+    # check that variable declarations have not been duplicated
+    declarations = FindNodes(VariableDeclaration).visit(routine.spec)
+    symbols = [s.name.lower() for decl in declarations for s in decl.symbols]
+    assert len(symbols) == 8
+    assert set(symbols) == {'klon', 'klev', 'nblk', 'var0', 'var1', 'var2', 'var3', 'var4'}
+
+    # check number of declarations and symbols per declarations
+    assert len(declarations) == 5
+    assert len(declarations[0].symbols) == 3
+    for decl in declarations[1:4]:
+        assert len(decl.symbols) == 1
+    assert len(declarations[4].symbols) == 2
 
     os.remove(kernel.path)