Allow built-in identifiers for non-type entities, with a warning.

R=brianwilkerson@google.com

Bug: https://github.com/dart-lang/sdk/issues/32893
Change-Id: I0d3d25b24d68e9cf30bfd69e6818aaa2a24b6f90
Reviewed-on: https://dart-review.googlesource.com/54100
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/services/refactoring/naming_conventions.dart b/pkg/analysis_server/lib/src/services/refactoring/naming_conventions.dart
index 199e929..0e5e556 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/naming_conventions.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/naming_conventions.dart
@@ -27,7 +27,7 @@
   if (name != null && name.isEmpty) {
     return new RefactoringStatus();
   }
-  return _validateLowerCamelCase(name, "Constructor");
+  return _validateLowerCamelCase(name, "Constructor", allowBuiltIn: true);
 }
 
 /**
@@ -37,7 +37,7 @@
  *   FATAL if the name is illegal.
  */
 RefactoringStatus validateFieldName(String name) {
-  return _validateLowerCamelCase(name, "Field");
+  return _validateLowerCamelCase(name, "Field", allowBuiltIn: true);
 }
 
 /**
@@ -47,7 +47,7 @@
  *   FATAL if the name is illegal.
  */
 RefactoringStatus validateFunctionName(String name) {
-  return _validateLowerCamelCase(name, "Function");
+  return _validateLowerCamelCase(name, "Function", allowBuiltIn: true);
 }
 
 /**
@@ -80,7 +80,7 @@
  *   FATAL if the name is illegal.
  */
 RefactoringStatus validateLabelName(String name) {
-  return _validateLowerCamelCase(name, "Label");
+  return _validateLowerCamelCase(name, "Label", allowBuiltIn: true);
 }
 
 /**
@@ -127,7 +127,7 @@
  *   FATAL if the name is illegal.
  */
 RefactoringStatus validateMethodName(String name) {
-  return _validateLowerCamelCase(name, "Method");
+  return _validateLowerCamelCase(name, "Method", allowBuiltIn: true);
 }
 
 /**
@@ -137,7 +137,7 @@
  *   FATAL if the name is illegal.
  */
 RefactoringStatus validateParameterName(String name) {
-  return _validateLowerCamelCase(name, "Parameter");
+  return _validateLowerCamelCase(name, "Parameter", allowBuiltIn: true);
 }
 
 /**
@@ -147,11 +147,12 @@
  *   FATAL if the name is illegal.
  */
 RefactoringStatus validateVariableName(String name) {
-  return _validateLowerCamelCase(name, "Variable");
+  return _validateLowerCamelCase(name, "Variable", allowBuiltIn: true);
 }
 
 RefactoringStatus _validateIdentifier(
-    String identifier, String desc, String beginDesc) {
+    String identifier, String desc, String beginDesc,
+    {bool allowBuiltIn: false}) {
   // has leading/trailing spaces
   String trimmed = identifier.trim();
   if (identifier != trimmed) {
@@ -167,9 +168,14 @@
   // keyword
   {
     Keyword keyword = Keyword.keywords[identifier];
-    if (keyword != null && !keyword.isPseudo) {
-      String message = "$desc must not be a keyword.";
-      return new RefactoringStatus.fatal(message);
+    if (keyword != null) {
+      if (keyword.isBuiltInOrPseudo && allowBuiltIn) {
+        String message = "Avoid using built-in identifiers as names.";
+        return new RefactoringStatus.warning(message);
+      } else {
+        String message = "$desc must not be a keyword.";
+        return new RefactoringStatus.fatal(message);
+      }
     }
   }
   // first character
@@ -198,7 +204,8 @@
 /**
  * Validates [identifier], should be lower camel case.
  */
-RefactoringStatus _validateLowerCamelCase(String identifier, String desc) {
+RefactoringStatus _validateLowerCamelCase(String identifier, String desc,
+    {bool allowBuiltIn: false}) {
   desc += ' name';
   // null
   if (identifier == null) {
@@ -206,8 +213,9 @@
     return new RefactoringStatus.fatal(message);
   }
   // is not identifier
-  RefactoringStatus status =
-      _validateIdentifier(identifier, desc, "a lowercase letter or underscore");
+  RefactoringStatus status = _validateIdentifier(
+      identifier, desc, "a lowercase letter or underscore",
+      allowBuiltIn: allowBuiltIn);
   if (!status.isOK) {
     return status;
   }
diff --git a/pkg/analysis_server/test/services/refactoring/naming_conventions_test.dart b/pkg/analysis_server/test/services/refactoring/naming_conventions_test.dart
index 5674877..fb78873 100644
--- a/pkg/analysis_server/test/services/refactoring/naming_conventions_test.dart
+++ b/pkg/analysis_server/test/services/refactoring/naming_conventions_test.dart
@@ -2,6 +2,7 @@
 // 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:analysis_server/src/services/correction/status.dart';
 import 'package:analysis_server/src/services/refactoring/naming_conventions.dart';
 import 'package:analysis_server/src/services/refactoring/refactoring.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart'
@@ -188,7 +189,7 @@
   }
 
   void test_validateFieldName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateFieldName("await"));
+    _assertWarningBuiltIn(validateFieldName("await"));
   }
 
   void test_validateFieldName_trailingBlanks() {
@@ -253,7 +254,7 @@
   }
 
   void test_validateFunctionName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateFunctionName("yield"));
+    _assertWarningBuiltIn(validateFunctionName("yield"));
   }
 
   void test_validateFunctionName_trailingBlanks() {
@@ -380,7 +381,9 @@
   }
 
   void test_validateImportPrefixName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateImportPrefixName("await"));
