feat: implement core analysis engine (INH001, INH002) with ADRs by steven-cutting · Pull Request #2 · steven-cutting/flake8-inheritance

steven-cutting · 2026-02-07T12:02:45Z

https://claude.ai/code/session_01XCWZSwXFCDx4b71TT1sh5C

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8077110316

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-02-07T12:04:52Z

+    for kw in node.keywords:
+        if kw.arg == "metaclass" and isinstance(kw.value, ast.Name):
+            name = kw.value.id
+            if import_map.get(name) == "abc" and name in _abc_metaclass_aliases(import_map):


Detect metaclass=abc.ABCMeta in ABC checks

The ABC detection only recognizes metaclass= when the value is an ast.Name. That misses the common pattern class X(metaclass=abc.ABCMeta) (or metaclass=abc.ABCMeta with import abc), because kw.value is an ast.Attribute and the class won’t be treated as abstract. In that case, concrete methods in such ABCs won’t trigger INH002, leading to false negatives. Consider handling ast.Attribute for metaclass values whose root resolves to abc and whose attribute is ABCMeta.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

Implements the core analysis layer for flake8-inheritance, adding AST-based detection for rules INH001 (internal inheritance) and INH002 (concrete methods in ABCs), along with ADR documentation and unit tests to validate behavior independently of the flake8 adapter.

Changes:

Add framework-agnostic ErrorCode definitions for INH001/INH002.
Implement import tracking, base-class classification, and INH001/INH002 checks via InheritanceVisitor.
Add ADRs and tests covering import mapping, classification, rule detection, and edge cases.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/flake8_inheritance/visitors.py`	Core AST analysis (import map, classification, INH001/INH002 detection).
`src/flake8_inheritance/codes.py`	Frozen dataclass error-code definitions and formatting.
`tests/test_visitors.py`	Unit tests for import mapping, classification, INH001/INH002, and edge cases.
`tests/test_codes.py`	Unit tests for error-code immutability and message formatting.
`doc/adr/README.md`	Index update to include new ADRs.
`doc/adr/0007-define-error-codes-as-framework-agnostic-frozen-dataclasses.md`	ADR for error-code design.
`doc/adr/0008-classify-base-classes-using-import-map-lookup-with-sys-stdlib-module-names.md`	ADR for classification strategy.
`doc/adr/0009-decouple-visitor-analysis-logic-from-flake8-plugin-interface.md`	ADR for architecture split (analysis vs adapter).
`doc/adr/0010-silently-skip-unanalyzable-patterns-rather-than-guessing-or-crashing.md`	ADR for “skip vs guess” behavior on unanalyzable patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-07T12:07:15Z

+    def test_unknown_not_flagged(self) -> None:
+        # Absolute import with no project_packages configured → external
+        errors = _visit("""\
+            from somelib import Thing
+            class Foo(Thing):
+                pass
+        """)
+        assert errors == []


The test name test_unknown_not_flagged (and the comment) don't match the behavior under test: Thing is imported from somelib, so it’s classified as external, not unknown. Renaming this test (or adjusting the scenario to truly be unknown) would make the intent clearer.

Copilot · 2026-02-07T12:07:15Z

+            class MyABC(ABC):
+                @abstractmethod
+                async def do_thing(self):
+                    pass


This test duplicates test_async_abstractmethod_not_flagged above with the same scenario/assertions. Consider removing one of them (or varying the setup) to avoid redundant coverage.

Suggested change

class MyABC(ABC):

@abstractmethod

async def do_thing(self):

pass

class Base(ABC):

@abstractmethod

async def do_thing(self) -> None:

pass

class Impl(Base):

async def do_thing(self) -> None:

pass

Copilot · 2026-02-07T12:07:15Z

+    import_map: dict[str, str] = {}
+    for node in ast.walk(tree):
+        if isinstance(node, ast.Import):
+            for alias in node.names:
+                # ``import foo.bar`` -> local name "foo", package "foo"
+                top = alias.name.split(".")[0]
+                local_name = alias.asname if alias.asname else top
+                import_map[local_name] = top


build_import_map uses ast.walk(tree), which collects imports from nested scopes (functions/classes) and also imports that appear after a class statement. That can make a base name look "imported" even when it isn’t actually in scope at the point the class bases are evaluated, leading to incorrect INH001/INH002 diagnostics. Consider restricting the map to module-level imports (and/or only statements that occur before a given class), or making the import map scope-aware.

Copilot · 2026-02-07T12:07:16Z

+        return "internal_relative"
+    if pkg in _project_set(project_packages):
+        return "internal_project"


classify_name() calls _project_set(project_packages) on every invocation, recreating a set repeatedly during AST walking. Since project_packages is effectively constant for a visitor run, compute a set once (e.g., in InheritanceVisitor.__init__ or at the start of visit()) and reuse it to avoid unnecessary allocations.

Copilot · 2026-02-07T12:07:16Z

+    return {
+        name
+        for name, pkg in import_map.items()
+        if pkg == "abc" and name != "abstractmethod"
+        and name not in _abc_class_aliases(import_map)
+    } | {
+        name
+        for name, pkg in import_map.items()
+        if pkg == "abc" and name == "ABCMeta"


_abc_metaclass_aliases() recomputes _abc_class_aliases(import_map) inside a comprehension and then unions with a second set that is redundant for ABCMeta (it already satisfies the first set’s predicate). Consider computing the alias sets once and simplifying the logic to a single pass for readability and efficiency.

Suggested change

return {

name

for name, pkg in import_map.items()

if pkg == "abc" and name != "abstractmethod"

and name not in _abc_class_aliases(import_map)

} | {

name

for name, pkg in import_map.items()

if pkg == "abc" and name == "ABCMeta"

class_aliases = _abc_class_aliases(import_map)

return {

name

for name, pkg in import_map.items()

if pkg == "abc"

and name != "abstractmethod"

and name not in class_aliases

Copilot · 2026-02-07T12:07:16Z

+import pytest
+


pytest is imported but never used in this test module. With Ruff configured to check F401, this will fail linting; remove the import or use it.

Suggested change

import pytest

Copilot · 2026-02-07T12:07:16Z

+
+import pytest
+
+from flake8_inheritance.codes import INH001, INH002, ErrorCode


ErrorCode is imported but never used. This will trigger an unused-import lint error (e.g., Ruff F401); either remove it from the import list or add an assertion that references ErrorCode.

Suggested change

from flake8_inheritance.codes import INH001, INH002, ErrorCode

from flake8_inheritance.codes import INH001, INH002

feat: implement core analysis engine (INH001, INH002) with ADRs

8077110

https://claude.ai/code/session_01XCWZSwXFCDx4b71TT1sh5C

Copilot AI review requested due to automatic review settings February 7, 2026 12:02

Copilot started reviewing on behalf of steven-cutting February 7, 2026 12:03 View session

chatgpt-codex-connector Bot reviewed Feb 7, 2026

View reviewed changes

Copilot AI reviewed Feb 7, 2026

View reviewed changes

Navigation Menu

feat: implement core analysis engine (INH001, INH002) with ADRs - #2

feat: implement core analysis engine (INH001, INH002) with ADRs#2
steven-cutting wants to merge 1 commit into
mainfrom
claude/core-analysis-engine-TGWg6

steven-cutting commented Feb 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Feb 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		import pytest

		from flake8_inheritance.codes import INH001, INH002, ErrorCode

Conversation

steven-cutting commented Feb 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants