diff --git a/src/analyze_includes/evaluate_includes.py b/src/analyze_includes/evaluate_includes.py index 225a41b7..e80fc4d4 100644 --- a/src/analyze_includes/evaluate_includes.py +++ b/src/analyze_includes/evaluate_includes.py @@ -102,7 +102,7 @@ def _filter_empty_dependencies(system_under_inspection: SystemUnderInspection) - return SystemUnderInspection( target_under_inspection=system_under_inspection.target_under_inspection, deps=[dep for dep in system_under_inspection.deps if dep.header_files], - implementation_deps=[dep for dep in system_under_inspection.implementation_deps if dep.header_files], + impl_deps=[dep for dep in system_under_inspection.impl_deps if dep.header_files], include_paths=system_under_inspection.include_paths, defines=system_under_inspection.defines, ) @@ -126,17 +126,17 @@ def evaluate_includes( ) result.private_includes_without_dep = _check_for_invalid_includes( includes=private_includes, - dependencies=system_under_inspection.deps + system_under_inspection.implementation_deps, + dependencies=system_under_inspection.deps + system_under_inspection.impl_deps, usage=UsageStatus.PRIVATE, target_under_inspection=system_under_inspection.target_under_inspection, include_paths=system_under_inspection.include_paths, ) result.unused_deps = _check_for_unused_dependencies(system_under_inspection.deps) - result.unused_implementation_deps = _check_for_unused_dependencies(system_under_inspection.implementation_deps) + result.unused_impl_deps = _check_for_unused_dependencies(system_under_inspection.impl_deps) if ensure_private_deps: result.deps_which_should_be_private = _check_for_public_deps_which_should_be_private(system_under_inspection) - result.use_implementation_deps = True + result.use_impl_deps = True return result diff --git a/src/analyze_includes/main.py b/src/analyze_includes/main.py index b19503c5..45b63389 100644 --- a/src/analyze_includes/main.py +++ b/src/analyze_includes/main.py @@ -73,7 +73,7 @@ def main(args: Namespace) -> int: system_under_inspection = get_system_under_inspection( target_under_inspection=args.target_under_inspection, deps=args.deps, - implementation_deps=args.implementation_deps, + impl_deps=args.implementation_deps, ) all_includes_from_public = get_relevant_includes_from_files( files=args.public_files, diff --git a/src/analyze_includes/result.py b/src/analyze_includes/result.py index deccfbe3..c9625056 100644 --- a/src/analyze_includes/result.py +++ b/src/analyze_includes/result.py @@ -12,16 +12,16 @@ class Result: 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) + unused_impl_deps: List[str] = field(default_factory=list) deps_which_should_be_private: List[str] = field(default_factory=list) - use_implementation_deps: bool = False + use_impl_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.unused_impl_deps) == 0 and len(self.deps_which_should_be_private) == 0 ) @@ -38,9 +38,9 @@ def to_str(self) -> str: 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: + if self.unused_impl_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) + msg += "\n".join(f" Dependency='{dep}'" for dep in self.unused_impl_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) @@ -52,9 +52,9 @@ def to_json(self) -> str: "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, + "unused_implementation_deps": self.unused_impl_deps, "deps_which_should_be_private": self.deps_which_should_be_private, - "use_implementation_deps": self.use_implementation_deps, + "use_implementation_deps": self.use_impl_deps, } return dumps(content, indent=2) + "\n" @@ -67,6 +67,6 @@ def _make_includes_map(includes: List[Include]) -> DefaultDict[str, List[str]]: @staticmethod def _framed_msg(msg: str) -> str: - """Put a msg vertically between 2 borders""" + """Put a message between 2 horizontal borders""" border = 80 * "=" return border + "\n" + msg + "\n" + border diff --git a/src/analyze_includes/system_under_inspection.py b/src/analyze_includes/system_under_inspection.py index 9ebf2424..f5d869cd 100644 --- a/src/analyze_includes/system_under_inspection.py +++ b/src/analyze_includes/system_under_inspection.py @@ -70,7 +70,7 @@ class SystemUnderInspection: deps: List[CcTarget] # Dependencies which are NOT available to downstream dependencies of the target under inspection. Exists only for # cc_library targets - implementation_deps: List[CcTarget] + impl_deps: List[CcTarget] # All include paths available to the target under inspection. Combines all kinds of includes. include_paths: List[str] # Defines influencing the preprocessor @@ -116,14 +116,14 @@ def _get_defines(target_info: Dict[str, Any]) -> List[str]: def get_system_under_inspection( - target_under_inspection: Path, deps: List[Path], implementation_deps: List[Path] + target_under_inspection: Path, deps: List[Path], impl_deps: List[Path] ) -> SystemUnderInspection: with open(target_under_inspection, encoding="utf-8") as target: target_info = json.load(target) return SystemUnderInspection( target_under_inspection=_make_cc_target(target_under_inspection), deps=_cc_targets_from_deps(deps), - implementation_deps=_cc_targets_from_deps(implementation_deps), + impl_deps=_cc_targets_from_deps(impl_deps), include_paths=_get_include_paths(target_info), defines=_get_defines(target_info), ) diff --git a/src/analyze_includes/test/evaluate_includes_test.py b/src/analyze_includes/test/evaluate_includes_test.py index 4137aa3a..84afa40b 100644 --- a/src/analyze_includes/test/evaluate_includes_test.py +++ b/src/analyze_includes/test/evaluate_includes_test.py @@ -74,7 +74,7 @@ def test_success_for_valid_dependencies(self): CcTarget(name="foo_pkg", header_files=["foo.h", "foo/bar.h"]), CcTarget(name="lib_without_hdrs_purely_for_linking", header_files=[]), ], - implementation_deps=[CcTarget(name="baz_pkg", header_files=["baz.h"])], + impl_deps=[CcTarget(name="baz_pkg", header_files=["baz.h"])], include_paths=[""], defines=[], ), @@ -93,7 +93,7 @@ def test_success_for_valid_dependencies_with_virtual_include_paths(self): system_under_inspection=SystemUnderInspection( target_under_inspection=CcTarget(name="foo", header_files=["self/own_header.h"]), deps=[CcTarget(name="foo_pkg", header_files=["foo.h", "some/virtual/dir/bar.h"])], - implementation_deps=[CcTarget(name="baz_pkg", header_files=["long/nested/path/baz.h"])], + impl_deps=[CcTarget(name="baz_pkg", header_files=["long/nested/path/baz.h"])], include_paths=["", "long/nested", "some/virtual"], defines=[], ), @@ -112,7 +112,7 @@ def test_success_for_valid_dependencies_with_virtual_include_paths_and_relative_ system_under_inspection=SystemUnderInspection( target_under_inspection=CcTarget(name="self", header_files=["self/own_header.h"]), deps=[CcTarget(name="foo_pkg", header_files=["dir/foo.h"])], - implementation_deps=[CcTarget(name="bar_pkg", header_files=["some/virtual/dir/bar.h"])], + impl_deps=[CcTarget(name="bar_pkg", header_files=["some/virtual/dir/bar.h"])], include_paths=["", "some/virtual", "other/virtual"], defines=[], ), @@ -128,7 +128,7 @@ def test_success_for_internal_relative_includes_with_flat_structure(self): system_under_inspection=SystemUnderInspection( target_under_inspection=CcTarget(name="foo", header_files=["foo.h", "bar.h"]), deps=[], - implementation_deps=[], + impl_deps=[], include_paths=[""], defines=[], ), @@ -157,7 +157,7 @@ def test_success_for_internal_relative_includes_with_nested_structure(self): ], ), deps=[], - implementation_deps=[], + impl_deps=[], include_paths=[""], defines=[], ), @@ -182,7 +182,7 @@ def test_success_for_relative_includes_to_dependency(self): header_files=["bar/dir/bar.h", "bar/dir/sub/tick.h", "bar/dir/sub/tock.h"], ) ], - implementation_deps=[], + impl_deps=[], include_paths=[""], defines=[], ), @@ -202,7 +202,7 @@ def test_invalid_includes_missing_internal_include(self): header_files=["some/dir/foo.h", "some/dir/bar.h", "unrelated/dir/tick.h", "unrelated/dir/tock.h"], ), deps=[], - implementation_deps=[], + impl_deps=[], include_paths=[""], defines=[], ), @@ -211,7 +211,7 @@ def test_invalid_includes_missing_internal_include(self): self.assertFalse(result.is_ok()) self.assertEqual(result.unused_deps, []) - self.assertEqual(result.unused_implementation_deps, []) + self.assertEqual(result.unused_impl_deps, []) self.assertEqual(result.deps_which_should_be_private, []) self.assertEqual(result.public_includes_without_dep, [Include(file=Path("some/dir/foo.h"), include="tick.h")]) self.assertEqual(result.private_includes_without_dep, [Include(file=Path("some/dir/bar.h"), include="tock.h")]) @@ -231,7 +231,7 @@ def test_missing_includes_from_dependencies(self): system_under_inspection=SystemUnderInspection( target_under_inspection=CcTarget(name="foo", header_files=[]), deps=[CcTarget(name="foo", header_files=["foo.h"])], - implementation_deps=[CcTarget(name="bar", header_files=["bar.h"])], + impl_deps=[CcTarget(name="bar", header_files=["bar.h"])], include_paths=[""], defines=[], ), @@ -240,7 +240,7 @@ def test_missing_includes_from_dependencies(self): self.assertFalse(result.is_ok()) self.assertEqual(result.unused_deps, []) - self.assertEqual(result.unused_implementation_deps, []) + self.assertEqual(result.unused_impl_deps, []) self.assertEqual(result.deps_which_should_be_private, []) self.assertEqual(len(result.public_includes_without_dep), 2) self.assertTrue(Include(file=Path("public_file"), include="foo/foo.h") in result.public_includes_without_dep) @@ -260,7 +260,7 @@ def test_unused_dependencies(self): CcTarget(name="foo", header_files=["foo.h"]), CcTarget(name="bar", header_files=["bar.h"]), ], - implementation_deps=[ + impl_deps=[ CcTarget(name="impl_dep", header_files=["impl_dep.h"]), CcTarget(name="impl_foo", header_files=["impl_dep_foo.h"]), CcTarget(name="impl_bar", header_files=["impl_dep_bar.h"]), @@ -276,11 +276,11 @@ def test_unused_dependencies(self): self.assertEqual(result.private_includes_without_dep, []) self.assertEqual(result.deps_which_should_be_private, []) self.assertEqual(len(result.unused_deps), 2) - self.assertEqual(len(result.unused_implementation_deps), 2) + self.assertEqual(len(result.unused_impl_deps), 2) self.assertTrue("foo" in result.unused_deps) self.assertTrue("bar" in result.unused_deps) - self.assertTrue("impl_foo" in result.unused_implementation_deps) - self.assertTrue("impl_bar" in result.unused_implementation_deps) + self.assertTrue("impl_foo" in result.unused_impl_deps) + self.assertTrue("impl_bar" in result.unused_impl_deps) def test_public_dependencies_which_should_be_private(self): result = evaluate_includes( @@ -296,7 +296,7 @@ def test_public_dependencies_which_should_be_private(self): CcTarget(name="foo", header_files=["impl_dep_foo.h"]), CcTarget(name="bar", header_files=["impl_dep_bar.h"]), ], - implementation_deps=[], + impl_deps=[], include_paths=[""], defines=[], ), @@ -307,7 +307,7 @@ def test_public_dependencies_which_should_be_private(self): self.assertEqual(result.public_includes_without_dep, []) self.assertEqual(result.private_includes_without_dep, []) self.assertEqual(result.unused_deps, []) - self.assertEqual(result.unused_implementation_deps, []) + self.assertEqual(result.unused_impl_deps, []) self.assertEqual(len(result.deps_which_should_be_private), 2) self.assertTrue("foo" in result.deps_which_should_be_private) self.assertTrue("bar" in result.deps_which_should_be_private) @@ -326,7 +326,7 @@ def test_public_dependencies_which_should_be_private_disabled(self): CcTarget(name="foo", header_files=["impl_dep_foo.h"]), CcTarget(name="bar", header_files=["impl_dep_bar.h"]), ], - implementation_deps=[], + impl_deps=[], include_paths=[""], defines=[], ), diff --git a/src/analyze_includes/test/result_test.py b/src/analyze_includes/test/result_test.py index 63943103..9e0b0869 100644 --- a/src/analyze_includes/test/result_test.py +++ b/src/analyze_includes/test/result_test.py @@ -156,7 +156,7 @@ def test_is_ok_fails_due_to_unused_public_deps(self): ) def test_is_ok_fails_due_to_unused_private_deps(self): - unit = Result(target="//foo:bar", unused_implementation_deps=["foo", "baz"]) + unit = Result(target="//foo:bar", unused_impl_deps=["foo", "baz"]) self.assertFalse(unit.is_ok()) self.assertEqual( @@ -187,7 +187,7 @@ def test_is_ok_fails_due_to_unused_private_deps(self): ) 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"]) + unit = Result(target="//foo:bar", unused_deps=["foo"], unused_impl_deps=["baz"]) self.assertFalse(unit.is_ok()) self.assertEqual( @@ -252,7 +252,7 @@ def test_is_ok_fails_due_to_deps_which_should_be_private(self): def test_set_use_implementation_deps(self): - unit = Result(target="//:foo", use_implementation_deps=True) + unit = Result(target="//:foo", use_impl_deps=True) self.assertEqual( unit.to_json(), diff --git a/src/analyze_includes/test/system_under_inspection_test.py b/src/analyze_includes/test/system_under_inspection_test.py index a1d25c7c..beba5009 100644 --- a/src/analyze_includes/test/system_under_inspection_test.py +++ b/src/analyze_includes/test/system_under_inspection_test.py @@ -86,7 +86,7 @@ def test_load_full_file(self): Path("src/analyze_includes/test/data/dep_info_foo.json"), Path("src/analyze_includes/test/data/dep_info_bar.json"), ], - implementation_deps=[ + impl_deps=[ Path("src/analyze_includes/test/data/implementation_dep_info_foo.json"), Path("src/analyze_includes/test/data/implementation_dep_info_bar.json"), ], @@ -104,13 +104,13 @@ def test_load_full_file(self): self.assertEqual(sui.deps[1].header_files, ["public/dep/bar_1.h", "public/dep/bar_2.h"]) self.assertEqual(sui.deps[1].usage.usage, UsageStatus.NONE) - self.assertEqual(len(sui.implementation_deps), 2) - self.assertEqual(sui.implementation_deps[0].name, "//private/dep:foo") - self.assertEqual(sui.implementation_deps[0].header_files, ["private/dep/foo_1.h", "private/dep/foo_2.h"]) - self.assertEqual(sui.implementation_deps[0].usage.usage, UsageStatus.NONE) - self.assertEqual(sui.implementation_deps[1].name, "//private/dep:bar") - self.assertEqual(sui.implementation_deps[1].header_files, ["private/dep/bar_1.h", "private/dep/bar_2.h"]) - self.assertEqual(sui.implementation_deps[1].usage.usage, UsageStatus.NONE) + self.assertEqual(len(sui.impl_deps), 2) + self.assertEqual(sui.impl_deps[0].name, "//private/dep:foo") + self.assertEqual(sui.impl_deps[0].header_files, ["private/dep/foo_1.h", "private/dep/foo_2.h"]) + self.assertEqual(sui.impl_deps[0].usage.usage, UsageStatus.NONE) + self.assertEqual(sui.impl_deps[1].name, "//private/dep:bar") + self.assertEqual(sui.impl_deps[1].header_files, ["private/dep/bar_1.h", "private/dep/bar_2.h"]) + self.assertEqual(sui.impl_deps[1].usage.usage, UsageStatus.NONE) self.assertEqual(sui.include_paths, ["", "some/dir", "another/dir"]) self.assertEqual(sui.defines, ["SOME_DEFINE", "ANOTHER_DEFINE 42"]) @@ -119,14 +119,14 @@ def test_load_empty_file(self): sui = get_system_under_inspection( target_under_inspection=Path("src/analyze_includes/test/data/target_under_inspection_empty.json"), deps=[], - implementation_deps=[], + impl_deps=[], ) self.assertEqual(sui.target_under_inspection.name, "//:foo") self.assertEqual(sui.target_under_inspection.header_files, []) self.assertEqual(sui.target_under_inspection.usage.usage, UsageStatus.NONE) self.assertEqual(sui.deps, []) - self.assertEqual(sui.implementation_deps, []) + self.assertEqual(sui.impl_deps, []) self.assertEqual(sui.include_paths, []) self.assertEqual(sui.defines, []) diff --git a/src/apply_fixes/apply_fixes.py b/src/apply_fixes/apply_fixes.py index 48df52bf..bc5d7034 100644 --- a/src/apply_fixes/apply_fixes.py +++ b/src/apply_fixes/apply_fixes.py @@ -21,7 +21,7 @@ class RequestedFixes: def __init__(self, main_args: Namespace) -> None: self.remove_unused_deps = main_args.fix_unused_deps or main_args.fix_all - self.move_private_deps_to_implementation_deps = main_args.fix_deps_which_should_be_private or main_args.fix_all + self.move_private_deps_to_impl_deps = main_args.fix_deps_which_should_be_private or main_args.fix_all self.add_missing_deps = main_args.fix_missing_deps or main_args.fix_all @@ -147,23 +147,21 @@ def add_discovered_deps( discovered_private_deps: List[str], target: str, buildozer: BuildozerExecutor, - use_implementation_deps: bool, + use_impl_deps: bool, ) -> None: add_to_deps = discovered_public_deps - add_to_implementation_deps = [] - if use_implementation_deps: + add_to_impl_deps = [] + if use_impl_deps: for dep in discovered_private_deps: if dep not in add_to_deps: - add_to_implementation_deps.append(dep) + add_to_impl_deps.append(dep) else: add_to_deps.extend(discovered_private_deps) if add_to_deps: buildozer.execute(task=f"add deps {' '.join(list(set(add_to_deps)))}", target=target) - if add_to_implementation_deps: - buildozer.execute( - task=f"add implementation_deps {' '.join(list(set(add_to_implementation_deps)))}", target=target - ) + if add_to_impl_deps: + buildozer.execute(task=f"add implementation_deps {' '.join(list(set(add_to_impl_deps)))}", target=target) def perform_fixes(buildozer: BuildozerExecutor, workspace: Path, report: Path, requested_fixes: RequestedFixes) -> None: @@ -177,7 +175,7 @@ def perform_fixes(buildozer: BuildozerExecutor, workspace: Path, report: Path, r if unused_deps := content["unused_implementation_deps"]: buildozer.execute(task=f"remove implementation_deps {' '.join(unused_deps)}", target=target) - if requested_fixes.move_private_deps_to_implementation_deps: + if requested_fixes.move_private_deps_to_impl_deps: deps_which_should_be_private = content["deps_which_should_be_private"] if deps_which_should_be_private: buildozer.execute( @@ -196,7 +194,7 @@ def perform_fixes(buildozer: BuildozerExecutor, workspace: Path, report: Path, r discovered_private_deps=discovered_private_deps, target=target, buildozer=buildozer, - use_implementation_deps=content["use_implementation_deps"], + use_impl_deps=content["use_implementation_deps"], ) diff --git a/src/aspect/dwyu.bzl b/src/aspect/dwyu.bzl index 2e7ec135..f784c0ee 100644 --- a/src/aspect/dwyu.bzl +++ b/src/aspect/dwyu.bzl @@ -194,7 +194,6 @@ def dwyu_aspect_impl(target, ctx): verbose = False, ) - # TODO consistently use impl_deps as variable name and only use long name in docs and APIs target_deps, target_impl_deps = _preprocess_deps(ctx) # TODO Investigate if we can prevent running this multiple times for the same dep if multiple