From 7f009e528d84db76e4f06517e412c8a3bab8df42 Mon Sep 17 00:00:00 2001 From: Nicholas Shahan Date: Mon, 5 Apr 2021 18:54:56 +0000 Subject: [PATCH] [ddc] Fix private class exposed by typedef - Ensure public fields of private classes exposed by typedef are properly treated as virtual. - Add modular test that exercises the errors fixed by this change. Change-Id: If97f66de5b0742612024a2f727d83ca9dc73389c Fixes: https://github.com/dart-lang/sdk/issues/45512 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193899 Reviewed-by: Nicholas Shahan Reviewed-by: Sigmund Cherem Reviewed-by: Mark Zhou Commit-Queue: Nicholas Shahan --- .../lib/src/kernel/property_model.dart | 23 +++++-- .../main.dart | 61 +++++++++++++++++++ .../modules.yaml | 8 +++ .../private_name_library.dart | 39 ++++++++++++ 4 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 tests/modular/private_class_exposed_by_typedef/main.dart create mode 100644 tests/modular/private_class_exposed_by_typedef/modules.yaml create mode 100644 tests/modular/private_class_exposed_by_typedef/private_name_library.dart diff --git a/pkg/dev_compiler/lib/src/kernel/property_model.dart b/pkg/dev_compiler/lib/src/kernel/property_model.dart index 5e403c2f852..fc403b30014 100644 --- a/pkg/dev_compiler/lib/src/kernel/property_model.dart +++ b/pkg/dev_compiler/lib/src/kernel/property_model.dart @@ -46,20 +46,35 @@ class _LibraryVirtualFieldModel { /// Private classes that can be extended outside of this library. /// /// Normally private classes cannot be accessed outside this library, however, - /// this can happen if they are extended by a public class, for example: + /// this can happen if they are extended by a public class or exposed by a + /// public typedef, for example: /// /// class _A { int x = 42; } /// class _B { int x = 42; } + /// class _C { int x = 42; } /// - /// // _A is now effectively public for the purpose of overrides. - /// class C extends _A {} + /// // _A and _B are now effectively public for the purpose of overrides. + /// class D extends _A {} + /// typedef E = _B; /// - /// The class _A must treat is "x" as virtual, however _B does not. + /// The classes _A and _B must treat is "x" as virtual, however _C does not. final _extensiblePrivateClasses = HashSet(); _LibraryVirtualFieldModel.build(Library library) { var allClasses = library.classes; + for (var typedef in library.typedefs) { + // Ignore private typedefs. + if (typedef.name.startsWith('_')) continue; + + var type = typedef.type; + if (type is InterfaceType && type.classNode.name.startsWith('_')) { + // Public typedefs of private classes expose those classes for + // extension. + _extensiblePrivateClasses.add(type.classNode); + } + } + // The set of public types is our initial extensible type set. // From there, visit all immediate private types in this library, and so on // from those private types, marking them as extensible. diff --git a/tests/modular/private_class_exposed_by_typedef/main.dart b/tests/modular/private_class_exposed_by_typedef/main.dart new file mode 100644 index 00000000000..70c26d1ce85 --- /dev/null +++ b/tests/modular/private_class_exposed_by_typedef/main.dart @@ -0,0 +1,61 @@ +// Copyright (c) 2021, 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:expect/expect.dart"; + +import "private_name_library.dart"; + +// These tests are adapted from the set located in +// language/nonfunction_type_aliases/private_names/ to highlight a specific +// DDC issue with public fields in private classes. + +void main() { + test1(); + test2(); +} + +/// Extend a private class via a public typedef without overriding any methods. +class Derived extends PublicClass { + Derived() : super(); +} + +/// Extend a private class via a public typedef overriding methods and +/// properties. The final field `x` is overriden with a getter which returns +/// different values every time it is called. +class AlsoDerived extends AlsoPublicClass { + int backingStore = publicNameSentinel; + int get x => ++backingStore; + int get y => super.x; + AlsoDerived() : super.named(privateNameSentinel); +} + +/// Test that inherited properties work correctly +void test1() { + var p = Derived(); + // Reading the virtual field should give the private value + Expect.equals(privateNameSentinel, p.x); + // Reading the virtual field from the private library should give the private + // value + Expect.equals(privateNameSentinel, readInstanceField(p)); +} + +/// Test that overridden properties work correctly. +void test2() { + var p = AlsoDerived(); + // Reading the original virtual field should give the private value. + Expect.equals(privateNameSentinel, p.y); + // Reading the overriding getter from the private library should give the + // public value and increment it each time it is called. + Expect.equals(publicNameSentinel, p.backingStore); + Expect.equals(publicNameSentinel + 1, readInstanceField2(p)); + Expect.equals(publicNameSentinel + 1, p.backingStore); + // Reading the overriding getter from the original library should give the + // public value and increment it each time it is called. + Expect.equals(publicNameSentinel + 2, p.x); + Expect.equals(publicNameSentinel + 2, p.backingStore); + Expect.equals(privateNameSentinel, p.y); + Expect.equals(publicNameSentinel + 2, p.backingStore); + Expect.equals(publicNameSentinel + 3, readInstanceField2(p)); + Expect.equals(publicNameSentinel + 4, p.x); +} diff --git a/tests/modular/private_class_exposed_by_typedef/modules.yaml b/tests/modular/private_class_exposed_by_typedef/modules.yaml new file mode 100644 index 00000000000..530016ea900 --- /dev/null +++ b/tests/modular/private_class_exposed_by_typedef/modules.yaml @@ -0,0 +1,8 @@ +# Copyright (c) 2021, 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. +# +dependencies: + main: [expect, private_name_library] +flags: + - nonfunction-type-aliases diff --git a/tests/modular/private_class_exposed_by_typedef/private_name_library.dart b/tests/modular/private_class_exposed_by_typedef/private_name_library.dart new file mode 100644 index 00000000000..74559cc7c97 --- /dev/null +++ b/tests/modular/private_class_exposed_by_typedef/private_name_library.dart @@ -0,0 +1,39 @@ +// Copyright (c) 2021, 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. +// Shared code for tests that private names exported publicly via a typedef work +// as expected. +library private; + +/// Sentinel values for checking that the correct methods are called. +const int privateNameSentinel = -1; +const int publicNameSentinel = privateNameSentinel + 1; + +/// A private class that will be exported via a public typedef. +class _PrivateClass { + int x; + _PrivateClass(): x = privateNameSentinel; + _PrivateClass.named(this.x); +} + +class _PrivateClass2 { + int x; + _PrivateClass2(): x = privateNameSentinel; + _PrivateClass2.named(this.x); +} + +/// Export the private class publicly. +typedef PublicClass = _PrivateClass; + +/// Export the private class publicly via an indirection through another private +/// typedef. +typedef _PrivateTypeDef = _PrivateClass2; +typedef AlsoPublicClass = _PrivateTypeDef; + +/// Helper methods to do virtual calls on instances of _PrivateClass in this +/// library context. +int readInstanceField(_PrivateClass other) => other.x; + +/// Helper methods to do virtual calls on instances of _PrivateClass in this +/// library context. +int readInstanceField2(_PrivateClass2 other) => other.x;