From 5e8c374a3053ab43f511802484779b7a0f17d5ed Mon Sep 17 00:00:00 2001 From: kikashy <35789537+kikashy@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:04:11 -0400 Subject: [PATCH 1/3] Add an opt-in duplication check with per-repository thresholds and an ignore block (#35) Duplicated logic is where two copies drift, and nothing flagged a new copy. lib/duplication.py is a standard-library copy-paste detector a repository vendors as scripts/dev/duplication.py and runs as the check-list step `duplication`. It is off, and its summary line says so, until the repository's jpdev.json declares a `duplication` member: the languages to read (go, javascript, python, shell, typescript), optional path and exclude globs, a token threshold and a line threshold. Each declared language is read as tokens with its own comment and string syntax, so layout and comments do not hide a copy. Runs of min_tokens tokens are hashed with a rolling hash and each run is compared with the previous place it appeared; a match is extended to the whole shared block and reported when each place spans at least min_lines lines. Two places in one file may not overlap. The report names each pair with both file:line ranges and the token count and exits 1. The code between comments holding duplication:ignore-start and duplication:ignore-end, in any comment syntax of the language, is left out; an unclosed, unopened or nested marker is reported. An invalid declaration is refused with exit 2 and a message that names the member. templates/dev.json carries the step, templates/jpdev.json and templates/jpdev.schema.json the member (generators is now required only when duplication is absent), and the generated-document checker accepts the member without reading it. README.md, docs/adopting.md, docs/policy.md and docs/generated-docs.md describe it. tests/duplication_test.py holds handwritten fixture repositories run through the command and through the template manifest's step, and checks that the schema and the command agree on every declaration vector; tests/run.sh runs it. Signed-off-by: kikashy <35789537+kikashy@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- README.md | 46 +++ docs/adopting.md | 10 + docs/generated-docs.md | 5 +- docs/policy.md | 3 + lib/duplication.py | 301 +++++++++++++++++++ lib/generated_docs.py | 2 +- scripts/dev/generated_docs.py | 2 +- templates/dev.json | 1 + templates/jpdev.json | 7 + templates/jpdev.schema.json | 74 ++++- tests/duplication_test.py | 530 ++++++++++++++++++++++++++++++++++ tests/generated_docs_test.py | 11 + tests/run.sh | 5 + 13 files changed, 992 insertions(+), 5 deletions(-) create mode 100644 lib/duplication.py create mode 100644 tests/duplication_test.py diff --git a/README.md b/README.md index 1abc6d3..16cfacf 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,52 @@ the same name overrides it. `config/env.example` documents each setting with its the same check first and does not start the run on a refusal. - `jpdev env`: the effective settings and where each came from. +## The duplication check + +Duplicated logic is where two copies drift. `lib/duplication.py` (python3 standard library) is an +opt-in check-list step: a repository vendors it as `scripts/dev/duplication.py` and lists the step +`{"name": "duplication", "run": "python3 scripts/dev/duplication.py", "toolchain": "shell", +"summary": "^duplication:", "timeout": 300}`, which `templates/dev.json` carries. The step is off, +and its summary line says so, until the repository's `jpdev.json` declares a `duplication` member +with its thresholds (`templates/jpdev.schema.json` holds the shape): + +```json +"duplication": { + "languages": ["go", "python", "typescript"], + "exclude": ["**/testdata/**"], + "min_tokens": 70, + "min_lines": 6 +} +``` + +- `languages` (required): any of `go`, `javascript`, `python`, `shell` and `typescript`; each names + its file extensions and comment syntax. The files read are the tracked, and the untracked but not + ignored, files of those extensions that match a `paths` glob (default `**`) and no `exclude` glob. + Globs are repository-relative: `*` and `?` stay within one segment and a `**` segment spans any + number of segments. A symbolic link is not followed. +- `min_tokens` (10 to 10000) and `min_lines` (1 to 10000), both required: a pair is reported when + two places share at least `min_tokens` tokens and each place spans at least `min_lines` lines. + Whitespace, layout and comments are not tokens; identifiers, keywords, numbers, string literals + and each punctuation character are. A copy laid out or commented differently is still a copy; a + copy whose names were changed is not. +- An intentional copy is marked with an ignore block: the code between a comment that holds + `duplication:ignore-start` and a comment that holds `duplication:ignore-end` is not read, in any + comment syntax of the language (`//`, `/* */`, `#`). The text in a string is not a marker. A start + that is never closed, an end with no start, and a start inside an open block are reported. + +The report names each pair with both places and the shared token count, one line each. Three +copies of a block make two pairs: each copy is named against the one before it. + +``` +duplication: 1 duplicate pair (at least 70 tokens over at least 6 lines, 214 files of go, python, typescript) + internal/audit/receipt.go:18-50 and internal/export/receipt.go:40-72: 131 tokens +``` + +The exit status is 0 when nothing is found or the check is off, 1 when a pair or an ignore marker +problem is found, and 2 when the declaration is refused. A refusal names the member, for example +`duplication: refused: jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required +(got 5)`. + ## The caps | Toolchain | Cap | diff --git a/docs/adopting.md b/docs/adopting.md index e127f31..6312a24 100644 --- a/docs/adopting.md +++ b/docs/adopting.md @@ -167,6 +167,16 @@ JPDEV_DESK=path/to/desk-checkout jpdev check --all --manifest path/to/devkit/exa Copy the example to the repository as `dev.json` when it adopts the kit. +## Duplication check + +The `duplication` step is opt-in. Vendor `lib/duplication.py` as `scripts/dev/duplication.py`, keep +the `duplication` step of `templates/dev.json` in the check list, and declare the repository's +languages and its token and line thresholds in the `duplication` member of `jpdev.json` +(`generators` may be left out when `duplication` is present). Until the member is present the step +passes and its summary line says `duplication: off`. Choose thresholds the current tree passes, or +remove or mark the copies the first run names. [The duplication check](../README.md#the-duplication-check) +describes the member, the ignore block and the report. + ## Generated reference documents Keep `dev.json` as the checklist manifest. Repositories with checked-in generated documents add diff --git a/docs/generated-docs.md b/docs/generated-docs.md index 3a54885..7e4f484 100644 --- a/docs/generated-docs.md +++ b/docs/generated-docs.md @@ -27,7 +27,10 @@ segment. Declare every output file individually; directories/globs are unsupport `timeout` is an integer1–3600 seconds (default120). The optional `dependencies` list contains explicit existing `node_modules` directories only, such as `node_modules` or `packages/example/node_modules`. Omit it when the generator needs no installed npm dependencies. -The example command/output must be adjusted to the repository's actual generator. +The example command/output must be adjusted to the repository's actual generator. The optional +`duplication` member of the same file configures +[the duplication check](../README.md#the-duplication-check); this checker accepts that member and +does not read it. Add the ordinary `dev.json` check (and keep the generator's useful existing coverage): diff --git a/docs/policy.md b/docs/policy.md index 7365675..ea50aa3 100644 --- a/docs/policy.md +++ b/docs/policy.md @@ -10,6 +10,9 @@ are the ones a brief does not repeat. - **CI owns everything deterministic.** Formatting, builds, unit tests, schema checks, certification suites, whitespace and path checks run in CI on every pull request. A session does not re-run them to reassure itself and does not watch CI run. +- **Duplication is where two copies drift:** a repository opts in to the `duplication` check with + its own token and line thresholds in `jpdev.json`, and marks an intentional copy with an explicit + ignore block. - **Define and implement a dependency-ordered coherent batch first.** Name the related issues, dependency order and acceptance criteria; write acceptance and regression fixtures by hand alongside code. During development, run useful focused checks of changed paths, affected diff --git a/lib/duplication.py b/lib/duplication.py new file mode 100644 index 0000000..1e43ef8 --- /dev/null +++ b/lib/duplication.py @@ -0,0 +1,301 @@ +#!/usr/bin/env python3 +"""The duplication check: name each block of code that appears in two places. + +Off unless the repository's jpdev.json declares a `duplication` member: the languages to read, a +token threshold, a line threshold and, optionally, path globs. A pair is reported when two places +share at least min_tokens tokens and each place spans at least min_lines lines. Layout and +comments are not tokens; identifiers, keywords, numbers, string literals and each punctuation +character are. The code between a comment holding `duplication:ignore-start` and a comment holding +`duplication:ignore-end` is left out, so an intentional copy can be marked. Standard library only. + +Exit status: 0 when no pair is found or the check is off, 1 when a pair or an ignore marker problem +is found (the report names each), 2 when the configuration is refused. +""" +import argparse +import itertools +import json +import os +from pathlib import Path +import re +import subprocess +import sys + +MIN_TOKENS = (10, 10000) +MIN_LINES = (1, 10000) +MEMBERS = ('exclude', 'languages', 'min_lines', 'min_tokens', 'paths') + +_C_COMMENTS = r'//[^\n]*|/\*[\s\S]*?(?:\*/|\Z)' +# A quoted string that may not span a line ends at the line's end when it is not closed. +_LINE_STRINGS = r'"(?:[^"\\\n]|\\[\s\S])*"?|' r"'(?:[^'\\\n]|\\[\s\S])*'?" +_SCRIPT_STRINGS = _LINE_STRINGS + r'|`(?:[^`\\]|\\[\s\S])*`?' +# name: (file extensions, comment syntax, string syntax) +LANGUAGES = { + 'go': (('.go',), _C_COMMENTS, _LINE_STRINGS + r'|`[^`]*`?'), + 'javascript': (('.cjs', '.js', '.jsx', '.mjs'), _C_COMMENTS, _SCRIPT_STRINGS), + 'python': (('.py', '.pyi'), r'#[^\n]*', + r'"""(?:[^\\]|\\[\s\S])*?(?:"""|\Z)|' r"'''(?:[^\\]|\\[\s\S])*?(?:'''|\Z)|" + _LINE_STRINGS), + # A shell comment starts where a word may start: `$#` and `${x#y}` are not comments. + 'shell': (('.bash', '.sh'), r'(?%s)|(?P%s)|(?P\w+)|(?P\s+)|(?P[^\w\s])' + % (comments, strings)) for name, (_, comments, strings) in LANGUAGES.items()} +MARKER = re.compile(r'(? 0 and start_b > floor and a[start_a - 1] == b[start_b - 1]: + start_a, start_b = start_a - 1, start_b - 1 + # Two places in one file may not overlap: the block stops where its second place starts. + limit = start_b if first == second else len(a) + end_a, end_b = i, j + while end_a < limit and end_b < len(b) and a[end_a] == b[end_b]: + end_a, end_b = end_a + 1, end_b + 1 + if end_a > limit: + end_a, end_b = limit, limit + (start_b - start_a) + examined[key] = max(end_b, j + 1) + count = end_a - start_a + if count < min_tokens: + continue + places = [] + for index, start, end in ((first, start_a, end_a), (second, start_b, end_b)): + path, _, firsts, lasts = files[index] + places.append((path, firsts[start], lasts[end - 1])) + if min(last - first_line + 1 for _, first_line, last in places) >= min_lines: + found.append((places[0], places[1], count)) + return sorted(found) + + +def check(root, config): + """The pairs, the ignore marker problems and the number of files read.""" + vocabulary, barriers, files, problems = {}, itertools.count(-1, -1), [], [] + for value, language in candidates(root, config): + text = (root / value).read_bytes().decode('utf-8', 'replace').replace('\r\n', '\n') + ids, firsts, lasts, found = tokenize(text, language, vocabulary, barriers) + files.append((value, ids, firsts, lasts)) + problems.extend((value, line, message) for line, message in found) + return duplicates(files, config['min_tokens'], config['min_lines']), problems, len(files) + + +def plural(count, word): + return '%d %s%s' % (count, word, '' if count == 1 else 's') + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__.split('\n')[0]) + parser.add_argument('--root', type=Path, default=Path.cwd(), help='the repository (default: the working directory)') + parser.add_argument('--config', default='jpdev.json', help='the declaration, relative to the root (default: jpdev.json)') + args = parser.parse_args(argv) + root = args.root.resolve() + try: + path = root / args.config + if not path.is_file(): + print('duplication: off: there is no %s (a repository opts in with its thresholds)' % args.config) + return 0 + try: + declaration = json.loads(path.read_text(encoding='utf-8')) + except (OSError, ValueError) as error: + raise Refused('%s cannot be read as JSON: %s' % (args.config, error)) + config = settings(declaration, args.config) + if config is None: + print('duplication: off: %s declares no duplication member (a repository opts in with its thresholds)' + % args.config) + return 0 + pairs, problems, count = check(root, config) + except (Refused, OSError, subprocess.SubprocessError) as error: + print('duplication: refused: %s' % error, file=sys.stderr) + return 2 + scope = 'at least %s over at least %s, %s of %s' % ( + plural(config['min_tokens'], 'token'), plural(config['min_lines'], 'line'), plural(count, 'file'), + ', '.join(config['languages'])) + if not pairs and not problems: + print('duplication: no duplicate block (%s)' % scope) + return 0 + if pairs: + print('duplication: %s (%s)' % (plural(len(pairs), 'duplicate pair'), scope)) + for (path_a, first_a, last_a), (path_b, first_b, last_b), tokens in pairs: + print(' %s:%d-%d and %s:%d-%d: %d tokens' % (path_a, first_a, last_a, path_b, first_b, last_b, tokens)) + if problems: + print('duplication: %s' % plural(len(problems), 'ignore marker problem')) + for path, line, message in sorted(problems): + print(' %s:%d: %s' % (path, line, message)) + return 1 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/lib/generated_docs.py b/lib/generated_docs.py index 50230ca..2de34b4 100644 --- a/lib/generated_docs.py +++ b/lib/generated_docs.py @@ -39,7 +39,7 @@ def inside(root, value): def config(value): - need(isinstance(value, dict) and set(value) == {'version', 'generators'} and + need(isinstance(value, dict) and {'version', 'generators'} <= set(value) <= {'version', 'generators', 'duplication'} and type(value['version']) is int and value['version'] == 1, 'expected version1 generator declaration') need(isinstance(value['generators'], list) and value['generators'], 'generators must be nonempty') names, outputs = set(), [] diff --git a/scripts/dev/generated_docs.py b/scripts/dev/generated_docs.py index 50230ca..2de34b4 100644 --- a/scripts/dev/generated_docs.py +++ b/scripts/dev/generated_docs.py @@ -39,7 +39,7 @@ def inside(root, value): def config(value): - need(isinstance(value, dict) and set(value) == {'version', 'generators'} and + need(isinstance(value, dict) and {'version', 'generators'} <= set(value) <= {'version', 'generators', 'duplication'} and type(value['version']) is int and value['version'] == 1, 'expected version1 generator declaration') need(isinstance(value['generators'], list) and value['generators'], 'generators must be nonempty') names, outputs = set(), [] diff --git a/templates/dev.json b/templates/dev.json index 184b854..2057a82 100644 --- a/templates/dev.json +++ b/templates/dev.json @@ -11,6 +11,7 @@ ], "checks": [ {"name": "generated-docs", "run": "python3 scripts/dev/generated_docs.py", "toolchain": "shell", "summary": "^generated-docs:", "timeout": 300}, + {"name": "duplication", "run": "python3 scripts/dev/duplication.py", "toolchain": "shell", "summary": "^duplication:", "timeout": 300}, {"name": "build", "run": "npm run build", "toolchain": "node", "timeout": 600}, {"name": "unit-tests", "run": "npm test", "cwd": "packages/example", "toolchain": "node", "summary": "^\\s*(Test Files|Tests)\\s", "timeout": 1200}, diff --git a/templates/jpdev.json b/templates/jpdev.json index d399b55..f4f2dcc 100644 --- a/templates/jpdev.json +++ b/templates/jpdev.json @@ -1,5 +1,12 @@ { "version": 1, + "duplication": { + "languages": ["go", "python", "typescript"], + "paths": ["**"], + "exclude": ["**/testdata/**", "**/*.gen.go"], + "min_tokens": 70, + "min_lines": 6 + }, "generators": [ { "name": "reference", diff --git a/templates/jpdev.schema.json b/templates/jpdev.schema.json index a874add..dd337f9 100644 --- a/templates/jpdev.schema.json +++ b/templates/jpdev.schema.json @@ -4,13 +4,83 @@ "type": "object", "additionalProperties": false, "required": [ - "version", - "generators" + "version" + ], + "anyOf": [ + { + "required": [ + "generators" + ] + }, + { + "required": [ + "duplication" + ] + } ], "properties": { "version": { "const": 1 }, + "duplication": { + "description": "the duplication check (lib/duplication.py, vendored as scripts/dev/duplication.py): off unless this member is present; a pair of places sharing at least min_tokens tokens, each over at least min_lines lines, fails it", + "type": "object", + "additionalProperties": false, + "required": [ + "languages", + "min_tokens", + "min_lines" + ], + "properties": { + "languages": { + "description": "the languages read; each names its file extensions and comment syntax", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "enum": [ + "go", + "javascript", + "python", + "shell", + "typescript" + ] + } + }, + "paths": { + "description": "repository-relative globs of the files read (* and ? within one segment, ** as a whole segment); default **", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^(?:\\*\\*|(?!\\.\\.?(?:/|$))(?![^/]*\\*\\*)[^/\\\\\\[\\]{}]+)(?:/(?:\\*\\*|(?!\\.\\.?(?:/|$))(?![^/]*\\*\\*)[^/\\\\\\[\\]{}]+))*$" + } + }, + "exclude": { + "description": "repository-relative globs of files not read", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^(?:\\*\\*|(?!\\.\\.?(?:/|$))(?![^/]*\\*\\*)[^/\\\\\\[\\]{}]+)(?:/(?:\\*\\*|(?!\\.\\.?(?:/|$))(?![^/]*\\*\\*)[^/\\\\\\[\\]{}]+))*$" + } + }, + "min_tokens": { + "description": "the fewest shared tokens reported", + "type": "integer", + "minimum": 10, + "maximum": 10000 + }, + "min_lines": { + "description": "the fewest lines each place of a reported pair spans", + "type": "integer", + "minimum": 1, + "maximum": 10000 + } + } + }, "generators": { "type": "array", "minItems": 1, diff --git a/tests/duplication_test.py b/tests/duplication_test.py new file mode 100644 index 0000000..3e34064 --- /dev/null +++ b/tests/duplication_test.py @@ -0,0 +1,530 @@ +"""Handwritten fixture repositories exercise the duplication check through its command line.""" +import copy +import importlib.util +import itertools +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +HELPER = Path(os.environ.get('DUPLICATION_HELPER', str(ROOT / 'lib' / 'duplication.py'))) +SPEC = importlib.util.spec_from_file_location('duplication', str(HELPER)) +d = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(d) + +# One function, 29 tokens: def total ( rows ) : / result = 0 / for row in rows : / +# if row . amount > 0 : / result + = row . amount / return result. It spans 6 lines. +LEDGER = '''import os + + +def total(rows): + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result + + +def name(value): + return value.strip().lower() +''' +# The same function after other code, with a comment inside it: 7 lines (8-14). +REPORT = '''"""Report helpers.""" + + +class Ledger: + pass + + +def total(rows): + # copied from the ledger + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result +''' +# The copy between ignore markers, in the comment syntax of the language. +REPORT_IGNORED = '''"""Report helpers.""" + + +class Ledger: + pass + + +# duplication:ignore-start +def total(rows): + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result +# duplication:ignore-end +''' +# The marker text in a string is not a marker: the copy is still read. +REPORT_STRING_MARKER = '''"""Report helpers.""" + +MARK = "duplication:ignore-start" + + +def total(rows): + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result +''' +# 59 tokens on lines 6-12. +AUDIT_GO = '''package audit + +import "strings" + +// Normalize trims and lowers each name. +func Normalize(names []string) []string { + out := make([]string, 0, len(names)) + for _, name := range names { + out = append(out, strings.ToLower(strings.TrimSpace(name))) + } + return out +} +''' +# The same tokens laid out differently, with a comment: lines 7-14. +EXPORT_GO = '''package export + +import ( + "strings" +) + +func Normalize(names []string) []string { + out := make([]string, 0, len(names)) // capacity first + for _, name := range names { + out = append(out, + strings.ToLower(strings.TrimSpace(name))) + } + return out +} +''' +# The Go copy between a block-comment start marker and a line-comment end marker. +EXPORT_GO_IGNORED = '''package export + +import ( + "strings" +) + +/* duplication:ignore-start: kept apart from audit on purpose */ +func Normalize(names []string) []string { + out := make([]string, 0, len(names)) // capacity first + for _, name := range names { + out = append(out, + strings.ToLower(strings.TrimSpace(name))) + } + return out +} +// duplication:ignore-end +''' +# Three copies in one file (lines 1-6, 12-17, 23-28): two pairs, each copy against the one before it. +THREE = '''def total(rows): + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result + + +first = 1 + + +def total(rows): + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result + + +second = 2 + + +def total(rows): + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result +''' +PYTHON = {'languages': ['python'], 'min_tokens': 20, 'min_lines': 5} + + +def texts(language, text): + """The token texts of a text, a barrier shown as , and its marker problems.""" + vocabulary = {} + ids, _, _, problems = d.tokenize(text, language, vocabulary, itertools.count(-1, -1)) + names = {value: key for key, value in vocabulary.items()} + return [names.get(token, '') for token in ids], problems + + +class Fixture(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix='duplication-fixture-') + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + subprocess.run(['git', 'init', '-q', str(self.root)], check=True, capture_output=True) + + def write(self, name, text): + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + def declare(self, duplication): + self.write('jpdev.json', json.dumps({'version': 1, 'duplication': duplication})) + + def cli(self, *args): + return subprocess.run([sys.executable, str(HELPER), '--root', str(self.root)] + list(args), + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=60) + + +class DuplicatePairs(Fixture): + def test_a_copy_above_the_threshold_fails_and_names_both_places(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines(), [ + 'duplication: 1 duplicate pair (at least 20 tokens over at least 5 lines, 2 files of python)', + ' src/ledger.py:4-9 and tools/report.py:8-14: 29 tokens', + ]) + + def test_the_token_threshold_is_inclusive(self): + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + self.declare(dict(PYTHON, min_tokens=29)) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn(' src/ledger.py:4-9 and tools/report.py:8-14: 29 tokens', result.stdout) + self.declare(dict(PYTHON, min_tokens=30)) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(result.stdout, + 'duplication: no duplicate block (at least 30 tokens over at least 5 lines, 2 files of python)\n') + + def test_each_place_must_span_the_line_threshold(self): + # The ledger copy spans 6 lines and the report copy 7: the shorter one decides. + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + self.declare(dict(PYTHON, min_lines=6)) + self.assertEqual(self.cli().returncode, 1) + self.declare(dict(PYTHON, min_lines=7)) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('duplication: no duplicate block', result.stdout) + + def test_layout_and_comments_do_not_hide_a_copy(self): + self.declare({'languages': ['go'], 'min_tokens': 50, 'min_lines': 5}) + self.write('audit/normalize.go', AUDIT_GO) + self.write('export/normalize.go', EXPORT_GO) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn(' audit/normalize.go:6-12 and export/normalize.go:7-14: 59 tokens', result.stdout) + + def test_three_copies_in_one_file_make_two_pairs(self): + self.declare(PYTHON) + self.write('three.py', THREE) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], [ + ' three.py:1-6 and three.py:12-17: 29 tokens', + ' three.py:12-17 and three.py:23-28: 29 tokens', + ]) + + def test_a_repeated_short_pattern_is_not_a_copy_of_itself(self): + # "0," repeated: every run of 10 tokens recurs 2 tokens later, but the two places overlap. + self.declare({'languages': ['python'], 'min_tokens': 10, 'min_lines': 1}) + self.write('table.py', 'VALUES = [\n' + ' 0,\n' * 300 + ']\n') + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('duplication: no duplicate block', result.stdout) + + +class IgnoreBlock(Fixture): + def test_the_ignore_block_suppresses_an_intentional_copy(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT_IGNORED) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(result.stdout, + 'duplication: no duplicate block (at least 20 tokens over at least 5 lines, 2 files of python)\n') + + def test_the_markers_work_in_block_and_line_comments(self): + self.declare({'languages': ['go'], 'min_tokens': 50, 'min_lines': 5}) + self.write('audit/normalize.go', AUDIT_GO) + self.write('export/normalize.go', EXPORT_GO_IGNORED) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('duplication: no duplicate block', result.stdout) + + def test_the_marker_text_in_a_string_is_not_a_marker(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT_STRING_MARKER) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn(' src/ledger.py:4-9 and tools/report.py:6-11: 29 tokens', result.stdout) + self.assertNotIn('ignore marker problem', result.stdout) + + def test_an_ignored_region_does_not_join_the_code_around_it(self): + tokens, problems = texts('python', 'a = 1\n# duplication:ignore-start\nb = 2\n# duplication:ignore-end\nc = 3\n') + self.assertEqual(tokens, ['a', '=', '1', '', 'c', '=', '3']) + self.assertEqual(problems, []) + + def test_marker_problems_fail_and_name_the_line(self): + self.declare(PYTHON) + self.write('a.py', 'x = 1\n# duplication:ignore-start\ny = 2\n') + self.write('b.py', 'x = 1 # duplication:ignore-end\n') + self.write('c.py', '# duplication:ignore-start\n# duplication:ignore-start\nz = 3\n# duplication:ignore-end\n') + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[-4:], [ + 'duplication: 3 ignore marker problems', + ' a.py:2: duplication:ignore-start is never closed by a duplication:ignore-end', + ' b.py:1: duplication:ignore-end without a duplication:ignore-start before it', + ' c.py:2: duplication:ignore-start inside the ignore block opened at line 1', + ]) + + +class Tokens(unittest.TestCase): + def test_go_strings_and_comments(self): + tokens, _ = texts('go', 'x := `a // b` + "c /* d" // e\n/* f */ y\n') + self.assertEqual(tokens, ['x', ':', '=', '`a // b`', '+', '"c /* d"', 'y']) + + def test_script_strings_and_comments(self): + for language in ('javascript', 'typescript'): + with self.subTest(language=language): + tokens, _ = texts(language, "const u = 'http://x'; // note\nconst t = `line\n// in template`;\n") + self.assertEqual(tokens, ['const', 'u', '=', "'http://x'", ';', 'const', 't', '=', + '`line\n// in template`', ';']) + + def test_python_strings_and_comments(self): + tokens, _ = texts('python', 'a = "#" # note\nb = """x\n# not a comment\n"""\n') + self.assertEqual(tokens, ['a', '=', '"#"', 'b', '=', '"""x\n# not a comment\n"""']) + + def test_shell_comments_start_a_word(self): + tokens, _ = texts('shell', 'echo "$#" ${#list[@]} a#b # note\n') + self.assertEqual(tokens, ['echo', '"$#"', '$', '{', '#', 'list', '[', '@', ']', '}', 'a', '#', 'b']) + + def test_an_unclosed_quote_ends_at_the_line_end(self): + tokens, _ = texts('go', 'a := "open\nb := 1\n') + self.assertEqual(tokens, ['a', ':', '=', '"open', 'b', ':', '=', '1']) + + def test_a_string_over_lines_ends_on_its_last_line(self): + vocabulary = {} + _, firsts, lasts, _ = d.tokenize('x = """a\nb\nc"""\n', 'python', vocabulary, itertools.count(-1, -1)) + self.assertEqual((firsts, lasts), ([1, 1, 1], [1, 1, 3])) + + +class Scope(Fixture): + def test_off_without_a_declaration(self): + result = self.cli() + self.assertEqual((result.returncode, result.stdout), + (0, 'duplication: off: there is no jpdev.json (a repository opts in with its thresholds)\n')) + self.write('jpdev.json', json.dumps({'version': 1, 'generators': [ + {'name': 'reference', 'run': 'true', 'outputs': ['docs/reference.md']}]})) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + result = self.cli() + self.assertEqual((result.returncode, result.stdout), (0, 'duplication: off: jpdev.json declares no duplication ' + 'member (a repository opts in with its thresholds)\n')) + + def test_only_the_declared_languages_are_read(self): + self.declare({'languages': ['go'], 'min_tokens': 20, 'min_lines': 5}) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('0 files of go', result.stdout) + + def test_paths_and_exclude_globs(self): + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + self.declare(dict(PYTHON, paths=['src/**'])) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('1 file of python', result.stdout) + self.declare(dict(PYTHON, exclude=['tools/*.py'])) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('1 file of python', result.stdout) + self.declare(dict(PYTHON, paths=['**/*.py'], exclude=['docs/**'])) + self.assertEqual(self.cli().returncode, 1) + + def test_ignored_files_and_links_are_not_read(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('.gitignore', 'build/\n') + self.write('build/ledger.py', LEDGER) + (self.root / 'link.py').symlink_to(self.root / 'src' / 'ledger.py') + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('1 file of python', result.stdout) + + def test_tracked_and_untracked_files_are_read(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + subprocess.run(['git', 'add', 'src/ledger.py'], cwd=str(self.root), check=True, capture_output=True) + self.write('tools/report.py', REPORT) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn('2 files of python', result.stdout) + + def test_the_vendored_step_of_the_template_manifest_fails_the_check_list(self): + step = [s for s in json.loads((ROOT / 'templates' / 'dev.json').read_text())['checks'] + if s['name'] == 'duplication'] + self.assertEqual(len(step), 1, 'templates/dev.json lists the duplication step once') + self.write('dev.json', json.dumps({'version': 1, 'checks': step})) + (self.root / 'scripts' / 'dev').mkdir(parents=True) + shutil.copy(str(HELPER), str(self.root / 'scripts' / 'dev' / 'duplication.py')) + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + env = {k: v for k, v in os.environ.items() if not k.startswith('JPDEV_') and k != 'CI'} + env['JPDEV_ROOT'] = str(self.root) + result = subprocess.run([str(ROOT / 'templates' / 'check.sh'), '--manifest', str(self.root / 'dev.json')], + env=env, stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=120) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertRegex(result.stdout, r'(?m)^FAIL +duplication ') + self.assertIn(' duplication: 1 duplicate pair (at least 20 tokens', result.stdout) + self.assertIn(' src/ledger.py:4-9 and tools/report.py:8-14: 29 tokens', result.stdout) + + +GOOD = {'languages': ['go', 'javascript', 'python', 'shell', 'typescript'], 'paths': ['**', 'src/*.go'], + 'exclude': ['**/testdata/**', 'gen/?.ts'], 'min_tokens': 10, 'min_lines': 1} +# A change to a good declaration, the member the refusal names, and the words it says. +REFUSED = [ + ('version 2', lambda v: v.update(version=2), 'jpdev.json version: 1 is required (got 2)'), + ('not an object', lambda v: v.update(duplication=[]), 'jpdev.json duplication: an object is required'), + ('unknown member', lambda v: v['duplication'].update(threshold=50), + "jpdev.json duplication: unknown member 'threshold' (allowed: exclude, languages, min_lines, min_tokens, paths)"), + ('missing languages', lambda v: v['duplication'].pop('languages'), 'jpdev.json duplication.languages: required'), + ('missing min_tokens', lambda v: v['duplication'].pop('min_tokens'), 'jpdev.json duplication.min_tokens: required'), + ('missing min_lines', lambda v: v['duplication'].pop('min_lines'), 'jpdev.json duplication.min_lines: required'), + ('empty languages', lambda v: v['duplication'].update(languages=[]), + 'jpdev.json duplication.languages: a non-empty list of language names is required'), + ('unknown language', lambda v: v['duplication'].update(languages=['go', 'cobol']), + 'jpdev.json duplication.languages[1]: "cobol" is not one of go, javascript, python, shell, typescript'), + ('repeated language', lambda v: v['duplication'].update(languages=['go', 'go']), + "jpdev.json duplication.languages[1]: 'go' is repeated"), + ('min_tokens too small', lambda v: v['duplication'].update(min_tokens=9), + 'jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required (got 9)'), + ('min_tokens oversized', lambda v: v['duplication'].update(min_tokens=10001), + 'jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required (got 10001)'), + ('min_tokens boolean', lambda v: v['duplication'].update(min_tokens=True), + 'jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required (got true)'), + ('min_tokens string', lambda v: v['duplication'].update(min_tokens='50'), + 'jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required (got "50")'), + ('min_lines zero', lambda v: v['duplication'].update(min_lines=0), + 'jpdev.json duplication.min_lines: an integer from 1 to 10000 is required (got 0)'), + ('min_lines oversized', lambda v: v['duplication'].update(min_lines=10001), + 'jpdev.json duplication.min_lines: an integer from 1 to 10000 is required (got 10001)'), + ('empty paths', lambda v: v['duplication'].update(paths=[]), + 'jpdev.json duplication.paths: a non-empty list of globs is required'), + ('parent segment', lambda v: v['duplication'].update(paths=['../src/**']), + "jpdev.json duplication.paths[0]: '../src/**' has an empty, . or .. segment"), + ('rooted glob', lambda v: v['duplication'].update(paths=['/src/**']), + "jpdev.json duplication.paths[0]: '/src/**' must be relative to the repository root"), + ('partial double star', lambda v: v['duplication'].update(paths=['src/**.py']), + "jpdev.json duplication.paths[0]: 'src/**.py': ** must be a whole segment"), + ('character class', lambda v: v['duplication'].update(paths=['[ab]/*.py']), + "jpdev.json duplication.paths[0]: '[ab]/*.py': '[' is not supported"), + ('empty glob', lambda v: v['duplication'].update(paths=['']), + 'jpdev.json duplication.paths[0]: must be a non-empty string'), + ('empty exclude', lambda v: v['duplication'].update(exclude=[]), + 'jpdev.json duplication.exclude: a non-empty list of globs is required'), + ('repeated exclude', lambda v: v['duplication'].update(exclude=['gen/**', 'gen/**']), + "jpdev.json duplication.exclude[1]: 'gen/**' is repeated"), +] + + +class Configuration(Fixture): + def declaration(self, change=None): + value = {'version': 1, 'duplication': copy.deepcopy(GOOD)} + if change: + change(value) + return value + + def test_a_good_declaration_is_read(self): + config = d.settings(self.declaration()) + self.assertEqual((config['languages'], config['min_tokens'], config['min_lines']), + (GOOD['languages'], 10, 1)) + + def test_each_invalid_declaration_is_refused_with_its_member_named(self): + for name, change, message in REFUSED: + with self.subTest(case=name): + with self.assertRaises(d.Refused) as caught: + d.settings(self.declaration(change)) + self.assertEqual(str(caught.exception), message) + + def test_the_command_refuses_before_reading_any_file(self): + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT) + self.declare(dict(PYTHON, min_tokens=5)) + result = self.cli() + self.assertEqual(result.returncode, 2) + self.assertEqual(result.stdout, '') + self.assertEqual(result.stderr, 'duplication: refused: jpdev.json duplication.min_tokens: an integer from ' + '10 to 10000 is required (got 5)\n') + self.write('jpdev.json', '{"version": 1, "duplication": ') + result = self.cli() + self.assertEqual(result.returncode, 2) + self.assertTrue(result.stderr.startswith('duplication: refused: jpdev.json cannot be read as JSON: '), result.stderr) + self.write('jpdev.json', '[]') + result = self.cli() + self.assertEqual((result.returncode, result.stderr), (2, 'duplication: refused: jpdev.json: an object is required\n')) + + def test_the_shipped_declarations_are_read(self): + template = json.loads((ROOT / 'templates' / 'jpdev.json').read_text()) + self.assertIsNotNone(d.settings(template), 'templates/jpdev.json shows the duplication member') + + +class Schema(unittest.TestCase): + """templates/jpdev.schema.json holds the shape the command reads.""" + + def setUp(self): + self.schema = json.loads((ROOT / 'templates' / 'jpdev.schema.json').read_text()) + self.member = self.schema['properties']['duplication'] + + def test_the_schema_and_the_command_name_the_same_members_and_bounds(self): + member = self.member + self.assertEqual(sorted(member['properties']), list(d.MEMBERS)) + self.assertEqual(member['required'], ['languages', 'min_tokens', 'min_lines']) + self.assertFalse(member['additionalProperties']) + self.assertEqual(member['properties']['languages']['items']['enum'], sorted(d.LANGUAGES)) + for name, bounds in (('min_tokens', d.MIN_TOKENS), ('min_lines', d.MIN_LINES)): + limits = member['properties'][name] + self.assertEqual((limits['type'], limits['minimum'], limits['maximum']), ('integer',) + bounds) + + def test_the_schema_and_the_command_agree_on_every_vector(self): + try: + import jsonschema + except ImportError: + self.skipTest('the jsonschema package is not installed') + validator = jsonschema.Draft202012Validator(self.schema) + good = {'version': 1, 'duplication': copy.deepcopy(GOOD)} + self.assertEqual(list(validator.iter_errors(good)), []) + self.assertEqual(list(validator.iter_errors({'version': 1, 'duplication': dict(PYTHON)})), [], + 'a declaration of duplication alone is valid') + self.assertEqual(list(validator.iter_errors(json.loads((ROOT / 'templates' / 'jpdev.json').read_text()))), []) + for name, change, _ in REFUSED: + value = copy.deepcopy(good) + change(value) + with self.subTest(case=name): + self.assertTrue(list(validator.iter_errors(value)), 'the schema accepts what the command refuses') + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/generated_docs_test.py b/tests/generated_docs_test.py index 281f4de..8c3e2aa 100644 --- a/tests/generated_docs_test.py +++ b/tests/generated_docs_test.py @@ -215,6 +215,17 @@ def test_missing_dependency_refused(self): self.assertNotEqual(result.returncode, 0) self.assertIn('missing installed dependency', result.stderr) + def test_duplication_member_is_left_to_the_duplication_check(self): + duplication = {'languages':['python'], 'min_tokens':50, 'min_lines':5} + self.assertEqual(g.config({'version':1, 'generators':[self.item], 'duplication':duplication}), [self.item]) + with self.assertRaises(g.Invalid): + g.config({'version':1, 'duplication':duplication}) + self.write('jpdev.json', json.dumps({'version':1, 'generators':[self.item], 'duplication':duplication})) + result = subprocess.run([sys.executable, str(HELPER), '--root', str(self.root)], + capture_output=True, text=True, timeout=15) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn('reference current', result.stdout) + if __name__ == '__main__': unittest.main() diff --git a/tests/run.sh b/tests/run.sh index c1ae706..54f83a5 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -45,6 +45,11 @@ absent() { out=$(python3 "$kit/tests/ci_evidence_test.py" 2>&1) && s=0 || s=$? expect "trusted CI evidence admission and collector fixtures" 0 'OK' "$out" "$s" +# The duplication check: handwritten fixture repositories through its command line and the +# template manifest's step; the schema and the command agree on every declaration vector. +out=$(python3 "$kit/tests/duplication_test.py" 2>&1) && s=0 || s=$? +expect "duplication check fixtures: pairs, thresholds, ignore blocks, refused declarations" 0 'OK' "$out" "$s" + # Generated documents: actual fresh/stale fixture repository CLI invocations. out=$(python3 "$kit/tests/generated_docs_test.py" 2>&1) && s=0 || s=$? expect "generated document scratch regeneration fixtures" 0 'OK' "$out" "$s" From 7008b8a39945e5f40626f5f31d169d4dd0227108 Mon Sep 17 00:00:00 2001 From: kikashy <35789537+kikashy@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:02:13 -0400 Subject: [PATCH 2/3] Run the duplication check on the kit itself jpdev.json declares the kit's duplication member: its Python and shell files at 100 tokens and 10 lines, leaving out scripts/dev/**, the vendored copies of templates/check.sh, lib/generated_docs.py and lib/handover_check.sh that are kept the same on purpose (the only pairs the tree has at those thresholds). dev.json runs `python3 lib/duplication.py` as the `duplication` step, the CI lint job runs it, and .github/ci-evidence.json lists it among the lint job's checks with lib/duplication.py and jpdev.json as inputs. tests/duplication_test.py reads the kit's own declaration, and runs the schema vectors through the standard-library reader of tests/changed_checks_test.py as well as jsonschema, so the vector test no longer skips where jsonschema is not installed. Signed-off-by: kikashy <35789537+kikashy@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- .github/ci-evidence.json | 3 +++ .github/workflows/ci.yml | 7 ++++--- README.md | 9 +++++++-- dev.json | 1 + jpdev.json | 6 ++++++ tests/duplication_test.py | 35 ++++++++++++++++++++++------------- 6 files changed, 43 insertions(+), 18 deletions(-) diff --git a/.github/ci-evidence.json b/.github/ci-evidence.json index 2c6ff23..ef0cc19 100644 --- a/.github/ci-evidence.json +++ b/.github/ci-evidence.json @@ -8,6 +8,8 @@ "lib/ci_evidence.py", "templates/check.sh", "lib/generated_docs.py", + "lib/duplication.py", + "jpdev.json", "scripts/dev/generated_docs.py", "tests/generated_docs_test.py", "tests/run.sh", @@ -60,6 +62,7 @@ "local-paths", "agent-docs", "bash-n", + "duplication", "json", "shellcheck" ] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa8e2ff..aeadbc7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,12 +24,13 @@ jobs: with: persist-credentials: false # Whitespace; no machine-local path in tracked text; CLAUDE.md is AGENTS.md and the root - # copy starts with the template; every script parses; the JSON documents parse; shellcheck - # (the program the runner image carries). + # copy starts with the template; every script parses; no block of code is duplicated over the + # thresholds of jpdev.json; the JSON documents parse; shellcheck (the program the runner image + # carries). - name: The lint steps of dev.json run: | set -o pipefail - scripts/dev/check.sh --all --only diff-check,local-paths,agent-docs,bash-n,json,shellcheck | tee "${{ runner.temp }}/checks.log" + scripts/dev/check.sh --all --only diff-check,local-paths,agent-docs,bash-n,duplication,json,shellcheck | tee "${{ runner.temp }}/checks.log" - name: Record exact CI inputs and coverage if: github.ref == 'refs/heads/main' && github.event_name != 'pull_request' diff --git a/README.md b/README.md index d333cbb..9629d3c 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,11 @@ duplication: 1 duplicate pair (at least 70 tokens over at least 6 lines, 214 fil internal/audit/receipt.go:18-50 and internal/export/receipt.go:40-72: 131 tokens ``` +The kit runs the check on itself with `python3 lib/duplication.py`, over its Python and shell +files at 100 tokens and 10 lines, and leaves out `scripts/dev/**`: those files are the vendored +copies of `templates/check.sh`, `lib/generated_docs.py` and `lib/handover_check.sh`, kept the +same on purpose. + The exit status is 0 when nothing is found or the check is off, 1 when a pair or an ignore marker problem is found, and 2 when the declaration is refused. A refusal names the member, for example `duplication: refused: jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required @@ -267,8 +272,8 @@ Python steps, and for other steps that invoke `python3`, name the interpreter us - `tests/run.sh`: the kit's own tests (`tests/fixture/` holds a manifest of shell echoes and one deliberate failure, and the manifest and mapping of the `jpdev check --changed` fixture clones). - `dev.json`, `jpdev.json`, `scripts/dev/check.sh`, `scripts/dev/handover_check.sh`, `AGENTS.md`, - `CLAUDE.md`: the kit on itself, the list of checks CI runs, its changed-path mapping, and the - handover guard. `CONTRIBUTING.md`, `SECURITY.md`, `docs/releasing.md`: contributing, reporting + `CLAUDE.md`: the kit on itself, the list of checks CI runs, its changed-path mapping and + duplication thresholds, and the handover guard. `CONTRIBUTING.md`, `SECURITY.md`, `docs/releasing.md`: contributing, reporting and releasing. Apache-2.0; see `LICENSE`. diff --git a/dev.json b/dev.json index e2f4020..877428c 100644 --- a/dev.json +++ b/dev.json @@ -6,6 +6,7 @@ {"name": "local-paths", "run": "if git grep -nE '(^|[^A-Za-z0-9_])/(home|tmp)/[A-Za-z0-9_.-]+'; then echo 'a machine-local path is in tracked text'; exit 1; fi", "toolchain": "shell"}, {"name": "agent-docs", "run": "scripts/sync-agent-docs.sh --check && scripts/sync-agent-docs.sh --check . && head -c \"$(wc -c < templates/AGENTS.md)\" AGENTS.md | cmp - templates/AGENTS.md", "toolchain": "shell", "summary": "^sync-agent-docs:"}, {"name": "bash-n", "run": "set -eu; for file in bin/* lib/*.sh templates/*.sh scripts/*.sh scripts/dev/*.sh tests/*.sh tests/fixture/*.sh; do bash -n \"$file\"; done; echo 'bash -n: every script parses'", "toolchain": "shell", "summary": "^bash -n:"}, + {"name": "duplication", "run": "python3 lib/duplication.py", "toolchain": "shell", "summary": "^duplication:", "timeout": 300}, {"name": "json", "run": "python3 -I -c 'import glob, json; files = sorted(glob.glob(\"*.json\") + glob.glob(\"templates/*.json\") + glob.glob(\"examples/*.json\") + glob.glob(\"tests/fixture/*.json\")); [json.load(open(f, encoding=\"utf-8\")) for f in files]; print(\"parsed %d JSON documents\" % len(files))'", "toolchain": "shell", "summary": "^parsed "}, {"name": "shellcheck", "run": "shellcheck -x -P \".:SCRIPTDIR\" bin/* lib/*.sh templates/*.sh scripts/*.sh scripts/dev/*.sh tests/*.sh tests/fixture/*.sh && echo 'shellcheck: clean'", "toolchain": "shell", "summary": "^shellcheck:"}, {"name": "tests", "run": "tests/run.sh", "toolchain": "shell", "summary": "^(Failed cases:|[0-9]+ passed, [0-9]+ failed)", "timeout": 900} diff --git a/jpdev.json b/jpdev.json index bec5253..40891ad 100644 --- a/jpdev.json +++ b/jpdev.json @@ -1,5 +1,11 @@ { "version": 1, + "duplication": { + "languages": ["python", "shell"], + "exclude": ["scripts/dev/**"], + "min_tokens": 100, + "min_lines": 10 + }, "changed": [ { "paths": ["docs/**", "*.md", "LICENSE", "NOTICE", ".github/PULL_REQUEST_TEMPLATE.md"], diff --git a/tests/duplication_test.py b/tests/duplication_test.py index 3e34064..56db3a5 100644 --- a/tests/duplication_test.py +++ b/tests/duplication_test.py @@ -487,8 +487,9 @@ def test_the_command_refuses_before_reading_any_file(self): self.assertEqual((result.returncode, result.stderr), (2, 'duplication: refused: jpdev.json: an object is required\n')) def test_the_shipped_declarations_are_read(self): - template = json.loads((ROOT / 'templates' / 'jpdev.json').read_text()) - self.assertIsNotNone(d.settings(template), 'templates/jpdev.json shows the duplication member') + for name in ('templates/jpdev.json', 'jpdev.json'): + with self.subTest(declaration=name): + self.assertIsNotNone(d.settings(json.loads((ROOT / name).read_text())), name + ' declares the member') class Schema(unittest.TestCase): @@ -509,21 +510,29 @@ def test_the_schema_and_the_command_name_the_same_members_and_bounds(self): self.assertEqual((limits['type'], limits['minimum'], limits['maximum']), ('integer',) + bounds) def test_the_schema_and_the_command_agree_on_every_vector(self): + # The standard-library reader of tests/changed_checks_test.py always runs, so the test never + # skips; the jsonschema package, where it is installed, reads the same vectors too. + spec = importlib.util.spec_from_file_location('changed_checks_test', str(ROOT / 'tests' / 'changed_checks_test.py')) + reader = importlib.util.module_from_spec(spec) + spec.loader.exec_module(reader) + readers = [('stdlib', lambda value: reader.schema_errors(self.schema, value))] try: import jsonschema + validator = jsonschema.Draft202012Validator(self.schema) + readers.append(('jsonschema', lambda value: list(validator.iter_errors(value)))) except ImportError: - self.skipTest('the jsonschema package is not installed') - validator = jsonschema.Draft202012Validator(self.schema) + pass good = {'version': 1, 'duplication': copy.deepcopy(GOOD)} - self.assertEqual(list(validator.iter_errors(good)), []) - self.assertEqual(list(validator.iter_errors({'version': 1, 'duplication': dict(PYTHON)})), [], - 'a declaration of duplication alone is valid') - self.assertEqual(list(validator.iter_errors(json.loads((ROOT / 'templates' / 'jpdev.json').read_text()))), []) - for name, change, _ in REFUSED: - value = copy.deepcopy(good) - change(value) - with self.subTest(case=name): - self.assertTrue(list(validator.iter_errors(value)), 'the schema accepts what the command refuses') + shipped = [json.loads((ROOT / name).read_text()) for name in ('templates/jpdev.json', 'jpdev.json')] + for name, errors in readers: + with self.subTest(reader=name): + for value in [good, {'version': 1, 'duplication': dict(PYTHON)}] + shipped: + self.assertEqual(errors(value), [], 'the schema refuses what the command reads') + for case, change, _ in REFUSED: + value = copy.deepcopy(good) + change(value) + with self.subTest(reader=name, case=case): + self.assertTrue(errors(value), 'the schema accepts what the command refuses') if __name__ == '__main__': From 5dd5f4418165a3e6972bb8a6f04f111477c8a5fe Mon Sep 17 00:00:00 2001 From: kikashy <35789537+kikashy@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:23:19 -0400 Subject: [PATCH 3/3] Duplication check review fixes: earlier places, mappings, ignore blocks, links, size - A run of min_tokens tokens was compared only with the most recent place it appeared. When that pair spanned too few lines, earlier copies were never compared: two copies around a compact one went unreported. The 16 most recent places of each run are now kept and walked nearest first until one makes a reported pair; each block is still examined once. After a run that repeats itself in one file and is not reported, the other places of that file are passed over for that run, so a table of one short row is still not a copy of itself. - templates/jpdev.json selects `duplication` in every changed-path rule that can match a file of a declared language, as does the kit's own docs rule; docs/adopting.md says to do so; a test holds both files to it. - New fixtures: an ignore block elsewhere in a file does not hide a copy outside it; the code around two ignore blocks does not join; an ignore block inside a copy splits it. - A path is read only when it resolves to itself, so a tracked directory replaced by a link is not followed. - `max_file_bytes` (1024 to 104857600, default 1048576): a larger file is not read and is named; so is a file with a NUL byte in its first 8192 bytes. Running out of memory is a named refusal (exit 2). - The schema gives the integers `multipleOf: 1` and gains max_file_bytes; the standard-library schema reader of tests/changed_checks_test.py reads multipleOf. README says how the walk, the size and binary limits, here-documents, carriage-return line endings and a split copy behave. Signed-off-by: kikashy <35789537+kikashy@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- README.md | 26 ++-- docs/adopting.md | 7 +- jpdev.json | 2 +- lib/duplication.py | 150 ++++++++++++++------- templates/jpdev.json | 8 +- templates/jpdev.schema.json | 9 ++ tests/changed_checks_test.py | 4 +- tests/duplication_test.py | 247 ++++++++++++++++++++++++++++++++++- 8 files changed, 387 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 9629d3c..abd20a8 100644 --- a/README.md +++ b/README.md @@ -205,19 +205,29 @@ with its thresholds (`templates/jpdev.schema.json` holds the shape): its file extensions and comment syntax. The files read are the tracked, and the untracked but not ignored, files of those extensions that match a `paths` glob (default `**`) and no `exclude` glob. Globs are repository-relative: `*` and `?` stay within one segment and a `**` segment spans any - number of segments. A symbolic link is not followed. + number of segments. A symbolic link is not followed, whether it is the file or a directory + above it. A file larger than `max_file_bytes` (1024 to 104857600, default 1048576) is not read, + nor is a file with a NUL byte in its first 8192 bytes (binary); each is named in a `not read:` + line, and the check does not fail on it. A large generated file belongs in `exclude`. - `min_tokens` (10 to 10000) and `min_lines` (1 to 10000), both required: a pair is reported when two places share at least `min_tokens` tokens and each place spans at least `min_lines` lines. Whitespace, layout and comments are not tokens; identifiers, keywords, numbers, string literals and each punctuation character are. A copy laid out or commented differently is still a copy; a - copy whose names were changed is not. + copy whose names were changed is not. A line ends at a line feed: a file with carriage-return + line endings alone counts as one line. - An intentional copy is marked with an ignore block: the code between a comment that holds `duplication:ignore-start` and a comment that holds `duplication:ignore-end` is not read, in any - comment syntax of the language (`//`, `/* */`, `#`). The text in a string is not a marker. A start + comment syntax of the language (`//`, `/* */`, `#`). The text in a string is not a marker; the + text of a shell here-document is read as code, so a marker after a `#` there counts. An ignore + block inside a copy splits it in two, and each part is held to the thresholds on its own. A start that is never closed, an end with no start, and a start inside an open block are reported. -The report names each pair with both places and the shared token count, one line each. Three -copies of a block make two pairs: each copy is named against the one before it. +The report names each pair with both places and the shared token count, one line each. Each run of +`min_tokens` tokens is compared with the places it appeared before (the 16 most recent), nearest +first, until one makes a reported pair: three copies of a block make two pairs, and a copy is still +named when a nearer place of the same tokens spans too few lines. Two places in one file never +overlap, and a run that repeats a pattern shorter than `min_tokens` in one file (a table of one +short row) is not reported as copies of itself. ``` duplication: 1 duplicate pair (at least 70 tokens over at least 6 lines, 214 files of go, python, typescript) @@ -230,9 +240,9 @@ copies of `templates/check.sh`, `lib/generated_docs.py` and `lib/handover_check. same on purpose. The exit status is 0 when nothing is found or the check is off, 1 when a pair or an ignore marker -problem is found, and 2 when the declaration is refused. A refusal names the member, for example -`duplication: refused: jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required -(got 5)`. +problem is found, and 2 when the declaration is refused or the files in scope do not fit in +memory. A refusal names the member, for example `duplication: refused: jpdev.json +duplication.min_tokens: an integer from 10 to 10000 is required (got 5)`. ## The caps diff --git a/docs/adopting.md b/docs/adopting.md index e4a8a5a..5bed692 100644 --- a/docs/adopting.md +++ b/docs/adopting.md @@ -174,8 +174,11 @@ the `duplication` step of `templates/dev.json` in the check list, and declare th languages and its token and line thresholds in the `duplication` member of `jpdev.json` (`generators` may be left out when `duplication` is present). Until the member is present the step passes and its summary line says `duplication: off`. Choose thresholds the current tree passes, or -remove or mark the copies the first run names. [The duplication check](../README.md#the-duplication-check) -describes the member, the ignore block and the report. +remove or mark the copies the first run names. When `jpdev.json` also maps changed paths +(`changed`), list `duplication` in every rule whose globs can match a file of a declared language, +or `jpdev check --changed` does not run the step for those changes. +[The duplication check](../README.md#the-duplication-check) describes the member, the ignore block +and the report. ## Generated reference documents diff --git a/jpdev.json b/jpdev.json index 40891ad..eed957f 100644 --- a/jpdev.json +++ b/jpdev.json @@ -9,7 +9,7 @@ "changed": [ { "paths": ["docs/**", "*.md", "LICENSE", "NOTICE", ".github/PULL_REQUEST_TEMPLATE.md"], - "checks": ["diff-check", "local-paths"] + "checks": ["diff-check", "local-paths", "duplication"] }, { "paths": ["AGENTS.md", "CLAUDE.md", "templates/AGENTS.md", "templates/CLAUDE.md"], diff --git a/lib/duplication.py b/lib/duplication.py index 1e43ef8..646bb2e 100644 --- a/lib/duplication.py +++ b/lib/duplication.py @@ -2,7 +2,8 @@ """The duplication check: name each block of code that appears in two places. Off unless the repository's jpdev.json declares a `duplication` member: the languages to read, a -token threshold, a line threshold and, optionally, path globs. A pair is reported when two places +token threshold, a line threshold and, optionally, path globs and a file size limit. A file over +the limit, or with a NUL byte near its start, is not read and is named. A pair is reported when two places share at least min_tokens tokens and each place spans at least min_lines lines. Layout and comments are not tokens; identifiers, keywords, numbers, string literals and each punctuation character are. The code between a comment holding `duplication:ignore-start` and a comment holding @@ -22,7 +23,13 @@ MIN_TOKENS = (10, 10000) MIN_LINES = (1, 10000) -MEMBERS = ('exclude', 'languages', 'min_lines', 'min_tokens', 'paths') +MAX_FILE_BYTES = (1024, 104857600) +DEFAULT_MAX_FILE_BYTES = 1048576 +MEMBERS = ('exclude', 'languages', 'max_file_bytes', 'min_lines', 'min_tokens', 'paths') +# A file whose first BINARY_PROBE bytes hold a NUL byte is binary and is not read. +BINARY_PROBE = 8192 +# The most recent places of each run of min_tokens tokens that a later place is compared with. +RECENT = 16 _C_COMMENTS = r'//[^\n]*|/\*[\s\S]*?(?:\*/|\Z)' # A quoted string that may not span a line ends at the line's end when it is not closed. @@ -124,6 +131,8 @@ def settings(declaration, name='jpdev.json'): 'exclude': _globs(value['exclude'], where + '.exclude') if 'exclude' in value else [], 'min_tokens': _integer(value['min_tokens'], MIN_TOKENS, where + '.min_tokens'), 'min_lines': _integer(value['min_lines'], MIN_LINES, where + '.min_lines'), + 'max_file_bytes': _integer(value.get('max_file_bytes', DEFAULT_MAX_FILE_BYTES), MAX_FILE_BYTES, + where + '.max_file_bytes'), } @@ -141,8 +150,10 @@ def candidates(root, config): or any(p.fullmatch(value) for p in config['exclude'])): continue path = root / value - # A symbolic link is not followed; a tracked file deleted from the work tree is not read. - if path.is_symlink() or not path.is_file(): + # A symbolic link is not followed, whether it is the file or a directory above it (a + # tracked directory replaced by a link), and a tracked file deleted from the work tree is + # not read: the path must resolve to itself, a regular file below the root. + if path.is_symlink() or not path.is_file() or path.resolve() != path: continue result.append((value, language)) return result @@ -198,64 +209,109 @@ def windows(ids, size): yield position, value +def _examine(files, first, i, second, j, floor, min_tokens, min_lines, found): + """Extend the shared run at (first, i) and (second, j) to its whole block and report it. + + Returns the end of the block in the second file, whether it was reported, and whether it is a + run that repeats itself: two places in one file whose shared tokens go on past the second + place's start, cut there so the places do not overlap. + """ + a, b = files[first][1], files[second][1] + start_a, start_b = i, j + while start_a > 0 and start_b > floor and a[start_a - 1] == b[start_b - 1]: + start_a, start_b = start_a - 1, start_b - 1 + # Two places in one file may not overlap: the block stops where its second place starts. + limit = start_b if first == second else len(a) + end_a, end_b = i, j + while end_a < limit and end_b < len(b) and a[end_a] == b[end_b]: + end_a, end_b = end_a + 1, end_b + 1 + repeats = end_a > limit or (first == second and end_a == limit and end_b < len(b) and a[end_a] == b[end_b]) + if end_a > limit: + end_a, end_b = limit, limit + (start_b - start_a) + count = end_a - start_a + reported = False + if count >= min_tokens: + places = [] + for index, start, end in ((first, start_a, end_a), (second, start_b, end_b)): + path, _, firsts, lasts = files[index] + places.append((path, firsts[start], lasts[end - 1])) + if min(last - first_line + 1 for _, first_line, last in places) >= min_lines: + found.append((places[0], places[1], count)) + reported = True + return max(end_b, j + 1), reported, repeats + + def duplicates(files, min_tokens, min_lines): """Each pair of places sharing at least min_tokens tokens over at least min_lines lines. - files is a list of (path, ids, first lines, last lines). A run of min_tokens tokens is - compared with the previous place the same run appeared, so three copies make two pairs; the - pair is then extended both ways to the whole shared block, which is examined once. + files is a list of (path, ids, first lines, last lines). Each run of min_tokens tokens is + compared with the places the same run appeared before, the most recent first, until one makes + a reported pair: three copies make two pairs, and a copy is still named when a nearer place of + the same tokens spans too few lines. The RECENT most recent places of a run are kept. A pair is + extended both ways to its whole shared block, and each block is examined once. After a run + that repeats itself in one file (a table of one short row) and is not reported, the other + places of that file are passed over for that run; the places in other files are still compared. """ - latest, examined, found = {}, {}, [] + seen, examined, found = {}, {}, [] for second, (_, b, _, _) in enumerate(files): for j, value in windows(b, min_tokens): - previous = latest.get(value) - latest[value] = second * _STRIDE + j - if previous is None: - continue - first, i = divmod(previous, _STRIDE) - key = (first, second, j - i) - floor = examined.get(key, 0) - if j < floor: - continue - a = files[first][1] - start_a, start_b = i, j - while start_a > 0 and start_b > floor and a[start_a - 1] == b[start_b - 1]: - start_a, start_b = start_a - 1, start_b - 1 - # Two places in one file may not overlap: the block stops where its second place starts. - limit = start_b if first == second else len(a) - end_a, end_b = i, j - while end_a < limit and end_b < len(b) and a[end_a] == b[end_b]: - end_a, end_b = end_a + 1, end_b + 1 - if end_a > limit: - end_a, end_b = limit, limit + (start_b - start_a) - examined[key] = max(end_b, j + 1) - count = end_a - start_a - if count < min_tokens: + here = second * _STRIDE + j + earlier = seen.get(value) + if earlier is None: + seen[value] = here continue - places = [] - for index, start, end in ((first, start_a, end_a), (second, start_b, end_b)): - path, _, firsts, lasts = files[index] - places.append((path, firsts[start], lasts[end - 1])) - if min(last - first_line + 1 for _, first_line, last in places) >= min_lines: - found.append((places[0], places[1], count)) + if type(earlier) is int: + earlier = seen[value] = [earlier] + skip_file = None + for previous in reversed(earlier): + first, i = divmod(previous, _STRIDE) + if first == skip_file: + continue + key = (first, second, j - i) + state = examined.get(key) + if state is None or j >= state[0]: + state = examined[key] = _examine(files, first, i, second, j, state[0] if state else 0, + min_tokens, min_lines, found) + if state[1]: + break + if state[2]: + skip_file = second + earlier.append(here) + if len(earlier) > RECENT: + del earlier[0] return sorted(found) def check(root, config): - """The pairs, the ignore marker problems and the number of files read.""" - vocabulary, barriers, files, problems = {}, itertools.count(-1, -1), [], [] + """The pairs, the ignore marker problems, the number of files read and the files not read.""" + vocabulary, barriers, files, problems, skipped = {}, itertools.count(-1, -1), [], [], [] for value, language in candidates(root, config): - text = (root / value).read_bytes().decode('utf-8', 'replace').replace('\r\n', '\n') + path = root / value + size = path.stat().st_size + if size > config['max_file_bytes']: + skipped.append((value, '%d bytes, over max_file_bytes %d (a large generated file belongs in exclude)' + % (size, config['max_file_bytes']))) + continue + data = path.read_bytes() + if b'\0' in data[:BINARY_PROBE]: + skipped.append((value, 'binary (a NUL byte in its first %d bytes)' % BINARY_PROBE)) + continue + text = data.decode('utf-8', 'replace').replace('\r\n', '\n') ids, firsts, lasts, found = tokenize(text, language, vocabulary, barriers) files.append((value, ids, firsts, lasts)) problems.extend((value, line, message) for line, message in found) - return duplicates(files, config['min_tokens'], config['min_lines']), problems, len(files) + return duplicates(files, config['min_tokens'], config['min_lines']), problems, len(files), skipped def plural(count, word): return '%d %s%s' % (count, word, '' if count == 1 else 's') +def notices(skipped): + for path, reason in skipped: + print(' not read: %s: %s' % (path, reason)) + + def main(argv=None): parser = argparse.ArgumentParser(description=__doc__.split('\n')[0]) parser.add_argument('--root', type=Path, default=Path.cwd(), help='the repository (default: the working directory)') @@ -276,15 +332,20 @@ def main(argv=None): print('duplication: off: %s declares no duplication member (a repository opts in with its thresholds)' % args.config) return 0 - pairs, problems, count = check(root, config) + try: + pairs, problems, count, skipped = check(root, config) + except MemoryError: + raise Refused('out of memory reading the files in scope; put large generated files in exclude or ' + 'lower max_file_bytes') except (Refused, OSError, subprocess.SubprocessError) as error: print('duplication: refused: %s' % error, file=sys.stderr) return 2 - scope = 'at least %s over at least %s, %s of %s' % ( + scope = 'at least %s over at least %s, %s of %s%s' % ( plural(config['min_tokens'], 'token'), plural(config['min_lines'], 'line'), plural(count, 'file'), - ', '.join(config['languages'])) + ', '.join(config['languages']), '; %s not read' % plural(len(skipped), 'file') if skipped else '') if not pairs and not problems: print('duplication: no duplicate block (%s)' % scope) + notices(skipped) return 0 if pairs: print('duplication: %s (%s)' % (plural(len(pairs), 'duplicate pair'), scope)) @@ -294,6 +355,7 @@ def main(argv=None): print('duplication: %s' % plural(len(problems), 'ignore marker problem')) for path, line, message in sorted(problems): print(' %s:%d: %s' % (path, line, message)) + notices(skipped) return 1 diff --git a/templates/jpdev.json b/templates/jpdev.json index 30a274e..a88ea38 100644 --- a/templates/jpdev.json +++ b/templates/jpdev.json @@ -21,19 +21,19 @@ "changed": [ { "paths": ["docs/**", "*.md"], - "checks": ["generated-docs", "diff-check"] + "checks": ["generated-docs", "duplication", "diff-check"] }, { "paths": ["go/**"], - "checks": ["generated-docs", "go-test", "diff-check"] + "checks": ["generated-docs", "duplication", "go-test", "diff-check"] }, { "paths": ["packages/example/**"], - "checks": ["build", "unit-tests", "diff-check"] + "checks": ["duplication", "build", "unit-tests", "diff-check"] }, { "paths": ["tests/**/*.py"], - "checks": ["python-tests", "diff-check"] + "checks": ["duplication", "python-tests", "diff-check"] } ] } diff --git a/templates/jpdev.schema.json b/templates/jpdev.schema.json index 36a8ad7..99906cd 100644 --- a/templates/jpdev.schema.json +++ b/templates/jpdev.schema.json @@ -75,14 +75,23 @@ "min_tokens": { "description": "the fewest shared tokens reported", "type": "integer", + "multipleOf": 1, "minimum": 10, "maximum": 10000 }, "min_lines": { "description": "the fewest lines each place of a reported pair spans", "type": "integer", + "multipleOf": 1, "minimum": 1, "maximum": 10000 + }, + "max_file_bytes": { + "description": "a file larger than this is not read, and is named; default 1048576", + "type": "integer", + "multipleOf": 1, + "minimum": 1024, + "maximum": 104857600 } } }, diff --git a/tests/changed_checks_test.py b/tests/changed_checks_test.py index 7da7580..7e5b5b8 100644 --- a/tests/changed_checks_test.py +++ b/tests/changed_checks_test.py @@ -370,7 +370,7 @@ def test_changed_and_only_are_not_given_together(self): SCHEMA_KEYWORDS = {'$schema', 'title', 'description', 'type', 'const', 'enum', 'required', 'anyOf', 'properties', 'additionalProperties', 'items', 'minItems', 'uniqueItems', 'minLength', 'minimum', - 'maximum', 'pattern'} + 'maximum', 'multipleOf', 'pattern'} SCHEMA_TYPES = {'object': dict, 'array': list, 'string': str} @@ -416,6 +416,8 @@ def schema_errors(schema, value, where='$'): errors.append('%s: does not match the pattern' % where) if type(value) is int and not schema.get('minimum', value) <= value <= schema.get('maximum', value): errors.append('%s: out of range' % where) + if type(value) in (int, float) and 'multipleOf' in schema and value % schema['multipleOf']: + errors.append('%s: not a multiple of %r' % (where, schema['multipleOf'])) return errors diff --git a/tests/duplication_test.py b/tests/duplication_test.py index 56db3a5..a8981df 100644 --- a/tests/duplication_test.py +++ b/tests/duplication_test.py @@ -2,14 +2,18 @@ import copy import importlib.util import itertools +import contextlib +import io import json import os from pathlib import Path +import re import shutil import subprocess import sys import tempfile import unittest +from unittest.mock import patch ROOT = Path(__file__).resolve().parents[1] HELPER = Path(os.environ.get('DUPLICATION_HELPER', str(ROOT / 'lib' / 'duplication.py'))) @@ -157,6 +161,86 @@ def total(rows): result += row.amount return result ''' +# One JavaScript function, 31 tokens: function total ( rows ) { / let sum = 0 ; / +# for ( const row of rows ) { / sum + = row . amount ; / } / return sum ; / }. On lines 2-8. +SPREAD_JS = '''// ledger totals +function total(rows) { + let sum = 0; + for (const row of rows) { + sum += row.amount; + } + return sum; +} +''' +# The same 31 tokens on lines 2-4: too few lines for a line threshold of 5. +COMPACT_JS = '''// the same tokens on three lines +function total(rows) { let sum = 0; + for (const row of rows) { sum += row.amount; } + return sum; } +''' +# The spread copy (2-8), the compact copy (10-12) and the spread copy again (14-20) in one file. +ONE_JS = '''// three places of one function +function total(rows) { + let sum = 0; + for (const row of rows) { + sum += row.amount; + } + return sum; +} +const first = 1; +function total(rows) { let sum = 0; + for (const row of rows) { sum += row.amount; } + return sum; } +const second = 2; +function total(rows) { + let sum = 0; + for (const row of rows) { + sum += row.amount; + } + return sum; +} +''' +JAVASCRIPT = {'languages': ['javascript'], 'min_tokens': 25, 'min_lines': 5} +# A valid ignore block in the file, and the copy outside it on lines 9-15. +REPORT_BLOCK_ELSEWHERE = '''"""Report helpers.""" + +# duplication:ignore-start +class Ledger: + pass +# duplication:ignore-end + + +def total(rows): + # copied from the ledger + result = 0 + for row in rows: + if row.amount > 0: + result += row.amount + return result +''' +# The code around two ignore blocks is the same 8 + 8 tokens twice: it may not join across them. +TWO_BLOCKS = '''alpha = compute(1, 2) +# duplication:ignore-start +skipped = 1 +# duplication:ignore-end +beta = combine(alpha, 3) +gamma = 0 +alpha = compute(1, 2) +# duplication:ignore-start +skipped = 2 +# duplication:ignore-end +beta = combine(alpha, 3) +''' +# The ledger's function with an ignore block inside it: 14 tokens (lines 1-3), the block, 8 tokens. +SPLIT = '''def total(rows): + result = 0 + for row in rows: + # duplication:ignore-start + if row.amount > 0: + # duplication:ignore-end + result += row.amount + return result +''' PYTHON = {'languages': ['python'], 'min_tokens': 20, 'min_lines': 5} @@ -242,6 +326,30 @@ def test_three_copies_in_one_file_make_two_pairs(self): ' three.py:12-17 and three.py:23-28: 29 tokens', ]) + def test_a_copy_is_named_when_a_nearer_place_spans_too_few_lines(self): + self.declare(JAVASCRIPT) + self.write('lib/a.js', SPREAD_JS) + self.write('lib/b.js', COMPACT_JS) + self.write('lib/c.js', SPREAD_JS) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines(), [ + 'duplication: 1 duplicate pair (at least 25 tokens over at least 5 lines, 3 files of javascript)', + ' lib/a.js:2-8 and lib/c.js:2-8: 31 tokens', + ]) + # The compact place sorting last changes nothing. + (self.root / 'lib' / 'b.js').rename(self.root / 'lib' / 'd.js') + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], [' lib/a.js:2-8 and lib/c.js:2-8: 31 tokens']) + + def test_a_copy_is_named_past_a_compact_place_in_the_same_file(self): + self.declare(JAVASCRIPT) + self.write('lib/one.js', ONE_JS) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], [' lib/one.js:2-8 and lib/one.js:14-20: 31 tokens']) + def test_a_repeated_short_pattern_is_not_a_copy_of_itself(self): # "0," repeated: every run of 10 tokens recurs 2 tokens later, but the two places overlap. self.declare({'languages': ['python'], 'min_tokens': 10, 'min_lines': 1}) @@ -261,6 +369,29 @@ def test_the_ignore_block_suppresses_an_intentional_copy(self): self.assertEqual(result.stdout, 'duplication: no duplicate block (at least 20 tokens over at least 5 lines, 2 files of python)\n') + def test_an_ignore_block_does_not_hide_a_copy_outside_it(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('tools/report.py', REPORT_BLOCK_ELSEWHERE) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], [' src/ledger.py:4-9 and tools/report.py:9-15: 29 tokens']) + + def test_the_code_around_two_ignore_blocks_does_not_join(self): + self.declare({'languages': ['python'], 'min_tokens': 10, 'min_lines': 1}) + self.write('join.py', TWO_BLOCKS) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('duplication: no duplicate block', result.stdout) + + def test_an_ignore_block_inside_a_copy_splits_it(self): + self.declare({'languages': ['python'], 'min_tokens': 12, 'min_lines': 2}) + self.write('src/ledger.py', LEDGER) + self.write('tools/split.py', SPLIT) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], [' src/ledger.py:4-6 and tools/split.py:1-3: 14 tokens']) + def test_the_markers_work_in_block_and_line_comments(self): self.declare({'languages': ['go'], 'min_tokens': 50, 'min_lines': 5}) self.write('audit/normalize.go', AUDIT_GO) @@ -318,6 +449,12 @@ def test_shell_comments_start_a_word(self): tokens, _ = texts('shell', 'echo "$#" ${#list[@]} a#b # note\n') self.assertEqual(tokens, ['echo', '"$#"', '$', '{', '#', 'list', '[', '@', ']', '}', 'a', '#', 'b']) + def test_a_marker_in_a_shell_here_document_counts(self): + tokens, problems = texts('shell', "cat <<'EOF'\n# duplication:ignore-start\nEOF\necho hidden\n" + "# duplication:ignore-end\necho shown\n") + self.assertEqual(tokens, ['cat', '<', '<', "'EOF'", '', 'echo', 'shown']) + self.assertEqual(problems, []) + def test_an_unclosed_quote_ends_at_the_line_end(self): tokens, _ = texts('go', 'a := "open\nb := 1\n') self.assertEqual(tokens, ['a', ':', '=', '"open', 'b', ':', '=', '1']) @@ -373,6 +510,69 @@ def test_ignored_files_and_links_are_not_read(self): self.assertEqual(result.returncode, 0, result.stdout + result.stderr) self.assertIn('1 file of python', result.stdout) + def test_a_tracked_directory_replaced_by_a_link_is_not_read(self): + outside = tempfile.TemporaryDirectory(prefix='duplication-outside-') + self.addCleanup(outside.cleanup) + Path(outside.name, 'report.py').write_text(REPORT) + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + self.write('inner/report.py', 'x = 1\n') + subprocess.run(['git', 'add', 'inner/report.py'], cwd=str(self.root), check=True, capture_output=True) + shutil.rmtree(str(self.root / 'inner')) + (self.root / 'inner').symlink_to(outside.name, target_is_directory=True) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('1 file of python', result.stdout) + + def test_a_file_over_the_size_limit_is_named_and_not_read(self): + big = LEDGER + '# padding\n' * 120 + self.declare(dict(PYTHON, max_file_bytes=1024)) + self.write('gen/big.py', big) + self.write('tools/report.py', REPORT) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines(), [ + 'duplication: no duplicate block (at least 20 tokens over at least 5 lines, 1 file of python; ' + '1 file not read)', + ' not read: gen/big.py: %d bytes, over max_file_bytes 1024 (a large generated file belongs in ' + 'exclude)' % len(big.encode()), + ]) + self.declare(dict(PYTHON, max_file_bytes=len(big.encode()))) + self.assertEqual(self.cli().returncode, 1, 'a file at the limit is read') + + def test_a_binary_file_is_named_and_not_read(self): + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + (self.root / 'tools').mkdir() + (self.root / 'tools' / 'blob.py').write_bytes(b'\0' + REPORT.encode()) + result = self.cli() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], + [' not read: tools/blob.py: binary (a NUL byte in its first 8192 bytes)']) + + def test_carriage_return_line_endings_count_as_one_line(self): + self.write('src/ledger.py', LEDGER) + (self.root / 'tools').mkdir() + (self.root / 'tools' / 'cr.py').write_bytes(LEDGER.replace('\n', '\r').encode()) + self.declare(PYTHON) + self.assertEqual(self.cli().returncode, 0) + self.declare(dict(PYTHON, min_lines=1)) + result = self.cli() + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertEqual(result.stdout.splitlines()[1:], [' src/ledger.py:1-13 and tools/cr.py:1-1: 47 tokens']) + + def test_running_out_of_memory_is_a_named_refusal(self): + # A mock: running out of memory cannot be produced reliably in CI, so the detector raises it. + self.declare(PYTHON) + self.write('src/ledger.py', LEDGER) + out, err = io.StringIO(), io.StringIO() + with patch.object(d, 'duplicates', side_effect=MemoryError), contextlib.redirect_stdout(out), \ + contextlib.redirect_stderr(err): + status = d.main(['--root', str(self.root)]) + self.assertEqual((status, out.getvalue()), (2, '')) + self.assertEqual(err.getvalue(), 'duplication: refused: out of memory reading the files in scope; put large ' + 'generated files in exclude or lower max_file_bytes\n') + def test_tracked_and_untracked_files_are_read(self): self.declare(PYTHON) self.write('src/ledger.py', LEDGER) @@ -403,13 +603,14 @@ def test_the_vendored_step_of_the_template_manifest_fails_the_check_list(self): GOOD = {'languages': ['go', 'javascript', 'python', 'shell', 'typescript'], 'paths': ['**', 'src/*.go'], - 'exclude': ['**/testdata/**', 'gen/?.ts'], 'min_tokens': 10, 'min_lines': 1} + 'exclude': ['**/testdata/**', 'gen/?.ts'], 'min_tokens': 10, 'min_lines': 1, 'max_file_bytes': 2048} # A change to a good declaration, the member the refusal names, and the words it says. REFUSED = [ ('version 2', lambda v: v.update(version=2), 'jpdev.json version: 1 is required (got 2)'), ('not an object', lambda v: v.update(duplication=[]), 'jpdev.json duplication: an object is required'), ('unknown member', lambda v: v['duplication'].update(threshold=50), - "jpdev.json duplication: unknown member 'threshold' (allowed: exclude, languages, min_lines, min_tokens, paths)"), + "jpdev.json duplication: unknown member 'threshold' (allowed: exclude, languages, max_file_bytes, min_lines, " + "min_tokens, paths)"), ('missing languages', lambda v: v['duplication'].pop('languages'), 'jpdev.json duplication.languages: required'), ('missing min_tokens', lambda v: v['duplication'].pop('min_tokens'), 'jpdev.json duplication.min_tokens: required'), ('missing min_lines', lambda v: v['duplication'].pop('min_lines'), 'jpdev.json duplication.min_lines: required'), @@ -431,6 +632,12 @@ def test_the_vendored_step_of_the_template_manifest_fails_the_check_list(self): 'jpdev.json duplication.min_lines: an integer from 1 to 10000 is required (got 0)'), ('min_lines oversized', lambda v: v['duplication'].update(min_lines=10001), 'jpdev.json duplication.min_lines: an integer from 1 to 10000 is required (got 10001)'), + ('min_tokens fraction', lambda v: v['duplication'].update(min_tokens=50.5), + 'jpdev.json duplication.min_tokens: an integer from 10 to 10000 is required (got 50.5)'), + ('max_file_bytes too small', lambda v: v['duplication'].update(max_file_bytes=1023), + 'jpdev.json duplication.max_file_bytes: an integer from 1024 to 104857600 is required (got 1023)'), + ('max_file_bytes oversized', lambda v: v['duplication'].update(max_file_bytes=104857601), + 'jpdev.json duplication.max_file_bytes: an integer from 1024 to 104857600 is required (got 104857601)'), ('empty paths', lambda v: v['duplication'].update(paths=[]), 'jpdev.json duplication.paths: a non-empty list of globs is required'), ('parent segment', lambda v: v['duplication'].update(paths=['../src/**']), @@ -450,6 +657,31 @@ def test_the_vendored_step_of_the_template_manifest_fails_the_check_list(self): ] +def reads(config, path): + """Whether the declaration reads a repository-relative path, by its extension and globs.""" + extensions = [e for language in config['languages'] for e in d.LANGUAGES[language][0]] + return (os.path.splitext(path)[1] in extensions and any(p.fullmatch(path) for p in config['paths']) + and not any(p.fullmatch(path) for p in config['exclude'])) + + +class ChangedRules(unittest.TestCase): + def test_every_rule_that_can_match_a_file_read_selects_the_step(self): + for name in ('templates/jpdev.json', 'jpdev.json'): + declaration = json.loads((ROOT / name).read_text()) + config = d.settings(declaration) + extensions = [e for language in config['languages'] for e in d.LANGUAGES[language][0]] + for index, rule in enumerate(declaration['changed']): + reachable = [] + for glob in rule['paths']: + pattern, base = d.glob_pattern(glob, glob), re.sub(r'\*\*|\*|\?', 'x', glob) + for sample in [base] + [base + e for e in extensions] + [base + '/x' + e for e in extensions]: + if pattern.fullmatch(sample) and reads(config, sample): + reachable.append(sample) + if reachable: + with self.subTest(declaration=name, rule=index): + self.assertIn('duplication', rule['checks'], 'the rule matches %s' % reachable[0]) + + class Configuration(Fixture): def declaration(self, change=None): value = {'version': 1, 'duplication': copy.deepcopy(GOOD)} @@ -459,8 +691,9 @@ def declaration(self, change=None): def test_a_good_declaration_is_read(self): config = d.settings(self.declaration()) - self.assertEqual((config['languages'], config['min_tokens'], config['min_lines']), - (GOOD['languages'], 10, 1)) + self.assertEqual((config['languages'], config['min_tokens'], config['min_lines'], config['max_file_bytes']), + (GOOD['languages'], 10, 1, 2048)) + self.assertEqual(d.settings({'version': 1, 'duplication': PYTHON})['max_file_bytes'], 1048576) def test_each_invalid_declaration_is_refused_with_its_member_named(self): for name, change, message in REFUSED: @@ -505,9 +738,11 @@ def test_the_schema_and_the_command_name_the_same_members_and_bounds(self): self.assertEqual(member['required'], ['languages', 'min_tokens', 'min_lines']) self.assertFalse(member['additionalProperties']) self.assertEqual(member['properties']['languages']['items']['enum'], sorted(d.LANGUAGES)) - for name, bounds in (('min_tokens', d.MIN_TOKENS), ('min_lines', d.MIN_LINES)): + for name, bounds in (('min_tokens', d.MIN_TOKENS), ('min_lines', d.MIN_LINES), + ('max_file_bytes', d.MAX_FILE_BYTES)): limits = member['properties'][name] - self.assertEqual((limits['type'], limits['minimum'], limits['maximum']), ('integer',) + bounds) + self.assertEqual((limits['type'], limits['multipleOf'], limits['minimum'], limits['maximum']), + ('integer', 1) + bounds) def test_the_schema_and_the_command_agree_on_every_vector(self): # The standard-library reader of tests/changed_checks_test.py always runs, so the test never