Skip to content

Commit 082c710

Browse files
srawlinsCommit Queue
authored and
Commit Queue
committed
analyzer: Move enableTiming out of AnalysisOptionsImpl to AnalysisDriver.
It has seemed disingenuous to me that `enableTiming` has been a property of AnalysisOptionsImpl, when it cannot be specified in an analysis options file. Additionally, it is used in exactly _one_ place: the linter package's `benchmark.dart` script. Instead, it makes more sense to think of this "flag" as a process-wide flag. It is not something that may be set one way for some files, and another for another set of files. And it is not something that can be set to one value early during an analysis process, and then change; it never changes. So instead this change makes it a _final_, _private_ field on AnalysisDriver, and on LibraryAnalyzer. LibraryAnalyzer gets its value from AnalysisDriver, and AnalysisDriver gets its value from the AnalysisContextCollectionImpl constructor. Change-Id: I347605775680a8ea1c1237f0ee20bd4bbe2c6216 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393961 Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent e7c0452 commit 082c710

File tree

10 files changed

+46
-34
lines changed

10 files changed

+46
-34
lines changed

pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ class BulkFixProcessor {
550550
LintRuleUnitContext currentUnit,
551551
List<LintRuleUnitContext> allUnits,
552552
) {
553-
var nodeRegistry = NodeLintRegistry(false);
553+
var nodeRegistry = NodeLintRegistry(enableTiming: false);
554554
var context = LinterContextWithParsedResults(allUnits, currentUnit);
555555
var lintRules =
556556
_syntacticLintCodes

pkg/analysis_server_plugin/lib/src/plugin_server.dart

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import 'package:analyzer/src/dart/analysis/byte_store.dart';
2626
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
2727
import 'package:analyzer/src/dart/analysis/session.dart';
2828
import 'package:analyzer/src/dart/element/type_system.dart';
29-
import 'package:analyzer/src/lint/analysis_rule_timers.dart';
3029
import 'package:analyzer/src/lint/linter.dart';
3130
import 'package:analyzer/src/lint/linter_visitor.dart';
3231
import 'package:analyzer/src/lint/registry.dart';
@@ -240,8 +239,9 @@ class PluginServer {
240239
),
241240
];
242241

243-
var enableTiming = analysisOptions.enableTiming;
244-
var nodeRegistry = NodeLintRegistry(enableTiming);
242+
// TODO(srawlins): Enable timing similar to what the linter package's
243+
// `benchhmark.dart` script does.
244+
var nodeRegistry = NodeLintRegistry(enableTiming: false);
245245

246246
var context = LinterContextWithResolvedResults(
247247
allUnits,
@@ -261,10 +261,9 @@ class PluginServer {
261261
Registry.ruleRegistry.enabled(configuration.diagnosticConfigs);
262262
for (var rule in rules) {
263263
rule.reporter = errorReporter;
264-
var timer = enableTiming ? analysisRuleTimers.getTimer(rule) : null;
265-
timer?.start();
264+
// TODO(srawlins): Enable timing similar to what the linter package's
265+
// `benchhmark.dart` script does.
266266
rule.registerNodeProcessors(nodeRegistry, context);
267-
timer?.stop();
268267
}
269268
}
270269

pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
6666
required DartSdk sdk,
6767
})? updateAnalysisOptions2,
6868
MacroSupportFactory? macroSupportFactory,
69+
bool enableLintRuleTiming = false,
6970
}) : resourceProvider =
7071
resourceProvider ?? PhysicalResourceProvider.INSTANCE {
7172
sdkPath ??= getSdkPath();
@@ -124,6 +125,7 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
124125
infoDeclarationStore: infoDeclarationStore,
125126
macroSupport: macroSupport,
126127
ownedFiles: ownedFiles,
128+
enableLintRuleTiming: enableLintRuleTiming,
127129
);
128130
contexts.add(context);
129131
}

