[rfw] Implement /* block comments */ in the text format (#4769)

Also, fix the coverage which previously was not 100% anymore (mostly by making it possible to skip some lines because there's no way to track the coverage on the web side of things as far as I can tell).

Fixes https://github.com/flutter/flutter/issues/133285

packages/rfw/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.0.13
+
+* Block comments in RFW's text format. (`/*...*/`)
+
 ## 1.0.12
 
 * Improves web compatibility.

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.

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 a = bytes.getUint32(byteOffset, _blobEndian); // dead code on VM target
-    final int b = bytes.getInt32(byteOffset + 4, _blobEndian);
+    final int b = bytes.getInt32(byteOffset + 4, _blobEndian); // dead code on VM target
-    return a + (b * 0x100000000);
+    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) {
+      if (value >= 0) { // dead code on VM target
-        _scratchIn.setInt32(0, value, _blobEndian);
+        _scratchIn.setInt32(0, value, _blobEndian); // dead code on VM target
-        _scratchIn.setInt32(4, value ~/ 0x100000000, _blobEndian);
+        _scratchIn.setInt32(4, value ~/ 0x100000000, _blobEndian); // dead code on VM target
       } else {
-        _scratchIn.setInt32(0, value, _blobEndian);
+        _scratchIn.setInt32(0, value, _blobEndian); // dead code on VM target
-        _scratchIn.setInt32(4, -((-value) ~/ 0x100000000 + 1), _blobEndian);
+        _scratchIn.setInt32(4, -((-value) ~/ 0x100000000 + 1), _blobEndian); // dead code on VM target
       }
     }
     bytes.add(_scratchOut);

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 ::= ( <U+0020> | <U+000A> | "//" comment* <U+000A or EOF> )
+/// WS ::= ( <U+0020> | <U+000A> | "//" comment* <U+000A or EOF> | "/*" blockcomment "*/" )
 /// comment ::= <U+0000..U+10FFFF except U+000A>
+/// blockcomment ::= <any number of U+0000..U+10FFFF characters not containing the sequence U+002A U+002F>
 /// ```
 ///
 /// 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;
   }

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"

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 <EOF> 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\' }'), <String, Object?>{ 'a': '\u{dddd}' });
     expect(parseDataFile('{ "a": \'\\uEEEE\' }'), <String, Object?>{ 'a': '\u{eeee}' });
     expect(parseDataFile('{ "a": \'\\uFFFF\' }'), <String, Object?>{ 'a': '\u{ffff}' });
+    expect(parseDataFile('{ "a": /**/ "1" }'), <String, Object?>{ 'a': '1' });
+    expect(parseDataFile('{ "a": /* */ "1" }'), <String, Object?>{ 'a': '1' });
+    expect(parseDataFile('{ "a": /*\n*/ "1" }'), <String, Object?>{ '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});');

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<void> main(List<String> arguments) async {
   // This script is mentioned in the README.md file.
 
@@ -54,17 +77,78 @@ Future<void> main(List<String> arguments) async {
     exit(0);
   }
 
+  final List<File> libFiles = Directory('lib')
+      .listSync(recursive: true)
+      .whereType<File>()
+      .where((File file) => file.path.endsWith('.dart'))
+      .toList();
+  final Set<LcovLine> flakyLines = <LcovLine>{};
+  final Set<LcovLine> deadLines = <LcovLine>{};
+  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<lcov.Record> 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;
+    if (record.lines != null) {
-    coveredLines += record.lines?.hit ?? 0;
+      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 (totalLines == 0 || totalLines < coveredLines) {
+            }
-    print('Failed to compute coverage.');
+            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 (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<void> main(List<String> 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,14 +184,24 @@ Future<void> main(List<String> 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);
+    }
   }
 
   coverageDirectory.deleteSync(recursive: true);

packages/rfw/test_coverage/pubspec.yaml

@@ -8,3 +8,4 @@ environment:
 
 dependencies:
   lcov_parser: 0.1.1
+  meta: ^1.7.0