Add line numbers for metrics in Cyclomatic and Halstead operators (#218)

* Add line numbers for detailed metrics in Cyclomatic and Halstead operators.

* Fix typing of HalsteadOperator._report_to_dict().

* Add a test for metric fields and values after building, covers the new lineno and endline entries for Halstead and Cyclomatic.

* Guard against FuncDef missing end_lineno in Python 3.7.

* Silence a ruff error for checking Python version.

* Correctly check for Python version < 3.8.

* Add missing version check for test of Halstead endline.

---------

Co-authored-by: Anthony Shaw <[email protected]>

src/wily/operators/cyclomatic.py

@@ -106,6 +106,8 @@ def _dict_from_function(l: Function) -> Dict[str, Any]:
             "complexity": l.complexity,
             "fullname": l.fullname,
             "loc": l.endline - l.lineno,
+            "lineno": l.lineno,
+            "endline": l.endline,
         }
 
     @staticmethod
@@ -117,4 +119,6 @@ def _dict_from_class(l: Class) -> Dict[str, Any]:
             "complexity": l.complexity,
             "fullname": l.fullname,
             "loc": l.endline - l.lineno,
+            "lineno": l.lineno,
+            "endline": l.endline,
         }

src/wily/operators/halstead.py

@@ -3,17 +3,81 @@
 
 Measures all of the halstead metrics (volume, vocab, difficulty)
 """
+import ast
+import collections
 from typing import Any, Dict, Iterable
 
 import radon.cli.harvest as harvesters
 from radon.cli import Config
-from radon.metrics import HalsteadReport
+from radon.metrics import Halstead, HalsteadReport, halstead_visitor_report
+from radon.visitors import HalsteadVisitor
 
 from wily import logger
 from wily.config.types import WilyConfig
 from wily.lang import _
 from wily.operators import BaseOperator, Metric, MetricType
 
+NumberedHalsteadReport = collections.namedtuple(
+    "NumberedHalsteadReport",
+    HalsteadReport._fields + ("lineno", "endline"),
+)
+
+
+class NumberedHalsteadVisitor(HalsteadVisitor):
+    """HalsteadVisitor that adds class name, lineno and endline for code blocks."""
+
+    def __init__(self, context=None, lineno=None, endline=None, classname=None):
+        """
+        Initialize the numbered visitor.
+
+        :param context: Function/method name.
+        :param lineno: The starting line of the code block, if any.
+        :param endline: The ending line of the code block, if any.
+        :param classname: The class name for a method.
+        """
+        super().__init__(context)
+        self.lineno = lineno
+        self.endline = endline
+        self.class_name = classname
+
+    def visit_FunctionDef(self, node):
+        """Visit functions and methods, adding class name if any, lineno and endline."""
+        if self.class_name:
+            node.name = f"{self.class_name}.{node.name}"
+        super().visit_FunctionDef(node)
+        self.function_visitors[-1].lineno = node.lineno
+        # FuncDef is missing end_lineno in Python 3.7
+        endline = node.end_lineno if hasattr(node, "end_lineno") else None
+        self.function_visitors[-1].endline = endline
+
+    def visit_ClassDef(self, node):
+        """Visit classes, adding class name and creating visitors for methods."""
+        self.class_name = node.name
+        for child in node.body:
+            visitor = NumberedHalsteadVisitor(classname=self.class_name)
+            visitor.visit(child)
+            self.function_visitors.extend(visitor.function_visitors)
+        self.class_name = None
+
+
+def number_report(visitor):
+    """Create a report with added lineno and endline."""
+    return NumberedHalsteadReport(
+        *(halstead_visitor_report(visitor) + (visitor.lineno, visitor.endline))
+    )
+
+
+class NumberedHCHarvester(harvesters.HCHarvester):
+    """Version of HCHarvester that adds lineno and endline."""
+
+    def gobble(self, fobj):
+        """Analyze the content of the file object, adding line numbers for blocks."""
+        code = fobj.read()
+        visitor = NumberedHalsteadVisitor.from_ast(ast.parse(code))
+        total = number_report(visitor)
+        functions = [(v.context, number_report(v)) for v in visitor.function_visitors]
+        return Halstead(total, functions)
+
 
 class HalsteadOperator(BaseOperator):
     """Halstead Operator."""
@@ -58,7 +122,7 @@ def __init__(self, config: WilyConfig, targets: Iterable[str]):
         # TODO : Import config from wily.cfg
         logger.debug("Using %s with %s for HC metrics", targets, self.defaults)
 
-        self.harvester = harvesters.HCHarvester(targets, config=Config(**self.defaults))
+        self.harvester = NumberedHCHarvester(targets, config=Config(**self.defaults))
 
     def run(self, module: str, options: Dict[str, Any]) -> Dict[Any, Any]:
         """
