From 8030eeba085cee1136e62880d3af91ea08a34051 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 16 Oct 2024 21:21:10 +0000 Subject: [PATCH 1/7] add lazy merged map impl and extension for merging model objects --- pkgs/dart_model/lib/dart_model.dart | 1 + pkgs/dart_model/lib/src/dart_model.dart | 22 +++++- pkgs/dart_model/lib/src/lazy_merged_map.dart | 78 +++++++++++++++++++ .../dart_model/test/lazy_merged_map_test.dart | 66 ++++++++++++++++ pkgs/dart_model/test/model_test.dart | 22 ++++++ 5 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 pkgs/dart_model/lib/src/lazy_merged_map.dart create mode 100644 pkgs/dart_model/test/lazy_merged_map_test.dart diff --git a/pkgs/dart_model/lib/dart_model.dart b/pkgs/dart_model/lib/dart_model.dart index f1d5f905..cd839cae 100644 --- a/pkgs/dart_model/lib/dart_model.dart +++ b/pkgs/dart_model/lib/dart_model.dart @@ -5,6 +5,7 @@ export 'src/dart_model.dart'; export 'src/json.dart'; export 'src/json_changes.dart'; +export 'src/lazy_merged_map.dart' show MergeModels; export 'src/scopes.dart'; export 'src/type.dart'; export 'src/type_system.dart'; diff --git a/pkgs/dart_model/lib/src/dart_model.dart b/pkgs/dart_model/lib/src/dart_model.dart index cf4257b7..ea958cef 100644 --- a/pkgs/dart_model/lib/src/dart_model.dart +++ b/pkgs/dart_model/lib/src/dart_model.dart @@ -4,6 +4,7 @@ import 'dart_model.g.dart'; import 'json_buffer/json_buffer_builder.dart'; +import 'lazy_merged_map.dart'; export 'dart_model.g.dart'; @@ -56,12 +57,25 @@ extension ModelExtension on Model { throw ArgumentError('Value not in map: $value, $map'); } - /// Gets the `Map` that contains [node], or `null` if there isn't one. - Map? _getParent(Map node) { + /// Gets the `Map` that contains [child], or `null` if there isn't one. + Map? _getParent(Map child) { + // All the MapInBuffer maps for `map`, checking for the left and right maps + // of lazy merged maps recursively. + Iterable bufferMaps(Map map) sync* { + switch (map) { + case LazyMergedMapView map: + yield* bufferMaps(map.left); + yield* bufferMaps(map.right); + case MapInBuffer map: + yield map; + } + } + // If both maps are in the same `JsonBufferBuilder` then the parent is // immediately available. - if (this case MapInBuffer thisMapInBuffer) { - if (node case MapInBuffer thatMapInBuffer) { + final childBufferMaps = bufferMaps(child); + for (final thisMapInBuffer in bufferMaps(node)) { + for (final thatMapInBuffer in childBufferMaps) { if (thisMapInBuffer.buffer == thatMapInBuffer.buffer) { return thatMapInBuffer.parent; } diff --git a/pkgs/dart_model/lib/src/lazy_merged_map.dart b/pkgs/dart_model/lib/src/lazy_merged_map.dart new file mode 100644 index 00000000..dd35aaf2 --- /dev/null +++ b/pkgs/dart_model/lib/src/lazy_merged_map.dart @@ -0,0 +1,78 @@ +// Copyright (c) 2024, 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 'dart:collection'; + +import 'dart_model.dart'; + +/// An implementation of a lazy merged json [Map] view over two [Map]s. +/// +/// The intended use case is for merging JSON payloads together into a single +/// payload, where their structure is the same. +/// +/// If both maps have the same key present, the logic for the values of those +/// shared keys goes as follows: +/// +/// - If both values are `Map`, a nested [LazyMergedMapView] +/// is returned. +/// - Else if they are equal values, the value from [left] is returned. +/// - Else a [StateError] is thrown. +/// +/// Nested [List]s are not specifically handled at this time and must be equal. +/// +/// The [keys] getter will de-duplicate the keys. +class LazyMergedMapView extends MapBase { + final Map left; + final Map right; + + LazyMergedMapView(this.left, this.right); + + @override + Object? operator [](Object? key) { + var leftValue = left[key]; + var rightValue = right[key]; + if (leftValue != null) { + if (rightValue != null) { + if (leftValue is Map && + rightValue is Map) { + return LazyMergedMapView(leftValue, rightValue); + } + if (leftValue != rightValue) { + throw StateError('Cannot merge maps with different values, and ' + '$leftValue != $rightValue'); + } + return leftValue; + } + return leftValue; + } else if (rightValue != null) { + return rightValue; + } + return null; + } + + @override + void operator []=(String key, Object? value) => + throw UnsupportedError('Merged maps are read only'); + + @override + void clear() => throw UnsupportedError('Merged maps are read only'); + + @override + Iterable get keys sync* { + var seen = {}; + for (var key in left.keys.followedBy(right.keys)) { + if (seen.add(key)) yield key; + } + } + + @override + Object? remove(Object? key) => + throw UnsupportedError('Merged maps are read only'); +} + +extension MergeModels on Model { + /// Creates a lazy merged view of `this` with [other]. + Model mergeWith(Model other) => + Model.fromJson(LazyMergedMapView(node, other.node)); +} diff --git a/pkgs/dart_model/test/lazy_merged_map_test.dart b/pkgs/dart_model/test/lazy_merged_map_test.dart new file mode 100644 index 00000000..95e6126e --- /dev/null +++ b/pkgs/dart_model/test/lazy_merged_map_test.dart @@ -0,0 +1,66 @@ +// Copyright (c) 2024, 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:dart_model/dart_model.dart'; +import 'package:dart_model/src/lazy_merged_map.dart'; +import 'package:test/test.dart' hide test; +import 'package:test/test.dart' as t show test; + +void main() { + test('Can merge models with different libraries', () async { + final libA = Library(); + final libB = Library(); + final a = Model()..uris['package:a/a.dart'] = libA; + final b = Model()..uris['package:b/b.dart'] = libB; + final c = a.mergeWith(b); + expect(c.uris['package:a/a.dart'], libA); + expect(c.uris['package:b/b.dart'], libB); + }); + + test('Can merge models with different scopes from the same library', + () async { + final interfaceA = Interface(); + final interfaceB = Interface(); + final a = Model() + ..uris['package:a/a.dart'] = (Library()..scopes['A'] = interfaceA); + final b = Model() + ..uris['package:a/a.dart'] = (Library()..scopes['B'] = interfaceB); + final c = a.mergeWith(b); + expect(c.uris['package:a/a.dart'], isA()); + expect(c.uris['package:a/a.dart']!.scopes['A'], interfaceA); + expect(c.uris['package:a/a.dart']!.scopes['B'], interfaceB); + }); + + test('Can merge models with the same interface but different properties', + () async { + final interfaceA1 = Interface(properties: Properties(isClass: true)); + final interfaceA2 = Interface(properties: Properties(isAbstract: true)); + final a = Model() + ..uris['package:a/a.dart'] = (Library()..scopes['A'] = interfaceA1); + final b = Model() + ..uris['package:a/a.dart'] = (Library()..scopes['A'] = interfaceA2); + final c = a.mergeWith(b); + final properties = c.uris['package:a/a.dart']!.scopes['A']!.properties; + expect(properties, isA()); + expect(properties.isClass, true); + expect(properties.isAbstract, true); + // Not set + expect(() => properties.isConstructor, throwsA(isA())); + }); + + test('Errors if identical keys are not equal', () async { + final interfaceA1 = Interface(properties: Properties(isClass: true)); + final interfaceA2 = Interface(properties: Properties(isClass: false)); + final a = Model() + ..uris['package:a/a.dart'] = (Library()..scopes['A'] = interfaceA1); + final b = Model() + ..uris['package:a/a.dart'] = (Library()..scopes['A'] = interfaceA2); + final c = a.mergeWith(b); + expect(() => c.uris['package:a/a.dart']!.scopes['A']!.properties.isClass, + throwsA(isA())); + }); +} + +void test(String description, void Function() body) => + t.test(description, () => Scope.query.run(body)); diff --git a/pkgs/dart_model/test/model_test.dart b/pkgs/dart_model/test/model_test.dart index 7ffbbf6f..d798c794 100644 --- a/pkgs/dart_model/test/model_test.dart +++ b/pkgs/dart_model/test/model_test.dart @@ -123,6 +123,28 @@ void main() { 'package:dart_model/dart_model.dart#JsonData'); }); + test('can give the path to Members in merged maps', () { + Scope.macro.run(() { + final rootMember = model.uris['package:dart_model/dart_model.dart']! + .scopes['JsonData']!.members['_root']!; + + final fooMember = Member(); + final otherModel = Model() + ..uris['package:dart_model/dart_model.dart'] = (Library() + ..scopes['JsonData'] = (Interface()..members['foo'] = fooMember)); + final mergedModel = model.mergeWith(otherModel); + + // TODO: Once QualifiedName works better, this will be a better test. + expect(mergedModel.qualifiedNameOfMember(rootMember)!.asString, + 'package:dart_model/dart_model.dart#JsonData'); + expect(mergedModel.qualifiedNameOfMember(fooMember)!.asString, + 'package:dart_model/dart_model.dart#JsonData'); + + expect(model.qualifiedNameOfMember(fooMember), null); + expect(otherModel.qualifiedNameOfMember(rootMember), null); + }); + }); + test('path to Members throws on cycle', () { final copiedModel = Model.fromJson(_copyMap(model.node)); // Add an invalid link creating a loop in the map structure. From a621fc99a9fb27d258ea0c82b283d8f972045465 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Thu, 17 Oct 2024 17:10:28 +0000 Subject: [PATCH 2/7] use string instead of type for test group --- pkgs/dart_model/test/model_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/dart_model/test/model_test.dart b/pkgs/dart_model/test/model_test.dart index d798c794..d01eedd1 100644 --- a/pkgs/dart_model/test/model_test.dart +++ b/pkgs/dart_model/test/model_test.dart @@ -8,7 +8,7 @@ import 'package:dart_model/dart_model.dart'; import 'package:test/test.dart'; void main() { - group(Model, () { + group('Model', () { late Model model; setUp(() { From e612cbf1b9c453414a14099036f18b4cce6d9408 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 21 Oct 2024 16:35:12 +0000 Subject: [PATCH 3/7] actually merge models in addModel --- pkgs/dart_model/lib/src/scopes.dart | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkgs/dart_model/lib/src/scopes.dart b/pkgs/dart_model/lib/src/scopes.dart index 99e1e358..c2cb609f 100644 --- a/pkgs/dart_model/lib/src/scopes.dart +++ b/pkgs/dart_model/lib/src/scopes.dart @@ -157,10 +157,12 @@ class MacroScope { /// Merges a new partial [model] into the scope's accumulated model spanning /// multiple queries. - // TODO: Actually accumulate instead of replacing the model void addModel(Model model) { - _accumulatedModel = model; - _typeSystem = null; + if (_accumulatedModel case var accumulated?) { + accumulated.mergeWith(model); + } else { + _accumulatedModel = model; + } } static MacroScope get current { From 0e1d46870eab14678d07efaca0d16ade5a40364e Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 21 Oct 2024 16:38:04 +0000 Subject: [PATCH 4/7] add todo and rename test --- pkgs/dart_model/lib/src/lazy_merged_map.dart | 1 + pkgs/dart_model/test/lazy_merged_map_test.dart | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/dart_model/lib/src/lazy_merged_map.dart b/pkgs/dart_model/lib/src/lazy_merged_map.dart index dd35aaf2..9b001895 100644 --- a/pkgs/dart_model/lib/src/lazy_merged_map.dart +++ b/pkgs/dart_model/lib/src/lazy_merged_map.dart @@ -30,6 +30,7 @@ class LazyMergedMapView extends MapBase { @override Object? operator [](Object? key) { + // TODO: Can we do better? These lookups can each be linear for buffer maps. var leftValue = left[key]; var rightValue = right[key]; if (leftValue != null) { diff --git a/pkgs/dart_model/test/lazy_merged_map_test.dart b/pkgs/dart_model/test/lazy_merged_map_test.dart index 95e6126e..c7384857 100644 --- a/pkgs/dart_model/test/lazy_merged_map_test.dart +++ b/pkgs/dart_model/test/lazy_merged_map_test.dart @@ -49,7 +49,7 @@ void main() { expect(() => properties.isConstructor, throwsA(isA())); }); - test('Errors if identical keys are not equal', () async { + test('Errors if maps have same the key with different values', () async { final interfaceA1 = Interface(properties: Properties(isClass: true)); final interfaceA2 = Interface(properties: Properties(isClass: false)); final a = Model() From 04f5c9972e0b341e554debbdc022d59ca81d094f Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 21 Oct 2024 16:52:00 +0000 Subject: [PATCH 5/7] fixes after merge --- pkgs/dart_model/test/model_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/dart_model/test/model_test.dart b/pkgs/dart_model/test/model_test.dart index d0804c40..a820031a 100644 --- a/pkgs/dart_model/test/model_test.dart +++ b/pkgs/dart_model/test/model_test.dart @@ -152,13 +152,13 @@ void main() { final mergedModel = model.mergeWith(otherModel); // TODO: Once QualifiedName works better, this will be a better test. - expect(mergedModel.qualifiedNameOfMember(rootMember)!.asString, + expect(mergedModel.qualifiedNameOf(rootMember.node)!.asString, 'package:dart_model/dart_model.dart#JsonData'); - expect(mergedModel.qualifiedNameOfMember(fooMember)!.asString, + expect(mergedModel.qualifiedNameOf(fooMember.node)!.asString, 'package:dart_model/dart_model.dart#JsonData'); - expect(model.qualifiedNameOfMember(fooMember), null); - expect(otherModel.qualifiedNameOfMember(rootMember), null); + expect(model.qualifiedNameOf(fooMember.node), null); + expect(otherModel.qualifiedNameOf(rootMember.node), null); }); }); From 88d1009a7b09b5651e7b2be3a18882dd144c91cf Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 21 Oct 2024 19:41:06 +0000 Subject: [PATCH 6/7] fix tests, clean up --- pkgs/dart_model/lib/src/dart_model.dart | 56 +++++++++++++------- pkgs/dart_model/lib/src/lazy_merged_map.dart | 12 +++++ pkgs/dart_model/test/model_test.dart | 37 +++++++------ 3 files changed, 71 insertions(+), 34 deletions(-) diff --git a/pkgs/dart_model/lib/src/dart_model.dart b/pkgs/dart_model/lib/src/dart_model.dart index c2d94f76..590629bf 100644 --- a/pkgs/dart_model/lib/src/dart_model.dart +++ b/pkgs/dart_model/lib/src/dart_model.dart @@ -33,20 +33,49 @@ extension ModelExtension on Model { /// Returns the [QualifiedName] in the model to [node], or `null` if [node] /// is not in this [Model]. - QualifiedName? _qualifiedNameOf(Map node) { - var parent = _getParent(node); + QualifiedName? _qualifiedNameOf(Map model) { + var parent = _getParent(model); if (parent == null) return null; final path = []; - path.add(_keyOf(node, parent)); + path.add(_keyOf(model, parent)); var previousParent = parent; - while ((parent = _getParent(previousParent)) != this.node) { + + // Checks if any merged map of `left` == any merged map of `right. + bool isEqualNested(Map left, Map right) { + if (left == right) return true; + if (left case final LazyMergedMapView merged) { + if (merged.expand.any((map) => isEqualNested(right, map))) { + return true; + } + } + if (right case final LazyMergedMapView merged) { + if (merged.expand.any((map) => isEqualNested(left, map))) { + return true; + } + } + return false; + } + + while (true) { + parent = _getParent(previousParent); if (parent == null) return null; + + /// We reached this models node, stop searching higher. + if (isEqualNested(parent, node)) break; + path.insert(0, _keyOf(previousParent, parent)); previousParent = parent; } if (path case [final uri, 'scopes', final name]) { return QualifiedName(uri: uri, name: name); + } else if (path + case [final uri, 'scopes', final scope, 'members', final name]) { + return QualifiedName( + uri: uri, + scope: scope, + name: name, + isStatic: Member.fromJson(model).properties.isStatic); } throw UnsupportedError( 'Unsupported node type for `qualifiedNameOf`, only top level members ' @@ -65,22 +94,11 @@ extension ModelExtension on Model { /// Gets the `Map` that contains [child], or `null` if there isn't one. Map? _getParent(Map child) { - // All the MapInBuffer maps for `map`, checking for the left and right maps - // of lazy merged maps recursively. - Iterable bufferMaps(Map map) sync* { - switch (map) { - case LazyMergedMapView map: - yield* bufferMaps(map.left); - yield* bufferMaps(map.right); - case MapInBuffer map: - yield map; - } - } - // If both maps are in the same `JsonBufferBuilder` then the parent is // immediately available. - final childBufferMaps = bufferMaps(child); - for (final thisMapInBuffer in bufferMaps(node)) { + final childMaps = child.expand; + final childBufferMaps = childMaps.whereType(); + for (final thisMapInBuffer in node.expand.whereType()) { for (final thatMapInBuffer in childBufferMaps) { if (thisMapInBuffer.buffer == thatMapInBuffer.buffer) { return thatMapInBuffer.parent; @@ -88,7 +106,7 @@ extension ModelExtension on Model { } } // Otherwise, build a `Map` of references to parents and use that. - return _lazyParentsMap[node]; + return _lazyParentsMap[child]; } /// Gets a `Map` from values to parent `Map`s. diff --git a/pkgs/dart_model/lib/src/lazy_merged_map.dart b/pkgs/dart_model/lib/src/lazy_merged_map.dart index 9b001895..2b6e2198 100644 --- a/pkgs/dart_model/lib/src/lazy_merged_map.dart +++ b/pkgs/dart_model/lib/src/lazy_merged_map.dart @@ -72,6 +72,18 @@ class LazyMergedMapView extends MapBase { throw UnsupportedError('Merged maps are read only'); } +extension AllMaps on Map { + /// All the maps merged into this map, recursively expanded. + Iterable> get expand sync* { + if (this case final LazyMergedMapView self) { + yield* self.left.expand; + yield* self.right.expand; + } else { + yield this; + } + } +} + extension MergeModels on Model { /// Creates a lazy merged view of `this` with [other]. Model mergeWith(Model other) => diff --git a/pkgs/dart_model/test/model_test.dart b/pkgs/dart_model/test/model_test.dart index a820031a..f9cc4a30 100644 --- a/pkgs/dart_model/test/model_test.dart +++ b/pkgs/dart_model/test/model_test.dart @@ -24,7 +24,7 @@ void main() { name: 'SomeAnnotation')) ]) ..members['_root'] = Member( - properties: Properties(isField: true), + properties: Properties(isField: true, isStatic: false), ))); }); }); @@ -44,7 +44,7 @@ void main() { ], 'members': { '_root': { - 'properties': {'isField': true} + 'properties': {'isField': true, 'isStatic': false} } }, 'properties': {'isClass': true} @@ -118,9 +118,8 @@ void main() { test('can give the path to Members in buffer backed maps', () { final member = model.uris['package:dart_model/dart_model.dart']! .scopes['JsonData']!.members['_root']!; - expect(() => model.qualifiedNameOf(member.node)!.asString, - throwsUnsupportedError, - reason: 'Requires https://github.com/dart-lang/macros/pull/101'); + expect(model.qualifiedNameOf(member.node)!.asString, + 'package:dart_model/dart_model.dart#JsonData._root'); }); test('can give the path to Interfaces in SDK maps', () { @@ -135,27 +134,35 @@ void main() { final copiedModel = Model.fromJson(_copyMap(model.node)); final member = copiedModel.uris['package:dart_model/dart_model.dart']! .scopes['JsonData']!.members['_root']!; - expect(() => copiedModel.qualifiedNameOf(member.node)!.asString, - throwsUnsupportedError, - reason: 'Requires https://github.com/dart-lang/macros/pull/101'); + expect(copiedModel.qualifiedNameOf(member.node)!.asString, + 'package:dart_model/dart_model.dart#JsonData._root'); }); test('can give the path to Members in merged maps', () { + late final Member fooMember; + late final Model otherModel; + + /// Create one model in a different scope so it gets a different buffer. + Scope.query.run(() { + otherModel = Model() + ..uris['package:dart_model/dart_model.dart'] = (Library() + ..scopes['JsonData'] = (Interface() + ..members['foo'] = + Member(properties: Properties(isStatic: true)))); + fooMember = otherModel.uris['package:dart_model/dart_model.dart']! + .scopes['JsonData']!.members['foo']!; + }); + Scope.macro.run(() { final rootMember = model.uris['package:dart_model/dart_model.dart']! .scopes['JsonData']!.members['_root']!; - final fooMember = Member(); - final otherModel = Model() - ..uris['package:dart_model/dart_model.dart'] = (Library() - ..scopes['JsonData'] = (Interface()..members['foo'] = fooMember)); final mergedModel = model.mergeWith(otherModel); - // TODO: Once QualifiedName works better, this will be a better test. expect(mergedModel.qualifiedNameOf(rootMember.node)!.asString, - 'package:dart_model/dart_model.dart#JsonData'); + 'package:dart_model/dart_model.dart#JsonData._root'); expect(mergedModel.qualifiedNameOf(fooMember.node)!.asString, - 'package:dart_model/dart_model.dart#JsonData'); + 'package:dart_model/dart_model.dart#JsonData::foo'); expect(model.qualifiedNameOf(fooMember.node), null); expect(otherModel.qualifiedNameOf(rootMember.node), null); From 294bc7b7e3a632e452f48b1f71a75cdae3ef9c79 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 21 Oct 2024 20:13:38 +0000 Subject: [PATCH 7/7] simplify isEqualsNested --- pkgs/dart_model/lib/src/dart_model.dart | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pkgs/dart_model/lib/src/dart_model.dart b/pkgs/dart_model/lib/src/dart_model.dart index 590629bf..02f7d7cd 100644 --- a/pkgs/dart_model/lib/src/dart_model.dart +++ b/pkgs/dart_model/lib/src/dart_model.dart @@ -43,17 +43,7 @@ extension ModelExtension on Model { // Checks if any merged map of `left` == any merged map of `right. bool isEqualNested(Map left, Map right) { if (left == right) return true; - if (left case final LazyMergedMapView merged) { - if (merged.expand.any((map) => isEqualNested(right, map))) { - return true; - } - } - if (right case final LazyMergedMapView merged) { - if (merged.expand.any((map) => isEqualNested(left, map))) { - return true; - } - } - return false; + return left.expand.any((l) => right.expand.contains(l)); } while (true) {