From f122b6a59027400ca4d24711dcd531525bb22f9d Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 3 Sep 2026 16:09:51 -0700 Subject: [PATCH 1/4] feat(rfc_tools): implement semantic RFC number validator CLI --- bin/validate_rfc_number.dart | 86 ++++++ lib/src/git_lister.dart | 6 +- lib/src/validator.dart | 377 +++++++++++++++++++++++ test/validate_rfc_number_test.dart | 479 +++++++++++++++++++++++++++++ 4 files changed, 944 insertions(+), 4 deletions(-) create mode 100644 bin/validate_rfc_number.dart create mode 100644 lib/src/validator.dart create mode 100644 test/validate_rfc_number_test.dart diff --git a/bin/validate_rfc_number.dart b/bin/validate_rfc_number.dart new file mode 100644 index 0000000..4712a53 --- /dev/null +++ b/bin/validate_rfc_number.dart @@ -0,0 +1,86 @@ +// Copyright 2026 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; +import 'package:args/args.dart'; +import 'package:file/local.dart'; +import 'package:rfc_tools/src/validator.dart'; + +void main(List arguments) async { + final parser = ArgParser() + ..addOption( + 'base-branch', + defaultsTo: 'origin/main', + help: 'Base git branch to check against for collisions.', + ) + ..addFlag( + 'check-main', + negatable: false, + help: 'Check for number collisions against the base git branch.', + ) + ..addFlag( + 'no-drafts', + negatable: false, + help: + 'Reject any ".0000" draft RFCs (required for Merge Queue and main).', + ) + ..addFlag( + 'github-actions', + negatable: false, + help: + 'Output errors in GitHub Actions annotation format (::error file=...::).', + ) + ..addFlag( + 'help', + abbr: 'h', + negatable: false, + help: 'Show usage instructions.', + ); + + ArgResults results; + try { + results = parser.parse(arguments); + } catch (e) { + stderr.writeln('Error parsing arguments: $e\n'); + stderr.writeln(parser.usage); + exit(1); + } + + if (results.flag('help')) { + stdout.writeln('RFC Semantic Validator - Flutter RFC Repository Tooling\n'); + stdout.writeln(parser.usage); + exit(0); + } + + final checkMain = results.flag('check-main'); + final baseBranch = results.option('base-branch')!; + final noDrafts = results.flag('no-drafts'); + final githubActions = results.flag('github-actions'); + + const fs = LocalFileSystem(); + final validator = RfcValidator(fs: fs); + + final (:isSuccess, :errors) = await validator.validate( + noDrafts: noDrafts, + checkMain: checkMain, + baseBranch: baseBranch, + ); + + if (!isSuccess) { + stderr.writeln('RFC validation failed with ${errors.length} error(s):\n'); + for (final error in errors) { + if (githubActions) { + stderr.writeln(error.toGithubAnnotation()); + } else { + stderr.writeln('[ERROR] $error'); + } + } + exit(1); + } + + stdout.writeln( + 'RFC numbers validated cleanly. No collisions or illegal drafts found.', + ); + exit(0); +} diff --git a/lib/src/git_lister.dart b/lib/src/git_lister.dart index 6b20cdc..cc963aa 100644 --- a/lib/src/git_lister.dart +++ b/lib/src/git_lister.dart @@ -7,13 +7,11 @@ import 'dart:io'; import 'github_client.dart' show ProcessRunner; /// Signature for querying RFC files on a remote/base git branch. -typedef GitListFunction = - Future> Function({String baseBranch, String rfcDir}); +typedef GitListFunction = Future> Function({String baseBranch}); /// Default implementation querying git via `git ls-tree`. Future> defaultGitList({ String baseBranch = 'origin/main', - String rfcDir = 'rfc', ProcessRunner processRunner = Process.run, }) async { try { @@ -23,7 +21,7 @@ Future> defaultGitList({ '--name-only', baseBranch, '--', - '$rfcDir/', + 'rfc/', ]); if (result.exitCode != 0) { stdout.writeln('exit code: ${result.exitCode}'); diff --git a/lib/src/validator.dart b/lib/src/validator.dart new file mode 100644 index 0000000..f13655c --- /dev/null +++ b/lib/src/validator.dart @@ -0,0 +1,377 @@ +// Copyright 2026 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:math'; +import 'package:file/file.dart'; +import 'package:path/path.dart' as p; + +import 'git_lister.dart'; +import 'models/rfc_file.dart'; + +export 'git_lister.dart' show GitListFunction, defaultGitList; + +/// A validation error encountered during semantic RFC validation. +class ValidationError { + final String filePath; + final String message; + + const ValidationError({required this.filePath, required this.message}); + + /// Formats the error as a GitHub Actions workflow annotation. + /// + /// Percent-encodes special characters (%, \r, \n) per GitHub Actions workflow + /// command specifications. + String toGithubAnnotation() { + final encoded = message + .replaceAll('%', '%25') + .replaceAll('\r', '%0D') + .replaceAll('\n', '%0A'); + return '::error file=$filePath::$encoded'; + } + + @override + String toString() => '$filePath: $message'; +} + +/// Result of RFC semantic number validation. +typedef ValidationResult = ({bool isSuccess, List errors}); + +/// Extension on [ValidationResult] providing backwards compatibility. +extension ValidationResultExtension on ValidationResult { + /// Whether the validation succeeded with no errors. + /// + /// Alias for [isSuccess]. + bool get isValid => isSuccess; +} + +/// Validates RFC numbers for tree uniqueness, collision with main, and draft absence in merge queues. +class RfcValidator { + /// Default directory name containing RFC markdown documents. + static const String rfcDir = 'rfc'; + + final FileSystem fs; + final GitListFunction gitList; + + const RfcValidator({required this.fs, this.gitList = defaultGitList}); + + /// Runs semantic validation on all RFC files in the repository. + /// + /// Checks: + /// 1. Filename structure matches `AAA.NNNN-.md`. + /// 2. Placeholder `.0000` draft status against [noDrafts]. + /// 3. Tree uniqueness (no duplicate assigned RFC numbers across working tree files). + /// 4. Collision checks against [baseBranch] when [checkMain] is `true`. + /// 5. Sequential numbering gap checks: ensures assigned RFC numbers in each + /// category are strictly sequential without gaps (e.g., following the + /// latest RFC on [baseBranch] when [checkMain] is `true`, or starting at + /// `0001` and contiguous when [checkMain] is `false`). + /// + /// Parameters: + /// - [noDrafts]: If `true`, rejects any RFC files with the `.0000` draft index. + /// During drafting and socialization (before a number is assigned), `.0000` + /// is allowed. Once an RFC number is assigned, in the merge queue, and on + /// the `main` branch, `.0000` is forbidden. + /// - [checkMain]: If `true`, queries [gitList] to discover RFCs on [baseBranch] + /// and rejects working tree RFCs whose assigned numbers collide with existing + /// files on that branch. + /// - [baseBranch]: The git branch ref to check against for collisions when + /// [checkMain] is `true` (defaults to `'origin/main'`). + /// + /// Returns a [ValidationResult] containing any [ValidationError]s found. + Future validate({ + bool noDrafts = false, + bool checkMain = false, + String baseBranch = 'origin/main', + }) async { + final errors = []; + final dir = fs.directory(rfcDir); + + if (!await dir.exists()) { + errors.add( + ValidationError( + filePath: rfcDir, + message: 'RFC directory "$rfcDir" does not exist.', + ), + ); + return (isSuccess: false, errors: errors); + } + + final entries = await dir.list().toList(); + entries.sort((a, b) => a.path.compareTo(b.path)); + + // Validate file-level naming/draft invariants and group valid RFCs by category (AAA). + final rfcsByCategory = >{}; + for (final entry in entries) { + if (entry is File && entry.path.endsWith('.md')) { + final rfc = RfcFile.parse('', path: entry.path); + + _validateFileStructure( + rfc: rfc, + filePath: entry.path, + noDrafts: noDrafts, + errors: errors, + ); + + if (rfc.hasValidFilename) { + (rfcsByCategory[rfc.category!] ??= []).add(rfc); + } + } + } + + // Discover and index base branch RFCs if --check-main is requested. + final baseBranchCategories = { + if (checkMain) ...await _indexBaseBranchBySubsystem(baseBranch), + }; + + // For each category AAA: + // - verify no duplicates + // - verify no collisions (if checkMain) + // - verify no gaps + for (final category in [...rfcsByCategory.keys]..sort()) { + _validateSubsystemCategory( + category: category, + rfcs: rfcsByCategory[category]!, + checkMain: checkMain, + baseBranch: baseBranch, + baseBranchCategory: baseBranchCategories[category], + errors: errors, + ); + } + + return (isSuccess: errors.isEmpty, errors: errors); + } + + /// Validates file-level invariants for a single RFC file: + /// - Filename matches `AAA.NNNN-.md`. + /// - Draft `.0000` status against [noDrafts]. + void _validateFileStructure({ + required RfcFile rfc, + required String filePath, + required bool noDrafts, + required List errors, + }) { + if (!rfc.hasValidFilename) { + errors.add( + ValidationError( + filePath: filePath, + message: + 'Filename "${p.basename(filePath)}" does not match required format "AAA.NNNN-.md".', + ), + ); + return; + } + + if (noDrafts && rfc.isDraft) { + errors.add( + ValidationError( + filePath: filePath, + message: + 'Draft RFC ".0000" detected in "${p.basename(filePath)}". ' + 'Drafts must be assigned a sequential RFC number before merging.', + ), + ); + } + } + + /// Validates subsystem category invariants for [category] (`AAA`): + /// 1. Verify no duplicate assigned numbers within the category. + /// 2. If [checkMain] is enabled: + /// - Verify no collisions with [baseBranch]. + /// - Verify no gaps following the latest RFC on [baseBranch]. + /// 3. Otherwise: + /// - Verify no internal gaps starting from 0001. + void _validateSubsystemCategory({ + required String category, + required List rfcs, + required bool checkMain, + required String baseBranch, + required _BaseBranchCategory? baseBranchCategory, + required List errors, + }) { + _checkDuplicateNumbers(category: category, rfcs: rfcs, errors: errors); + + if (checkMain) { + _checkBaseBranchCollisions( + category: category, + rfcs: rfcs, + baseBranch: baseBranch, + baseBranchCategory: baseBranchCategory, + errors: errors, + ); + _checkBaseBranchGaps( + category: category, + rfcs: rfcs, + baseBranch: baseBranch, + baseBranchCategory: baseBranchCategory, + errors: errors, + ); + } else { + _checkWorkingTreeGaps(category: category, rfcs: rfcs, errors: errors); + } + } + + /// Discovers and indexes RFC files on [baseBranch] grouped by category `AAA`. + Future> _indexBaseBranchBySubsystem( + String baseBranch, + ) async { + final mainFiles = await gitList(baseBranch: baseBranch); + final categories = {}; + + for (final mf in mainFiles) { + final baseName = p.basename(mf); + final match = RfcFile.filenamePattern.firstMatch(baseName); + if (match != null) { + final cat = match.group(1)!; + final idx = int.parse(match.group(2)!); + if (idx != 0) { + final category = categories.putIfAbsent( + cat, + () => _BaseBranchCategory(), + ); + category.rfcIdToBasename['$cat.${idx.toNNNN()}'] = baseName; + category.indices.add(idx); + } + } + } + return categories; + } + + /// Verifies no duplicate assigned RFC numbers exist within [category]. + void _checkDuplicateNumbers({ + required String category, + required List rfcs, + required List errors, + }) { + final rfcIdToFiles = >{}; + for (final rfc in rfcs) { + if (!rfc.isDraft && rfc.rfcId != null) { + (rfcIdToFiles[rfc.rfcId!] ??= []).add(rfc.path); + } + } + + for (final entry in rfcIdToFiles.entries) { + if (entry.value.length > 1) { + errors.add( + ValidationError( + filePath: entry.value.first, + message: + 'Duplicate RFC number "${entry.key}" detected across multiple files: ' + '${entry.value.map(p.basename).join(', ')}.', + ), + ); + } + } + } + + /// Identifies collisions where working tree RFCs reuse an identifier already + /// landed on [baseBranch] with a different file name. + void _checkBaseBranchCollisions({ + required String category, + required List rfcs, + required String baseBranch, + required _BaseBranchCategory? baseBranchCategory, + required List errors, + }) { + if (baseBranchCategory == null) return; + + for (final rfc in rfcs) { + if (rfc.isDraft || rfc.rfcId == null) continue; + final currentBasename = p.basename(rfc.path); + final existingOnMain = baseBranchCategory.rfcIdToBasename[rfc.rfcId]; + if (existingOnMain != null && existingOnMain != currentBasename) { + errors.add( + ValidationError( + filePath: rfc.path, + message: + 'RFC number "${rfc.rfcId}" collides with existing RFC in $baseBranch ("$existingOnMain"). ' + 'Re-run number assignment to obtain the next sequential number.', + ), + ); + } + } + } + + /// Ensures newly introduced RFCs in [category] are strictly sequential + /// following the latest RFC on [baseBranch] without gaps. + void _checkBaseBranchGaps({ + required String category, + required List rfcs, + required String baseBranch, + required _BaseBranchCategory? baseBranchCategory, + required List errors, + }) { + final newRfcs = [ + for (var rfc in rfcs) + if (!rfc.isDraft && + rfc.index != null && + baseBranchCategory?.rfcIdToBasename[rfc.rfcId] == null) + rfc, + ]..sort((a, b) => a.index!.compareTo(b.index!)); + + final maxOnMain = baseBranchCategory?.maxIndex ?? 0; + var expectedNext = maxOnMain + 1; + + for (final newRfc in newRfcs) { + final actualIndex = newRfc.index!; + if (actualIndex > expectedNext) { + final expectedPadded = expectedNext.toNNNN(); + final maxPadded = maxOnMain.toNNNN(); + final mainContext = maxOnMain == 0 + ? 'no RFCs exist in category "$category"' + : 'the latest RFC is "$category.$maxPadded"'; + errors.add( + ValidationError( + filePath: newRfc.path, + message: + 'RFC number "${newRfc.rfcId}" creates a numbering gap. ' + 'Expected next sequential number is "$category.$expectedPadded" ($mainContext in $baseBranch).', + ), + ); + expectedNext = actualIndex + 1; + } else if (actualIndex == expectedNext) { + expectedNext++; + } + } + } + + /// Ensures assigned RFC numbers in [category] start at 0001 and are + /// strictly contiguous within the working tree. + void _checkWorkingTreeGaps({ + required String category, + required List rfcs, + required List errors, + }) { + final nonDraftRfcs = [ + for (var rfc in rfcs) + if (!rfc.isDraft && rfc.index != null) rfc, + ]..sort((a, b) => a.index!.compareTo(b.index!)); + + var expectedIndex = 1; + for (final rfc in nonDraftRfcs) { + final actualIndex = rfc.index!; + if (actualIndex > expectedIndex) { + final expectedPadded = expectedIndex.toNNNN(); + errors.add( + ValidationError( + filePath: rfc.path, + message: + 'RFC number "${rfc.rfcId}" creates a numbering gap. ' + 'Expected next sequential number is "$category.$expectedPadded".', + ), + ); + expectedIndex = actualIndex + 1; + } else if (actualIndex == expectedIndex) { + expectedIndex++; + } + } + } +} + +/// Discovered base branch RFC files and metadata for a category `AAA`. +class _BaseBranchCategory { + final Map rfcIdToBasename = {}; + final List indices = []; + + int get maxIndex => indices.isEmpty ? 0 : indices.reduce(max); +} diff --git a/test/validate_rfc_number_test.dart b/test/validate_rfc_number_test.dart new file mode 100644 index 0000000..d07f40d --- /dev/null +++ b/test/validate_rfc_number_test.dart @@ -0,0 +1,479 @@ +// Copyright 2026 The Flutter Authors. 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:file/memory.dart'; +import 'package:rfc_tools/src/validator.dart'; +import 'package:test/test.dart'; + +void main() { + group('RfcValidator', () { + late MemoryFileSystem fs; + + setUp(() async { + fs = MemoryFileSystem(); + await fs.directory('rfc').create(recursive: true); + }); + + test('passes valid unique RFC tree', () async { + await fs.file('rfc/110.0001-feature-a.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature A +--- +'''); + + await fs.file('rfc/110.0002-feature-b.md').writeAsString('''--- +type: rfc +rfc: '110.0002' +title: Feature B +--- +'''); + + final validator = RfcValidator(fs: fs); + final result = await validator.validate(); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + }); + + test( + 'ValidationResult is a record supporting isSuccess, isValid, and destructuring', + () async { + await fs.file('rfc/110.0001-feature-a.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature A +--- +'''); + + final validator = RfcValidator(fs: fs); + final result = await validator.validate(); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + + // Pattern matching and record destructuring + final (:isSuccess, :errors) = await validator.validate(); + expect(isSuccess, isTrue); + expect(errors, isEmpty); + }, + ); + + test('detects duplicate RFC numbers within tree', () async { + await fs.file('rfc/110.0001-feature-a.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature A +--- +'''); + + await fs.file('rfc/110.0001-feature-duplicate.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature Duplicate +--- +'''); + + final validator = RfcValidator(fs: fs); + final result = await validator.validate(); + expect(result.isValid, isFalse); + expect( + result.errors.any( + (e) => e.message.contains('Duplicate RFC number "110.0001"'), + ), + isTrue, + ); + }); + + test('rejects .0000 drafts when noDrafts is true', () async { + await fs.file('rfc/110.0000-unassigned-draft.md').writeAsString('''--- +type: rfc +rfc: '110.0000' +title: Draft +--- +'''); + + final validator = RfcValidator(fs: fs); + + // Passes in PR mode (noDrafts = false) + final prResult = await validator.validate(noDrafts: false); + expect(prResult.isValid, isTrue); + + // Fails in Merge Queue / Main mode (noDrafts = true) + final mqResult = await validator.validate(noDrafts: true); + expect(mqResult.isValid, isFalse); + expect( + mqResult.errors.any( + (e) => e.message.contains('Draft RFC ".0000" detected'), + ), + isTrue, + ); + }); + + test('detects collision against simulated main branch', () async { + await fs.file('rfc/110.0003-my-branch-feature.md').writeAsString('''--- +type: rfc +rfc: '110.0003' +title: My Branch Feature +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0003-merged-pr-feature.md', + }, + ); + final result = await validator.validate(checkMain: true); + + expect(result.isValid, isFalse); + expect( + result.errors.any( + (e) => e.message.contains('collides with existing RFC'), + ), + isTrue, + ); + }); + + test( + 'ignores frontmatter contents and focuses on filename numbers', + () async { + await fs.file('rfc/110.0001-feature.md').writeAsString('''--- +arbitrary: content +--- +# Arbitrary content +'''); + + final validator = RfcValidator(fs: fs); + final result = await validator.validate(); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + }, + ); + + test('reports error when rfc directory does not exist', () async { + final emptyFs = MemoryFileSystem(); + final validator = RfcValidator(fs: emptyFs); + final result = await validator.validate(); + expect(result.isValid, isFalse); + expect( + result.errors.any((e) => e.message.contains('does not exist')), + isTrue, + ); + }); + + test('passes baseBranch to gitList when checkMain is true', () async { + String? capturedBranch; + + await fs.file('rfc/110.0001-feature.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async { + capturedBranch = baseBranch; + return {}; + }, + ); + + await validator.validate(checkMain: true, baseBranch: 'custom-branch'); + + expect(capturedBranch, equals('custom-branch')); + }); + + test('does not invoke gitList when checkMain is false', () async { + var gitListCalled = false; + await fs.file('rfc/110.0001-feature.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async { + gitListCalled = true; + return {}; + }, + ); + + final result = await validator.validate(checkMain: false); + expect(result.isValid, isTrue); + expect(gitListCalled, isFalse); + }); + + test('default constructor uses defaultGitList', () { + final validator = RfcValidator(fs: fs); + expect(validator.gitList, equals(defaultGitList)); + }); + + group('sequential numbering gap checks', () { + test('rejects gap when new RFC jumps ahead of baseBranch', () async { + await fs.file('rfc/110.0050-jump-ahead.md').writeAsString('''--- +type: rfc +rfc: '110.0050' +title: Jump Ahead +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0041-feature-41.md', + 'rfc/110.0042-feature-42.md', + }, + ); + + final result = await validator.validate(checkMain: true); + expect(result.isValid, isFalse); + expect( + result.errors.any( + (e) => e.message.contains( + 'RFC number "110.0050" creates a numbering gap. ' + 'Expected next sequential number is "110.0043" (the latest RFC is "110.0042" in origin/main).', + ), + ), + isTrue, + ); + }); + + test( + 'rejects non-0001 start when no RFCs exist in category on baseBranch', + () async { + await fs.file('rfc/120.1234-unallocated-start.md').writeAsString( + '''--- +type: rfc +rfc: '120.1234' +title: Unallocated Start +--- +''', + ); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => {}, + ); + + final result = await validator.validate(checkMain: true); + expect(result.isValid, isFalse); + expect( + result.errors.any( + (e) => e.message.contains( + 'RFC number "120.1234" creates a numbering gap. ' + 'Expected next sequential number is "120.0001" (no RFCs exist in category "120" in origin/main).', + ), + ), + isTrue, + ); + }, + ); + + test('accepts single sequential RFC following baseBranch', () async { + await fs.file('rfc/110.0043-next-feature.md').writeAsString('''--- +type: rfc +rfc: '110.0043' +title: Next Feature +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0042-existing-feature.md', + }, + ); + + final result = await validator.validate(checkMain: true); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + }); + + test('accepts multiple sequential RFCs following baseBranch', () async { + await fs.file('rfc/110.0043-feature-c.md').writeAsString('''--- +type: rfc +rfc: '110.0043' +title: Feature C +--- +'''); + await fs.file('rfc/110.0044-feature-d.md').writeAsString('''--- +type: rfc +rfc: '110.0044' +title: Feature D +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0042-feature-b.md', + }, + ); + + final result = await validator.validate(checkMain: true); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + }); + + test( + 'rejects non-contiguous multiple new RFCs following baseBranch', + () async { + await fs.file('rfc/110.0043-feature-c.md').writeAsString('''--- +type: rfc +rfc: '110.0043' +title: Feature C +--- +'''); + await fs.file('rfc/110.0045-feature-e.md').writeAsString('''--- +type: rfc +rfc: '110.0045' +title: Feature E +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0042-feature-b.md', + }, + ); + + final result = await validator.validate(checkMain: true); + expect(result.isValid, isFalse); + expect(result.errors, hasLength(1)); + expect( + result.errors.first.message, + contains( + 'RFC number "110.0045" creates a numbering gap. ' + 'Expected next sequential number is "110.0044" (the latest RFC is "110.0042" in origin/main).', + ), + ); + }, + ); + + test( + 'allows modifying existing RFC from baseBranch without gap error', + () async { + await fs.file('rfc/110.0042-feature-b.md').writeAsString('''--- +type: rfc +rfc: '110.0042' +title: Feature B Updated +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0001-feature-a.md', + 'rfc/110.0042-feature-b.md', + }, + ); + + final result = await validator.validate(checkMain: true); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + }, + ); + + test('rejects internal gap when checkMain is false', () async { + await fs.file('rfc/110.0001-feature-a.md').writeAsString('''--- +type: rfc +rfc: '110.0001' +title: Feature A +--- +'''); + await fs.file('rfc/110.0003-feature-c.md').writeAsString('''--- +type: rfc +rfc: '110.0003' +title: Feature C +--- +'''); + + final validator = RfcValidator(fs: fs); + final result = await validator.validate(checkMain: false); + expect(result.isValid, isFalse); + expect( + result.errors.any( + (e) => e.message.contains( + 'RFC number "110.0003" creates a numbering gap. ' + 'Expected next sequential number is "110.0002".', + ), + ), + isTrue, + ); + }); + + test( + 'rejects tree starting with index > 0001 when checkMain is false', + () async { + await fs.file('rfc/110.0005-feature.md').writeAsString('''--- +type: rfc +rfc: '110.0005' +title: Feature +--- +'''); + + final validator = RfcValidator(fs: fs); + final result = await validator.validate(checkMain: false); + expect(result.isValid, isFalse); + expect( + result.errors.any( + (e) => e.message.contains( + 'RFC number "110.0005" creates a numbering gap. ' + 'Expected next sequential number is "110.0001".', + ), + ), + isTrue, + ); + }, + ); + + test('skips draft .0000 RFCs during gap validation', () async { + await fs.file('rfc/110.0000-unassigned-draft.md').writeAsString('''--- +type: rfc +rfc: '110.0000' +title: Draft +--- +'''); + + final validator = RfcValidator( + fs: fs, + gitList: ({String baseBranch = 'origin/main'}) async => { + 'rfc/110.0042-feature.md', + }, + ); + + final result = await validator.validate( + checkMain: true, + noDrafts: false, + ); + expect(result.isValid, isTrue); + expect(result.errors, isEmpty); + }); + }); + + group('ValidationError', () { + test( + 'toGithubAnnotation percent-encodes newlines and special characters', + () { + const error = ValidationError( + filePath: 'rfc/110.0001-feature.md', + message: 'First line\nSecond line 100%', + ); + + final annotation = error.toGithubAnnotation(); + expect(annotation.contains('\n'), isFalse); + expect( + annotation, + equals( + '::error file=rfc/110.0001-feature.md::First line%0ASecond line 100%25', + ), + ); + }, + ); + }); + }); +} From 7f0cc108af23b5753b745e1da13d3285652c83f2 Mon Sep 17 00:00:00 2001 From: John McDole Date: Fri, 4 Sep 2026 12:32:57 -0700 Subject: [PATCH 2/4] your rights have been unreserved --- bin/validate_rfc_number.dart | 2 +- lib/src/validator.dart | 2 +- test/validate_rfc_number_test.dart | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/validate_rfc_number.dart b/bin/validate_rfc_number.dart index 4712a53..2c4f96a 100644 --- a/bin/validate_rfc_number.dart +++ b/bin/validate_rfc_number.dart @@ -1,4 +1,4 @@ -// Copyright 2026 The Flutter Authors. All rights reserved. +// Copyright 2026 The Flutter Authors. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/lib/src/validator.dart b/lib/src/validator.dart index f13655c..532759b 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -1,4 +1,4 @@ -// Copyright 2026 The Flutter Authors. All rights reserved. +// Copyright 2026 The Flutter Authors. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/test/validate_rfc_number_test.dart b/test/validate_rfc_number_test.dart index d07f40d..3fd6705 100644 --- a/test/validate_rfc_number_test.dart +++ b/test/validate_rfc_number_test.dart @@ -1,4 +1,4 @@ -// Copyright 2026 The Flutter Authors. All rights reserved. +// Copyright 2026 The Flutter Authors. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. From 186090b7833932b30fe9ef056eae1b0dd330887f Mon Sep 17 00:00:00 2001 From: John McDole Date: Fri, 4 Sep 2026 16:04:38 -0700 Subject: [PATCH 3/4] github annotation and validator cleanup --- lib/src/github_annotation.dart | 53 +++++++++ lib/src/linter.dart | 16 +-- lib/src/validator.dart | 129 ++++++++++------------ test/github_annotation_test.dart | 184 +++++++++++++++++++++++++++++++ 4 files changed, 304 insertions(+), 78 deletions(-) create mode 100644 lib/src/github_annotation.dart create mode 100644 test/github_annotation_test.dart diff --git a/lib/src/github_annotation.dart b/lib/src/github_annotation.dart new file mode 100644 index 0000000..96df73c --- /dev/null +++ b/lib/src/github_annotation.dart @@ -0,0 +1,53 @@ +// Copyright 2026 The Flutter Authors. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// Interface for objects that can be formatted as GitHub Actions workflow annotations. +abstract interface class GithubAnnotatable { + /// Formats this object as a GitHub Actions workflow annotation string. + String toGithubAnnotation(); +} + +/// Extension on [String] for GitHub Actions workflow command formatting. +extension GithubAnnotationExtension on String { + /// Encodes special characters (`%`, `\r`, `\n`) in this string per GitHub Actions + /// workflow command specifications so multiline formatting is preserved. + String toGithubWorkflowValue() { + return replaceAll( + '%', + '%25', + ).replaceAll('\r', '%0D').replaceAll('\n', '%0A'); + } + + /// Formats this string message as a GitHub Actions workflow annotation. + /// + /// Example: + /// ```dart + /// 'File not found'.toGithubAnnotation(filePath: 'rfc/110.0001.md'); + /// => '::error file=rfc/110.0001.md::File not found' + /// + /// 'Syntax error'.toGithubAnnotation( + /// filePath: 'rfc/110.0001.md', + /// line: 12, + /// column: 4, + /// ); + /// => '::error file=rfc/110.0001.md,line=12,col=4::Syntax error' + /// ``` + String toGithubAnnotation({ + required String filePath, + int? line, + int? column, + String type = 'error', + String? title, + }) { + final encoded = toGithubWorkflowValue(); + final params = [ + 'file=$filePath', + if (line != null) 'line=$line', + if (column != null) 'col=$column', + if (title != null) 'title=$title', + ].join(','); + + return '::$type $params::$encoded'; + } +} diff --git a/lib/src/linter.dart b/lib/src/linter.dart index a9d6fa2..b3c42e1 100644 --- a/lib/src/linter.dart +++ b/lib/src/linter.dart @@ -5,12 +5,13 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; +import 'github_annotation.dart'; import 'github_client.dart'; import 'models/rfc_file.dart'; import 'taxonomy.dart'; /// A lint issue discovered in an RFC document. -class LintIssue { +class LintIssue implements GithubAnnotatable { final String filePath; final int line; final int column; @@ -27,13 +28,12 @@ class LintIssue { /// /// Percent-encodes special characters (%, \r, \n) per GitHub Actions workflow /// command specifications so multiline schema templates are preserved cleanly. - String toGithubAnnotation() { - final encoded = message - .replaceAll('%', '%25') - .replaceAll('\r', '%0D') - .replaceAll('\n', '%0A'); - return '::error file=$filePath,line=$line,col=$column::$encoded'; - } + @override + String toGithubAnnotation() => message.toGithubAnnotation( + filePath: filePath, + line: line, + column: column, + ); @override String toString() => '$filePath:$line:$column: $message'; diff --git a/lib/src/validator.dart b/lib/src/validator.dart index 532759b..8ebde28 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -7,31 +7,46 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; import 'git_lister.dart'; +import 'github_annotation.dart'; import 'models/rfc_file.dart'; export 'git_lister.dart' show GitListFunction, defaultGitList; /// A validation error encountered during semantic RFC validation. -class ValidationError { +class ValidationError implements GithubAnnotatable { final String filePath; final String message; + final int? line; + final int? column; - const ValidationError({required this.filePath, required this.message}); + const ValidationError({ + required this.filePath, + required this.message, + this.line, + this.column, + }); /// Formats the error as a GitHub Actions workflow annotation. /// /// Percent-encodes special characters (%, \r, \n) per GitHub Actions workflow /// command specifications. - String toGithubAnnotation() { - final encoded = message - .replaceAll('%', '%25') - .replaceAll('\r', '%0D') - .replaceAll('\n', '%0A'); - return '::error file=$filePath::$encoded'; - } + @override + String toGithubAnnotation() => message.toGithubAnnotation( + filePath: filePath, + line: line, + column: column, + ); @override - String toString() => '$filePath: $message'; + String toString() { + if (line != null) { + if (column != null) { + return '$filePath:$line:$column: $message'; + } + return '$filePath:$line: $message'; + } + return '$filePath: $message'; + } } /// Result of RFC semantic number validation. @@ -199,16 +214,15 @@ class RfcValidator { baseBranchCategory: baseBranchCategory, errors: errors, ); - _checkBaseBranchGaps( - category: category, - rfcs: rfcs, - baseBranch: baseBranch, - baseBranchCategory: baseBranchCategory, - errors: errors, - ); - } else { - _checkWorkingTreeGaps(category: category, rfcs: rfcs, errors: errors); } + + _checkGaps( + category: category, + rfcs: rfcs, + baseBranch: checkMain ? baseBranch : null, + baseBranchCategory: checkMain ? baseBranchCategory : null, + errors: errors, + ); } /// Discovers and indexes RFC files on [baseBranch] grouped by category `AAA`. @@ -292,40 +306,47 @@ class RfcValidator { } } - /// Ensures newly introduced RFCs in [category] are strictly sequential - /// following the latest RFC on [baseBranch] without gaps. - void _checkBaseBranchGaps({ + /// Ensures assigned RFC numbers in [category] are strictly sequential without gaps. + /// + /// When [baseBranch] is provided, validates that new RFCs follow the latest landed RFC + /// on [baseBranch]. When omitted, validates that working tree RFCs start at 0001 and + /// are contiguous. + void _checkGaps({ required String category, required List rfcs, - required String baseBranch, - required _BaseBranchCategory? baseBranchCategory, required List errors, + String? baseBranch, + _BaseBranchCategory? baseBranchCategory, }) { - final newRfcs = [ + final candidateRfcs = [ for (var rfc in rfcs) if (!rfc.isDraft && rfc.index != null && - baseBranchCategory?.rfcIdToBasename[rfc.rfcId] == null) + (baseBranchCategory == null || + baseBranchCategory.rfcIdToBasename[rfc.rfcId] == null)) rfc, ]..sort((a, b) => a.index!.compareTo(b.index!)); - final maxOnMain = baseBranchCategory?.maxIndex ?? 0; - var expectedNext = maxOnMain + 1; - - for (final newRfc in newRfcs) { - final actualIndex = newRfc.index!; + final maxBase = baseBranch != null + ? (baseBranchCategory?.maxIndex ?? 0) + : 0; + final contextSuffix = baseBranch != null + ? (maxBase == 0 + ? ' (no RFCs exist in category "$category" in $baseBranch)' + : ' (the latest RFC is "$category.${maxBase.toNNNN()}" in $baseBranch)') + : ''; + + var expectedNext = maxBase + 1; + for (final rfc in candidateRfcs) { + final actualIndex = rfc.index!; if (actualIndex > expectedNext) { final expectedPadded = expectedNext.toNNNN(); - final maxPadded = maxOnMain.toNNNN(); - final mainContext = maxOnMain == 0 - ? 'no RFCs exist in category "$category"' - : 'the latest RFC is "$category.$maxPadded"'; errors.add( ValidationError( - filePath: newRfc.path, + filePath: rfc.path, message: - 'RFC number "${newRfc.rfcId}" creates a numbering gap. ' - 'Expected next sequential number is "$category.$expectedPadded" ($mainContext in $baseBranch).', + 'RFC number "${rfc.rfcId}" creates a numbering gap. ' + 'Expected next sequential number is "$category.$expectedPadded"$contextSuffix.', ), ); expectedNext = actualIndex + 1; @@ -334,38 +355,6 @@ class RfcValidator { } } } - - /// Ensures assigned RFC numbers in [category] start at 0001 and are - /// strictly contiguous within the working tree. - void _checkWorkingTreeGaps({ - required String category, - required List rfcs, - required List errors, - }) { - final nonDraftRfcs = [ - for (var rfc in rfcs) - if (!rfc.isDraft && rfc.index != null) rfc, - ]..sort((a, b) => a.index!.compareTo(b.index!)); - - var expectedIndex = 1; - for (final rfc in nonDraftRfcs) { - final actualIndex = rfc.index!; - if (actualIndex > expectedIndex) { - final expectedPadded = expectedIndex.toNNNN(); - errors.add( - ValidationError( - filePath: rfc.path, - message: - 'RFC number "${rfc.rfcId}" creates a numbering gap. ' - 'Expected next sequential number is "$category.$expectedPadded".', - ), - ); - expectedIndex = actualIndex + 1; - } else if (actualIndex == expectedIndex) { - expectedIndex++; - } - } - } } /// Discovered base branch RFC files and metadata for a category `AAA`. diff --git a/test/github_annotation_test.dart b/test/github_annotation_test.dart new file mode 100644 index 0000000..53eeb88 --- /dev/null +++ b/test/github_annotation_test.dart @@ -0,0 +1,184 @@ +// Copyright 2026 The Flutter Authors. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:rfc_tools/src/github_annotation.dart'; +import 'package:rfc_tools/src/linter.dart'; +import 'package:rfc_tools/src/validator.dart'; +import 'package:test/test.dart'; + +void main() { + group('GithubAnnotationExtension on String', () { + group('toGithubWorkflowValue', () { + test('leaves clean strings unchanged', () { + expect( + 'Simple message'.toGithubWorkflowValue(), + equals('Simple message'), + ); + }); + + test('percent-encodes % as %25', () { + expect( + 'Progress: 100%'.toGithubWorkflowValue(), + equals('Progress: 100%25'), + ); + }); + + test('percent-encodes \\n as %0A', () { + expect( + 'Line 1\nLine 2'.toGithubWorkflowValue(), + equals('Line 1%0ALine 2'), + ); + }); + + test('percent-encodes \\r as %0D', () { + expect( + 'Line 1\rLine 2'.toGithubWorkflowValue(), + equals('Line 1%0DLine 2'), + ); + }); + + test('percent-encodes combination of %, \\r, and \\n', () { + const input = '100% complete\r\nNext line: 50%'; + expect( + input.toGithubWorkflowValue(), + equals('100%25 complete%0D%0ANext line: 50%25'), + ); + }); + }); + + group('toGithubAnnotation', () { + test('formats file-only annotation', () { + final annotation = 'File error'.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + ); + expect( + annotation, + equals('::error file=rfc/000.0001-sample.md::File error'), + ); + }); + + test('formats file + line + col annotation', () { + final annotation = 'Syntax error'.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + line: 15, + column: 3, + ); + expect( + annotation, + equals( + '::error file=rfc/000.0001-sample.md,line=15,col=3::Syntax error', + ), + ); + }); + + test('formats file + line only annotation', () { + final annotation = 'Line error'.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + line: 20, + ); + expect( + annotation, + equals('::error file=rfc/000.0001-sample.md,line=20::Line error'), + ); + }); + + test('formats custom annotation type (warning, notice)', () { + final warning = 'Warning message'.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + type: 'warning', + ); + expect( + warning, + equals('::warning file=rfc/000.0001-sample.md::Warning message'), + ); + + final notice = 'Notice message'.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + type: 'notice', + ); + expect( + notice, + equals('::notice file=rfc/000.0001-sample.md::Notice message'), + ); + }); + + test('formats title parameter when provided', () { + final annotation = 'Schema violation'.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + title: 'BadFrontmatter', + ); + expect( + annotation, + equals( + '::error file=rfc/000.0001-sample.md,title=BadFrontmatter::Schema violation', + ), + ); + }); + + test('percent-encodes multiline messages in annotation output', () { + const multiline = 'Line 1\nLine 2 100%'; + final annotation = multiline.toGithubAnnotation( + filePath: 'rfc/000.0001-sample.md', + line: 5, + column: 1, + ); + expect(annotation.contains('\n'), isFalse); + expect(annotation.contains('\r'), isFalse); + expect( + annotation, + equals( + '::error file=rfc/000.0001-sample.md,line=5,col=1::Line 1%0ALine 2 100%25', + ), + ); + }); + }); + }); + + group('GithubAnnotatable interface conformance', () { + test('LintIssue implements GithubAnnotatable', () { + const issue = LintIssue( + filePath: 'rfc/110.0000-draft.md', + line: 2, + column: 1, + message: 'Invalid type', + ); + expect(issue, isA()); + expect( + issue.toGithubAnnotation(), + equals('::error file=rfc/110.0000-draft.md,line=2,col=1::Invalid type'), + ); + }); + + test('ValidationError implements GithubAnnotatable', () { + const error = ValidationError( + filePath: 'rfc/110.0001-feature.md', + message: 'Collision detected', + ); + expect(error, isA()); + expect( + error.toGithubAnnotation(), + equals('::error file=rfc/110.0001-feature.md::Collision detected'), + ); + }); + + test('ValidationError supports optional line and column', () { + const error = ValidationError( + filePath: 'rfc/110.0001-feature.md', + line: 42, + column: 5, + message: 'Positioned error', + ); + expect( + error.toGithubAnnotation(), + equals( + '::error file=rfc/110.0001-feature.md,line=42,col=5::Positioned error', + ), + ); + expect( + error.toString(), + equals('rfc/110.0001-feature.md:42:5: Positioned error'), + ); + }); + }); +} From 66c1dccc1ea2d0e9d5ed4f77b20ee392bf4def25 Mon Sep 17 00:00:00 2001 From: John McDole Date: Fri, 4 Sep 2026 16:35:20 -0700 Subject: [PATCH 4/4] refactor(rfc_tools): standardize validator exit codes and add RfcFile.fromPath --- bin/validate_rfc_number.dart | 9 +++++---- lib/src/models/rfc_file.dart | 30 ++++++++++++++++++++++++++++++ lib/src/validator.dart | 2 +- test/rfc_file_test.dart | 29 +++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/bin/validate_rfc_number.dart b/bin/validate_rfc_number.dart index 2c4f96a..0177e58 100644 --- a/bin/validate_rfc_number.dart +++ b/bin/validate_rfc_number.dart @@ -44,13 +44,14 @@ void main(List arguments) async { } catch (e) { stderr.writeln('Error parsing arguments: $e\n'); stderr.writeln(parser.usage); - exit(1); + exitCode = 1; + return; } if (results.flag('help')) { stdout.writeln('RFC Semantic Validator - Flutter RFC Repository Tooling\n'); stdout.writeln(parser.usage); - exit(0); + return; } final checkMain = results.flag('check-main'); @@ -76,11 +77,11 @@ void main(List arguments) async { stderr.writeln('[ERROR] $error'); } } - exit(1); + exitCode = 1; + return; } stdout.writeln( 'RFC numbers validated cleanly. No collisions or illegal drafts found.', ); - exit(0); } diff --git a/lib/src/models/rfc_file.dart b/lib/src/models/rfc_file.dart index 75b5742..e00f718 100644 --- a/lib/src/models/rfc_file.dart +++ b/lib/src/models/rfc_file.dart @@ -97,6 +97,31 @@ class RfcFile { required this.headingError, }); + /// Creates an [RfcFile] from a file path with parsed filename components, + /// without reading or parsing markdown content or frontmatter. + /// + /// Useful for lightweight filename-based validation. + factory RfcFile.fromPath(String path) { + final parsed = _parseFilename(path); + return RfcFile._( + path: path, + category: parsed.category, + index: parsed.index, + slug: parsed.slug, + hasFrontmatter: false, + frontmatterRaw: '', + frontmatter: null, + frontmatterError: null, + frontmatterErrors: const [], + body: '', + firstHeading: null, + firstHeadingId: null, + firstHeadingTitle: null, + firstHeadingLine: null, + headingError: null, + ); + } + /// Regular expression for RFC filenames: `AAA.NNNN-.md`. static final RegExp filenamePattern = RegExp( r'^(\d{3})\.(\d{4})-([a-z0-9]+(?:-[a-z0-9]+)*)\.md$', @@ -134,6 +159,11 @@ class RfcFile { bool get hasValidHeading => headingError == null && firstHeading != null && firstHeadingId != null; + /// Extracts category, index, and slug from an RFC file path. + static ({String? category, int? index, String? slug}) parseFilename( + String path, + ) => _parseFilename(path); + /// Extracts category, index, and slug from an RFC file path. static ({String? category, int? index, String? slug}) _parseFilename( String path, diff --git a/lib/src/validator.dart b/lib/src/validator.dart index 8ebde28..330a752 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -119,7 +119,7 @@ class RfcValidator { final rfcsByCategory = >{}; for (final entry in entries) { if (entry is File && entry.path.endsWith('.md')) { - final rfc = RfcFile.parse('', path: entry.path); + final rfc = RfcFile.fromPath(entry.path); _validateFileStructure( rfc: rfc, diff --git a/test/rfc_file_test.dart b/test/rfc_file_test.dart index b41b910..ba2666b 100644 --- a/test/rfc_file_test.dart +++ b/test/rfc_file_test.dart @@ -83,6 +83,35 @@ Some markdown text. expect(rfc.firstHeadingTitle, equals('Extract Value Notifier')); }); + test( + 'RfcFile.fromPath parses filename components without reading content', + () { + final rfc = RfcFile.fromPath('rfc/110.0001-extract-value-notifier.md'); + expect(rfc.hasValidFilename, isTrue); + expect(rfc.category, equals('110')); + expect(rfc.index, equals(1)); + expect(rfc.slug, equals('extract-value-notifier')); + expect(rfc.rfcId, equals('110.0001')); + expect(rfc.isDraft, isFalse); + expect(rfc.hasFrontmatter, isFalse); + expect(rfc.frontmatter, isNull); + expect(rfc.body, isEmpty); + expect(rfc.firstHeading, isNull); + }, + ); + + test('RfcFile.parseFilename extracts filename components', () { + final parsed = RfcFile.parseFilename('rfc/000.0002-review-process.md'); + expect(parsed.category, equals('000')); + expect(parsed.index, equals(2)); + expect(parsed.slug, equals('review-process')); + + final invalid = RfcFile.parseFilename('rfc/README.md'); + expect(invalid.category, isNull); + expect(invalid.index, isNull); + expect(invalid.slug, isNull); + }); + test('parses email authors into typed RfcAuthor list', () { const emailSample = '''--- type: rfc