@@ -76,7 +140,7 @@ def run(self, module: str, options: Dict[str, Any]) -> Dict[Any, Any]:
                 if isinstance(instance, list):
                     for item in instance:
                         function, report = item
-                        assert isinstance(report, HalsteadReport)
+                        assert isinstance(report, NumberedHalsteadReport)
                         results[filename]["detailed"][function] = self._report_to_dict(
                             report
                         )
@@ -88,11 +152,11 @@ def run(self, module: str, options: Dict[str, Any]) -> Dict[Any, Any]:
                             details["error"],
                         )
                         continue
-                    assert isinstance(instance, HalsteadReport)
+                    assert isinstance(instance, NumberedHalsteadReport)
                     results[filename]["total"] = self._report_to_dict(instance)
         return results
 
-    def _report_to_dict(self, report: HalsteadReport) -> Dict[str, Any]:
+    def _report_to_dict(self, report: NumberedHalsteadReport) -> Dict[str, Any]:
         return {
             "h1": report.h1,
             "h2": report.h2,
@@ -103,4 +167,6 @@ def _report_to_dict(self, report: HalsteadReport) -> Dict[str, Any]:
             "length": report.length,
             "effort": report.effort,
             "difficulty": report.difficulty,
+            "lineno": report.lineno,
+            "endline": report.endline,
         }

test/integration/test_complex_commits.py

@@ -112,3 +112,152 @@ def test_skip_files(tmpdir, cache_path):
     assert "raw" in data3["operator_data"]
     assert _path1 in data3["operator_data"]["raw"]
     assert _path2 in data3["operator_data"]["raw"]
