Skip to content

Commit aebc7fd

Browse files
srawlinsCommit Queue
authored andcommitted
analyzer: Tidy up Spelunker API
Work towards #55660 Spelunker will move to the analyzer_testing package, so we must first clean it up. Spelunker had two subclasses, but their only differentiation was the calculation of a getter, which could be a final field. So I remove the subclasses, move the "File"-based one to the spelunk utility script, and change the Spelunker constructor to take in a source string. Additionally: * Make all of the fields private. * Remove isDartFileName and isPubspecFileName. These calculations can be inlined into their call sites and make use of the `file_paths` library. Change-Id: I4b97ed03a2845440dabd3a9ab0aa3b3ba4af5959 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428083 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent dcf3531 commit aebc7fd

File tree

6 files changed

+28
-82
lines changed

6 files changed

+28
-82
lines changed

pkg/analyzer/lib/src/lint/io.dart

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,11 @@
44

55
import 'dart:io';
66

7-
import 'package:analyzer/src/lint/util.dart';
8-
import 'package:path/path.dart' as p;
9-
107
/// A shared sink for standard error reporting.
118
StringSink errorSink = stderr;
129

1310
/// A shared sink for standard out reporting.
1411
StringSink outSink = stdout;
1512

16-
/// Returns `true` if this [entry] is a Dart file.
17-
bool isDartFile(FileSystemEntity entry) => isDartFileName(entry.path);
18-
19-
/// Returns `true` if this [entry] is a pubspec file.
20-
bool isPubspecFile(FileSystemEntity entry) =>
21-
isPubspecFileName(p.basename(entry.path));
22-
2313
/// Synchronously read the contents of the file at the given [path] as a string.
2414
String readFile(String path) => File(path).readAsStringSync();

pkg/analyzer/lib/src/lint/util.dart

Lines changed: 10 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -9,71 +9,20 @@ import 'package:analyzer/dart/analysis/utilities.dart';
99
import 'package:analyzer/dart/ast/ast.dart';
1010
import 'package:analyzer/dart/ast/token.dart';
1111
import 'package:analyzer/dart/ast/visitor.dart';
12-
import 'package:path/path.dart' as path;
13-
14-
final _pubspec = RegExp(r'^[_]?pubspec\.yaml$');
15-
16-
/// Create a library name prefix based on [libraryPath], [projectRoot] and
17-
/// current [packageName].
18-
String createLibraryNamePrefix({
19-
required String libraryPath,
20-
String? projectRoot,
21-
String? packageName,
22-
}) {
23-
// Use the posix context to canonicalize separators (`\`).
24-
var libraryDirectory = path.posix.dirname(libraryPath);
25-
var relativePath = path.posix.relative(libraryDirectory, from: projectRoot);
26-
// Drop 'lib/'.
27-
var segments = path.split(relativePath);
28-
if (segments[0] == 'lib') {
29-
relativePath = path.posix.joinAll(segments.sublist(1));
30-
}
31-
// Replace separators.
32-
relativePath = relativePath.replaceAll('/', '.');
33-
// Add separator if needed.
34-
if (relativePath.isNotEmpty) {
35-
relativePath = '.$relativePath';
36-
}
37-
38-
return '$packageName$relativePath';
39-
}
4012

