dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[analysis_server] Create context roots where plugins are enabled

Browse Source 
Fixes https://github.com/dart-lang/sdk/issues/56475

Change-Id: I3e8da83a6fea07482faba4d86aa3db6c6ce8df4e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381481
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Danny Tuppeny
2024-08-21 16:21:30 +00:00
committed by Commit Queue
parent f1a0c38af5
commit c0c697bebe
4 changed files with 321 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analysis_server/test/analysis_server_base.dart
+6
View File
@@ -30,6 +30,7 @@ import 'test_macros.dart';
// TODO(scheglov): this is duplicate
class AnalysisOptionsFileConfig {
final String? include;
final List<String> experiments;
final List<String> plugins;
final List<String> lints;
@@ -38,6 +39,7 @@ class AnalysisOptionsFileConfig {
final bool strictRawTypes;
AnalysisOptionsFileConfig({
this.include,
this.experiments = const [],
this.plugins = const [],
this.lints = const [],
@@ -49,6 +51,10 @@ class AnalysisOptionsFileConfig {
String toContent() {
var buffer = StringBuffer();
var include = this.include;
if (include != null) {
buffer.writeln('include: $include');
}
buffer.writeln('analyzer:');
if (experiments.isNotEmpty) {
buffer.writeln(' enable-experiment:');
pkg/analysis_server/test/domain_analysis_test.dart
+234 -48
View File
@@ -2099,6 +2099,228 @@ AnalysisErrors
@reflectiveTest
class SetAnalysisRootsTest extends PubPackageAnalysisServerTest {
/// Verifies the set of context roots created by the server and which were
/// provided to each plugin.
void expectPluginMapping(Map<String, List<String>> expected) {
var pluginMapping = pluginManager.contextRootPlugins.map(
(root, plugins) => MapEntry(
pathContext.basename(root.root.path),
plugins.map((pluginPath) => pathContext
.basename(pathContext.dirname(pathContext.dirname(pluginPath)))),
),
);
// Additionally, add any context roots from the server that were not
// provided to plugins so the tests can also explicitly verify roots were
// created that didn't show up in the plugins list.
for (var serverContext in server.contextManager.analysisContexts) {
var name = pathContext.basename(serverContext.contextRoot.root.path);
pluginMapping.putIfAbsent(name, () => []);
}
expect(pluginMapping, expected);
}
@override
void setUp() {
super.setUp();
// These tests don't use the "test" package folder but have their own named
// package folders. Delete the "test" folder so it doesn't show up as a
// context root (else it would need listing in each test expectation).
deleteFolder(testPackageRootPath);
}
/// Tests a package with a nested folder that has an additional
/// `analysis_options.yaml` that does not enable the plugin. An additional
/// context should be created for the non-plugin folder and it should be
/// excluded from the parents plugin root.
Future<void>
test_sentToPlugins_inNestedPackages_withNestedAnalysisOptions_enabledPlugin_disabledPlugin() async {
if (!AnalysisServer.supportsPlugins) return;
var plugin1 = (name: 'plugin1', path: _createPlugin('plugin1'));
// package1 has plugin2 enabled.
_createTestPackage(
'package1',
withPackageConfig: false,
plugins: [plugin1],
);
// nestedFolder1 has no plugins enabled.
newAnalysisOptionsYamlFile(
join(workspaceRootPath, 'package1', 'nestedFolder1'),
AnalysisOptionsFileConfig(experiments: experiments).toContent(),
);
// Write the single package config at the root that can resolve both
// plugins.
newPackageConfigJsonFileFromBuilder(
workspaceRootPath,
PackageConfigFileBuilder()
..add(name: 'plugin1', rootPath: plugin1.path));
// Set the analysis roots to the folder ('/home') that contains both
// packages but not the plugins (which are in '/plugins').
await setRoots(
included: [workspaceRootPath],
excluded: [],
);
await waitForTasksFinished();
expectPluginMapping({
'home': [],
'package1': ['plugin1'],
'nestedFolder1': [],
});
}
/// Tests a package with a nested folder that has an additional
/// `analysis_options.yaml` that enables a different plugin. An additional
/// root should be created.
Future<void>
test_sentToPlugins_inNestedPackages_withNestedAnalysisOptions_enabledPlugin_enabledDifferentPlugin() async {
if (!AnalysisServer.supportsPlugins) return;
var plugin1 = (name: 'plugin1', path: _createPlugin('plugin1'));
var plugin2 = (name: 'plugin2', path: _createPlugin('plugin2'));
// package1 has plugin2 enabled.
_createTestPackage(
'package1',
withPackageConfig: false,
plugins: [plugin1],
);
// nestedFolder1 has plugin2 enabled.
newAnalysisOptionsYamlFile(
join(workspaceRootPath, 'package1', 'nestedFolder1'),
AnalysisOptionsFileConfig(
experiments: experiments,
plugins: [plugin2.name],
).toContent(),
);
// Write the single package config at the root that can resolve both
// plugins.
newPackageConfigJsonFileFromBuilder(
workspaceRootPath,
PackageConfigFileBuilder()
..add(name: 'plugin1', rootPath: plugin1.path)
..add(name: 'plugin2', rootPath: plugin2.path),
);
// Set the analysis roots to the folder ('/home') that contains both
// packages but not the plugins (which are in '/plugins').
await setRoots(
included: [workspaceRootPath],
excluded: [],
);
await waitForTasksFinished();
expectPluginMapping({
'home': [],
'package1': ['plugin1'],
'nestedFolder1': ['plugin2'],
});
}
/// Tests a package with a nested folder that has an additional
/// `analysis_options.yaml` that enables the same plugin explicitly. No
/// additional context needs to be created.
Future<void>
test_sentToPlugins_inNestedPackages_withNestedAnalysisOptions_enabledPlugin_enabledPluginExplicit() async {
if (!AnalysisServer.supportsPlugins) return;
var plugin1 = (name: 'plugin1', path: _createPlugin('plugin1'));
// package1 has plugin2 enabled.
_createTestPackage(
'package1',
withPackageConfig: false,
plugins: [plugin1],
);
// nestedFolder1 also has plugin1 enabled.
newAnalysisOptionsYamlFile(
join(workspaceRootPath, 'package1', 'nestedFolder1'),
AnalysisOptionsFileConfig(
experiments: experiments,
plugins: [plugin1.name],
).toContent(),
);
// Write the single package config at the root that can resolve both
// plugins.
newPackageConfigJsonFileFromBuilder(
workspaceRootPath,
PackageConfigFileBuilder()
..add(name: 'plugin1', rootPath: plugin1.path));
// Set the analysis roots to the folder ('/home') that contains both
// packages but not the plugins (which are in '/plugins').
await setRoots(
included: [workspaceRootPath],
excluded: [],
);
await waitForTasksFinished();
expectPluginMapping({
'home': [],
'package1': ['plugin1'],
// nestedFolder1 is included as part of package1.
});
}
/// Tests a package with a nested folder that has an additional
/// `analysis_options.yaml` that enables the same plugin by including the
/// parents `analysis_options.yaml`. No additional context needs to be
/// created.
Future<void>
test_sentToPlugins_inNestedPackages_withNestedAnalysisOptions_enabledPlugin_enabledPluginInclude() async {
if (!AnalysisServer.supportsPlugins) return;
var plugin1 = (name: 'plugin1', path: _createPlugin('plugin1'));
// package1 has plugin2 enabled.
_createTestPackage(
'package1',
withPackageConfig: false,
plugins: [plugin1],
);
// nestedFolder1 also has plugin1 enabled because it includes the parent
// `analysis_options.yaml`.
newAnalysisOptionsYamlFile(
join(workspaceRootPath, 'package1', 'nestedFolder1'),
AnalysisOptionsFileConfig(
include: '../analysis_options.yaml',
).toContent(),
);
// Write the single package config at the root that can resolve both
// plugins.
newPackageConfigJsonFileFromBuilder(
workspaceRootPath,
PackageConfigFileBuilder()
..add(name: 'plugin1', rootPath: plugin1.path));
// Set the analysis roots to the folder ('/home') that contains both
// packages but not the plugins (which are in '/plugins').
await setRoots(
included: [workspaceRootPath],
excluded: [],
);
await waitForTasksFinished();
expectPluginMapping({
'home': [],
'package1': ['plugin1'],
// nestedFolder1 is included as part of package1.
});
}
/// Test that the correct context roots are passed to plugins when they are
/// enabled only for projects nested within the workspace (which do not have
/// their own package configs but use the one from the root).
@@ -2109,14 +2331,6 @@ class SetAnalysisRootsTest extends PubPackageAnalysisServerTest {
/// - package1/ (plugin1, (plugin2 - disabled due to limit))
/// - package2/ (plugin2, (plugin1 - disabled due to limit))
/// - package3/ (plugin1)
@FailingTest(
issue: 'https://github.com/dart-lang/sdk/issues/56475',
reason: 'Test passes in 3.5 if the `singleOptionContexts` flag is set back '
'to=true but as `false` (as shipped), we: '
' a) enable plugins from previous sibling due to ContextBuilder reuse'
' b) read child analysis_options so the root gets plugins'
' c) do not create context roots for nested projects so do not provide those roots to plugins',
)
Future<void>
test_sentToPlugins_inNestedPackages_withoutPackageConfigs() async {
if (!AnalysisServer.supportsPlugins) return;
@@ -2158,22 +2372,12 @@ class SetAnalysisRootsTest extends PubPackageAnalysisServerTest {
);
await waitForTasksFinished();
// Verify the assignment of plugins to roots.
var pluginMapping = pluginManager.contextRootPlugins.map(
(root, plugins) => MapEntry(
pathContext.basename(root.root.path),
plugins.map((pluginPath) => pathContext
.basename(pathContext.dirname(pathContext.dirname(pluginPath)))),
),
);
expect(
pluginMapping,
{
'package1': ['plugin1'],
'package2': ['plugin2'],
'package3': ['plugin1'],
},
);
expectPluginMapping({
'home': [],
'package1': ['plugin1'],
'package2': ['plugin2'],
'package3': ['plugin1'],
});
}
/// Test that the correct context roots are passed to plugins when they are
@@ -2186,14 +2390,6 @@ class SetAnalysisRootsTest extends PubPackageAnalysisServerTest {
/// - package1/ (plugin1, (plugin2 - disabled due to limit))
/// - package2/ (plugin2, (plugin1 - disabled due to limit))
/// - package3/ (plugin1)
@FailingTest(
issue: 'https://github.com/dart-lang/sdk/issues/56475',
reason: 'Test passes in 3.5 if the `singleOptionContexts` flag is set back '
'to=true but as `false` (as shipped), we: '
' a) enable plugins from previous sibling due to ContextBuilder reuse'
' b) read child analysis_options so the root gets plugins'
' c) include duplicate plugins as a result of (b)',
)
Future<void> test_sentToPlugins_inNestedPackages_withPackageConfigs() async {
if (!AnalysisServer.supportsPlugins) return;
@@ -2223,22 +2419,12 @@ class SetAnalysisRootsTest extends PubPackageAnalysisServerTest {
);
await waitForTasksFinished();
// Verify the assignment of plugins to roots.
var pluginMapping = pluginManager.contextRootPlugins.map(
(root, plugins) => MapEntry(
pathContext.basename(root.root.path),
plugins.map((pluginPath) => pathContext
.basename(pathContext.dirname(pathContext.dirname(pluginPath)))),
),
);
expect(
pluginMapping,
{
'package1': ['plugin1'],
'package2': ['plugin2'],
'package3': ['plugin1'],
},
);
expectPluginMapping({
'home': [],
'package1': ['plugin1'],
'package2': ['plugin2'],
'package3': ['plugin1'],
});
}
/// Creates a plugin package named [name] and returns the path to the root
pkg/analyzer/lib/src/dart/analysis/context_locator.dart
+71 -8
View File
@@ -10,8 +10,10 @@ import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/file_system/physical_file_system.dart'
show PhysicalResourceProvider;
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
import 'package:analyzer/src/analysis_options/apply_options.dart';
import 'package:analyzer/src/context/packages.dart';
import 'package:analyzer/src/dart/analysis/context_root.dart';
import 'package:analyzer/src/generated/engine.dart';
import 'package:analyzer/src/task/options.dart';
import 'package:analyzer/src/util/file_paths.dart' as file_paths;
import 'package:analyzer/src/util/yaml.dart';
@@ -21,6 +23,7 @@ import 'package:analyzer/src/workspace/blaze.dart';
import 'package:analyzer/src/workspace/gn.dart';
import 'package:analyzer/src/workspace/pub.dart';
import 'package:analyzer/src/workspace/workspace.dart';
import 'package:collection/collection.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart';
import 'package:yaml/yaml.dart';
@@ -121,8 +124,20 @@ class ContextLocatorImpl implements ContextLocator {
root.included.add(folder);
}
_createContextRootsIn(roots, {}, folder, excludedFolders, root,
root.excludedGlobs, defaultOptionsFile, defaultPackagesFile);
var rootEnabledPlugins =
_getEnabledPlugins(location.workspace, location.optionsFile);
_createContextRootsIn(
roots,
{},
folder,
excludedFolders,
root,
rootEnabledPlugins,
root.excludedGlobs,
defaultOptionsFile,
defaultPackagesFile,
);
}
for (File file in includedFiles) {
@@ -290,6 +305,7 @@ class ContextLocatorImpl implements ContextLocator {
Folder folder,
List<Folder> excludedFolders,
ContextRoot containingRoot,
Set<String> containingRootEnabledPlugins,
List<LocatedGlob> excludedGlobs,
File? optionsFile,
File? packagesFile) {
@@ -307,11 +323,16 @@ class ContextLocatorImpl implements ContextLocator {
}
var buildGnFile = folder.getExistingFile(file_paths.buildGn);
var localEnabledPlugins =
_getEnabledPlugins(containingRoot.workspace, localOptionsFile);
var pluginsDiffer = !const SetEquality<String>()
.equals(containingRootEnabledPlugins, localEnabledPlugins);
//
// Create a context root for the given [folder] if a packages or build file
// is locally specified.
// is locally specified, or the set of enabled plugins changed.
//
if (localPackagesFile != null || buildGnFile != null) {
if (pluginsDiffer || localPackagesFile != null || buildGnFile != null) {
if (optionsFile != null) {
localOptionsFile = optionsFile;
}
@@ -331,6 +352,7 @@ class ContextLocatorImpl implements ContextLocator {
containingRoot.excluded.add(folder);
roots.add(root);
containingRoot = root;
containingRootEnabledPlugins = localEnabledPlugins;
excludedGlobs = _getExcludedGlobs(root.optionsFile, workspace);
root.excludedGlobs = excludedGlobs;
}
@@ -343,8 +365,17 @@ class ContextLocatorImpl implements ContextLocator {
_getExcludedGlobs(localOptionsFile, containingRoot.workspace);
containingRoot.excludedGlobs.addAll(excludes);
}
_createContextRootsIn(roots, visited, folder, excludedFolders,
containingRoot, excludedGlobs, optionsFile, packagesFile);
_createContextRootsIn(
roots,
visited,
folder,
excludedFolders,
containingRoot,
containingRootEnabledPlugins,
excludedGlobs,
optionsFile,
packagesFile,
);
}
/// For each directory within the given [folder] that is neither in the list
@@ -359,6 +390,7 @@ class ContextLocatorImpl implements ContextLocator {
Folder folder,
List<Folder> excludedFolders,
ContextRoot containingRoot,
Set<String> containingRootEnabledPlugins,
List<LocatedGlob> excludedGlobs,
File? optionsFile,
File? packagesFile) {
@@ -396,8 +428,17 @@ class ContextLocatorImpl implements ContextLocator {
if (excludedFolders.contains(child)) {
containingRoot.excluded.add(child);
} else if (!isExcluded(child)) {
_createContextRoots(roots, visited, child, excludedFolders,
containingRoot, excludedGlobs, optionsFile, packagesFile);
_createContextRoots(
roots,
visited,
child,
excludedFolders,
containingRoot,
containingRootEnabledPlugins,
excludedGlobs,
optionsFile,
packagesFile,
);
}
}
}
@@ -478,6 +519,28 @@ class ContextLocatorImpl implements ContextLocator {
return null;
}
/// Gets the set of enabled plugins for [optionsFile]m taking into account
/// any includes.
Set<String> _getEnabledPlugins(Workspace workspace, File? optionsFile) {
if (optionsFile == null) {
return const {};
}
try {
var provider =
AnalysisOptionsProvider(workspace.createSourceFactory(null, null));
var options = AnalysisOptionsImpl(file: optionsFile);
var optionsYaml = provider.getOptionsFromFile(optionsFile);
options.applyOptions(optionsYaml);
return options.enabledPluginNames.toSet();
} catch (_) {
// No plugins will be enabled if the file doesn't parse or cannot be read
// for any reason.
return {};
}
}
/// Return a list containing the glob patterns used to exclude files from
/// analysis by the given [optionsFile]. The list will be empty if there is no
/// options file or if there are no exclusion patterns in the options file.
pkg/analyzer/lib/src/dart/analysis/driver.dart
+10 -4
View File
@@ -346,11 +346,17 @@ class AnalysisDriver {
return libraryContext.elementFactory.analysisSession;
}
/// Return a list of the names of all the plugins enabled in analysis options
/// Return a set of the names of all the plugins enabled in analysis options
/// in this driver.
List<String> get enabledPluginNames => analysisOptionsMap.entries
.map((e) => e.options.enabledPluginNames)
.flattenedToList;
Set<String> get enabledPluginNames {
// We currently only support plugins enabled at the very root of a context
// (and we create contexts for any analysis options that changes plugins
// from its parent context).
var rootOptionsFile = analysisContext?.contextRoot.optionsFile;
return rootOptionsFile != null
? getAnalysisOptionsForFile(rootOptionsFile).enabledPluginNames.toSet()
: const {};
}
/// Return the stream that produces [ExceptionResult]s.
Stream<ExceptionResult> get exceptions => _exceptionController.stream;