Skip to content

Commit

Permalink
[analysis_server] Trigger reanalysis when LSP TODO settings change
Browse files Browse the repository at this point in the history
Previously the setting only applied to future analysis, but this triggers reanalysis immediately.

Change-Id: Ia5687e2bbf5f5a48d9659645c5b15976e712f44f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/263800
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
DanTup authored and Commit Queue committed Oct 12, 2022
1 parent 10e7e47 commit 20af356
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
17 changes: 15 additions & 2 deletions pkg/analysis_server/lib/src/lsp/client_configuration.dart
Expand Up @@ -38,11 +38,24 @@ class LspClientConfiguration {
/// Returns the global configuration for the whole workspace.
LspGlobalClientConfiguration get global => _globalSettings;

/// Returns whether or not the provided new configuration changes any values
/// that would affect analysis results.
bool affectsAnalysisResults(LspGlobalClientConfiguration otherConfig) {
// Check whether TODO settings have changed.
final oldFlag = _globalSettings.showAllTodos;
final newFlag = otherConfig.showAllTodos;
final oldTypes = _globalSettings.showTodoTypes;
final newTypes = otherConfig.showTodoTypes;
return newFlag != oldFlag ||
!const SetEquality().equals(oldTypes, newTypes);
}

/// Returns whether or not the provided new configuration changes any values
/// that would require analysis roots to be updated.
bool affectsAnalysisRoots(LspGlobalClientConfiguration otherConfig) {
return _globalSettings.analysisExcludedFolders !=
otherConfig.analysisExcludedFolders;
final oldExclusions = _globalSettings.analysisExcludedFolders;
final newExclusions = otherConfig.analysisExcludedFolders;
return !const ListEquality().equals(oldExclusions, newExclusions);
}

/// Returns config for a given resource.
Expand Down
5 changes: 5 additions & 0 deletions pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
Expand Up @@ -272,6 +272,11 @@ class LspAnalysisServer extends AnalysisServer {

if (clientConfiguration.affectsAnalysisRoots(oldGlobalConfig)) {
await _refreshAnalysisRoots();
} else if (clientConfiguration
.affectsAnalysisResults(oldGlobalConfig)) {
// Some settings affect analysis results and require re-analysis
// (such as showTodos).
await reanalyze();
}
}
}
Expand Down
22 changes: 17 additions & 5 deletions pkg/analysis_server/test/lsp/diagnostic_test.dart
Expand Up @@ -531,12 +531,24 @@ analyzer:
// Ensure initial analysis completely finished before we continue.
await initialAnalysis;

// Enable showTodos and update the file to ensure TODOs now come through.
final secondDiagnosticsUpdate = waitForDiagnostics(mainFileUri);
// Capture any diagnostic updates. We might get multiple, because during
// a reanalyze, all diagnostics are flushed (to empty) and then analysis
// occurs.
List<Diagnostic>? latestDiagnostics;
notificationsFromServer
.where((notification) =>
notification.method == Method.textDocument_publishDiagnostics)
.map((notification) => PublishDiagnosticsParams.fromJson(
notification.params as Map<String, Object?>))
.where((diagnostics) => diagnostics.uri == mainFileUri)
.listen((diagnostics) {
latestDiagnostics = diagnostics.diagnostics;
});

final nextAnalysis = waitForAnalysisComplete();
await updateConfig({'showTodos': true});
await replaceFile(222, mainFileUri, contents);
final updatedDiagnostics = await secondDiagnosticsUpdate;
expect(updatedDiagnostics, hasLength(1));
await nextAnalysis;
expect(latestDiagnostics, hasLength(1));
}

Future<void> test_todos_specific() async {
Expand Down

0 comments on commit 20af356

Please sign in to comment.