-
Notifications
You must be signed in to change notification settings - Fork 1
feat(rfc_tools): implement RFC linter CLI #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| // 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 'dart:io'; | ||
| import 'package:args/args.dart'; | ||
| import 'package:file/local.dart'; | ||
| import 'package:rfc_tools/src/git_lister.dart'; | ||
| import 'package:rfc_tools/src/github_client.dart'; | ||
| import 'package:rfc_tools/src/linter.dart'; | ||
| import 'package:rfc_tools/src/taxonomy.dart'; | ||
|
|
||
| void main(List<String> arguments) async { | ||
| final parser = ArgParser() | ||
| ..addMultiOption( | ||
| 'labels', | ||
| help: 'Comma-separated list of GitHub Pull Request labels.', | ||
| ) | ||
| ..addFlag( | ||
| 'enforce-drafts', | ||
| negatable: false, | ||
| help: | ||
| 'Enforce that RFCs under review must use ".0000" unless labeled with "rfc-ready" or "rfc-assigned".', | ||
| ) | ||
| ..addFlag( | ||
| 'validate-github-users', | ||
| negatable: false, | ||
| help: 'Verify that GitHub profile authors exist via the GitHub API.', | ||
| ) | ||
| ..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); | ||
| exitCode = 1; | ||
| return; | ||
| } | ||
|
|
||
| if (results.flag('help')) { | ||
| stdout.writeln('RFC Linter - Flutter RFC Repository Tooling\n'); | ||
| stdout.writeln(parser.usage); | ||
| return; | ||
| } | ||
|
|
||
| final enforceDrafts = results.flag('enforce-drafts'); | ||
| final validateGitHubUsers = results.flag('validate-github-users'); | ||
| final githubActions = results.flag('github-actions'); | ||
|
|
||
| final labels = <String>{ | ||
| for (var label in results.multiOption('labels')) | ||
| if (label.trim() case final trimmed when trimmed.isNotEmpty) trimmed, | ||
| }; | ||
|
|
||
| const fs = LocalFileSystem(); | ||
| const gh = CliGitHubClient(); | ||
|
|
||
| Taxonomy taxonomy; | ||
| try { | ||
| taxonomy = await Taxonomy.load(fs); | ||
| } catch (e) { | ||
| stderr.writeln('Failed to load taxonomy: $e'); | ||
| exitCode = 1; | ||
| return; | ||
| } | ||
|
|
||
| final filesOnMain = await defaultGitList(baseBranch: 'origin/main'); | ||
|
|
||
| final linter = RfcLinter( | ||
| fs: fs, | ||
| gh: gh, | ||
| taxonomy: taxonomy, | ||
| labels: labels, | ||
| validateGitHubUsers: validateGitHubUsers, | ||
| existingFilesOnMain: filesOnMain, | ||
| enforceDrafts: enforceDrafts, | ||
| ); | ||
|
|
||
| final issues = <LintIssue>[]; | ||
| if (results.rest.isNotEmpty) { | ||
| for (final path in results.rest) { | ||
| issues.addAll(await linter.lintFile(fs.file(path))); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If someone passes a directory, instead of a specific file, this will come back false. Do we need to handle that scenario by checking fs.isDirectorySync(path) and then passing on to linter.lintDirectory(fs.directory(path))? |
||
| } | ||
| } else { | ||
| issues.addAll(await linter.lintDirectory(fs.directory('rfc'))); | ||
| } | ||
|
|
||
| if (issues.isNotEmpty) { | ||
| stderr.writeln('RFC Lint failed with ${issues.length} issue(s):\n'); | ||
| for (final issue in issues) { | ||
| if (githubActions) { | ||
| stderr.writeln(issue.toGithubAnnotation()); | ||
| } else { | ||
| stderr.writeln('[ERROR] $issue'); | ||
| } | ||
| } | ||
| exitCode = 1; | ||
| return; | ||
| } | ||
|
|
||
| stdout.writeln('All RFC documents passed lint checks cleanly.'); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,262 @@ | ||
| // 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:file/file.dart'; | ||
| import 'package:path/path.dart' as p; | ||
|
|
||
| import 'github_client.dart'; | ||
| import 'models/rfc_file.dart'; | ||
| import 'taxonomy.dart'; | ||
|
|
||
| /// A lint issue discovered in an RFC document. | ||
| class LintIssue { | ||
| final String filePath; | ||
| final int line; | ||
| final int column; | ||
| final String message; | ||
|
|
||
| const LintIssue({ | ||
| required this.filePath, | ||
| required this.message, | ||
| this.line = 1, | ||
| this.column = 1, | ||
| }); | ||
|
|
||
| /// Formats the issue as a GitHub Actions workflow annotation. | ||
| /// | ||
| /// 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 toString() => '$filePath:$line:$column: $message'; | ||
| } | ||
|
|
||
| /// Linter enforcing RFC structure, metadata, taxonomy, and number allocation rules. | ||
| class RfcLinter { | ||
| final FileSystem fs; | ||
| final GitHubClient gh; | ||
| final Taxonomy taxonomy; | ||
| final Set<String> labels; | ||
| final bool validateGitHubUsers; | ||
| final Set<String> existingFilesOnMain; | ||
| final bool enforceDrafts; | ||
|
|
||
| RfcLinter({ | ||
| required this.fs, | ||
| required this.gh, | ||
| required this.taxonomy, | ||
| this.labels = const <String>{}, | ||
| this.validateGitHubUsers = false, | ||
| this.existingFilesOnMain = const <String>{}, | ||
| this.enforceDrafts = false, | ||
| }); | ||
|
|
||
| /// Lints a single RFC file. | ||
| Future<List<LintIssue>> lintFile(File file) async { | ||
| final issues = <LintIssue>[]; | ||
| final relativePath = file.path; | ||
|
|
||
| if (!await file.exists()) { | ||
| issues.add(LintIssue(filePath: relativePath, message: 'File not found.')); | ||
| return issues; | ||
| } | ||
|
|
||
| final content = await file.readAsString(); | ||
| final rfc = RfcFile.parse(content, path: file.path); | ||
| final fileName = p.basename(file.path); | ||
|
|
||
| // 1. Filename & Path Validation | ||
| if (!rfc.hasValidFilename) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 1, | ||
| message: | ||
| 'Filename "$fileName" does not match required format "AAA.NNNN-<slug>.md" ' | ||
| '(where AAA is 3 digits, NNNN is 4 digits, and slug is lowercase kebab-case).', | ||
| ), | ||
| ); | ||
| return issues; // Cannot perform further structural checks reliably | ||
| } | ||
|
|
||
| // 2. Taxonomy Validation | ||
| if (!taxonomy.isValidCategory(rfc.category!)) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 1, | ||
| message: | ||
| 'Subsystem category "${rfc.category}" is not defined in the architecture taxonomy. ' | ||
| 'See rfc/000.0001-flutter-architecture-and-reference-taxonomy.md.', | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| // 3. Draft vs Assigned Number Enforcement (PR Context) | ||
| if (enforceDrafts) { | ||
| final existingBasenames = existingFilesOnMain.map(p.basename).toSet(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we declare this as late final above rather than recreating it for each file? |
||
| final isExistingOnMain = existingBasenames.contains(fileName); | ||
| const bootstrapRfcs = {'000.0001', '000.0002'}; | ||
| final isBootstrap = bootstrapRfcs.contains(rfc.rfcId); | ||
|
|
||
| if (!rfc.isDraft && !isExistingOnMain && !isBootstrap) { | ||
| final hasReadyOrAssigned = | ||
| labels.contains('rfc-ready') || labels.contains('rfc-assigned'); | ||
| if (!hasReadyOrAssigned) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 1, | ||
| message: | ||
| 'RFC has assigned number "${rfc.rfcId}", but PR does not have ' | ||
| '"rfc-ready" or "rfc-assigned" label. RFCs under review must use index "0000".', | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 4. YAML Frontmatter Validation | ||
| if (!rfc.hasFrontmatter) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 1, | ||
| message: | ||
| '${rfc.frontmatterError ?? 'Missing YAML frontmatter block.'}\n\n' | ||
| 'Expected frontmatter format:\n${RfcFrontmatter.expectedSchemaTemplate.trimRight()}', | ||
| ), | ||
| ); | ||
| return issues; | ||
| } | ||
|
|
||
| if (rfc.frontmatterError != null) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 1, | ||
| message: | ||
| '${rfc.frontmatterError!}\n\n' | ||
| 'Expected frontmatter format:\n${RfcFrontmatter.expectedSchemaTemplate.trimRight()}', | ||
| ), | ||
| ); | ||
| return issues; | ||
| } | ||
|
|
||
| if (rfc.frontmatterErrors.isNotEmpty) { | ||
| for (final err in rfc.frontmatterErrors) { | ||
| issues.add(LintIssue(filePath: relativePath, line: 2, message: err)); | ||
| } | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 2, | ||
| message: | ||
| 'Expected frontmatter format:\n${RfcFrontmatter.expectedSchemaTemplate.trimRight()}', | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| final fm = rfc.frontmatter; | ||
| final rfcId = fm?.rfc; | ||
| final expectedId = rfc.rfcId; | ||
|
|
||
| if (rfcId != null && rfcId != expectedId) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 2, | ||
| message: | ||
| 'Frontmatter "rfc" value ("$rfcId") does not match filename identifier ("$expectedId").', | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| // GitHub author existence verification (if enabled) | ||
| if (validateGitHubUsers && fm != null) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we cache validated authors so we are not making repeated calls to gh api users/$username for folks we arlready know about? |
||
| for (final author in fm.authors) { | ||
| if (author is GitHubAuthor) { | ||
| final exists = await gh.userExists(author.username); | ||
| if (!exists) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 2, | ||
| message: 'GitHub user "${author.username}" does not exist.', | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 5. First Heading Validation | ||
| if (rfc.firstHeading == null) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: 1, | ||
| message: | ||
| 'Document must contain a top-level heading matching "# RFC ${rfc.rfcId}: <Title>".', | ||
| ), | ||
| ); | ||
| } else { | ||
| if (rfc.firstHeadingId != expectedId) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: rfc.firstHeadingLine ?? 1, | ||
| message: | ||
| 'First heading RFC identifier ("${rfc.firstHeadingId}") does not match "$expectedId".', | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| final fmTitle = fm?.title.trim(); | ||
| if (fmTitle != null && | ||
| fmTitle.isNotEmpty && | ||
| rfc.firstHeadingTitle != fmTitle) { | ||
| issues.add( | ||
| LintIssue( | ||
| filePath: relativePath, | ||
| line: rfc.firstHeadingLine ?? 1, | ||
| message: | ||
| 'First heading title ("${rfc.firstHeadingTitle}") does not match frontmatter title ("$fmTitle").', | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return issues; | ||
| } | ||
|
|
||
| /// Lints all RFC markdown files in the specified directory. | ||
| Future<List<LintIssue>> lintDirectory(Directory dir) async { | ||
| final issues = <LintIssue>[]; | ||
| if (!await dir.exists()) { | ||
| issues.add( | ||
| LintIssue(filePath: dir.path, message: 'Directory does not exist.'), | ||
| ); | ||
| return issues; | ||
| } | ||
|
|
||
| final entries = await dir.list().toList(); | ||
| entries.sort((a, b) => a.path.compareTo(b.path)); | ||
|
|
||
| for (final entry in entries) { | ||
| if (entry is File && entry.path.endsWith('.md')) { | ||
| issues.addAll(await lintFile(entry)); | ||
| } | ||
| } | ||
|
|
||
| return issues; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if I might be getting scrambled across stacked PRs. 🙃 In #10 in bin/validate_rfc_number.dart --base-branch was added so users can specify upstream/main or custom branches. In bin/rfc_lint.dart, the branch is hardcoded to 'origin/main' - is this a bug?