From 4af6107492840c44631640f50a69186a9b3e8574 Mon Sep 17 00:00:00 2001 From: Kelvin Kavisi <68240897+kekavc24@users.noreply.github.com> Date: Sat, 23 Dec 2023 19:43:53 +0000 Subject: [PATCH] fix: switch to class `PairType` > replace `Key` & `Value` record with subclasses of `PairType` > add package Equatable to ease equality comparison. Poor hashing led to test failures despite objects being same. > refactor `NodeData` to use new pair types > clean up code references using old `PairType` record --- .../console_printer/console_printer.dart | 2 +- .../data/matched_node_data.dart | 43 ++++---- .../yaml_transformers/data/node_data.dart | 99 +++++++------------ .../pair_definition/custom_pair_type.dart | 69 +++++++++++++ .../data/pair_definition/pair_definition.dart | 59 ----------- .../data/pair_definition/pair_subtypes.dart | 15 +++ .../finders/value_finder.dart | 4 +- .../indexers/yaml_indexer.dart | 43 ++++---- .../managers/replacer_manager.dart | 2 +- .../yaml_transformers/yaml_transformer.dart | 4 +- lib/src/utils/extensions/map_extensions.dart | 6 +- pubspec.yaml | 1 + 12 files changed, 181 insertions(+), 166 deletions(-) create mode 100644 lib/src/core/yaml_transformers/data/pair_definition/custom_pair_type.dart delete mode 100644 lib/src/core/yaml_transformers/data/pair_definition/pair_definition.dart create mode 100644 lib/src/core/yaml_transformers/data/pair_definition/pair_subtypes.dart diff --git a/lib/src/core/yaml_transformers/console_printer/console_printer.dart b/lib/src/core/yaml_transformers/console_printer/console_printer.dart index 62d5d89..6807835 100644 --- a/lib/src/core/yaml_transformers/console_printer/console_printer.dart +++ b/lib/src/core/yaml_transformers/console_printer/console_printer.dart @@ -93,7 +93,7 @@ base class ConsolePrinter { }.toList(); // Wrap with ansicodes - final pathToTrack = wrapMatches(path: data.getPath(), matches: matches); + final pathToTrack = wrapMatches(path: data.toString(), matches: matches); final trackedPath = TrackedValue(key: pathToTrack, origin: Origin.custom); // Get all keys that where found at this path diff --git a/lib/src/core/yaml_transformers/data/matched_node_data.dart b/lib/src/core/yaml_transformers/data/matched_node_data.dart index 8554f6b..e07a97c 100644 --- a/lib/src/core/yaml_transformers/data/matched_node_data.dart +++ b/lib/src/core/yaml_transformers/data/matched_node_data.dart @@ -78,26 +78,35 @@ class MatchedNodeData extends NodeData { /// Get path of keys upto the last renameable key String getPathToLastKey() { - return getUptoLastRenameable().map((e) => e.value!).join('/'); + return getUptoLastRenameable().map((e) => e.toString()).join('/'); } - /// - - @override - bool operator ==(Object other) => - other is MatchedNodeData && - super == other && - collectionsUnorderedMatch(matchedKeys, other.matchedKeys) && - matchedValue == other.matchedValue && - collectionsUnorderedMatch(matchedPairs, other.matchedPairs); - + /// @override - int get hashCode => Object.hashAll([ - super.precedingKeys, - super.key, - super.value, + List get props => [ + ...super.props, matchedKeys, - matchedValue, matchedPairs, - ]); + matchedValue, + ]; + + // @override + // bool operator ==(Object other) => + // other is MatchedNodeData && + // super == other && + // collectionsUnorderedMatch(matchedKeys, other.matchedKeys) && + // matchedValue == other.matchedValue && + // collectionsUnorderedMatch(matchedPairs, other.matchedPairs); + + // @override + // int get hashCode { + // return Object.hashAll([ + // super.precedingKeys, + // super.key, + // super.value, + // matchedKeys, + // matchedValue, + // matchedPairs, + // ]); + // } } diff --git a/lib/src/core/yaml_transformers/data/node_data.dart b/lib/src/core/yaml_transformers/data/node_data.dart index 0f71577..9e6a0e0 100644 --- a/lib/src/core/yaml_transformers/data/node_data.dart +++ b/lib/src/core/yaml_transformers/data/node_data.dart @@ -5,39 +5,34 @@ part of '../yaml_transformer.dart'; /// Typically denotes a terminal node found while indexing a Yaml map or any /// map @immutable -class NodeData { +class NodeData extends Equatable { const NodeData(this.precedingKeys, this.key, this.value); /// Create with default constructor - factory NodeData.skeleton({ + const NodeData.skeleton({ required List precedingKeys, required Key key, required Value value, - }) { - return NodeData(precedingKeys, key, value); - } + }) : this(precedingKeys, key, value); /// Create using List path and key - factory NodeData.stringSkeleton({ + NodeData.stringSkeleton({ required List path, required String key, required String value, - }) { - return NodeData.skeleton( - precedingKeys: path.map((e) => createKey(value: e)).toList(), - key: createKey(value: key), - value: createValue(value: value), - ); - } + }) : this.skeleton( + precedingKeys: path.map((e) => createPair(value: e)).toList(), + key: createPair(value: key), + value: createPair(value: value), + ); /// Create from the root anchor key - factory NodeData.fromRoot({required String key, required dynamic value}) { - return NodeData( - const [], - createKey(value: key), - createValue(value: value), - ); - } + NodeData.fromRoot({required dynamic key, required dynamic value}) + : this.skeleton( + precedingKeys: const [], + key: createPair(value: key), + value: createPair(value: value), + ); /// Creates from entry in map. /// @@ -45,17 +40,15 @@ class NodeData { /// the value. /// * This is because we recursed the list to reach this key rather than the /// value. - factory NodeData.fromMapEntry({ + NodeData.fromMapEntry({ required NodeData parent, required MapEntry current, required List indices, - }) { - return NodeData( - [...parent.precedingKeys, parent.key], - createKey(value: current.key as String?, indices: indices), - createValue(value: current.value), - ); - } + }) : this.skeleton( + precedingKeys: [...parent.precedingKeys, parent.key], + key: createPair(value: current.key, indices: indices), + value: createPair(value: current.value), + ); /// Creates from terminal value at the end of a node /// @@ -65,17 +58,15 @@ class NodeData { /// /// * The parent's key will be this terminal value's key too as it's the /// nearest key linking this value to a map. - factory NodeData.atRootTerminal({ + NodeData.atRootTerminal({ required NodeData parent, - required String terminalValue, + required dynamic terminalValue, required List indices, - }) { - return NodeData( - [...parent.precedingKeys], - parent.key, - createValue(value: terminalValue, indices: indices), - ); - } + }) : this.skeleton( + precedingKeys: [...parent.precedingKeys], + key: parent.key, + value: createPair(value: terminalValue, indices: indices), + ); /// Any preceding keys for this node final List precedingKeys; @@ -86,21 +77,18 @@ class NodeData { /// Current data at this node final Value value; - /// Gets the actual value rather than the object storing it i.e. - /// - /// ```dart - /// Value.value - /// ``` - dynamic get data => value.value; + /// Gets the actual value at terminal end as a string. A null value will be + /// returned as 'null'. + String get data => value.toString(); /// Transform to key value pairs, based on this node data's path. /// /// Note: the terminal value must be a string - Map transformToPairs() { + Map transformToPairs() { // Get length of list final lastIndex = precedingKeys.length - 1; - final mapOfPairs = {}; + final mapOfPairs = {}; // Loop all and create keys in tandem for (final (index, candidate) in precedingKeys.indexed) { @@ -117,7 +105,7 @@ class NodeData { } // Add key and value as last pair - mapOfPairs.addAll({key.toString(): data as String?}); + mapOfPairs.addAll({key.toString(): data}); return mapOfPairs; } @@ -131,7 +119,7 @@ class NodeData { /// Obtains the keys as string. Ignores any indices present List getKeysAsString() { - return getKeys().map((e) => e.value!).toList(); + return getKeys().map((e) => e.toString()).toList(); } /// Get key path for this node @@ -139,26 +127,15 @@ class NodeData { return getKeysAsString().join('/'); } - /// Get full path to terminal value - String getPath() { - return "${getKeyPath()}${(data == null) ? '' : '/$data'}"; - } - - @override - bool operator ==(Object other) => - other is NodeData && - other.key == key && - other.value == value && - collectionsMatch(precedingKeys, other.precedingKeys); - @override - int get hashCode => Object.hashAll([key, value, precedingKeys]); + List get props => [precedingKeys, key, value]; + /// Obtains full path to terminal value of this node @override String toString() => '${getKeyPath()}/$data'; /// Checks whether this node is nested - bool isNested() => + bool isNestedInList() => key.isNested() || value.isNested() || precedingKeys.isNotEmpty && diff --git a/lib/src/core/yaml_transformers/data/pair_definition/custom_pair_type.dart b/lib/src/core/yaml_transformers/data/pair_definition/custom_pair_type.dart new file mode 100644 index 0000000..b98d6fb --- /dev/null +++ b/lib/src/core/yaml_transformers/data/pair_definition/custom_pair_type.dart @@ -0,0 +1,69 @@ +import 'package:magical_version_bump/src/utils/extensions/map_extensions.dart'; +import 'package:magical_version_bump/src/utils/typedefs/typedefs.dart'; +import 'package:meta/meta.dart'; + +part 'pair_subtypes.dart'; + +/// Custom key/value definition +@immutable +abstract base class PairType { + PairType({required dynamic value, List? indices}) + : _value = value, + _indices = indices ?? []; + + /// Indicates the actual value + final dynamic _value; + + /// Indicates the list of indices when nested in 1 or more lists + final List _indices; + + /// Obtains the actual value stored here at runtime. + dynamic get rawValue => _value; + + /// Obtains the list of indices when nested in 1 or more lists + List get indices => _indices; + + @override + bool operator ==(Object other) { + return other is PairType && + _hasSameValue(other.rawValue) && + collectionsMatch(indices, other.indices); + } + + bool _hasSameValue(dynamic other) { + if (rawValue.runtimeType != other.runtimeType) return false; + if (isTerminal(rawValue)) return rawValue == other; + return collectionsMatch(rawValue, other); + } + + @override + int get hashCode => Object.hashAll([rawValue, indices]); + + /// Returns the value stored at this node as a string + @override + String toString() { + return _value.toString(); + } + + /// Checks if nested in a list + bool isNested() => _indices.isNotEmpty; +} + +/// Creates desired pair type on the fly. +T createPair({ + required dynamic value, + List? indices, +}) { + if (T == Key) return Key(value: value, indices: indices) as T; + return Value(value: value, indices: indices) as T; +} + +/// Creates a list of desired pairs. +List createListOfPair({ + required List values, + required Map> indices, +}) { + return values + .map((value) => createPair(value: value, indices: indices[value])) + .toList(); +} diff --git a/lib/src/core/yaml_transformers/data/pair_definition/pair_definition.dart b/lib/src/core/yaml_transformers/data/pair_definition/pair_definition.dart deleted file mode 100644 index 2fd03b3..0000000 --- a/lib/src/core/yaml_transformers/data/pair_definition/pair_definition.dart +++ /dev/null @@ -1,59 +0,0 @@ -/// Custom key/value definition -/// -/// `value` - indicates the actual value -/// `indices` - indicates the list of indices when nested in 1 or more lists -typedef PairType = ({T? value, List indices}); - -/// A key in a custom key/value definition -/// -/// See [PairType] -typedef Key = PairType; - -/// A value in a custom key/value definition -/// -/// See [PairType] -typedef Value = PairType; - -T _pairType({ - required dynamic value, - required List indices, -}) { - return (value: value, indices: indices) as T; -} - -/// Create key -Key createKey({String? value, List? indices}) => _pairType( - value: value, - indices: indices ?? [], - ); - -/// Creates a list of keys -List createListOfKeys({ - required List keys, - required Map> linkedIndices, -}) { - return keys - .map((e) => createKey(value: e, indices: linkedIndices[e])) - .toList(); -} - -/// Create value -Value createValue({required dynamic value, List? indices}) => _pairType( - value: value, - indices: indices ?? [], - ); - -/// Creates a list of keys -List createListOfValues({ - required List keys, - required Map> linkedIndices, -}) { - return keys - .map((e) => createValue(value: e, indices: linkedIndices[e])) - .toList(); -} - -extension PairTypeExtension on PairType { - /// Checks if nested in a list - bool isNested() => this.indices.isNotEmpty; -} diff --git a/lib/src/core/yaml_transformers/data/pair_definition/pair_subtypes.dart b/lib/src/core/yaml_transformers/data/pair_definition/pair_subtypes.dart new file mode 100644 index 0000000..f2574cd --- /dev/null +++ b/lib/src/core/yaml_transformers/data/pair_definition/pair_subtypes.dart @@ -0,0 +1,15 @@ +part of 'custom_pair_type.dart'; + +/// A key in a custom key/value definition +final class Key extends PairType { + Key({required super.value, required super.indices}) + : assert( + isTerminal(value), + 'A type of ${value.runtimeType} cannot be a key', + ); +} + +/// A value in a custom key/value definition +final class Value extends PairType { + Value({required super.value, required super.indices}); +} diff --git a/lib/src/core/yaml_transformers/finders/value_finder.dart b/lib/src/core/yaml_transformers/finders/value_finder.dart index b2b13ab..78df0ce 100644 --- a/lib/src/core/yaml_transformers/finders/value_finder.dart +++ b/lib/src/core/yaml_transformers/finders/value_finder.dart @@ -59,7 +59,7 @@ class MagicalFinder extends Finder { final nodeKeys = [ ...nodeData.precedingKeys, nodeData.key, - ].map((e) => e.value!); + ].map((e) => e.toString()); // If not grouped, we check if any key we are searching for is present if (_keysToFind.orderType == OrderType.loose) { @@ -114,7 +114,7 @@ class MagicalFinder extends Finder { if (valuesToFind == null || _valuesToFind.isEmpty) return ''; return _valuesToFind.firstWhere( - (element) => element == nodeData.data.toString(), + (element) => element == nodeData.data, orElse: () => '', ); } diff --git a/lib/src/core/yaml_transformers/indexers/yaml_indexer.dart b/lib/src/core/yaml_transformers/indexers/yaml_indexer.dart index 40cac11..c888a79 100644 --- a/lib/src/core/yaml_transformers/indexers/yaml_indexer.dart +++ b/lib/src/core/yaml_transformers/indexers/yaml_indexer.dart @@ -27,16 +27,19 @@ part of '../yaml_transformer.dart'; /// marked as nested. See [ NodeData ] /// class MagicalIndexer { - MagicalIndexer(this._yamlMap); + MagicalIndexer._(this._map); - /// Instantiate with yaml - factory MagicalIndexer.forYaml(YamlMap yamlMap) => MagicalIndexer(yamlMap); + /// Instantiate with yaml map + MagicalIndexer.forYaml(YamlMap yamlMap) : this._(yamlMap); + + /// Instantiate with dart map + MagicalIndexer.forDartMap(Map map) : this._(map); /// Yaml map to search and index - final YamlMap _yamlMap; + final Map _map; Iterable indexYaml() sync* { - for (final entry in _yamlMap.entries) { + for (final entry in _map.entries) { final setUpData = NodeData.fromRoot( key: entry.key as String, value: entry.value, @@ -48,20 +51,20 @@ class MagicalIndexer { /// Entry point for indexing a node. Can be called recursively. Iterable _recursiveIndex(NodeData parent) sync* { - if (isTerminal(parent.data)) { + if (isTerminal(parent.value.rawValue)) { yield parent; + } else if (parent.value.rawValue is Map) { + yield* _indexNestedMap( + parent: parent, + child: parent.value.rawValue as Map, + indices: [], + ); } else { - yield* parent.data is Map - ? _indexNestedMap( - parent: parent, - child: parent.data as YamlMap, - indices: [], - ) - : _indexNestedList( - parent: parent, - children: parent.data as List, - indices: [], - ); + yield* _indexNestedList( + parent: parent, + children: parent.value.rawValue as List, + indices: [], + ); } } @@ -85,7 +88,7 @@ class MagicalIndexer { ); /// If terminal, we return it as is - if (isTerminal(nestedData.data)) { + if (isTerminal(nestedData.value.rawValue)) { yield nestedData; } @@ -105,7 +108,7 @@ class MagicalIndexer { else { yield* _indexNestedMap( parent: nestedData, - child: nestedData.data as Map, + child: nestedData.value.rawValue as Map, indices: [], ); } @@ -151,7 +154,7 @@ class MagicalIndexer { // If map, we call recursive map indexer yield* _indexNestedMap( parent: parent, - child: child as YamlMap, + child: child as Map, indices: updatedIndices, ); } diff --git a/lib/src/core/yaml_transformers/managers/replacer_manager.dart b/lib/src/core/yaml_transformers/managers/replacer_manager.dart index a7adb7b..522b313 100644 --- a/lib/src/core/yaml_transformers/managers/replacer_manager.dart +++ b/lib/src/core/yaml_transformers/managers/replacer_manager.dart @@ -95,7 +95,7 @@ class ReplacerManager extends TransformerManager { ? Origin.key : Origin.value, replacements: output.mapping, - oldPath: match.data.getPath(), + oldPath: match.data.toString(), ); } } diff --git a/lib/src/core/yaml_transformers/yaml_transformer.dart b/lib/src/core/yaml_transformers/yaml_transformer.dart index b56f151..948c37f 100644 --- a/lib/src/core/yaml_transformers/yaml_transformer.dart +++ b/lib/src/core/yaml_transformers/yaml_transformer.dart @@ -1,7 +1,7 @@ import 'package:collection/collection.dart'; -import 'package:magical_version_bump/src/core/yaml_transformers/data/pair_definition/pair_definition.dart'; +import 'package:equatable/equatable.dart'; +import 'package:magical_version_bump/src/core/yaml_transformers/data/pair_definition/custom_pair_type.dart'; import 'package:magical_version_bump/src/utils/extensions/extensions.dart'; -import 'package:magical_version_bump/src/utils/typedefs/typedefs.dart'; import 'package:meta/meta.dart'; import 'package:yaml/yaml.dart'; diff --git a/lib/src/utils/extensions/map_extensions.dart b/lib/src/utils/extensions/map_extensions.dart index 7c43d4f..5d1ef25 100644 --- a/lib/src/utils/extensions/map_extensions.dart +++ b/lib/src/utils/extensions/map_extensions.dart @@ -1,4 +1,4 @@ -import 'package:magical_version_bump/src/core/yaml_transformers/data/pair_definition/pair_definition.dart'; +import 'package:magical_version_bump/src/core/yaml_transformers/data/pair_definition/custom_pair_type.dart'; import 'package:magical_version_bump/src/utils/enums/enums.dart'; import 'package:magical_version_bump/src/utils/exceptions/command_exceptions.dart'; import 'package:magical_version_bump/src/utils/typedefs/typedefs.dart'; @@ -221,7 +221,7 @@ extension MapUtility on Map { /// /// This will always be called on a map, not a list thus we won't /// need to recurse list - final keyFromPath = path.first.value!; + final keyFromPath = path.first.toString(); // Attempt a swap. If replacement is null, no swap. final attemptedSwap = _attemptSwap( @@ -280,7 +280,7 @@ extension MapUtility on Map { required KeyAndReplacement keyAndReplacement, required Value? value, }) { - final candidate = target.value!; // Key that may change + final candidate = target.toString(); // Key that may change // Attempt a swap to have latest version of this map final attemptedSwap = _attemptSwap( diff --git a/pubspec.yaml b/pubspec.yaml index c7bb9e0..bc6cb7b 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -23,6 +23,7 @@ dependencies: args: ^2.3.1 cli_completion: ^0.4.0 collection: ^1.18.0 + equatable: ^2.0.5 mason_logger: ^0.2.4 meta: ^1.11.0 pub_semver: ^2.1.4