+
+
+complex_test = """
+import abc
+foo = 1
+def function1():
+    a = 1 + 1
+    if a == 2:
+        print(1)
+class Class1(object):
+    def method(self):
+        b = 1 + 5
+        if b == 6:
+            if 1==2:
+               if 2==3:
+                  print(1)
+"""
+
+
+def test_metric_entries(tmpdir, cache_path):
+    """Test that the expected fields and values are present in metric results."""
+    repo = Repo.init(path=tmpdir)
+    tmppath = pathlib.Path(tmpdir) / "src"
+    tmppath.mkdir()
+
+    # Write and commit one test file to the repo
+    with open(tmppath / "test1.py", "w") as test1_txt:
+        test1_txt.write(complex_test)
+    index = repo.index
+    index.add([str(tmppath / "test1.py")])
+    author = Actor("An author", "[email protected]")
+    committer = Actor("A committer", "[email protected]")
+    commit = index.commit("commit one file", author=author, committer=committer)
+    repo.close()
+
+    # Build the wily cache
+    runner = CliRunner()
+    result = runner.invoke(
+        main.cli,
+        ["--debug", "--path", tmpdir, "--cache", cache_path, "build", str(tmppath)],
+    )
+    assert result.exit_code == 0, result.stdout
+
+    # Get the revision path and the revision data
+    cache_path = pathlib.Path(cache_path)
+    rev_path = cache_path / "git" / (commit.name_rev.split(" ")[0] + ".json")
+    assert rev_path.exists()
+    with open(rev_path) as rev_file:
+        data = json.load(rev_file)
+
+    # Check that basic data format is correct
+    assert "cyclomatic" in data["operator_data"]
+    assert _path1 in data["operator_data"]["cyclomatic"]
+    assert "detailed" in data["operator_data"]["cyclomatic"][_path1]
+    assert "total" in data["operator_data"]["cyclomatic"][_path1]
+
+    # Test total and detailed metrics
+    expected_cyclomatic_total = {"complexity": 11}
+    total_cyclomatic = data["operator_data"]["cyclomatic"][_path1]["total"]
+    assert total_cyclomatic == expected_cyclomatic_total
+
+    detailed_cyclomatic = data["operator_data"]["cyclomatic"][_path1]["detailed"]
+    assert "function1" in detailed_cyclomatic
+    assert "lineno" in detailed_cyclomatic["function1"]
+    assert "endline" in detailed_cyclomatic["function1"]
+    expected_cyclomatic_function1 = {
+        "name": "function1",
+        "is_method": False,
+        "classname": None,
+        "closures": [],
+        "complexity": 2,
+        "loc": 3,
+        "lineno": 4,
+        "endline": 7,
+    }
+    assert detailed_cyclomatic["function1"] == expected_cyclomatic_function1
+
+    expected_cyclomatic_Class1 = {
+        "name": "Class1",
+        "inner_classes": [],
+        "real_complexity": 5,
+        "complexity": 5,
+        "loc": 6,
+        "lineno": 8,
+        "endline": 14,
+    }
+    assert detailed_cyclomatic["Class1"] == expected_cyclomatic_Class1
+
+    expected_cyclomatic_method = {
+        "name": "method",
+        "is_method": True,
+        "classname": "Class1",
+        "closures": [],
+        "complexity": 4,
+        "loc": 5,
+        "lineno": 9,
+        "endline": 14,
+    }
+    assert detailed_cyclomatic["Class1.method"] == expected_cyclomatic_method
+
+    expected_halstead_total = {
+        "h1": 2,
+        "h2": 3,
+        "N1": 2,
+        "N2": 4,
+        "vocabulary": 5,
+        "volume": 13.931568569324174,
+        "length": 6,
+        "effort": 18.575424759098897,
+        "difficulty": 1.3333333333333333,
+        "lineno": None,
+        "endline": None,
+    }
+    total_halstead = data["operator_data"]["halstead"][_path1]["total"]
+    assert total_halstead == expected_halstead_total
+
+    detailed_halstead = data["operator_data"]["halstead"][_path1]["detailed"]
+    assert "function1" in detailed_halstead
+    assert "lineno" in detailed_halstead["function1"]
+    assert detailed_halstead["function1"]["lineno"] is not None
+    assert "endline" in detailed_halstead["function1"]
+    if sys.version_info >= (3, 8):
+        # FuncDef is missing end_lineno in Python 3.7
+        assert detailed_halstead["function1"]["endline"] is not None
+
+    assert "Class1" not in detailed_halstead
+
+    assert "Class1.method" in detailed_halstead
+    assert "lineno" in detailed_halstead["Class1.method"]
+    assert detailed_halstead["Class1.method"]["lineno"] is not None
+    assert "endline" in detailed_halstead["Class1.method"]
+    if sys.version_info >= (3, 8):
+        assert detailed_halstead["Class1.method"]["endline"] is not None
+
+    expected_raw_total = {
+        "loc": 14,
+        "lloc": 13,
+        "sloc": 13,
+        "comments": 0,
+        "multi": 0,
+        "blank": 1,
+        "single_comments": 0,
+    }
+    total_raw = data["operator_data"]["raw"][_path1]["total"]
+    assert total_raw == expected_raw_total
+
+    expected_maintainability = {"mi": 62.3299092923013, "rank": "A"}
+    total_maintainability = data["operator_data"]["maintainability"][_path1]["total"]
+    assert total_maintainability == expected_maintainability