Refactor patterns in config to accept ast: AST

aibolit/config.py

@@ -169,17 +169,17 @@ def get_patterns_config() -> PatternsConfig:
                 {
                     'name': 'Var declaration distance for 5 lines',
                     'code': 'P20_5',
-                    'make': lambda: P20(5)  # type: ignore
+                    'make': lambda: P20(5)
                 },
                 {
                     'name': 'Var declaration distance for 7 lines',
                     'code': 'P20_7',
-                    'make': lambda: P20(7)  # type: ignore
+                    'make': lambda: P20(7)
                 },
                 {
                     'name': 'Var declaration distance for 11 lines',
                     'code': 'P20_11',
-                    'make': lambda: P20(11)  # type: ignore
+                    'make': lambda: P20(11)
                 },
                 {'name': 'Var in the middle', 'code': 'P21', 'make': P21},
                 {'name': 'Array as function argument', 'code': 'P22', 'make': P22},
@@ -188,7 +188,7 @@ def get_patterns_config() -> PatternsConfig:
                 {'name': 'Private static method', 'code': 'P25', 'make': P25},
                 {'name': 'Public static method', 'code': 'P26', 'make': P26},
                 {'name': 'Var siblings', 'code': 'P27', 'make': P27},
-                {'name': 'Null Assignment', 'code': 'P28', 'make': P28},  # type: ignore
+                {'name': 'Null Assignment', 'code': 'P28', 'make': P28},
                 {'name': 'Multiple While', 'code': 'P29', 'make': P29},
                 {'name': 'Protected Method', 'code': 'P30', 'make': P30},
                 {'name': 'Send Null', 'code': 'P31', 'make': P31},
@@ -233,9 +233,7 @@ def get_patterns_config() -> PatternsConfig:
 
             },
             'patterns_exclude': [
-                'P27',  # empty implementation
-                'P20_5', 'P20_7', 'P20_11',  # wasn't refactored yet
-                'P28', 'P9',  # patterns based on text cannot accept arbitrary AST
+                'P9',  # patterns based on text cannot accept arbitrary AST
             ],
             'metrics_exclude': ['M1', 'M3_1', 'M3_2', 'M3_3', 'M3_4', 'M5', 'M7', 'M8']
         }

aibolit/patterns/assign_null_finder/assign_null_finder.py

@@ -1,15 +1,23 @@
 # SPDX-FileCopyrightText: Copyright (c) 2019-2025 Aibolit
 # SPDX-License-Identifier: MIT
-
-import re
 from typing import List
 
-from aibolit.utils.encoding_detector import read_text_with_autodetected_encoding
+from aibolit.ast_framework.ast import AST
+from aibolit.ast_framework.ast_node import ASTNode
+from aibolit.ast_framework.ast_node_type import ASTNodeType
+from aibolit.types_decl import LineNumber
 
 
 class NullAssignment:
-    def value(self, filename: str) -> List:
-        source_code = read_text_with_autodetected_encoding(filename)
-        pattern = r'[^=!><]=(\s)*null(\s)*;'
-        return [lineIndex + 1 for lineIndex, line in
-                enumerate(source_code.split('\n')) if re.search(pattern, line)]
+    def value(self, ast: AST) -> List[LineNumber]:
+        lines = set()
+        for assignment in ast.get_proxy_nodes(ASTNodeType.ASSIGNMENT):
+            if self._is_null_literal(assignment.value):
+                lines.add(assignment.line)
+        for declarator in ast.get_proxy_nodes(ASTNodeType.VARIABLE_DECLARATOR):
+            if self._is_null_literal(declarator.initializer):
+                lines.add(declarator.line)
+        return sorted(lines)
+
+    def _is_null_literal(self, node: ASTNode | None) -> bool:
+        return node is not None and node.node_type == ASTNodeType.LITERAL and node.value == 'null'

aibolit/patterns/var_decl_diff/var_decl_diff.py

@@ -1,13 +1,11 @@
 # SPDX-FileCopyrightText: Copyright (c) 2019-2025 Aibolit
 # SPDX-License-Identifier: MIT
+from typing import List, Optional
 
-
-from typing import List, Optional, Tuple, Dict
-
-import javalang
-import javalang.tree
-
-from aibolit.utils.java_parser import JavalangImproved, ASTNode
+from aibolit.ast_framework.ast import AST
+from aibolit.ast_framework.ast_node import ASTNode
+from aibolit.ast_framework.ast_node_type import ASTNodeType
+from aibolit.types_decl import LineNumber
 
 
 class VarDeclarationDistance:
@@ -25,75 +23,59 @@ def __node_name(self, node) -> Optional[str]:
         name = node.name if hasattr(node, 'name') else None
         return qualifier or member or name
 
