Skip to content

Commit 3699147

Browse files
srawlinsCommit Queue
srawlins
authored and
Commit Queue
committed
Analyzer warnings: add support for @Deprecation.optional
This reports when an argument is omitted for a parameter which will soon be required. Work towards #60504 * The annotation causes certain usage to generate a warning (+ tests). * Possible fixes for this new warning are offered (+ tests). * Placing the annotation on an invalid element generates a different warning (+ tests). Change-Id: I45d54717dbeb05c45096270147df68e347b0daa6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/449367 Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 35880b6 commit 3699147

File tree

11 files changed

+574
-31
lines changed

11 files changed

+574
-31
lines changed

‎pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3497,6 +3497,8 @@ WarningCode.DEPRECATED_MIXIN_FUNCTION:
34973497
The fix is to remove `Function` from where it's referenced.
34983498
WarningCode.DEPRECATED_NEW_IN_COMMENT_REFERENCE:
34993499
status: hasFix
3500+
WarningCode.DEPRECATED_OPTIONAL:
3501+
status: needsEvaluation
35003502
WarningCode.DEPRECATED_SUBCLASS:
35013503
status: hasFix
35023504
WarningCode.DOC_DIRECTIVE_ARGUMENT_WRONG_FORMAT:
@@ -3576,6 +3578,7 @@ WarningCode.INVALID_ANNOTATION_TARGET:
35763578
status: hasFix
35773579
WarningCode.INVALID_AWAIT_NOT_REQUIRED_ANNOTATION:
35783580
status: needsFix
3581+
notes: The fix is to remove the annotation.
35793582
WarningCode.INVALID_DEPRECATED_EXTEND_ANNOTATION:
35803583
status: needsFix
35813584
notes: The fix is to remove the annotation.
@@ -3588,6 +3591,9 @@ WarningCode.INVALID_DEPRECATED_INSTANTIATE_ANNOTATION:
35883591
WarningCode.INVALID_DEPRECATED_MIXIN_ANNOTATION:
35893592
status: needsFix
35903593
notes: The fix is to remove the annotation.
3594+
WarningCode.INVALID_DEPRECATED_OPTIONAL_ANNOTATION:
3595+
status: needsFix
3596+
notes: The fix is to remove the annotation.
35913597
WarningCode.INVALID_DEPRECATED_SUBCLASS_ANNOTATION:
35923598
status: needsFix
35933599
notes: The fix is to remove the annotation.

