ConstantReader.revive implementation now matches the documentation (#376)

* Dartfmt.

* Fix markdown.

Author: matanlurey

source_gen/CHANGELOG.md

@@ -1,3 +1,16 @@
+## 0.9.1
+
+* The result of `ConstantReader.revive()` now returns a `Revivable` that assumes
+  access to a private class, constructor, or function _instead_ of `null` where
+  possible. This allows generators that use `part` files to still use this
+  functionality _and_ allows generators that use separate libraries to emit more
+  actionable error messages (i.e. `"cannot use private class _X"`).
+
+* `Revivable.isPrivate` now returns `true` when the underyling class was public
+  but the constructor was private, or the `Revivable` was pointing to a
+  top-level or static private field or method. Previously it was only `true`
+  when referencing a private class.
+
 ## 0.9.0+1
 
 * Fix `LibraryReader.classElements` to return classes from parts, if they exist,

source_gen/lib/src/constants/revive.dart

@@ -34,8 +34,8 @@ Revivable reviveInstance(DartObject object, [LibraryElement origin]) {
       accessor: '${element.enclosingElement.name}.${element.name}',
     );
   }
-  final clazz = element as ClassElement;
   // Enums are not included in .definingCompilationUnit.types.
+  final clazz = element as ClassElement;
   if (clazz.isEnum) {
     for (final e in clazz.fields.where(
         (f) => f.isPublic && f.isConst && f.computeConstantValue() == object)) {
@@ -45,39 +45,55 @@ Revivable reviveInstance(DartObject object, [LibraryElement origin]) {
       );
     }
   }
+
+  // We try and return a public accessor/constructor if available.
+  final allResults = <Revivable>[];
+
+  /// Returns whether [result] is an acceptable result to immediately return.
+  bool tryResult(Revivable result) {
+    allResults.add(result);
+    return !result.isPrivate;
+  }
+
   for (final e in origin.definingCompilationUnit.types
       .expand((t) => t.fields)
-      .where((f) =>
-          f.isPublic && f.isConst && f.computeConstantValue() == object)) {
-    return Revivable._(
+      .where((f) => f.isConst && f.computeConstantValue() == object)) {
+    final result = Revivable._(
       source: url.removeFragment(),
       accessor: '${clazz.name}.${e.name}',
     );
+    if (tryResult(result)) {
+      return result;
+    }
   }
   final i = (object as DartObjectImpl).getInvocation();
-  if (i != null &&
-      i.constructor.isPublic &&
-      i.constructor.enclosingElement.isPublic) {
+  if (i != null) {
     url = Uri.parse(urlOfElement(i.constructor.enclosingElement));
-    return Revivable._(
+    final result = Revivable._(
       source: url,
       accessor: i.constructor.name,
       namedArguments: i.namedArguments,
       positionalArguments: i.positionalArguments,
     );
+    if (tryResult(result)) {
+      return result;
+    }
   }
-  if (origin == null) {
-    return null;
-  }
-  for (final e in origin.definingCompilationUnit.topLevelVariables.where(
-    (f) => f.isPublic && f.isConst && f.computeConstantValue() == object,
-  )) {
-    return Revivable._(
-      source: Uri.parse(urlOfElement(origin)).replace(fragment: ''),
-      accessor: e.name,
-    );
+  if (origin != null) {
+    for (final e in origin.definingCompilationUnit.topLevelVariables.where(
+      (f) => f.isConst && f.computeConstantValue() == object,
+    )) {
+      final result = Revivable._(
+        source: Uri.parse(urlOfElement(origin)).replace(fragment: ''),
+        accessor: e.name,
+      );
+      if (tryResult(result)) {
+        return result;
+      }
+    }
   }
-  return null;
+  // We could try and return the "best" result more intelligently.
+  return allResults.first;
 }
 
 /// Decoded "instructions" for re-creating a const [DartObject] at runtime.
@@ -113,5 +129,18 @@ class Revivable {
   ///
   /// Builds tools may use this to fail when the symbol is expected to be
   /// importable (i.e. isn't used with `part of`).
-  bool get isPrivate => accessor.startsWith('_');
+  bool get isPrivate {
+    return source.fragment.startsWith('_') || accessor.startsWith('_');
+  }
+
+  @override
+  String toString() {
+    if (source.fragment.isNotEmpty) {
+      if (accessor.isEmpty) {
+        return 'const $source';
+      }
+      return 'const $source.$accessor';
+    }
+    return '$source::$accessor';
+  }
 }

source_gen/pubspec.yaml

@@ -1,5 +1,5 @@
 name: source_gen
-version: 0.9.1-dev
+version: 0.9.1
 author: Dart Team <[email protected]>
 description: Automated source code generation for Dart.
 homepage: https://github.com/dart-lang/source_gen

source_gen/test/constants_test.dart

@@ -197,8 +197,12 @@ void main() {
         @ClassWithStaticField.staticField
         @Wrapper(someFunction)
         @Wrapper(Wrapper.someFunction)
+        @_NotAccessible()
+        @PublicWithPrivateConstructor._()
+        @_privateField
+        @Wrapper(_privateFunction)
         class Example {}
-        
+
         class Int64Like {
           static const Int64Like ZERO = const Int64Like._bits(0, 0, 0);
         
@@ -248,6 +252,16 @@ void main() {
         }
 
         void someFunction(int x, String y) {}
+
+        class _NotAccessible {
+          const _NotAccessible();
+        }
+
+        class PublicWithPrivateConstructor {
+          const PublicWithPrivateConstructor._();
+        }
+
+        void _privateFunction() {}
       ''', (resolver) => resolver.findLibraryByName('test_lib'));
       constants = library
           .getType('Example')
@@ -315,5 +329,25 @@ void main() {
       expect(fieldOnly.source.fragment, isEmpty);
       expect(fieldOnly.accessor, 'Wrapper.someFunction');
     });
+
+    test('should decode private classes', () {
+      final notAccessible = constants[9].revive();
+      expect(notAccessible.isPrivate, isTrue);
+      expect(notAccessible.source.fragment, '_NotAccessible');
+    });
+
+    test('should decode private constructors', () {
+      final notAccessible = constants[10].revive();
+      expect(notAccessible.isPrivate, isTrue);
+      expect(notAccessible.source.fragment, 'PublicWithPrivateConstructor');
+      expect(notAccessible.accessor, '_');
+    });
+
+    test('should decode private functions', () {
+      final function = constants[12].read('f').revive();
+      expect(function.isPrivate, isTrue);
+      expect(function.source.fragment, isEmpty);
+      expect(function.accessor, '_privateFunction');
+    });
   });
 }