Add a test to verify that we only link to real issue templates (#93528)
diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index a642f49..794d5b1 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart
@@ -51,12 +51,15 @@ exitWithError(<String>['The analyze.dart script must be run with --enable-asserts.']); } - print('$clock runtimeType in toString...'); + print('$clock No runtimeType in toString...'); await verifyNoRuntimeTypeInToString(flutterRoot); - print('$clock debug mode instead of checked mode...'); + print('$clock Debug mode instead of checked mode...'); await verifyNoCheckedMode(flutterRoot); + print('$clock Links for creating GitHub issues'); + await verifyIssueLinks(flutterRoot); + print('$clock Unexpected binaries...'); await verifyNoBinaries(flutterRoot); @@ -728,6 +731,81 @@ exitWithError(problems); } +String _bullets(String value) => ' * $value'; + +Future<void> verifyIssueLinks(String workingDirectory) async { + const String issueLinkPrefix = 'https://github.com/flutter/flutter/issues/new'; + const Set<String> stops = <String>{ '\n', ' ', "'", '"', r'\', ')', '>' }; + assert(!stops.contains('.')); // instead of "visit https://foo." say "visit: https://", it copy-pastes better + const String kGiveTemplates = + 'Prefer to provide a link either to $issueLinkPrefix/choose (the list of issue ' + 'templates) or to a specific template directly ($issueLinkPrefix?template=...).\n'; + final Set<String> templateNames = + Directory(path.join(workingDirectory, '.github', 'ISSUE_TEMPLATE')) + .listSync() + .whereType<File>() + .where((File file) => path.extension(file.path) == '.md') + .map<String>((File file) => path.basename(file.path)) + .toSet(); + final String kTemplates = 'The available templates are:\n${templateNames.map(_bullets).join("\n")}'; + final List<String> problems = <String>[]; + final Set<String> suggestions = <String>{}; + final List<File> files = await _gitFiles(workingDirectory); + for (final File file in files) { + if (path.basename(file.path).endsWith('_test.dart')) + continue; // Skip tests, they're not public-facing. + final Uint8List bytes = file.readAsBytesSync(); + // We allow invalid UTF-8 here so that binaries don't trip us up. + // There's a separate test in this file that verifies that all text + // files are actually valid UTF-8 (see verifyNoBinaries below). + final String contents = utf8.decode(bytes, allowMalformed: true); + int start = 0; + while ((start = contents.indexOf(issueLinkPrefix, start)) >= 0) { + int end = start + issueLinkPrefix.length; + while (end < contents.length && !stops.contains(contents[end])) { + end += 1; + } + final String url = contents.substring(start, end); + if (url == issueLinkPrefix) { + if (file.path != path.join(workingDirectory, 'dev', 'bots', 'analyze.dart')) { + problems.add('${file.path} contains a direct link to $issueLinkPrefix.'); + suggestions.add(kGiveTemplates); + suggestions.add(kTemplates); + } + } else if (url.startsWith('$issueLinkPrefix?')) { + final Uri parsedUrl = Uri.parse(url); + final List<String>? templates = parsedUrl.queryParametersAll['template']; + if (templates == null) { + problems.add('${file.path} contains $url, which has no "template" argument specified.'); + suggestions.add(kGiveTemplates); + suggestions.add(kTemplates); + } else if (templates.length != 1) { + problems.add('${file.path} contains $url, which has ${templates.length} templates specified.'); + suggestions.add(kGiveTemplates); + suggestions.add(kTemplates); + } else if (!templateNames.contains(templates.single)) { + problems.add('${file.path} contains $url, which specifies a non-existent template ("${templates.single}").'); + suggestions.add(kTemplates); + } else if (parsedUrl.queryParametersAll.keys.length > 1) { + problems.add('${file.path} contains $url, which the analyze.dart script is not sure how to handle.'); + suggestions.add('Update analyze.dart to handle the URLs above, or change them to the expected pattern.'); + } + } else if (url != '$issueLinkPrefix/choose') { + problems.add('${file.path} contains $url, which the analyze.dart script is not sure how to handle.'); + suggestions.add('Update analyze.dart to handle the URLs above, or change them to the expected pattern.'); + } + start = end; + } + } + assert(problems.isEmpty == suggestions.isEmpty); + if (problems.isNotEmpty) { + exitWithError(<String>[ + ...problems, + ...suggestions, + ]); + } +} + @immutable class Hash256 { const Hash256(this.a, this.b, this.c, this.d); @@ -1163,7 +1241,7 @@ ); legacyBinaries ??= _legacyBinaries; if (!Platform.isWindows) { // TODO(ianh): Port this to Windows - final List<File> files = await _gitFiles(workingDirectory, runSilently: false); + final List<File> files = await _gitFiles(workingDirectory); final List<String> problems = <String>[]; for (final File file in files) { final Uint8List bytes = file.readAsBytesSync();
diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart index 04e6a4b..5ba5ff4 100644 --- a/dev/bots/test/analyze_test.dart +++ b/dev/bots/test/analyze_test.dart
@@ -156,9 +156,7 @@ legacyBinaries: <Hash256>{const Hash256(0x39A050CD69434936, 0, 0, 0)}, ), exitCode: Platform.isWindows ? 0 : 1); if (!Platform.isWindows) { - // The output starts with the call to git ls-files, the details of which - // change from run to run, so we only check the trailing end of the output. - expect(result, endsWith('\n' + expect(result, '━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n' 'test/analyze-test-input/root/packages/foo/serviceaccount.enc:0: file is not valid UTF-8\n' 'All files in this repository must be UTF-8. In particular, images and other binaries\n' @@ -167,7 +165,7 @@ 'to which you need access, you should consider how to fetch it from another repository;\n' 'for example, the "assets-for-api-docs" repository is used for images in API docs.\n' '━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n' - )); + ); } });