Skip to content

Commit

Permalink
meta: Support @doNotStore on constructors and mixins
Browse files Browse the repository at this point in the history
There were no tests of this component of the annotation's description:

> The annotation can also be applied to a class to implicitly
> annotate all of the valid members of the class

So I add those as well.

Bug: #48476
Change-Id: If5f0f4c6057f57b4dfd01d8f648110d69fbc5eb4
Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367880
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
  • Loading branch information
srawlins authored and Commit Queue committed May 23, 2024
1 parent 798cbcf commit 0d5830e
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 20 deletions.
2 changes: 2 additions & 0 deletions pkg/analyzer/lib/src/test_utilities/mock_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,12 @@ class _Checked {
@Target({
TargetKind.classType,
TargetKind.constructor,
TargetKind.function,
TargetKind.getter,
TargetKind.library,
TargetKind.method,
TargetKind.mixinType,
})
class _DoNotStore {
const _DoNotStore();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,62 @@ class AssignmentOfDoNotStoreTest extends PubPackageResolutionTest {
writeTestPackageConfigWithMeta();
}

test_class_containingInstanceGetter() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
@doNotStore
class A {
String get v => '';
}
String f = A().v;
''', [
error(WarningCode.ASSIGNMENT_OF_DO_NOT_STORE, 91, 5),
]);
}

test_class_containingInstanceMethod() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
@doNotStore
class A {
String v() => '';
}
String f = A().v();
''', [
error(WarningCode.ASSIGNMENT_OF_DO_NOT_STORE, 89, 7),
]);
}

test_class_containingStaticGetter() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
@doNotStore
class A {
static String get v => '';
}
String f = A.v;
''', [
error(WarningCode.ASSIGNMENT_OF_DO_NOT_STORE, 98, 3),
]);
}

test_class_containingStaticMethod() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
@doNotStore
class A {
static String v() => '';
}
String f = A.v();
''', [
error(WarningCode.ASSIGNMENT_OF_DO_NOT_STORE, 96, 5),
]);
}

test_classMemberGetter() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -171,6 +227,23 @@ class B {
]);
}

test_mixin_containingInstanceMethod() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
@doNotStore
mixin M {
String v() => '';
}
abstract class A {
M get m;
late String f = m.v();
}
''', [
error(WarningCode.ASSIGNMENT_OF_DO_NOT_STORE, 126, 5),
]);
}

test_tearOff() async {
await assertNoErrorsInCode('''
import 'package:meta/meta.dart';
Expand Down
17 changes: 17 additions & 0 deletions pkg/analyzer/test/src/diagnostics/return_of_do_not_store_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,23 @@ class ReturnOfDoNotStoreTest extends PubPackageResolutionTest {
writeTestPackageConfigWithMeta();
}

test_constructor() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@doNotStore
A();
String getA() {
return A();
}
}
''', [
error(CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD, 95, 3),
]);
}

@FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/48476')
test_returnFromClosureInFunction() async {
await assertErrorsInCode('''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@ const _AlwaysThrows alwaysThrows = _AlwaysThrows();
@Deprecated('Use the `covariant` modifier instead')
const _Checked checked = _Checked();

/// Used to annotate a method, getter or top-level getter or function to
/// indicate that the value obtained by invoking it should not be stored in a
/// Used to annotate a method, getter, top-level function, or top-level getter
/// to indicate that the value obtained by invoking it should not be stored in a
/// field or top-level variable. The annotation can also be applied to a class
/// to implicitly annotate all of the valid members of the class, or applied to
/// a library to annotate all of the valid members of the library, including
/// classes. If a value returned by an element marked as `doNotStore` is returned
/// from a function or getter, that function or getter should be similarly
/// annotated.
/// classes. If a value returned by an element marked as `doNotStore` is
/// returned from a function or getter, that function or getter should be
/// similarly annotated.
///
/// Tools, such as the analyzer, can provide feedback if
///
Expand All @@ -90,9 +90,9 @@ const _Checked checked = _Checked();
/// or top-level variable.
const _DoNotStore doNotStore = _DoNotStore();

/// Used to annotate a method, getter or top-level getter or function that is
/// not intended to be accessed in checked-in code, but might be ephemerally
/// used during development or local testing.
/// Used to annotate a method, getter, top-level function, or top-level getter
/// that is not intended to be accessed in checked-in code, but might be
/// ephemerally used during development or local testing.
///
/// The intention of this annotation is to signify an API is available for
/// temporary or ephemeral use (such as debugging or local testing), but should
Expand Down Expand Up @@ -610,12 +610,12 @@ class _Checked {

@Target({
TargetKind.classType,
// TODO(srawlins): Add `TargetKind.constructor` when this annotation has
// functional tests. See https://github.com/dart-lang/sdk/issues/48476.
TargetKind.constructor,
TargetKind.function,
TargetKind.getter,
TargetKind.library,
TargetKind.method,
TargetKind.mixinType,
})
class _DoNotStore {
const _DoNotStore();
Expand Down
20 changes: 10 additions & 10 deletions pkg/meta/lib/meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@ const _AlwaysThrows alwaysThrows = _AlwaysThrows();
@Deprecated('Use the `covariant` modifier instead')
const _Checked checked = _Checked();

/// Used to annotate a method, getter or top-level getter or function to
/// indicate that the value obtained by invoking it should not be stored in a
/// Used to annotate a method, getter, top-level function, or top-level getter
/// to indicate that the value obtained by invoking it should not be stored in a
/// field or top-level variable. The annotation can also be applied to a class
/// to implicitly annotate all of the valid members of the class, or applied to
/// a library to annotate all of the valid members of the library, including
/// classes. If a value returned by an element marked as `doNotStore` is returned
/// from a function or getter, that function or getter should be similarly
/// annotated.
/// classes. If a value returned by an element marked as `doNotStore` is
/// returned from a function or getter, that function or getter should be
/// similarly annotated.
///
/// Tools, such as the analyzer, can provide feedback if
///
Expand All @@ -90,9 +90,9 @@ const _Checked checked = _Checked();
/// or top-level variable.
const _DoNotStore doNotStore = _DoNotStore();

/// Used to annotate a method, getter or top-level getter or function that is
/// not intended to be accessed in checked-in code, but might be ephemerally
/// used during development or local testing.
/// Used to annotate a method, getter, top-level function, or top-level getter
/// that is not intended to be accessed in checked-in code, but might be
/// ephemerally used during development or local testing.
///
/// The intention of this annotation is to signify an API is available for
/// temporary or ephemeral use (such as debugging or local testing), but should
Expand Down Expand Up @@ -641,12 +641,12 @@ class _Checked {

@Target({
TargetKind.classType,
// TODO(srawlins): Add `TargetKind.constructor` when this annotation has
// functional tests. See https://github.com/dart-lang/sdk/issues/48476.
TargetKind.constructor,
TargetKind.function,
TargetKind.getter,
TargetKind.library,
TargetKind.method,
TargetKind.mixinType,
})
class _DoNotStore {
const _DoNotStore();
Expand Down

0 comments on commit 0d5830e

Please sign in to comment.