From 11be4fac9b682c303b08ae3bdd34f8d94784ceae Mon Sep 17 00:00:00 2001 From: Martin Medler Date: Sun, 10 Sep 2023 15:42:07 +0200 Subject: [PATCH] Move Result class into independent file We prefer more but shortish and specialized files. --- src/analyze_includes/BUILD | 1 + src/analyze_includes/evaluate_includes.py | 72 +---- src/analyze_includes/result.py | 72 +++++ src/analyze_includes/test/BUILD | 6 + .../test/evaluate_includes_test.py | 266 ----------------- src/analyze_includes/test/result_test.py | 274 ++++++++++++++++++ 6 files changed, 355 insertions(+), 336 deletions(-) create mode 100644 src/analyze_includes/result.py create mode 100644 src/analyze_includes/test/result_test.py diff --git a/src/analyze_includes/BUILD b/src/analyze_includes/BUILD index af9c5c9d..c868abaf 100644 --- a/src/analyze_includes/BUILD +++ b/src/analyze_includes/BUILD @@ -7,6 +7,7 @@ py_library( "evaluate_includes.py", "parse_config.py", "parse_source.py", + "result.py", "std_header.py", "system_under_inspection.py", ], diff --git a/src/analyze_includes/evaluate_includes.py b/src/analyze_includes/evaluate_includes.py index 3fb338eb..225a41b7 100644 --- a/src/analyze_includes/evaluate_includes.py +++ b/src/analyze_includes/evaluate_includes.py @@ -1,10 +1,8 @@ -from collections import defaultdict -from dataclasses import dataclass, field -from json import dumps from pathlib import Path -from typing import DefaultDict, List +from typing import List from src.analyze_includes.parse_source import Include +from src.analyze_includes.result import Result from src.analyze_includes.system_under_inspection import ( CcTarget, SystemUnderInspection, @@ -12,72 +10,6 @@ ) -@dataclass -class Result: - target: str - public_includes_without_dep: List[Include] = field(default_factory=list) - private_includes_without_dep: List[Include] = field(default_factory=list) - unused_deps: List[str] = field(default_factory=list) - unused_implementation_deps: List[str] = field(default_factory=list) - deps_which_should_be_private: List[str] = field(default_factory=list) - use_implementation_deps: bool = False - - def is_ok(self) -> bool: - return ( - len(self.public_includes_without_dep) == 0 - and len(self.private_includes_without_dep) == 0 - and len(self.unused_deps) == 0 - and len(self.unused_implementation_deps) == 0 - and len(self.deps_which_should_be_private) == 0 - ) - - def to_str(self) -> str: - msg = f"DWYU analyzing: '{self.target}'\n\n" - if self.is_ok(): - msg += "Result: SUCCESS" - return self._framed_msg(msg) - - msg += "Result: FAILURE\n" - if self.public_includes_without_dep or self.private_includes_without_dep: - msg += "\nIncludes which are not available from the direct dependencies:\n" - msg += "\n".join(f" {inc}" for inc in self.public_includes_without_dep + self.private_includes_without_dep) - if self.unused_deps: - msg += "\nUnused dependencies in 'deps' (none of their headers are referenced):\n" - msg += "\n".join(f" Dependency='{dep}'" for dep in self.unused_deps) - if self.unused_implementation_deps: - msg += "\nUnused dependencies in 'implementation_deps' (none of their headers are referenced):\n" - msg += "\n".join(f" Dependency='{dep}'" for dep in self.unused_implementation_deps) - if self.deps_which_should_be_private: - msg += "\nPublic dependencies which are used only in private code:\n" - msg += "\n".join(f" Dependency='{dep}'" for dep in self.deps_which_should_be_private) - return self._framed_msg(msg) - - def to_json(self) -> str: - content = { - "analyzed_target": self.target, - "public_includes_without_dep": self._make_includes_map(self.public_includes_without_dep), - "private_includes_without_dep": self._make_includes_map(self.private_includes_without_dep), - "unused_deps": self.unused_deps, - "unused_implementation_deps": self.unused_implementation_deps, - "deps_which_should_be_private": self.deps_which_should_be_private, - "use_implementation_deps": self.use_implementation_deps, - } - return dumps(content, indent=2) + "\n" - - @staticmethod - def _make_includes_map(includes: List[Include]) -> DefaultDict[str, List[str]]: - includes_mapping = defaultdict(list) - for inc in includes: - includes_mapping[str(inc.file)].append(inc.include) - return includes_mapping - - @staticmethod - def _framed_msg(msg: str) -> str: - """Put a msg vertically between 2 borders""" - border = 80 * "=" - return border + "\n" + msg + "\n" + border - - def does_include_match_available_files( include_statement: str, include_paths: List[str], header_files: List[str] ) -> bool: diff --git a/src/analyze_includes/result.py b/src/analyze_includes/result.py new file mode 100644 index 00000000..deccfbe3 --- /dev/null +++ b/src/analyze_includes/result.py @@ -0,0 +1,72 @@ +from collections import defaultdict +from dataclasses import dataclass, field +from json import dumps +from typing import DefaultDict, List + +from src.analyze_includes.parse_source import Include + + +@dataclass +class Result: + target: str + public_includes_without_dep: List[Include] = field(default_factory=list) + private_includes_without_dep: List[Include] = field(default_factory=list) + unused_deps: List[str] = field(default_factory=list) + unused_implementation_deps: List[str] = field(default_factory=list) + deps_which_should_be_private: List[str] = field(default_factory=list) + use_implementation_deps: bool = False + + def is_ok(self) -> bool: + return ( + len(self.public_includes_without_dep) == 0 + and len(self.private_includes_without_dep) == 0 + and len(self.unused_deps) == 0 + and len(self.unused_implementation_deps) == 0 + and len(self.deps_which_should_be_private) == 0 + ) + + def to_str(self) -> str: + msg = f"DWYU analyzing: '{self.target}'\n\n" + if self.is_ok(): + msg += "Result: SUCCESS" + return self._framed_msg(msg) + + msg += "Result: FAILURE\n" + if self.public_includes_without_dep or self.private_includes_without_dep: + msg += "\nIncludes which are not available from the direct dependencies:\n" + msg += "\n".join(f" {inc}" for inc in self.public_includes_without_dep + self.private_includes_without_dep) + if self.unused_deps: + msg += "\nUnused dependencies in 'deps' (none of their headers are referenced):\n" + msg += "\n".join(f" Dependency='{dep}'" for dep in self.unused_deps) + if self.unused_implementation_deps: + msg += "\nUnused dependencies in 'implementation_deps' (none of their headers are referenced):\n" + msg += "\n".join(f" Dependency='{dep}'" for dep in self.unused_implementation_deps) + if self.deps_which_should_be_private: + msg += "\nPublic dependencies which are used only in private code:\n" + msg += "\n".join(f" Dependency='{dep}'" for dep in self.deps_which_should_be_private) + return self._framed_msg(msg) + + def to_json(self) -> str: + content = { + "analyzed_target": self.target, + "public_includes_without_dep": self._make_includes_map(self.public_includes_without_dep), + "private_includes_without_dep": self._make_includes_map(self.private_includes_without_dep), + "unused_deps": self.unused_deps, + "unused_implementation_deps": self.unused_implementation_deps, + "deps_which_should_be_private": self.deps_which_should_be_private, + "use_implementation_deps": self.use_implementation_deps, + } + return dumps(content, indent=2) + "\n" + + @staticmethod + def _make_includes_map(includes: List[Include]) -> DefaultDict[str, List[str]]: + includes_mapping = defaultdict(list) + for inc in includes: + includes_mapping[str(inc.file)].append(inc.include) + return includes_mapping + + @staticmethod + def _framed_msg(msg: str) -> str: + """Put a msg vertically between 2 borders""" + border = 80 * "=" + return border + "\n" + msg + "\n" + border diff --git a/src/analyze_includes/test/BUILD b/src/analyze_includes/test/BUILD index 6ed7b6ac..5a08974d 100644 --- a/src/analyze_includes/test/BUILD +++ b/src/analyze_includes/test/BUILD @@ -33,6 +33,12 @@ py_test( deps = ["//src/analyze_includes:lib"], ) +py_test( + name = "result_test", + srcs = ["result_test.py"], + deps = ["//src/analyze_includes:lib"], +) + py_test( name = "system_under_inspection_test", srcs = ["system_under_inspection_test.py"], diff --git a/src/analyze_includes/test/evaluate_includes_test.py b/src/analyze_includes/test/evaluate_includes_test.py index 2478ac77..4137aa3a 100644 --- a/src/analyze_includes/test/evaluate_includes_test.py +++ b/src/analyze_includes/test/evaluate_includes_test.py @@ -2,7 +2,6 @@ from pathlib import Path from src.analyze_includes.evaluate_includes import ( - Result, does_include_match_available_files, evaluate_includes, ) @@ -10,271 +9,6 @@ from src.analyze_includes.system_under_inspection import CcTarget, SystemUnderInspection -class TestResult(unittest.TestCase): - @staticmethod - def _expected_msg(target: str, errors: str = "") -> str: - border = 80 * "=" - msg = f"DWYU analyzing: '{target}'\n\n" - if errors: - msg += "Result: FAILURE\n\n" - else: - msg += "Result: SUCCESS" - return border + "\n" + msg + errors + "\n" + border - - def test_is_ok(self): - unit = Result("//foo:bar") - - self.assertTrue(unit.is_ok()) - self.assertEqual(unit.to_str(), self._expected_msg(target="//foo:bar")) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": {}, - "private_includes_without_dep": {}, - "unused_deps": [], - "unused_implementation_deps": [], - "deps_which_should_be_private": [], - "use_implementation_deps": false -} -""".lstrip(), - ) - - def test_is_ok_fails_due_to_invalid_private_includes(self): - unit = Result( - target="//foo:bar", - private_includes_without_dep=[ - Include(file=Path("foo"), include="missing_1"), - Include(file=Path("bar"), include="missing_2"), - Include(file=Path("bar"), include="missing_3"), - ], - ) - - self.assertFalse(unit.is_ok()) - self.assertEqual( - unit.to_str(), - self._expected_msg( - target="//foo:bar", - errors="Includes which are not available from the direct dependencies:\n" - " File='foo', include='missing_1'\n" - " File='bar', include='missing_2'\n" - " File='bar', include='missing_3'", - ), - ) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": {}, - "private_includes_without_dep": { - "foo": [ - "missing_1" - ], - "bar": [ - "missing_2", - "missing_3" - ] - }, - "unused_deps": [], - "unused_implementation_deps": [], - "deps_which_should_be_private": [], - "use_implementation_deps": false -} -""".lstrip(), - ) - - def test_is_ok_fails_due_to_invalid_public_includes(self): - unit = Result( - target="//foo:bar", - public_includes_without_dep=[ - Include(file=Path("foo"), include="missing_1"), - Include(file=Path("bar"), include="missing_2"), - Include(file=Path("bar"), include="missing_3"), - ], - ) - - self.assertFalse(unit.is_ok()) - self.assertEqual( - unit.to_str(), - self._expected_msg( - target="//foo:bar", - errors="Includes which are not available from the direct dependencies:\n" - " File='foo', include='missing_1'\n" - " File='bar', include='missing_2'\n" - " File='bar', include='missing_3'", - ), - ) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": { - "foo": [ - "missing_1" - ], - "bar": [ - "missing_2", - "missing_3" - ] - }, - "private_includes_without_dep": {}, - "unused_deps": [], - "unused_implementation_deps": [], - "deps_which_should_be_private": [], - "use_implementation_deps": false -} -""".lstrip(), - ) - - def test_is_ok_fails_due_to_unused_public_deps(self): - unit = Result(target="//foo:bar", unused_deps=["foo", "baz"]) - - self.assertFalse(unit.is_ok()) - self.assertEqual( - unit.to_str(), - self._expected_msg( - target="//foo:bar", - errors="Unused dependencies in 'deps' (none of their headers are referenced):\n" - " Dependency='foo'\n" - " Dependency='baz'", - ), - ) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": {}, - "private_includes_without_dep": {}, - "unused_deps": [ - "foo", - "baz" - ], - "unused_implementation_deps": [], - "deps_which_should_be_private": [], - "use_implementation_deps": false -} -""".lstrip(), - ) - - def test_is_ok_fails_due_to_unused_private_deps(self): - unit = Result(target="//foo:bar", unused_implementation_deps=["foo", "baz"]) - - self.assertFalse(unit.is_ok()) - self.assertEqual( - unit.to_str(), - self._expected_msg( - target="//foo:bar", - errors="Unused dependencies in 'implementation_deps' (none of their headers are referenced):\n" - " Dependency='foo'\n" - " Dependency='baz'", - ), - ) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": {}, - "private_includes_without_dep": {}, - "unused_deps": [], - "unused_implementation_deps": [ - "foo", - "baz" - ], - "deps_which_should_be_private": [], - "use_implementation_deps": false -} -""".lstrip(), - ) - - def test_is_ok_fails_due_to_unused_public_and_private_deps(self): - unit = Result(target="//foo:bar", unused_deps=["foo"], unused_implementation_deps=["baz"]) - - self.assertFalse(unit.is_ok()) - self.assertEqual( - unit.to_str(), - self._expected_msg( - target="//foo:bar", - errors="Unused dependencies in 'deps' (none of their headers are referenced):\n" - " Dependency='foo'\n" - "Unused dependencies in 'implementation_deps' (none of their headers are referenced):\n" - " Dependency='baz'", - ), - ) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": {}, - "private_includes_without_dep": {}, - "unused_deps": [ - "foo" - ], - "unused_implementation_deps": [ - "baz" - ], - "deps_which_should_be_private": [], - "use_implementation_deps": false -} -""".lstrip(), - ) - - def test_is_ok_fails_due_to_deps_which_should_be_private(self): - unit = Result(target="//foo:bar", deps_which_should_be_private=["foo", "baz"]) - - self.assertFalse(unit.is_ok()) - self.assertEqual( - unit.to_str(), - self._expected_msg( - target="//foo:bar", - errors="Public dependencies which are used only in private code:\n" - " Dependency='foo'\n" - " Dependency='baz'", - ), - ) - self.assertEqual( - unit.to_json(), - """ -{ - "analyzed_target": "//foo:bar", - "public_includes_without_dep": {}, - "private_includes_without_dep": {}, - "unused_deps": [], - "unused_implementation_deps": [], - "deps_which_should_be_private": [ - "foo", - "baz" - ], - "use_implementation_deps": false -} -""".lstrip(), - ) - - -def test_set_use_implementation_deps(self): - unit = Result(target="//:foo", use_implementation_deps=True) - - self.assertEqual( - unit.to_json(), - """ -{ -"analyzed_target": "//:foo", -"public_includes_without_dep": {}, -"private_includes_without_dep": {}, -"unused_deps": [], -"unused_implementation_deps": [], -"deps_which_should_be_private": [], -"use_implementation_deps": false -} -""".lstrip(), - ) - - class TestIncludeTofileMatching(unittest.TestCase): def test_match_with_standard_include_path(self): self.assertTrue( diff --git a/src/analyze_includes/test/result_test.py b/src/analyze_includes/test/result_test.py new file mode 100644 index 00000000..63943103 --- /dev/null +++ b/src/analyze_includes/test/result_test.py @@ -0,0 +1,274 @@ +import unittest +from pathlib import Path + +from src.analyze_includes.parse_source import Include +from src.analyze_includes.result import Result + + +class TestResult(unittest.TestCase): + @staticmethod + def _expected_msg(target: str, errors: str = "") -> str: + border = 80 * "=" + msg = f"DWYU analyzing: '{target}'\n\n" + if errors: + msg += "Result: FAILURE\n\n" + else: + msg += "Result: SUCCESS" + return border + "\n" + msg + errors + "\n" + border + + def test_is_ok(self): + unit = Result("//foo:bar") + + self.assertTrue(unit.is_ok()) + self.assertEqual(unit.to_str(), self._expected_msg(target="//foo:bar")) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": {}, + "private_includes_without_dep": {}, + "unused_deps": [], + "unused_implementation_deps": [], + "deps_which_should_be_private": [], + "use_implementation_deps": false +} +""".lstrip(), + ) + + def test_is_ok_fails_due_to_invalid_private_includes(self): + unit = Result( + target="//foo:bar", + private_includes_without_dep=[ + Include(file=Path("foo"), include="missing_1"), + Include(file=Path("bar"), include="missing_2"), + Include(file=Path("bar"), include="missing_3"), + ], + ) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + target="//foo:bar", + errors="Includes which are not available from the direct dependencies:\n" + " File='foo', include='missing_1'\n" + " File='bar', include='missing_2'\n" + " File='bar', include='missing_3'", + ), + ) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": {}, + "private_includes_without_dep": { + "foo": [ + "missing_1" + ], + "bar": [ + "missing_2", + "missing_3" + ] + }, + "unused_deps": [], + "unused_implementation_deps": [], + "deps_which_should_be_private": [], + "use_implementation_deps": false +} +""".lstrip(), + ) + + def test_is_ok_fails_due_to_invalid_public_includes(self): + unit = Result( + target="//foo:bar", + public_includes_without_dep=[ + Include(file=Path("foo"), include="missing_1"), + Include(file=Path("bar"), include="missing_2"), + Include(file=Path("bar"), include="missing_3"), + ], + ) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + target="//foo:bar", + errors="Includes which are not available from the direct dependencies:\n" + " File='foo', include='missing_1'\n" + " File='bar', include='missing_2'\n" + " File='bar', include='missing_3'", + ), + ) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": { + "foo": [ + "missing_1" + ], + "bar": [ + "missing_2", + "missing_3" + ] + }, + "private_includes_without_dep": {}, + "unused_deps": [], + "unused_implementation_deps": [], + "deps_which_should_be_private": [], + "use_implementation_deps": false +} +""".lstrip(), + ) + + def test_is_ok_fails_due_to_unused_public_deps(self): + unit = Result(target="//foo:bar", unused_deps=["foo", "baz"]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + target="//foo:bar", + errors="Unused dependencies in 'deps' (none of their headers are referenced):\n" + " Dependency='foo'\n" + " Dependency='baz'", + ), + ) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": {}, + "private_includes_without_dep": {}, + "unused_deps": [ + "foo", + "baz" + ], + "unused_implementation_deps": [], + "deps_which_should_be_private": [], + "use_implementation_deps": false +} +""".lstrip(), + ) + + def test_is_ok_fails_due_to_unused_private_deps(self): + unit = Result(target="//foo:bar", unused_implementation_deps=["foo", "baz"]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + target="//foo:bar", + errors="Unused dependencies in 'implementation_deps' (none of their headers are referenced):\n" + " Dependency='foo'\n" + " Dependency='baz'", + ), + ) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": {}, + "private_includes_without_dep": {}, + "unused_deps": [], + "unused_implementation_deps": [ + "foo", + "baz" + ], + "deps_which_should_be_private": [], + "use_implementation_deps": false +} +""".lstrip(), + ) + + def test_is_ok_fails_due_to_unused_public_and_private_deps(self): + unit = Result(target="//foo:bar", unused_deps=["foo"], unused_implementation_deps=["baz"]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + target="//foo:bar", + errors="Unused dependencies in 'deps' (none of their headers are referenced):\n" + " Dependency='foo'\n" + "Unused dependencies in 'implementation_deps' (none of their headers are referenced):\n" + " Dependency='baz'", + ), + ) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": {}, + "private_includes_without_dep": {}, + "unused_deps": [ + "foo" + ], + "unused_implementation_deps": [ + "baz" + ], + "deps_which_should_be_private": [], + "use_implementation_deps": false +} +""".lstrip(), + ) + + def test_is_ok_fails_due_to_deps_which_should_be_private(self): + unit = Result(target="//foo:bar", deps_which_should_be_private=["foo", "baz"]) + + self.assertFalse(unit.is_ok()) + self.assertEqual( + unit.to_str(), + self._expected_msg( + target="//foo:bar", + errors="Public dependencies which are used only in private code:\n" + " Dependency='foo'\n" + " Dependency='baz'", + ), + ) + self.assertEqual( + unit.to_json(), + """ +{ + "analyzed_target": "//foo:bar", + "public_includes_without_dep": {}, + "private_includes_without_dep": {}, + "unused_deps": [], + "unused_implementation_deps": [], + "deps_which_should_be_private": [ + "foo", + "baz" + ], + "use_implementation_deps": false +} +""".lstrip(), + ) + + +def test_set_use_implementation_deps(self): + unit = Result(target="//:foo", use_implementation_deps=True) + + self.assertEqual( + unit.to_json(), + """ +{ +"analyzed_target": "//:foo", +"public_includes_without_dep": {}, +"private_includes_without_dep": {}, +"unused_deps": [], +"unused_implementation_deps": [], +"deps_which_should_be_private": [], +"use_implementation_deps": false +} +""".lstrip(), + ) + + +if __name__ == "__main__": + unittest.main()