41-
/// Returns `true` if this [fileName] is a Dart file.
42-
bool isDartFileName(String fileName) => fileName.endsWith('.dart');
13+
final class Spelunker {
14+
final StringSink _sink;
15+
final FeatureSet _featureSet;
16+
final String _source;
4317

44-
/// Returns `true` if this [fileName] is a Pubspec file.
45-
bool isPubspecFileName(String fileName) => _pubspec.hasMatch(fileName);
46-
47-
class FileSpelunker extends _AbstractSpelunker {
48-
final String path;
49-
FileSpelunker(this.path, {super.sink, super.featureSet});
50-
@override
51-
String getSource() => File(path).readAsStringSync();
52-
}
53-
54-
class StringSpelunker extends _AbstractSpelunker {
55-
final String source;
56-
StringSpelunker(this.source, {super.sink, super.featureSet});
57-
@override
58-
String getSource() => source;
59-
}
60-
61-
abstract class _AbstractSpelunker {
62-
final StringSink sink;
63-
FeatureSet featureSet;
64-
65-
_AbstractSpelunker({StringSink? sink, FeatureSet? featureSet})
66-
: sink = sink ?? stdout,
67-
featureSet = featureSet ?? FeatureSet.latestLanguageVersion();
68-
69-
String getSource();
18+
Spelunker(this._source, {StringSink? sink, FeatureSet? featureSet})
19+
: _sink = sink ?? stdout,
20+
_featureSet = featureSet ?? FeatureSet.latestLanguageVersion();
7021

7122
void spelunk() {
72-
var contents = getSource();
73-
74-
var parseResult = parseString(content: contents, featureSet: featureSet);
23+
var parseResult = parseString(content: _source, featureSet: _featureSet);
7524

76-
var visitor = _SourceVisitor(sink);
25+
var visitor = _SourceVisitor(_sink);
7726
parseResult.unit.accept(visitor);
7827
}
7928
}
@@ -85,8 +34,7 @@ class _SourceVisitor extends GeneralizingAstVisitor {
8534

8635
_SourceVisitor(this.sink);
8736

88-
String asString(AstNode node) =>
89-
'${typeInfo(node.runtimeType)} [${node.toString()}]';
37+
String asString(AstNode node) => '${typeInfo(node.runtimeType)} [$node]';
9038

9139
List<CommentToken> getPrecedingComments(Token token) {
9240
var comments = <CommentToken>[];

pkg/linter/lib/src/test_utilities/test_linter.dart

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ import 'package:analyzer/file_system/physical_file_system.dart' as file_system;
1111
import 'package:analyzer/source/file_source.dart';
1212
import 'package:analyzer/source/source.dart';
1313
// ignore: implementation_imports
14-
import 'package:analyzer/src/lint/io.dart';
15-
// ignore: implementation_imports
1614
import 'package:analyzer/src/lint/pub.dart';
15+
// ignore: implementation_imports
16+
import 'package:analyzer/src/util/file_paths.dart' as file_paths;
1717
import 'package:meta/meta.dart';
1818
import 'package:path/path.dart' as path;
1919

@@ -39,10 +39,11 @@ class TestLinter implements AnalysisErrorListener {
3939
resourceProvider ?? file_system.PhysicalResourceProvider.INSTANCE;
4040

4141
Future<List<DiagnosticInfo>> lintFiles(List<File> files) async {
42-
var errors = <DiagnosticInfo>[];
4342
var lintDriver = LintDriver(options, _resourceProvider);
44-
errors.addAll(await lintDriver.analyze(files.where(isDartFile)));
45-
for (var file in files.where(isPubspecFile)) {
43+
var errors = await lintDriver.analyze(
44+
files.where((f) => f.path.endsWith('.dart')),
45+
);
46+
for (var file in files.where(_isPubspecFile)) {
4647
lintPubspecSource(
4748
contents: file.readAsStringSync(),
4849
sourcePath: _resourceProvider.pathContext.normalize(file.absolute.path),
@@ -77,4 +78,8 @@ class TestLinter implements AnalysisErrorListener {
7778

7879
@override
7980
void onError(Diagnostic error) => errors.add(error);
81+
82+
/// Returns whether this [entry] is a pubspec file.
83+
bool _isPubspecFile(FileSystemEntity entry) =>
84+
path.basename(entry.path) == file_paths.pubspecYaml;
8085
}

pkg/linter/test/rule_test_support.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ class PubPackageResolutionTest extends _ContextResolutionTest {
446446
try {
447447
var astSink = StringBuffer();
448448

449-
StringSpelunker(
449+
Spelunker(
450450
result.unit.toSource(),
451451
sink: astSink,
452452
featureSet: result.unit.featureSet,

pkg/linter/tool/benchmark.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'dart:math' as math;
99
import 'package:analyzer/src/lint/config.dart';
1010
import 'package:analyzer/src/lint/io.dart';
1111
import 'package:analyzer/src/lint/registry.dart';
12-
import 'package:analyzer/src/lint/util.dart';
12+
import 'package:analyzer/src/util/file_paths.dart' as file_paths;
1313
import 'package:args/args.dart';
1414
import 'package:linter/src/analyzer.dart';
1515
import 'package:linter/src/extensions.dart';
@@ -266,7 +266,7 @@ extension on String {
266266

267267
/// Whether this path is a Dart file or a Pubspec file.
268268
bool get isLintable =>
269-
isDartFileName(this) || isPubspecFileName(path.basename(this));
269+
endsWith('.dart') || path.basename(this) == file_paths.pubspecYaml;
270270
}
271271

272272
extension on StringSink {

pkg/linter/tool/spelunk.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:analyzer/src/lint/util.dart' show FileSpelunker;
5+
import 'dart:io';
6+
7+
import 'package:analyzer/src/lint/util.dart' show Spelunker;
68
import 'package:args/args.dart';
79

810
/// AST Spelunker
@@ -11,6 +13,7 @@ void main(List<String> args) {
1113

1214
var options = parser.parse(args);
1315
for (var path in options.rest) {
14-
FileSpelunker(path).spelunk();
16+
var source = File(path).readAsStringSync();
17+
Spelunker(source).spelunk();
1518
}
1619
}

0 commit comments

Comments
 (0)