Add a Hint enforcing contract of @literal
Example code:
```dart
import 'package:meta/meta.dart';
class A {
@literal
const A();
@literal
const A.named();
}
var a = A();
var b = const A();
const c = A();
var d = new A();
var e = new A.named();
```
Produces:
Analyzing b.dart...
hint • This instance creation must be 'const', because the A constructor is marked as '@literal' at b.dart:11:9 • non_const_call_to_literal_constructor
hint • This instance creation must be 'const', because the A constructor is marked as '@literal' at b.dart:14:9 • non_const_call_to_literal_constructor
hint • This instance creation must be 'const', because the A.named constructor is marked as '@literal' at b.dart:16:9 • non_const_call_to_literal_constructor
3 hints found.
Bug: https://github.com/dart-lang/sdk/issues/34259
Change-Id: Iba10e5e0a0b2d0f7e99556cd6c201cea229ad675
Reviewed-on: https://dart-review.googlesource.com/c/88422
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
diff --git a/pkg/analyzer/lib/dart/element/element.dart b/pkg/analyzer/lib/dart/element/element.dart
index 547cb82..0260c07 100644
--- a/pkg/analyzer/lib/dart/element/element.dart
+++ b/pkg/analyzer/lib/dart/element/element.dart
@@ -534,6 +534,9 @@
/// Return `true` if this element has an annotation of the form `@JS(..)`.
bool get hasJS;
+ /// Return `true` if this element has an annotation of the form '@literal'.
+ bool get hasLiteral;
+
/// Return `true` if this element has an annotation of the form `@override`.
bool get hasOverride;
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index 7ffa3f6..7d59012 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -310,6 +310,8 @@
HintCode.MIXIN_ON_SEALED_CLASS,
HintCode.MUST_BE_IMMUTABLE,
HintCode.MUST_CALL_SUPER,
+ HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR,
+ HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW,
HintCode.NULL_AWARE_BEFORE_OPERATOR,
HintCode.NULL_AWARE_IN_CONDITION,
HintCode.NULL_AWARE_IN_LOGICAL_OPERATOR,
diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart
index f23c65a..77eabab 100644
--- a/pkg/analyzer/lib/src/dart/element/element.dart
+++ b/pkg/analyzer/lib/src/dart/element/element.dart
@@ -2836,6 +2836,18 @@
}
@override
+ bool get hasLiteral {
+ var metadata = this.metadata;
+ for (var i = 0; i < metadata.length; i++) {
+ var annotation = metadata[i];
+ if (annotation.isLiteral) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @override
bool get hasOverride {
var metadata = this.metadata;
for (var i = 0; i < metadata.length; i++) {
@@ -6400,6 +6412,9 @@
bool get hasJS => false;
@override
+ bool get hasLiteral => false;
+
+ @override
bool get hasOverride => false;
@override
diff --git a/pkg/analyzer/lib/src/dart/element/handle.dart b/pkg/analyzer/lib/src/dart/element/handle.dart
index 97f978f..67f47f7 100644
--- a/pkg/analyzer/lib/src/dart/element/handle.dart
+++ b/pkg/analyzer/lib/src/dart/element/handle.dart
@@ -46,6 +46,9 @@
bool get hasJS => actualElement.hasJS;
@override
+ bool get hasLiteral => actualElement.hasLiteral;
+
+ @override
bool get hasNonFinalField => actualElement.hasNonFinalField;
@override
@@ -389,6 +392,9 @@
bool get hasJS => actualElement.hasJS;
@override
+ bool get hasLiteral => actualElement.hasLiteral;
+
+ @override
bool get hasOverride => actualElement.hasOverride;
@override
diff --git a/pkg/analyzer/lib/src/dart/element/member.dart b/pkg/analyzer/lib/src/dart/element/member.dart
index bc18d7b7..484458a 100644
--- a/pkg/analyzer/lib/src/dart/element/member.dart
+++ b/pkg/analyzer/lib/src/dart/element/member.dart
@@ -422,6 +422,9 @@
bool get hasJS => _baseElement.hasJS;
@override
+ bool get hasLiteral => _baseElement.hasLiteral;
+
+ @override
bool get hasOverride => _baseElement.hasOverride;
@override
diff --git a/pkg/analyzer/lib/src/dart/element/wrapped.dart b/pkg/analyzer/lib/src/dart/element/wrapped.dart
index 3dfc8d6..3c6f7f1 100644
--- a/pkg/analyzer/lib/src/dart/element/wrapped.dart
+++ b/pkg/analyzer/lib/src/dart/element/wrapped.dart
@@ -65,6 +65,9 @@
bool get hasJS => wrappedUnit.hasJS;
@override
+ bool get hasLiteral => wrappedUnit.hasLiteral;
+
+ @override
bool get hasLoadLibraryFunction => wrappedUnit.hasLoadLibraryFunction;
@override
@@ -255,6 +258,9 @@
bool get hasJS => wrappedImport.hasJS;
@override
+ bool get hasLiteral => wrappedImport.hasLiteral;
+
+ @override
bool get hasOverride => wrappedImport.hasOverride;
@override
@@ -454,6 +460,9 @@
bool get hasJS => wrappedLib.hasJS;
@override
+ bool get hasLiteral => wrappedLib.hasLiteral;
+
+ @override
bool get hasLoadLibraryFunction => wrappedLib.hasLoadLibraryFunction;
@override
diff --git a/pkg/analyzer/lib/src/dart/error/hint_codes.dart b/pkg/analyzer/lib/src/dart/error/hint_codes.dart
index 07700ec..76fe3c2 100644
--- a/pkg/analyzer/lib/src/dart/error/hint_codes.dart
+++ b/pkg/analyzer/lib/src/dart/error/hint_codes.dart
@@ -399,6 +399,27 @@
"but does not invoke the overridden method.");
/**
+ * Generate a hint for non-const instance creation using a constructor
+ * annotated with `@literal`.
+ */
+ static const HintCode NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR = const HintCode(
+ 'NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR',
+ "This instance creation must be 'const', because the {0} constructor is "
+ "marked as '@literal'.",
+ correction: "Try adding a 'const' keyword.");
+
+ /**
+ * Generate a hint for non-const instance creation (with the `new` keyword)
+ * using a constructor annotated with `@literal`.
+ */
+ static const HintCode NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW =
+ const HintCode(
+ 'NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW',
+ "This instance creation must be 'const', because the {0} constructor is "
+ "marked as '@literal'.",
+ correction: "Try replacing the 'new' keyword with 'const'.");
+
+ /**
* When the left operand of a binary expression uses '?.' operator, it can be
* `null`.
*/
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 3696609..f239508 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -34,6 +34,7 @@
import 'package:analyzer/src/generated/static_type_analyzer.dart';
import 'package:analyzer/src/generated/testing/element_factory.dart';
import 'package:analyzer/src/generated/type_system.dart';
+import 'package:analyzer/src/lint/linter.dart';
import 'package:analyzer/src/workspace/workspace.dart';
import 'package:path/path.dart' as path;
@@ -286,6 +287,9 @@
/// The [WorkspacePackage] in which [_currentLibrary] is declared.
WorkspacePackage _workspacePackage;
+ /// The [LinterContext] used for possible const calculations.
+ LinterContext _linterContext;
+
/// Create a new instance of the [BestPracticesVerifier].
///
/// @param errorReporter the error reporter
@@ -295,6 +299,7 @@
this._currentLibrary, {
TypeSystem typeSystem,
ResourceProvider resourceProvider,
+ DeclaredVariables declaredVariables,
}) : _nullType = typeProvider.nullType,
_futureNullType = typeProvider.futureNullType,
_typeSystem = typeSystem ?? new Dart2TypeSystem(typeProvider),
@@ -305,6 +310,8 @@
Workspace workspace = ContextBuilder.createWorkspace(
resourceProvider, libraryPath, null /* ContextBuilder */);
_workspacePackage = workspace.findPackageFor(libraryPath);
+ _linterContext = LinterContextImpl(null /* allUnits */,
+ null /* currentUnit */, declaredVariables, typeProvider, _typeSystem);
}
@override
@@ -480,6 +487,7 @@
@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
_checkForDeprecatedMemberUse(node.staticElement, node);
+ _checkForLiteralConstructorUse(node);
super.visitInstanceCreationExpression(node);
}
@@ -1003,6 +1011,32 @@
return false;
}
+ /// Check that the instance creation node is const if the constructor is
+ /// marked with [literal].
+ _checkForLiteralConstructorUse(InstanceCreationExpression node) {
+ ConstructorName constructorName = node.constructorName;
+ ConstructorElement constructor = constructorName.staticElement;
+ if (constructor == null) {
+ return;
+ }
+ if (!node.isConst &&
+ constructor.hasLiteral &&
+ _linterContext.canBeConst(node)) {
+ // Echoing jwren's TODO from _checkForDeprecatedMemberUse:
+ // TODO(jwren) We should modify ConstructorElement.getDisplayName(), or
+ // have the logic centralized elsewhere, instead of doing this logic
+ // here.
+ String fullConstructorName = constructorName.type.name.name;
+ if (constructorName.name != null) {
+ fullConstructorName = '$fullConstructorName.${constructorName.name}';
+ }
+ HintCode hint = node.keyword?.keyword == Keyword.NEW
+ ? HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW
+ : HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR;
+ _errorReporter.reportErrorForNode(hint, node, [fullConstructorName]);
+ }
+ }
+
/// Generate a hint for functions or methods that have a return type, but do
/// not have a return statement on all branches. At the end of blocks with no
/// return, Dart implicitly returns `null`, avoiding these implicit returns is
diff --git a/pkg/analyzer/test/generated/hint_code_test.dart b/pkg/analyzer/test/generated/hint_code_test.dart
index 0a7da26..d537953 100644
--- a/pkg/analyzer/test/generated/hint_code_test.dart
+++ b/pkg/analyzer/test/generated/hint_code_test.dart
@@ -1323,6 +1323,55 @@
verify([source]);
}
+ test_nonConstCallToLiteralConstructor_nonConstContext() async {
+ Source source = addSource(r'''
+import 'package:meta/meta.dart';
+class A {
+ @literal
+ const A();
+}
+void main() {
+ var a = A();
+}
+''');
+ await computeAnalysisResult(source);
+ assertErrors(source, [HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR]);
+ verify([source]);
+ }
+
+ test_nonConstCallToLiteralConstructor_usingNew() async {
+ Source source = addSource(r'''
+import 'package:meta/meta.dart';
+class A {
+ @literal
+ const A();
+}
+void main() {
+ var a = new A();
+}
+''');
+ await computeAnalysisResult(source);
+ assertErrors(
+ source, [HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW]);
+ verify([source]);
+ }
+
+ test_nonConstCallToLiteralConstructor_namedConstructor() async {
+ Source source = addSource(r'''
+import 'package:meta/meta.dart';
+class A {
+ @literal
+ const A.named();
+}
+void main() {
+ var a = A.named();
+}
+''');
+ await computeAnalysisResult(source);
+ assertErrors(source, [HintCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR]);
+ verify([source]);
+ }
+
test_invalidLiteralAnnotation_nonConstructor() async {
Source source = addSource(r'''
import 'package:meta/meta.dart';
diff --git a/pkg/analyzer/test/generated/non_hint_code_test.dart b/pkg/analyzer/test/generated/non_hint_code_test.dart
index f1af9c5..26c4abc 100644
--- a/pkg/analyzer/test/generated/non_hint_code_test.dart
+++ b/pkg/analyzer/test/generated/non_hint_code_test.dart
@@ -483,6 +483,57 @@
verify([source]);
}
+ test_nonConstCallToLiteralConstructor_constCreation() async {
+ Source source = addSource(r'''
+import 'package:meta/meta.dart';
+class A {
+ @literal
+ const A();
+}
+
+void main() {
+ const a = const A();
+}
+''');
+ await computeAnalysisResult(source);
+ assertNoErrors(source);
+ verify([source]);
+ }
+
+ test_nonConstCallToLiteralConstructor_constContextCreation() async {
+ Source source = addSource(r'''
+import 'package:meta/meta.dart';
+class A {
+ @literal
+ const A();
+}
+
+void main() {
+ const a = A();
+}
+''');
+ await computeAnalysisResult(source);
+ assertNoErrors(source);
+ verify([source]);
+ }
+
+ test_nonConstCallToLiteralConstructor_unconstableCreation() async {
+ Source source = addSource(r'''
+import 'package:meta/meta.dart';
+class A {
+ @literal
+ const A(List list);
+}
+
+void main() {
+ var a = A(new List());
+}
+''');
+ await computeAnalysisResult(source);
+ assertNoErrors(source);
+ verify([source]);
+ }
+
test_overrideOnNonOverridingField_inInterface() async {
Source source = addSource(r'''
class A {
