From dd98765a3200bc6465f716afee606b8bad295900 Mon Sep 17 00:00:00 2001 From: Louise Hsu Date: Mon, 4 Dec 2023 10:09:43 -0800 Subject: [PATCH 1/3] initial commit --- .../tool/lib/src/pubspec_check_command.dart | 11 +- .../tool/test/pubspec_check_command_test.dart | 120 ++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index bccdad5c785..c099c161b38 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -332,7 +332,7 @@ class PubspecCheckCommand extends PackageLoopingCommand { false; } - // Validates the "implements" keyword for a plugin, returning an error + // Validates the "topics" keyword for a plugin, returning an error // string if there are any issues. String? _checkTopics( Pubspec pubspec, { @@ -352,6 +352,15 @@ class PubspecCheckCommand extends PackageLoopingCommand { 'a topic. Add "$topicName" to the "topics" section.'; } } + for (final String topic in topics) { + final RegExp expectedTopicFormat = + RegExp(r'^[a-z](?:[a-z0-9]*[a-z0-9])?(-[a-z0-9]+)*$'); + if (!expectedTopicFormat.hasMatch(topic)) { + return 'Invalid topic value "$topic" in "topics" section. ' + 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' + 'starting with a-z and ending with a-z or 0-9.'; + } + } return null; } diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/pubspec_check_command_test.dart index 60bcc2d4784..7e9086d8967 100644 --- a/script/tool/test/pubspec_check_command_test.dart +++ b/script/tool/test/pubspec_check_command_test.dart @@ -633,6 +633,126 @@ ${_topicsSection()} ); }); + test('fails when topic a topic name contains a space', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['plugin a'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic value "plugin a" in "topics" section. ' + 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' + 'starting with a-z and ending with a-z or 0-9.'), + ]), + ); + }); + + test('fails when topic a topic name contains double dash', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['plugin--a'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic value "plugin--a" in "topics" section. ' + 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' + 'starting with a-z and ending with a-z or 0-9.'), + ]), + ); + }); + + test('fails when topic a topic name starts with a number', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['1plugin-a'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic value "1plugin-a" in "topics" section. ' + 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' + 'starting with a-z and ending with a-z or 0-9.'), + ]), + ); + }); + + test('fails when topic a topic name contains uppercase', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['plugin-A'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic value "plugin-A" in "topics" section. ' + 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' + 'starting with a-z and ending with a-z or 0-9.'), + ]), + ); + }); + test('fails when environment section is out of order', () async { final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, examples: []); From 4d8126340a208b7b461d4336d8c407fd1d714445 Mon Sep 17 00:00:00 2001 From: Louise Hsu Date: Tue, 5 Dec 2023 11:17:49 -0800 Subject: [PATCH 2/3] git add length and num topics validation --- script/tool/lib/src/pubspec_check_command.dart | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index c099c161b38..83c9edc6062 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -343,6 +343,10 @@ class PubspecCheckCommand extends PackageLoopingCommand { return 'A published package should include "topics". ' 'See https://dart.dev/tools/pub/pubspec#topics.'; } + if (topics.length > 5) { + return 'A published package should have maximum 5 topics. ' + 'See https://dart.dev/tools/pub/pubspec#topics.'; + } if (isFlutterPlugin(package) && package.isFederated) { final String pluginName = package.directory.parent.basename; // '_' isn't allowed in topics, so convert to '-'. @@ -353,12 +357,14 @@ class PubspecCheckCommand extends PackageLoopingCommand { } } for (final String topic in topics) { + // Validates topic names according to https://dart.dev/tools/pub/pubspec#topics final RegExp expectedTopicFormat = - RegExp(r'^[a-z](?:[a-z0-9]*[a-z0-9])?(-[a-z0-9]+)*$'); - if (!expectedTopicFormat.hasMatch(topic)) { + RegExp(r'^[a-z](?:-?[a-z0-9]+)*$'); + if (!expectedTopicFormat.hasMatch(topic) || topic.length < 2 || topic.length > 32 ) { return 'Invalid topic value "$topic" in "topics" section. ' 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' - 'starting with a-z and ending with a-z or 0-9.'; + 'start with a-z and ending with a-z or 0-9, have a minimum of 2 characters ' + 'and have a maximum of 32 characters.'; } } return null; From 8184b903acb052da87ace053452340752408ef14 Mon Sep 17 00:00:00 2001 From: Louise Hsu Date: Tue, 5 Dec 2023 12:54:16 -0800 Subject: [PATCH 3/3] fix tests --- .../tool/lib/src/pubspec_check_command.dart | 22 +-- .../tool/test/pubspec_check_command_test.dart | 168 ++++++++++++++++-- 2 files changed, 168 insertions(+), 22 deletions(-) diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index 83c9edc6062..378dfc65caa 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -356,16 +356,18 @@ class PubspecCheckCommand extends PackageLoopingCommand { 'a topic. Add "$topicName" to the "topics" section.'; } } - for (final String topic in topics) { - // Validates topic names according to https://dart.dev/tools/pub/pubspec#topics - final RegExp expectedTopicFormat = - RegExp(r'^[a-z](?:-?[a-z0-9]+)*$'); - if (!expectedTopicFormat.hasMatch(topic) || topic.length < 2 || topic.length > 32 ) { - return 'Invalid topic value "$topic" in "topics" section. ' - 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' - 'start with a-z and ending with a-z or 0-9, have a minimum of 2 characters ' - 'and have a maximum of 32 characters.'; - } + + // Validates topic names according to https://dart.dev/tools/pub/pubspec#topics + final RegExp expectedTopicFormat = RegExp(r'^[a-z](?:-?[a-z0-9]+)*$'); + final Iterable invalidTopics = topics.where((String topic) => + !expectedTopicFormat.hasMatch(topic) || + topic.length < 2 || + topic.length > 32); + if (invalidTopics.isNotEmpty) { + return 'Invalid topic(s): ${invalidTopics.join(', ')} in "topics" section. ' + 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' + 'start with a-z and ending with a-z or 0-9, have a minimum of 2 characters ' + 'and have a maximum of 32 characters.'; } return null; } diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/pubspec_check_command_test.dart index 7e9086d8967..8e99d720f57 100644 --- a/script/tool/test/pubspec_check_command_test.dart +++ b/script/tool/test/pubspec_check_command_test.dart @@ -633,7 +633,7 @@ ${_topicsSection()} ); }); - test('fails when topic a topic name contains a space', () async { + test('fails when topic name contains a space', () async { final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, examples: []); @@ -656,9 +656,7 @@ ${_topicsSection(['plugin a'])} expect( output, containsAllInOrder([ - contains('Invalid topic value "plugin a" in "topics" section. ' - 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' - 'starting with a-z and ending with a-z or 0-9.'), + contains('Invalid topic(s): plugin a in "topics" section. '), ]), ); }); @@ -686,9 +684,7 @@ ${_topicsSection(['plugin--a'])} expect( output, containsAllInOrder([ - contains('Invalid topic value "plugin--a" in "topics" section. ' - 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' - 'starting with a-z and ending with a-z or 0-9.'), + contains('Invalid topic(s): plugin--a in "topics" section. '), ]), ); }); @@ -716,9 +712,7 @@ ${_topicsSection(['1plugin-a'])} expect( output, containsAllInOrder([ - contains('Invalid topic value "1plugin-a" in "topics" section. ' - 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' - 'starting with a-z and ending with a-z or 0-9.'), + contains('Invalid topic(s): 1plugin-a in "topics" section. '), ]), ); }); @@ -746,9 +740,159 @@ ${_topicsSection(['plugin-A'])} expect( output, containsAllInOrder([ - contains('Invalid topic value "plugin-A" in "topics" section. ' + contains('Invalid topic(s): plugin-A in "topics" section. '), + ]), + ); + }); + + test('fails when there are more than 5 topics', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection([ + 'plugin-a', + 'plugin-a', + 'plugin-a', + 'plugin-a', + 'plugin-a', + 'plugin-a' + ])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + ' A published package should have maximum 5 topics. See https://dart.dev/tools/pub/pubspec#topics.'), + ]), + ); + }); + + test('fails if a topic name is longer than 32 characters', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['foobarfoobarfoobarfoobarfoobarfoobarfoo'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Invalid topic(s): foobarfoobarfoobarfoobarfoobarfoobarfoo in "topics" section. '), + ]), + ); + }); + + test('fails if a topic name is longer than 2 characters', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['a'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic(s): a in "topics" section. '), + ]), + ); + }); + + test('fails if a topic name ends in a dash', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['plugin-'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic(s): plugin- in "topics" section. '), + ]), + ); + }); + + test('Invalid topics section has expected error message', () async { + final RepositoryPackage plugin = + createFakePlugin('plugin', packagesDir, examples: []); + + plugin.pubspecFile.writeAsStringSync(''' +${_headerSection('plugin')} +${_environmentSection()} +${_flutterSection(isPlugin: true)} +${_dependenciesSection()} +${_devDependenciesSection()} +${_topicsSection(['plugin-A', 'Plugin-b'])} +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['pubspec-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid topic(s): plugin-A, Plugin-b in "topics" section. ' 'Topics must consist of lowercase alphanumerical characters or dash (but no double dash), ' - 'starting with a-z and ending with a-z or 0-9.'), + 'start with a-z and ending with a-z or 0-9, have a minimum of 2 characters ' + 'and have a maximum of 32 characters.'), ]), ); });