From 63018fe6cbd7f95a0e3d3e64b488ccb660430d55 Mon Sep 17 00:00:00 2001 From: David Martos Date: Tue, 23 Jan 2024 10:35:22 +0100 Subject: [PATCH] Relax the warning of unavailable tokens in `gen_defaults` when a default value is provided (#140872) Fixes some false positives and false negatives from the `gen_defaults` tool regarding unavailable tokens in the Material spec. Fixes #140871 # Before ![image](https://github.com/flutter/flutter/assets/22084723/eb6c43ee-d919-4203-80ee-e36869e5bbcf) # After ![image](https://github.com/flutter/flutter/assets/22084723/67093dcb-1ab0-439f-9338-a6f364d2a9e1) These new issues that are logged are being fixed in https://github.com/flutter/flutter/pull/140573 --- *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* --- .../lib/input_decorator_template.dart | 8 +- dev/tools/gen_defaults/lib/template.dart | 25 ++++-- dev/tools/gen_defaults/lib/token_logger.dart | 14 +++- .../gen_defaults/test/gen_defaults_test.dart | 82 ++++++++++++++++++- 4 files changed, 115 insertions(+), 14 deletions(-) diff --git a/dev/tools/gen_defaults/lib/input_decorator_template.dart b/dev/tools/gen_defaults/lib/input_decorator_template.dart index 6c7363bf11..fcd2477e32 100644 --- a/dev/tools/gen_defaults/lib/input_decorator_template.dart +++ b/dev/tools/gen_defaults/lib/input_decorator_template.dart @@ -215,10 +215,10 @@ class _${blockName}DefaultsM3 extends InputDecorationTheme { ? componentColor(componentToken1) : componentColor(componentToken2); final double width = ( - getToken('$componentToken1.width') ?? - getToken('$componentToken1.height') ?? - getToken('$componentToken2.width') ?? - getToken('$componentToken2.height') ?? + getToken('$componentToken1.width', optional: true) ?? + getToken('$componentToken1.height', optional: true) ?? + getToken('$componentToken2.width', optional: true) ?? + getToken('$componentToken2.height', optional: true) ?? 1.0) as double; return 'BorderSide(color: $borderColor${width != 1.0 ? ", width: $width" : ""})'; } diff --git a/dev/tools/gen_defaults/lib/template.dart b/dev/tools/gen_defaults/lib/template.dart index fffff0461b..1e611feb98 100644 --- a/dev/tools/gen_defaults/lib/template.dart +++ b/dev/tools/gen_defaults/lib/template.dart @@ -38,7 +38,11 @@ abstract class TokenTemplate { bool tokenAvailable(String tokenName) => _tokens.containsKey(tokenName); /// Resolve a token while logging its usage. - dynamic getToken(String tokenName) { + /// There will be no log if [optional] is true and the token doesn't exist. + dynamic getToken(String tokenName, {bool optional = false}) { + if (optional && !tokenAvailable(tokenName)) { + return null; + } tokenLogger.log(tokenName); return _tokens[tokenName]; } @@ -108,14 +112,17 @@ abstract class TokenTemplate { /// If there is a value for the given token, this will return /// the value prepended with [colorSchemePrefix]. /// - /// Otherwise it will return [defaultValue]. + /// Otherwise it will return [defaultValue] if provided or 'null' if not. + /// + /// If a [defaultValue] is not provided and the token doesn't exist, the token + /// lookup is logged and a warning will be shown at the end of the process. /// /// See also: /// * [componentColor], that provides support for an optional opacity. - String color(String colorToken, [String defaultValue = 'null']) { - return tokenAvailable(colorToken) - ? '$colorSchemePrefix${getToken(colorToken)}' - : defaultValue; + String color(String colorToken, [String? defaultValue]) { + final String effectiveDefault = defaultValue ?? 'null'; + final dynamic tokenVal = getToken(colorToken, optional: defaultValue != null); + return tokenVal == null ? effectiveDefault : '$colorSchemePrefix$tokenVal'; } /// Generate a [ColorScheme] color name for the given token or a transparent @@ -244,7 +251,11 @@ abstract class TokenTemplate { return 'null'; } final String borderColor = componentColor(componentToken); - final double width = (getToken('$componentToken.width') ?? getToken('$componentToken.height') ?? 1.0) as double; + final double width = ( + getToken('$componentToken.width', optional: true) ?? + getToken('$componentToken.height', optional: true) ?? + 1.0 + ) as double; return 'BorderSide(color: $borderColor${width != 1.0 ? ", width: $width" : ""})'; } diff --git a/dev/tools/gen_defaults/lib/token_logger.dart b/dev/tools/gen_defaults/lib/token_logger.dart index 3347e5fbd9..88d0aca8dc 100644 --- a/dev/tools/gen_defaults/lib/token_logger.dart +++ b/dev/tools/gen_defaults/lib/token_logger.dart @@ -28,16 +28,20 @@ class TokenLogger { // Sorted set of used tokens. final SplayTreeSet _usedTokens = SplayTreeSet(); + // Set of tokens that were referenced on some templates, but do not exist. + final Set _unavailableTokens = {}; + void clear() { _allTokens.clear(); _versionMap.clear(); _usedTokens.clear(); + _unavailableTokens.clear(); } /// Logs a token. void log(String token) { if (!_allTokens.containsKey(token)) { - print('\x1B[31m' 'Token unavailable: $token' '\x1B[0m'); + _unavailableTokens.add(token); return; } _usedTokens.add(token); @@ -76,6 +80,14 @@ class TokenLogger { } print('Tokens used: ${_usedTokens.length}/${_allTokens.length}'); + + if (_unavailableTokens.isNotEmpty) { + print(''); + print('\x1B[31m' 'Some referenced tokens do not exist: ${_unavailableTokens.length}' '\x1B[0m'); + for (final String token in _unavailableTokens) { + print(' $token'); + } + } } /// Dumps version and tokens usage to a file. diff --git a/dev/tools/gen_defaults/test/gen_defaults_test.dart b/dev/tools/gen_defaults/test/gen_defaults_test.dart index a921b9be19..bcf1d53740 100644 --- a/dev/tools/gen_defaults/test/gen_defaults_test.dart +++ b/dev/tools/gen_defaults/test/gen_defaults_test.dart @@ -325,10 +325,69 @@ static final String tokenBar = 'foo'; logger.log('foobar'); logger.printTokensUsage(verbose: true); - expect(printLog, contains(errorColoredString('Token unavailable: baz'))); - expect(printLog, contains(errorColoredString('Token unavailable: foobar'))); expect(printLog, contains('❌ foo')); expect(printLog, contains('Tokens used: 0/1')); + expect(printLog, contains(errorColoredString('Some referenced tokens do not exist: 2'))); + expect(printLog, contains(' baz')); + expect(printLog, contains(' foobar')); + })); + + test("color function doesn't log when providing a default", overridePrint(() { + allTokens['color_foo_req'] = 'value'; + + // color_foo_opt is not available, but because it has a default value, it won't warn about it + + TestColorTemplate('block', 'filename', allTokens).generate(); + logger.printTokensUsage(verbose: true); + + expect(printLog, contains('✅ color_foo_req')); + expect(printLog, contains('Tokens used: 1/1')); + })); + + test('color function logs when not providing a default', overridePrint(() { + // Nor color_foo_req or color_foo_opt are available, but only color_foo_req will be logged. + // This mimics a token being removed, but expected to exist. + + TestColorTemplate('block', 'filename', allTokens).generate(); + logger.printTokensUsage(verbose: true); + + expect(printLog, contains('Tokens used: 0/0')); + expect(printLog, contains(errorColoredString('Some referenced tokens do not exist: 1'))); + expect(printLog, contains(' color_foo_req')); + })); + + test('border function logs width token when available', overridePrint(() { + allTokens['border_foo.color'] = 'red'; + allTokens['border_foo.width'] = 3.0; + + TestBorderTemplate('block', 'filename', allTokens).generate(); + logger.printTokensUsage(verbose: true); + + expect(printLog, contains('✅ border_foo.color')); + expect(printLog, contains('✅ border_foo.width')); + expect(printLog, contains('Tokens used: 2/2')); + })); + + test('border function logs height token when width token not available', overridePrint(() { + allTokens['border_foo.color'] = 'red'; + allTokens['border_foo.height'] = 3.0; + + TestBorderTemplate('block', 'filename', allTokens).generate(); + logger.printTokensUsage(verbose: true); + + expect(printLog, contains('✅ border_foo.color')); + expect(printLog, contains('✅ border_foo.height')); + expect(printLog, contains('Tokens used: 2/2')); + })); + + test("border function doesn't log when width or height tokens not available", overridePrint(() { + allTokens['border_foo.color'] = 'red'; + + TestBorderTemplate('block', 'filename', allTokens).generate(); + logger.printTokensUsage(verbose: true); + + expect(printLog, contains('✅ border_foo.color')); + expect(printLog, contains('Tokens used: 1/1')); })); test('can log and dump versions & tokens to a file', overridePrint(() { @@ -369,3 +428,22 @@ static final String tokenFoo = '${getToken('foo')}'; static final String tokenBar = '${getToken('bar')}'; '''; } + +class TestColorTemplate extends TokenTemplate { + TestColorTemplate(super.blockName, super.fileName, super.tokens); + + @override + String generate() => ''' +static final Color color_1 = '${color('color_foo_req')}'; +static final Color color_2 = '${color('color_foo_opt', 'Colors.red')}'; +'''; +} + +class TestBorderTemplate extends TokenTemplate { + TestBorderTemplate(super.blockName, super.fileName, super.tokens); + + @override + String generate() => ''' +static final BorderSide border = '${border('border_foo')}'; +'''; +}