+    assertRefactoringStatus(
+        validateImportPrefixName("await"), RefactoringProblemSeverity.FATAL,
+        expectedMessage: "Import prefix name must not be a keyword.");
   }
 
   void test_validateImportPrefixName_trailingBlanks() {
@@ -450,7 +453,7 @@
   }
 
   void test_validateLabelName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateLabelName("await"));
+    _assertWarningBuiltIn(validateLabelName("await"));
   }
 
   void test_validateLabelName_trailingBlanks() {
@@ -588,7 +591,7 @@
   }
 
   void test_validateMethodName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateMethodName("yield"));
+    _assertWarningBuiltIn(validateMethodName("yield"));
   }
 
   void test_validateMethodName_trailingBlanks() {
@@ -598,7 +601,7 @@
   }
 
   void test_validateParameterName_builtIn() {
-    assertRefactoringStatusOK(validateParameterName("await"));
+    _assertWarningBuiltIn(validateParameterName("await"));
   }
 
   void test_validateParameterName_doesNotStartWithLowerCase() {
@@ -658,7 +661,7 @@
   }
 
   void test_validateParameterName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateParameterName("await"));
+    _assertWarningBuiltIn(validateParameterName("await"));
   }
 
   void test_validateParameterName_trailingBlanks() {
@@ -667,6 +670,10 @@
         expectedMessage: "Parameter name must not start or end with a blank.");
   }
 
+  void test_validateVariableName_builtIn() {
+    _assertWarningBuiltIn(validateVariableName('abstract'));
+  }
+
   void test_validateVariableName_doesNotStartWithLowerCase() {
     assertRefactoringStatus(
         validateVariableName("NewName"), RefactoringProblemSeverity.WARNING,
@@ -727,7 +734,7 @@
   }
 
   void test_validateVariableName_pseudoKeyword() {
-    assertRefactoringStatusOK(validateVariableName("await"));
+    _assertWarningBuiltIn(validateVariableName("await"));
   }
 
   void test_validateVariableName_trailingBlanks() {
@@ -735,4 +742,9 @@
         validateVariableName("newName "), RefactoringProblemSeverity.FATAL,
         expectedMessage: "Variable name must not start or end with a blank.");
   }
+
+  void _assertWarningBuiltIn(RefactoringStatus status) {
+    assertRefactoringStatus(status, RefactoringProblemSeverity.WARNING,
+        expectedMessage: 'Avoid using built-in identifiers as names.');
+  }
 }