Skip to content

Commit a30dc89

Browse files
pqCommit Queue
authored andcommitted
[CQ] [linter] de-duplicate reflectiveTestLoader handling
De-duplicate `reflectiveTestLoader` detection and testing Change-Id: Ia125bd10638b76452583b1a79c69884edcd9c72d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412666 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Phil Quitslund <[email protected]>
1 parent 4f4f867 commit a30dc89

File tree

6 files changed

+38
-58
lines changed

6 files changed

+38
-58
lines changed

pkg/linter/lib/src/extensions.dart

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,16 @@ extension DartTypeExtension on DartType? {
312312
_extendsClass(type.superclass, seenElements, className, library));
313313
}
314314

315+
extension ElementAnnotationExtension on ElementAnnotation {
316+
bool get isReflectiveTest => switch (element2) {
317+
GetterElement(:var name3, :var library2) =>
318+
name3 == 'reflectiveTest' &&
319+
library2.uri.toString() ==
320+
'package:test_reflective_loader/test_reflective_loader.dart',
321+
_ => false,
322+
};
323+
}
324+
315325
extension ElementExtension on Element2? {
316326
Element2? get canonicalElement2 => switch (this) {
317327
PropertyAccessorElement2(:var variable3?) => variable3,
@@ -480,6 +490,12 @@ extension InhertanceManager3Extension on InheritanceManager3 {
480490
}
481491
}
482492

493+
extension InstanceElementExtension on InstanceElement2 {
494+
bool get isReflectiveTest =>
495+
this is ClassElement2 &&
496+
metadata2.annotations.any((a) => a.isReflectiveTest);
497+
}
498+
483499
extension InterfaceElementExtension on InterfaceElement2 {
484500
/// Whether this element has the exact [name] and defined in the file with
485501
/// the given [uri].

pkg/linter/lib/src/rules/strict_top_level_inference.dart

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -268,23 +268,3 @@ class _Visitor extends SimpleAstVisitor<void> {
268268
}
269269
}
270270
}
271-
272-
extension on InstanceElement2 {
273-
// TODO(pq): share w/ unreachable_from_main
274-
bool get isReflectiveTest {
275-
var self = this;
276-
if (self is! ClassElement2) return false;
277-
278-
var metadata = self.metadata2;
279-
for (var i = 0; i < metadata.annotations.length; i++) {
280-
var annotation = metadata.annotations[i].element2;
281-
if (annotation is GetterElement &&
282-
annotation.name3 == 'reflectiveTest' &&
283-
annotation.library2.uri.toString() ==
284-
'package:test_reflective_loader/test_reflective_loader.dart') {
285-
return true;
286-
}
287-
}
288-
return false;
289-
}
290-
}

pkg/linter/lib/src/rules/unreachable_from_main.dart

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:collection/collection.dart';
1313
import 'package:pub_semver/pub_semver.dart';
1414

1515
import '../analyzer.dart';
16+
import '../extensions.dart';
1617

1718
const _desc = 'Unreachable top-level members in executable libraries.';
1819

@@ -187,10 +188,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor<void> {
187188
// This for-loop style is copied from analyzer's `hasX` getters on
188189
// [Element].
189190
for (var i = 0; i < metadata.annotations.length; i++) {
190-
var annotation = metadata.annotations[i].element2;
191-
if (annotation is GetterElement &&
192-
annotation.name3 == 'reflectiveTest' &&
193-
annotation.library2.name3 == 'test_reflective_loader') {
191+
if (metadata.annotations[i].isReflectiveTest) {
194192
// The class is instantiated through the use of mirrors in
195193
// 'test_reflective_loader'.
196194
var unnamedConstructor = element.constructors2.firstWhereOrNull(

pkg/linter/test/rule_test_support.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,25 @@ class PubPackageResolutionTest extends _ContextResolutionTest {
199199
@override
200200
List<String> get _collectionIncludedPaths => [workspaceRootPath];
201201

202+
void addReflectiveTestLoaderDep() {
203+
// TODO(pq): consider mopcing this into `writeTestPackageConfig`
204+
var testReflectiveLoaderPath = '$workspaceRootPath/test_reflective_loader';
205+
var packageConfigBuilder = PackageConfigFileBuilder();
206+
packageConfigBuilder.add(
207+
name: 'test_reflective_loader',
208+
rootPath: testReflectiveLoaderPath,
209+
);
210+
writeTestPackageConfig(packageConfigBuilder);
211+
newFile('$testReflectiveLoaderPath/lib/test_reflective_loader.dart', r'''
212+
library test_reflective_loader;
213+
214+
const Object reflectiveTest = _ReflectiveTest();
215+
class _ReflectiveTest {
216+
const _ReflectiveTest();
217+
}
218+
''');
219+
}
220+
202221
/// Asserts that the number of diagnostics reported in [content] matches the
203222
/// number of [expectedDiagnostics] and that they have the expected error
204223
/// descriptions and locations.

pkg/linter/test/rules/strict_top_level_inference_test.dart

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,25 +24,6 @@ class StrictTopLevelInferenceTest extends LintRuleTest {
2424
@override
2525
String get lintRule => LintNames.strict_top_level_inference;
2626

27-
void addReflectiveTestLoaderDep() {
28-
// TODO(pq): share setup logic with unreachable_from_main_test
29-
var testReflectiveLoaderPath = '$workspaceRootPath/test_reflective_loader';
30-
var packageConfigBuilder = PackageConfigFileBuilder();
31-
packageConfigBuilder.add(
32-
name: 'test_reflective_loader',
33-
rootPath: testReflectiveLoaderPath,
34-
);
35-
writeTestPackageConfig(packageConfigBuilder);
36-
newFile('$testReflectiveLoaderPath/lib/test_reflective_loader.dart', r'''
37-
library test_reflective_loader;
38-
39-
const Object reflectiveTest = _ReflectiveTest();
40-
class _ReflectiveTest {
41-
const _ReflectiveTest();
42-
}
43-
''');
44-
}
45-
4627
test_constructorParameter_named() async {
4728
await assertDiagnostics(
4829
r'''

pkg/linter/test/rules/unreachable_from_main_test.dart

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -798,21 +798,7 @@ class C {
798798
}
799799

800800
test_constructor_reachableViaTestReflectiveLoader() async {
801-
var testReflectiveLoaderPath = '$workspaceRootPath/test_reflective_loader';
802-
var packageConfigBuilder = PackageConfigFileBuilder();
803-
packageConfigBuilder.add(
804-
name: 'test_reflective_loader',
805-
rootPath: testReflectiveLoaderPath,
806-
);
807-
writeTestPackageConfig(packageConfigBuilder);
808-
newFile('$testReflectiveLoaderPath/lib/test_reflective_loader.dart', r'''
809-
library test_reflective_loader;
810-
811-
const Object reflectiveTest = _ReflectiveTest();
812-
class _ReflectiveTest {
813-
const _ReflectiveTest();
814-
}
815-
''');
801+
addReflectiveTestLoaderDep();
816802
await assertNoDiagnostics(r'''
817803
import 'package:test_reflective_loader/test_reflective_loader.dart';
818804

0 commit comments

Comments
 (0)