Fix and test permissive compare in proto3json (google#329)

protobuf/CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 1.0.2
 
 * Fix hashcode of bytes fields.
+* Fix issue with the `permissiveEnums` option to `mergeFromProto3Json`.
+  The comparison did not work properly.
 
 ## 1.0.1
 

protobuf/lib/protobuf.dart

@@ -14,6 +14,7 @@ import 'dart:typed_data' show TypedData, Uint8List, ByteData, Endian;
 import 'package:fixnum/fixnum.dart' show Int64;
 
 import 'src/protobuf/json_parsing_context.dart';
+import 'src/protobuf/permissive_compare.dart';
 import 'src/protobuf/type_registry.dart';
 export 'src/protobuf/type_registry.dart' show TypeRegistry;
 

protobuf/lib/src/protobuf/permissive_compare.dart

@@ -0,0 +1,41 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/// Returns true if [a] and [b] are the same ignoring case and all instances of
+///  `-` and `_`.
+///
+/// This is specialized code for comparing enum names.
+/// Works only for ascii strings containing letters and `_` and `-`.
+bool permissiveCompare(String a, String b) {
+  const dash = 45;
+  const underscore = 95;
+
+  int i = 0;
+  int j = 0;
+
+  while (true) {
+    int ca, cb;
+    do {
+      ca = i < a.length ? a.codeUnitAt(i++) : -1;
+    } while (ca == dash || ca == underscore);
+    do {
+      cb = j < b.length ? b.codeUnitAt(j++) : -1;
+    } while (cb == dash || cb == underscore);
+    if (ca == cb) {
+      if (ca == -1) return true; // Both at end
+      continue;
+    }
+    if (ca ^ cb != 0x20 || !_isAsciiLetter(ca)) {
+      return false;
+    }
+  }
+}
+
+bool _isAsciiLetter(int char) {
+  const lowerA = 97;
+  const lowerZ = 122;
+  const capitalA = 65;
+  char |= lowerA ^ capitalA;
+  return lowerA <= char && char <= lowerZ;
+}

protobuf/lib/src/protobuf/proto3_json.dart

@@ -198,7 +198,7 @@ void _mergeFromProto3Json(
             // TODO(sigurdm): Do we want to avoid linear search here? Measure...
             final result = permissiveEnums
                 ? fieldInfo.enumValues.firstWhere(
-                    (e) => _permissiveCompare(e.name, value),
+                    (e) => permissiveCompare(e.name, value),
                     orElse: () => null)
                 : fieldInfo.enumValues
                     .firstWhere((e) => e.name == value, orElse: () => null);
@@ -405,42 +405,3 @@ void _mergeFromProto3Json(
 
   recursionHelper(json, fieldSet);
 }
-
-bool _isAsciiLetter(int char) {
-  const lowerA = 97;
-  const lowerZ = 122;
-  const capitalA = 65;
-  char |= lowerA ^ capitalA;
-  return lowerA <= char && char <= lowerZ;
-}
-
-/// Returns true if [a] and [b] are the same ignoring case and all instances of
-///  `-` and `_`.
-bool _permissiveCompare(String a, String b) {
-  const dash = 45;
-  const underscore = 95;
-
-  // Enum names are always ascii.
-  int i = 0;
-  int j = 0;
-
-  outer:
-  while (i < a.length && j < b.length) {
-    int ca = a.codeUnitAt(i);
-    if (ca == dash || ca == underscore) {
-      i++;
-      continue;
-    }
-    int cb = b.codeUnitAt(j);
-    while (cb == dash || cb == underscore) {
-      j++;
-      if (j == b.length) break outer;
-      cb = b.codeUnitAt(j);
-    }
-
-    if (ca != cb && (ca ^ cb != 0x20 || !_isAsciiLetter(ca))) return false;
-    i++;
-    j++;
-  }
-  return true;
-}

protobuf/test/permissive_compare_test.dart

@@ -0,0 +1,46 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. 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 'package:protobuf/src/protobuf/permissive_compare.dart';
+
+void main() {
+  void symmetric(String a, String b, bool expected) {
+    expect(permissiveCompare(a, b), expected);
+    expect(permissiveCompare(b, a), expected);
+  }
+
+  List<String> variationsFromSeed(String seed) {
+    final result = [
+      seed,
+      seed.toUpperCase(),
+      '-$seed',
+      '-${seed.toUpperCase()}',
+      '_$seed',
+      '_${seed.toUpperCase()}',
+      '$seed-',
+      '${seed}_',
+    ];
+    if (2 <= seed.length) {
+      result.add('${seed.substring(0, 1)}_${seed.substring(1)}');
+      result.add('${seed.substring(0, 1)}-${seed.substring(1)}');
+      result.add('${seed.substring(0, 1).toUpperCase()}${seed.substring(1)}');
+      result.add('${seed.substring(0, 1)}${seed.substring(1).toUpperCase()}');
+    }
+    return result;
+  }
+
+  test('permissive compare', () {
+    final seeds = ['', 'a', 'b', 'aa', 'ab', 'bb', 'aaaa'];
+    for (final a in seeds) {
+      for (final aVariant in variationsFromSeed(a)) {
+        for (final b in seeds) {
+          for (final bVariant in variationsFromSeed(b)) {
+            symmetric(aVariant, bVariant, a == b);
+          }
+        }
+      }
+    }
+  });
+}