-    def __group_vars_by_method(self, items: List[Tuple[ASTNode, Optional[str]]]) -> List[Dict]:
-        """
-        Group variables by method scope and calculate for each the declaration
-        line and first usage line
-        """
-        var_scopes: List[Dict] = []
-        vars: Dict = {}
-        unique_methods = list(set(
-            map(lambda v: v[0].method_line, items)
-        ))
-
-        for method in unique_methods:
-            # Filter items for current method to avoid cell-var-from-loop issue
-            method_filtered_items = [item for item in items if item[0].method_line == method]
-            method_items = map(
-                lambda v: {'line': v[0].line, 'name': v[1], 'ntype': type(v[0].node)},
-                method_filtered_items
-            )
-            vars = {}
-            var_scopes += [vars]
-            for item in method_items:
-                if item['ntype'] in [javalang.tree.MethodDeclaration]:
-                    vars = {}
-                    var_scopes += [vars]
-                    continue
-
-                if item['ntype'] == javalang.tree.VariableDeclarator:
-                    vars[item['name']] = {'decl': item['line'], 'first_usage': None}
-                    continue
-
-                if item['ntype'] == javalang.tree.VariableDeclarator:
-                    vars[item['name']] = {'decl': item['line']}
-                    continue
-
-                if item['name'] in vars and vars[item['name']]['first_usage'] is None:
-                    vars[item['name']]['first_usage'] = item['line']
-
-        return var_scopes
-
-    def __line_diff(self, usage_line: int, declaration_line: int, empty_lines: List[int]) -> int:
+    def __line_diff(self, usage_line: int, declaration_line: int, empty_lines: set[int]) -> int:
         """
         Calculate line difference between variable declaration and first usage
         taking into account empty lines
         """
         lines_range = set(range(declaration_line + 1, usage_line))
         return len(lines_range.difference(empty_lines))
 
-    def value(self, filename: str) -> List[int]:
-        """Find variables declared far from their first usage."""
-        tree = JavalangImproved(filename)
-        empty_lines = tree.get_empty_lines()
-        items = list(
-            map(lambda v: (v, self.__node_name(v.node)), tree.tree_to_nodes())
-        )
-        var_scopes = self.__group_vars_by_method(items)
-        violations = []
-        for scope in var_scopes:
-            for var, var_data in scope.items():
-                if var_data['first_usage'] is None:
-                    continue
+    def __find_empty_lines_in_ast(self, ast: AST) -> set[LineNumber]:
+        """Figure out lines that are either empty or multiline statements"""
+
+        lines_with_nodes = set()
+        ast.traverse(lambda node: lines_with_nodes.add(node.line))
+        max_line = max(lines_with_nodes)
+        all_lines = set(range(1, max_line + 1))
+        return all_lines.difference(lines_with_nodes)
+
+    def __value_for_method_declaration(
+        self, method_declaration: AST, empty_lines: set[LineNumber]
+    ) -> set[LineNumber]:
+        """Find variables declared far from their first usage in the method."""
+        result = set()
+        name_to_declaration_line = {}
+        name_to_first_usage_line = {}
+
+        def collect_declaration_or_usage(node: ASTNode):
+            node_name = self.__node_name(node)
+            if node_name:
+                if node.node_type == ASTNodeType.VARIABLE_DECLARATOR:
+                    name_to_declaration_line[node_name] = node.line
+                elif (
+                    node_name in name_to_declaration_line and
+                    node_name not in name_to_first_usage_line
+                ):
+                    name_to_first_usage_line[node_name] = node.line
+
+        method_declaration.traverse(collect_declaration_or_usage)
+        for name, usage_line in name_to_first_usage_line.items():
+            line_diff = self.__line_diff(
+                usage_line,
+                name_to_declaration_line[name],
+                empty_lines,
+            )
+            if line_diff >= self.__lines_th:
+                result.add(usage_line)
+        return result
 
-                line_diff = self.__line_diff(
-                    var_data['first_usage'],
-                    var_data['decl'],
-                    empty_lines
-                )
-                if line_diff < self.__lines_th:
-                    continue
+    def value(self, ast: AST) -> List[LineNumber]:
+        """Find variables declared far from their first usage."""
 
-                violations.append(var_data['first_usage'])
+        result = set()
+        empty_lines = self.__find_empty_lines_in_ast(ast)
+        for declaration in ast.get_subtrees(ASTNodeType.METHOD_DECLARATION):
+            result.update(self.__value_for_method_declaration(declaration, empty_lines))
 
-        return violations
+        return sorted(result)

test/config/test_config.py

@@ -27,18 +27,7 @@ def test_each_metric_in_config_accepts_ast():
         assert metric_signature.parameters['ast'].annotation is AST
 
 
-@pytest.mark.xfail
 def test_each_pattern_in_config_accepts_ast():
