Skip to content

Commit

Permalink
Add @mustBeConst annotation for parameters which should be
Browse files Browse the repository at this point in the history
constant.

As const-only parameters are not planned, see
dart-lang/language#1684, an annotation with analyzer support could help a bit at least, see
#29381.

The motivation is to enforce const arguments for methods annotated with
`@ResourceIdentifier`, to be able to record the argument values at build time, see https://dart-review.googlesource.com/c/sdk/+/329961.

Tested: pkg/analyzer/test/src/diagnostics/const_argument_test.dart
Change-Id: I2b8d2dce0c899fc0caa4985d892a5d031c747521
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357701
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Lasse Nielsen <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Moritz Sümmermann <[email protected]>
Reviewed-by: Marya Belanger <[email protected]>
  • Loading branch information
mosuem authored and Commit Queue committed Apr 4, 2024
1 parent 4e68819 commit b321254
Show file tree
Hide file tree
Showing 15 changed files with 904 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3721,6 +3721,8 @@ WarningCode.MUST_BE_IMMUTABLE:
status: noFix
WarningCode.MUST_CALL_SUPER:
status: hasFix
WarningCode.NON_CONST_ARGUMENT_FOR_CONST_PARAMETER:
status: needsEvaluation
WarningCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR:
status: needsFix
notes: |-
Expand Down
7 changes: 7 additions & 0 deletions pkg/analyzer/lib/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,9 @@ abstract class Element implements AnalysisTarget {
/// Whether the element has an annotation of the form `@literal`.
bool get hasLiteral;

/// Whether the element has an annotation of the form `@mustBeConst`.
bool get hasMustBeConst;

/// Whether the element has an annotation of the form `@mustBeOverridden`.
bool get hasMustBeOverridden;

Expand Down Expand Up @@ -872,6 +875,10 @@ abstract class ElementAnnotation implements ConstantEvaluationTarget {
/// Whether the annotation marks the associated constructor as being literal.
bool get isLiteral;

/// Whether the annotation marks the associated returned element as
/// requiring a constant argument.
bool get isMustBeConst;

/// Whether the annotation marks the associated member as requiring
/// subclasses to override this member.
bool get isMustBeOverridden;
Expand Down
22 changes: 22 additions & 0 deletions pkg/analyzer/lib/src/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,10 @@ class ElementAnnotationImpl implements ElementAnnotation {
/// literal.
static const String _literalVariableName = 'literal';

/// The name of the top-level variable used to mark a returned element as
/// requiring use.
static const String _mustBeConstVariableName = 'mustBeConst';

/// The name of the top-level variable used to mark a type as having
/// "optional" type arguments.
static const String _optionalTypeArgsVariableName = 'optionalTypeArgs';
Expand Down Expand Up @@ -1732,6 +1736,9 @@ class ElementAnnotationImpl implements ElementAnnotation {
@override
bool get isLiteral => _isPackageMetaGetter(_literalVariableName);

@override
bool get isMustBeConst => _isPackageMetaGetter(_mustBeConstVariableName);

@override
bool get isMustBeOverridden => _isPackageMetaGetter(_mustBeOverridden);

Expand Down Expand Up @@ -2113,6 +2120,18 @@ abstract class ElementImpl implements Element {
return false;
}

@override
bool get hasMustBeConst {
final metadata = this.metadata;
for (var i = 0; i < metadata.length; i++) {
var annotation = metadata[i];
if (annotation.isMustBeConst) {
return true;
}
}
return false;
}

@override
bool get hasMustBeOverridden {
final metadata = this.metadata;
Expand Down Expand Up @@ -5509,6 +5528,9 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
@override
bool get hasLiteral => false;

@override
bool get hasMustBeConst => false;

@override
bool get hasMustBeOverridden => false;

Expand Down
3 changes: 3 additions & 0 deletions pkg/analyzer/lib/src/dart/element/member.dart
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ abstract class Member implements Element {
@override
bool get hasLiteral => _declaration.hasLiteral;

@override
bool get hasMustBeConst => _declaration.hasMustBeConst;

@override
bool get hasMustBeOverridden => _declaration.hasMustBeOverridden;

Expand Down
8 changes: 8 additions & 0 deletions pkg/analyzer/lib/src/error/codes.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6934,6 +6934,14 @@ class WarningCode extends AnalyzerErrorCode {
hasPublishedDocs: true,
);

/// Parameters:
/// 0: the name of the argument
static const WarningCode NON_CONST_ARGUMENT_FOR_CONST_PARAMETER = WarningCode(
'NON_CONST_ARGUMENT_FOR_CONST_PARAMETER',
"Argument '{0}' must be a constant.",
correctionMessage: "Try replacing the argument with a constant.",
);

/// Generates a warning for non-const instance creation using a constructor
/// annotated with `@literal`.
///
Expand Down
118 changes: 118 additions & 0 deletions pkg/analyzer/lib/src/error/const_argument_verifier.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Copyright (c) 2024, 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/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/syntactic_entity.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/src/dart/ast/utilities.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/error/codes.dart';

/// Checks if the arguments for a parameter label as `@mustBeConst` are actually
/// constant.
class ConstArgumentsVerifier extends SimpleAstVisitor<void> {
final ErrorReporter _errorReporter;

final ConstantEvaluator _constantEvaluator;

ConstArgumentsVerifier(this._errorReporter)
: _constantEvaluator = ConstantEvaluator();

@override
void visitAssignmentExpression(AssignmentExpression node) {
if (node.operator.type == TokenType.EQ) {
_check(
arguments: [node.rightHandSide],
errorNode: node.operator,
);
} else if (node.rightHandSide.staticParameterElement?.hasMustBeConst ??
false) {
// If the operator is not `=`, then the argument cannot be const, as it
// depends on the value of the left hand side.
_errorReporter.atNode(
node.rightHandSide,
WarningCode.NON_CONST_ARGUMENT_FOR_CONST_PARAMETER,
arguments: [node.rightHandSide],
);
}
}

@override
void visitBinaryExpression(BinaryExpression node) {
_check(
arguments: [node.rightOperand],
errorNode: node.operator,
);
}

@override
void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
if (node.staticInvokeType is FunctionType) {
_check(
arguments: node.argumentList.arguments,
errorNode: node,
);
}
}

@override
void visitIndexExpression(IndexExpression node) {
_check(
arguments: [node.index],
errorNode: node.leftBracket,
);
}

@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
if (node.inConstantContext) return;
_check(
arguments: node.argumentList.arguments,
errorNode: node.constructorName,
);
}

@override
void visitMethodInvocation(MethodInvocation node) {
_check(
arguments: node.argumentList.arguments,
errorNode: node.methodName,
);
}

void _check({
required List<Expression> arguments,
required SyntacticEntity errorNode,
}) {
for (final argument in arguments) {
final parameter = argument.staticParameterElement;
if (parameter != null && parameter.hasMustBeConst) {
Expression resolvedArgument;
if (parameter.isNamed) {
resolvedArgument = (argument as NamedExpression).expression;
} else {
resolvedArgument = argument;
}
if (resolvedArgument is Identifier) {
final staticElement = resolvedArgument.staticElement;
if (staticElement != null &&
staticElement.nonSynthetic is ConstVariableElement) {
return;
}
}
if (resolvedArgument.accept(_constantEvaluator) ==
ConstantEvaluator.NOT_A_CONSTANT) {
_errorReporter.atNode(
argument,
WarningCode.NON_CONST_ARGUMENT_FOR_CONST_PARAMETER,
arguments: [parameter.name],
);
}
}
}
}
}
1 change: 1 addition & 0 deletions pkg/analyzer/lib/src/error/error_code_values.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,7 @@ const List<ErrorCode> errorCodeValues = [
WarningCode.MIXIN_ON_SEALED_CLASS,
WarningCode.MUST_BE_IMMUTABLE,
WarningCode.MUST_CALL_SUPER,
WarningCode.NON_CONST_ARGUMENT_FOR_CONST_PARAMETER,
WarningCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR,
WarningCode.NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW,
WarningCode.NON_NULLABLE_EQUALS_PARAMETER,
Expand Down
9 changes: 9 additions & 0 deletions pkg/analyzer/lib/src/generated/error_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import 'package:analyzer/src/dart/resolver/variance.dart';
import 'package:analyzer/src/diagnostic/diagnostic.dart';
import 'package:analyzer/src/diagnostic/diagnostic_factory.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/error/const_argument_verifier.dart';
import 'package:analyzer/src/error/constructor_fields_verifier.dart';
import 'package:analyzer/src/error/correct_override.dart';
import 'package:analyzer/src/error/duplicate_definition_verifier.dart';
Expand Down Expand Up @@ -241,6 +242,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>

final LibraryVerificationContext libraryVerificationContext;
final RequiredParametersVerifier _requiredParametersVerifier;
final ConstArgumentsVerifier _constArgumentsVerifier;
final DuplicateDefinitionVerifier _duplicateDefinitionVerifier;
final UseResultVerifier _checkUseVerifier;
late final TypeArgumentsVerifier _typeArgumentsVerifier;
Expand All @@ -260,6 +262,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
_UninstantiatedBoundChecker(errorReporter),
_checkUseVerifier = UseResultVerifier(errorReporter),
_requiredParametersVerifier = RequiredParametersVerifier(errorReporter),
_constArgumentsVerifier = ConstArgumentsVerifier(errorReporter),
_duplicateDefinitionVerifier = DuplicateDefinitionVerifier(
_inheritanceManager,
_currentLibrary,
Expand Down Expand Up @@ -353,6 +356,8 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
_checkForDeadNullCoalesce(node.readType as TypeImpl, node.rightHandSide);
}
_checkForAssignmentToFinal(lhs);

_constArgumentsVerifier.visitAssignmentExpression(node);
super.visitAssignmentExpression(node);
}

Expand Down Expand Up @@ -386,6 +391,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
}

checkForUseOfVoidResult(node.leftOperand);
_constArgumentsVerifier.visitBinaryExpression(node);

super.visitBinaryExpression(node);
}
Expand Down Expand Up @@ -918,6 +924,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
_typeArgumentsVerifier.checkFunctionExpressionInvocation(node);
}
_requiredParametersVerifier.visitFunctionExpressionInvocation(node);
_constArgumentsVerifier.visitFunctionExpressionInvocation(node);
_checkUseVerifier.checkFunctionExpressionInvocation(node);
super.visitFunctionExpressionInvocation(node);
}
Expand Down Expand Up @@ -1017,6 +1024,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
_checkForInvalidGenerativeConstructorReference(constructorName);
_checkForConstOrNewWithMixin(node, namedType, type);
_requiredParametersVerifier.visitInstanceCreationExpression(node);
_constArgumentsVerifier.visitInstanceCreationExpression(node);
if (node.isConst) {
_checkForConstWithNonConst(node);
_checkForConstWithUndefinedConstructor(
Expand Down Expand Up @@ -1110,6 +1118,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
}
_typeArgumentsVerifier.checkMethodInvocation(node);
_requiredParametersVerifier.visitMethodInvocation(node);
_constArgumentsVerifier.visitMethodInvocation(node);
_checkUseVerifier.checkMethodInvocation(node);
super.visitMethodInvocation(node);
}
Expand Down
11 changes: 10 additions & 1 deletion pkg/analyzer/lib/src/test_utilities/mock_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ const _IsTestGroup isTestGroup = _IsTestGroup();
const _Literal literal = _Literal();
const mustBeConst = _MustBeConst();
const _MustBeOverridden mustBeOverridden = _MustBeOverridden();
const _MustCallSuper mustCallSuper = _MustCallSuper();
Expand Down Expand Up @@ -191,6 +193,14 @@ class _Checked {
const _Checked();
}
@Target({
TargetKind.parameter,
TargetKind.typedefType,
})
class _MustBeConst {
const _MustBeConst();
}
@Target({
TargetKind.classType,
TargetKind.function,
Expand Down Expand Up @@ -296,7 +306,6 @@ class Target {
const Target(this.kinds);
}
class TargetKind {
const TargetKind._(this.displayString, this.name);
Expand Down
42 changes: 42 additions & 0 deletions pkg/analyzer/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24889,6 +24889,48 @@ WarningCode:
}
}
```
NON_CONST_ARGUMENT_FOR_CONST_PARAMETER:
problemMessage: "Argument '{0}' must be a constant."
correctionMessage: Try replacing the argument with a constant.
comment: |-
Parameters:
0: the name of the argument
hasPublishedDocs: false
documentation: |-
#### Description

The analyzer produces this diagnostic when a parameter is
annotated with the `@mustBeConst` annotation and the
corresponding argument is not a constant expression.

#### Example

The following code produces this diagnostic on the invocation of
the function `f` because the value of the argument passed to the
function `g` isn't a constant:

```dart
import 'package:meta/meta.dart' show mustBeConst;

int f(int value) => g([!value!]);

int g(@mustBeConst int value) => value + 1;
```

#### Common fixes

If a suitable constant is available to use, then replace the argument
with a constant:

```dart
import 'package:meta/meta.dart' show mustBeConst;

const v = 3;

int f() => g(v);

int g(@mustBeConst int value) => value + 1;
```
NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR_USING_NEW:
sharedName: NON_CONST_CALL_TO_LITERAL_CONSTRUCTOR
problemMessage: "This instance creation must be 'const', because the {0} constructor is marked as '@literal'."
Expand Down
Loading

0 comments on commit b321254

Please sign in to comment.