Skip to content

Commit

Permalink
Add WorkspacePackage to each FileState.
Browse files Browse the repository at this point in the history
We need this so that we can look if the file is NullSafe by asking
the package that contains it.

Bug: #42594
Change-Id: I11c71faf7bddd53b458a66f0786454ecd7453b5e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153521
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
scheglov authored and commit-bot@chromium.org committed Jul 8, 2020
1 parent 4210268 commit c76fc43
Show file tree
Hide file tree
Showing 14 changed files with 139 additions and 64 deletions.
10 changes: 6 additions & 4 deletions pkg/analyzer/lib/src/context/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ class ContextBuilder {
includedPaths: [contextRoot.root],
excludedPaths: contextRoot.exclude,
);
driver.analysisContext = api.DriverBasedAnalysisContext(
resourceProvider,
apiContextRoots.first,
driver,
driver.configure(
analysisContext: api.DriverBasedAnalysisContext(
resourceProvider,
apiContextRoots.first,
driver,
),
);

// temporary plugin support:
Expand Down
7 changes: 5 additions & 2 deletions pkg/analyzer/lib/src/dart/analysis/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,14 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/// At least one of the optional parameters should be provided, but only those
/// that represent state that has actually changed need be provided.
void configure({
api.AnalysisContext analysisContext,
AnalysisOptions analysisOptions,
Packages packages,
SourceFactory sourceFactory,
}) {
if (analysisContext != null) {
this.analysisContext = analysisContext;
}
if (analysisOptions != null) {
_analysisOptions = analysisOptions;
}
Expand Down Expand Up @@ -1286,7 +1290,6 @@ class AnalysisDriver implements AnalysisDriverGeneric {
libraryContext.elementFactory,
libraryContext.analysisSession.inheritanceManager,
library,
_resourceProvider,
testingData: testingData);
Map<FileState, UnitAnalysisResult> results = analyzer.analyze();

Expand Down Expand Up @@ -1363,7 +1366,6 @@ class AnalysisDriver implements AnalysisDriverGeneric {
libraryContext.elementFactory,
libraryContext.analysisSession.inheritanceManager,
library,
_resourceProvider,
testingData: testingData);
Map<FileState, UnitAnalysisResult> unitResults = analyzer.analyze();
var resolvedUnits = <ResolvedUnitResult>[];
Expand Down Expand Up @@ -1465,6 +1467,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
_resourceProvider,
name,
sourceFactory,
analysisContext?.workspace,
analysisOptions,
declaredVariables,
_saltForUnlinked,
Expand Down
24 changes: 18 additions & 6 deletions pkg/analyzer/lib/src/dart/analysis/file_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import 'package:analyzer/src/summary/format.dart';
import 'package:analyzer/src/summary/idl.dart';
import 'package:analyzer/src/summary/package_bundle_reader.dart';
import 'package:analyzer/src/summary2/informative_data.dart';
import 'package:analyzer/src/workspace/workspace.dart';
import 'package:convert/convert.dart';
import 'package:crypto/crypto.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -91,6 +92,11 @@ class FileState {
/// The [Source] of the file with the [uri].
final Source source;

/// The [WorkspacePackage] that contains this file.
///
/// It might be `null` if the file is outside of the workspace.
final WorkspacePackage workspacePackage;

/// Return `true` if this file is a stub created for a file in the provided
/// external summary store. The values of most properties are not the same
/// as they would be if the file were actually read from the file system.
Expand Down Expand Up @@ -144,6 +150,7 @@ class FileState {
this.path,
this.uri,
this.source,
this.workspacePackage,
this._contextFeatureSet,
this._packageLanguageVersion,
) : isInExternalSummaries = false;
Expand All @@ -152,6 +159,7 @@ class FileState {
: isInExternalSummaries = true,
path = null,
source = null,
workspacePackage = null,
_exists = true,
_contextFeatureSet = null,
_packageLanguageVersion = null {
Expand Down Expand Up @@ -734,6 +742,7 @@ class FileSystemState {
final ByteStore _byteStore;
final FileContentOverlay _contentOverlay;
final SourceFactory _sourceFactory;
final Workspace _workspace;
final AnalysisOptions _analysisOptions;
final DeclaredVariables _declaredVariables;
final Uint32List _saltForUnlinked;
Expand Down Expand Up @@ -793,6 +802,7 @@ class FileSystemState {
this._resourceProvider,
this.contextName,
this._sourceFactory,
this._workspace,
this._analysisOptions,
this._declaredVariables,
this._saltForUnlinked,
Expand All @@ -814,8 +824,8 @@ class FileSystemState {
FileState get unresolvedFile {
if (_unresolvedFile == null) {
var featureSet = FeatureSet.fromEnableFlags([]);
_unresolvedFile = FileState._(
this, null, null, null, featureSet, ExperimentStatus.currentVersion);
_unresolvedFile = FileState._(this, null, null, null, null, featureSet,
ExperimentStatus.currentVersion);
_unresolvedFile.refresh();
}
return _unresolvedFile;
Expand All @@ -841,11 +851,12 @@ class FileSystemState {
}
// Create a new file.
FileSource uriSource = FileSource(resource, uri);
WorkspacePackage workspacePackage = _workspace?.findPackageFor(path);
FeatureSet featureSet = featureSetProvider.getFeatureSet(path, uri);
Version packageLanguageVersion =
featureSetProvider.getLanguageVersion(path, uri);
file = FileState._(
this, path, uri, uriSource, featureSet, packageLanguageVersion);
file = FileState._(this, path, uri, uriSource, workspacePackage,
featureSet, packageLanguageVersion);
_uriToFile[uri] = file;
_addFileWithPath(path, file);
_pathToCanonicalFile[path] = file;
Expand Down Expand Up @@ -884,11 +895,12 @@ class FileSystemState {
String path = uriSource.fullName;
File resource = _resourceProvider.getFile(path);
FileSource source = FileSource(resource, uri);
WorkspacePackage workspacePackage = _workspace?.findPackageFor(path);
FeatureSet featureSet = featureSetProvider.getFeatureSet(path, uri);
Version packageLanguageVersion =
featureSetProvider.getLanguageVersion(path, uri);
file = FileState._(
this, path, uri, source, featureSet, packageLanguageVersion);
file = FileState._(this, path, uri, source, workspacePackage, featureSet,
packageLanguageVersion);
_uriToFile[uri] = file;
_addFileWithPath(path, file);
file.refresh(allowCached: true);
Expand Down
28 changes: 2 additions & 26 deletions pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/src/context/builder.dart';
import 'package:analyzer/src/dart/analysis/file_state.dart';
import 'package:analyzer/src/dart/analysis/testing_data.dart';
import 'package:analyzer/src/dart/ast/ast.dart';
Expand Down Expand Up @@ -50,7 +48,6 @@ import 'package:analyzer/src/lint/linter_visitor.dart';
import 'package:analyzer/src/services/lint.dart';
import 'package:analyzer/src/summary2/linked_element_factory.dart';
import 'package:analyzer/src/task/strong/checker.dart';
import 'package:analyzer/src/workspace/workspace.dart';
import 'package:pub_semver/pub_semver.dart';

var timerLibraryAnalyzer = Stopwatch();
Expand All @@ -70,7 +67,6 @@ class LibraryAnalyzer {
final DeclaredVariables _declaredVariables;
final SourceFactory _sourceFactory;
final FileState _library;
final ResourceProvider _resourceProvider;

final InheritanceManager3 _inheritance;
final bool Function(Uri) _isLibraryUri;
Expand Down Expand Up @@ -100,7 +96,6 @@ class LibraryAnalyzer {
this._elementFactory,
this._inheritance,
this._library,
this._resourceProvider,
{TestingData testingData})
: _testingData = testingData;

Expand Down Expand Up @@ -251,8 +246,8 @@ class LibraryAnalyzer {
declaredVariables: _declaredVariables,
typeSystem: _typeSystem,
inheritanceManager: _inheritance,
resourceProvider: _resourceProvider,
analysisOptions: _context.analysisOptions,
workspacePackage: _library.workspacePackage,
),
);

Expand Down Expand Up @@ -309,8 +304,6 @@ class LibraryAnalyzer {
var nodeRegistry = NodeLintRegistry(_analysisOptions.enableTiming);
var visitors = <AstVisitor>[];

final workspacePackage = _getPackage(currentUnit.unit);

var context = LinterContextImpl(
allUnits,
currentUnit,
Expand All @@ -319,7 +312,7 @@ class LibraryAnalyzer {
_typeSystem,
_inheritance,
_analysisOptions,
workspacePackage,
file.workspacePackage,
);
for (Linter linter in _analysisOptions.lintRules) {
linter.reporter = errorReporter;
Expand Down Expand Up @@ -479,23 +472,6 @@ class LibraryAnalyzer {
});
}

WorkspacePackage _getPackage(CompilationUnit unit) {
final libraryPath = _library.source.fullName;
Workspace workspace =
unit.declaredElement.session?.analysisContext?.workspace;

// If there is no driver setup (as in test environments), we need to create
// a workspace ourselves.
// todo (pq): fix tests or otherwise de-dup this logic shared w/ resolver.
if (workspace == null) {
final builder = ContextBuilder(
_resourceProvider, null /* sdkManager */, null /* contentCache */);
workspace = ContextBuilder.createWorkspace(
_resourceProvider, libraryPath, builder);
}
return workspace?.findPackageFor(libraryPath);
}

/// Return the name of the library that the given part is declared to be a
/// part of, or `null` if the part does not contain a part-of directive.
_NameOrSource _getPartLibraryNameOrUri(Source partSource,
Expand Down
2 changes: 1 addition & 1 deletion pkg/analyzer/lib/src/dart/micro/library_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,8 @@ class LibraryAnalyzer {
declaredVariables: _declaredVariables,
typeSystem: _typeSystem,
inheritanceManager: _inheritance,
resourceProvider: _resourceProvider,
analysisOptions: _context.analysisOptions,
workspacePackage: null, // TODO(scheglov) implement it
),
);

Expand Down
25 changes: 4 additions & 21 deletions pkg/analyzer/lib/src/error/best_practices_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/type_provider.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/src/context/builder.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
import 'package:analyzer/src/dart/element/type.dart';
Expand Down Expand Up @@ -65,7 +63,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
final _InvalidAccessVerifier _invalidAccessVerifier;

/// The [WorkspacePackage] in which [_currentLibrary] is declared.
WorkspacePackage _workspacePackage;
final WorkspacePackage _workspacePackage;

/// The [LinterContext] used for possible const calculations.
LinterContext _linterContext;
Expand All @@ -87,20 +85,19 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
String content, {
@required TypeSystemImpl typeSystem,
@required InheritanceManager3 inheritanceManager,
@required ResourceProvider resourceProvider,
@required DeclaredVariables declaredVariables,
@required AnalysisOptions analysisOptions,
@required WorkspacePackage workspacePackage,
}) : _nullType = typeProvider.nullType,
_typeSystem = typeSystem,
_isNonNullableByDefault = typeSystem.isNonNullableByDefault,
_strictInference =
(analysisOptions as AnalysisOptionsImpl).strictInference,
_inheritanceManager = inheritanceManager,
_invalidAccessVerifier =
_InvalidAccessVerifier(_errorReporter, _currentLibrary) {
_InvalidAccessVerifier(_errorReporter, _currentLibrary),
_workspacePackage = workspacePackage {
_inDeprecatedMember = _currentLibrary.hasDeprecated;
String libraryPath = _currentLibrary.source.fullName;
_workspacePackage = _getPackage(libraryPath, resourceProvider);

_linterContext = LinterContextImpl(
null /* allUnits */,
Expand Down Expand Up @@ -1356,20 +1353,6 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
}
}

WorkspacePackage _getPackage(String root, ResourceProvider resourceProvider) {
Workspace workspace = _currentLibrary.session?.analysisContext?.workspace;
// If there is no driver setup (as in test environments), we need to create
// a workspace ourselves.
// todo (pq): fix tests or otherwise de-dup this logic shared w/ library_analyzer.
if (workspace == null) {
final builder = ContextBuilder(
resourceProvider, null /* sdkManager */, null /* contentCache */);
workspace =
ContextBuilder.createWorkspace(resourceProvider, root, builder);
}
return workspace?.findPackageFor(root);
}

bool _isLibraryInWorkspacePackage(LibraryElement library) {
if (_workspacePackage == null || library == null) {
// Better to not make a big claim that they _are_ in the same package,
Expand Down
20 changes: 16 additions & 4 deletions pkg/analyzer/test/src/dart/analysis/file_state_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer/src/source/package_map_resolver.dart';
import 'package:analyzer/src/test_utilities/mock_sdk.dart';
import 'package:analyzer/src/test_utilities/resource_provider_mixin.dart';
import 'package:analyzer/src/workspace/basic.dart';
import 'package:convert/convert.dart';
import 'package:crypto/crypto.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -50,15 +51,25 @@ class FileSystemStateTest with ResourceProviderMixin {
void setUp() {
logger = PerformanceLog(logBuffer);
sdk = MockSdk(resourceProvider: resourceProvider);

var packageMap = <String, List<Folder>>{
'aaa': [getFolder('/aaa/lib')],
'bbb': [getFolder('/bbb/lib')],
};

var workspace = BasicWorkspace.find(
resourceProvider,
packageMap,
convertPath('/test'),
);

sourceFactory = SourceFactory([
DartUriResolver(sdk),
generatedUriResolver,
PackageMapUriResolver(resourceProvider, <String, List<Folder>>{
'aaa': [getFolder('/aaa/lib')],
'bbb': [getFolder('/bbb/lib')],
}),
PackageMapUriResolver(resourceProvider, packageMap),
ResourceUriResolver(resourceProvider)
]);

AnalysisOptions analysisOptions = AnalysisOptionsImpl();
var featureSetProvider = FeatureSetProvider.build(
sourceFactory: sourceFactory,
Expand All @@ -73,6 +84,7 @@ class FileSystemStateTest with ResourceProviderMixin {
resourceProvider,
'contextName',
sourceFactory,
workspace,
analysisOptions,
DeclaredVariables(),
Uint32List(0),
Expand Down

0 comments on commit c76fc43

Please sign in to comment.