diff --git a/CHANGELOG.md b/CHANGELOG.md index 647fd01..0855828 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project are documented here. Versions follow ## [Unreleased] +### Added + +- **Skip variable renames with `--skip-var-renames`.** Hides the changes that + are only a variable name swap, so a regenerate full of renames shows you what + else moved. A quick check, not a review: a rewired signal has the same shape + and is hidden too, and the report says when the flag was used. + +### Changed + +- Simplify the report layout. + ## [1.13.0] — 2026-09-06 ### Added diff --git a/README.md b/README.md index 61c54b8..8cdcd6e 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,11 @@ Automatically identifies common code-generation churn: Changes that cannot be safely explained remain visible as real changes. +One opt-in exception: `--skip-var-renames` folds variable renames it cannot +prove are noise, so a fresh regenerate can be swept for what is *not* a rename. +It trades away real changes that look the same, announces itself everywhere it +was used, and is never on by default. + See [What Counts as Noise](docs/usage.md#what-counts-as-noise). --- diff --git a/compare_tool/c_rules.py b/compare_tool/c_rules.py index eed337a..e1abf42 100644 --- a/compare_tool/c_rules.py +++ b/compare_tool/c_rules.py @@ -436,6 +436,152 @@ def autogen_noise_map(old_lines, new_lines, old_ids=None, new_ids=None): return mapping +# --- assumed variable renames (--skip-var-renames quick check) --- +# +# Every other rule in this module proves its case before folding a difference +# away. This one does not, and says so: `a = b;` becoming `x = y;` is equally +# consistent with "two signals were renamed" and with "the block was rewired", +# and nothing in the file says which. It exists for the one job where that +# trade pays -- sweeping a regenerate for what is NOT a rename -- so it is +# opt-in, off by default, and everything it folds is labelled `assumed-rename` +# rather than `rename` on every surface. +# +# The shape is kept narrow: a whole hunk of bindings, paired 1-1, differing +# only by names, with the declared type unchanged. A binding names objects and +# copies one into another; it never computes. Anything with an operator, a +# call, a cast or a changed literal is not a binding and stays a real change. + + +def _path_len(toks): + """Length of the plain lvalue path starting at ``toks[0]``, else 0. + + A path is an identifier followed by any number of ``.field``, ``->field`` + and ``[index]`` steps, the index itself a name or an integer. That is the + shape Embedded Coder writes its ports and state through -- ``rtU.Pedal``, + ``rtDW->Filter_DSTATE``, ``buf[2]`` -- and it still only *names* one object: + no operator, no call, so nothing is computed on the way. + """ + if not toks or not is_identifier(toks[0]): + return 0 + i = 1 + while i < len(toks): + if toks[i] == '.' and i + 1 < len(toks) and is_identifier(toks[i + 1]): + i += 2 + elif (toks[i] == '-' and i + 2 < len(toks) and toks[i + 1] == '>' + and is_identifier(toks[i + 2])): + i += 3 # '->' tokenizes as two single characters + elif (toks[i] == '[' and i + 2 < len(toks) and toks[i + 2] == ']' + and (is_identifier(toks[i + 1]) or toks[i + 1].isdigit())): + i += 3 + else: + break + return i + + +# Keywords that may precede a declared name. Deliberately a whitelist rather +# than "any keyword": `return a;` is `return` followed by a name and would +# otherwise parse as a declaration of `a`, and so would `else x;`. +DECL_KEYWORDS = frozenset(""" + auto char const double enum extern float inline int long register restrict + short signed static struct union unsigned void volatile _Bool _Complex +""".split()) + + +def _is_decl_token(tok): + """True for a token that may sit in front of a declared name: a type name, + a qualifier (``static``, ``const``, ``unsigned``) or a pointer star.""" + return tok == '*' or is_identifier(tok) or tok in DECL_KEYWORDS + + +def binding_parts(line): + """``(declaration_tokens, target_tokens, value_tokens)`` for a binding, or + ``None`` for anything else. + + A binding is one statement that names an object and, optionally, copies a + single named object or literal into it: + + - ``a = b;`` / ``rtY.Out = rtU.Pedal;`` / ``buf[2] = rtDW->State;`` + - ``real_T x;`` -- a bare declaration, which carries a name and a type and + no value at all + - ``boolean_T flag = FALSE;`` / ``sint32 n = -1;`` + + Not a binding: an expression, a call, a cast, two statements on a line, a + multi-declarator list, a comparison. Those carry meaning a name swap cannot + account for. + """ + toks = tokenize(line.strip()) + if len(toks) < 2 or toks[-1] != ';': + return None + body = toks[:-1] + if body.count('=') > 1: + return None # '==' tokenizes as two '=', so a comparison is out + if '=' in body: + cut = body.index('=') + lhs, rhs = body[:cut], body[cut + 1:] + else: + lhs, rhs = body, [] + + if lhs and _path_len(lhs) == len(lhs): + decl, target = (), tuple(lhs) # a store into something declared already + elif lhs and is_identifier(lhs[-1]) and all(_is_decl_token(t) for t in lhs[:-1]): + # a declaration introduces a BARE name; `real_T a.b;` is not C, so a + # path target always means the object was declared somewhere else + decl, target = tuple(lhs[:-1]), (lhs[-1],) + else: + return None + + if not rhs: + # no value: only a declaration can say nothing. A bare `a;` is a + # statement whose point is elsewhere (a macro, a volatile read). + return (decl, target, ()) if decl else None + v = rhs[1:] if rhs[0] in ('-', '+') else rhs + if not v: + return None + if _path_len(v) != len(v) and len(v) != 1: + return None # one named object, or one literal -- never an expression + return decl, target, tuple(rhs) + + +def _is_assumed_rename_pair(a, b): + """A name swap the quick check is willing to take for a rename. + + ALL_CAPS is refused on both sides: C reserves it for macros and enum + constants, so ``mode = IDLE;`` becoming ``mode = DRIVE;`` is a value change + wearing the shape of a rename, and ``flag = FALSE;`` -> ``flag = TRUE;`` is + the same thing on a declaration. A changed numeric literal never reaches + here at all -- it is not an identifier, so the line pair is rejected. + """ + if a == b or not (is_identifier(a) and is_identifier(b)): + return False + return not (ALL_CAPS_RE.match(a) or ALL_CAPS_RE.match(b)) + + +def assumed_rename_map(old_lines, new_lines): + """Name map for a hunk that is nothing but bindings differing by names, or + None. + + Unlike :func:`autogen_noise_map` the names do not have to look generated -- + that is what makes this the unproven quick check. It still refuses a hunk + whose sides do not pair 1-1, one that holds a line which is not a binding, + one whose declared type or literal changed, and one whose map contains a + cycle (an ``a`` <-> ``b`` swap is a real change, never a rename). + """ + if not old_lines or len(old_lines) != len(new_lines): + return None + mapping = {} + for la, lb in zip(old_lines, new_lines): + pa, pb = binding_parts(la), binding_parts(lb) + if pa is None or pb is None: + return None + if pa[0] != pb[0]: + return None # the declared TYPE changed -- sint32 -> uint8 truncates + if not _collect_line_pair_renames(la, lb, mapping, _is_assumed_rename_pair): + return None + if not mapping or _map_has_cycle(mapping): + return None + return mapping + + # --- straight-line reorder (Embedded Coder reschedules independent stmts) --- # # Regenerating a model routinely emits the same independent assignments in a diff --git a/compare_tool/diff_engine.py b/compare_tool/diff_engine.py index 749c672..2d88c4e 100644 --- a/compare_tool/diff_engine.py +++ b/compare_tool/diff_engine.py @@ -6,8 +6,12 @@ by testing single normalization rules one at a time. Hunk dict: {kind, old_range: [i1, i2), new_range: [j1, j2)} (0-based lines) -kind in {real, moved, comment, rename, reorder, uuid, timestamp, sw-version, - description, whitespace, mixed} +kind in {real, moved, comment, rename, assumed-rename, reorder, uuid, + timestamp, sw-version, description, whitespace, mixed} + +'assumed-rename' only ever appears when the caller asked for it +(``skip_var_renames``): it is the one label this engine applies WITHOUT +proving the difference is noise. See c_rules.assumed_rename_map. Moved blocks: a pure-delete hunk whose non-blank shadow content reappears verbatim as exactly one pure-insert hunk (and vice versa) is labeled 'moved' @@ -235,6 +239,16 @@ def _is_autogen_hunk(h, old_sh_lines, new_sh_lines, old_ids, new_ids): return c_rules.autogen_noise_map(a, b, old_ids, new_ids) is not None +def _is_assumed_rename_hunk(h, old_sh_lines, new_sh_lines): + """True when a shadow hunk is only bindings whose lines differ by variable + names -- the ``--skip-var-renames`` quick check. Unproven by + design (see :func:`compare_tool.c_rules.assumed_rename_map`); reached only + when the caller opted in.""" + i1, i2, j1, j2 = h + return c_rules.assumed_rename_map(_nonblank(old_sh_lines[i1:i2]), + _nonblank(new_sh_lines[j1:j2])) is not None + + def _slices_equal(h, old_variant_lines, new_variant_lines): """Compare a hunk's line slices under some normalization variant, ignoring blank lines (handles pure insert/delete of comment lines).""" @@ -305,7 +319,8 @@ def _build_variants(old_text, new_text, ruleset, rename_map, ext='', user_rules= return variants -def compare_pair(old_text, new_text, path, user_rules=()): +def compare_pair(old_text, new_text, path, user_rules=(), + skip_var_renames=False): """Compare two file contents. Returns dict: {status, hunks, renames, notes} status in {identical, comment-only, ignorable-only, real-change} @@ -313,7 +328,13 @@ def compare_pair(old_text, new_text, path, user_rules=()): ``user_rules`` are extra noise patterns from a ``--rules`` file (:mod:`compare_tool.userrules`). They run ON TOP of the built-in Embedded Coder rules -- an empty tuple, the default, leaves every existing verdict - exactly as it was.""" + exactly as it was. + + ``skip_var_renames`` is the opt-in quick check (``--skip-var-renames``): a + C/C++ hunk that is nothing but bindings differing by variable names is + labelled ``assumed-rename`` instead of ``real``, WITHOUT proof that the + swap preserves behaviour. It can therefore hide a rewiring, which is why it + is off by default and why every hunk it folds keeps a label of its own.""" result = {'status': 'identical', 'hunks': [], 'renames': {}, 'notes': []} if old_text == new_text: return result @@ -403,6 +424,21 @@ def compare_pair(old_text, new_text, path, user_rules=()): kept.append(h) candidates = kept + # Opt-in quick check (--skip-var-renames): hunks that are only bindings + # differing by variable names. NOT proven noise -- a rewiring has + # the same shape -- so it runs last among the name rules, after the proven + # ones have taken what they can explain, and what it takes keeps its own + # 'assumed-rename' label everywhere it is shown or counted. + assumed_rename_hunks = [] + if skip_var_renames and ruleset in ('c', 'cpp') and candidates: + kept = [] + for h in candidates: + if _is_assumed_rename_hunk(h, final_old_shadow_lines, new_shadow_lines): + assumed_rename_hunks.append(h) + else: + kept.append(h) + candidates = kept + # MATLAB codegen reschedules independent statements (output assignments, # temporaries): the raw and shadow text both read as a change, but the block # computes the same values. When the whole surviving change set is one @@ -454,6 +490,9 @@ def compare_pair(old_text, new_text, path, user_rules=()): kind = 'rename' # autogen-name swap (rtb_/mangle/temp) if kind is None and reorder_hunks and _overlaps(h, reorder_hunks): kind = 'reorder' # independent statements rescheduled + if (kind is None and assumed_rename_hunks + and _overlaps(h, assumed_rename_hunks)): + kind = 'assumed-rename' # --skip-var-renames: assumed, not proven if kind is None: kind = 'mixed' # ignorable but caused by >1 rule combined for name, ov, nv in variants: diff --git a/compare_tool/main.py b/compare_tool/main.py index 3877e09..e9da0d9 100644 --- a/compare_tool/main.py +++ b/compare_tool/main.py @@ -55,7 +55,8 @@ def default_report_name(arxml_only): def run_compare(old_root, new_root, out, arxml_only=False, exclude=(), progress=None, reviews=None, theme_name=theme.DEFAULT, - old_label=None, new_label=None, max_diff_lines=0, user_rules=()): + old_label=None, new_label=None, max_diff_lines=0, user_rules=(), + skip_var_renames=False): """Scan two trees and write the HTML report. ``old_label`` / ``new_label`` name the two sides in the report header when @@ -78,7 +79,8 @@ def run_compare(old_root, new_root, out, arxml_only=False, exclude=(), include = tuple('*' + ext for ext, rs in RULES.items() if rs in ('arxml', 'a2l')) if arxml_only else () results = scan(old_root, new_root, progress=progress, exclude=exclude, - include=include, user_rules=user_rules) + include=include, user_rules=user_rules, + skip_var_renames=skip_var_renames) counts = summarize(results) if arxml_only: # ALWAYS written: "no changes" must be an explicit statement, never @@ -104,6 +106,14 @@ def summary_lines(results, counts): lines.append('Summary: {real-change} modified, {comment-only} comment-only, ' '{ignorable-only} unimportant, {added} added, {deleted} deleted, ' '{identical} identical, {error} error(s)'.format(**counts)) + n_assumed = sum(1 for r in results.values() + if any(h['kind'] == 'assumed-rename' for h in r['hunks'])) + if n_assumed: + lines.append('!! QUICK CHECK (--skip-var-renames): variable renames were ' + 'SKIPPED in {} file(s) without proof that they are noise. A ' + 'rewiring has the same shape, so a real change can be ' + 'missing from the counts above -- re-run without the flag ' + 'before signing anything off.'.format(n_assumed)) if counts['error']: lines.append('!! COMPARE INCOMPLETE: {} path(s) could NOT be compared -- ' 'treat them as potentially changed:'.format(counts['error'])) @@ -247,6 +257,21 @@ def _parser(): 'file\'s line count, is skipped with a warning -- a ' 'filter can never hide a real change. Applies to both ' 'the report and the viewer') + ap.add_argument('--skip-var-renames', action='store_true', + help='QUICK CHECK, UNSAFE: fold away any C/C++ hunk that is ' + 'only bindings differing by variable names -- an ' + 'assignment or a declaration that names one object and ' + 'at most copies one into it (a = b, rtY.Out = rtU.In, ' + 'real_T x, boolean_T a = FALSE) -- WITHOUT proving ' + 'the swap is noise. Built for one job -- sweeping a ' + 'regenerate for what is NOT a rename -- and it pays ' + 'for that with false negatives: a rewiring ' + '(a = speed becoming a = torque) has exactly the same ' + 'shape and is folded too. Changed literals, ALL_CAPS ' + 'macro/enum constants and variable swaps still count ' + 'as real. Everything folded is labelled Assumed rename ' + 'and the report and the terminal both carry a warning; ' + 'do not sign a review off on a run that used this') ap.add_argument('--json', metavar='OUT.json', default=None, help='also write the full scan as schema-versioned JSON for ' 'a pipeline to read -- per-file verdict, hunks, renames, ' @@ -362,7 +387,8 @@ def _run(ap, args, zip_temp): try: return run_viewer(old_dir, new_dir, exclude=args.exclude, arxml_only=args.arxml_only, theme_name=args.theme, - user_rules=user_rules) + user_rules=user_rules, + skip_var_renames=args.skip_var_renames) except ImportError as e: # a stdlib-only install (the .pyz, a locked-down box) has no Qt. # Say so plainly instead of dumping a traceback. @@ -396,6 +422,9 @@ def _run(ap, args, zip_temp): if args.arxml_only: print('note: --review has no effect with --arxml-only (that report ' 'lists files, not individual changes)', file=sys.stderr) + if args.skip_var_renames and args.arxml_only: + print('note: --skip-var-renames has no effect with --arxml-only (it ' + 'only ever folds C/C++ bindings)', file=sys.stderr) out = Path(args.report) print('Scanning...') @@ -411,7 +440,8 @@ def progress(done, total, rel): old_label=args.baseline_name or old_zip, new_label=args.current_name or new_zip, max_diff_lines=args.max_diff_lines, - user_rules=user_rules) + user_rules=user_rules, + skip_var_renames=args.skip_var_renames) except ReportWriteError as e: # what WAS scanned still goes to the terminal -- the compare itself may # have been fine, it is only the record that is missing diff --git a/compare_tool/qtviewer/app.py b/compare_tool/qtviewer/app.py index fffa03f..bb044f3 100644 --- a/compare_tool/qtviewer/app.py +++ b/compare_tool/qtviewer/app.py @@ -93,7 +93,7 @@ def focusOutEvent(self, event): class MainWindow(QMainWindow): def __init__(self, old=None, new=None, exclude=(), arxml_only=False, - theme_name=theme.DEFAULT, user_rules=()): + theme_name=theme.DEFAULT, user_rules=(), skip_var_renames=False): super().__init__() self._theme = theme.set_current(theme_name) self._state = ('idle', 'Ready') @@ -105,6 +105,10 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False, # extra --rules noise patterns; the viewer and the CLI share the engine, # so both must apply them or the same folders would read two ways self.user_rules = tuple(user_rules) + # --skip-var-renames: the unproven quick check. Baked into the scan + # like --rules is, because it decides verdicts -- it is not a view + # toggle the reviewer can flip back on over the same results. + self.skip_var_renames = bool(skip_var_renames) self._raw_results = {} # verdicts straight from the scan self.results = {} # ... after the current compare rules self.worker = None @@ -981,14 +985,18 @@ def _start_scan(self): # enough for a pair of them, and both roots are already named over the # diff panes -- with the full path in their tooltip name = Path(self.new).name or str(self.new) - self.setWindowTitle('AUTOSAR CodeGen Compare — {}'.format(name)) + # the quick check rides in the title because it has no undo inside this + # window: it changed what the scan judged, so the reviewer has to see + # which mode produced the tree they are reading + quick = ' — QUICK CHECK: variable renames skipped' if self.skip_var_renames else '' + self.setWindowTitle('AUTOSAR CodeGen Compare — {}{}'.format(name, quick)) self._load_reviews() self.counts_label.setText('') self._set_state('busy', 'Scanning…') # the scan itself is rule-free; the rules are applied to its results, # so flipping a category never costs a second walk of the disk self.worker = ScanWorker(self.old, self.new, self.exclude, self.include, - self.user_rules) + self.user_rules, self.skip_var_renames) self.worker.progressed.connect(self._on_progress) self.worker.done.connect(self._on_done) self.worker.failed.connect(self._on_fail) @@ -1636,7 +1644,7 @@ def _taskbar_identity(): def run_viewer(old=None, new=None, exclude=(), arxml_only=False, - theme_name=theme.DEFAULT, user_rules=()): + theme_name=theme.DEFAULT, user_rules=(), skip_var_renames=False): app = QApplication.instance() owns = app is None if owns: @@ -1646,7 +1654,8 @@ def run_viewer(old=None, new=None, exclude=(), arxml_only=False, apply_theme(app) app.setApplicationName('CodeGen Compare') app.setWindowIcon(app_icon()) - win = MainWindow(old, new, exclude, arxml_only, theme_name, user_rules) + win = MainWindow(old, new, exclude, arxml_only, theme_name, user_rules, + skip_var_renames) win.show() return app.exec() if owns else 0 diff --git a/compare_tool/qtviewer/dialogs.py b/compare_tool/qtviewer/dialogs.py index c202ed7..a7893ac 100644 --- a/compare_tool/qtviewer/dialogs.py +++ b/compare_tool/qtviewer/dialogs.py @@ -74,6 +74,11 @@ the tree, and still appears as Comment in an exported report - Tick back on to bring the colour back - Real changes can never be quietened +- If the title bar says **QUICK CHECK: variable renames skipped**, this window + was started with `--skip-var-renames`: that scan folded variable renames it + could not prove were noise, so a rewired signal can be sitting under + Unimportant. It is a sweep, not a review -- re-run without the flag before + signing anything off ## 5. Read the diff diff --git a/compare_tool/qtviewer/worker.py b/compare_tool/qtviewer/worker.py index d7a5d14..6bb696b 100644 --- a/compare_tool/qtviewer/worker.py +++ b/compare_tool/qtviewer/worker.py @@ -13,13 +13,15 @@ class ScanWorker(QThread): done = Signal(dict) # results failed = Signal(str) # loud failure -> red banner - def __init__(self, old, new, exclude=(), include=(), user_rules=()): + def __init__(self, old, new, exclude=(), include=(), user_rules=(), + skip_var_renames=False): super().__init__() self.old = old self.new = new self.exclude = tuple(exclude) self.include = tuple(include) self.user_rules = tuple(user_rules) + self.skip_var_renames = bool(skip_var_renames) def run(self): try: @@ -29,7 +31,8 @@ def run(self): # like the built-in rules do, so they are baked into the scan here. results = scan(self.old, self.new, progress=self._progress, exclude=self.exclude, include=self.include, - user_rules=self.user_rules) + user_rules=self.user_rules, + skip_var_renames=self.skip_var_renames) self.done.emit(results) except Exception as e: # scan is internally fail-safe, but a crash here must still be diff --git a/compare_tool/report.py b/compare_tool/report.py index 7719423..2536500 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -52,6 +52,7 @@ .errbox div { padding: 1px 0; } .errbox code { background: var(--err-code-bg); color: var(--err-fg); } .hint { color: var(--fg-faint); font-size: 11px; margin: -14px 0 18px; } +.qcnote { color: var(--fg-faint); font-size: 11px; margin: 26px 0 6px; } body.hide-real .sec-real, body.hide-ign .sec-ign, body.hide-add .sec-add, body.hide-del .sec-del { display: none; } ul.files { margin: 4px 0 14px; padding-left: 22px; font-size: 13px; } @@ -1152,26 +1153,14 @@ def _content_table(lines, cls, language=None): # Anything not listed here still shows, with its first letter raised: a kind # added to a rules module may look plain, it may never go missing. _KIND_LABEL = {'uuid': 'UUID', 'sw-version': 'SW version', - 'line-endings': 'Line endings'} + 'line-endings': 'Line endings', + 'assumed-rename': 'Assumed rename'} def _kind_label(kind): return _KIND_LABEL.get(kind, kind[:1].upper() + kind[1:]) -def _kinds_of(r): - """Short ignorable-kind summary for a file, e.g. 'Comment, Rename ×3'.""" - kinds = {h['kind'] for h in r['hunks'] if h['kind'] != 'real'} | set(r['notes']) - labels = set() - # only the counted spelling replaces the plain one -- a rename hunk with no - # pair recorded still has to say 'Rename' - if r['renames']: - kinds.discard('rename') - labels.add('Rename ×{}'.format(len(r['renames']))) - labels |= {_kind_label(k) for k in kinds} - return ', '.join(sorted(labels)) - - def _autosar_section(results, anchors): """Top-of-report rollup of every AUTOSAR-level change across all files: port-interfaces, software components, ports, runnables, events, RTE @@ -1296,20 +1285,6 @@ def _notes(r): return _iface_note(r) + _swc_note(r) + _rte_note(r) + _a2l_note(r) -def _affected_extra(names, limit=3): - """The functions/blocks a file's real changes land in, for its header -- - the seed of an Impact Analysis "Affected Module" column. Capped so a - heavily churned file does not spill its whole symbol table into the - summary line; the diff below still names every one.""" - if not names: - return '' - shown = [_esc(n) for n in names[:limit]] - text = 'Affected: ' + ', '.join(shown) - if len(names) > limit: - text += ' (+{})'.format(len(names) - limit) - return text - - def _file_open(anchor, rel, status, extra='', expanded=False, reviewed=False): label, tag = _LABEL[status] sec = _TREE[status][2] @@ -1342,6 +1317,22 @@ def _error_banner(results): 'does not cover them.'.format(len(errs), ''.join(lines))) +def _quick_check_note(results): + """One line recording that this run ran with ``--skip-var-renames``. + + Sits between the folder tree and the detailed changes: the mode is the + reader's own choice, not a failure, so it reads as a caption rather than a + banner -- but it is never absent, and it is the last thing read before the + diffs, because what it folded is not provably noise and never appears in + them. Empty string when the mode folded nothing.""" + n = sum(1 for r in results.values() + if any(h['kind'] == 'assumed-rename' for h in r.get('hunks', []))) + if not n: + return '' + return ('
⚠ Variable renames ignored in {} file(s) ' + '(--skip-var-renames).
'.format(n)) + + def _file_section(rel, results, old_root, new_root, anchors, rv, max_rows=0): """One collapsible detail section for a non-identical file. @@ -1369,12 +1360,13 @@ def _file_section(rel, results, old_root, new_root, anchors, rv, max_rows=0): notes = rv.annotate(rel, r, old_lines, new_lines) lang = syntax.language_for(rel) labels = None - extra = '' if not r['binary']: labels = (funcname.enclosing(old_lines, lang), funcname.enclosing(new_lines, lang)) - extra = _affected_extra(funcname.affected(labels[0], labels[1], hunks)) - parts.append(_file_open(anchors[rel], rel, 'real-change', extra, + # only the verdict badge: the header answers "must I read this file", + # and which symbols / which noise kinds are in it is what the diff + # below is for + parts.append(_file_open(anchors[rel], rel, 'real-change', expanded=True, reviewed=rel in rv.files)) parts.append(_notes(r)) if r['binary']: @@ -1386,7 +1378,7 @@ def _file_section(rel, results, old_root, new_root, anchors, rv, max_rows=0): parts.append(_groups_html(old_lines, new_lines, hunks, notes, lang, labels, max_rows)) elif status in ('comment-only', 'ignorable-only'): - parts.append(_file_open(anchors[rel], rel, status, _esc(_kinds_of(r)))) + parts.append(_file_open(anchors[rel], rel, status)) if not r['hunks']: parts.append('
Line endings / BOM only; ' 'no content difference.
') @@ -1702,6 +1694,7 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, parts.append('