-    '''
-    TODO #813:30min/DEV Ensure All Patterns Accept `ast: AST` Parameter with Type Hints
-    Verify that all patterns in the config accept an AST parameter with proper type hints.
-    Probable solutions:
-    1. Every pattern factory in patterns_config["patterns"] produces a pattern with:
-       - A parameter named "ast" in its call signature
-       - The "ast" parameter properly annotated as `aibolit.ast_framework.ast.AST` or a subclass
-    2. Remove any patterns that cannot comply with this interface.
-    Once all patterns meet requirements, remove the decorator.
-    '''
     patterns_config = Config.get_patterns_config()
     for pattern_config in patterns_config['patterns']:
         pattern = pattern_config['make']()

test/integration/test_patterns_and_metrics.py

@@ -11,13 +11,7 @@
 from aibolit.ast_framework import AST
 from aibolit.utils.ast_builder import build_ast
 
-# TO-FIX: refactor or delete following patterns and metrics
-PATTERNS_ACCEPT_FILE_PATH = {
-    'P20_5',
-    'P20_7',
-    'P20_11',  # P20* wasn't refactored yet
-    'P28',  # patterns based on text cannot accept arbitrary AST
-}
+# TO-FIX: refactor or delete following metrics
 METRICS_ACCEPT_FILE_PATH = {'M1', 'M3_1', 'M3_2', 'M3_3', 'M3_4', 'M5', 'M7'}
 
 samples_path = Path(__file__).absolute().parent / 'samples'
@@ -26,11 +20,8 @@
 def _check_pattern(p_info, filepath):
     try:
         pattern = p_info['make']()
-        if p_info['code'] in PATTERNS_ACCEPT_FILE_PATH:
-            pattern_result = pattern.value(filepath)
-        else:
-            ast = AST.build_from_javalang(build_ast(filepath))
-            pattern_result = pattern.value(ast)
+        ast = AST.build_from_javalang(build_ast(filepath))
+        pattern_result = pattern.value(ast)
 
         assert isinstance(pattern_result, list) and all(
             isinstance(item, int) for item in pattern_result

test/patterns/test_assign_null/test_find_assign_null.py

@@ -3,20 +3,25 @@
 
 import os
 from unittest import TestCase
+from aibolit.ast_framework.ast import AST
 from aibolit.patterns.assign_null_finder.assign_null_finder import NullAssignment
+from aibolit.utils.ast_builder import build_ast
 
 
 class NullAssignmentTestCase(TestCase):
     cur_dir = os.path.dirname(os.path.realpath(__file__))
 
     def test_several(self):
-        lines = NullAssignment().value(self.cur_dir + '/several.java')
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/several.java'))
+        lines = NullAssignment().value(ast)
         self.assertEqual(lines, [8, 9, 14, 18, 24, 25])
 
     def test_one(self):
-        lines = NullAssignment().value(self.cur_dir + '/one.java')
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/one.java'))
+        lines = NullAssignment().value(ast)
         self.assertEqual(lines, [11])
 
     def test_not_null(self):
-        lines = NullAssignment().value(self.cur_dir + '/not_null.java')
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/not_null.java'))
+        lines = NullAssignment().value(ast)
         self.assertEqual(lines, [])

test/patterns/var_decl_diff/test_var_decl_diff.py

@@ -3,26 +3,34 @@
 
 import os
 from unittest import TestCase
+from aibolit.ast_framework.ast import AST
 from aibolit.patterns.var_decl_diff.var_decl_diff import VarDeclarationDistance
+from aibolit.utils.ast_builder import build_ast
 
 
 class VarDeclarationDiffTestCase(TestCase):
+    cur_dir = os.path.dirname(os.path.realpath(__file__))
+
     def test_good_class(self):
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/1.java'))
         pattern = VarDeclarationDistance(lines_th=2)
-        lines = pattern.value(os.path.dirname(os.path.realpath(__file__)) + '/1.java')
+        lines = pattern.value(ast)
         self.assertEqual(lines, [])
 
     def test_bad_class(self):
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/2.java'))
         pattern = VarDeclarationDistance(lines_th=2)
-        lines = pattern.value(os.path.dirname(os.path.realpath(__file__)) + '/2.java')
+        lines = pattern.value(ast)
         self.assertEqual(lines, [16])
 
     def test_bad_class2(self):
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/3.java'))
         pattern = VarDeclarationDistance(lines_th=5)
-        lines = pattern.value(os.path.dirname(os.path.realpath(__file__)) + '/3.java')
+        lines = pattern.value(ast)
         self.assertEqual(sorted(lines), [216, 785, 971])
 
     def test_case_with_multiline_function_arguments(self):
+        ast = AST.build_from_javalang(build_ast(self.cur_dir + '/4.java'))
         pattern = VarDeclarationDistance(lines_th=2)
-        lines = pattern.value(os.path.dirname(os.path.realpath(__file__)) + '/4.java')
+        lines = pattern.value(ast)
         self.assertEqual(lines, [17, 21])