fix: Json::op_equal returns false when the keys of 2 maps have different order

[Lampese]

• Json::op_equal returns false when the keys of 2 maps have different order #1188

[peter-jerry-ye-code-review]

From the provided git diff output, here are three observations that suggest potential issues or improvements:

1. Missing Hash Trait in op_equal Implementation:

   • In the file builtin/builtin.mbti, the op_equal method for the Map type has been updated to require the Hash trait for the key type K. This is a correct change since the op_get method also requires Hash and Eq traits for K. However, this change should be reflected consistently across all relevant implementations and method signatures.
   • Suggested Action: Ensure that all methods interacting with Map that require key equality also explicitly mention the Hash trait requirement. This includes checking other methods like remove, op_set, etc., to ensure they are consistent.

2. Inefficient Equality Check in linked_hash_map.mbt:

   • The new implementation of op_equal in linked_hash_map.mbt iterates over one map and checks the existence and value equality in the other map. While this works, it might be less efficient than comparing both maps simultaneously, especially if the maps are large.
   • Suggested Action: Consider optimizing the equality check by iterating over both maps simultaneously, similar to the previous implementation that used a loop with continue statements. This could improve performance, especially for larger maps.

3. Test Naming Convention:

   • The test named "op_equal" has been renamed to "op_equal_1" in linked_hash_map_wbtest.mbt. Additionally, a new test "op_equal_2" has been added. This indicates a potential issue with test naming convention or management.
   • Suggested Action: Ensure that test names are descriptive and unique without needing numerical suffixes. If multiple tests for the same functionality are necessary, consider making their descriptions more specific to differentiate their purposes clearly. This will help in maintaining and understanding the test suite over time.

These observations are based on the provided context and the changes reflected in the git diff output. It's important to review these changes in the broader context of the codebase to ensure consistency and efficiency across all functionalities affected by these modifications.

[coveralls]

Pull Request Test Coverage Report for Build 3785

Details

• 1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 83.783%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
builtin/linked_hash_map.mbt	1	2	50.0%

Totals
Change from base Build 3783:	0.01%
Covered Lines:	4319
Relevant Lines:	5155

---

💛 - Coveralls

[bobzhang]

In Dart, two maps are considered equal if they contain the same keys and values, and the values for each key are equal. However, order matters for Map equality in Dart; this is because Dart Maps, by default, preserve insertion order.

For example:

void main() {
  var map1 = {'a': 1, 'b': 2};
  var map2 = {'b': 2, 'a': 1};

  print(map1 == map2); // false, because the order is different
}

We need some more research on existing behaivors and doucment it properly