diff --git a/dev/tools/test/update_icons_test.dart b/dev/tools/test/update_icons_test.dart new file mode 100644 index 0000000000..a24a5f658f --- /dev/null +++ b/dev/tools/test/update_icons_test.dart @@ -0,0 +1,39 @@ +// Copyright 2014 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:test/test.dart'; + +import '../update_icons.dart'; + +Map codepointsA = { + 'airplane': '111', + 'boat': '222', +}; +Map codepointsB = { + 'airplane': '333', +}; +Map codepointsC = { + 'airplane': '111', + 'train': '444', +}; + +void main() { + group('safety checks', () { + test('superset', () { + expect(testIsSuperset(codepointsA, codepointsA), true); + + expect(testIsSuperset(codepointsA, codepointsB), true); + expect(testIsSuperset(codepointsB, codepointsA), false); + }); + test('stability', () { + expect(testIsStable(codepointsA, codepointsA), true); + + expect(testIsStable(codepointsA, codepointsB), false); + expect(testIsStable(codepointsB, codepointsA), false); + + expect(testIsStable(codepointsA, codepointsC), true); + expect(testIsStable(codepointsC, codepointsA), true); + }); + }); +} diff --git a/dev/tools/update_icons.dart b/dev/tools/update_icons.dart index 33cb323c49..539aa35911 100644 --- a/dev/tools/update_icons.dart +++ b/dev/tools/update_icons.dart @@ -10,16 +10,21 @@ import 'dart:convert' show LineSplitter; import 'dart:io'; import 'package:args/args.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; +const String _iconsPathOption = 'icons'; +const String _iconsTemplatePathOption = 'icons-template'; const String _newCodepointsPathOption = 'new-codepoints'; const String _oldCodepointsPathOption = 'old-codepoints'; -const String _iconsClassPathOption = 'icons'; +const String _fontFamilyOption = 'font-family'; +const String _enforceSafetyChecks = 'enforce-safety-checks'; const String _dryRunOption = 'dry-run'; +const String _defaultIconsPath = 'packages/flutter/lib/src/material/icons.dart'; const String _defaultNewCodepointsPath = 'codepoints'; const String _defaultOldCodepointsPath = 'bin/cache/artifacts/material_fonts/codepoints'; -const String _defaultIconsPath = 'packages/flutter/lib/src/material/icons.dart'; +const String _defaultFontFamily = 'MaterialIcons'; const String _beginGeneratedMark = '// BEGIN GENERATED ICONS'; const String _endGeneratedMark = '// END GENERATED ICONS'; @@ -38,36 +43,37 @@ const Map> _platformAdaptiveIdentifiers = identifierPrefixRewrites = { - '_1': 'one_', - '_2': 'two_', - '_3': 'three_', - '_4': 'four_', - '_5': 'five_', - '_6': 'six_', - '_7': 'seven_', - '_8': 'eight_', - '_9': 'nine_', - '_10': 'ten_', - '_11': 'eleven_', - '_12': 'twelve_', - '_13': 'thirteen_', - '_14': 'fourteen_', - '_15': 'fifteen_', - '_16': 'sixteen_', - '_17': 'seventeen_', - '_18': 'eighteen_', - '_19': 'nineteen_', - '_20': 'twenty_', - '_21': 'twenty_one_', - '_22': 'twenty_two_', - '_23': 'twenty_three_', - '_24': 'twenty_four_', - '_30': 'thirty_', - '_60': 'sixty_', - '_360': 'threesixty', - '_2d': 'twod', - '_3d': 'threed', - '_3d_rotation': 'threed_rotation', + '1': 'one_', + '2': 'two_', + '3': 'three_', + '4': 'four_', + '5': 'five_', + '6': 'six_', + '7': 'seven_', + '8': 'eight_', + '9': 'nine_', + '10': 'ten_', + '11': 'eleven_', + '12': 'twelve_', + '13': 'thirteen_', + '14': 'fourteen_', + '15': 'fifteen_', + '16': 'sixteen_', + '17': 'seventeen_', + '18': 'eighteen_', + '19': 'nineteen_', + '20': 'twenty_', + '21': 'twenty_one_', + '22': 'twenty_two_', + '23': 'twenty_three_', + '24': 'twenty_four_', + '30': 'thirty_', + '60': 'sixty_', + '123': 'onetwothree', + '360': 'threesixty', + '2d': 'twod', + '3d': 'threed', + '3d_rotation': 'threed_rotation', }; // Rewrite certain Flutter IDs (reserved keywords) using exact matching. @@ -163,9 +169,14 @@ void main(List args) { final ArgResults argResults = _handleArguments(args); - final File iconClassFile = File(path.normalize(path.absolute(argResults[_iconsClassPathOption] as String))); - if (!iconClassFile.existsSync()) { - stderr.writeln('Error: Icons file not found: ${iconClassFile.path}'); + final File iconsFile = File(path.normalize(path.absolute(argResults[_iconsPathOption] as String))); + if (!iconsFile.existsSync()) { + stderr.writeln('Error: Icons file not found: ${iconsFile.path}'); + exit(1); + } + final File iconsTemplateFile = File(path.normalize(path.absolute(argResults[_iconsTemplatePathOption] as String))); + if (!iconsTemplateFile.existsSync()) { + stderr.writeln('Error: Icons template file not found: ${iconsTemplateFile.path}'); exit(1); } final File newCodepointsFile = File(argResults[_newCodepointsPathOption] as String); @@ -185,33 +196,51 @@ void main(List args) { final String oldCodepointsString = oldCodepointsFile.readAsStringSync(); final Map oldTokenPairMap = _stringToTokenPairMap(oldCodepointsString); - _testIsMapSuperset(newTokenPairMap, oldTokenPairMap); + stderr.writeln('Performing safety checks'); + final bool isSuperset = testIsSuperset(newTokenPairMap, oldTokenPairMap); + final bool isStable = testIsStable(newTokenPairMap, oldTokenPairMap); + if ((!isSuperset || !isStable) && argResults[_enforceSafetyChecks] as bool) { + exit(1); + } + final String iconsTemplateContents = iconsTemplateFile.readAsStringSync(); - final String iconClassFileData = iconClassFile.readAsStringSync(); - - stderr.writeln('Generating icons file...'); - final String newIconData = _regenerateIconsFile(iconClassFileData, newTokenPairMap); + stderr.writeln("Generating icons ${argResults[_dryRunOption] as bool ? '' : 'to ${iconsFile.path}'}"); + final String newIconsContents = _regenerateIconsFile( + iconsTemplateContents, + newTokenPairMap, + argResults[_fontFamilyOption] as String, + argResults[_enforceSafetyChecks] as bool, + ); if (argResults[_dryRunOption] as bool) { - stdout.write(newIconData); + stdout.write(newIconsContents); } else { - stderr.writeln('\nWriting to ${iconClassFile.path}.'); - iconClassFile.writeAsStringSync(newIconData); + iconsFile.writeAsStringSync(newIconsContents); _regenerateCodepointsFile(oldCodepointsFile, newTokenPairMap); } } ArgResults _handleArguments(List args) { final ArgParser argParser = ArgParser() + ..addOption(_iconsPathOption, + defaultsTo: _defaultIconsPath, + help: 'Location of the material icons file') + ..addOption(_iconsTemplatePathOption, + defaultsTo: _defaultIconsPath, + help: + 'Location of the material icons file template. Usually the same as --$_iconsPathOption') ..addOption(_newCodepointsPathOption, defaultsTo: _defaultNewCodepointsPath, help: 'Location of the new codepoints directory') ..addOption(_oldCodepointsPathOption, defaultsTo: _defaultOldCodepointsPath, help: 'Location of the existing codepoints directory') - ..addOption(_iconsClassPathOption, - defaultsTo: _defaultIconsPath, - help: 'Location of the material icons file') + ..addOption(_fontFamilyOption, + defaultsTo: _defaultFontFamily, + help: 'The font family to use for the IconData constants') + ..addFlag(_enforceSafetyChecks, + defaultsTo: true, + help: 'Whether to exit if safety checks fail (e.g. codepoints are missing or unstable') ..addFlag(_dryRunOption); argParser.addFlag('help', abbr: 'h', negatable: false, callback: (bool help) { if (help) { @@ -227,7 +256,7 @@ Map _stringToTokenPairMap(String codepointData) { .map((String line) => line.trim()) .where((String line) => line.isNotEmpty); - final Map pairs = {}; + final Map pairs = {}; for (final String line in cleanData) { final List tokens = line.split(' '); @@ -240,40 +269,49 @@ Map _stringToTokenPairMap(String codepointData) { return pairs; } -String _regenerateIconsFile(String iconData, Map tokenPairMap) { +String _regenerateIconsFile( + String templateFileContents, + Map tokenPairMap, + String fontFamily, + bool enforceSafetyChecks, + ) { final List<_Icon> newIcons = tokenPairMap.entries - .map((MapEntry entry) => _Icon(entry)) + .map((MapEntry entry) => _Icon(entry, fontFamily)) .toList(); newIcons.sort((_Icon a, _Icon b) => a._compareTo(b)); final StringBuffer buf = StringBuffer(); bool generating = false; - for (final String line in LineSplitter.split(iconData)) { + for (final String line in LineSplitter.split(templateFileContents)) { if (!generating) { buf.writeln(line); } - // Generate for _PlatformAdaptiveIcons + // Generate for PlatformAdaptiveIcons if (line.contains(_beginPlatformAdaptiveGeneratedMark)) { generating = true; final List platformAdaptiveDeclarations = []; _platformAdaptiveIdentifiers.forEach((String flutterId, List ids) { - // Automatically finds and generates styled icon declarations. + // Automatically finds and generates all icon declarations. for (final String style in ['', '_outlined', '_rounded', '_sharp']) { try { final _Icon agnosticIcon = newIcons.firstWhere( - (_Icon icon) => icon.id == '${ids[0]}$style', + (_Icon icon) => icon.id == '${ids[0]}$style', orElse: () => throw ids[0]); final _Icon iOSIcon = newIcons.firstWhere( - (_Icon icon) => icon.id == '${ids[1]}$style', + (_Icon icon) => icon.id == '${ids[1]}$style', orElse: () => throw ids[1]); - platformAdaptiveDeclarations.add(_Icon.platformAdaptiveDeclaration('$flutterId$style', agnosticIcon, iOSIcon)); + platformAdaptiveDeclarations.add(_Icon.platformAdaptiveDeclaration('$flutterId$style', agnosticIcon, iOSIcon), + ); } catch (e) { if (style == '') { - // Throw an error for regular (unstyled) icons. - stderr.writeln("Error while generating platformAdaptiveDeclarations: Icon '$e' not found."); - exit(1); + // Throw an error for baseline icons. + stderr.writeln("❌ Platform adaptive icon '$e' not found."); + if (enforceSafetyChecks) { + stderr.writeln('Safety checks failed'); + exit(1); + } } else { // Ignore errors for styled icons since some don't exist. } @@ -299,28 +337,50 @@ String _regenerateIconsFile(String iconData, Map tokenPairMap) { return buf.toString(); } -void _testIsMapSuperset(Map newCodepoints, Map oldCodepoints) { +@visibleForTesting +bool testIsSuperset(Map newCodepoints, Map oldCodepoints) { final Set newCodepointsSet = newCodepoints.keys.toSet(); final Set oldCodepointsSet = oldCodepoints.keys.toSet(); + final int diff = newCodepointsSet.length - oldCodepointsSet.length; + if (diff > 0) { + stderr.writeln('🆕 $diff new codepoints: ${newCodepointsSet.difference(oldCodepointsSet)}'); + } if (!newCodepointsSet.containsAll(oldCodepointsSet)) { - stderr.writeln(''' -Error: New codepoints file does not contain all ${oldCodepointsSet.length} existing codepoints.\n - Missing: ${oldCodepointsSet.difference(newCodepointsSet)} - ''', - ); - exit(1); + stderr.writeln( + '❌ new codepoints file does not contain all ${oldCodepointsSet.length} ' + 'existing codepoints. Missing: ${oldCodepointsSet.difference(newCodepointsSet)}'); + return false; } else { - final int diff = newCodepointsSet.length - oldCodepointsSet.length; - stderr.writeln('New codepoints file contains all ${oldCodepointsSet.length} existing codepoints.'); - if (diff > 0) { - stderr.writeln('It also contains $diff new codepoints: ${newCodepointsSet.difference(oldCodepointsSet)}'); + stderr.writeln('✅ new codepoints file contains all ${oldCodepointsSet.length} existing codepoints'); + } + return true; +} + +@visibleForTesting +bool testIsStable(Map newCodepoints, Map oldCodepoints) { + final int oldCodepointsCount = oldCodepoints.length; + final List unstable = []; + + oldCodepoints.forEach((String key, String value) { + if (newCodepoints.containsKey(key)) { + if (value != newCodepoints[key]) { + unstable.add(key); + } } + }); + + if (unstable.isNotEmpty) { + stderr.writeln('❌ out of $oldCodepointsCount existing codepoints, ${unstable.length} were unstable: $unstable'); + return false; + } else { + stderr.writeln('✅ all existing $oldCodepointsCount codepoints are stable'); + return true; } } void _regenerateCodepointsFile(File oldCodepointsFile, Map newTokenPairMap) { - stderr.writeln('Regenerating old codepoints file ${oldCodepointsFile.path}.\n'); + stderr.writeln('Regenerating old codepoints file ${oldCodepointsFile.path}'); final StringBuffer buf = StringBuffer(); final SplayTreeMap sortedNewTokenPairMap = SplayTreeMap.of(newTokenPairMap); @@ -330,7 +390,7 @@ void _regenerateCodepointsFile(File oldCodepointsFile, Map newTo class _Icon { // Parse tokenPair (e.g. {"6_ft_apart_outlined": "e004"}). - _Icon(MapEntry tokenPair) { + _Icon(MapEntry tokenPair, this.fontFamily) { id = tokenPair.key; hexCodepoint = tokenPair.value; @@ -349,14 +409,15 @@ class _Icon { htmlSuffix = '-filled'; } else { family = 'material'; - if (id.endsWith('_outlined') && id != 'insert_chart_outlined') { + if (id.endsWith('_baseline')) { + id = _removeLast(id, '_baseline'); + htmlSuffix = ''; + } else if (id.endsWith('_outlined')) { htmlSuffix = '-outlined'; } else if (id.endsWith('_rounded')) { htmlSuffix = '-round'; } else if (id.endsWith('_sharp')) { htmlSuffix = '-sharp'; - } else { - htmlSuffix = ''; } } @@ -379,7 +440,8 @@ class _Icon { late String flutterId; // e.g. five_g, five_g_outlined, five_g_rounded, five_g_sharp late String family; // e.g. material late String hexCodepoint; // e.g. e547 - late String htmlSuffix; // The suffix for the 'material-icons' HTML class. + late String htmlSuffix = ''; // The suffix for the 'material-icons' HTML class. + String fontFamily; // The IconData font family. String get name => shortId.replaceAll('_', ' ').trim(); @@ -393,7 +455,7 @@ class _Icon { : ''; String get declaration => - "static const IconData $flutterId = IconData(0x$hexCodepoint, fontFamily: 'MaterialIcons'$mirroredInRTL);"; + "static const IconData $flutterId = IconData(0x$hexCodepoint, fontFamily: '$fontFamily'$mirroredInRTL);"; String get fullDeclaration => ''' @@ -419,16 +481,14 @@ class _Icon { return shortId.compareTo(b.shortId); } - static String _replaceLast(String string, String toReplace) { + static String _removeLast(String string, String toReplace) { return string.replaceAll(RegExp('$toReplace\$'), ''); } static String _generateShortId(String id) { String shortId = id; for (final String styleSuffix in _idSuffixes) { - if (styleSuffix == '_outlined' && id == 'insert_chart_outlined') - continue; - shortId = _replaceLast(shortId, styleSuffix); + shortId = _removeLast(shortId, styleSuffix); if (shortId != id) { break; } @@ -440,22 +500,22 @@ class _Icon { static String generateFlutterId(String id) { String flutterId = id; // Exact identifier rewrites. - for (final MapEntry rewritePair - in identifierExactRewrites.entries) { + for (final MapEntry rewritePair in identifierExactRewrites.entries) { final String shortId = _Icon._generateShortId(id); if (shortId == rewritePair.key) { - flutterId = id.replaceFirst(rewritePair.key, identifierExactRewrites[rewritePair.key]!); + flutterId = id.replaceFirst( + rewritePair.key, + identifierExactRewrites[rewritePair.key]!, + ); } } // Prefix identifier rewrites. - for (final MapEntry rewritePair - in identifierPrefixRewrites.entries) { + for (final MapEntry rewritePair in identifierPrefixRewrites.entries) { if (id.startsWith(rewritePair.key)) { - flutterId = id.replaceFirst(rewritePair.key, identifierPrefixRewrites[rewritePair.key]!); - } - // TODO(guidezpl): With the next icon update, this won't be necessary, remove it. - if (id.startsWith(rewritePair.key.replaceFirst('_', ''))) { - flutterId = id.replaceFirst(rewritePair.key.replaceFirst('_', ''), identifierPrefixRewrites[rewritePair.key]!); + flutterId = id.replaceFirst( + rewritePair.key, + identifierPrefixRewrites[rewritePair.key]!, + ); } } return flutterId; diff --git a/packages/flutter/lib/src/material/icons.dart b/packages/flutter/lib/src/material/icons.dart index 65022b8e5d..b4dfcec5a9 100644 --- a/packages/flutter/lib/src/material/icons.dart +++ b/packages/flutter/lib/src/material/icons.dart @@ -97,8 +97,7 @@ class PlatformAdaptiveIcons implements Icons { /// /// Use with the [Icon] class to show specific icons. /// -/// Icons are identified by their name as listed below. **Do not use codepoints -/// directly, as they are subject to change.** +/// Icons are identified by their name as listed below, e.g. [Icons.airplanemode_on]. /// /// To use this class, make sure you set `uses-material-design: true` in your /// project's `pubspec.yaml` file in the `flutter` section. This ensures that @@ -10287,7 +10286,7 @@ class Icons { /// insert_chart — material icon named "insert chart" (round). static const IconData insert_chart_rounded = IconData(0xf819, fontFamily: 'MaterialIcons'); - /// insert_chart_outlined — material icon named "insert chart outlined". + /// insert_chart — material icon named "insert chart" (outlined). static const IconData insert_chart_outlined = IconData(0xf12a, fontFamily: 'MaterialIcons'); /// insert_chart_outlined — material icon named "insert chart outlined" (sharp).