pkg/analyzer/lib/src/dart/analysis/analysis_options.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,6 @@ class AnalysisOptionsImpl implements AnalysisOptions {
328328
@override
329329
List<PluginConfiguration> pluginConfigurations = [];
330330

331-
/// Whether timing data should be gathered during execution.
332-
bool enableTiming = false;
333-
334331
@override
335332
final List<ErrorProcessor> errorProcessors;
336333

pkg/analyzer/lib/src/dart/analysis/context_builder.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class ContextBuilderImpl {
9090
InfoDeclarationStore? infoDeclarationStore,
9191
MacroSupport? macroSupport,
9292
OwnedFiles? ownedFiles,
93+
bool enableLintRuleTiming = false,
9394
}) {
9495
byteStore ??= MemoryByteStore();
9596
performanceLog ??= PerformanceLog(null);
@@ -162,6 +163,7 @@ class ContextBuilderImpl {
162163
declaredVariables: declaredVariables,
163164
testView: retainDataForTesting ? AnalysisDriverTestView() : null,
164165
ownedFiles: ownedFiles,
166+
enableLintRuleTiming: enableLintRuleTiming,
165167
);
166168

167169
// AnalysisDriver reports results into streams.

pkg/analyzer/lib/src/dart/analysis/driver.dart

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ import 'package:meta/meta.dart';
7676
/// "analysis state" be either "working" or "idle".
7777
///
7878
/// (These are theoretical constructs; they may not necessarily reflect data
79-
/// structures maintained explicitly by the driver).
79+
/// structures maintained explicitly by the driver.)
8080
///
8181
/// Then we make the following guarantees:
8282
///
@@ -267,6 +267,9 @@ class AnalysisDriver {
267267
/// A map that associates files to corresponding analysis options.
268268
late final AnalysisOptionsMap analysisOptionsMap;
269269

270+
/// Whether timing data should be gathered during lint rule execution.
271+
final bool _enableLintRuleTiming;
272+
270273
/// Create a new instance of [AnalysisDriver].
271274
///
272275
/// The given [SourceFactory] is cloned to ensure that it does not contain a
@@ -293,6 +296,7 @@ class AnalysisDriver {
293296
DeclaredVariables? declaredVariables,
294297
bool retainDataForTesting = false,
295298
this.testView,
299+
bool enableLintRuleTiming = false,
296300
}) : _scheduler = scheduler,
297301
_resourceProvider = resourceProvider,
298302
_byteStore = byteStore,
@@ -306,7 +310,8 @@ class AnalysisDriver {
306310
_sourceFactory = sourceFactory,
307311
_externalSummaries = externalSummaries,
308312
declaredVariables = declaredVariables ?? DeclaredVariables(),
309-
testingData = retainDataForTesting ? TestingData() : null {
313+
testingData = retainDataForTesting ? TestingData() : null,
314+
_enableLintRuleTiming = enableLintRuleTiming {
310315
analysisContext?.driver = this;
311316
testView?.driver = this;
312317

@@ -1400,6 +1405,7 @@ class AnalysisDriver {
14001405
library,
14011406
testingData: testingData,
14021407
typeSystemOperations: typeSystemOperations,
1408+
enableLintRuleTiming: _enableLintRuleTiming,
14031409
).analyze();
14041410

14051411
var isLibraryWithPriorityFile = _isLibraryWithPriorityFile(library);

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,21 @@ class LibraryAnalyzer {
8484
final TestingData? _testingData;
8585
final TypeSystemOperations _typeSystemOperations;
8686

87-
LibraryAnalyzer(this._analysisOptions, this._declaredVariables,
88-
this._libraryElement, this._inheritance, this._library,
89-
{TestingData? testingData,
90-
required TypeSystemOperations typeSystemOperations})
91-
: _testingData = testingData,
92-
_typeSystemOperations = typeSystemOperations {
87+
/// Whether timing data should be gathered during lint rule execution.
88+
final bool _enableLintRuleTiming;
89+
90+
LibraryAnalyzer(
91+
this._analysisOptions,
92+
this._declaredVariables,
93+
this._libraryElement,
94+
this._inheritance,
95+
this._library, {
96+
TestingData? testingData,
97+
required TypeSystemOperations typeSystemOperations,
98+
bool enableLintRuleTiming = false,
99+
}) : _testingData = testingData,
100+
_typeSystemOperations = typeSystemOperations,
101+
_enableLintRuleTiming = enableLintRuleTiming {
93102
_libraryVerificationContext = LibraryVerificationContext(
94103
libraryKind: _library,
95104
constructorFieldsVerifier: ConstructorFieldsVerifier(
@@ -371,8 +380,7 @@ class LibraryAnalyzer {
371380
var allUnits = analysesToContextUnits.values.toList();
372381
definingContextUnit ??= allUnits.first;
373382

374-
var enableTiming = _analysisOptions.enableTiming;
375-
var nodeRegistry = NodeLintRegistry(enableTiming);
383+
var nodeRegistry = NodeLintRegistry(enableTiming: _enableLintRuleTiming);
376384
var context = LinterContextWithResolvedResults(
377385
allUnits,
378386
definingContextUnit,
@@ -383,7 +391,8 @@ class LibraryAnalyzer {
383391
);
384392

385393
for (var linter in _analysisOptions.lintRules) {
386-
var timer = enableTiming ? analysisRuleTimers.getTimer(linter) : null;
394+
var timer =
395+
_enableLintRuleTiming ? analysisRuleTimers.getTimer(linter) : null;
387396
timer?.start();
388397
linter.registerNodeProcessors(nodeRegistry, context);
389398
timer?.stop();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ class AnalysisRuleVisitor implements AstVisitor<void> {
10961096

10971097
/// The container to register visitors for separate AST node types.
10981098
class NodeLintRegistry {
1099-
final bool enableTiming;
1099+
final bool _enableTiming;
11001100
final List<_AfterLibrarySubscription> _afterLibrary = [];
11011101
final List<_Subscription<AdjacentStrings>> _forAdjacentStrings = [];
11021102
final List<_Subscription<Annotation>> _forAnnotation = [];
@@ -1307,7 +1307,7 @@ class NodeLintRegistry {
13071307
final List<_Subscription<WithClause>> _forWithClause = [];
13081308
final List<_Subscription<YieldStatement>> _forYieldStatement = [];
13091309

1310-
NodeLintRegistry(this.enableTiming);
1310+
NodeLintRegistry({required bool enableTiming}) : _enableTiming = enableTiming;
13111311

13121312
void addAdjacentStrings(AnalysisRule rule, AstVisitor visitor) {
13131313
_forAdjacentStrings.add(_Subscription(rule, visitor, _getTimer(rule)));
@@ -2019,7 +2019,7 @@ class NodeLintRegistry {
20192019

20202020
/// Get the timer associated with the given [rule].
20212021
Stopwatch? _getTimer(AnalysisRule rule) {
2022-
if (enableTiming) {
2022+
if (_enableTiming) {
20232023
return analysisRuleTimers.getTimer(rule);
20242024
} else {
20252025
return null;

pkg/analyzer/test/src/dart/analysis/context_builder_test.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,6 @@ class ContextBuilderImplTest with ResourceProviderMixin {
198198
AnalysisOptionsImpl expected,
199199
) {
200200
// TODO(brianwilkerson): Consider moving this to AnalysisOptionsImpl.==.
201-
expect(actual.enableTiming, expected.enableTiming);
202201
expect(actual.lint, expected.lint);
203202
expect(actual.warning, expected.warning);
204203
expect(

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,33 +31,31 @@ class LintDriver {
3131
Future<List<AnalysisErrorInfo>> analyze(Iterable<io.File> files) async {
3232
AnalysisEngine.instance.instrumentationService = _StdInstrumentation();
3333

34+
var filesPaths =
35+
files.map((file) => _absoluteNormalizedPath(file.path)).toList();
36+
3437
var contextCollection = AnalysisContextCollectionImpl(
3538
resourceProvider: _resourceProvider,
3639
sdkPath: options.dartSdkPath,
37-
includedPaths:
38-
files.map((file) => _absoluteNormalizedPath(file.path)).toList(),
40+
includedPaths: filesPaths,
3941
updateAnalysisOptions2: ({
4042
required analysisOptions,
4143
required contextRoot,
4244
required sdk,
4345
}) {
4446
analysisOptions.lint = true;
4547
analysisOptions.warning = false;
46-
analysisOptions.enableTiming = options.enableTiming;
4748
analysisOptions.lintRules =
4849
options.enabledRules.toList(growable: false);
4950
},
51+
enableLintRuleTiming: options.enableTiming,
5052
);
5153

52-
for (io.File file in files) {
53-
var path = _absoluteNormalizedPath(file.path);
54-
_filesAnalyzed.add(path);
55-
}
54+
_filesAnalyzed.addAll(filesPaths);
5655

5756
var result = <AnalysisErrorInfo>[];
5857
for (var path in _filesAnalyzed) {
59-
var analysisContext = contextCollection.contextFor(path);
60-
var analysisSession = analysisContext.currentSession;
58+
var analysisSession = contextCollection.contextFor(path).currentSession;
6159
var errorsResult = await analysisSession.getErrors(path);
6260
if (errorsResult is ErrorsResult) {
6361
result.add(

0 commit comments

Comments
 (0)