No real changes. All differences are ignorable ' '(comments / renames / UUIDs / timestamps / whitespace).

') + parts.append(_quick_check_note(results)) if detail_files: parts.append('

Detailed changes

') parts.append('
' diff --git a/compare_tool/scanner.py b/compare_tool/scanner.py index feaadca..c67a869 100644 --- a/compare_tool/scanner.py +++ b/compare_tool/scanner.py @@ -107,7 +107,8 @@ def on_error(err): return out -def compare_file(old_root, new_root, rel, user_rules=()): +def compare_file(old_root, new_root, rel, user_rules=(), + skip_var_renames=False): """Full comparison result for one relative path present in both trees.""" old_p = Path(old_root) / rel new_p = Path(new_root) / rel @@ -121,7 +122,7 @@ def compare_file(old_root, new_root, rel, user_rules=()): # bytes differed but normalized text equal: EOL style or BOM only return {'status': 'ignorable-only', 'hunks': [], 'renames': {}, 'notes': ['line-endings'], 'binary': False} - result = compare_pair(old_text, new_text, rel, user_rules) + result = compare_pair(old_text, new_text, rel, user_rules, skip_var_renames) result['binary'] = False # semantic summaries: only real changes can move the AUTOSAR surface # (ignorable-only means the shadows are equal, hence same content) @@ -242,7 +243,8 @@ def _candidate(root, rel): return filepair.Candidate(rel, ext, digest, lines) -def _link_moves(results, old_root, new_root, user_rules=()): +def _link_moves(results, old_root, new_root, user_rules=(), + skip_var_renames=False): """Cross-reference added files with the deleted ones they came from. The two entries KEEP their `added` / `deleted` verdicts and their place in @@ -259,7 +261,8 @@ def _link_moves(results, old_root, new_root, user_rules=()): for a_rel, (d_rel, sim) in filepair.find_moves(added, deleted).items(): try: pair = compare_pair(read_text(Path(old_root) / d_rel), - read_text(Path(new_root) / a_rel), a_rel, user_rules) + read_text(Path(new_root) / a_rel), a_rel, user_rules, + skip_var_renames) except (OSError, UnicodeError): continue results[a_rel]['moved_from'] = d_rel @@ -273,7 +276,7 @@ def _link_moves(results, old_root, new_root, user_rules=()): def scan(old_root, new_root, progress=None, exclude=(), include=(), fold=(), - user_rules=()): + user_rules=(), skip_var_renames=False): """Compare two trees. Returns {rel_path: result} sorted by path. result: {status, hunks, renames, notes, binary[, ifaces]}. status 'error' = the path could not be listed or compared (see notes). @@ -283,7 +286,10 @@ def scan(old_root, new_root, progress=None, exclude=(), include=(), fold=(), fold: noise statuses that should not be reported separately -- those files come back as 'identical' (see fold_status). user_rules: extra noise patterns from a --rules file, applied on top of the - built-in ones (see compare_tool.userrules).""" + built-in ones (see compare_tool.userrules). + skip_var_renames: the opt-in --skip-var-renames quick check -- C/C++ hunks + that are only bindings differing by variable names become 'assumed-rename' + instead of real, WITHOUT proof (see compare_tool.diff_engine).""" fold = tuple(fold) old_errors, new_errors = [], [] old_files = list_files(old_root, old_errors) @@ -311,7 +317,8 @@ def under_failed(rel, errs): for idx, rel in enumerate(all_paths): try: if rel in old_files and rel in new_files: - results[rel] = compare_file(old_root, new_root, rel, user_rules) + results[rel] = compare_file(old_root, new_root, rel, user_rules, + skip_var_renames) elif rel in new_files: if under_failed(rel, old_errors): results[rel] = _error_result( @@ -341,7 +348,7 @@ def under_failed(rel, errs): # after every verdict is settled: folding cannot reach 'added'/'deleted', # so the candidate set is the same either way, and pairing must never be # what decides a verdict - _link_moves(results, old_root, new_root, user_rules) + _link_moves(results, old_root, new_root, user_rules, skip_var_renames) return results diff --git a/compare_tool/serialize.py b/compare_tool/serialize.py index f756938..723b68a 100644 --- a/compare_tool/serialize.py +++ b/compare_tool/serialize.py @@ -98,6 +98,13 @@ def build(results, counts, old_root, new_root, exit_code, 'files': [_file_entry(rel, results[rel]) for rel in sorted(results)], 'consistency': [{'model': m, 'message': msg} for m, msg in advisories], } + # a run made with --skip-var-renames folded differences it could not prove + # were noise, so its summary and exit code are weaker than they look. A + # consumer gating on this file has to be able to see that without walking + # every hunk. + if any(h['kind'] == 'assumed-rename' + for r in results.values() for h in (r.get('hunks') or [])): + doc['quick_check'] = 'skip-var-renames' if old_label: doc['baseline_label'] = old_label if new_label: diff --git a/docs/architecture.md b/docs/architecture.md index 2256e73..02192f2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -114,6 +114,11 @@ decides what you see, and three peeling steps (rename, autogen-name, reorder) each remove only what they can justify. ARXML and A2L take the same two passes; only the *shadow* — what each strips — differs. +There is one opt-in exception to "prove it is noise", drawn dashed below: +`--skip-var-renames` peels hunks that are only variable-name swaps in bindings, +without a proof that the swap preserves behaviour. It is off unless asked for, +and what it peels is labelled `assumed-rename`, never `rename`. + ```mermaid flowchart TD START["scanner pairs a file by path"]:::io --> DISP{"present on…"} @@ -130,6 +135,8 @@ flowchart TD SHA --> P2["PASS 2 · diff the shadows
patience matcher (linediff.hunks)
→ candidate real hunks"]:::pass P2 --> F1["peel · 1-to-1 rename map
verified by re-diff"]:::filter F1 --> F2["peel · autogen-name noise
rtb_ · _DSTATE · tmp_N swaps"]:::filter + F2 -. "--skip-var-renames only" .-> FQ["peel · assumed rename
bindings differing by name
NOT proven"]:::filter + FQ -.-> F3 F2 --> F3["peel · safe reorder
dependence-preserving permutation"]:::filter F3 --> REM{"candidate hunks
still left?"} @@ -276,8 +283,19 @@ consumes one dict per compared path: ``` Ranges are 0-based, end-exclusive, into the **raw** lines of each side. -`kind` is one of `real`, `moved`, `comment`, `rename`, `reorder`, `uuid`, -`timestamp`, `sw-version`, `description`, `whitespace`, `mixed`. +`kind` is one of `real`, `moved`, `comment`, `rename`, `assumed-rename`, +`reorder`, `uuid`, `timestamp`, `sw-version`, `description`, `whitespace`, +`mixed`. + +`assumed-rename` is the only kind applied **without** proof, and it appears +only when the caller passed `skip_var_renames` (`--skip-var-renames`). It folds +a hunk whose every line is a binding — a statement that names one object and +at most copies another into it (`a = b;`, `rtY.Out = rtU.Pedal;`, `real_T x;`, +`boolean_T f = FALSE;`) — differing only by identifier names, which a rewiring +also looks like. It is +therefore a deliberate false-negative mode: it keeps its own kind so no surface +can spell it `rename`, and the report, the terminal summary, the JSON and the +viewer title each say the run used it. `reorder` is the one ignorable kind decided on *meaning* rather than spelling: when the whole surviving change set is a dependence-preserving permutation of a diff --git a/docs/usage.md b/docs/usage.md index 3bfb489..2384f2b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -34,6 +34,7 @@ Either side can be a `.zip` instead of a folder — an Azure DevOps build artifa | `--current-name NAME` | Same for the CURRENT side. Either flag only changes the header text — the folder path stays in the tooltip, so a compare is still traceable to where the files were read from | | `--theme dark\|light` | Colour scheme the report and the viewer open with (default `dark`). The report carries **both** and has its own switch, so this only sets what the reader sees first | | `--rules RULES.json` | Extra noise patterns applied on top of the built-in rules — see [Custom noise rules](#custom-noise-rules) | +| `--skip-var-renames` | **Quick check, unsafe.** Fold away C/C++ hunks that are only variable renames, without proving they are noise — see [Quick check: skipping variable renames](#quick-check-skipping-variable-renames) | | `--max-diff-lines N` | Cap the diff embedded per file at about N lines (0 = no cap, the default). Stops a whole-tree regenerate from rendering one file into a report so large the browser hangs; a capped file keeps its verdict and the exit code, and the cut is loud | | `--qt`, `--viewer` | Open the side-by-side viewer on folders named on the command line, instead of comparing them in the terminal. Needs the `viewer` extra | | `--version` | Print the version and exit | @@ -128,6 +129,7 @@ Turning on `Review mode` adds a note box and a `Review` column to the tree — g | `timestamp` | `` blocks, `` | .arxml .xml | | `sw-version` | `` version stamps (bumped on every regenerate). Anchored, so `` and the like are untouched | .arxml .xml | | `description` | ``, ``, `` — the prose an Identifiable carries (schema 4.2 and 4.4 alike). `` and `` are **not** included: the first is semantic, the second can carry tool payload | .arxml .xml | +| `assumed-rename` | Assignments and declarations differing only by variable names, folded **without proof** — only with `--skip-var-renames`, never by default | .c .h .cpp .hpp | | `whitespace` | Indentation, trailing spaces, blank lines | all | | `line-endings` | CRLF vs LF, BOM | all | @@ -149,6 +151,44 @@ Regenerating a model routinely emits the same independent assignments — output Two straight-line schedules that agree on the order of every dependent pair are guaranteed to compute the same result, so folding the reorder is behaviour-preserving, not a guess. If any of those three conditions fails — a call sneaks in between the lines, a right-hand side actually changed, a dependent pair got flipped — the whole block stays a real change. The rule errs toward calling a block real rather than toward hiding one; when in doubt, it shows you the diff. +### Quick check: skipping variable renames + +Every rule above proves its case before folding anything away. `--skip-var-renames` does not, and it is the only part of the tool that works this way — so it is off by default and has to be asked for by name. + +It is for one job: sweeping a fresh regenerate for what is **not** a rename, when you already expect a wave of renaming and want to see the rest. It is not for signing a review off. + +With the flag on, a C/C++ hunk is folded as `assumed-rename` when every line on both sides is a **binding** — one statement that names an object and at most copies a single other object or literal into it — and the two sides pair up line for line, differing only by identifier names, with the declared type unchanged. + +A binding never *computes*. Either side of the `=` may be a plain lvalue path — a name, a `.field`, a `->field`, a `[index]` — which is how Embedded Coder reaches its ports, DWork state and buffers: + +```c +a = b; → x = b; +acc_cmd = rtU.Pedal; → drv_cmd = rtU.Pedal; +rtY.Out = filt_in; → rtY.Out = filt_val; +buf[2] = rtDW->State; → buf[2] = rtDW->Level; +real_T filt_in; → real_T filt_val; +boolean_T flag = FALSE; → boolean_T other = FALSE; +``` + +**What it costs you.** A rewiring has exactly the same shape as a rename, so it is folded too. `a = speed;` becoming `a = torque;` is a genuine signal change, and this mode will not report it. That is the trade the flag exists to make; a run that used it can be missing a real change. + +**What still counts as real, even here:** + +- a changed literal — `a = 0;` → `x = 1;`; +- a changed type — `sint32 a = 0;` → `uint8 x = 0;`; +- an ALL_CAPS macro or enum constant on either side — `flag = FALSE;` → `flag = TRUE;`, `mode = IDLE;` → `mode = DRIVE;`. C reserves all-caps for constants declared elsewhere, so swapping one is a value change, not a rename; +- a variable swap — `a`↔`b` — which is a real change however consistent it looks; +- any hunk holding a line that is not a binding: an expression (`a = b + c;`), a call, a cast, an address-of or a dereference, a prototype, a multi-declarator list. One such line leaves the **whole** hunk real, so a rename sitting next to a real edit is still shown. + +**How it announces itself.** Folding without proof is only acceptable if it is impossible to miss: + +- the terminal summary prints a `QUICK CHECK` warning above the counts; +- the HTML report carries a one-line note at its foot, and a revealed hunk's placeholder row names the kind (`assumed-rename`) rather than calling it a proven rename; +- the JSON output carries `"quick_check": "skip-var-renames"`; +- the viewer's title bar says `QUICK CHECK: variable renames skipped`. + +The flag only ever touches C/C++ bindings, so it does nothing at all with `--arxml-only`. + ### Comment is its own category A file whose only differences are comments gets reported as **Comment**, kept separate from **Unimportant** (which covers UUIDs, timestamps, SW-VERSION, descriptions, renames and whitespace) — because "someone rewrote the comment banner" and "an identifier got renamed" are different enough findings that they shouldn't share a bucket. Each gets its own count in the CLI summary and its own tree marker in the viewer. A file that mixes comment changes *with* other noise stays classified as Unimportant, since the narrower Comment label wouldn't be accurate for it. The viewer has a separate rule toggle for each, and the HTML report gives `Unimportant` its own badge while never rendering comment lines at all. diff --git a/docs/vi/README.md b/docs/vi/README.md index ec00a85..da58969 100644 --- a/docs/vi/README.md +++ b/docs/vi/README.md @@ -122,6 +122,11 @@ Tự động nhận diện các thay đổi phổ biến do code generator tạo Những thay đổi không thể giải thích một cách an toàn vẫn được giữ lại như **real changes**. +Có đúng một ngoại lệ phải tự bật: `--skip-var-renames` gộp các chỗ đổi tên biến +mà tool không chứng minh được là noise, để quét nhanh một bản regenerate xem có +gì *không phải* đổi tên. Đổi lại, nó bỏ sót những thay đổi thật có cùng hình +dạng; mọi bề mặt đều báo rõ lần chạy có bật cờ, và mặc định cờ luôn tắt. + Xem [What Counts as Noise](usage.md#cái-gì-bị-tính-là-noise). --- diff --git a/docs/vi/architecture.md b/docs/vi/architecture.md index 92af0cb..5255f51 100644 --- a/docs/vi/architecture.md +++ b/docs/vi/architecture.md @@ -222,8 +222,18 @@ Mọi thứ ở phía sau — summary của CLI, HTML report, cây của viewer, ``` Range đánh số từ 0, hở đầu cuối (end-exclusive), tính trên dòng **thô** của mỗi bên. -`kind` là một trong `real`, `moved`, `comment`, `rename`, `uuid`, `timestamp`, -`sw-version`, `description`, `whitespace`, `mixed`. +`kind` là một trong `real`, `moved`, `comment`, `rename`, `assumed-rename`, +`reorder`, `uuid`, `timestamp`, `sw-version`, `description`, `whitespace`, +`mixed`. + +`assumed-rename` là kind duy nhất được gán **không kèm chứng minh**, và chỉ +xuất hiện khi caller truyền `skip_var_renames` (`--skip-var-renames`). Nó gộp +một hunk mà mọi dòng đều là binding — câu lệnh chỉ gọi tên một object và +nhiều nhất là chép một object khác vào đó (`a = b;`, `rtY.Out = rtU.Pedal;`, +`real_T x;`, `boolean_T f = FALSE;`) — chỉ khác nhau ở tên định danh, mà một +thay đổi đấu nối lại cũng trông y hệt vậy. Nói cách khác đây là chế độ cố ý chấp nhận báo thiếu: nó giữ kind riêng để +không bề mặt nào đọc thành `rename`, và report, summary trên terminal, JSON lẫn +tiêu đề viewer đều nói rõ lần chạy đó có bật cờ. Phần ngữ nghĩa chỉ được tính ở chỗ nó có thể có nghĩa: file có shadow bằng nhau thì nội dung như nhau, nên không thể làm xê dịch bề mặt AUTOSAR. diff --git a/docs/vi/usage.md b/docs/vi/usage.md index a5ddfbb..1c047f5 100644 --- a/docs/vi/usage.md +++ b/docs/vi/usage.md @@ -44,6 +44,7 @@ giờ âm thầm rơi xuống so sánh một thư mục rỗng. | `--current-name NAME` | Tương tự cho phía CURRENT. Cả hai cờ chỉ đổi chữ trên header — đường dẫn thư mục vẫn nằm ở tooltip, nên vẫn truy được file đã đọc từ đâu | | `--theme dark\|light` | Bảng màu lúc mở của report và viewer (mặc định `dark`). Report mang sẵn **cả hai** và có nút đổi riêng, nên cờ này chỉ quyết định người đọc thấy màu nào trước | | `--rules RULES.json` | Thêm pattern noise chạy trên nền các rule sẵn có — xem [Custom noise rules](#custom-noise-rules) | +| `--skip-var-renames` | **Quick check, không an toàn.** Gộp các hunk C/C++ chỉ đổi tên biến mà *không* chứng minh được đó là noise — xem [Quick check: bỏ qua đổi tên biến](#quick-check-bỏ-qua-đổi-tên-biến) | | `--max-diff-lines N` | Giới hạn diff nhúng mỗi file khoảng N dòng (0 = không giới hạn, mặc định). Chặn một lần regenerate cả cây render một file thành report to đến mức treo browser; file bị cắt vẫn giữ verdict và exit code, và chỗ cắt được báo rõ | | `--qt`, `--viewer` | Mở viewer trên hai thư mục truyền ở command line, thay vì so sánh trong terminal. Cần extra `viewer` | | `--version` | In phiên bản rồi thoát | @@ -177,6 +178,7 @@ trên cây vẫn nằm trong file export với verdict thật của nó. | `timestamp` | Block ``, `` | .arxml .xml | | `sw-version` | Version stamp `` (tăng mỗi lần regenerate). Regex có anchor, nên `` và các thẻ tương tự không bị đụng | .arxml .xml | | `description` | ``, ``, `` — các thẻ chứa mô tả bằng chữ, không ảnh hưởng hành vi (áp dụng cho cả schema 4.2 và 4.4). `` và `` **không** được lọc: `` ảnh hưởng cách phần tử được hiểu, còn `` có thể chứa dữ liệu do tool khác ghi vào | .arxml .xml | +| `assumed-rename` | Lệnh gán và khai báo chỉ khác nhau ở tên biến, gộp **không kèm chứng minh** — chỉ xuất hiện khi bật `--skip-var-renames`, mặc định không bao giờ | .c .h .cpp .hpp | | `whitespace` | Thụt đầu dòng, khoảng trắng cuối dòng, dòng trống | tất cả | | `line-endings` | CRLF vs LF, BOM | tất cả | @@ -228,6 +230,62 @@ vào giữa, vế phải của một phép gán thật sự đổi, hay một c đảo thứ tự — thì cả block vẫn tính là thay đổi thật. Khi không chắc, tool luôn chọn hiện diff ra chứ không giấu đi. +### Quick check: bỏ qua đổi tên biến + +Mọi rule ở trên đều chứng minh được trước khi gộp một khác biệt đi. +`--skip-var-renames` thì không, và đây là phần duy nhất của tool làm vậy — nên +nó mặc định tắt và phải gọi đích danh mới chạy. + +Nó phục vụ đúng một việc: quét nhanh một bản regenerate để tìm những gì **không +phải** đổi tên, khi bạn đã biết trước sẽ có một loạt biến bị đổi tên và chỉ muốn +xem phần còn lại. Nó không dùng để ký duyệt review. + +Khi bật cờ, một hunk C/C++ được gộp thành `assumed-rename` nếu mọi dòng ở cả hai +phía đều là **binding** — một câu lệnh chỉ gọi tên một object và nhiều nhất là +chép một object hoặc literal khác vào đó — hai phía ghép được 1-1 theo dòng, +chỉ khác nhau ở tên định danh, và kiểu khai báo không đổi. + +Binding không bao giờ *tính toán*. Hai vế của dấu `=` đều được phép là lvalue +path thuần — một cái tên, `.field`, `->field`, `[index]` — đúng cách Embedded +Coder truy cập port, DWork state và buffer: + +```c +a = b; → x = b; +acc_cmd = rtU.Pedal; → drv_cmd = rtU.Pedal; +rtY.Out = filt_in; → rtY.Out = filt_val; +buf[2] = rtDW->State; → buf[2] = rtDW->Level; +real_T filt_in; → real_T filt_val; +boolean_T flag = FALSE; → boolean_T other = FALSE; +``` + +**Cái giá phải trả.** Một thay đổi đấu nối lại (rewiring) có đúng hình dạng đó, +nên cũng bị gộp luôn. `a = speed;` đổi thành `a = torque;` là thay đổi tín hiệu +thật, và chế độ này sẽ không báo. Đó chính là sự đánh đổi mà cờ này sinh ra để +chấp nhận: một lần chạy có dùng cờ có thể đang thiếu một thay đổi thật. + +**Cái gì vẫn tính là thay đổi thật, kể cả khi bật cờ:** + +- literal đổi giá trị — `a = 0;` → `x = 1;`; +- kiểu dữ liệu đổi — `sint32 a = 0;` → `uint8 x = 0;`; +- macro hoặc enum constant viết hoa toàn bộ ở một trong hai phía — `flag = FALSE;` + → `flag = TRUE;`, `mode = IDLE;` → `mode = DRIVE;`. C dành tên viết hoa cho + hằng khai báo ở chỗ khác, nên tráo một cái là đổi giá trị chứ không phải đổi tên; +- hoán đổi hai biến — `a`↔`b` — dù trông nhất quán đến đâu vẫn là thay đổi thật; +- bất kỳ hunk nào có một dòng không phải binding: biểu thức (`a = b + c;`), lời + gọi hàm, ép kiểu, lấy địa chỉ hay dereference, khai báo prototype, khai báo + nhiều biến trên một dòng. Chỉ một dòng như vậy là **cả hunk** giữ nguyên trạng + thái thật, nên một chỗ đổi tên nằm cạnh một sửa đổi thật vẫn được hiện ra. + +**Cách nó tự báo.** Gộp mà không chứng minh chỉ chấp nhận được nếu không thể bỏ sót: + +- summary trên terminal in cảnh báo `QUICK CHECK` ngay phía trên phần đếm; +- HTML report có một dòng note ở cuối trang, và dòng placeholder của hunk bị + gộp ghi đúng kind `assumed-rename` chứ không gọi nó là rename đã chứng minh; +- output JSON mang thêm `"quick_check": "skip-var-renames"`; +- thanh tiêu đề của viewer ghi `QUICK CHECK: variable renames skipped`. + +Cờ này chỉ đụng tới binding C/C++, nên đi kèm `--arxml-only` thì không có tác dụng gì. + ### Comment là hạng mục riêng File mà khác biệt *chỉ* nằm ở comment được báo là **Comment**, tách riêng khỏi diff --git a/tests/test_report.py b/tests/test_report.py index 7452c85..f32d0ea 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -104,26 +104,25 @@ def test_report_carries_no_redundant_hunk_composition_text(self): sect = next(s for s in page.split('
')[0] - # the header may now carry an "Affected: " hint (which functions - # changed -- not a recount of the rows), so the check is on the - # composition wording itself, not the hcount span it once rode in on header = sect.split('
')[0] self.assertNotIn('hunk', header) self.assertNotIn('hunklabel', sect) self.assertNotIn('comment + real', sect) def test_report_captions_the_enclosing_function(self): - # the real hunk in NoiseDemo.c sits inside Calc_step; the group gets - # a caption naming it, and the file header lists it as Affected + # the real hunk in NoiseDemo.c sits inside Calc_step, and the group + # gets a caption naming it. The file header does NOT repeat it: a + # header answers "must I read this file", and the answer is the verdict results = scan(DEMO / 'old', DEMO / 'new') page = build_report(results, DEMO / 'old', DEMO / 'new') sect = next(s for s in page.split('
')[0] header = sect.split('
')[0] - self.assertIn('Affected: Calc_step', header) - self.assertIn('class="fnhdr"', sect) - self.assertIn('Calc_step', sect.split('')[0] for p in sect.split('class="fnhdr"')[1:]] + self.assertTrue(any('Calc_step' in c for c in captions), captions) def test_report_shows_minor_hunks_in_modified_files(self): results = scan(DEMO / 'old', DEMO / 'new') diff --git a/tests/test_skip_var_renames.py b/tests/test_skip_var_renames.py new file mode 100644 index 0000000..0333203 --- /dev/null +++ b/tests/test_skip_var_renames.py @@ -0,0 +1,282 @@ +"""The --skip-var-renames quick check. + +The one mode that folds a difference WITHOUT proving it is noise, so these +tests carry two burdens the other rule tests do not: what it folds (bindings +that differ only by variable names), and what it still refuses to fold even +here -- a changed literal, a changed type, an ALL_CAPS macro/enum swap, a +variable swap, and any hunk holding a line that is not a plain binding. + +Everything it does fold must stay visible as ``assumed-rename`` and must be +announced -- in the terminal summary, in the report and in the JSON -- so a +green verdict from this mode can never be mistaken for a green verdict from +the proven rules. +""" + +import io +import json +import unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path +from tempfile import TemporaryDirectory + +from compare_tool import c_rules +from compare_tool.diff_engine import compare_pair +from compare_tool.main import _parser, main, summary_lines +from compare_tool.report import build_report +from compare_tool.scanner import scan, summarize +from compare_tool.serialize import build + +# The renamed name is deliberately still referenced further down, so the proven +# file-wide rename map refuses the file (it only accepts a name that vanished). +# That is what leaves the hunk real without the flag -- and therefore what the +# quick check is being asked to fold. +BODY = 'void f(void)\n{\n %s\n keep(a);\n keep(mode);\n}\n' + + +def kinds(r): + return [h['kind'] for h in r['hunks']] + + +def pair(old_stmt, new_stmt, skip=True): + return compare_pair(BODY % old_stmt, BODY % new_stmt, 'f.c', (), skip) + + +class TestBindingParts(unittest.TestCase): + def test_plain_assignment(self): + self.assertEqual(c_rules.binding_parts('a = b;'), ((), ('a',), ('b',))) + + def test_declaration_with_initializer(self): + self.assertEqual(c_rules.binding_parts('boolean_T flag = FALSE;'), + (('boolean_T',), ('flag',), ('FALSE',))) + + def test_bare_declaration_has_no_value(self): + self.assertEqual(c_rules.binding_parts('real_T filt_in;'), + (('real_T',), ('filt_in',), ())) + + def test_signed_literal(self): + self.assertEqual(c_rules.binding_parts('sint32 x = -1;'), + (('sint32',), ('x',), ('-', '1'))) + + def test_member_and_index_paths_on_both_sides(self): + # how Embedded Coder actually reaches a port, a DWork or a buffer + self.assertEqual(c_rules.binding_parts('rtY.Out = rtU.Pedal;'), + ((), ('rtY', '.', 'Out'), ('rtU', '.', 'Pedal'))) + self.assertIsNotNone(c_rules.binding_parts('buf[2] = rtDW->State;')) + self.assertIsNotNone(c_rules.binding_parts('p->q = r[i];')) + + def test_what_is_not_a_binding(self): + for line in ('a = b + c;', # an expression + 'a = foo(b);', # a call + 'a = (real_T)b;', # a cast + 'a = &b;', # an address-of + 'x = *p;', # a dereference + 'a = b', # no terminator + 'if (a == b) {', # a comparison + 'a = b; c = d;', # two statements + 'real_T a, b;', # more than one declarator + 'void f(void);', # a prototype + 'a;', # a statement, not a declaration + 'return a;', # 'return' is not a type + 'break;'): + self.assertIsNone(c_rules.binding_parts(line), line) + + +class TestAssumedRenameMap(unittest.TestCase): + def test_name_swap_is_mapped(self): + self.assertEqual(c_rules.assumed_rename_map(['a = b;'], ['x = b;']), + {'a': 'x'}) + + def test_both_sides_of_the_binding_may_move(self): + self.assertEqual(c_rules.assumed_rename_map(['a = b;'], ['x = y;']), + {'a': 'x', 'b': 'y'}) + + def test_a_changed_literal_is_not_a_rename(self): + self.assertIsNone(c_rules.assumed_rename_map(['a = 0;'], ['x = 1;'])) + + def test_a_changed_type_is_not_a_rename(self): + self.assertIsNone(c_rules.assumed_rename_map(['sint32 a = 0;'], + ['uint8 x = 0;'])) + self.assertIsNone(c_rules.assumed_rename_map(['real_T a;'], ['uint8 a_;'])) + + def test_a_bare_declaration_rename_is_mapped(self): + self.assertEqual(c_rules.assumed_rename_map(['real_T filt_in;'], + ['real_T filt_val;']), + {'filt_in': 'filt_val'}) + + def test_a_path_rename_is_mapped(self): + self.assertEqual(c_rules.assumed_rename_map(['a = rtU.Pedal;'], + ['x = rtU.Pedal;']), + {'a': 'x'}) + + def test_a_changed_index_literal_is_not_a_rename(self): + self.assertIsNone(c_rules.assumed_rename_map(['buf[0] = b;'], + ['buf[1] = b;'])) + + def test_an_all_caps_constant_is_not_a_rename(self): + # FALSE -> TRUE and IDLE -> DRIVE are value changes wearing the shape + self.assertIsNone(c_rules.assumed_rename_map(['a = FALSE;'], ['a = TRUE;'])) + self.assertIsNone(c_rules.assumed_rename_map(['m = IDLE;'], ['m = DRIVE;'])) + + def test_a_variable_swap_is_not_a_rename(self): + self.assertIsNone(c_rules.assumed_rename_map(['a = c;', 'b = d;'], + ['b = c;', 'a = d;'])) + + def test_one_non_binding_line_refuses_the_whole_hunk(self): + self.assertIsNone(c_rules.assumed_rename_map(['a = b;', 'q = foo();'], + ['x = b;', 'q = bar();'])) + + def test_sides_that_do_not_pair_one_to_one(self): + self.assertIsNone(c_rules.assumed_rename_map(['a = b;'], + ['x = b;', 'y = c;'])) + self.assertIsNone(c_rules.assumed_rename_map([], [])) + + +class TestComparePair(unittest.TestCase): + def test_the_flag_is_off_by_default(self): + r = compare_pair(BODY % 'a = b;', BODY % 'x = b;', 'f.c') + self.assertEqual(r['status'], 'real-change') + self.assertIn('real', kinds(r)) + + def test_an_assignment_rename_is_folded(self): + r = pair('a = b;', 'x = b;') + self.assertEqual(r['status'], 'ignorable-only') + self.assertEqual(set(kinds(r)), {'assumed-rename'}) + + def test_a_declaration_rename_is_folded(self): + r = pair('boolean_T a = FALSE;', 'boolean_T x = FALSE;') + self.assertEqual(r['status'], 'ignorable-only') + self.assertEqual(set(kinds(r)), {'assumed-rename'}) + + def test_a_port_read_rename_is_folded(self): + # the everyday Embedded Coder shape the mode exists for + r = pair('a = rtU.Pedal;', 'x = rtU.Pedal;') + self.assertEqual(r['status'], 'ignorable-only') + self.assertEqual(set(kinds(r)), {'assumed-rename'}) + + def test_a_bare_declaration_rename_is_folded(self): + # `a` is still referenced below, so the proven map refuses it and the + # label is the quick check's own -- a name that vanished would be a + # plain `rename` and would not exercise this mode at all + r = pair('real_T a;', 'real_T a_val;') + self.assertEqual(r['status'], 'ignorable-only') + self.assertEqual(set(kinds(r)), {'assumed-rename'}) + + def test_the_documented_false_negative(self): + # stated plainly because it is the price of the mode: a rewiring has + # the same shape as a rename and is folded too, on a plain name and on + # a port field alike + self.assertEqual(pair('a = speed;', 'a = torque;')['status'], + 'ignorable-only') + self.assertEqual(pair('a = rtU.Pedal;', 'a = rtU.Brake;')['status'], + 'ignorable-only') + + def test_what_stays_real_even_in_quick_check(self): + for old, new in (('boolean_T a = FALSE;', 'boolean_T a = TRUE;'), + ('sint32 a = 0;', 'sint32 x = 1;'), + ('sint32 a = 0;', 'uint8 x = 0;'), + ('mode = IDLE;', 'mode = DRIVE;'), + ('a = b + c;', 'x = b + c;'), + ('a = foo(b);', 'x = foo(b);'), + ('real_T a;', 'uint8 a_;'), + ('buf[0] = b;', 'buf[1] = b;')): + r = pair(old, new) + self.assertEqual(r['status'], 'real-change', (old, new)) + + def test_a_real_change_beside_a_rename_keeps_the_hunk_real(self): + r = pair('a = b;\n q = foo(1);', 'x = b;\n q = foo(2);') + self.assertEqual(r['status'], 'real-change') + self.assertIn('real', kinds(r)) + + def test_arxml_is_untouched_by_the_flag(self): + old = '\na\n\n' + new = '\nx\n\n' + self.assertEqual(compare_pair(old, new, 'f.arxml', (), True)['status'], + 'real-change') + + +class TestAnnouncement(unittest.TestCase): + """Folded is never hidden: every surface has to say the mode was on.""" + + def setUp(self): + self.results = {'f.c': pair('a = b;', 'x = b;')} + self.results['f.c']['binary'] = False + + def test_the_terminal_summary_warns(self): + text = '\n'.join(summary_lines(self.results, summarize(self.results))) + self.assertIn('QUICK CHECK', text) + self.assertIn('--skip-var-renames', text) + + def test_a_clean_run_does_not_warn(self): + results = {'f.c': compare_pair(BODY % 'a = b;', BODY % 'a = b;', 'f.c')} + results['f.c']['binary'] = False + text = '\n'.join(summary_lines(results, summarize(results))) + self.assertNotIn('QUICK CHECK', text) + + def test_the_report_carries_a_footer_note(self): + html = build_report(self.results, 'old', 'new') + self.assertIn('Variable renames ignored', html) + self.assertIn('--skip-var-renames', html) + # a caption, not a banner: below the tree, above the diffs + # (rindex, because the CSS rule for the class comes first) + where = html.rindex('qcnote') + self.assertGreater(where, html.index('Folder tree')) + self.assertLess(where, html.index('Detailed changes')) + + def test_the_report_has_no_note_without_the_mode(self): + results = {'f.c': compare_pair(BODY % 'a = b;', BODY % 'x = b;', 'f.c')} + results['f.c']['binary'] = False + self.assertNotIn('Variable renames ignored', + build_report(results, 'old', 'new')) + + def test_the_json_records_the_mode(self): + doc = build(self.results, summarize(self.results), 'old', 'new', 0) + self.assertEqual(doc['quick_check'], 'skip-var-renames') + + def test_the_json_is_unmarked_without_the_mode(self): + results = {'f.c': compare_pair(BODY % 'a = b;', BODY % 'x = b;', 'f.c')} + doc = build(results, summarize(results), 'old', 'new', 1) + self.assertNotIn('quick_check', doc) + + +class TestCli(unittest.TestCase): + def test_the_flag_defaults_off(self): + self.assertFalse(_parser().parse_args(['old', 'new']).skip_var_renames) + + def test_the_flag_parses(self): + args = _parser().parse_args(['old', 'new', '--skip-var-renames']) + self.assertTrue(args.skip_var_renames) + + def _tree(self, tmp): + old, new = Path(tmp) / 'old', Path(tmp) / 'new' + old.mkdir() + new.mkdir() + (old / 'f.c').write_text(BODY % 'a = b;', encoding='utf-8') + (new / 'f.c').write_text(BODY % 'x = b;', encoding='utf-8') + return old, new + + def test_end_to_end_the_exit_code_drops_to_zero(self): + with TemporaryDirectory() as tmp: + old, new = self._tree(tmp) + out, js = Path(tmp) / 'r.html', Path(tmp) / 'r.json' + argv = [str(old), str(new), '--report', str(out), '--json', str(js)] + buf = io.StringIO() + with redirect_stdout(buf), redirect_stderr(io.StringIO()): + self.assertEqual(main(argv), 1) # real change: gate trips + self.assertEqual(main(argv + ['--skip-var-renames']), 0) + self.assertIn('QUICK CHECK', buf.getvalue()) + self.assertIn('Variable renames ignored', + out.read_text(encoding='utf-8')) + doc = json.loads(js.read_text(encoding='utf-8')) + self.assertEqual(doc['quick_check'], 'skip-var-renames') + self.assertEqual(doc['exit_code'], 0) + + def test_scan_takes_the_flag(self): + with TemporaryDirectory() as tmp: + old, new = self._tree(tmp) + self.assertEqual(scan(old, new)['f.c']['status'], 'real-change') + self.assertEqual(scan(old, new, skip_var_renames=True)['f.c']['status'], + 'ignorable-only') + + +if __name__ == '__main__': + unittest.main()