‎pkg/analyzer/lib/src/diagnostic/diagnostic_code_values.g.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ const List<DiagnosticCode> diagnosticCodeValues = [
991991
WarningCode.deprecatedMixin,
992992
WarningCode.deprecatedMixinFunction,
993993
WarningCode.deprecatedNewInCommentReference,
994+
WarningCode.deprecatedOptional,
994995
WarningCode.deprecatedSubclass,
995996
WarningCode.docDirectiveArgumentWrongFormat,
996997
WarningCode.docDirectiveHasExtraArguments,
@@ -1026,6 +1027,7 @@ const List<DiagnosticCode> diagnosticCodeValues = [
10261027
WarningCode.invalidDeprecatedImplementAnnotation,
10271028
WarningCode.invalidDeprecatedInstantiateAnnotation,
10281029
WarningCode.invalidDeprecatedMixinAnnotation,
1030+
WarningCode.invalidDeprecatedOptionalAnnotation,
10291031
WarningCode.invalidDeprecatedSubclassAnnotation,
10301032
WarningCode.invalidExportOfInternalElement,
10311033
WarningCode.invalidExportOfInternalElementIndirectly,

‎pkg/analyzer/lib/src/error/annotation_verifier.dart‎

Lines changed: 56 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -121,25 +121,27 @@ class AnnotationVerifier {
121121
assert(element.isDeprecated);
122122
switch (element.deprecationKind) {
123123
case 'extend':
124-
_checkDeprecatedExtend(node, node.parent);
124+
_checkDeprecatedExtend(node);
125125
case 'implement':
126-
_checkDeprecatedImplement(node, node.parent);
126+
_checkDeprecatedImplement(node);
127127
case 'instantiate':
128-
_checkDeprecatedInstantiate(node, node.parent);
128+
_checkDeprecatedInstantiate(node);
129129
case 'mixin':
130-
_checkDeprecatedMixin(node, node.parent);
130+
_checkDeprecatedMixin(node);
131+
case 'optional':
132+
_checkDeprecatedOptional(node);
131133
case 'subclass':
132-
_checkDeprecatedSubclass(node, node.parent);
134+
_checkDeprecatedSubclass(node);
133135
}
134136
}
135137

136-
void _checkDeprecatedExtend(Annotation node, AstNode parent) {
138+
void _checkDeprecatedExtend(Annotation node) {
137139
Element? declaredElement;
138-
if (parent
140+
if (node.parent
139141
case ClassDeclaration(:var declaredFragment) ||
140142
ClassTypeAlias(:var declaredFragment)) {
141143
declaredElement = declaredFragment!.element;
142-
} else if (parent is GenericTypeAlias) {
144+
} else if (node.parent case GenericTypeAlias parent) {
143145
declaredElement = parent.type.type?.element;
144146
}
145147

@@ -157,15 +159,15 @@ class AnnotationVerifier {
157159
);
158160
}
159161

160-
void _checkDeprecatedImplement(Annotation node, AstNode parent) {
162+
void _checkDeprecatedImplement(Annotation node) {
161163
Element? declaredElement;
162-
if (parent
164+
if (node.parent
163165
case ClassDeclaration(:var declaredFragment) ||
164166
ClassTypeAlias(:var declaredFragment)) {
165167
declaredElement = declaredFragment!.element;
166-
} else if (parent is MixinDeclaration) {
168+
} else if (node.parent case MixinDeclaration parent) {
167169
declaredElement = parent.declaredFragment!.element;
168-
} else if (parent is GenericTypeAlias) {
170+
} else if (node.parent case GenericTypeAlias parent) {
169171
declaredElement = parent.type.type?.element;
170172
}
171173

@@ -185,13 +187,13 @@ class AnnotationVerifier {
185187
);
186188
}
187189

188-
void _checkDeprecatedInstantiate(Annotation node, AstNode parent) {
190+
void _checkDeprecatedInstantiate(Annotation node) {
189191
Element? declaredElement;
190-
if (parent
192+
if (node.parent
191193
case ClassDeclaration(:var declaredFragment) ||
192194
ClassTypeAlias(:var declaredFragment)) {
193195
declaredElement = declaredFragment!.element;
194-
} else if (parent is GenericTypeAlias) {
196+
} else if (node.parent case GenericTypeAlias parent) {
195197
declaredElement = parent.type.type?.element;
196198
}
197199

@@ -208,7 +210,8 @@ class AnnotationVerifier {
208210
);
209211
}
210212

211-
void _checkDeprecatedMixin(Annotation node, AstNode parent) {
213+
void _checkDeprecatedMixin(Annotation node) {
214+
var parent = node.parent;
212215
if (parent is ClassDeclaration &&
213216
parent.declaredFragment!.element.isPublic &&
214217
parent.mixinKeyword != null) {
@@ -221,15 +224,48 @@ class AnnotationVerifier {
221224
);
222225
}
223226

224-
void _checkDeprecatedSubclass(Annotation node, AstNode parent) {
227+
void _checkDeprecatedOptional(Annotation node) {
228+
var parent = node.parent;
229+
if (parent is FormalParameter) {
230+
FormalParameterList parameterList;
231+
if (parent.parent case FormalParameterList parent2) {
232+
parameterList = parent2;
233+
} else if (parent.parent case DefaultFormalParameter(
234+
parent: FormalParameterList parent2,
235+
)) {
236+
parameterList = parent2;
237+
} else {
238+
// We shouldn't get here; if we do, don't report the annotation.
239+
return;
240+
}
241+
242+
// This annotation is only valid on method declarations, constructor
243+
// declarations, and top-level function declarations.
244+
var isValidFunction =
245+
parameterList.parent is MethodDeclaration ||
246+
parameterList.parent is ConstructorDeclaration ||
247+
(parameterList.parent is FunctionExpression &&
248+
parameterList.parent?.parent is FunctionDeclaration &&
249+
parameterList.parent?.parent?.parent is CompilationUnit);
250+
251+
if (parent.isOptional && isValidFunction) return;
252+
}
253+
254+
_diagnosticReporter.atNode(
255+
node.name,
256+
WarningCode.invalidDeprecatedOptionalAnnotation,
257+
);
258+
}
259+
260+
void _checkDeprecatedSubclass(Annotation node) {
225261
Element? declaredElement;
226-
if (parent
262+
if (node.parent
227263
case ClassDeclaration(:var declaredFragment) ||
228264
ClassTypeAlias(:var declaredFragment)) {
229265
declaredElement = declaredFragment!.element;
230-
} else if (parent is MixinDeclaration) {
266+
} else if (node.parent case MixinDeclaration parent) {
231267
declaredElement = parent.declaredFragment!.element;
232-
} else if (parent is GenericTypeAlias) {
268+
} else if (node.parent case GenericTypeAlias parent) {
233269
declaredElement = parent.type.type?.element;
234270
}
235271

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,11 +337,20 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
337337
void visitDotShorthandConstructorInvocation(
338338
DotShorthandConstructorInvocation node,
339339
) {
340+
_deprecatedFunctionalityVerifier.dotShorthandConstructorInvocation(node);
340341
_deprecatedMemberUseVerifier.dotShorthandConstructorInvocation(node);
341342
_checkForLiteralConstructorUseInDotShorthand(node);
342343
super.visitDotShorthandConstructorInvocation(node);
343344
}
344345

346+
@override
347+
void visitDotShorthandInvocation(DotShorthandInvocation node) {
348+
_deprecatedFunctionalityVerifier.dotShorthandInvocation(node);
349+
// TODO(srawlins): I imagine we need to check with
350+
// `_deprecatedMemberUseVerifier` here...
351+
super.visitDotShorthandInvocation(node);
352+
}
353+
345354
@override
346355
void visitEnumDeclaration(EnumDeclaration node) {
347356
_deprecatedMemberUseVerifier.pushElement(node.declaredFragment!.element);
@@ -577,6 +586,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
577586
covariant InstanceCreationExpressionImpl node,
578587
) {
579588
_deprecatedMemberUseVerifier.instanceCreationExpression(node);
589+
_deprecatedFunctionalityVerifier.instanceCreationExpression(node);
580590
_nullSafeApiVerifier.instanceCreation(node);
581591
_checkForLiteralConstructorUse(node);
582592
super.visitInstanceCreationExpression(node);
@@ -655,6 +665,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
655665
@override
656666
void visitMethodInvocation(covariant MethodInvocationImpl node) {
657667
_deprecatedMemberUseVerifier.methodInvocation(node);
668+
_deprecatedFunctionalityVerifier.methodInvocation(node);
658669
_errorHandlerVerifier.verifyMethodInvocation(node);
659670
_nullSafeApiVerifier.methodInvocation(node);
660671
super.visitMethodInvocation(node);

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10904,6 +10904,19 @@ class WarningCode extends DiagnosticCodeWithExpectedTypes {
1090410904
expectedTypes: [],
1090510905
);
1090610906

10907+
/// Parameters:
10908+
/// Object parameterName: the name of the parameter
10909+
static const WarningTemplate<
10910+
LocatableDiagnostic Function({required Object parameterName})
10911+
>
10912+
deprecatedOptional = WarningTemplate(
10913+
'DEPRECATED_OPTIONAL',
10914+
"Omitting an argument for the '{0}' parameter is deprecated.",
10915+
correctionMessage: "Try passing an argument for '{0}'.",
10916+
withArguments: _withArgumentsDeprecatedOptional,
10917+
expectedTypes: [ExpectedType.object],
10918+
);
10919+
1090710920
/// Parameters:
1090810921
/// Object typeName: the name of the type
1090910922
static const WarningTemplate<
@@ -11394,6 +11407,21 @@ class WarningCode extends DiagnosticCodeWithExpectedTypes {
1139411407
expectedTypes: [],
1139511408
);
1139611409

11410+
/// This warning is generated anywhere where `@Deprecated.optional`
11411+
/// annotates something other than an optional parameter.
11412+
///
11413+
/// No parameters.
11414+
static const WarningWithoutArguments invalidDeprecatedOptionalAnnotation =
11415+
WarningWithoutArguments(
11416+
'INVALID_DEPRECATED_OPTIONAL_ANNOTATION',
11417+
"The annotation '@Deprecated.optional' can only be applied to optional "
11418+
"parameters.",
11419+
correctionMessage:
11420+
"Try removing the '@Deprecated.optional' annotation.",
11421+
hasPublishedDocs: true,
11422+
expectedTypes: [],
11423+
);
11424+
1139711425
/// No parameters.
1139811426
static const WarningWithoutArguments
1139911427
invalidDeprecatedSubclassAnnotation = WarningWithoutArguments(
@@ -12987,6 +13015,12 @@ class WarningCode extends DiagnosticCodeWithExpectedTypes {
1298713015
return LocatableDiagnosticImpl(deprecatedMixin, [typeName]);
1298813016
}
1298913017

13018+
static LocatableDiagnostic _withArgumentsDeprecatedOptional({
13019+
required Object parameterName,
13020+
}) {
13021+
return LocatableDiagnosticImpl(deprecatedOptional, [parameterName]);
13022+
}
13023+
1299013024
static LocatableDiagnostic _withArgumentsDeprecatedSubclass({
1299113025
required Object typeName,
1299213026
}) {

‎pkg/analyzer/lib/src/error/deprecated_functionality_verifier.dart‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,54 @@ class DeprecatedFunctionalityVerifier {
4343
}
4444
}
4545

46+
void dotShorthandConstructorInvocation(
47+
DotShorthandConstructorInvocation node,
48+
) {
49+
var element = node.constructorName.element;
50+
if (element is! ExecutableElement) return;
51+
_checkForDeprecatedOptional(
52+
element: element,
53+
argumentList: node.argumentList,
54+
errorNode: node.constructorName,
55+
);
56+
}
57+
58+
void dotShorthandInvocation(DotShorthandInvocation node) {
59+
var element = node.memberName.element;
60+
if (element is! ExecutableElement) return;
61+
_checkForDeprecatedOptional(
62+
element: element,
63+
argumentList: node.argumentList,
64+
errorNode: node.memberName,
65+
);
66+
}
67+
4668
void enumDeclaration(EnumDeclaration node) {
4769
_checkForDeprecatedImplement(node.implementsClause?.interfaces);
4870
_checkForDeprecatedMixin(node.withClause);
4971
}
5072

73+
void instanceCreationExpression(InstanceCreationExpression node) {
74+
var constructor = node.constructorName.element;
75+
if (constructor == null) return;
76+
_checkForDeprecatedOptional(
77+
element: constructor,
78+
argumentList: node.argumentList,
79+
errorNode: node.constructorName,
80+
);
81+
}
82+
83+
void methodInvocation(MethodInvocation node) {
84+
var method = node.methodName.element;
85+
if (method is! ExecutableElement) return;
86+
if (method is LocalFunctionElement) return;
87+
_checkForDeprecatedOptional(
88+
element: method,
89+
argumentList: node.argumentList,
90+
errorNode: node.methodName,
91+
);
92+
}
93+
5194
void mixinDeclaration(MixinDeclaration node) {
5295
_checkForDeprecatedImplement(node.implementsClause?.interfaces);
5396
// Not technically "implementing," but is similar enough for
@@ -117,6 +160,28 @@ class DeprecatedFunctionalityVerifier {
117160
}
118161
}
119162

163+
void _checkForDeprecatedOptional({
164+
required ExecutableElement element,
165+
required ArgumentList argumentList,
166+
required AstNode errorNode,
167+
}) {
168+
var omittedParameters = element.formalParameters.toList();
169+
for (var argument in argumentList.arguments) {
170+
var parameter = argument.correspondingParameter;
171+
if (parameter == null) continue;
172+
omittedParameters.remove(parameter);
173+
}
174+
for (var parameter in omittedParameters) {
175+
if (parameter.isDeprecatedWithKind('optional')) {
176+
_diagnosticReporter.atNode(
177+
errorNode,
178+
WarningCode.deprecatedOptional,
179+
arguments: [parameter.name ?? '<unknown>'],
180+
);
181+
}
182+
}
183+
}
184+
120185
void _checkForDeprecatedSubclass(List<NamedType>? namedTypes) {
121186
if (namedTypes == null) return;
122187
for (var namedType in namedTypes) {

‎pkg/analyzer/lib/src/test_utilities/mock_sdk.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,11 @@ class Deprecated extends Object {
322322
const Deprecated.instantiate([this.message])
323323
: _kind = _DeprecationKind.instantiate;
324324
const Deprecated.mixin([this.message]) : _kind = _DeprecationKind.mixin;
325+
const Deprecated.optional([this.message]) : _kind = _DeprecationKind.optional;
325326
}
326327
327328
enum _DeprecationKind {
328-
use, implement, extend, subclass, instantiate, mixin;
329+
use, implement, extend, subclass, instantiate, mixin, optional;
329330
}
330331
331332
class pragma {

0 commit comments

Comments
 (0)