diff --git a/devtools/code_health/metrics.py b/devtools/code_health/metrics.py index f2d7a16ba5..81bed74623 100644 --- a/devtools/code_health/metrics.py +++ b/devtools/code_health/metrics.py @@ -14,10 +14,15 @@ class Metric: value: int limit: int line: int = 1 + # What the base revision is matched on, if not `symbol`. Functions match on + # their qualified name without parameters: changing a parameter's type + # (std::function to MoveOnlyFunction, #90) must not turn an old long + # function into a new one held to the limit. + name: str = None @property def key(self): - return (self.path, self.symbol, self.check) + return (self.path, self.name if self.name is not None else self.symbol, self.check) class BraceDepth: @@ -53,5 +58,5 @@ def measure(path, source): ) for check, value, limit in values: result.append(Metric(path, function.long_name, check, value, limit, - function.start_line)) + function.start_line, function.name)) return result diff --git a/devtools/code_health/tests/test_policy.py b/devtools/code_health/tests/test_policy.py index e525e2f2af..147e8324bf 100644 --- a/devtools/code_health/tests/test_policy.py +++ b/devtools/code_health/tests/test_policy.py @@ -1,6 +1,6 @@ import unittest -from devtools.code_health.metrics import Metric +from devtools.code_health.metrics import Metric, measure from devtools.code_health.policy import evaluate @@ -22,6 +22,13 @@ def test_new_function_must_meet_limit(self): def test_unchanged_bad_function_does_not_block_other_changes(self): self.assertEqual([], evaluate([function(70), function(25, 'new()')], [function(70)])) + def test_changed_parameter_types_keep_the_base(self): + old = measure('Common/test.cpp', 'void A::f(std::function f){\n' + 'x();\n' * 60 + '}\n') + new = measure('Common/test.cpp', 'void A::f(MoveOnlyFunction f){\n' + 'x();\n' * 60 + '}\n') + self.assertEqual([], evaluate(new, old)) + grown = measure('Common/test.cpp', 'void A::f(MoveOnlyFunction f){\n' + 'x();\n' * 61 + '}\n') + self.assertTrue(evaluate(grown, old)) + def test_growth_below_limit_is_allowed(self): self.assertEqual([], evaluate([function(50)], [function(25)]))