From f00dc3d05659addc2d3c4cfcf62765d1aba7da20 Mon Sep 17 00:00:00 2001 From: pq Date: Thu, 27 May 2021 00:26:54 +0000 Subject: [PATCH] @useResult verification See: https://github.com/dart-lang/linter/issues/1888 Change-Id: I3d5ed31f3a1dd0d69da5f47ae38496a4fe6051c0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201160 Reviewed-by: Brian Wilkerson Commit-Queue: Phil Quitslund --- .../lib/src/error/use_result_verifier.dart | 112 +++ .../lib/src/generated/error_verifier.dart | 7 +- .../test/src/diagnostics/test_all.dart | 2 + .../test/src/diagnostics/use_result_test.dart | 647 ++++++++++++++++++ 4 files changed, 767 insertions(+), 1 deletion(-) create mode 100644 pkg/analyzer/lib/src/error/use_result_verifier.dart create mode 100644 pkg/analyzer/test/src/diagnostics/use_result_test.dart diff --git a/pkg/analyzer/lib/src/error/use_result_verifier.dart b/pkg/analyzer/lib/src/error/use_result_verifier.dart new file mode 100644 index 00000000000..7a32f022b3e --- /dev/null +++ b/pkg/analyzer/lib/src/error/use_result_verifier.dart @@ -0,0 +1,112 @@ +// Copyright (c) 2021, 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/element/element.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:analyzer/src/dart/error/hint_codes.dart'; +import 'package:collection/collection.dart'; + +class UseResultVerifier { + final ErrorReporter _errorReporter; + + UseResultVerifier(this._errorReporter); + + void checkMethodInvocation(MethodInvocation node) { + var element = node.methodName.staticElement; + if (element == null) { + return; + } + + _check(node, element); + } + + void checkPropertyAccess(PropertyAccess node) { + var element = node.propertyName.staticElement; + if (element == null) { + return null; + } + + _check(node, element); + } + + void checkSimpleIdentifier(SimpleIdentifier node) { + if (node.inDeclarationContext()) { + return; + } + + var parent = node.parent; + // Covered by checkPropertyAccess and checkMethodInvocation respectively. + if (parent is PropertyAccess || parent is MethodInvocation) { + return; + } + + var element = node.staticElement; + if (element == null) { + return null; + } + + _check(node, element); + } + + void _check(AstNode node, Element element) { + var annotation = _getUseResultMetadata(element); + if (annotation == null) { + return; + } + + if (_isUsed(node)) { + return; + } + + var displayName = element.displayName; + + var message = _getUseResultMessage(annotation); + if (message == null || message.isEmpty) { + _errorReporter + .reportErrorForNode(HintCode.UNUSED_RESULT, node, [displayName]); + } else { + _errorReporter.reportErrorForNode( + HintCode.UNUSED_RESULT_WITH_MESSAGE, node, [displayName, message]); + } + } + + static String? _getUseResultMessage(ElementAnnotation annotation) { + if (annotation.element is PropertyAccessorElement) { + return null; + } + var constantValue = annotation.computeConstantValue(); + return constantValue?.getField('message')?.toStringValue(); + } + + static ElementAnnotation? _getUseResultMetadata(Element element) { + // Implicit getters/setters. + if (element.isSynthetic && element is PropertyAccessorElement) { + element = element.variable; + } + return element.metadata.firstWhereOrNull((e) => e.isUseResult); + } + + static bool _isUsed(AstNode node) { + var parent = node.parent; + if (parent == null) { + return false; + } + + if (parent is ParenthesizedExpression || parent is ConditionalExpression) { + return _isUsed(parent); + } + + return parent is ArgumentList || + parent is VariableDeclaration || + parent is MethodInvocation || + parent is PropertyAccess || + parent is ExpressionFunctionBody || + parent is ReturnStatement || + parent is FunctionExpressionInvocation || + parent is ListLiteral || + parent is SetOrMapLiteral || + parent is MapLiteralEntry; + } +} diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart index a1ab43c196b..9b72dd2ac9a 100644 --- a/pkg/analyzer/lib/src/generated/error_verifier.dart +++ b/pkg/analyzer/lib/src/generated/error_verifier.dart @@ -38,6 +38,7 @@ import 'package:analyzer/src/error/literal_element_verifier.dart'; import 'package:analyzer/src/error/required_parameters_verifier.dart'; import 'package:analyzer/src/error/return_type_verifier.dart'; import 'package:analyzer/src/error/type_arguments_verifier.dart'; +import 'package:analyzer/src/error/use_result_verifier.dart'; import 'package:analyzer/src/generated/element_resolver.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/error_detection_helpers.dart'; @@ -249,6 +250,7 @@ class ErrorVerifier extends RecursiveAstVisitor final RequiredParametersVerifier _requiredParametersVerifier; final DuplicateDefinitionVerifier _duplicateDefinitionVerifier; + final UseResultVerifier _checkUseVerifier; late final TypeArgumentsVerifier _typeArgumentsVerifier; late final ConstructorFieldsVerifier _constructorFieldsVerifier; late final ReturnTypeVerifier _returnTypeVerifier; @@ -258,6 +260,7 @@ class ErrorVerifier extends RecursiveAstVisitor this._inheritanceManager) : _uninstantiatedBoundChecker = _UninstantiatedBoundChecker(errorReporter), + _checkUseVerifier = UseResultVerifier(errorReporter), _requiredParametersVerifier = RequiredParametersVerifier(errorReporter), _duplicateDefinitionVerifier = DuplicateDefinitionVerifier(_currentLibrary, errorReporter) { @@ -930,6 +933,7 @@ class ErrorVerifier extends RecursiveAstVisitor } _typeArgumentsVerifier.checkMethodInvocation(node); _requiredParametersVerifier.visitMethodInvocation(node); + _checkUseVerifier.checkMethodInvocation(node); super.visitMethodInvocation(node); } @@ -1029,7 +1033,7 @@ class ErrorVerifier extends RecursiveAstVisitor _checkForInstanceAccessToStaticMember( typeReference, node.target, propertyName); _checkForUnnecessaryNullAware(target, node.operator); - + _checkUseVerifier.checkPropertyAccess(node); super.visitPropertyAccess(node); } @@ -1100,6 +1104,7 @@ class ErrorVerifier extends RecursiveAstVisitor if (!_isUnqualifiedReferenceToNonLocalStaticMemberAllowed(node)) { _checkForUnqualifiedReferenceToNonLocalStaticMember(node); } + _checkUseVerifier.checkSimpleIdentifier(node); super.visitSimpleIdentifier(node); } diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart index 715812c1afc..97e500502d7 100644 --- a/pkg/analyzer/test/src/diagnostics/test_all.dart +++ b/pkg/analyzer/test/src/diagnostics/test_all.dart @@ -685,6 +685,7 @@ import 'uri_with_interpolation_test.dart' as uri_with_interpolation; import 'use_of_native_extension_test.dart' as use_of_native_extension; import 'use_of_nullable_value_test.dart' as use_of_nullable_value_test; import 'use_of_void_result_test.dart' as use_of_void_result; +import 'use_result_test.dart' as use_result; import 'variable_type_mismatch_test.dart' as variable_type_mismatch; import 'void_with_type_arguments_test.dart' as void_with_type_arguments_test; import 'wrong_number_of_parameters_for_operator_test.dart' @@ -1165,6 +1166,7 @@ main() { use_of_native_extension.main(); use_of_nullable_value_test.main(); use_of_void_result.main(); + use_result.main(); variable_type_mismatch.main(); void_with_type_arguments_test.main(); wrong_number_of_parameters_for_operator.main(); diff --git a/pkg/analyzer/test/src/diagnostics/use_result_test.dart b/pkg/analyzer/test/src/diagnostics/use_result_test.dart new file mode 100644 index 00000000000..154d9c0d7fb --- /dev/null +++ b/pkg/analyzer/test/src/diagnostics/use_result_test.dart @@ -0,0 +1,647 @@ +// Copyright (c) 2021, 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/src/error/codes.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../dart/resolution/context_collection_resolution.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(UseResultTest); + }); +} + +@reflectiveTest +class UseResultTest extends PubPackageResolutionTest { + @override + void setUp() { + super.setUp(); + writeTestPackageConfigWithMeta(); + } + + test_field_result_assigned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + var bar = A().foo; // OK + print(bar); +} +'''); + } + + test_field_result_assigned_conditional_else() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(bool b) { + var bar = b ? 0 : A().foo; // OK + print(bar); +} +'''); + } + + test_field_result_assigned_conditional_if() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(bool b) { + var bar = b ? A().foo : 0; // OK + print(bar); +} +'''); + } + + test_field_result_assigned_conditional_if_parens() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(bool b) { + var c = b ? (A().foo) : 0; + print(c); +} +'''); + } + + test_field_result_assigned_parenthesized() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + var bar = ((A().foo)); // OK + print(bar); +} +'''); + } + + test_field_result_functionExpression_unused() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + Function foo = () {}; +} + +void main() { + A().foo; +} +''', [ + error(HintCode.UNUSED_RESULT, 100, 7), + ]); + } + + test_field_result_functionExpression_used() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + Function foo = () {}; +} + +void main() { + A().foo(); +} +'''); + } + + test_field_result_passed() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + print(A().foo); // OK +} +'''); + } + + test_field_result_returned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +int f() => A().foo; +int f2() { + return A().foo; +} +'''); + } + + test_field_result_targetedMethod() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + String foo = ''; +} + +void main() { + A().foo.toString(); // OK +} +'''); + } + + test_field_result_targetedProperty() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + String foo = ''; +} + +void main() { + A().foo.hashCode; // OK +} +'''); + } + + test_field_result_unassigned() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + A().foo; +} +''', [ + error(HintCode.UNUSED_RESULT, 91, 7), + ]); + } + + test_field_result_unassigned_conditional_if() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(bool b) { + b ? A().foo : 0; +} +''', [ + error(HintCode.UNUSED_RESULT, 98, 7), + ]); + } + + test_field_result_unassigned_conditional_if_parens() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(bool b) { + b ? (A().foo) : 0; +} +''', [ + error(HintCode.UNUSED_RESULT, 99, 7), + ]); + } + + test_field_result_unassigned_in_closure() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(Function g) { } + +void main() { + f(() { + A().foo; + }); +} +''', [ + error(HintCode.UNUSED_RESULT, 126, 7), + ]); + } + + test_field_result_used_conditional_if_parens() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void f(bool b) { + (b ? A().foo : 0).toString(); +} +'''); + } + + test_field_result_used_listLiteral() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + var l = [ A().foo ]; // OK + print(l); + [ A().foo ]; // Also OK +} +'''); + } + + test_field_result_used_mapLiteral_key() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + var m = { A().foo : 'baz'}; // OK + print(m); +} +'''); + } + + test_field_result_used_mapLiteral_value() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + var m = { 'baz': A().foo }; // OK + print(m); +} +'''); + } + + test_field_result_used_setLiteral() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo = 0; +} + +void main() { + var s = { A().foo }; // OK + print(s); +} +'''); + } + + test_getter_result_passed() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int get foo => 0; +} + +void main() { + print(A().foo); // OK +} +'''); + } + + test_getter_result_returned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int get foo => 0; +} + +int f() => A().foo; +int f2() { + return A().foo; +} +'''); + } + + test_getter_result_targetedMethod() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + String get foo => ''; +} + +void main() { + A().foo.toString(); // OK +} +'''); + } + + test_getter_result_targetedProperty() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + String get foo => ''; +} + +void main() { + A().foo.hashCode; // OK +} +'''); + } + + test_getter_result_unassigned() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int get foo => 0; +} + +void main() { + A().foo; +} +''', [error(HintCode.UNUSED_RESULT, 96, 7)]); + } + + test_method_result_assigned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo() => 0; +} + +void main() { + var bar = A().foo(); // OK + print(bar); +} +'''); + } + + test_method_result_passed() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo() => 0; +} + +void main() { + print(A().foo()); // OK +} +'''); + } + + test_method_result_returned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo() => 0; +} + +int f() => A().foo(); +int f2() { + return A().foo(); +} +'''); + } + + test_method_result_targetedMethod() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + String foo() => ''; +} + +void main() { + A().foo().toString(); // OK +} +'''); + } + + test_method_result_targetedProperty() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + String foo() => ''; +} + +void main() { + A().foo().hashCode; // OK +} +'''); + } + + test_method_result_unassigned() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +class A { + @useResult + int foo() => 0; +} + +void main() { + A().foo(); +} +''', [ + error(HintCode.UNUSED_RESULT, 94, 9), + ]); + } + + test_topLevelFunction_result_assigned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo() => 0; + +void main() { + var x = foo(); // OK + print(x); +} +'''); + } + + test_topLevelFunction_result_passed() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo() => 0; + +void main() { + print(foo()); // OK +} +'''); + } + + test_topLevelFunction_result_targetedMethod() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +String foo() => ''; + +void main() { + foo().toString(); +} +'''); + } + + test_topLevelFunction_result_targetedProperty() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +String foo() => ''; + +void main() { + foo().length; +} +'''); + } + + // todo(pq):implement visitExpressionStatement? + test_topLevelFunction_result_unassigned() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo() => 0; +void bar() {} +int baz() => 0; + +void main() { + foo(); + bar(); // OK + baz(); // OK +} +''', [ + error(HintCode.UNUSED_RESULT, 108, 5), + ]); + } + + test_topLevelVariable_assigned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo = 0; + +void main() { + var bar = foo; // OK + print(bar); +} +'''); + } + + test_topLevelVariable_passed() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo = 0; + +void main() { + print(foo); // OK +} +'''); + } + + test_topLevelVariable_returned() async { + await assertNoErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo = 0; + +int bar() => foo; // OK +int baz() { + return foo; // OK +} +'''); + } + + test_topLevelVariable_unused() async { + await assertErrorsInCode(r''' +import 'package:meta/meta.dart'; + +@useResult +int foo = 0; + +void main() { + foo; +} +''', [ + error(HintCode.UNUSED_RESULT, 75, 3), + ]); + } +}