diff --git a/pkg/analysis_server/test/analysis_server_base.dart b/pkg/analysis_server/test/analysis_server_base.dart index 3277bca9e4d..9d7dbd59ea1 100644 --- a/pkg/analysis_server/test/analysis_server_base.dart +++ b/pkg/analysis_server/test/analysis_server_base.dart @@ -30,6 +30,7 @@ import 'test_macros.dart'; // TODO(scheglov): this is duplicate class AnalysisOptionsFileConfig { + final String? include; final List experiments; final List plugins; final List 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:'); diff --git a/pkg/analysis_server/test/domain_analysis_test.dart b/pkg/analysis_server/test/domain_analysis_test.dart index 6d79ee24f39..4e9ca8463a7 100644 --- a/pkg/analysis_server/test/domain_analysis_test.dart +++ b/pkg/analysis_server/test/domain_analysis_test.dart @@ -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> 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 + 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 + 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 + 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 + 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 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 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 diff --git a/pkg/analyzer/lib/src/dart/analysis/context_locator.dart b/pkg/analyzer/lib/src/dart/analysis/context_locator.dart index 7246b254b8a..079c4be7337 100644 --- a/pkg/analyzer/lib/src/dart/analysis/context_locator.dart +++ b/pkg/analyzer/lib/src/dart/analysis/context_locator.dart @@ -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 excludedFolders, ContextRoot containingRoot, + Set containingRootEnabledPlugins, List 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() + .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 excludedFolders, ContextRoot containingRoot, + Set containingRootEnabledPlugins, List 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 _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. diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart index 1413e98ae15..1de17bdb865 100644 --- a/pkg/analyzer/lib/src/dart/analysis/driver.dart +++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart @@ -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 get enabledPluginNames => analysisOptionsMap.entries - .map((e) => e.options.enabledPluginNames) - .flattenedToList; + Set 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 get exceptions => _exceptionController.stream;