diff --git a/source_gen/CHANGELOG.md b/source_gen/CHANGELOG.md index ea2fd651..b5d744c8 100644 --- a/source_gen/CHANGELOG.md +++ b/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, diff --git a/source_gen/lib/src/constants/revive.dart b/source_gen/lib/src/constants/revive.dart index c8cff0ae..09946085 100644 --- a/source_gen/lib/src/constants/revive.dart +++ b/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 = []; + + /// 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'; + } } diff --git a/source_gen/pubspec.yaml b/source_gen/pubspec.yaml index d2302ef1..1e4b2c33 100644 --- a/source_gen/pubspec.yaml +++ b/source_gen/pubspec.yaml @@ -1,5 +1,5 @@ name: source_gen -version: 0.9.1-dev +version: 0.9.1 author: Dart Team description: Automated source code generation for Dart. homepage: https://github.com/dart-lang/source_gen diff --git a/source_gen/test/constants_test.dart b/source_gen/test/constants_test.dart index 9ded565d..b9d4779b 100644 --- a/source_gen/test/constants_test.dart +++ b/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'); + }); }); }