From f5a983535131d948950124c53df6b8f491debd9f Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 6 Nov 2023 10:46:18 -0800 Subject: [PATCH] Check sample links for malformed links (#137807) ## Description This checks API doc strings for malformed links to examples. It prevents errors in capitalization, spacing, number of asterisks, etc. It won't catch all errors, because it needs to have a minimally indicative string to know that it even is trying to be a link to an example. At a minimum, the line needs to look like (literally, not as a regexp) `///*seecode.*` in order to be seen as a link to an example. Separately, I'm going to add a check to the snippets tool that checks to make sure that an `{@tool}` block includes either a link to a sample file or a dart code block. ## Tests - Added a test to make sure it catches some malformed links. --- dev/bots/check_code_samples.dart | 69 +++++++++++++++++----- dev/bots/test/check_code_samples_test.dart | 56 ++++++++++++++++-- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/dev/bots/check_code_samples.dart b/dev/bots/check_code_samples.dart index 3be363b62b25f..e601c40db0e7c 100644 --- a/dev/bots/check_code_samples.dart +++ b/dev/bots/check_code_samples.dart @@ -97,6 +97,19 @@ void main(List args) { reportSuccessAndExit('All examples are linked and have tests.'); } +class LinkInfo { + const LinkInfo(this.link, this.file, this.line); + + final String link; + final File file; + final int line; + + @override + String toString() { + return '${file.path}:$line: $link'; + } +} + class SampleChecker { SampleChecker({ required this.examples, @@ -119,10 +132,12 @@ class SampleChecker { final List exampleFilenames = getExampleFilenames(examples); // Get a list of all the example link paths that appear in the source files. - final Set exampleLinks = getExampleLinks(packages); - + final (Set exampleLinks, Set malformedLinks) = getExampleLinks(packages); // Also add in any that might be found in the dart:ui directory. - exampleLinks.addAll(getExampleLinks(dartUIPath)); + final (Set uiExampleLinks, Set uiMalformedLinks) = getExampleLinks(dartUIPath); + + exampleLinks.addAll(uiExampleLinks); + malformedLinks.addAll(uiMalformedLinks); // Get a list of the filenames that were not found in the source files. final List missingFilenames = checkForMissingLinks(exampleFilenames, exampleLinks); @@ -136,7 +151,7 @@ class SampleChecker { // generate new examples. missingFilenames.removeWhere((String file) => _knownUnlinkedExamples.contains(file)); - if (missingFilenames.isEmpty && missingTests.isEmpty && noLongerMissing.isEmpty) { + if (missingFilenames.isEmpty && missingTests.isEmpty && noLongerMissing.isEmpty && malformedLinks.isEmpty) { return true; } @@ -167,6 +182,19 @@ class SampleChecker { buffer.write('Either link them to a source file API doc comment, or remove them.'); foundError(buffer.toString().split('\n')); } + + if (malformedLinks.isNotEmpty) { + final StringBuffer buffer = + StringBuffer('The following malformed links were found in API doc comments:\n'); + for (final LinkInfo link in malformedLinks) { + buffer.writeln(' $link'); + } + buffer.write( + 'Correct the formatting of these links so that they match the exact pattern:\n' + r" r'\*\* See code in (?.+) \*\*'" + ); + foundError(buffer.toString().split('\n')); + } return false; } @@ -199,21 +227,34 @@ class SampleChecker { ); } - Set getExampleLinks(Directory searchDirectory) { + (Set, Set) getExampleLinks(Directory searchDirectory) { final List files = getFiles(searchDirectory, RegExp(r'\.dart$')); final Set searchStrings = {}; - final RegExp exampleRe = RegExp(r'\*\* See code in (?.*) \*\*'); + final Set malformedStrings = {}; + final RegExp validExampleRe = RegExp(r'\*\* See code in (?.+) \*\*'); + // Looks for some common broken versions of example links. This looks for + // something that is at minimum "///*seecode*" to indicate that it + // looks like an example link. It should be narrowed if we start gettting false + // positives. + final RegExp malformedLinkRe = RegExp(r'^(?\s*///\s*\*\*?\s*[sS][eE][eE]\s*[Cc][Oo][Dd][Ee].+\*\*?)'); for (final File file in files) { final String contents = file.readAsStringSync(); - searchStrings.addAll( - contents.split('\n').where((String s) => s.contains(exampleRe)).map( - (String e) { - return exampleRe.firstMatch(e)!.namedGroup('path')!; - }, - ), - ); + final List lines = contents.split('\n'); + int count = 0; + for (final String line in lines) { + count += 1; + final RegExpMatch? validMatch = validExampleRe.firstMatch(line); + if (validMatch != null) { + searchStrings.add(validMatch.namedGroup('path')!); + } + final RegExpMatch? malformedMatch = malformedLinkRe.firstMatch(line); + // It's only malformed if it doesn't match the valid RegExp. + if (malformedMatch != null && validMatch == null) { + malformedStrings.add(LinkInfo(malformedMatch.namedGroup('malformed')!, file, count)); + } + } } - return searchStrings; + return (searchStrings, malformedStrings); } List checkForMissingLinks(List exampleFilenames, Set searchStrings) { diff --git a/dev/bots/test/check_code_samples_test.dart b/dev/bots/test/check_code_samples_test.dart index 2fcae906f37fa..1e748f0210db2 100644 --- a/dev/bots/test/check_code_samples_test.dart +++ b/dev/bots/test/check_code_samples_test.dart @@ -25,7 +25,8 @@ void main() { return path.relative(file.absolute.path, from: flutterRoot.absolute.path); } - void writeLink({required File source, required File example}) { + void writeLink({required File source, required File example, String? alternateLink}) { + final String link = alternateLink ?? ' ** See code in ${getRelativePath(example)} **'; source ..createSync(recursive: true) ..writeAsStringSync(''' @@ -34,12 +35,12 @@ void main() { /// {@tool dartpad} /// Example description /// -/// ** See code in ${getRelativePath(example)} ** +///$link /// {@end-tool} '''); } - void buildTestFiles({bool missingLinks = false, bool missingTests = false}) { + void buildTestFiles({bool missingLinks = false, bool missingTests = false, bool malformedLinks = false}) { final Directory examplesLib = examples.childDirectory('lib').childDirectory('layer')..createSync(recursive: true); final File fooExample = examplesLib.childFile('foo_example.0.dart') ..createSync(recursive: true) @@ -73,9 +74,15 @@ void main() { } final Directory flutterPackage = packages.childDirectory('flutter').childDirectory('lib').childDirectory('src') ..createSync(recursive: true); - writeLink(source: flutterPackage.childDirectory('layer').childFile('foo.dart'), example: fooExample); - writeLink(source: flutterPackage.childDirectory('layer').childFile('bar.dart'), example: barExample); - writeLink(source: flutterPackage.childDirectory('animation').childFile('curves.dart'), example: curvesExample); + if (malformedLinks) { + writeLink(source: flutterPackage.childDirectory('layer').childFile('foo.dart'), example: fooExample, alternateLink: '*See Code *'); + writeLink(source: flutterPackage.childDirectory('layer').childFile('bar.dart'), example: barExample, alternateLink: ' ** See code examples/api/lib/layer/bar_example.0.dart **'); + writeLink(source: flutterPackage.childDirectory('animation').childFile('curves.dart'), example: curvesExample, alternateLink: '* see code in examples/api/lib/animation/curves/curve2_d.0.dart *'); + } else { + writeLink(source: flutterPackage.childDirectory('layer').childFile('foo.dart'), example: fooExample); + writeLink(source: flutterPackage.childDirectory('layer').childFile('bar.dart'), example: barExample); + writeLink(source: flutterPackage.childDirectory('animation').childFile('curves.dart'), example: curvesExample); + } } setUp(() { @@ -124,6 +131,43 @@ void main() { expect(success, equals(false)); }); + test('check_code_samples.dart - checkCodeSamples catches malformed links', () async { + buildTestFiles(malformedLinks: true); + bool? success; + final String result = await capture( + () async { + success = checker.checkCodeSamples(); + }, + shouldHaveErrors: true, + ); + final bool isWindows = Platform.isWindows; + final String lines = [ + '╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════', + '║ The following examples are not linked from any source file API doc comments:', + if (!isWindows) '║ examples/api/lib/animation/curves/curve2_d.0.dart', + if (!isWindows) '║ examples/api/lib/layer/foo_example.0.dart', + if (!isWindows) '║ examples/api/lib/layer/bar_example.0.dart', + if (isWindows) r'║ examples\api\lib\animation\curves\curve2_d.0.dart', + if (isWindows) r'║ examples\api\lib\layer\foo_example.0.dart', + if (isWindows) r'║ examples\api\lib\layer\bar_example.0.dart', + '║ Either link them to a source file API doc comment, or remove them.', + '╚═══════════════════════════════════════════════════════════════════════════════', + '╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════', + '║ The following malformed links were found in API doc comments:', + if (!isWindows) '║ /flutter sdk/packages/flutter/lib/src/animation/curves.dart:6: ///* see code in examples/api/lib/animation/curves/curve2_d.0.dart *', + if (!isWindows) '║ /flutter sdk/packages/flutter/lib/src/layer/foo.dart:6: ///*See Code *', + if (!isWindows) '║ /flutter sdk/packages/flutter/lib/src/layer/bar.dart:6: /// ** See code examples/api/lib/layer/bar_example.0.dart **', + if (isWindows) r'║ C:\flutter sdk\packages\flutter\lib\src\animation\curves.dart:6: ///* see code in examples/api/lib/animation/curves/curve2_d.0.dart *', + if (isWindows) r'║ C:\flutter sdk\packages\flutter\lib\src\layer\foo.dart:6: ///*See Code *', + if (isWindows) r'║ C:\flutter sdk\packages\flutter\lib\src\layer\bar.dart:6: /// ** See code examples/api/lib/layer/bar_example.0.dart **', + '║ Correct the formatting of these links so that they match the exact pattern:', + r"║ r'\*\* See code in (?.+) \*\*'", + '╚═══════════════════════════════════════════════════════════════════════════════', + ].join('\n'); + expect(result, equals('$lines\n')); + expect(success, equals(false)); + }); + test('check_code_samples.dart - checkCodeSamples catches missing tests', () async { buildTestFiles(missingTests: true); bool? success;