diff --git a/packages/rfw/CHANGELOG.md b/packages/rfw/CHANGELOG.md index cbea927ff62d..227a8095c793 100644 --- a/packages/rfw/CHANGELOG.md +++ b/packages/rfw/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.0.13 + +* Block comments in RFW's text format. (`/*...*/`) + ## 1.0.12 * Improves web compatibility. diff --git a/packages/rfw/README.md b/packages/rfw/README.md index c41a66f7d352..93bfdeb2c22b 100644 --- a/packages/rfw/README.md +++ b/packages/rfw/README.md @@ -981,6 +981,13 @@ update `test_coverage/bin/test_coverage.dart` with the appropriate expectations to prevent future coverage regressions. (That program is run by `run_tests.sh`.) +To run the tests, use `flutter test`. + +To run the web tests, use `flutter test --platform=chrome`. If there +is code that only runs on the web target, mark each such line by +ending it with the string `// dead code on VM target`. This will +exclude that line from the coverage calculation. + Golden tests are only run against the Flutter master channel and only run on Linux, since minor rendering differences are expected on different platforms and on different versions of Flutter. diff --git a/packages/rfw/lib/src/dart/binary.dart b/packages/rfw/lib/src/dart/binary.dart index a062bdd16469..4d569289bdec 100644 --- a/packages/rfw/lib/src/dart/binary.dart +++ b/packages/rfw/lib/src/dart/binary.dart @@ -337,9 +337,9 @@ class _BlobDecoder { } // We use multiplication rather than bit shifts because << truncates to 32 bits when compiled to JS: // https://dart.dev/guides/language/numbers#bitwise-operations - final int a = bytes.getUint32(byteOffset, _blobEndian); - final int b = bytes.getInt32(byteOffset + 4, _blobEndian); - return a + (b * 0x100000000); + final int a = bytes.getUint32(byteOffset, _blobEndian); // dead code on VM target + final int b = bytes.getInt32(byteOffset + 4, _blobEndian); // dead code on VM target + return a + (b * 0x100000000); // dead code on VM target } double _readBinary64() { @@ -544,12 +544,12 @@ class _BlobEncoder { } else { // We use division rather than bit shifts because >> truncates to 32 bits when compiled to JS: // https://dart.dev/guides/language/numbers#bitwise-operations - if (value >= 0) { - _scratchIn.setInt32(0, value, _blobEndian); - _scratchIn.setInt32(4, value ~/ 0x100000000, _blobEndian); + if (value >= 0) { // dead code on VM target + _scratchIn.setInt32(0, value, _blobEndian); // dead code on VM target + _scratchIn.setInt32(4, value ~/ 0x100000000, _blobEndian); // dead code on VM target } else { - _scratchIn.setInt32(0, value, _blobEndian); - _scratchIn.setInt32(4, -((-value) ~/ 0x100000000 + 1), _blobEndian); + _scratchIn.setInt32(0, value, _blobEndian); // dead code on VM target + _scratchIn.setInt32(4, -((-value) ~/ 0x100000000 + 1), _blobEndian); // dead code on VM target } } bytes.add(_scratchOut); diff --git a/packages/rfw/lib/src/dart/text.dart b/packages/rfw/lib/src/dart/text.dart index bebb22e6ad60..7e8a00c747e3 100644 --- a/packages/rfw/lib/src/dart/text.dart +++ b/packages/rfw/lib/src/dart/text.dart @@ -24,6 +24,9 @@ import 'model.dart'; /// * end-of-line comments are supported, using the "//" syntax from C (and /// Dart). /// +/// * block comments are supported, using the "/*...*/" syntax from C (and +/// Dart). +/// /// * map keys may be unquoted when they are alphanumeric. /// /// * "null" is not a valid value except as a value in maps, where it is @@ -152,8 +155,9 @@ import 'model.dart'; /// binary64 format. /// /// ```bnf -/// WS ::= ( | | "//" comment* ) +/// WS ::= ( | | "//" comment* | "/*" blockcomment "*/" ) /// comment ::= +/// blockcomment ::= /// ``` /// /// The `WS` token is used to represent where whitespace and comments are @@ -746,6 +750,8 @@ enum _TokenizerMode { endQuote, slash, comment, + blockComment, + blockCommentEnd, } String _describeRune(int current) { @@ -1133,7 +1139,7 @@ Iterable<_Token> _tokenize(String file) sync* { case 0x37: // U+0037 DIGIT SEVEN character (7) case 0x38: // U+0038 DIGIT EIGHT character (8) case 0x39: // U+0039 DIGIT NINE character (9) - buffer.add(current); + buffer.add(current); // https://github.com/dart-lang/sdk/issues/53349 break; default: throw ParserException('Unexpected character ${_describeRune(current)} in integer', line, column); @@ -2051,6 +2057,9 @@ Iterable<_Token> _tokenize(String file) sync* { switch (current) { case -1: throw ParserException('Unexpected end of file inside comment delimiter', line, column); + case 0x2A: // U+002A ASTERISK character (*) + mode = _TokenizerMode.blockComment; + break; case 0x2F: // U+002F SOLIDUS character (/) mode = _TokenizerMode.comment; break; @@ -2072,6 +2081,33 @@ Iterable<_Token> _tokenize(String file) sync* { break; } break; + + case _TokenizerMode.blockComment: + switch (current) { + case -1: + throw ParserException('Unexpected end of file in block comment', line, column); + case 0x2A: // U+002A ASTERISK character (*) + mode = _TokenizerMode.blockCommentEnd; + break; + default: + // ignored, comment + break; + } + break; + + case _TokenizerMode.blockCommentEnd: + switch (current) { + case -1: + throw ParserException('Unexpected end of file in block comment', line, column); + case 0x2F: // U+002F SOLIDUS character (/) + mode = _TokenizerMode.main; + break; + default: + // ignored, comment + mode = _TokenizerMode.blockComment; + break; + } + break; } index += 1; } diff --git a/packages/rfw/pubspec.yaml b/packages/rfw/pubspec.yaml index 019bf5c6eb2a..48f9b4b446cc 100644 --- a/packages/rfw/pubspec.yaml +++ b/packages/rfw/pubspec.yaml @@ -2,7 +2,7 @@ name: rfw description: "Remote Flutter widgets: a library for rendering declarative widget description files at runtime." repository: https://github.com/flutter/packages/tree/main/packages/rfw issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+rfw%22 -version: 1.0.12 +version: 1.0.13 environment: sdk: ">=3.0.0 <4.0.0" diff --git a/packages/rfw/test/text_test.dart b/packages/rfw/test/text_test.dart index 64ec047491d0..8fe51d75a3d9 100644 --- a/packages/rfw/test/text_test.dart +++ b/packages/rfw/test/text_test.dart @@ -143,6 +143,10 @@ void main() { test('{ "a": /', 'Unexpected end of file inside comment delimiter at line 1 column 8.'); test('{ "a": /.', 'Unexpected character U+002E (".") inside comment delimiter at line 1 column 9.'); test('{ "a": //', 'Unexpected at line 1 column 9.'); + test('{ "a": /*', 'Unexpected end of file in block comment at line 1 column 9.'); + test('{ "a": /*/', 'Unexpected end of file in block comment at line 1 column 10.'); + test('{ "a": /**', 'Unexpected end of file in block comment at line 1 column 10.'); + test('{ "a": /* *', 'Unexpected end of file in block comment at line 1 column 11.'); }); testWidgets('valid values in parseDataFile', (WidgetTester tester) async { @@ -228,6 +232,9 @@ void main() { expect(parseDataFile('{ "a": \'\\uDDDD\' }'), { 'a': '\u{dddd}' }); expect(parseDataFile('{ "a": \'\\uEEEE\' }'), { 'a': '\u{eeee}' }); expect(parseDataFile('{ "a": \'\\uFFFF\' }'), { 'a': '\u{ffff}' }); + expect(parseDataFile('{ "a": /**/ "1" }'), { 'a': '1' }); + expect(parseDataFile('{ "a": /* */ "1" }'), { 'a': '1' }); + expect(parseDataFile('{ "a": /*\n*/ "1" }'), { 'a': '1' }); }); testWidgets('error handling in parseLibraryFile', (WidgetTester tester) async { @@ -294,6 +301,7 @@ void main() { }); testWidgets('parseLibraryFile: references', (WidgetTester tester) async { + expect(parseLibraryFile('widget a = b(c:data.11234567890."e");').toString(), 'widget a = b({c: data.11234567890.e});'); expect(parseLibraryFile('widget a = b(c: [...for d in []: d]);').toString(), 'widget a = b({c: [...for loop in []: loop0.]});'); expect(parseLibraryFile('widget a = b(c:args.foo.bar);').toString(), 'widget a = b({c: args.foo.bar});'); expect(parseLibraryFile('widget a = b(c:data.foo.bar);').toString(), 'widget a = b({c: data.foo.bar});'); diff --git a/packages/rfw/test_coverage/bin/test_coverage.dart b/packages/rfw/test_coverage/bin/test_coverage.dart index a1acbac8fbd6..7c753557a303 100644 --- a/packages/rfw/test_coverage/bin/test_coverage.dart +++ b/packages/rfw/test_coverage/bin/test_coverage.dart @@ -7,6 +7,7 @@ import 'dart:io'; import 'package:lcov_parser/lcov_parser.dart' as lcov; +import 'package:meta/meta.dart'; // After you run this script, `.../rfw/coverage/lcov.info` will represent the // latest coverage information for the package. Load that file into your IDE's @@ -19,10 +20,32 @@ import 'package:lcov_parser/lcov_parser.dart' as lcov; // Please update these targets when you update this package. // Please ensure that test coverage continues to be 100%. -const int targetLines = 3114; +const int targetLines = 3122; const String targetPercent = '100'; const String lastUpdate = '2023-06-29'; +@immutable +/* final */ class LcovLine { + const LcovLine(this.filename, this.line); + final String filename; + final int line; + + @override + int get hashCode => Object.hash(filename, line); + + @override + bool operator ==(Object other) { + return other is LcovLine && + other.line == line && + other.filename == filename; + } + + @override + String toString() { + return '$filename:$line'; + } +} + Future main(List arguments) async { // This script is mentioned in the README.md file. @@ -54,17 +77,78 @@ Future main(List arguments) async { exit(0); } + final List libFiles = Directory('lib') + .listSync(recursive: true) + .whereType() + .where((File file) => file.path.endsWith('.dart')) + .toList(); + final Set flakyLines = {}; + final Set deadLines = {}; + for (final File file in libFiles) { + int lineNumber = 0; + for (final String line in file.readAsLinesSync()) { + lineNumber += 1; + if (line.endsWith('// dead code on VM target')) { + deadLines.add(LcovLine(file.path, lineNumber)); + } + if (line.endsWith('// https://github.com/dart-lang/sdk/issues/53349')) { + flakyLines.add(LcovLine(file.path, lineNumber)); + } + } + } + final List records = await lcov.Parser.parse( 'coverage/lcov.info', ); int totalLines = 0; int coveredLines = 0; + bool deadLinesError = false; for (final lcov.Record record in records) { - totalLines += record.lines?.found ?? 0; - coveredLines += record.lines?.hit ?? 0; + if (record.lines != null) { + totalLines += record.lines!.found ?? 0; + coveredLines += record.lines!.hit ?? 0; + if (record.file != null && record.lines!.details != null) { + for (int index = 0; index < record.lines!.details!.length; index += 1) { + if (record.lines!.details![index].hit != null && + record.lines!.details![index].line != null) { + final LcovLine line = LcovLine( + record.file!, + record.lines!.details![index].line!, + ); + if (flakyLines.contains(line)) { + totalLines -= 1; + if (record.lines!.details![index].hit! > 0) { + coveredLines -= 1; + } + } + if (deadLines.contains(line)) { + deadLines.remove(line); + totalLines -= 1; + if (record.lines!.details![index].hit! > 0) { + print( + '$line: Line is marked as being dead code but was nonetheless covered.', + ); + deadLinesError = true; + } + } + } + } + } + } } - if (totalLines == 0 || totalLines < coveredLines) { - print('Failed to compute coverage.'); + if (deadLines.isNotEmpty || deadLinesError) { + for (final LcovLine line in deadLines) { + print( + '$line: Line is marked as being undetectably dead code but was not considered reachable.', + ); + } + print( + 'Consider removing the "dead code on VM target" comment from affected lines.', + ); + exit(1); + } + if (totalLines <= 0 || totalLines < coveredLines) { + print('Failed to compute coverage correctly.'); exit(1); } @@ -74,7 +158,7 @@ Future main(List arguments) async { // We only check the TARGET_LINES matches, not the TARGET_PERCENT, // because we expect the percentage to drop over time as Dart fixes // various bugs in how it determines what lines are coverable. - if (coveredLines < targetLines) { + if (coveredLines < targetLines && targetLines <= totalLines) { print(''); print(' ╭──────────────────────────────╮'); print(' │ COVERAGE REGRESSION DETECTED │'); @@ -100,13 +184,23 @@ Future main(List arguments) async { } else { if (coveredLines < totalLines) { print( - 'Warning: Coverage of package:rfw is no longer 100%. (Coverage is now $coveredPercent%.)', + 'Warning: Coverage of package:rfw is no longer 100%. (Coverage is now $coveredPercent%, $coveredLines/$totalLines lines.)', ); } if (coveredLines > targetLines) { print( - 'test_coverage/bin/test_coverage.dart should be updated to have a new target ($coveredLines).', + 'Total lines of covered code has increased, and coverage script is now out of date.\n' + 'Coverage is now $coveredPercent%, $coveredLines/$totalLines lines, whereas previously there were only $targetLines lines.\n' + 'Update the "\$targetLines" constant at the top of rfw/test_coverage/bin/test_coverage.dart (to $coveredLines).', + ); + } + if (targetLines > totalLines) { + print( + 'Total lines of code has reduced, and coverage script is now out of date.\n' + 'Coverage is now $coveredPercent%, $coveredLines/$totalLines lines, but previously there were $targetLines lines.\n' + 'Update the "\$targetLines" constant at the top of rfw/test_coverage/bin/test_coverage.dart (to $totalLines).', ); + exit(1); } } diff --git a/packages/rfw/test_coverage/pubspec.yaml b/packages/rfw/test_coverage/pubspec.yaml index a482a95c9e12..9e8ef6809f27 100644 --- a/packages/rfw/test_coverage/pubspec.yaml +++ b/packages/rfw/test_coverage/pubspec.yaml @@ -8,3 +8,4 @@ environment: dependencies: lcov_parser: 0.1.1 + meta: ^1.7.0