From 755432f73e1bb98604bae226e27d3ef78b77f8ac Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Sat, 25 Mar 2023 01:39:37 +0000 Subject: [PATCH] Reinstate HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE This reverts parts of https://dart-review.googlesource.com/c/sdk/+/289444 The code needs to be reinstated as flutter has tests which verify that certain data-driven fixes work, which happen to be uses of deprecated members, from the same package. See this failure for example: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8785707696655090305/+/u/analyze_flutter_flutter/stdout?format=raw The linter bump is kept the same, and some code which was unnecessarily coupled tightly with DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE is kept the same. The big test refactor which was included in 289444 is also kept the same, only a few tests are added to verify HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE gets reported. Bug: https://github.com/dart-lang/sdk/issues/51678 Change-Id: I6852376b299d8375c720ea56dc6a6a6119de3364 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291054 Commit-Queue: Samuel Rawlins Reviewed-by: Brian Wilkerson --- pkg/analysis_server/lib/src/lsp/mapping.dart | 6 +++ .../correction/bulk_fix_processor.dart | 6 +++ .../src/services/correction/fix_internal.dart | 3 ++ .../lib/src/services/linter/lint_names.dart | 2 + .../integration/linter/lint_names_test.dart | 31 +++++++-------- .../error/deprecated_member_use_verifier.dart | 11 +++--- .../deprecated_member_use_test.dart | 38 ++++++++++++++++++- .../test/verify_diagnostics_test.dart | 2 - 8 files changed, 73 insertions(+), 26 deletions(-) diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart index 2135f67f109..20b55ad67d1 100644 --- a/pkg/analysis_server/lib/src/lsp/mapping.dart +++ b/pkg/analysis_server/lib/src/lsp/mapping.dart @@ -34,6 +34,12 @@ const languageSourceName = 'dart'; final diagnosticTagsForErrorCode = >{ _errorCode(WarningCode.DEAD_CODE): [lsp.DiagnosticTag.Unnecessary], + _errorCode(HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE): [ + lsp.DiagnosticTag.Deprecated + ], + _errorCode(HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE_WITH_MESSAGE): [ + lsp.DiagnosticTag.Deprecated + ], _errorCode(HintCode.DEPRECATED_MEMBER_USE): [lsp.DiagnosticTag.Deprecated], 'deprecated_member_use_from_same_package': [lsp.DiagnosticTag.Deprecated], 'deprecated_member_use_from_same_package_with_message': [ diff --git a/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart index 47e07ed5abc..0ea868bd7ef 100644 --- a/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart +++ b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart @@ -132,6 +132,12 @@ class BulkFixProcessor { HintCode.DEPRECATED_MEMBER_USE: [ DataDriven.new, ], + HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE: [ + DataDriven.new, + ], + HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE_WITH_MESSAGE: [ + DataDriven.new, + ], HintCode.DEPRECATED_MEMBER_USE_WITH_MESSAGE: [ DataDriven.new, ], diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 13335b77031..17e581833d6 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -366,6 +366,9 @@ class FixProcessor extends BaseProcessor { LintNames.deprecated_member_use_from_same_package: [ DataDriven.new, ], + LintNames.deprecated_member_use_from_same_package_with_message: [ + DataDriven.new, + ], }; /// A map from the names of lint rules to a list of the generators that are diff --git a/pkg/analysis_server/lib/src/services/linter/lint_names.dart b/pkg/analysis_server/lib/src/services/linter/lint_names.dart index 1743c25c0de..9943b67bc12 100644 --- a/pkg/analysis_server/lib/src/services/linter/lint_names.dart +++ b/pkg/analysis_server/lib/src/services/linter/lint_names.dart @@ -59,6 +59,8 @@ class LintNames { 'dangling_library_doc_comments'; static const String deprecated_member_use_from_same_package = 'deprecated_member_use_from_same_package'; + static const String deprecated_member_use_from_same_package_with_message = + 'deprecated_member_use_from_same_package_with_message'; static const String diagnostic_describe_all_properties = 'diagnostic_describe_all_properties'; static const String directives_ordering = 'directives_ordering'; diff --git a/pkg/analysis_server/test/integration/linter/lint_names_test.dart b/pkg/analysis_server/test/integration/linter/lint_names_test.dart index ed2c4168b8b..e05b138e8a3 100644 --- a/pkg/analysis_server/test/integration/linter/lint_names_test.dart +++ b/pkg/analysis_server/test/integration/linter/lint_names_test.dart @@ -6,7 +6,6 @@ import 'package:analyzer/dart/analysis/analysis_context_collection.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -import 'package:analyzer/src/lint/linter.dart'; import 'package:analyzer/src/lint/registry.dart'; import 'package:analyzer_utilities/package_root.dart'; import 'package:linter/src/rules.dart'; @@ -14,6 +13,20 @@ import 'package:path/path.dart' as path; import 'package:test/test.dart'; void main() { + late List registeredLintNames; + + setUp(() { + if (Registry.ruleRegistry.isEmpty) { + registerLintRules(); + } + + registeredLintNames = [ + for (var rule in Registry.ruleRegistry) + ...rule.lintCodes + .map((e) => e.uniqueName.replaceFirst('LintCode.', '')), + ]; + }); + /// Ensure server lint name representations correspond w/ actual lint rules. /// See, e.g., https://dart-review.googlesource.com/c/sdk/+/95743. test('lint_names', () async { @@ -43,22 +56,6 @@ void main() { }); } -List? _registeredLints; - -Iterable get registeredLintNames => registeredLints.map((r) => r.name); - -List get registeredLints { - var registeredLints = _registeredLints; - if (registeredLints == null) { - if (Registry.ruleRegistry.isEmpty) { - registerLintRules(); - } - registeredLints = Registry.ruleRegistry.toList(); - _registeredLints = registeredLints; - } - return registeredLints; -} - class _FixCollector extends GeneralizingAstVisitor { final List lintNames = []; diff --git a/pkg/analyzer/lib/src/error/deprecated_member_use_verifier.dart b/pkg/analyzer/lib/src/error/deprecated_member_use_verifier.dart index cf5590ed769..8fff94eb497 100644 --- a/pkg/analyzer/lib/src/error/deprecated_member_use_verifier.dart +++ b/pkg/analyzer/lib/src/error/deprecated_member_use_verifier.dart @@ -306,14 +306,13 @@ class DeprecatedMemberUseVerifier extends BaseDeprecatedMemberUseVerifier { void reportError( AstNode errorNode, Element element, String displayName, String? message) { var library = element is LibraryElement ? element : element.library; - if (_isLibraryInWorkspacePackage(library)) { - return; - } message = message?.trim(); if (message == null || message.isEmpty || message == '.') { _errorReporter.reportErrorForNode( - HintCode.DEPRECATED_MEMBER_USE, + _isLibraryInWorkspacePackage(library) + ? HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE + : HintCode.DEPRECATED_MEMBER_USE, errorNode, [displayName], ); @@ -324,7 +323,9 @@ class DeprecatedMemberUseVerifier extends BaseDeprecatedMemberUseVerifier { message = '$message.'; } _errorReporter.reportErrorForNode( - HintCode.DEPRECATED_MEMBER_USE_WITH_MESSAGE, + _isLibraryInWorkspacePackage(library) + ? HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE_WITH_MESSAGE + : HintCode.DEPRECATED_MEMBER_USE_WITH_MESSAGE, errorNode, [displayName, message], ); diff --git a/pkg/analyzer/test/src/diagnostics/deprecated_member_use_test.dart b/pkg/analyzer/test/src/diagnostics/deprecated_member_use_test.dart index 60415ce05bf..437ba304015 100644 --- a/pkg/analyzer/test/src/diagnostics/deprecated_member_use_test.dart +++ b/pkg/analyzer/test/src/diagnostics/deprecated_member_use_test.dart @@ -341,6 +341,19 @@ export 'package:aaa/a.dart'; ]); } + test_export_fromSamePackage() async { + newFile('$testPackageLibPath/lib2.dart', r''' +@deprecated +library a; +'''); + + await assertErrorsInCode(''' +export 'lib2.dart'; +''', [ + error(HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE, 0, 19), + ]); + } + test_extensionOverride() async { await assertErrorsInCode2( externalCode: r''' @@ -786,6 +799,25 @@ var x = f(); ); } + test_methodInvocation_fromSamePackage() async { + newFile('$testPackageLibPath/lib2.dart', r''' +class A { + @deprecated + void foo() {} +} +'''); + + await assertErrorsInCode(r''' +import 'lib2.dart'; + +void f(A a) { + a.foo(); +} +''', [ + error(HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE, 39, 3), + ]); + } + test_methodInvocation_inDeprecatedConstructor() async { await assertNoErrorsInCode(r''' class A { @@ -1195,12 +1227,14 @@ class B extends A { } test_redirectingConstructorInvocation_namedParameter() async { - await assertNoErrorsInCode(r''' + await assertErrorsInCode(r''' class A { A({@deprecated int a = 0}) {} A.named() : this(a: 0); } -'''); +''', [ + error(HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE, 61, 1), + ]); } test_setterInvocation() async { diff --git a/pkg/analyzer/test/verify_diagnostics_test.dart b/pkg/analyzer/test/verify_diagnostics_test.dart index bb710e6f3e6..3be55075713 100644 --- a/pkg/analyzer/test/verify_diagnostics_test.dart +++ b/pkg/analyzer/test/verify_diagnostics_test.dart @@ -99,8 +99,6 @@ class DocumentationValidator { 'HintCode.DEPRECATED_COLON_FOR_DEFAULT_VALUE', // The code has been replaced but is not yet removed. 'HintCode.DEPRECATED_MEMBER_USE', - // This is deprecated. - 'HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE', // Produces two diagnostics when it should only produce one (see // https://github.com/dart-lang/sdk/issues/43051) 'HintCode.UNNECESSARY_NULL_COMPARISON_FALSE',