From 96750cd5cb734b98cf979331a3664587faa6b7e5 Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Thu, 20 Oct 2016 11:03:11 -0700 Subject: [PATCH] Split out options from ContextBuilder R=scheglov@google.com Review URL: https://codereview.chromium.org/2425423009 . --- .../lib/src/analysis_server.dart | 16 +- .../test/context_manager_test.dart | 8 +- pkg/analyzer/lib/src/context/builder.dart | 163 ++++++++++-------- .../test/src/context/builder_test.dart | 27 +-- .../test/stress/for_git_repository.dart | 8 +- .../tool/task_dependency_graph/generate.dart | 12 +- pkg/analyzer_cli/lib/src/driver.dart | 6 +- .../lib/src/analyzer/context.dart | 6 +- 8 files changed, 145 insertions(+), 101 deletions(-) diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart index ae1bb0aa14e..fdef53a210d 100644 --- a/pkg/analysis_server/lib/src/analysis_server.dart +++ b/pkg/analysis_server/lib/src/analysis_server.dart @@ -1645,16 +1645,18 @@ class ServerContextManagerCallbacks extends ContextManagerCallbacks { } } + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); + builderOptions.defaultOptions = options; + builderOptions.defaultPackageFilePath = defaultPackageFilePath; + builderOptions.defaultPackagesDirectoryPath = defaultPackagesDirectoryPath; + if (analysisServer.options.enablePubSummaryManager) { + builderOptions.pubSummaryManager = analysisServer.pubSummaryManager; + } ContextBuilder builder = new ContextBuilder(resourceProvider, - analysisServer.sdkManager, analysisServer.overlayState); - builder.defaultOptions = options; + analysisServer.sdkManager, analysisServer.overlayState, + options: builderOptions); builder.fileResolverProvider = analysisServer.fileResolverProvider; builder.packageResolverProvider = analysisServer.packageResolverProvider; - builder.defaultPackageFilePath = defaultPackageFilePath; - builder.defaultPackagesDirectoryPath = defaultPackagesDirectoryPath; - if (analysisServer.options.enablePubSummaryManager) { - builder.pubSummaryManager = analysisServer.pubSummaryManager; - } return builder; } diff --git a/pkg/analysis_server/test/context_manager_test.dart b/pkg/analysis_server/test/context_manager_test.dart index 46414f6cc3e..888c2ddd52e 100644 --- a/pkg/analysis_server/test/context_manager_test.dart +++ b/pkg/analysis_server/test/context_manager_test.dart @@ -2707,9 +2707,11 @@ class TestContextManagerCallbacks extends ContextManagerCallbacks { @override ContextBuilder createContextBuilder(Folder folder, AnalysisOptions options) { DartSdkManager sdkManager = new DartSdkManager('/', false); - ContextBuilder builder = - new ContextBuilder(resourceProvider, sdkManager, new ContentCache()); - builder.defaultOptions = options; + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); + builderOptions.defaultOptions = options; + ContextBuilder builder = new ContextBuilder( + resourceProvider, sdkManager, new ContentCache(), + options: builderOptions); return builder; } diff --git a/pkg/analyzer/lib/src/context/builder.dart b/pkg/analyzer/lib/src/context/builder.dart index 882094f350c..d0d9b1e85f5 100644 --- a/pkg/analyzer/lib/src/context/builder.dart +++ b/pkg/analyzer/lib/src/context/builder.dart @@ -68,6 +68,11 @@ class ContextBuilder { */ final ContentCache contentCache; + /** + * The options used by the context builder. + */ + final ContextBuilderOptions builderOptions; + /** * The resolver provider used to create a package: URI resolver, or `null` if * the normal (Package Specification DEP) lookup mechanism is to be used. @@ -82,57 +87,13 @@ class ContextBuilder { @deprecated ResolverProvider fileResolverProvider; - /** - * The file path of the .packages file that should be used in place of any - * file found using the normal (Package Specification DEP) lookup mechanism, - * or `null` if the normal lookup mechanism should be used. - */ - String defaultPackageFilePath; - - /** - * The file path of the packages directory that should be used in place of any - * file found using the normal (Package Specification DEP) lookup mechanism, - * or `null` if the normal lookup mechanism should be used. - */ - String defaultPackagesDirectoryPath; - - /** - * The file path of the file containing the summary of the SDK that should be - * used to "analyze" the SDK. This option should only be specified by - * command-line tools such as 'dartanalyzer' or 'ddc'. - */ - String dartSdkSummaryPath; - - /** - * The file path of the analysis options file that should be used in place of - * any file in the root directory or a parent of the root directory, or `null` - * if the normal lookup mechanism should be used. - */ - String defaultAnalysisOptionsFilePath; - - /** - * The default analysis options that should be used unless some or all of them - * are overridden in the analysis options file, or `null` if the default - * defaults should be used. - */ - AnalysisOptions defaultOptions; - - /** - * A table mapping variable names to values for the declared variables, or - * `null` if no additional variables should be declared. - */ - Map declaredVariables; - - /** - * The manager of pub package summaries. - */ - PubSummaryManager pubSummaryManager; - /** * Initialize a newly created builder to be ready to build a context rooted in * the directory with the given [rootDirectoryPath]. */ - ContextBuilder(this.resourceProvider, this.sdkManager, this.contentCache); + ContextBuilder(this.resourceProvider, this.sdkManager, this.contentCache, + {ContextBuilderOptions options}) + : builderOptions = options ?? new ContextBuilderOptions(); /** * Return an analysis context that is configured correctly to analyze code in @@ -158,9 +119,9 @@ class ContextBuilder { * Configure the context to make use of summaries. */ void configureSummaries(InternalAnalysisContext context) { - if (pubSummaryManager != null) { - List linkedBundles = - pubSummaryManager.getLinkedBundles(context); + PubSummaryManager manager = builderOptions.pubSummaryManager; + if (manager != null) { + List linkedBundles = manager.getLinkedBundles(context); if (linkedBundles.isNotEmpty) { SummaryDataStore store = new SummaryDataStore([]); for (LinkedPubPackage package in linkedBundles) { @@ -211,6 +172,7 @@ class ContextBuilder { * Return an analysis options object containing the default option values. */ AnalysisOptions createDefaultOptions() { + AnalysisOptions defaultOptions = builderOptions.defaultOptions; if (defaultOptions == null) { return new AnalysisOptionsImpl(); } @@ -218,14 +180,17 @@ class ContextBuilder { } Packages createPackageMap(String rootDirectoryPath) { - if (defaultPackageFilePath != null) { - File configFile = resourceProvider.getFile(defaultPackageFilePath); + String filePath = builderOptions.defaultPackageFilePath; + if (filePath != null) { + File configFile = resourceProvider.getFile(filePath); List bytes = configFile.readAsBytesSync(); Map map = parse(bytes, configFile.toUri()); resolveSymbolicLinks(map); return new MapPackages(map); - } else if (defaultPackagesDirectoryPath != null) { - Folder folder = resourceProvider.getFolder(defaultPackagesDirectoryPath); + } + String directoryPath = builderOptions.defaultPackagesDirectoryPath; + if (directoryPath != null) { + Folder folder = resourceProvider.getFolder(directoryPath); return getPackagesFromFolder(folder); } return findPackagesFromFile(rootDirectoryPath); @@ -258,9 +223,10 @@ class ContextBuilder { * given [context]. */ void declareVariables(InternalAnalysisContext context) { - if (declaredVariables != null && declaredVariables.isNotEmpty) { + Map variables = builderOptions.declaredVariables; + if (variables != null && variables.isNotEmpty) { DeclaredVariables contextVariables = context.declaredVariables; - declaredVariables.forEach((String variableName, String value) { + variables.forEach((String variableName, String value) { contextVariables.define(variableName, value); }); } @@ -292,12 +258,13 @@ class ContextBuilder { /** * Return the SDK that should be used to analyze code. Use the given - * [packageMap] and [options] to locate the SDK. + * [packageMap] and [analysisOptions] to locate the SDK. */ DartSdk findSdk( - Map> packageMap, AnalysisOptions options) { - if (dartSdkSummaryPath != null) { - return new SummaryBasedDartSdk(dartSdkSummaryPath, options.strongMode); + Map> packageMap, AnalysisOptions analysisOptions) { + String summaryPath = builderOptions.dartSdkSummaryPath; + if (summaryPath != null) { + return new SummaryBasedDartSdk(summaryPath, analysisOptions.strongMode); } else if (packageMap != null) { SdkExtensionFinder extFinder = new SdkExtensionFinder(packageMap); List extFilePaths = extFinder.extensionFilePaths; @@ -317,12 +284,12 @@ class ContextBuilder { .path); } paths.addAll(extFilePaths); - SdkDescription description = new SdkDescription(paths, options); + SdkDescription description = new SdkDescription(paths, analysisOptions); DartSdk dartSdk = sdkManager.getSdk(description, () { if (extFilePaths.isNotEmpty) { embedderSdk.addExtensions(extFinder.urlMappings); } - embedderSdk.analysisOptions = options; + embedderSdk.analysisOptions = analysisOptions; embedderSdk.useSummary = sdkManager.canUseSummaries; return embedderSdk; }); @@ -334,25 +301,26 @@ class ContextBuilder { String sdkPath = sdkManager.defaultSdkDirectory; List paths = [sdkPath]; paths.addAll(extFilePaths); - SdkDescription description = new SdkDescription(paths, options); + SdkDescription description = new SdkDescription(paths, analysisOptions); return sdkManager.getSdk(description, () { FolderBasedDartSdk sdk = new FolderBasedDartSdk( resourceProvider, resourceProvider.getFolder(sdkPath)); if (extFilePaths.isNotEmpty) { sdk.addExtensions(extFinder.urlMappings); } - sdk.analysisOptions = options; + sdk.analysisOptions = analysisOptions; sdk.useSummary = sdkManager.canUseSummaries; return sdk; }); } } String sdkPath = sdkManager.defaultSdkDirectory; - SdkDescription description = new SdkDescription([sdkPath], options); + SdkDescription description = + new SdkDescription([sdkPath], analysisOptions); return sdkManager.getSdk(description, () { FolderBasedDartSdk sdk = new FolderBasedDartSdk(resourceProvider, - resourceProvider.getFolder(sdkPath), options.strongMode); - sdk.analysisOptions = options; + resourceProvider.getFolder(sdkPath), analysisOptions.strongMode); + sdk.analysisOptions = analysisOptions; sdk.useSummary = sdkManager.canUseSummaries; return sdk; }); @@ -386,8 +354,9 @@ class ContextBuilder { * the directory with the given [path]. */ File getOptionsFile(String path) { - if (defaultAnalysisOptionsFilePath != null) { - return resourceProvider.getFile(defaultAnalysisOptionsFilePath); + String filePath = builderOptions.defaultAnalysisOptionsFilePath; + if (filePath != null) { + return resourceProvider.getFile(filePath); } Folder root = resourceProvider.getFolder(path); for (Folder folder = root; folder != null; folder = folder.parent) { @@ -503,6 +472,62 @@ class ContextBuilder { } } +/** + * Options used by a [ContextBuilder]. + */ +class ContextBuilderOptions { + /** + * The file path of the file containing the summary of the SDK that should be + * used to "analyze" the SDK. This option should only be specified by + * command-line tools such as 'dartanalyzer' or 'ddc'. + */ + String dartSdkSummaryPath; + + /** + * The file path of the analysis options file that should be used in place of + * any file in the root directory or a parent of the root directory, or `null` + * if the normal lookup mechanism should be used. + */ + String defaultAnalysisOptionsFilePath; + + /** + * A table mapping variable names to values for the declared variables, or + * `null` if no additional variables should be declared. + */ + Map declaredVariables; + + /** + * The default analysis options that should be used unless some or all of them + * are overridden in the analysis options file, or `null` if the default + * defaults should be used. + */ + AnalysisOptions defaultOptions; + + /** + * The file path of the .packages file that should be used in place of any + * file found using the normal (Package Specification DEP) lookup mechanism, + * or `null` if the normal lookup mechanism should be used. + */ + String defaultPackageFilePath; + + /** + * The file path of the packages directory that should be used in place of any + * file found using the normal (Package Specification DEP) lookup mechanism, + * or `null` if the normal lookup mechanism should be used. + */ + String defaultPackagesDirectoryPath; + + /** + * The manager of pub package summaries. + */ + PubSummaryManager pubSummaryManager; + + /** + * Initialize a newly created set of options + */ + ContextBuilderOptions(); +} + /** * Given a package map, check in each package's lib directory for the existence * of an `_embedder.yaml` file. If the file contains a top level YamlMap, it diff --git a/pkg/analyzer/test/src/context/builder_test.dart b/pkg/analyzer/test/src/context/builder_test.dart index fe5031cca9d..b05daa5240f 100644 --- a/pkg/analyzer/test/src/context/builder_test.dart +++ b/pkg/analyzer/test/src/context/builder_test.dart @@ -54,6 +54,11 @@ class ContextBuilderTest extends EngineTestCase { */ ContentCache contentCache; + /** + * The options passed to the context builder. + */ + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); + /** * The context builder to be used in the test. */ @@ -83,7 +88,8 @@ const Map libraries = const { }; '''); sdkManager = new DartSdkManager(defaultSdkPath, false); - builder = new ContextBuilder(resourceProvider, sdkManager, contentCache); + builder = new ContextBuilder(resourceProvider, sdkManager, contentCache, + options: builderOptions); } void createFile(String path, String content) { @@ -98,7 +104,8 @@ const Map libraries = const { sdkManager = new DartSdkManager(resourceProvider.convertPath('/sdk'), false); contentCache = new ContentCache(); - builder = new ContextBuilder(resourceProvider, sdkManager, contentCache); + builder = new ContextBuilder(resourceProvider, sdkManager, contentCache, + options: builderOptions); } @failingTest @@ -142,7 +149,7 @@ const Map libraries = const { defaultOptions.enableStrictCallChecks = !defaultOptions.enableStrictCallChecks; defaultOptions.enableSuperMixins = !defaultOptions.enableSuperMixins; - builder.defaultOptions = defaultOptions; + builderOptions.defaultOptions = defaultOptions; AnalysisOptions options = builder.createDefaultOptions(); _expectEqualOptions(options, defaultOptions); } @@ -165,7 +172,7 @@ const Map libraries = const { resourceProvider.newFolder(fooPath); resourceProvider.newFolder(barPath); - builder.defaultPackagesDirectoryPath = packageDirPath; + builderOptions.defaultPackagesDirectoryPath = packageDirPath; Packages packages = builder.createPackageMap(projectPath); expect(packages, isNotNull); @@ -209,7 +216,7 @@ foo:$fooUri bar:$barUri '''); - builder.defaultPackageFilePath = packageFilePath; + builderOptions.defaultPackageFilePath = packageFilePath; Packages packages = builder.createPackageMap(projectPath); expect(packages, isNotNull); Map map = packages.asMap(); @@ -410,7 +417,7 @@ b:${pathContext.toUri(packageB)} void test_declareVariables_emptyMap() { AnalysisContext context = AnalysisEngine.instance.createAnalysisContext(); Iterable expected = context.declaredVariables.variableNames; - builder.declaredVariables = {}; + builderOptions.declaredVariables = {}; builder.declareVariables(context); expect(context.declaredVariables.variableNames, unorderedEquals(expected)); @@ -422,7 +429,7 @@ b:${pathContext.toUri(packageB)} expect(expected, isNot(contains('a'))); expect(expected, isNot(contains('b'))); expected.addAll(['a', 'b']); - builder.declaredVariables = {'a': 'a', 'b': 'b'}; + builderOptions.declaredVariables = {'a': 'a', 'b': 'b'}; builder.declareVariables(context); expect(context.declaredVariables.variableNames, unorderedEquals(expected)); @@ -491,7 +498,7 @@ b:${pathContext.toUri(packageB)} void test_getAnalysisOptions_default_noOverrides() { AnalysisOptionsImpl defaultOptions = new AnalysisOptionsImpl(); defaultOptions.enableGenericMethods = true; - builder.defaultOptions = defaultOptions; + builderOptions.defaultOptions = defaultOptions; AnalysisOptionsImpl expected = new AnalysisOptionsImpl(); expected.enableGenericMethods = true; String path = resourceProvider.convertPath('/some/directory/path'); @@ -513,7 +520,7 @@ linter: void test_getAnalysisOptions_default_overrides() { AnalysisOptionsImpl defaultOptions = new AnalysisOptionsImpl(); defaultOptions.enableGenericMethods = true; - builder.defaultOptions = defaultOptions; + builderOptions.defaultOptions = defaultOptions; AnalysisOptionsImpl expected = new AnalysisOptionsImpl(); expected.enableSuperMixins = true; expected.enableGenericMethods = true; @@ -611,7 +618,7 @@ analyzer: String filePath = resourceProvider.convertPath('/options/analysis.yaml'); resourceProvider.newFile(filePath, ''); - builder.defaultAnalysisOptionsFilePath = filePath; + builderOptions.defaultAnalysisOptionsFilePath = filePath; File result = builder.getOptionsFile(path); expect(result, isNotNull); expect(result.path, filePath); diff --git a/pkg/analyzer/test/stress/for_git_repository.dart b/pkg/analyzer/test/stress/for_git_repository.dart index 0fe14f3d9a9..23105816ef2 100644 --- a/pkg/analyzer/test/stress/for_git_repository.dart +++ b/pkg/analyzer/test/stress/for_git_repository.dart @@ -246,9 +246,11 @@ class StressTest { FolderBasedDartSdk.defaultSdkDirectory(resourceProvider); sdkManager = new DartSdkManager(sdkDirectory.path, false); contentCache = new ContentCache(); - ContextBuilder builder = - new ContextBuilder(resourceProvider, sdkManager, contentCache); - builder.defaultOptions = new AnalysisOptionsImpl(); + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); + builderOptions.defaultOptions = new AnalysisOptionsImpl(); + ContextBuilder builder = new ContextBuilder( + resourceProvider, sdkManager, contentCache, + options: builderOptions); expectedContext = builder.buildContext(folderPath); actualContext = builder.buildContext(folderPath); expectedContext.analysisOptions = diff --git a/pkg/analyzer/tool/task_dependency_graph/generate.dart b/pkg/analyzer/tool/task_dependency_graph/generate.dart index 9761e90d067..26745f6fdb8 100644 --- a/pkg/analyzer/tool/task_dependency_graph/generate.dart +++ b/pkg/analyzer/tool/task_dependency_graph/generate.dart @@ -154,17 +154,19 @@ ${generateGraphData()} DartSdk sdk = new FolderBasedDartSdk(resourceProvider, FolderBasedDartSdk.defaultSdkDirectory(resourceProvider)); context = AnalysisEngine.instance.createAnalysisContext(); - ContextBuilder builder = new ContextBuilder(resourceProvider, null, null); + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); if (Platform.packageRoot != null) { - builder.defaultPackagesDirectoryPath = - Uri.parse(Platform.packageRoot).toFilePath(); + builderOptions.defaultPackagesDirectoryPath = + Uri.parse(Platform.packageRoot).toFilePath(); } else if (Platform.packageConfig != null) { - builder.defaultPackageFilePath = - Uri.parse(Platform.packageConfig).toFilePath(); + builderOptions.defaultPackageFilePath = + Uri.parse(Platform.packageConfig).toFilePath(); } else { // Let the context builder use the default algorithm for package // resolution. } + ContextBuilder builder = new ContextBuilder(resourceProvider, null, null, + options: builderOptions); List uriResolvers = [ new DartUriResolver(sdk), new PackageMapUriResolver(resourceProvider, diff --git a/pkg/analyzer_cli/lib/src/driver.dart b/pkg/analyzer_cli/lib/src/driver.dart index d46d3f531c6..aef22111d8c 100644 --- a/pkg/analyzer_cli/lib/src/driver.dart +++ b/pkg/analyzer_cli/lib/src/driver.dart @@ -373,8 +373,10 @@ class Driver implements CommandLineStarter { UriResolver packageUriResolver; if (options.packageRootPath != null) { - ContextBuilder builder = new ContextBuilder(resourceProvider, null, null); - builder.defaultPackagesDirectoryPath = options.packageRootPath; + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); + builderOptions.defaultPackagesDirectoryPath = options.packageRootPath; + ContextBuilder builder = new ContextBuilder(resourceProvider, null, null, + options: builderOptions); packageUriResolver = new PackageMapUriResolver(resourceProvider, builder.convertPackagesToMap(builder.createPackageMap(''))); } else if (options.packageConfigPath == null) { diff --git a/pkg/dev_compiler/lib/src/analyzer/context.dart b/pkg/dev_compiler/lib/src/analyzer/context.dart index bc47716a644..05bacf9c3f7 100644 --- a/pkg/dev_compiler/lib/src/analyzer/context.dart +++ b/pkg/dev_compiler/lib/src/analyzer/context.dart @@ -139,10 +139,12 @@ List createFileResolvers(AnalyzerOptions options, {ResourceProvider resourceProvider}) { resourceProvider ??= PhysicalResourceProvider.INSTANCE; UriResolver packageResolver() { - ContextBuilder builder = new ContextBuilder(resourceProvider, null, null); + ContextBuilderOptions builderOptions = new ContextBuilderOptions(); if (options.packageRoot != null) { - builder.defaultPackagesDirectoryPath = options.packageRoot; + builderOptions.defaultPackagesDirectoryPath = options.packageRoot; } + ContextBuilder builder = new ContextBuilder(resourceProvider, null, null, + options: builderOptions); return new PackageMapUriResolver(resourceProvider, builder.convertPackagesToMap(builder.createPackageMap(''))); }