From 8ef24d6974b78910273e0d7ebfdca270b5a06cb4 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 00:17:44 -0400 Subject: [PATCH 1/3] Use diff_match_patch for --fix dry-run output The previous _printDiff in validation_session.dart compared lines at the same index, which misrepresented any change that inserted or removed a line. Replace it with a line-tokenized diff backed by package:diff_match_patch. Fixes flutter/skills#79. --- .../lib/src/validation_session.dart | 86 ++++++++++++++----- tool/dart_skills_lint/pubspec.yaml | 1 + .../test/diff_printer_test.dart | 77 +++++++++++++++++ 3 files changed, 142 insertions(+), 22 deletions(-) create mode 100644 tool/dart_skills_lint/test/diff_printer_test.dart diff --git a/tool/dart_skills_lint/lib/src/validation_session.dart b/tool/dart_skills_lint/lib/src/validation_session.dart index 16872e0..eeb8879 100644 --- a/tool/dart_skills_lint/lib/src/validation_session.dart +++ b/tool/dart_skills_lint/lib/src/validation_session.dart @@ -5,6 +5,7 @@ import 'dart:convert'; import 'dart:io'; +import 'package:diff_match_patch/diff_match_patch.dart' as dmp; import 'package:logging/logging.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; @@ -462,29 +463,9 @@ class ValidationSession { return result; } - /// Prints a simple line-by-line diff between [original] and [modified]. - /// - /// **Limitation**: This naive diff algorithm does not handle line additions - /// or removals well, as it compares lines at the same index. It is - /// sufficient for current fixers that only modify existing lines, but - /// should be replaced with a more robust diffing solution (e.g., - /// `package:diff`) if future fixers add or remove lines. + /// Prints a line-level diff between [original] and [modified] to the logger. void _printDiff(String original, String modified) { - final List origLines = original.split('\n'); - final List modLines = modified.split('\n'); - final int maxLines = origLines.length > modLines.length ? origLines.length : modLines.length; - for (var i = 0; i < maxLines; i++) { - final String orig = i < origLines.length ? origLines[i] : ''; - final String mod = i < modLines.length ? modLines[i] : ''; - if (orig != mod) { - if (orig.isNotEmpty) { - _log.info('- Line ${i + 1}: $orig'); - } - if (mod.isNotEmpty) { - _log.info('+ Line ${i + 1}: $mod'); - } - } - } + computeLineDiff(original, modified).forEach(_log.info); } /// Mutates [ignores] in place to add baseline entries for any non-ignored @@ -557,3 +538,64 @@ class ValidationSession { return path; } } + +/// Computes a line-level diff between [original] and [modified] suitable for +/// dry-run preview output. +/// +/// Returns a list of formatted strings, one per changed line: +/// `'- Line N: '` for removed lines (numbered against [original]) and +/// `'+ Line N: '` for added lines (numbered against [modified]). +/// Unchanged lines are not emitted. +/// +/// Handles insertions, deletions, and modifications correctly. Each line of +/// each input is treated atomically (no intra-line diffs). +@visibleForTesting +List computeLineDiff(String original, String modified) { + final lineArray = ['']; + final lineToIndex = {}; + + String tokenize(String text) { + final buf = StringBuffer(); + for (final String line in text.split('\n')) { + final int idx = lineToIndex.putIfAbsent(line, () { + lineArray.add(line); + return lineArray.length - 1; + }); + buf.writeCharCode(idx); + } + return buf.toString(); + } + + final String tokenizedOriginal = tokenize(original); + final String tokenizedModified = tokenize(modified); + + final List diffs = dmp.diff( + tokenizedOriginal, + tokenizedModified, + checklines: false, + ); + + final output = []; + var originalLineNumber = 1; + var modifiedLineNumber = 1; + + for (final d in diffs) { + final List codes = d.text.codeUnits; + switch (d.operation) { + case dmp.DIFF_EQUAL: + originalLineNumber += codes.length; + modifiedLineNumber += codes.length; + case dmp.DIFF_DELETE: + for (final c in codes) { + output.add('- Line $originalLineNumber: ${lineArray[c]}'); + originalLineNumber++; + } + case dmp.DIFF_INSERT: + for (final c in codes) { + output.add('+ Line $modifiedLineNumber: ${lineArray[c]}'); + modifiedLineNumber++; + } + } + } + return output; +} diff --git a/tool/dart_skills_lint/pubspec.yaml b/tool/dart_skills_lint/pubspec.yaml index 91e0a0d..9f33fe3 100644 --- a/tool/dart_skills_lint/pubspec.yaml +++ b/tool/dart_skills_lint/pubspec.yaml @@ -9,6 +9,7 @@ environment: dependencies: args: ^2.4.0 + diff_match_patch: ^0.4.1 io: ^1.0.4 logging: ^1.2.0 meta: ^1.11.0 diff --git a/tool/dart_skills_lint/test/diff_printer_test.dart b/tool/dart_skills_lint/test/diff_printer_test.dart new file mode 100644 index 0000000..31d01b6 --- /dev/null +++ b/tool/dart_skills_lint/test/diff_printer_test.dart @@ -0,0 +1,77 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:dart_skills_lint/src/validation_session.dart'; +import 'package:test/test.dart'; + +void main() { + group('computeLineDiff', () { + test('returns empty list when texts are identical', () { + expect(computeLineDiff('a\nb\nc', 'a\nb\nc'), isEmpty); + }); + + test('emits paired - and + lines for a pure modification', () { + const original = 'line1\nline2 \nline3'; + const modified = 'line1\nline2\nline3'; + + expect(computeLineDiff(original, modified), [ + '- Line 2: line2 ', + '+ Line 2: line2', + ]); + }); + + test('emits only + lines for a pure insertion at the end', () { + const original = 'a\nb'; + const modified = 'a\nb\nc'; + + expect(computeLineDiff(original, modified), ['+ Line 3: c']); + }); + + test('emits only + lines for a pure insertion in the middle', () { + const original = 'a\nc'; + const modified = 'a\nb\nc'; + + expect(computeLineDiff(original, modified), ['+ Line 2: b']); + }); + + test('emits only - lines for a pure deletion', () { + const original = 'a\nb\nc'; + const modified = 'a\nc'; + + expect(computeLineDiff(original, modified), ['- Line 2: b']); + }); + + test('handles mixed modification + insertion with correct line numbers', () { + const original = 'header\nbody\nfooter'; + const modified = 'header\nbody-fixed\nextra\nfooter'; + + expect(computeLineDiff(original, modified), [ + '- Line 2: body', + '+ Line 2: body-fixed', + '+ Line 3: extra', + ]); + }); + + test('handles deletion followed by modification with correct line numbers', () { + const original = 'a\nb\nc\nd'; + const modified = 'a\nc-fixed\nd'; + + expect(computeLineDiff(original, modified), [ + '- Line 2: b', + '- Line 3: c', + '+ Line 2: c-fixed', + ]); + }); + + test('preserves significant whitespace in line contents', () { + const original = '\t indented\n spaced'; + const modified = '\t fixed\n spaced'; + + expect(computeLineDiff(original, modified), [ + '- Line 1: \t indented', + '+ Line 1: \t fixed', + ]); + }); + }); +} From 43e64496f372f75e21c32119078b3a3b45b345ab Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 00:35:58 -0400 Subject: [PATCH 2/3] Run dart format --- tool/dart_skills_lint/lib/src/validation_session.dart | 6 +----- tool/dart_skills_lint/test/diff_printer_test.dart | 5 +---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/tool/dart_skills_lint/lib/src/validation_session.dart b/tool/dart_skills_lint/lib/src/validation_session.dart index eeb8879..5b66acb 100644 --- a/tool/dart_skills_lint/lib/src/validation_session.dart +++ b/tool/dart_skills_lint/lib/src/validation_session.dart @@ -569,11 +569,7 @@ List computeLineDiff(String original, String modified) { final String tokenizedOriginal = tokenize(original); final String tokenizedModified = tokenize(modified); - final List diffs = dmp.diff( - tokenizedOriginal, - tokenizedModified, - checklines: false, - ); + final List diffs = dmp.diff(tokenizedOriginal, tokenizedModified, checklines: false); final output = []; var originalLineNumber = 1; diff --git a/tool/dart_skills_lint/test/diff_printer_test.dart b/tool/dart_skills_lint/test/diff_printer_test.dart index 31d01b6..b7ef8e7 100644 --- a/tool/dart_skills_lint/test/diff_printer_test.dart +++ b/tool/dart_skills_lint/test/diff_printer_test.dart @@ -15,10 +15,7 @@ void main() { const original = 'line1\nline2 \nline3'; const modified = 'line1\nline2\nline3'; - expect(computeLineDiff(original, modified), [ - '- Line 2: line2 ', - '+ Line 2: line2', - ]); + expect(computeLineDiff(original, modified), ['- Line 2: line2 ', '+ Line 2: line2']); }); test('emits only + lines for a pure insertion at the end', () { From 1a6ce7372bf6668e9ed4e0f266555b1ba7d74aff Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 00:38:58 -0400 Subject: [PATCH 3/3] Address review: drop unused lineArray sentinel Initialize lineArray empty and offset char codes by 1 (instead of using a dummy '' at index 0). Avoids char code 0 in the tokenized strings and removes the dead sentinel slot that was never indexed by lineToIndex. --- tool/dart_skills_lint/lib/src/validation_session.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tool/dart_skills_lint/lib/src/validation_session.dart b/tool/dart_skills_lint/lib/src/validation_session.dart index 5b66acb..7b4eb42 100644 --- a/tool/dart_skills_lint/lib/src/validation_session.dart +++ b/tool/dart_skills_lint/lib/src/validation_session.dart @@ -551,7 +551,7 @@ class ValidationSession { /// each input is treated atomically (no intra-line diffs). @visibleForTesting List computeLineDiff(String original, String modified) { - final lineArray = ['']; + final lineArray = []; final lineToIndex = {}; String tokenize(String text) { @@ -561,7 +561,8 @@ List computeLineDiff(String original, String modified) { lineArray.add(line); return lineArray.length - 1; }); - buf.writeCharCode(idx); + // Offset by 1 so the first line never collides with char code 0 (NUL). + buf.writeCharCode(idx + 1); } return buf.toString(); } @@ -583,12 +584,12 @@ List computeLineDiff(String original, String modified) { modifiedLineNumber += codes.length; case dmp.DIFF_DELETE: for (final c in codes) { - output.add('- Line $originalLineNumber: ${lineArray[c]}'); + output.add('- Line $originalLineNumber: ${lineArray[c - 1]}'); originalLineNumber++; } case dmp.DIFF_INSERT: for (final c in codes) { - output.add('+ Line $modifiedLineNumber: ${lineArray[c]}'); + output.add('+ Line $modifiedLineNumber: ${lineArray[c - 1]}'); modifiedLineNumber++; } }