Skip to content

Commit

Permalink
Consistently use impl_deps for implementation_deps internally
Browse files Browse the repository at this point in the history
We keep the long name in documentation and public APIs.
But internally we use a shorter name which eases reading the code and is
consistent to us already using `deps` instead of `dependencies`.
  • Loading branch information
martis42 committed Nov 1, 2023
1 parent 9b5a2b4 commit 6209dba
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 58 deletions.
8 changes: 4 additions & 4 deletions src/analyze_includes/evaluate_includes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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
2 changes: 1 addition & 1 deletion src/analyze_includes/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 8 additions & 8 deletions src/analyze_includes/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand All @@ -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)
Expand All @@ -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"

Expand All @@ -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
6 changes: 3 additions & 3 deletions src/analyze_includes/system_under_inspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
)
34 changes: 17 additions & 17 deletions src/analyze_includes/test/evaluate_includes_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=[],
),
Expand All @@ -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=[],
),
Expand All @@ -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=[],
),
Expand All @@ -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=[],
),
Expand Down Expand Up @@ -157,7 +157,7 @@ def test_success_for_internal_relative_includes_with_nested_structure(self):
],
),
deps=[],
implementation_deps=[],
impl_deps=[],
include_paths=[""],
defines=[],
),
Expand All @@ -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=[],
),
Expand All @@ -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=[],
),
Expand All @@ -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")])
Expand All @@ -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=[],
),
Expand All @@ -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)
Expand All @@ -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"]),
Expand All @@ -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(
Expand All @@ -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=[],
),
Expand All @@ -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)
Expand All @@ -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=[],
),
Expand Down
6 changes: 3 additions & 3 deletions src/analyze_includes/test/result_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(),
Expand Down
20 changes: 10 additions & 10 deletions src/analyze_includes/test/system_under_inspection_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
],
Expand All @@ -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"])
Expand All @@ -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, [])

Expand Down
Loading

0 comments on commit 6209dba

Please sign in to comment.