From 7b04653b7e58fa625a02d95d62f60f99d11885c3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 24 Sep 2026 12:56:42 +0100 Subject: [PATCH 1/5] docs: refresh bundled docs and describe empty-input detection as shipped --- docs/sql-test-coverage-rules.md | 16 ++-- .../references/docs/concepts/audits.md | 3 +- .../docs/concepts/observability/sinks.md | 16 ++++ .../rules/configuration-and-selection.md | 10 ++- .../references/docs/concepts/testing.md | 89 ++++++++++++++++++- 5 files changed, 123 insertions(+), 11 deletions(-) diff --git a/docs/sql-test-coverage-rules.md b/docs/sql-test-coverage-rules.md index 96996a1cd1..68e3216aa9 100644 --- a/docs/sql-test-coverage-rules.md +++ b/docs/sql-test-coverage-rules.md @@ -26,14 +26,16 @@ When a model's rows come only from its inputs, this test passes for almost any m adds no coverage. `SQBRTEST203` flags a model-mode test when all of the following hold: - Every mock (`__ref__`, `__source__`, `__seed__`, `__dbt_ref__`, `__table_fn__`) is a single - `SELECT` without a set operator that is filtered by `WHERE FALSE` or `WHERE 1 = 0`, limited by - `LIMIT 0`, or is `SELECT * FROM __EMPTY_FIXTURE()`. A `UNION` whose last branch is filtered is - not empty. + `SELECT` without a set operator that is limited by `LIMIT 0`, is `SELECT * FROM __EMPTY_FIXTURE()`, + or is filtered by `WHERE FALSE` or `WHERE 1 = 0` without calling any function (an aggregate such + as `ORDER BY COUNT(*)` can still return a row). A `UNION` whose last branch is filtered is not + empty. - Every `__expected__` CTE is empty in the same way, or there is none. -- Every `__assert__` CTE is a bare row-existence check, `SELECT FROM __ref("")` - on a tested model, with no `WHERE`, `JOIN`, `GROUP BY`, `HAVING`, `QUALIFY`, set operator, - `EXISTS`, or subquery. -- The test has no `__macro__` mocks. +- Every `__assert__` CTE is a bare row-existence check, `SELECT FROM + __ref("")` on a tested model, with no `WHERE`, `JOIN`, `GROUP BY`, `HAVING`, `QUALIFY`, + set operator, `EXISTS`, subquery, `LIMIT`, `OFFSET` or `FETCH`, and no function call in its + select list or `ORDER BY`. +- The test has no `__macro__` mocks and no model query overrides. Macro, UDF, and table-function tests are never flagged, and SQL that the rules parser cannot read is never flagged. diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/audits.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/audits.md index 56175d5e4f..3c32bbc01c 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/audits.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/audits.md @@ -108,7 +108,8 @@ contract as directly authored instances. Native warehouse adapters best-effort append confirmed audit outcomes to `_sqlbuild_audit_results`. Rows are immutable and use deterministic IDs, so retrying the same result is idempotent. Projection failure is reported separately and does not change the audit outcome or -command exit code. Lifecycle sinks can also consume the corresponding `audit_completed` fact. +command exit code. Lifecycle sinks can also consume the corresponding `audit_completed` fact, which +is published as each audit finishes. ## Built-in audits diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/observability/sinks.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/observability/sinks.md index cf4d2d323f..7b59dfdf64 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/observability/sinks.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/observability/sinks.md @@ -73,6 +73,16 @@ event_kinds = ["resource", "statement"] min_severity = "warning" ``` +`shutdown_timeout` sets how long a command waits at exit for lifecycle sinks to drain queued +events. It is a fixed duration from `"0s"` to `"10m"`, defaults to `"2s"`, and is accepted only +under `[sinks.lifecycle]`, not per named sink. `"0s"` drops anything still queued at exit. The +setting does not change the one-second timeout for each sink call. + +```toml +[sinks.lifecycle] +shutdown_timeout = "30s" +``` + SQLBuild owns envelope validation, local queueing, filtering, and bounded dispatch. The project owns destination credentials, serialization, routes/topics, acknowledgements, retries, retention, and durability. SQLBuild core does not provide a Kafka or ClickHouse implementation. @@ -82,6 +92,12 @@ durability. SQLBuild core does not provide a Kafka or ClickHouse implementation. - Lifecycle dispatch prioritizes failures and terminal facts, remaining FIFO within equal priority. - Queue overflow can displace or drop lower-priority records and is reflected in sink accounting. - A failing or timed-out sink is isolated from command correctness. +- `audit_completed` facts are published as each audit finishes (model audits in `sqb build` when + their model finishes), not in one burst at the end of the run. +- When lifecycle events are dropped or fail, the command prints one warning to stderr with the + counts and affected sinks, for example + `Warning: command completed successfully, but lifecycle event export was incomplete: 140 of 248 events dropped, 0 failed (sink 'orders_events'). Increase sinks.lifecycle.shutdown_timeout or check sink health.` + The exit code and stdout, including `--json` output, are unchanged. - Provider setup and declaration errors fail before execution because the project configuration is invalid. - Providers shared by multiple sinks are set up and torn down once per command. diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md index 0d4a09e6c6..2a12024177 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md @@ -42,13 +42,21 @@ sqb rules show SQBRSQL004 ## Rule options -Custom Rules can declare typed options. Configure them by exact code: +Built-in and Custom Rules can declare typed options. Configure them by exact code: ```toml [rules.rule_options.XSQBRNAME001] required_prefix = "order" + +[rules.rule_options.SQBRTEST203] +allowed_tests = ["customers__reviewed_empty_input"] ``` +`SQBRTEST203` (`empty-input-only-test`) accepts `allowed_tests`, a list of SQL test names that are +reviewed exceptions. Listed tests are not flagged and still count toward `SQBRTEST202`'s +`min_tests_per_model`. Entries that name no existing test are reported as stale. See +[Empty-input tests](../testing.md#empty-input-tests). + Unknown Rule codes, option names, and invalid option values fail configuration. ## Focused selection diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md index ce65030bb4..7a72e04ae0 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -17,6 +17,8 @@ Online: https://docs.sqlbuild.com/concepts/testing - Macro-powered mocks - Macro mocking - Assertions +- Cursor windows in tests +- Empty-input tests - Test modes - Multiple tests per file - Repeating test logic @@ -90,11 +92,45 @@ The trailing `SELECT 1` is required as a ceremonial closing statement. | `__ref__` | Mock a model. Replaces `__ref("")` in the model SQL. | | `__seed__` | Mock a seed. Replaces `__seed("")` in the model SQL. | | `__table_fn__` | Mock a table function in model mode. Replaces the complete `__table_fn("")(...)` invocation with the fixture relation. | -| `__expected__` | Define expected output for a model. SQLBuild resolves the model's real SQL and compares against this. | +| `__expected__` | Define expected output for a model. SQLBuild resolves the model's real SQL and compares both ways on the listed columns. | | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | | `__macro__` | Mock a macro. Replaces every `@(...)` call with the mock value. | -Any CTE without one of these prefixes is treated as a helper CTE, available to all mock and model SQL in the test. +Any CTE without one of these prefixes is treated as a helper CTE, available to mock, model, +`__expected__`, and `__assert__` SQL in the test. + +An `__expected__` CTE compares only the columns it lists, matched by name, so column order +does not matter and unlisted model columns are ignored. Row counts are always compared. Listing a +column the model does not output is a test error. When the expected CTE's columns are not explicit +(for example `SELECT *` from another CTE), SQLBuild compares every column by position. + +Helper CTEs can read other helpers and mocks by CTE name, such as `__ref__stg_orders`. Shared +expected rows can therefore live in one helper that both an `__expected__` CTE and an assertion +read: + +```sql +TEST(); + +WITH +__source__raw__orders AS ( + SELECT 1 AS id, 3 AS quantity UNION ALL SELECT 2 AS id, 5 AS quantity +), +expected_orders AS ( + SELECT 1 AS order_id, 3 AS quantity UNION ALL SELECT 2 AS order_id, 5 AS quantity +), +__expected__stg_orders AS ( + SELECT order_id, quantity FROM expected_orders +), +__assert__no_unexpected_order_ids AS ( + SELECT order_id FROM __ref("stg_orders") + EXCEPT + SELECT order_id FROM expected_orders +) +SELECT 1 +``` + +SQLBuild emits each helper and mock a query needs once, in dependency order, including mocks that +a helper reaches through another mock. ## Multi-model tests @@ -337,6 +373,55 @@ Assertions can be mixed with `__expected__` CTEs in the same test, or used on th During `sqb test` and `sqb build`, assertion results appear as nested check rows alongside expected comparisons. +## Cursor windows in tests + +Models that call `__cursor_start()` and `__cursor_end()` run in a test over one wide window, never +the model's configured `cursor_start` and `cursor_end`: `1900-01-01 00:00:00` to +`2999-12-31 00:00:00` for timestamp cursors, and `-1000000000000000` to `1000000000000000` for +integer cursors. The bounds render as the same typed literals as a real run, so date arithmetic +such as `DATEADD('day', -7, __cursor_start()::DATE)` keeps working and mocked rows are not filtered +out. Microbatch models run once over the whole window, not once per batch. + +To test windowing itself, declare the window in the header. Either key is optional. As in model +configuration, `cursor_start` is inclusive and `cursor_end` is exclusive: + +```sql +TEST (name "daily_orders_skips_rows_outside_window", cursor_start "2026-02-01", cursor_end "2026-02-03"); +``` + +The window applies to every model the test evaluates that uses the intrinsics. Values are checked +against each model's `cursor_type` and `cursor_grain` at compile time. Invalid, misaligned, or +inverted bounds, and a window on a test whose models do not use the intrinsics, are compile errors. +The compiled test SQL shows the rendered window. + +## Empty-input tests + +Do not write a test whose mocks are all empty (`WHERE FALSE`, `WHERE 1 = 0`, `LIMIT 0`, or +`__EMPTY_FIXTURE()`) and whose only checks are an empty `__expected__` CTE or a bare +`SELECT ... FROM __ref("")` assertion. It cannot fail for a model whose rows come from its +inputs. Rule `SQBRTEST203` rejects it, and `SQBRTEST202` does not count it toward +`min_tests_per_model`. Mock representative rows and assert concrete output instead. + +An empty-input test is legitimate when it asserts concrete output, for example a global aggregate +that must return one zero-valued summary row: + +```sql +TEST (name "order_summary__empty_inputs_return_zero_row"); + +WITH +__source__raw__orders AS ( + SELECT CAST(NULL AS INTEGER) AS id, CAST(NULL AS INTEGER) AS amount WHERE FALSE +), +__expected__order_summary AS ( + SELECT 0 AS order_count, 0 AS total_amount +) +SELECT 1 +``` + +Assertions that filter for invalid rows, compare against expected rows, aggregate, or limit their +result are not treated as bare existence checks. Reviewed exceptions are listed by test name in the +project file; see [Rule options](rules/configuration-and-selection.md#rule-options). + ## Test modes By default, `TEST()` runs in model mode - mocking sources/refs and comparing model outputs. Three additional modes let you test reusable logic directly without needing a model chain. From 3e6bae2d17409f0f12f30f1cf3be7bf2e36a4edf Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 24 Sep 2026 13:13:09 +0100 Subject: [PATCH 2/5] feat(dupscore): ignore forced adapter overrides and compare call targets Hide similarity links between methods that the strict adapter contract forces every adapter to define itself, keep call-target names in normalised token streams so same-layout functions that call different helpers no longer match as renamed clones, and require a 0.9 ratio for near-miss pairs of units under 80 tokens. --- scripts/dupscore/README.md | 37 ++- scripts/dupscore/_helpers/clones/matching.py | 18 +- .../dupscore/_helpers/clones/python_units.py | 46 +++- scripts/dupscore/_helpers/clones/rendering.py | 4 +- .../dupscore/_helpers/clones/rust_units.py | 28 +- .../dupscore/_helpers/contracts/__init__.py | 1 + .../_helpers/contracts/forced_overrides.py | 251 ++++++++++++++++++ scripts/dupscore/_helpers/inputs/config.py | 116 ++++++-- scripts/dupscore/constants.py | 2 + scripts/dupscore/dupscore.toml | 14 + scripts/dupscore/main/build_clone_report.py | 23 +- scripts/dupscore/models.py | 13 + .../_helpers/clones/matching/_test_types.py | 12 + .../_helpers/clones/matching/helpers.py | 20 ++ .../clones/matching/test_find_clone_pairs.py | 67 +++++ .../python_units/test_extract_python_units.py | 37 ++- .../_helpers/clones/rendering/_test_types.py | 11 + .../rendering/test_render_clone_report.py | 44 +++ .../rust_units/test_extract_rust_units.py | 30 ++- .../contracts/forced_overrides/_test_types.py | 30 +++ .../contracts/forced_overrides/helpers.py | 170 ++++++++++++ .../test_find_forced_overrides.py | 235 ++++++++++++++++ .../_helpers/inputs/config/_test_types.py | 10 +- .../inputs/config/test_load_config.py | 160 ++++++++++- .../main/build_clone_report/_test_types.py | 18 +- .../main/build_clone_report/helpers.py | 11 + .../test_build_clone_report.py | 106 ++++++++ .../main/dupscore/test_dupscore_cli.py | 1 + 28 files changed, 1468 insertions(+), 47 deletions(-) create mode 100644 scripts/dupscore/_helpers/contracts/__init__.py create mode 100644 scripts/dupscore/_helpers/contracts/forced_overrides.py create mode 100644 tests/unit/scripts/dupscore/_helpers/clones/matching/_test_types.py create mode 100644 tests/unit/scripts/dupscore/_helpers/clones/matching/helpers.py create mode 100644 tests/unit/scripts/dupscore/_helpers/clones/matching/test_find_clone_pairs.py create mode 100644 tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py create mode 100644 tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py create mode 100644 tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/_test_types.py create mode 100644 tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py create mode 100644 tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py diff --git a/scripts/dupscore/README.md b/scripts/dupscore/README.md index 70da1588ce..6686d37bad 100644 --- a/scripts/dupscore/README.md +++ b/scripts/dupscore/README.md @@ -13,17 +13,23 @@ Reports concrete function-level clones: chars versus lifetimes). Test code is skipped by default: Python `tests/`, Rust `tests/` and `benches/` directories, `#[cfg(test)]` items and modules, and `#[test]` functions. `--include-tests` analyzes them too. The dbt integration is included, unlike in `report`. -- Token streams are normalised: identifiers become one placeholder, literals one placeholder per - kind, keywords and operators stay. Comments, Python docstrings, annotations, and `->` are - dropped. +- Token streams are normalised: local names, parameters, receivers, and plain attribute reads + become one placeholder, literals one placeholder per kind, and keywords and operators stay. + Call targets keep their names, so two functions with the same layout that call different + helpers do not look identical: Python `f(...)` and `obj.f(...)` keep `f`; Rust keeps any + identifier directly followed by `(` or `!` (functions, methods, tuple constructors, macros) + except the name after `fn`. Comments, Python docstrings, annotations, and `->` are dropped. - Candidates come from MOSS-style winnowed fingerprints: 12-token k-grams, minimum of every window of 6 k-grams, so any shared run of 17 or more normalised tokens yields a shared fingerprint. Fingerprints present in more than 25 units are boilerplate and ignored. Pairs sharing fingerprints with a Jaccard index of at least 0.2 are scored with the `difflib` ratio of their normalised streams. - Categories: `exact` (identical tokens after dropping comments, docstrings, and annotations), - `renamed` (identical normalised stream), `near-miss` (ratio at or above `--min-similarity`, - default 0.8). Units under `--min-tokens` normalised tokens (default 60) are ignored. + `renamed` (identical normalised stream, so only abstracted names and literals differ), + `near-miss` (ratio at or above `--min-similarity`, default 0.8). Units under `--min-tokens` + normalised tokens (default 60) are ignored. A near-miss pair whose smaller unit has fewer + than 80 tokens must reach a ratio of at least 0.9: short wrappers share most of their + skeleton by idiom, so this removes a noisy low end without affecting exact or renamed pairs. - Similar pairs are grouped into transitive clusters and ranked by estimated duplicated tokens. Useful options: @@ -47,6 +53,27 @@ reason = "Backends intentionally mirror one protocol." A pair is suppressed when both sides match the entry's globs (`*` also matches `/`). +Contract methods that every implementation must define itself are declared once: + +```toml +[[contract_exemption]] +contract = "src/sqlbuild/adapter/contract/classes/strict_adapter.py:StrictAdapter" +paths = ["src/sqlbuild/adapter/contract/classes/*", "src/sqlbuild/adapters/*/classes/*"] +forbidden_owners = ["src/sqlbuild/adapter/contract/classes/base_adapter.py:BaseAdapter"] +reason = "The strict adapter contract test requires each adapter to define every method." +``` + +The contract method names are the class's abstract methods (`@abstractmethod` or +`@abc.abstractmethod`, including those inherited from resolvable bases and not overridden), +parsed with `ast` from the analysed worktree; nothing is imported. A method is a forced override +when its top-level class lies under `paths`, derives from the contract class, and would +otherwise inherit that method from the contract, a `forbidden_owners` class, or nowhere. Links +between two forced overrides are hidden and clusters left without links disappear; links from +a forced override to any other unit (a private helper copy, or an override that could have been +inherited instead) are still reported. The summary counts forced overrides hidden this way. +An entry whose contract file is absent from the analysed tree is inactive; a missing class in an +existing file is a configuration error. + ## `dupscore report` and `dupscore pair A B` The package-pair ranking fused from call-graph, state fan-in, dataclass overlap, same-name, and diff --git a/scripts/dupscore/_helpers/clones/matching.py b/scripts/dupscore/_helpers/clones/matching.py index 307ab53b7a..5342bb48cb 100644 --- a/scripts/dupscore/_helpers/clones/matching.py +++ b/scripts/dupscore/_helpers/clones/matching.py @@ -13,6 +13,8 @@ CLONE_CANDIDATE_MIN_JACCARD, CLONE_MAX_FINGERPRINT_UNITS, CLONE_MIN_FINGERPRINTS, + CLONE_SMALL_NEAR_MISS_MIN_SIMILARITY, + CLONE_SMALL_NEAR_MISS_TOKENS, ) from scripts.dupscore.models import ClonePair, CloneUnit @@ -63,12 +65,16 @@ def find_clone_pairs( continue if shared / (left_size + right_size - shared) < CLONE_CANDIDATE_MIN_JACCARD: continue + required: float = _required_similarity( + smaller=min(len(units[left].normalized), len(units[right].normalized)), + min_similarity=min_similarity, + ) similarity: float = _stream_similarity( left=units[left].normalized, right=units[right].normalized, - min_similarity=min_similarity, + min_similarity=required, ) - if similarity >= min_similarity: + if similarity >= required: pairs[(left, right)] = ClonePair( left=left, right=right, @@ -78,6 +84,14 @@ def find_clone_pairs( return [pairs[key] for key in sorted(pairs)] +def _required_similarity(*, smaller: int, min_similarity: float) -> float: + """Demand closer matches from short units, whose shared skeletons are mostly idioms.""" + + if smaller < CLONE_SMALL_NEAR_MISS_TOKENS: + return max(min_similarity, CLONE_SMALL_NEAR_MISS_MIN_SIMILARITY) + return min_similarity + + def _stream_similarity( *, left: tuple[str, ...], diff --git a/scripts/dupscore/_helpers/clones/python_units.py b/scripts/dupscore/_helpers/clones/python_units.py index 150f7454d0..f9c93474b4 100644 --- a/scripts/dupscore/_helpers/clones/python_units.py +++ b/scripts/dupscore/_helpers/clones/python_units.py @@ -74,7 +74,12 @@ def extract_python_units(*, relative_path: str, source: str) -> tuple[CloneUnit, except (SyntaxError, tokenize.TokenError): return () starts: list[_Position] = [item.start for item in tokens] - normalized: list[str | None] = [_normalize(item) for item in tokens] + call_targets: frozenset[_Position] = _call_target_positions( + tree=tree, lines=io.StringIO(source).readlines() + ) + normalized: list[str | None] = [ + _normalize(item=item, call_target=item.start in call_targets) for item in tokens + ] for span in _ignored_spans(tree.body): end: int = bisect_left(starts, span.end) if span.statement and end < len(tokens) and tokens[end].type == tokenize.NEWLINE: @@ -107,9 +112,11 @@ def extract_python_units(*, relative_path: str, source: str) -> tuple[CloneUnit, return tuple(units) -def _normalize(item: tokenize.TokenInfo) -> str | None: +def _normalize(*, item: tokenize.TokenInfo, call_target: bool) -> str | None: if item.type == tokenize.NAME: - return item.string if item.string in _KEYWORDS else PLACEHOLDER_IDENTIFIER + if call_target or item.string in _KEYWORDS: + return item.string + return PLACEHOLDER_IDENTIFIER if item.type == tokenize.OP: return None if item.string == _RETURN_ARROW else item.string if item.type in _FSTRING_BODY_TYPES: @@ -122,6 +129,39 @@ def _normalize(item: tokenize.TokenInfo) -> str | None: return _LITERAL_PLACEHOLDERS.get(item.type, item.string) +def _call_target_positions(*, tree: ast.Module, lines: list[str]) -> frozenset[_Position]: + """Locate the name tokens of called functions (``f(...)``) and methods (``x.f(...)``).""" + + positions: set[_Position] = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + function: ast.expr = node.func + if isinstance(function, ast.Name): + positions.add( + _token_position(lines=lines, line=function.lineno, byte_column=function.col_offset) + ) + elif ( + isinstance(function, ast.Attribute) + and function.end_lineno is not None + and function.end_col_offset is not None + ): + end: _Position = _token_position( + lines=lines, line=function.end_lineno, byte_column=function.end_col_offset + ) + positions.add((end[0], end[1] - len(function.attr))) + return frozenset(positions) + + +def _token_position(*, lines: list[str], line: int, byte_column: int) -> _Position: + """Convert an ``ast`` UTF-8 byte column into the character column ``tokenize`` reports.""" + + text: str = lines[line - 1] if 0 < line <= len(lines) else "" + if text.isascii(): + return (line, byte_column) + return (line, len(text.encode("utf-8")[:byte_column].decode("utf-8", errors="replace"))) + + def _collect_unit_nodes(*, body: list[ast.stmt], prefix: str) -> list[_UnitNode]: collected: list[_UnitNode] = [] for statement in body: diff --git a/scripts/dupscore/_helpers/clones/rendering.py b/scripts/dupscore/_helpers/clones/rendering.py index 4ec2f0618b..d0b2429bca 100644 --- a/scripts/dupscore/_helpers/clones/rendering.py +++ b/scripts/dupscore/_helpers/clones/rendering.py @@ -20,7 +20,8 @@ def render_clone_text(*, report: CloneReport, top: int) -> str: scope: str = f" changed since {report.since}" if report.since is not None else "" lines: list[str] = [ f"dupscore clones{scope}: {len(report.clusters)} clusters " - f"({counts}; {report.allowlisted_pairs} allowlisted pairs hidden)" + f"({counts}; {report.allowlisted_pairs} allowlisted pairs hidden; " + f"{report.contract_exempt_members} contract-exempt members hidden)" ] for position, cluster in enumerate(report.clusters[:top], start=1): lines.append(f"{position:3d}. {_cluster_heading(cluster)}") @@ -43,6 +44,7 @@ def render_clone_json(*, report: CloneReport, top: int) -> str: "since": report.since, "unit_counts": report.unit_counts, "allowlisted_pairs": report.allowlisted_pairs, + "contract_exempt_members": report.contract_exempt_members, "total_clusters": len(report.clusters), "clusters": [ {"rank": position, **asdict(cluster)} diff --git a/scripts/dupscore/_helpers/clones/rust_units.py b/scripts/dupscore/_helpers/clones/rust_units.py index 5b92341774..681ea75e9e 100644 --- a/scripts/dupscore/_helpers/clones/rust_units.py +++ b/scripts/dupscore/_helpers/clones/rust_units.py @@ -100,6 +100,7 @@ _RUST_SUFFIX: str = ".rs" _DIRECTORY_SUFFIX: str = "/" _OWNER_SEPARATOR: str = "::" +_CALL_SUFFIXES: frozenset[str] = frozenset({"(", "!"}) def extract_rust_units( @@ -123,10 +124,27 @@ def extract_rust_units( ) -def _normalize(item: RustToken) -> str: - if item.kind == RUST_KIND_IDENTIFIER: - return item.text if item.text in _RUST_KEYWORDS else PLACEHOLDER_IDENTIFIER - return _LITERAL_PLACEHOLDERS.get(item.kind, item.text) +def _normalize_stream(tokens: list[RustToken]) -> list[str]: + normalized: list[str] = [] + for index, item in enumerate(tokens): + if item.kind == RUST_KIND_IDENTIFIER: + keep: bool = item.text in _RUST_KEYWORDS or _is_call_target(tokens=tokens, index=index) + normalized.append(item.text if keep else PLACEHOLDER_IDENTIFIER) + else: + normalized.append(_LITERAL_PLACEHOLDERS.get(item.kind, item.text)) + return normalized + + +def _is_call_target(*, tokens: list[RustToken], index: int) -> bool: + """Name a called function, method, or macro: an identifier directly before ``(`` or ``!``.""" + + following: int = index + 1 + if following >= len(tokens): + return False + after: RustToken = tokens[following] + if after.kind != RUST_KIND_PUNCTUATION or after.text not in _CALL_SUFFIXES: + return False + return index == 0 or tokens[index - 1].text != _FUNCTION_KEYWORD def _match_brackets(tokens: list[RustToken]) -> list[int]: @@ -341,7 +359,7 @@ def _emit_unit(self, *, start: int, close: int, owner: str | None, item_start: i name=f"{owner}{_OWNER_SEPARATOR}{function_name}" if owner else function_name, start_line=self._tokens[item_start].line, end_line=self._tokens[close].line, - normalized=[_normalize(item) for item in body], + normalized=_normalize_stream(body), concrete=[item.text for item in body], ) ) diff --git a/scripts/dupscore/_helpers/contracts/__init__.py b/scripts/dupscore/_helpers/contracts/__init__.py new file mode 100644 index 0000000000..43717e6c56 --- /dev/null +++ b/scripts/dupscore/_helpers/contracts/__init__.py @@ -0,0 +1 @@ +"""Dupscore clones helpers.""" diff --git a/scripts/dupscore/_helpers/contracts/forced_overrides.py b/scripts/dupscore/_helpers/contracts/forced_overrides.py new file mode 100644 index 0000000000..8186379757 --- /dev/null +++ b/scripts/dupscore/_helpers/contracts/forced_overrides.py @@ -0,0 +1,251 @@ +"""Recognise forced overrides of contract methods, derived statically from the analysed source.""" + +from __future__ import annotations + +import ast +from dataclasses import dataclass +from pathlib import Path, PurePosixPath + +from scripts.dupscore._helpers.clones.filters import path_matches_any +from scripts.dupscore.constants import LANGUAGE_PYTHON, SOURCE_ROOT +from scripts.dupscore.exceptions import DupscoreConfigError +from scripts.dupscore.models import CloneUnit, ContractExemptionEntry + +_ABSTRACT_DECORATOR: str = "abstractmethod" +_MODULE_SEPARATOR: str = "." +_PYTHON_SUFFIX: str = ".py" +_PACKAGE_INIT: str = "__init__.py" +_METHOD_NAME_PARTS: int = 2 + +type _FunctionNode = ast.FunctionDef | ast.AsyncFunctionDef +type _ClassKey = tuple[str, str] + + +@dataclass(frozen=True, slots=True) +class _ClassInfo: + bases: tuple[_ClassKey, ...] + concrete: frozenset[str] + abstract: frozenset[str] + + +@dataclass(frozen=True, slots=True) +class _ContractScope: + contract: _ClassKey + method_names: frozenset[str] + forbidden_owners: frozenset[_ClassKey] + paths: tuple[str, ...] + + +def find_forced_overrides( + *, + repo_root: Path, + entries: tuple[ContractExemptionEntry, ...], + units: list[CloneUnit], +) -> tuple[bool, ...]: + """Flag each unit that overrides a contract method it may not inherit from an ancestor.""" + + exemptions: _ContractExemptions = _ContractExemptions(repo_root=repo_root, entries=entries) + return tuple(exemptions.is_forced(unit) for unit in units) + + +class _ContractExemptions: + def __init__(self, *, repo_root: Path, entries: tuple[ContractExemptionEntry, ...]) -> None: + self._resolver: _ClassResolver = _ClassResolver(repo_root=repo_root) + active: list[ContractExemptionEntry] = [ + entry for entry in entries if (repo_root / entry.contract_path).is_file() + ] + self._scopes: tuple[_ContractScope, ...] = tuple(self._scope(entry) for entry in active) + + def is_forced(self, unit: CloneUnit) -> bool: + """Require a top-level class method under the paths that derives from the contract.""" + + if unit.language != LANGUAGE_PYTHON: + return False + parts: list[str] = unit.name.split(_MODULE_SEPARATOR) + if len(parts) != _METHOD_NAME_PARTS: + return False + class_name, method_name = parts + for scope in self._scopes: + if method_name not in scope.method_names: + continue + if not path_matches_any(path=unit.path, globs=scope.paths): + continue + if self._forced_in_scope( + scope=scope, key=(unit.path, class_name), method_name=method_name + ): + return True + return False + + def _forced_in_scope(self, *, scope: _ContractScope, key: _ClassKey, method_name: str) -> bool: + order: tuple[_ClassKey, ...] = self._resolver.method_resolution_order(key) + if scope.contract not in order: + return False + for ancestor in order[1:]: + if method_name in self._resolver.info(ancestor).concrete: + return ancestor in scope.forbidden_owners + return True + + def _scope(self, entry: ContractExemptionEntry) -> _ContractScope: + contract: _ClassKey = (entry.contract_path, entry.contract_class) + forbidden: set[_ClassKey] = {contract} + for key in (contract, *entry.forbidden_owners): + if not self._resolver.exists(key): + raise DupscoreConfigError( + f"contract_exemption class {key[0]}:{key[1]} was not found in the analysed " + "source" + ) + forbidden.add(key) + return _ContractScope( + contract=contract, + method_names=self._resolver.abstract_methods(contract), + forbidden_owners=frozenset(forbidden), + paths=entry.paths, + ) + + +class _ClassResolver: + def __init__(self, *, repo_root: Path) -> None: + self._repo_root: Path = repo_root + self._trees: dict[str, ast.Module | None] = {} + self._infos: dict[_ClassKey, _ClassInfo] = {} + self._orders: dict[_ClassKey, tuple[_ClassKey, ...]] = {} + self._abstract: dict[_ClassKey, frozenset[str]] = {} + + def exists(self, key: _ClassKey) -> bool: + tree: ast.Module | None = self._parse(key[0]) + return tree is not None and _find_class(tree=tree, name=key[1]) is not None + + def info(self, key: _ClassKey) -> _ClassInfo: + if key not in self._infos: + self._infos[key] = self._build_info(key) + return self._infos[key] + + def abstract_methods(self, key: _ClassKey) -> frozenset[str]: + """Mirror ``__abstractmethods__``: own abstract methods plus unoverridden inherited ones.""" + + if key in self._abstract: + return self._abstract[key] + self._abstract[key] = frozenset() + info: _ClassInfo = self.info(key) + names: set[str] = set(info.abstract) + for base in info.bases: + names.update(self.abstract_methods(base) - info.concrete) + self._abstract[key] = frozenset(names) + return self._abstract[key] + + def method_resolution_order(self, key: _ClassKey) -> tuple[_ClassKey, ...]: + """C3-linearise the statically resolvable bases, as ``type.__mro__`` would.""" + + if key in self._orders: + return self._orders[key] + self._orders[key] = (key,) + bases: tuple[_ClassKey, ...] = self.info(key).bases + sequences: list[list[_ClassKey]] = [ + list(self.method_resolution_order(base)) for base in bases + ] + self._orders[key] = (key, *_c3_merge([*sequences, list(bases)])) + return self._orders[key] + + def _parse(self, relative_path: str) -> ast.Module | None: + if relative_path not in self._trees: + try: + source: str = (self._repo_root / relative_path).read_text(encoding="utf-8") + self._trees[relative_path] = ast.parse(source) + except (OSError, SyntaxError, UnicodeDecodeError, ValueError): + self._trees[relative_path] = None + return self._trees[relative_path] + + def _build_info(self, key: _ClassKey) -> _ClassInfo: + tree: ast.Module | None = self._parse(key[0]) + node: ast.ClassDef | None = None if tree is None else _find_class(tree=tree, name=key[1]) + if tree is None or node is None: + return _ClassInfo(bases=(), concrete=frozenset(), abstract=frozenset()) + functions: list[_FunctionNode] = [ + item for item in node.body if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) + ] + bases: list[_ClassKey] = [] + for base in node.bases: + resolved: _ClassKey | None = self._resolve_base(tree=tree, importer=key[0], base=base) + if resolved is not None: + bases.append(resolved) + return _ClassInfo( + bases=tuple(bases), + concrete=frozenset(item.name for item in functions if not _is_abstract(item)), + abstract=frozenset(item.name for item in functions if _is_abstract(item)), + ) + + def _resolve_base(self, *, tree: ast.Module, importer: str, base: ast.expr) -> _ClassKey | None: + if not isinstance(base, ast.Name): + return None + if _find_class(tree=tree, name=base.id) is not None: + return (importer, base.id) + for statement in tree.body: + if not isinstance(statement, ast.ImportFrom): + continue + for alias in statement.names: + if (alias.asname or alias.name) == base.id: + module_path: str | None = self._module_path( + importer=importer, module=statement.module, level=statement.level + ) + return None if module_path is None else (module_path, alias.name) + return None + + def _module_path(self, *, importer: str, module: str | None, level: int) -> str | None: + if not module: + return None + parts: list[str] = module.split(_MODULE_SEPARATOR) + if level: + anchor: PurePosixPath = PurePosixPath(importer).parent + for _ in range(level - 1): + anchor = anchor.parent + bases: list[PurePosixPath] = [anchor.joinpath(*parts)] + else: + bases = [PurePosixPath(SOURCE_ROOT).joinpath(*parts), PurePosixPath(*parts)] + for base in bases: + for candidate in (base.with_name(base.name + _PYTHON_SUFFIX), base / _PACKAGE_INIT): + if (self._repo_root / candidate).is_file(): + return candidate.as_posix() + return None + + +def _c3_merge(sequences: list[list[_ClassKey]]) -> list[_ClassKey]: + """Merge linearisations; an inconsistent hierarchy falls back to first-seen order.""" + + merged: list[_ClassKey] = [] + pending: list[list[_ClassKey]] = [sequence for sequence in sequences if sequence] + while pending: + head: _ClassKey = _c3_head(pending) + merged.append(head) + remaining: list[list[_ClassKey]] = [] + for sequence in pending: + rest: list[_ClassKey] = [item for item in sequence if item != head] + if rest: + remaining.append(rest) + pending = remaining + return merged + + +def _c3_head(pending: list[list[_ClassKey]]) -> _ClassKey: + tails: set[_ClassKey] = set() + for sequence in pending: + tails.update(sequence[1:]) + for sequence in pending: + if sequence[0] not in tails: + return sequence[0] + return pending[0][0] + + +def _find_class(*, tree: ast.Module, name: str) -> ast.ClassDef | None: + for statement in tree.body: + if isinstance(statement, ast.ClassDef) and statement.name == name: + return statement + return None + + +def _is_abstract(function: _FunctionNode) -> bool: + for decorator in function.decorator_list: + if isinstance(decorator, ast.Name) and decorator.id == _ABSTRACT_DECORATOR: + return True + if isinstance(decorator, ast.Attribute) and decorator.attr == _ABSTRACT_DECORATOR: + return True + return False diff --git a/scripts/dupscore/_helpers/inputs/config.py b/scripts/dupscore/_helpers/inputs/config.py index 25bce159c2..281e452e58 100644 --- a/scripts/dupscore/_helpers/inputs/config.py +++ b/scripts/dupscore/_helpers/inputs/config.py @@ -6,7 +6,7 @@ from pathlib import Path from scripts.dupscore.exceptions import DupscoreConfigError -from scripts.dupscore.models import CloneAllowlistEntry, DupscoreConfig +from scripts.dupscore.models import CloneAllowlistEntry, ContractExemptionEntry, DupscoreConfig _ALLOWLIST_KEY: str = "allowlist" _SURFACES_KEY: str = "persisted_state_surfaces" @@ -16,6 +16,17 @@ _CLONE_ALLOWLIST_KEY: str = "clone_allowlist" _PATHS_KEY: str = "paths" _CLONE_ALLOWLIST_KEYS: frozenset[str] = frozenset({_PATHS_KEY, _REASON_KEY}) +_CONTRACT_EXEMPTION_KEY: str = "contract_exemption" +_CONTRACT_KEY: str = "contract" +_FORBIDDEN_OWNERS_KEY: str = "forbidden_owners" +_CONTRACT_EXEMPTION_KEYS: frozenset[str] = frozenset( + {_CONTRACT_KEY, _FORBIDDEN_OWNERS_KEY, _PATHS_KEY, _REASON_KEY} +) +_CLASS_SPEC_SEPARATOR: str = ":" +_CLASS_SPEC_PARTS: int = 2 +_CLASS_SPEC_FORMAT: str = "'relative/path.py:ClassName'" +_PYTHON_SUFFIX: str = ".py" +_ABSOLUTE_PATH_PREFIX: str = "/" def load_config(config_path: Path) -> DupscoreConfig: @@ -55,37 +66,94 @@ def load_config(config_path: Path) -> DupscoreConfig: persisted_state_surfaces=tuple(sorted(surfaces)), allowlisted_pairs=allowlisted_pairs, clone_allowlist=_load_clone_allowlist(raw.get(_CLONE_ALLOWLIST_KEY, [])), + contract_exemptions=_load_contract_exemptions(raw.get(_CONTRACT_EXEMPTION_KEY, [])), ) def _load_clone_allowlist(raw: object) -> tuple[CloneAllowlistEntry, ...]: + return tuple( + CloneAllowlistEntry( + paths=_glob_list(item=item, label=label), reason=_reason(item=item, label=label) + ) + for label, item in _table_entries( + raw=raw, key=_CLONE_ALLOWLIST_KEY, allowed=_CLONE_ALLOWLIST_KEYS + ) + ) + + +def _load_contract_exemptions(raw: object) -> tuple[ContractExemptionEntry, ...]: + entries: list[ContractExemptionEntry] = [] + for label, item in _table_entries( + raw=raw, key=_CONTRACT_EXEMPTION_KEY, allowed=_CONTRACT_EXEMPTION_KEYS + ): + contract_path, contract_class = _class_spec( + raw=item.get(_CONTRACT_KEY), label=f"{label} {_CONTRACT_KEY}" + ) + owners_raw: object = item.get(_FORBIDDEN_OWNERS_KEY, []) + if not isinstance(owners_raw, list): + raise DupscoreConfigError( + f"{label} {_FORBIDDEN_OWNERS_KEY} must be a list of {_CLASS_SPEC_FORMAT} strings" + ) + entries.append( + ContractExemptionEntry( + contract_path=contract_path, + contract_class=contract_class, + paths=_glob_list(item=item, label=label), + reason=_reason(item=item, label=label), + forbidden_owners=tuple( + _class_spec(raw=owner, label=f"{label} {_FORBIDDEN_OWNERS_KEY}") + for owner in owners_raw + ), + ) + ) + return tuple(entries) + + +def _table_entries( + *, raw: object, key: str, allowed: frozenset[str] +) -> list[tuple[str, dict[str, object]]]: if not isinstance(raw, list): - raise DupscoreConfigError(f"{_CLONE_ALLOWLIST_KEY} must be an array of tables") - entries: list[CloneAllowlistEntry] = [] + raise DupscoreConfigError(f"{key} must be an array of tables") + entries: list[tuple[str, dict[str, object]]] = [] for position, raw_item in enumerate(raw, start=1): - label: str = f"{_CLONE_ALLOWLIST_KEY} entry {position}" + label: str = f"{key} entry {position}" if not isinstance(raw_item, dict): raise DupscoreConfigError(f"{label} must be a table") - item: dict[str, object] = {str(key): value for key, value in raw_item.items()} - unknown: set[str] = set(item) - _CLONE_ALLOWLIST_KEYS + item: dict[str, object] = {str(name): value for name, value in raw_item.items()} + unknown: set[str] = set(item) - allowed if unknown: raise DupscoreConfigError(f"{label} has unknown keys: {', '.join(sorted(unknown))}") - paths_raw: object = item.get(_PATHS_KEY) - if ( - not isinstance(paths_raw, list) - or not paths_raw - or not all(isinstance(path, str) and path.strip() for path in paths_raw) - ): - raise DupscoreConfigError( - f"{label} needs a non-empty {_PATHS_KEY} list of non-empty glob strings" - ) - reason_raw: object = item.get(_REASON_KEY) - if not isinstance(reason_raw, str) or not reason_raw.strip(): - raise DupscoreConfigError(f"{label} needs a non-empty {_REASON_KEY}") - entries.append( - CloneAllowlistEntry( - paths=tuple(str(path) for path in paths_raw), - reason=reason_raw.strip(), - ) + entries.append((label, item)) + return entries + + +def _glob_list(*, item: dict[str, object], label: str) -> tuple[str, ...]: + paths_raw: object = item.get(_PATHS_KEY) + if ( + not isinstance(paths_raw, list) + or not paths_raw + or not all(isinstance(path, str) and path.strip() for path in paths_raw) + ): + raise DupscoreConfigError( + f"{label} needs a non-empty {_PATHS_KEY} list of non-empty glob strings" ) - return tuple(entries) + return tuple(str(path) for path in paths_raw) + + +def _reason(*, item: dict[str, object], label: str) -> str: + reason_raw: object = item.get(_REASON_KEY) + if not isinstance(reason_raw, str) or not reason_raw.strip(): + raise DupscoreConfigError(f"{label} needs a non-empty {_REASON_KEY}") + return reason_raw.strip() + + +def _class_spec(*, raw: object, label: str) -> tuple[str, str]: + parts: list[str] = raw.split(_CLASS_SPEC_SEPARATOR) if isinstance(raw, str) else [] + if ( + len(parts) != _CLASS_SPEC_PARTS + or not parts[0].endswith(_PYTHON_SUFFIX) + or parts[0].startswith(_ABSOLUTE_PATH_PREFIX) + or not parts[1].isidentifier() + ): + raise DupscoreConfigError(f"{label} must be {_CLASS_SPEC_FORMAT}") + return parts[0], parts[1] diff --git a/scripts/dupscore/constants.py b/scripts/dupscore/constants.py index 9c6f345e55..4c9b5f88ff 100644 --- a/scripts/dupscore/constants.py +++ b/scripts/dupscore/constants.py @@ -68,6 +68,8 @@ CLONE_MIN_FINGERPRINTS: int = 6 CLONE_CANDIDATE_MIN_JACCARD: float = 0.2 CLONE_MAX_FINGERPRINT_UNITS: int = 25 +CLONE_SMALL_NEAR_MISS_TOKENS: int = 80 +CLONE_SMALL_NEAR_MISS_MIN_SIMILARITY: float = 0.9 DEFAULT_CLONE_MIN_TOKENS: int = 60 DEFAULT_CLONE_MIN_SIMILARITY: float = 0.8 DEFAULT_CLONE_TOP_RESULTS: int = 30 diff --git a/scripts/dupscore/dupscore.toml b/scripts/dupscore/dupscore.toml index 39c2df498e..e4e2061317 100644 --- a/scripts/dupscore/dupscore.toml +++ b/scripts/dupscore/dupscore.toml @@ -47,3 +47,17 @@ reason = "Adapter backends intentionally conform to the shared adapter contract. # [[clone_allowlist]] # paths = ["src/sqlbuild/example/backends/*"] # reason = "Backends intentionally mirror one protocol." + +# Clone mode: a method is a forced override when it is named after an abstract method of +# `contract` (own or inherited, parsed statically from the analysed source), is defined in a +# class under `paths` that derives from `contract`, and would otherwise be inherited from +# `contract`, a `forbidden_owners` class, or nowhere. Links between two forced overrides are +# hidden; links to any other unit are still reported. Every entry needs a non-empty reason. +[[contract_exemption]] +contract = "src/sqlbuild/adapter/contract/classes/strict_adapter.py:StrictAdapter" +paths = ["src/sqlbuild/adapter/contract/classes/*", "src/sqlbuild/adapters/*/classes/*"] +forbidden_owners = ["src/sqlbuild/adapter/contract/classes/base_adapter.py:BaseAdapter"] +reason = """\ +tests/unit/src/sqlbuild/adapter/contract/classes/strict_adapter/test_strict_adapter.py requires \ +every first-class adapter to define each StrictAdapter abstract method itself instead of \ +inheriting it from BaseAdapter or StrictAdapter, so identical overrides are intentional.""" diff --git a/scripts/dupscore/main/build_clone_report.py b/scripts/dupscore/main/build_clone_report.py index 34d5ab7447..cded4e0a29 100644 --- a/scripts/dupscore/main/build_clone_report.py +++ b/scripts/dupscore/main/build_clone_report.py @@ -15,6 +15,7 @@ from scripts.dupscore._helpers.clones.fingerprints import fingerprint_tokens from scripts.dupscore._helpers.clones.matching import find_clone_pairs from scripts.dupscore._helpers.clones.sources import collect_clone_units +from scripts.dupscore._helpers.contracts.forced_overrides import find_forced_overrides from scripts.dupscore.models import ( CloneCluster, CloneOptions, @@ -49,9 +50,19 @@ def build_clone_report( fingerprints=fingerprints, min_similarity=options.min_similarity, ) + paired: list[int] = sorted(_paired_units(candidate_pairs)) + flags: tuple[bool, ...] = find_forced_overrides( + repo_root=repo_root, + entries=config.contract_exemptions, + units=[units[unit_index] for unit_index in paired], + ) + forced: set[int] = {unit_index for unit_index, flag in zip(paired, flags, strict=True) if flag} + retained_pairs: list[ClonePair] = [ + pair for pair in candidate_pairs if pair.left not in forced or pair.right not in forced + ] pairs: list[ClonePair] = [ pair - for pair in candidate_pairs + for pair in retained_pairs if allowlist_reason( left_path=units[pair.left].path, right_path=units[pair.right].path, @@ -79,11 +90,19 @@ def change_of(unit: CloneUnit) -> str | None: return CloneReport( since=options.since, unit_counts={language: unit_counts[language] for language in options.languages}, - allowlisted_pairs=len(candidate_pairs) - len(pairs), + allowlisted_pairs=len(retained_pairs) - len(pairs), + contract_exempt_members=len(forced - _paired_units(retained_pairs)), clusters=tuple(clusters), ) +def _paired_units(pairs: list[ClonePair]) -> set[int]: + indexes: set[int] = set() + for pair in pairs: + indexes.update((pair.left, pair.right)) + return indexes + + def _cluster_selected(*, cluster: CloneCluster, options: CloneOptions) -> bool: if options.since is not None and all(member.change is None for member in cluster.members): return False diff --git a/scripts/dupscore/models.py b/scripts/dupscore/models.py index a086b39c6a..a22df54912 100644 --- a/scripts/dupscore/models.py +++ b/scripts/dupscore/models.py @@ -127,6 +127,17 @@ class CloneAllowlistEntry: reason: str +@dataclass(frozen=True, slots=True) +class ContractExemptionEntry: + """Methods of a contract class that classes under ``paths`` must each define themselves.""" + + contract_path: str + contract_class: str + paths: tuple[str, ...] + reason: str + forbidden_owners: tuple[tuple[str, str], ...] = () + + @dataclass(frozen=True, slots=True) class DupscoreConfig: """User configuration loaded from dupscore.toml.""" @@ -134,6 +145,7 @@ class DupscoreConfig: persisted_state_surfaces: tuple[str, ...] = () allowlisted_pairs: dict[tuple[str, str], str] = field(default_factory=dict) clone_allowlist: tuple[CloneAllowlistEntry, ...] = () + contract_exemptions: tuple[ContractExemptionEntry, ...] = () @dataclass(frozen=True, slots=True) @@ -223,6 +235,7 @@ class CloneReport: since: str | None unit_counts: dict[str, int] allowlisted_pairs: int + contract_exempt_members: int clusters: tuple[CloneCluster, ...] diff --git a/tests/unit/scripts/dupscore/_helpers/clones/matching/_test_types.py b/tests/unit/scripts/dupscore/_helpers/clones/matching/_test_types.py new file mode 100644 index 0000000000..12f2929a7f --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/clones/matching/_test_types.py @@ -0,0 +1,12 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class NearMissFloorTestCase: + description: str + shared_tokens: int + differing_tokens: int + min_similarity: float + expected_categories: list[str] diff --git a/tests/unit/scripts/dupscore/_helpers/clones/matching/helpers.py b/tests/unit/scripts/dupscore/_helpers/clones/matching/helpers.py new file mode 100644 index 0000000000..7107a9233d --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/clones/matching/helpers.py @@ -0,0 +1,20 @@ +from __future__ import annotations + +from scripts.dupscore._helpers.clones.tokens import build_clone_unit +from scripts.dupscore.models import CloneUnit + + +def diverging_unit(*, name: str, shared_tokens: int, differing_tokens: int) -> CloneUnit: + """Build a unit whose stream starts with common tokens and ends with side-specific ones.""" + + tokens: list[str] = [f"shared{index}" for index in range(shared_tokens)] + tokens.extend(f"{name}{index}" for index in range(differing_tokens)) + return build_clone_unit( + language="python", + path=f"src/sqlbuild/{name}.py", + name=name, + start_line=1, + end_line=len(tokens), + normalized=tokens, + concrete=tokens, + ) diff --git a/tests/unit/scripts/dupscore/_helpers/clones/matching/test_find_clone_pairs.py b/tests/unit/scripts/dupscore/_helpers/clones/matching/test_find_clone_pairs.py new file mode 100644 index 0000000000..c10de2e878 --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/clones/matching/test_find_clone_pairs.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +import pytest + +from scripts.dupscore._helpers.clones.fingerprints import fingerprint_tokens +from scripts.dupscore._helpers.clones.matching import find_clone_pairs +from scripts.dupscore.constants import CATEGORY_NEAR_MISS +from scripts.dupscore.models import ClonePair, CloneUnit +from tests.unit.scripts.dupscore._helpers.clones.matching._test_types import ( + NearMissFloorTestCase, +) +from tests.unit.scripts.dupscore._helpers.clones.matching.helpers import diverging_unit + + +@pytest.mark.parametrize( + "test_case", + [ + NearMissFloorTestCase( + description="short pair below the small-unit similarity floor is dropped", + shared_tokens=60, + differing_tokens=10, + min_similarity=0.8, + expected_categories=[], + ), + NearMissFloorTestCase( + description="short pair above the small-unit similarity floor is kept", + shared_tokens=66, + differing_tokens=4, + min_similarity=0.8, + expected_categories=[CATEGORY_NEAR_MISS], + ), + NearMissFloorTestCase( + description="larger pair with the same ratio uses the configured threshold", + shared_tokens=86, + differing_tokens=14, + min_similarity=0.8, + expected_categories=[CATEGORY_NEAR_MISS], + ), + NearMissFloorTestCase( + description="a stricter configured threshold still applies to short pairs", + shared_tokens=66, + differing_tokens=4, + min_similarity=0.95, + expected_categories=[], + ), + ], + ids=lambda case: case.description, +) +def test_given_near_miss_pair_when_matching_then_applies_size_dependent_floor( + test_case: NearMissFloorTestCase, +) -> None: + units: list[CloneUnit] = [ + diverging_unit( + name=side, + shared_tokens=test_case.shared_tokens, + differing_tokens=test_case.differing_tokens, + ) + for side in ("left", "right") + ] + + pairs: list[ClonePair] = find_clone_pairs( + units=units, + fingerprints=[fingerprint_tokens(unit.normalized) for unit in units], + min_similarity=test_case.min_similarity, + ) + + assert [pair.category for pair in pairs] == test_case.expected_categories diff --git a/tests/unit/scripts/dupscore/_helpers/clones/python_units/test_extract_python_units.py b/tests/unit/scripts/dupscore/_helpers/clones/python_units/test_extract_python_units.py index 2b187c1986..23178402ac 100644 --- a/tests/unit/scripts/dupscore/_helpers/clones/python_units/test_extract_python_units.py +++ b/tests/unit/scripts/dupscore/_helpers/clones/python_units/test_extract_python_units.py @@ -93,7 +93,42 @@ def test_given_python_source_when_extracting_units_then_returns_expected_units( PythonKeyTestCase( description="renamed identifiers and literals share the normalised stream", left_source='def f(a):\n return g(a, "x", 1)\n', - right_source='def other(value):\n return h(value, "y", 2)\n', + right_source='def other(value):\n return g(value, "y", 2)\n', + expected_same_concrete=False, + expected_same_normalized=True, + ), + PythonKeyTestCase( + description="different called functions keep distinct normalised streams", + left_source='def f(a):\n return g(a, "x", 1)\n', + right_source='def f(a):\n return h(a, "x", 1)\n', + expected_same_concrete=False, + expected_same_normalized=False, + ), + PythonKeyTestCase( + description="different called methods keep distinct normalised streams", + left_source="def f(orders):\n return orders.load(1)\n", + right_source="def f(orders):\n return orders.save(1)\n", + expected_same_concrete=False, + expected_same_normalized=False, + ), + PythonKeyTestCase( + description="call receivers and plain attribute reads stay abstracted", + left_source="def f(orders):\n return orders.total(orders.count)\n", + right_source="def f(items):\n return items.total(items.size)\n", + expected_same_concrete=False, + expected_same_normalized=True, + ), + PythonKeyTestCase( + description="method calls after non-ascii text keep their call target", + left_source='def f(a):\n label = "caf\u00e9"; return label.upper()\n', + right_source='def f(a):\n label = "caf\u00e9"; return label.lower()\n', + expected_same_concrete=False, + expected_same_normalized=False, + ), + PythonKeyTestCase( + description="non-ascii literals before the same method call stay renamed", + left_source='def f(a):\n label = "caf\u00e9"; return label.upper()\n', + right_source='def f(b):\n name = "cr\u00e8me"; return name.upper()\n', expected_same_concrete=False, expected_same_normalized=True, ), diff --git a/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py b/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py new file mode 100644 index 0000000000..ca1ebfd5fb --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py @@ -0,0 +1,11 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class HiddenCountsTestCase: + description: str + allowlisted_pairs: int + contract_exempt_members: int + expected_summary_fragment: str diff --git a/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py b/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py new file mode 100644 index 0000000000..9bcba0d47a --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py @@ -0,0 +1,44 @@ +from __future__ import annotations + +import json + +import pytest + +from scripts.dupscore._helpers.clones.rendering import render_clone_json, render_clone_text +from scripts.dupscore.models import CloneReport +from tests.unit.scripts.dupscore._helpers.clones.rendering._test_types import ( + HiddenCountsTestCase, +) + + +@pytest.mark.parametrize( + "test_case", + [ + HiddenCountsTestCase( + description="summary reports allowlisted pairs and contract-exempt members", + allowlisted_pairs=2, + contract_exempt_members=5, + expected_summary_fragment=( + "2 allowlisted pairs hidden; 5 contract-exempt members hidden" + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_hidden_counts_when_rendering_then_text_and_json_report_them( + test_case: HiddenCountsTestCase, +) -> None: + report: CloneReport = CloneReport( + since=None, + unit_counts={"python": 3}, + allowlisted_pairs=test_case.allowlisted_pairs, + contract_exempt_members=test_case.contract_exempt_members, + clusters=(), + ) + + text: str = render_clone_text(report=report, top=5) + payload: dict[str, object] = json.loads(render_clone_json(report=report, top=5)) + + assert test_case.expected_summary_fragment in text.splitlines()[0] + assert payload["allowlisted_pairs"] == test_case.allowlisted_pairs + assert payload["contract_exempt_members"] == test_case.contract_exempt_members diff --git a/tests/unit/scripts/dupscore/_helpers/clones/rust_units/test_extract_rust_units.py b/tests/unit/scripts/dupscore/_helpers/clones/rust_units/test_extract_rust_units.py index 825cca5b80..807e22dbf0 100644 --- a/tests/unit/scripts/dupscore/_helpers/clones/rust_units/test_extract_rust_units.py +++ b/tests/unit/scripts/dupscore/_helpers/clones/rust_units/test_extract_rust_units.py @@ -171,7 +171,35 @@ def test_given_cfg_test_module_declaration_when_extracting_then_reports_test_pat RustKeyTestCase( description="renamed identifiers, literals, and visibility share the normalised stream", left_source='fn count_orders(x: u8) -> u8 { let y = x + 1; println!("{y}"); y }', - right_source='pub fn tally(value: u16) -> u16 { let n = value + 9; print!("n"); n }', + right_source='pub fn tally(value: u16) -> u16 { let n = value + 9; println!("n"); n }', + expected_same_concrete=False, + expected_same_normalized=True, + ), + RustKeyTestCase( + description="different macros keep distinct normalised streams", + left_source='fn f(x: u8) { println!("{x}"); }', + right_source='fn f(x: u8) { print!("{x}"); }', + expected_same_concrete=False, + expected_same_normalized=False, + ), + RustKeyTestCase( + description="different called functions keep distinct normalised streams", + left_source="fn f(x: u8) -> u8 { load(x) }", + right_source="fn f(x: u8) -> u8 { save(x) }", + expected_same_concrete=False, + expected_same_normalized=False, + ), + RustKeyTestCase( + description="different called methods keep distinct normalised streams", + left_source="fn f(x: &str) -> usize { x.len() }", + right_source="fn f(x: &str) -> usize { x.count() }", + expected_same_concrete=False, + expected_same_normalized=False, + ), + RustKeyTestCase( + description="field reads and receivers stay abstracted", + left_source="fn f(order: &Order) -> u8 { order.total(order.count) }", + right_source="fn g(item: &Item) -> u8 { item.total(item.size) }", expected_same_concrete=False, expected_same_normalized=True, ), diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/_test_types.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/_test_types.py new file mode 100644 index 0000000000..ac19279dc4 --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/_test_types.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +from dataclasses import dataclass + +from scripts.dupscore.models import ContractExemptionEntry + + +@dataclass(frozen=True) +class ForcedOverrideTestCase: + description: str + language: str + path: str + name: str + expected_forced: bool + + +@dataclass(frozen=True) +class ContractLookupErrorTestCase: + description: str + entry: ContractExemptionEntry + expected_error_fragment: str + + +@dataclass(frozen=True) +class InactiveContractTestCase: + description: str + entry: ContractExemptionEntry + path: str + name: str + expected_forced: bool diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py new file mode 100644 index 0000000000..c2395492e8 --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py @@ -0,0 +1,170 @@ +from __future__ import annotations + +from scripts.dupscore._helpers.clones.tokens import build_clone_unit +from scripts.dupscore.models import CloneUnit, ContractExemptionEntry + +CONTRACT_PATH: str = "src/sqlbuild/demo/contract/strict_store.py" +BASE_PATH: str = "src/sqlbuild/demo/contract/base_store.py" +ALPHA_PATH: str = "src/sqlbuild/demo/stores/alpha/classes/alpha_store.py" +BETA_PATH: str = "src/sqlbuild/demo/stores/beta/classes/beta_store.py" +GAMMA_PATH: str = "src/sqlbuild/demo/stores/gamma/classes/gamma_store.py" +CONNECTION_PATH: str = "src/sqlbuild/demo/stores/beta/classes/orders_connection.py" +OUTSIDE_PATH: str = "src/sqlbuild/demo/elsewhere/outside_store.py" +DELTA_PATH: str = "src/sqlbuild/demo/stores/delta/classes/delta_store.py" + +_MIXIN_SOURCE: str = """\ +from abc import abstractmethod + + +class ReaderMixin: + @abstractmethod + def read_orders(self, connection, keys): + ... + + @abstractmethod + def count_orders(self, connection): + ... +""" + +_CONTRACT_SOURCE: str = """\ +import abc +from abc import abstractmethod + +from sqlbuild.demo.contract.reader_mixin import ReaderMixin as Reader + + +class StrictStore(Reader): + @abstractmethod + def write_orders(self, connection, rows): + ... + + @abc.abstractmethod + def delete_orders(self, connection, keys): + ... + + @property + @abstractmethod + def store_name(self): + ... + + def count_orders(self, connection): + return 0 +""" + +WRITE_ORDERS: str = """ + def write_orders(self, connection, rows): + written = 0 + for row in rows: + if row.get("status") == "cancelled": + continue + values = [row["order_id"], row["customer"], row["amount"]] + connection.execute("INSERT INTO orders VALUES (?, ?, ?)", values) + written += 1 + connection.commit() + return written +""" + +DELETE_ORDERS: str = """ + def delete_orders(self, connection, keys): + pending = sorted(set(keys)) + while pending: + batch = pending[:50] + pending = pending[50:] + placeholders = ", ".join("?" for _ in batch) + try: + connection.execute(f"DELETE FROM orders WHERE id IN ({placeholders})", batch) + except KeyError as error: + raise ValueError(str(error)) from error + return len(keys) +""" + +READ_ORDERS: str = """ + def read_orders(self, connection, keys): + cursor = connection.cursor() + results = {} + with cursor: + for key in keys: + cursor.execute("SELECT * FROM orders WHERE id = ?", [key]) + fetched = cursor.fetchone() + if fetched is None: + results[key] = None + else: + results[key] = dict(zip(("id", "customer", "amount"), fetched)) + return results +""" + +RENDER_ORDERS_SQL: str = """ + def _render_orders_sql(self, table, columns, predicates): + selected = ", ".join(f'"{column}"' for column in columns) or "*" + clauses = [] + for name, value in predicates.items(): + if value is None: + clauses.append(f"{name} IS NULL") + elif isinstance(value, (list, tuple)): + clauses.append(f"{name} IN ({', '.join(repr(item) for item in value)})") + else: + clauses.append(f"{name} = {value!r}") + where = f" WHERE {' AND '.join(clauses)}" if clauses else "" + return f"SELECT {selected} FROM {table}{where}" +""" + +_BASE_IMPORT: str = "from sqlbuild.demo.contract.base_store import BaseStore\n\n\n" + +CONTRACT_FILES: dict[str, str] = { + "src/sqlbuild/demo/contract/reader_mixin.py": _MIXIN_SOURCE, + CONTRACT_PATH: _CONTRACT_SOURCE, + BASE_PATH: ( + "from .strict_store import StrictStore\n\n\nclass BaseStore(StrictStore):\n" + WRITE_ORDERS + ), + ALPHA_PATH: ( + _BASE_IMPORT + + "class AlphaStore(BaseStore):\n" + + WRITE_ORDERS + + DELETE_ORDERS + + RENDER_ORDERS_SQL + + "\n def helper(self):\n return 1\n" + ), + BETA_PATH: ( + _BASE_IMPORT + + "class BetaStore(BaseStore):\n" + + WRITE_ORDERS + + READ_ORDERS + + RENDER_ORDERS_SQL + ), + GAMMA_PATH: ( + "from sqlbuild.demo.stores.alpha.classes.alpha_store import AlphaStore\n\n\n" + "class GammaStore(AlphaStore):\n" + DELETE_ORDERS + ), + CONNECTION_PATH: "class OrdersConnection:\n" + READ_ORDERS, + OUTSIDE_PATH: ( + _BASE_IMPORT + + "class OutsideStore(BaseStore):\n" + + " def write_orders(self, connection, rows):\n return 0\n" + ), +} + +DELTA_FILES: dict[str, str] = { + DELTA_PATH: ( + _BASE_IMPORT + "class DeltaStore(BaseStore):\n" + WRITE_ORDERS + RENDER_ORDERS_SQL + ), +} + +STORE_EXEMPTION: ContractExemptionEntry = ContractExemptionEntry( + contract_path=CONTRACT_PATH, + contract_class="StrictStore", + paths=("src/sqlbuild/demo/contract/*", "src/sqlbuild/demo/stores/*/classes/*"), + reason="Every store must define each StrictStore method itself.", + forbidden_owners=((BASE_PATH, "BaseStore"),), +) + + +def method_unit(*, language: str, path: str, name: str) -> CloneUnit: + return build_clone_unit( + language=language, + path=path, + name=name, + start_line=1, + end_line=2, + normalized=[], + concrete=[], + ) diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py new file mode 100644 index 0000000000..ae5064972c --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py @@ -0,0 +1,235 @@ +from __future__ import annotations + +from dataclasses import replace +from pathlib import Path + +import pytest + +from scripts.dupscore._helpers.contracts.forced_overrides import find_forced_overrides +from scripts.dupscore._helpers.inputs.config import load_config +from scripts.dupscore.exceptions import DupscoreConfigError +from scripts.dupscore.models import DupscoreConfig +from tests.unit.scripts.dupscore._helpers.contracts.forced_overrides._test_types import ( + ContractLookupErrorTestCase, + ForcedOverrideTestCase, + InactiveContractTestCase, +) +from tests.unit.scripts.dupscore._helpers.contracts.forced_overrides.helpers import ( + ALPHA_PATH, + BASE_PATH, + BETA_PATH, + CONNECTION_PATH, + CONTRACT_FILES, + CONTRACT_PATH, + GAMMA_PATH, + OUTSIDE_PATH, + STORE_EXEMPTION, + method_unit, +) +from tests.unit.scripts.dupscore.main.build_report.helpers import write_project_files + +_ADAPTER_CLASSES: str = "src/sqlbuild/adapter/contract/classes" +_ADAPTERS: str = "src/sqlbuild/adapters" + + +@pytest.mark.parametrize( + "test_case", + [ + ForcedOverrideTestCase( + description="base class override of an abstractmethod is forced", + language="python", + path=BASE_PATH, + name="BaseStore.write_orders", + expected_forced=True, + ), + ForcedOverrideTestCase( + description="subclass would otherwise inherit from a forbidden owner", + language="python", + path=ALPHA_PATH, + name="AlphaStore.write_orders", + expected_forced=True, + ), + ForcedOverrideTestCase( + description="abc.abstractmethod declarations are contract methods", + language="python", + path=ALPHA_PATH, + name="AlphaStore.delete_orders", + expected_forced=True, + ), + ForcedOverrideTestCase( + description="abstract methods inherited through an aliased mixin import count", + language="python", + path=BETA_PATH, + name="BetaStore.read_orders", + expected_forced=True, + ), + ForcedOverrideTestCase( + description="stacked property and abstractmethod decorators count", + language="python", + path=BETA_PATH, + name="BetaStore.store_name", + expected_forced=True, + ), + ForcedOverrideTestCase( + description="mixin abstract method made concrete by the contract is not a contract", + language="python", + path=ALPHA_PATH, + name="AlphaStore.count_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="private helper methods are not contract methods", + language="python", + path=ALPHA_PATH, + name="AlphaStore._render_orders_sql", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="override that could inherit from an allowed ancestor is voluntary", + language="python", + path=GAMMA_PATH, + name="GammaStore.delete_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="class that does not derive from the contract is not exempt", + language="python", + path=CONNECTION_PATH, + name="OrdersConnection.read_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="contract subclass outside the listed paths is not exempt", + language="python", + path=OUTSIDE_PATH, + name="OutsideStore.write_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="top-level functions named like contract methods are not exempt", + language="python", + path=ALPHA_PATH, + name="write_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="rust units are never exempt", + language="rust", + path=ALPHA_PATH, + name="AlphaStore.write_orders", + expected_forced=False, + ), + ], + ids=lambda case: case.description, +) +def test_given_contract_exemption_when_checking_unit_then_reports_forced_override( + test_case: ForcedOverrideTestCase, + tmp_path: Path, +) -> None: + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + + forced: tuple[bool, ...] = find_forced_overrides( + repo_root=tmp_path, + entries=(STORE_EXEMPTION,), + units=[method_unit(language=test_case.language, path=test_case.path, name=test_case.name)], + ) + + assert forced == (test_case.expected_forced,) + + +@pytest.mark.parametrize( + "test_case", + [ + ContractLookupErrorTestCase( + description="missing contract class in an existing file", + entry=replace(STORE_EXEMPTION, contract_class="MissingStore"), + expected_error_fragment=f"{CONTRACT_PATH}:MissingStore was not found", + ), + ContractLookupErrorTestCase( + description="missing forbidden owner class", + entry=replace(STORE_EXEMPTION, forbidden_owners=((BASE_PATH, "MissingBase"),)), + expected_error_fragment=f"{BASE_PATH}:MissingBase was not found", + ), + ], + ids=lambda case: case.description, +) +def test_given_unresolvable_contract_class_when_loading_then_raises_config_error( + test_case: ContractLookupErrorTestCase, + tmp_path: Path, +) -> None: + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + + with pytest.raises(DupscoreConfigError) as raised: + _ = find_forced_overrides(repo_root=tmp_path, entries=(test_case.entry,), units=[]) + + assert test_case.expected_error_fragment in str(raised.value) + + +@pytest.mark.parametrize( + "test_case", + [ + InactiveContractTestCase( + description="entry whose contract file is absent exempts nothing", + entry=replace(STORE_EXEMPTION, contract_path="src/sqlbuild/demo/absent.py"), + path=ALPHA_PATH, + name="AlphaStore.write_orders", + expected_forced=False, + ) + ], + ids=lambda case: case.description, +) +def test_given_absent_contract_file_when_checking_unit_then_nothing_is_exempt( + test_case: InactiveContractTestCase, + tmp_path: Path, +) -> None: + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + + forced: tuple[bool, ...] = find_forced_overrides( + repo_root=tmp_path, + entries=(test_case.entry,), + units=[method_unit(language="python", path=test_case.path, name=test_case.name)], + ) + + assert forced == (test_case.expected_forced,) + + +@pytest.mark.parametrize( + "test_case", + [ + ForcedOverrideTestCase( + description="duckdb-backed contract override is forced", + language="python", + path=f"{_ADAPTER_CLASSES}/duckdb_backed_adapter.py", + name="DuckDbBackedAdapter.connect", + expected_forced=True, + ), + ForcedOverrideTestCase( + description="motherduck override could inherit the duckdb-backed method", + language="python", + path=f"{_ADAPTERS}/motherduck/classes/motherduck_adapter.py", + name="MotherDuckAdapter.connect", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="base adapter contract method is forced", + language="python", + path=f"{_ADAPTER_CLASSES}/base_adapter.py", + name="BaseAdapter.render_merge", + expected_forced=True, + ), + ], + ids=lambda case: case.description, +) +def test_given_shipped_contract_exemption_when_checking_repository_then_matches_contract( + test_case: ForcedOverrideTestCase, +) -> None: + repo_root: Path = Path(__file__).resolve().parents[7] + config: DupscoreConfig = load_config(repo_root / "scripts/dupscore/dupscore.toml") + + forced: tuple[bool, ...] = find_forced_overrides( + repo_root=repo_root, + entries=config.contract_exemptions, + units=[method_unit(language=test_case.language, path=test_case.path, name=test_case.name)], + ) + + assert forced == (test_case.expected_forced,) diff --git a/tests/unit/scripts/dupscore/_helpers/inputs/config/_test_types.py b/tests/unit/scripts/dupscore/_helpers/inputs/config/_test_types.py index 02e969847e..d224d8ef76 100644 --- a/tests/unit/scripts/dupscore/_helpers/inputs/config/_test_types.py +++ b/tests/unit/scripts/dupscore/_helpers/inputs/config/_test_types.py @@ -2,7 +2,7 @@ from dataclasses import dataclass -from scripts.dupscore.models import CloneAllowlistEntry +from scripts.dupscore.models import CloneAllowlistEntry, ContractExemptionEntry @dataclass(frozen=True) @@ -41,3 +41,11 @@ class ShippedConfigTestCase: relative_path: str expected_pair_allowlist_size: int expected_clone_allowlist_size: int + expected_contract_exemption_size: int + + +@dataclass(frozen=True) +class ContractExemptionConfigTestCase: + description: str + toml_text: str + expected_entries: tuple[ContractExemptionEntry, ...] diff --git a/tests/unit/scripts/dupscore/_helpers/inputs/config/test_load_config.py b/tests/unit/scripts/dupscore/_helpers/inputs/config/test_load_config.py index af112d19c5..7dbc1e1710 100644 --- a/tests/unit/scripts/dupscore/_helpers/inputs/config/test_load_config.py +++ b/tests/unit/scripts/dupscore/_helpers/inputs/config/test_load_config.py @@ -6,9 +6,10 @@ from scripts.dupscore._helpers.inputs.config import load_config from scripts.dupscore.exceptions import DupscoreConfigError -from scripts.dupscore.models import CloneAllowlistEntry, DupscoreConfig +from scripts.dupscore.models import CloneAllowlistEntry, ContractExemptionEntry, DupscoreConfig from tests.unit.scripts.dupscore._helpers.inputs.config._test_types import ( CloneAllowlistTestCase, + ContractExemptionConfigTestCase, InvalidConfigTestCase, LoadConfigTestCase, MissingConfigTestCase, @@ -113,6 +114,102 @@ def test_given_missing_config_file_when_loading_then_returns_empty_config( toml_text='clone_allowlist = ["src/*"]\n', expected_error_fragment="clone_allowlist entry 1 must be a table", ), + InvalidConfigTestCase( + description="rejects contract exemptions without a reason", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'paths = ["src/demo/*"]\n' + ), + expected_error_fragment="contract_exemption entry 1 needs a non-empty reason", + ), + InvalidConfigTestCase( + description="rejects contract specs without a class name", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py"\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 contract must be", + ), + InvalidConfigTestCase( + description="rejects contract specs that are not python files", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.rs:StrictStore"\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 contract must be", + ), + InvalidConfigTestCase( + description="rejects absolute contract paths", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "/src/demo/contract.py:StrictStore"\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 contract must be", + ), + InvalidConfigTestCase( + description="rejects contract class names that are not identifiers", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:Strict.Store"\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 contract must be", + ), + InvalidConfigTestCase( + description="rejects contract exemptions without a contract", + toml_text='[[contract_exemption]]\npaths = ["src/demo/*"]\nreason = "forced"\n', + expected_error_fragment="contract_exemption entry 1 contract must be", + ), + InvalidConfigTestCase( + description="rejects contract exemptions without paths", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 needs a non-empty paths list", + ), + InvalidConfigTestCase( + description="rejects malformed forbidden owners", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'forbidden_owners = ["BaseStore"]\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 forbidden_owners must be", + ), + InvalidConfigTestCase( + description="rejects a non-list forbidden owners value", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'forbidden_owners = "src/demo/base.py:BaseStore"\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + ), + expected_error_fragment="contract_exemption entry 1 forbidden_owners must be a list", + ), + InvalidConfigTestCase( + description="rejects contract exemptions with unknown keys", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'paths = ["src/demo/*"]\n' + 'reason = "forced"\n' + 'methods = ["write_orders"]\n' + ), + expected_error_fragment="contract_exemption entry 1 has unknown keys: methods", + ), ], ids=lambda case: case.description, ) @@ -173,6 +270,7 @@ def test_given_clone_allowlist_when_loading_then_returns_entries( relative_path="scripts/dupscore/dupscore.toml", expected_pair_allowlist_size=9, expected_clone_allowlist_size=0, + expected_contract_exemption_size=1, ) ], ids=lambda case: case.description, @@ -186,3 +284,63 @@ def test_given_shipped_config_when_loading_then_it_is_valid( assert len(config.allowlisted_pairs) == test_case.expected_pair_allowlist_size assert len(config.clone_allowlist) == test_case.expected_clone_allowlist_size + assert len(config.contract_exemptions) == test_case.expected_contract_exemption_size + + +@pytest.mark.parametrize( + "test_case", + [ + ContractExemptionConfigTestCase( + description="loads contract, forbidden owners, globs, and the stripped reason", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'forbidden_owners = ["src/demo/base.py:BaseStore"]\n' + 'paths = ["src/demo/stores/*"]\n' + 'reason = " Stores must define every contract method. "\n' + ), + expected_entries=( + ContractExemptionEntry( + contract_path="src/demo/contract.py", + contract_class="StrictStore", + paths=("src/demo/stores/*",), + reason="Stores must define every contract method.", + forbidden_owners=(("src/demo/base.py", "BaseStore"),), + ), + ), + ), + ContractExemptionConfigTestCase( + description="forbidden owners are optional", + toml_text=( + "[[contract_exemption]]\n" + 'contract = "src/demo/contract.py:StrictStore"\n' + 'paths = ["src/demo/stores/*"]\n' + 'reason = "forced"\n' + ), + expected_entries=( + ContractExemptionEntry( + contract_path="src/demo/contract.py", + contract_class="StrictStore", + paths=("src/demo/stores/*",), + reason="forced", + ), + ), + ), + ContractExemptionConfigTestCase( + description="missing contract exemptions are empty", + toml_text="", + expected_entries=(), + ), + ], + ids=lambda case: case.description, +) +def test_given_contract_exemption_when_loading_then_returns_entries( + test_case: ContractExemptionConfigTestCase, + tmp_path: Path, +) -> None: + config_path: Path = tmp_path / "dupscore.toml" + config_path.write_text(test_case.toml_text, encoding="utf-8") + + config: DupscoreConfig = load_config(config_path) + + assert config.contract_exemptions == test_case.expected_entries diff --git a/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py b/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py index a917578c06..903a28c9d0 100644 --- a/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py +++ b/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py @@ -2,7 +2,7 @@ from dataclasses import dataclass -from scripts.dupscore.models import CloneAllowlistEntry +from scripts.dupscore.models import CloneAllowlistEntry, ContractExemptionEntry @dataclass(frozen=True) @@ -63,3 +63,19 @@ class SinceUnchangedTestCase: description: str changed_files: dict[str, str] expected_cluster_count: int + + +@dataclass(frozen=True) +class ContractExemptionReportTestCase: + description: str + entries: tuple[ContractExemptionEntry, ...] + expected_clusters: list[list[str]] + expected_contract_exempt_members: int + + +@dataclass(frozen=True) +class ContractExemptionSinceTestCase: + description: str + changed_files: dict[str, str] + expected_clusters: list[list[str]] + expected_member_changes: dict[str, str | None] diff --git a/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py b/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py index 6abd5c492c..6516b2b8df 100644 --- a/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py +++ b/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py @@ -267,3 +267,14 @@ def cluster_languages(report: CloneReport) -> list[tuple[str, ...]]: def member_changes(cluster: CloneCluster) -> dict[str, str | None]: return {member.path: member.change for member in cluster.members} + + +def cluster_member_names(report: CloneReport) -> list[list[str]]: + names: list[list[str]] = [] + for cluster in report.clusters: + names.append(sorted(member.name for member in cluster.members)) + return sorted(names) + + +def member_changes_by_name(report: CloneReport) -> dict[str, str | None]: + return {member.name: member.change for member in reported_members(report)} diff --git a/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py b/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py index d9da41dda9..deb6966bd3 100644 --- a/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py +++ b/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py @@ -13,9 +13,18 @@ ) from scripts.dupscore.main.build_clone_report import build_clone_report from scripts.dupscore.models import CloneAllowlistEntry, CloneReport, DupscoreConfig +from tests.unit.scripts.dupscore._helpers.contracts.forced_overrides.helpers import ( + CONTRACT_FILES, + DELTA_PATH, + RENDER_ORDERS_SQL, + STORE_EXEMPTION, + WRITE_ORDERS, +) from tests.unit.scripts.dupscore.main.build_clone_report._test_types import ( AllowlistTestCase, ClusterShapeTestCase, + ContractExemptionReportTestCase, + ContractExemptionSinceTestCase, ExcludedUnitTestCase, PathFilterTestCase, ReportedPathTestCase, @@ -28,8 +37,10 @@ PYTHON_NEAR_MISS_COPY, clone_options, cluster_languages, + cluster_member_names, link_categories, member_changes, + member_changes_by_name, reported_members, seed_repository, ) @@ -39,6 +50,15 @@ write_project_files, ) +_DELTA_HEADER: str = ( + "from sqlbuild.demo.contract.base_store import BaseStore\n\n\nclass DeltaStore(BaseStore):\n" +) +_VOLUNTARY_CLUSTERS: list[list[str]] = [ + ["AlphaStore._render_orders_sql", "BetaStore._render_orders_sql"], + ["AlphaStore.delete_orders", "GammaStore.delete_orders"], + ["BetaStore.read_orders", "OrdersConnection.read_orders"], +] + @pytest.mark.parametrize( "test_case", @@ -366,3 +386,89 @@ def test_given_since_revision_when_building_report_then_marks_touched_members( assert {path: changes[path] for path in test_case.expected_changes} == ( test_case.expected_changes ) + + +@pytest.mark.parametrize( + "test_case", + [ + ContractExemptionReportTestCase( + description="links between forced overrides are hidden, voluntary copies stay", + entries=(STORE_EXEMPTION,), + expected_clusters=_VOLUNTARY_CLUSTERS, + expected_contract_exempt_members=3, + ), + ContractExemptionReportTestCase( + description="without an exemption every override cluster is reported", + entries=(), + expected_clusters=sorted( + [ + *_VOLUNTARY_CLUSTERS, + ["AlphaStore.write_orders", "BaseStore.write_orders", "BetaStore.write_orders"], + ] + ), + expected_contract_exempt_members=0, + ), + ], + ids=lambda case: case.description, +) +def test_given_contract_exemption_when_building_report_then_hides_forced_overrides( + test_case: ContractExemptionReportTestCase, + tmp_path: Path, +) -> None: + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + + report: CloneReport = build_clone_report( + repo_root=tmp_path, + options=clone_options(), + config=DupscoreConfig(contract_exemptions=test_case.entries), + ) + + assert cluster_member_names(report) == test_case.expected_clusters + assert report.contract_exempt_members == test_case.expected_contract_exempt_members + + +@pytest.mark.parametrize( + "test_case", + [ + ContractExemptionSinceTestCase( + description="new private helper copy is reported beside a hidden forced override", + changed_files={DELTA_PATH: _DELTA_HEADER + WRITE_ORDERS + RENDER_ORDERS_SQL}, + expected_clusters=[ + [ + "AlphaStore._render_orders_sql", + "BetaStore._render_orders_sql", + "DeltaStore._render_orders_sql", + ] + ], + expected_member_changes={ + "AlphaStore._render_orders_sql": None, + "BetaStore._render_orders_sql": None, + "DeltaStore._render_orders_sql": CHANGE_NEW, + }, + ), + ContractExemptionSinceTestCase( + description="new forced override alone reports nothing", + changed_files={DELTA_PATH: _DELTA_HEADER + WRITE_ORDERS}, + expected_clusters=[], + expected_member_changes={}, + ), + ], + ids=lambda case: case.description, +) +def test_given_since_revision_with_contract_exemption_when_building_report_then_hides_forced( + test_case: ContractExemptionSinceTestCase, + tmp_path: Path, +) -> None: + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + initialize_repo(tmp_path) + base_revision: str = commit_all(repo_root=tmp_path, message="seed stores") + write_project_files(repo_root=tmp_path, files=test_case.changed_files) + + report: CloneReport = build_clone_report( + repo_root=tmp_path, + options=clone_options(since=base_revision), + config=DupscoreConfig(contract_exemptions=(STORE_EXEMPTION,)), + ) + + assert cluster_member_names(report) == test_case.expected_clusters + assert member_changes_by_name(report) == test_case.expected_member_changes diff --git a/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py b/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py index 615e556dc1..a52e2f498b 100644 --- a/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py +++ b/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py @@ -74,6 +74,7 @@ def test_given_seeded_tree_when_running_json_cli_then_prints_cluster_document( assert payload["total_clusters"] == test_case.expected_total_clusters assert payload["unit_counts"] == test_case.expected_unit_counts assert payload["since"] is None + assert payload["contract_exempt_members"] == 0 assert [set(cluster) for cluster in clusters] == [set(test_case.expected_cluster_keys)] assert all(set(member) == test_case.expected_member_keys for member in members) assert all(fragment in errors for fragment in test_case.expected_progress_fragments) From 3975ae2c7f7155607727a60444d987d98cbcd9c9 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 24 Sep 2026 13:16:32 +0100 Subject: [PATCH 3/5] feat(dupscore): mark forced overrides and rank clusters by voluntary copies List forced contract overrides after the other cluster members, mark them [forced] in text and forced_override in JSON, and leave them out of the duplicated-token estimate that ranks clusters. --- scripts/dupscore/README.md | 4 ++ .../dupscore/_helpers/clones/clustering.py | 25 +++++++- scripts/dupscore/_helpers/clones/rendering.py | 4 +- scripts/dupscore/main/build_clone_report.py | 4 +- scripts/dupscore/models.py | 1 + .../_helpers/clones/clustering/_test_types.py | 10 ++++ .../clustering/test_cluster_clone_pairs.py | 59 ++++++++++++++++++- .../_helpers/clones/rendering/_test_types.py | 8 +++ .../_helpers/clones/rendering/helpers.py | 43 ++++++++++++++ .../rendering/test_render_clone_report.py | 35 +++++++++++ .../main/build_clone_report/_test_types.py | 6 ++ .../main/build_clone_report/helpers.py | 7 +++ .../test_build_clone_report.py | 31 ++++++++++ .../main/dupscore/test_dupscore_cli.py | 11 +++- 14 files changed, 240 insertions(+), 8 deletions(-) create mode 100644 tests/unit/scripts/dupscore/_helpers/clones/rendering/helpers.py diff --git a/scripts/dupscore/README.md b/scripts/dupscore/README.md index 6686d37bad..5c95b32c65 100644 --- a/scripts/dupscore/README.md +++ b/scripts/dupscore/README.md @@ -71,6 +71,10 @@ otherwise inherit that method from the contract, a `forbidden_owners` class, or between two forced overrides are hidden and clusters left without links disappear; links from a forced override to any other unit (a private helper copy, or an override that could have been inherited instead) are still reported. The summary counts forced overrides hidden this way. +Forced overrides that remain in a reported cluster are marked `[forced]` in text output +(`"forced_override": true` in JSON) and listed after the other members, so a cluster's first +lines show what to fix. They also add nothing to the duplicated-token estimate that ranks +clusters: a forced override already serves as the copy that must stay. An entry whose contract file is absent from the analysed tree is inactive; a missing class in an existing file is a configuration error. diff --git a/scripts/dupscore/_helpers/clones/clustering.py b/scripts/dupscore/_helpers/clones/clustering.py index e25f5ae0a2..a4f000fa78 100644 --- a/scripts/dupscore/_helpers/clones/clustering.py +++ b/scripts/dupscore/_helpers/clones/clustering.py @@ -19,11 +19,12 @@ def cluster_clone_pairs( units: list[CloneUnit], pairs: list[ClonePair], change_of: Callable[[CloneUnit], str | None], + forced: frozenset[int], ) -> list[CloneCluster]: """Group pairs into connected components ordered by estimated duplicated tokens.""" clusters: list[CloneCluster] = [ - _build_cluster(units=units, pairs=component, change_of=change_of) + _build_cluster(units=units, pairs=component, change_of=change_of, forced=forced) for component in _connected_pair_groups(pairs) ] return sorted(clusters, key=_cluster_sort_key) @@ -63,10 +64,16 @@ def _build_cluster( units: list[CloneUnit], pairs: list[ClonePair], change_of: Callable[[CloneUnit], str | None], + forced: frozenset[int], ) -> CloneCluster: unit_indexes: list[int] = sorted( _unit_indexes(pairs), - key=lambda index: (units[index].path, units[index].start_line, units[index].name), + key=lambda index: ( + index in forced, + units[index].path, + units[index].start_line, + units[index].name, + ), ) position: dict[int, int] = {unit_index: slot for slot, unit_index in enumerate(unit_indexes)} best_similarity: dict[int, float] = {} @@ -76,6 +83,7 @@ def _build_cluster( weights: list[float] = [ len(units[unit_index].normalized) * best_similarity[unit_index] for unit_index in unit_indexes + if unit_index not in forced ] links: list[ClonePairLink] = sorted( ( @@ -102,6 +110,7 @@ def _build_cluster( end_line=units[unit_index].end_line, tokens=len(units[unit_index].normalized), change=change_of(units[unit_index]), + forced_override=unit_index in forced, ) for unit_index in unit_indexes ) @@ -109,12 +118,22 @@ def _build_cluster( category=category, similarity_min=min(similarities), similarity_max=max(similarities), - duplicated_tokens=round(sum(weights) - max(weights)), + duplicated_tokens=_duplicated_tokens( + weights=weights, has_forced=len(weights) < len(members) + ), members=members, links=tuple(links), ) +def _duplicated_tokens(*, weights: list[float], has_forced: bool) -> int: + """Estimate removable tokens: keep one copy, or a required forced override when present.""" + + if has_forced: + return round(sum(weights)) + return round(sum(weights) - max(weights)) + + def _cluster_sort_key(cluster: CloneCluster) -> tuple[int, float, str, int]: first: CloneMember = cluster.members[0] return (-cluster.duplicated_tokens, -cluster.similarity_max, first.path, first.start_line) diff --git a/scripts/dupscore/_helpers/clones/rendering.py b/scripts/dupscore/_helpers/clones/rendering.py index d0b2429bca..256ff5bae7 100644 --- a/scripts/dupscore/_helpers/clones/rendering.py +++ b/scripts/dupscore/_helpers/clones/rendering.py @@ -9,6 +9,7 @@ from scripts.dupscore.models import CloneCluster, CloneMember, CloneReport _INDENT: str = " " +_FORCED_MARKER: str = " [forced]" def render_clone_text(*, report: CloneReport, top: int) -> str: @@ -66,7 +67,8 @@ def _cluster_heading(cluster: CloneCluster) -> str: def _member_line(member: CloneMember) -> str: change: str = f" [{member.change}]" if member.change is not None else "" + forced: str = _FORCED_MARKER if member.forced_override else "" return ( f"{member.path}:{member.start_line}-{member.end_line} {member.name} " - f"({member.tokens} tokens){change}" + f"({member.tokens} tokens){forced}{change}" ) diff --git a/scripts/dupscore/main/build_clone_report.py b/scripts/dupscore/main/build_clone_report.py index cded4e0a29..6a04ffa1ee 100644 --- a/scripts/dupscore/main/build_clone_report.py +++ b/scripts/dupscore/main/build_clone_report.py @@ -83,7 +83,9 @@ def change_of(unit: CloneUnit) -> str | None: clusters: list[CloneCluster] = [ cluster - for cluster in cluster_clone_pairs(units=units, pairs=pairs, change_of=change_of) + for cluster in cluster_clone_pairs( + units=units, pairs=pairs, change_of=change_of, forced=frozenset(forced) + ) if _cluster_selected(cluster=cluster, options=options) ] unit_counts: Counter[str] = Counter(unit.language for unit in units) diff --git a/scripts/dupscore/models.py b/scripts/dupscore/models.py index a22df54912..4aabec8287 100644 --- a/scripts/dupscore/models.py +++ b/scripts/dupscore/models.py @@ -192,6 +192,7 @@ class CloneMember: end_line: int tokens: int change: str | None + forced_override: bool @dataclass(frozen=True, slots=True) diff --git a/tests/unit/scripts/dupscore/_helpers/clones/clustering/_test_types.py b/tests/unit/scripts/dupscore/_helpers/clones/clustering/_test_types.py index c71f739cab..d891d9263a 100644 --- a/tests/unit/scripts/dupscore/_helpers/clones/clustering/_test_types.py +++ b/tests/unit/scripts/dupscore/_helpers/clones/clustering/_test_types.py @@ -17,3 +17,13 @@ class ClusterEstimateTestCase: expected_links: tuple[tuple[int, int], ...] expected_similarity_range: tuple[float, float] expected_duplicated_tokens: int + + +@dataclass(frozen=True) +class ForcedClusterTestCase: + description: str + pairs: tuple[tuple[int, int, float, str], ...] + forced: frozenset[int] + expected_members: tuple[tuple[str, bool], ...] + expected_links: tuple[tuple[int, int], ...] + expected_duplicated_tokens: int diff --git a/tests/unit/scripts/dupscore/_helpers/clones/clustering/test_cluster_clone_pairs.py b/tests/unit/scripts/dupscore/_helpers/clones/clustering/test_cluster_clone_pairs.py index 09553f46ca..a7f5170833 100644 --- a/tests/unit/scripts/dupscore/_helpers/clones/clustering/test_cluster_clone_pairs.py +++ b/tests/unit/scripts/dupscore/_helpers/clones/clustering/test_cluster_clone_pairs.py @@ -8,6 +8,7 @@ from tests.unit.scripts.dupscore._helpers.clones.clustering._test_types import ( ClusterEstimateTestCase, ClusterPairsTestCase, + ForcedClusterTestCase, ) from tests.unit.scripts.dupscore._helpers.clones.clustering.helpers import ( build_pairs, @@ -44,7 +45,10 @@ def test_given_clone_pairs_when_clustering_then_groups_transitively( test_case: ClusterPairsTestCase, ) -> None: clusters: list[CloneCluster] = cluster_clone_pairs( - units=build_units(), pairs=build_pairs(test_case.pairs), change_of=no_change + units=build_units(), + pairs=build_pairs(test_case.pairs), + change_of=no_change, + forced=frozenset(), ) assert summarize_clusters(clusters) == test_case.expected_clusters @@ -67,9 +71,60 @@ def test_given_cluster_when_clustering_then_reports_links_and_estimates( test_case: ClusterEstimateTestCase, ) -> None: cluster: CloneCluster = cluster_clone_pairs( - units=build_units(), pairs=build_pairs(test_case.pairs), change_of=no_change + units=build_units(), + pairs=build_pairs(test_case.pairs), + change_of=no_change, + forced=frozenset(), )[0] assert tuple((link.left, link.right) for link in cluster.links) == test_case.expected_links assert (cluster.similarity_min, cluster.similarity_max) == test_case.expected_similarity_range assert cluster.duplicated_tokens == test_case.expected_duplicated_tokens + + +@pytest.mark.parametrize( + "test_case", + [ + ForcedClusterTestCase( + description="forced members sort last and add no duplicated tokens", + pairs=((0, 3, 1.0, CATEGORY_EXACT), (1, 3, 1.0, CATEGORY_EXACT)), + forced=frozenset({0, 1}), + expected_members=( + ("function_3", False), + ("function_0", True), + ("function_1", True), + ), + expected_links=((0, 1), (0, 2)), + expected_duplicated_tokens=40, + ), + ForcedClusterTestCase( + description="without forced members one copy is kept out of the estimate", + pairs=((0, 3, 1.0, CATEGORY_EXACT), (1, 3, 1.0, CATEGORY_EXACT)), + forced=frozenset(), + expected_members=( + ("function_0", False), + ("function_1", False), + ("function_3", False), + ), + expected_links=((0, 2), (1, 2)), + expected_duplicated_tokens=140, + ), + ], + ids=lambda case: case.description, +) +def test_given_forced_members_when_clustering_then_orders_marks_and_estimates( + test_case: ForcedClusterTestCase, +) -> None: + cluster: CloneCluster = cluster_clone_pairs( + units=build_units(), + pairs=build_pairs(test_case.pairs), + change_of=no_change, + forced=test_case.forced, + )[0] + + assert ( + tuple((member.name, member.forced_override) for member in cluster.members) + == test_case.expected_members + ) + assert tuple((link.left, link.right) for link in cluster.links) == test_case.expected_links + assert cluster.duplicated_tokens == test_case.expected_duplicated_tokens diff --git a/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py b/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py index ca1ebfd5fb..0c2e6cfd3c 100644 --- a/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py +++ b/tests/unit/scripts/dupscore/_helpers/clones/rendering/_test_types.py @@ -9,3 +9,11 @@ class HiddenCountsTestCase: allowlisted_pairs: int contract_exempt_members: int expected_summary_fragment: str + + +@dataclass(frozen=True) +class ForcedMarkerTestCase: + description: str + forced_flags: tuple[bool, ...] + expected_member_lines: tuple[str, ...] + expected_json_flags: list[bool] diff --git a/tests/unit/scripts/dupscore/_helpers/clones/rendering/helpers.py b/tests/unit/scripts/dupscore/_helpers/clones/rendering/helpers.py new file mode 100644 index 0000000000..fd923f5696 --- /dev/null +++ b/tests/unit/scripts/dupscore/_helpers/clones/rendering/helpers.py @@ -0,0 +1,43 @@ +from __future__ import annotations + +from scripts.dupscore.constants import CATEGORY_EXACT +from scripts.dupscore.models import CloneCluster, CloneMember, ClonePairLink, CloneReport + +_MEMBER_NAMES: tuple[str, ...] = ("OrdersStore.write_orders", "BaseStore.write_orders") +_FIRST_MEMBER_LINE: int = 2 + + +def report_with_members(forced_flags: tuple[bool, ...]) -> CloneReport: + members: list[CloneMember] = [] + for position, (name, forced) in enumerate(zip(_MEMBER_NAMES, forced_flags, strict=True)): + members.append( + CloneMember( + language="python", + path="src/sqlbuild/demo/orders.py", + name=name, + start_line=1 + 10 * position, + end_line=9 + 10 * position, + tokens=80, + change=None, + forced_override=forced, + ) + ) + cluster: CloneCluster = CloneCluster( + category=CATEGORY_EXACT, + similarity_min=1.0, + similarity_max=1.0, + duplicated_tokens=80, + members=tuple(members), + links=(ClonePairLink(left=0, right=1, similarity=1.0, category=CATEGORY_EXACT),), + ) + return CloneReport( + since=None, + unit_counts={"python": 2}, + allowlisted_pairs=0, + contract_exempt_members=0, + clusters=(cluster,), + ) + + +def member_lines(text: str) -> tuple[str, ...]: + return tuple(line.strip() for line in text.splitlines()[_FIRST_MEMBER_LINE:]) diff --git a/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py b/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py index 9bcba0d47a..b21f78d2ee 100644 --- a/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py +++ b/tests/unit/scripts/dupscore/_helpers/clones/rendering/test_render_clone_report.py @@ -1,14 +1,20 @@ from __future__ import annotations import json +from typing import Any import pytest from scripts.dupscore._helpers.clones.rendering import render_clone_json, render_clone_text from scripts.dupscore.models import CloneReport from tests.unit.scripts.dupscore._helpers.clones.rendering._test_types import ( + ForcedMarkerTestCase, HiddenCountsTestCase, ) +from tests.unit.scripts.dupscore._helpers.clones.rendering.helpers import ( + member_lines, + report_with_members, +) @pytest.mark.parametrize( @@ -42,3 +48,32 @@ def test_given_hidden_counts_when_rendering_then_text_and_json_report_them( assert test_case.expected_summary_fragment in text.splitlines()[0] assert payload["allowlisted_pairs"] == test_case.allowlisted_pairs assert payload["contract_exempt_members"] == test_case.contract_exempt_members + + +@pytest.mark.parametrize( + "test_case", + [ + ForcedMarkerTestCase( + description="forced members are marked in text and flagged in json", + forced_flags=(False, True), + expected_member_lines=( + "src/sqlbuild/demo/orders.py:1-9 OrdersStore.write_orders (80 tokens)", + "src/sqlbuild/demo/orders.py:11-19 BaseStore.write_orders (80 tokens) [forced]", + ), + expected_json_flags=[False, True], + ) + ], + ids=lambda case: case.description, +) +def test_given_forced_members_when_rendering_then_marks_them( + test_case: ForcedMarkerTestCase, +) -> None: + report: CloneReport = report_with_members(test_case.forced_flags) + + text: str = render_clone_text(report=report, top=5) + payload: dict[str, Any] = json.loads(render_clone_json(report=report, top=5)) + + assert member_lines(text) == test_case.expected_member_lines + assert [ + member["forced_override"] for member in payload["clusters"][0]["members"] + ] == test_case.expected_json_flags diff --git a/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py b/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py index 903a28c9d0..3374deaf85 100644 --- a/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py +++ b/tests/unit/scripts/dupscore/main/build_clone_report/_test_types.py @@ -79,3 +79,9 @@ class ContractExemptionSinceTestCase: changed_files: dict[str, str] expected_clusters: list[list[str]] expected_member_changes: dict[str, str | None] + + +@dataclass(frozen=True) +class ForcedMemberOrderTestCase: + description: str + expected_member_flags: list[list[tuple[str, bool]]] diff --git a/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py b/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py index 6516b2b8df..963e58eddc 100644 --- a/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py +++ b/tests/unit/scripts/dupscore/main/build_clone_report/helpers.py @@ -278,3 +278,10 @@ def cluster_member_names(report: CloneReport) -> list[list[str]]: def member_changes_by_name(report: CloneReport) -> dict[str, str | None]: return {member.name: member.change for member in reported_members(report)} + + +def cluster_member_flags(report: CloneReport) -> list[list[tuple[str, bool]]]: + flags: list[list[tuple[str, bool]]] = [] + for cluster in report.clusters: + flags.append([(member.name, member.forced_override) for member in cluster.members]) + return sorted(flags) diff --git a/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py b/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py index deb6966bd3..61536a7a31 100644 --- a/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py +++ b/tests/unit/scripts/dupscore/main/build_clone_report/test_build_clone_report.py @@ -26,6 +26,7 @@ ContractExemptionReportTestCase, ContractExemptionSinceTestCase, ExcludedUnitTestCase, + ForcedMemberOrderTestCase, PathFilterTestCase, ReportedPathTestCase, SeededCloneTestCase, @@ -37,6 +38,7 @@ PYTHON_NEAR_MISS_COPY, clone_options, cluster_languages, + cluster_member_flags, cluster_member_names, link_categories, member_changes, @@ -472,3 +474,32 @@ def test_given_since_revision_with_contract_exemption_when_building_report_then_ assert cluster_member_names(report) == test_case.expected_clusters assert member_changes_by_name(report) == test_case.expected_member_changes + + +@pytest.mark.parametrize( + "test_case", + [ + ForcedMemberOrderTestCase( + description="voluntary copies are listed before flagged forced overrides", + expected_member_flags=[ + [("AlphaStore._render_orders_sql", False), ("BetaStore._render_orders_sql", False)], + [("GammaStore.delete_orders", False), ("AlphaStore.delete_orders", True)], + [("OrdersConnection.read_orders", False), ("BetaStore.read_orders", True)], + ], + ) + ], + ids=lambda case: case.description, +) +def test_given_contract_exemption_when_building_report_then_lists_forced_members_last( + test_case: ForcedMemberOrderTestCase, + tmp_path: Path, +) -> None: + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + + report: CloneReport = build_clone_report( + repo_root=tmp_path, + options=clone_options(), + config=DupscoreConfig(contract_exemptions=(STORE_EXEMPTION,)), + ) + + assert cluster_member_flags(report) == test_case.expected_member_flags diff --git a/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py b/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py index a52e2f498b..f02556a67e 100644 --- a/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py +++ b/tests/unit/scripts/dupscore/main/dupscore/test_dupscore_cli.py @@ -45,7 +45,16 @@ } ), expected_member_keys=frozenset( - {"change", "end_line", "language", "name", "path", "start_line", "tokens"} + { + "change", + "end_line", + "forced_override", + "language", + "name", + "path", + "start_line", + "tokens", + } ), expected_progress_fragments=( "detecting function clones", From aad78e5646605dee501fef321a70b90d5ca223e5 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 24 Sep 2026 13:21:44 +0100 Subject: [PATCH 4/5] fix(dupscore): keep overrides visible when a repository ancestor is unresolved A base that names a repository module but cannot be resolved to a class (a package re-export or a module-qualified base) might supply the method before the forbidden owner, so the override is no longer treated as forced. --- .../_helpers/contracts/forced_overrides.py | 37 ++++++++++++++++++- .../contracts/forced_overrides/helpers.py | 23 ++++++++++++ .../test_find_forced_overrides.py | 27 +++++++++++++- 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/scripts/dupscore/_helpers/contracts/forced_overrides.py b/scripts/dupscore/_helpers/contracts/forced_overrides.py index 8186379757..a179556e39 100644 --- a/scripts/dupscore/_helpers/contracts/forced_overrides.py +++ b/scripts/dupscore/_helpers/contracts/forced_overrides.py @@ -26,6 +26,7 @@ class _ClassInfo: bases: tuple[_ClassKey, ...] concrete: frozenset[str] abstract: frozenset[str] + complete: bool @dataclass(frozen=True, slots=True) @@ -80,6 +81,8 @@ def _forced_in_scope(self, *, scope: _ContractScope, key: _ClassKey, method_name order: tuple[_ClassKey, ...] = self._resolver.method_resolution_order(key) if scope.contract not in order: return False + if not all(self._resolver.info(ancestor).complete for ancestor in order): + return False for ancestor in order[1:]: if method_name in self._resolver.info(ancestor).concrete: return ancestor in scope.forbidden_owners @@ -159,21 +162,53 @@ def _build_info(self, key: _ClassKey) -> _ClassInfo: tree: ast.Module | None = self._parse(key[0]) node: ast.ClassDef | None = None if tree is None else _find_class(tree=tree, name=key[1]) if tree is None or node is None: - return _ClassInfo(bases=(), concrete=frozenset(), abstract=frozenset()) + return _ClassInfo(bases=(), concrete=frozenset(), abstract=frozenset(), complete=False) functions: list[_FunctionNode] = [ item for item in node.body if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) ] bases: list[_ClassKey] = [] + complete: bool = True for base in node.bases: resolved: _ClassKey | None = self._resolve_base(tree=tree, importer=key[0], base=base) if resolved is not None: bases.append(resolved) + elif self._references_local_module(tree=tree, importer=key[0], base=base): + complete = False return _ClassInfo( bases=tuple(bases), concrete=frozenset(item.name for item in functions if not _is_abstract(item)), abstract=frozenset(item.name for item in functions if _is_abstract(item)), + complete=complete, ) + def _references_local_module(self, *, tree: ast.Module, importer: str, base: ast.expr) -> bool: + """Report bases that name this repository but could not be resolved to a class.""" + + root: ast.expr = base.value if isinstance(base, ast.Subscript) else base + while isinstance(root, ast.Attribute): + root = root.value + if not isinstance(root, ast.Name): + return True + for statement in tree.body: + if isinstance(statement, ast.ImportFrom): + for alias in statement.names: + if (alias.asname or alias.name) == root.id: + return ( + self._module_path( + importer=importer, module=statement.module, level=statement.level + ) + is not None + or statement.level > 0 + ) + elif isinstance(statement, ast.Import): + for alias in statement.names: + if (alias.asname or alias.name.split(_MODULE_SEPARATOR)[0]) == root.id: + return ( + self._module_path(importer=importer, module=alias.name, level=0) + is not None + ) + return False + def _resolve_base(self, *, tree: ast.Module, importer: str, base: ast.expr) -> _ClassKey | None: if not isinstance(base, ast.Name): return None diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py index c2395492e8..c661f8d538 100644 --- a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py @@ -11,6 +11,9 @@ CONNECTION_PATH: str = "src/sqlbuild/demo/stores/beta/classes/orders_connection.py" OUTSIDE_PATH: str = "src/sqlbuild/demo/elsewhere/outside_store.py" DELTA_PATH: str = "src/sqlbuild/demo/stores/delta/classes/delta_store.py" +REEXPORT_PATH: str = "src/sqlbuild/demo/stores/epsilon/classes/epsilon_store.py" +QUALIFIED_PATH: str = "src/sqlbuild/demo/stores/zeta/classes/zeta_store.py" +STDLIB_BASE_PATH: str = "src/sqlbuild/demo/stores/eta/classes/eta_store.py" _MIXIN_SOURCE: str = """\ from abc import abstractmethod @@ -143,6 +146,26 @@ def _render_orders_sql(self, table, columns, predicates): ), } +ANCESTRY_FILES: dict[str, str] = { + "src/sqlbuild/demo/shared/__init__.py": "from .writer import WriterMixin\n", + "src/sqlbuild/demo/shared/writer.py": "class WriterMixin:\n" + WRITE_ORDERS, + REEXPORT_PATH: ( + "from sqlbuild.demo.shared import WriterMixin\n" + + _BASE_IMPORT + + "class EpsilonStore(WriterMixin, BaseStore):\n" + + WRITE_ORDERS + ), + QUALIFIED_PATH: ( + "import sqlbuild.demo.shared.writer\n" + + _BASE_IMPORT + + "class ZetaStore(sqlbuild.demo.shared.writer.WriterMixin, BaseStore):\n" + + WRITE_ORDERS + ), + STDLIB_BASE_PATH: ( + "from abc import ABC\n" + _BASE_IMPORT + "class EtaStore(BaseStore, ABC):\n" + WRITE_ORDERS + ), +} + DELTA_FILES: dict[str, str] = { DELTA_PATH: ( _BASE_IMPORT + "class DeltaStore(BaseStore):\n" + WRITE_ORDERS + RENDER_ORDERS_SQL diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py index ae5064972c..7386a57c73 100644 --- a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py @@ -16,6 +16,7 @@ ) from tests.unit.scripts.dupscore._helpers.contracts.forced_overrides.helpers import ( ALPHA_PATH, + ANCESTRY_FILES, BASE_PATH, BETA_PATH, CONNECTION_PATH, @@ -23,6 +24,9 @@ CONTRACT_PATH, GAMMA_PATH, OUTSIDE_PATH, + QUALIFIED_PATH, + REEXPORT_PATH, + STDLIB_BASE_PATH, STORE_EXEMPTION, method_unit, ) @@ -112,6 +116,27 @@ name="write_orders", expected_forced=False, ), + ForcedOverrideTestCase( + description="re-exported mixin that could supply the method blocks the exemption", + language="python", + path=REEXPORT_PATH, + name="EpsilonStore.write_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="module-qualified repository base blocks the exemption", + language="python", + path=QUALIFIED_PATH, + name="ZetaStore.write_orders", + expected_forced=False, + ), + ForcedOverrideTestCase( + description="standard-library bases do not block the exemption", + language="python", + path=STDLIB_BASE_PATH, + name="EtaStore.write_orders", + expected_forced=True, + ), ForcedOverrideTestCase( description="rust units are never exempt", language="rust", @@ -126,7 +151,7 @@ def test_given_contract_exemption_when_checking_unit_then_reports_forced_overrid test_case: ForcedOverrideTestCase, tmp_path: Path, ) -> None: - write_project_files(repo_root=tmp_path, files=CONTRACT_FILES) + write_project_files(repo_root=tmp_path, files=CONTRACT_FILES | ANCESTRY_FILES) forced: tuple[bool, ...] = find_forced_overrides( repo_root=tmp_path, From 6616efb848d4bbb3d2d3baa1f547c7ed087239f0 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 24 Sep 2026 13:23:25 +0100 Subject: [PATCH 5/5] fix(dupscore): resolve subscripted bases when checking forced overrides --- .../dupscore/_helpers/contracts/forced_overrides.py | 4 ++++ .../_helpers/contracts/forced_overrides/helpers.py | 10 ++++++++++ .../forced_overrides/test_find_forced_overrides.py | 8 ++++++++ 3 files changed, 22 insertions(+) diff --git a/scripts/dupscore/_helpers/contracts/forced_overrides.py b/scripts/dupscore/_helpers/contracts/forced_overrides.py index a179556e39..8e470c037f 100644 --- a/scripts/dupscore/_helpers/contracts/forced_overrides.py +++ b/scripts/dupscore/_helpers/contracts/forced_overrides.py @@ -189,6 +189,8 @@ def _references_local_module(self, *, tree: ast.Module, importer: str, base: ast root = root.value if not isinstance(root, ast.Name): return True + if _find_class(tree=tree, name=root.id) is not None: + return True for statement in tree.body: if isinstance(statement, ast.ImportFrom): for alias in statement.names: @@ -210,6 +212,8 @@ def _references_local_module(self, *, tree: ast.Module, importer: str, base: ast return False def _resolve_base(self, *, tree: ast.Module, importer: str, base: ast.expr) -> _ClassKey | None: + if isinstance(base, ast.Subscript): + base = base.value if not isinstance(base, ast.Name): return None if _find_class(tree=tree, name=base.id) is not None: diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py index c661f8d538..906ae09173 100644 --- a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/helpers.py @@ -14,6 +14,7 @@ REEXPORT_PATH: str = "src/sqlbuild/demo/stores/epsilon/classes/epsilon_store.py" QUALIFIED_PATH: str = "src/sqlbuild/demo/stores/zeta/classes/zeta_store.py" STDLIB_BASE_PATH: str = "src/sqlbuild/demo/stores/eta/classes/eta_store.py" +GENERIC_BASE_PATH: str = "src/sqlbuild/demo/stores/theta/classes/theta_store.py" _MIXIN_SOURCE: str = """\ from abc import abstractmethod @@ -161,6 +162,15 @@ def _render_orders_sql(self, table, columns, predicates): + "class ZetaStore(sqlbuild.demo.shared.writer.WriterMixin, BaseStore):\n" + WRITE_ORDERS ), + GENERIC_BASE_PATH: ( + "from typing import Generic, TypeVar\n" + + _BASE_IMPORT + + 'RowT = TypeVar("RowT")\n\n\n' + + "class WriterMixin(Generic[RowT]):\n" + + WRITE_ORDERS + + "\n\nclass ThetaStore(WriterMixin[int], BaseStore):\n" + + WRITE_ORDERS + ), STDLIB_BASE_PATH: ( "from abc import ABC\n" + _BASE_IMPORT + "class EtaStore(BaseStore, ABC):\n" + WRITE_ORDERS ), diff --git a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py index 7386a57c73..9d8d531eb9 100644 --- a/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py +++ b/tests/unit/scripts/dupscore/_helpers/contracts/forced_overrides/test_find_forced_overrides.py @@ -23,6 +23,7 @@ CONTRACT_FILES, CONTRACT_PATH, GAMMA_PATH, + GENERIC_BASE_PATH, OUTSIDE_PATH, QUALIFIED_PATH, REEXPORT_PATH, @@ -130,6 +131,13 @@ name="ZetaStore.write_orders", expected_forced=False, ), + ForcedOverrideTestCase( + description="same-file generic mixin that supplies the method keeps it voluntary", + language="python", + path=GENERIC_BASE_PATH, + name="ThetaStore.write_orders", + expected_forced=False, + ), ForcedOverrideTestCase( description="standard-library bases do not block the exemption", language="python",