Add hints to find equal elements in literal sets and maps

Change-Id: Ib930b49b338d7cb088b17909991934b16c4f1538
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138602
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: bwilkerson

pkg/analyzer/lib/error/error.dart

@@ -367,6 +367,8 @@ const List<ErrorCode> errorCodeValues = [
   HintCode.DUPLICATE_IMPORT,
   HintCode.DUPLICATE_HIDDEN_NAME,
   HintCode.DUPLICATE_SHOWN_NAME,
+  HintCode.EQUAL_ELEMENTS_IN_SET,
+  HintCode.EQUAL_KEYS_IN_MAP,
   HintCode.FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE,
   HintCode.FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE,
   HintCode.IMPORT_DEFERRED_LIBRARY_WITH_LOAD_FUNCTION,

pkg/analyzer/lib/src/dart/error/hint_codes.dart

@@ -370,6 +370,21 @@ class HintCode extends AnalyzerErrorCode {
           correction: "Try removing the repeated name from the list of shown "
               "members.");
 
+  /**
+   * No parameters.
+   */
+  static const HintCode EQUAL_ELEMENTS_IN_SET = HintCode(
+      'EQUAL_ELEMENTS_IN_SET',
+      "Two elements in a set literal shouldn't be equal.",
+      correction: "Change or remove the duplicate element.");
+
+  /**
+   * No parameters.
+   */
+  static const HintCode EQUAL_KEYS_IN_MAP = HintCode(
+      'EQUAL_KEYS_IN_MAP', "Two keys in a map literal shouldn't be equal.",
+      correction: "Change or remove the duplicate key.");
+
   /**
    * It is a bad practice for a source file in a package "lib" directory
    * hierarchy to traverse outside that directory hierarchy. For example, a

pkg/analyzer/lib/src/error/best_practices_verifier.dart

@@ -565,6 +565,12 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     super.visitRedirectingConstructorInvocation(node);
   }
 
+  @override
+  void visitSetOrMapLiteral(SetOrMapLiteral node) {
+    _checkForDuplications(node);
+    super.visitSetOrMapLiteral(node);
+  }
+
   @override
   void visitSimpleIdentifier(SimpleIdentifier node) {
     _checkForDeprecatedMemberUseAtIdentifier(node);
@@ -824,6 +830,31 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     return false;
   }
 
+  /// Generate hints related to duplicate elements (keys) in sets (maps).
+  void _checkForDuplications(SetOrMapLiteral node) {
+    // This only checks for top-level elements. If, for, and spread elements
+    // that contribute duplicate values are not detected.
+    if (node.isConst) {
+      // This case is covered by the ErrorVerifier.
+      return;
+    }
+    final expressions = node.isSet
+        ? node.elements.whereType<Expression>()
+        : node.elements.whereType<MapLiteralEntry>().map((entry) => entry.key);
+    final alreadySeen = <DartObject>{};
+    for (final expression in expressions) {
+      final constEvaluation = _linterContext.evaluateConstant(expression);
+      if (constEvaluation.errors.isEmpty) {
+        if (!alreadySeen.add(constEvaluation.value)) {
+          var errorCode = node.isSet
+              ? HintCode.EQUAL_ELEMENTS_IN_SET
+              : HintCode.EQUAL_KEYS_IN_MAP;
+          _errorReporter.reportErrorForNode(errorCode, expression);
+        }
+      }
+    }
+  }
+
   /// Checks whether [node] violates the rules of [immutable].
   ///
   /// If [node] is marked with [immutable] or inherits from a class or mixin

pkg/analyzer/lib/src/error/codes.dart

@@ -1654,7 +1654,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
   // returned by an iterator.
   static const CompileTimeErrorCode EQUAL_ELEMENTS_IN_CONST_SET =
       CompileTimeErrorCode('EQUAL_ELEMENTS_IN_CONST_SET',
-          "Two values in a constant set can't be equal.",
+          "Two elements in a constant set literal can't be equal.",
+          correction: "Change or remove the duplicate element.",
           hasPublishedDocs: true);
 
   /**

pkg/analyzer/test/src/diagnostics/equal_elements_in_const_set_test.dart

@@ -177,9 +177,12 @@ var c = const {1, ...{1}};
   }
 
   test_nonConst_entry() async {
-    await assertNoErrorsInCode('''
+    // No error, but there is a hint.
+    await assertErrorsInCode('''
 var c = {1, 2, 1};
-''');
+''', [
+      error(HintCode.EQUAL_ELEMENTS_IN_SET, 15, 1),
+    ]);
   }
 }
 

pkg/analyzer/test/src/diagnostics/equal_elements_in_set_test.dart

@@ -0,0 +1,44 @@
+// Copyright (c) 2020, 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:analyzer/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(EqualElementsInSetTest);
+  });
+}
+
+@reflectiveTest
+class EqualElementsInSetTest extends DriverResolutionTest {
+  test_constant_constant() async {
+    await assertErrorsInCode('''
+const a = 1;
+const b = 1;
+var s = {a, b};
+''', [
+      error(HintCode.EQUAL_ELEMENTS_IN_SET, 38, 1),
+    ]);
+  }
+
+  test_literal_constant() async {
+    await assertErrorsInCode('''
+const one = 1;
+var s = {1, one};
+''', [
+      error(HintCode.EQUAL_ELEMENTS_IN_SET, 27, 3),
+    ]);
+  }
+
+  test_literal_literal() async {
+    await assertErrorsInCode('''
+var s = {1, 1};
+''', [
+      error(HintCode.EQUAL_ELEMENTS_IN_SET, 12, 1),
+    ]);
+  }
+}

pkg/analyzer/test/src/diagnostics/equal_keys_in_const_map_test.dart

@@ -158,9 +158,12 @@ var c = const {1: null, ...{1: null}};
   }
 
   test_nonConst_entry() async {
-    await assertNoErrorsInCode('''
+    // No error, but there is a hint.
+    await assertErrorsInCode('''
 var c = {1: null, 2: null, 1: null};
-''');
+''', [
+      error(HintCode.EQUAL_KEYS_IN_MAP, 27, 1),
+    ]);
   }
 }
 

pkg/analyzer/test/src/diagnostics/equal_keys_in_map_test.dart

@@ -0,0 +1,44 @@
+// Copyright (c) 2020, 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:analyzer/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(EqualKeysInMapTest);
+  });
+}
+
+@reflectiveTest
+class EqualKeysInMapTest extends DriverResolutionTest {
+  test_constant_constant() async {
+    await assertErrorsInCode('''
+const a = 1;
+const b = 1;
+var s = {a: 2, b: 3};
+''', [
+      error(HintCode.EQUAL_KEYS_IN_MAP, 41, 1),
+    ]);
+  }
+
+  test_literal_constant() async {
+    await assertErrorsInCode('''
+const one = 1;
+var s = {1: 2, one: 3};
+''', [
+      error(HintCode.EQUAL_KEYS_IN_MAP, 30, 3),
+    ]);
+  }
+
+  test_literal_literal() async {
+    await assertErrorsInCode('''
+var s = {1: 2, 1: 3};
+''', [
+      error(HintCode.EQUAL_KEYS_IN_MAP, 15, 1),
+    ]);
+  }
+}

pkg/analyzer/test/src/diagnostics/recursive_compile_time_constant_test.dart

@@ -47,10 +47,12 @@ const int y = x;
 ''',
     );
     // No errors, because the cycle is not in this source.
-    await assertNoErrorsInCode(r'''
+    await assertErrorsInCode(r'''
 import 'constants.dart';
 final z = {x: 0, y: 1};
-''');
+''', [
+      error(HintCode.EQUAL_KEYS_IN_MAP, 42, 1),
+    ]);
   }
 
   test_initializer_after_toplevel_var() async {

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -82,7 +82,9 @@ import 'duplicate_named_argument_test.dart' as duplicate_named_argument;
 import 'duplicate_part_test.dart' as duplicate_part;
 import 'duplicate_shown_name_test.dart' as duplicate_shown_name;
 import 'equal_elements_in_const_set_test.dart' as equal_elements_in_const_set;
+import 'equal_elements_in_set_test.dart' as equal_elements_in_set;
 import 'equal_keys_in_const_map_test.dart' as equal_keys_in_const_map;
+import 'equal_keys_in_map_test.dart' as equal_keys_in_map;
 import 'expected_one_list_type_arguments_test.dart'
     as expected_one_list_type_arguments;
 import 'expected_one_set_type_arguments_test.dart'
@@ -557,7 +559,9 @@ main() {
     duplicate_part.main();
     duplicate_shown_name.main();
     equal_elements_in_const_set.main();
+    equal_elements_in_set.main();
     equal_keys_in_const_map.main();
+    equal_keys_in_map.main();
     expected_one_list_type_arguments.main();
     expected_one_set_type_arguments.main();
     expected_two_map_type_arguments.main();

pkg/analyzer/tool/diagnostics/diagnostics.md

@@ -1399,7 +1399,7 @@ class C {
 
 ### equal_elements_in_const_set
 
-_Two values in a constant set can't be equal._
+_Two elements in a constant set literal can't be equal._
 
 #### Description