Skip to content

Commit

Permalink
Avoid resolving all transitive imports in build
Browse files Browse the repository at this point in the history
  • Loading branch information
simolus3 committed Aug 12, 2023
1 parent f857cb1 commit 788420b
Show file tree
Hide file tree
Showing 23 changed files with 342 additions and 113 deletions.
4 changes: 2 additions & 2 deletions drift_dev/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ builders:
# Regular build flow, emitting a shared part file for source_gen to pick up
drift_dev:
import: "package:drift_dev/integrations/build.dart"
builder_factories: ["analyzer", "driftBuilder"]
builder_factories: ["discover", "analyzer", "driftBuilder"]
build_extensions:
".dart": [".drift.g.part", ".dart.drift_module.json"]
".drift": [".drift.drift_module.json"]
Expand Down Expand Up @@ -54,7 +54,7 @@ builders:
applies_builders: [":analyzer"]
analyzer:
import: "package:drift_dev/integrations/build.dart"
builder_factories: ["analyzer"]
builder_factories: ["discover", "analyzer"]
build_extensions:
".dart": [".dart.drift_module.json"]
".drift": [".drift.drift_module.json"]
Expand Down
2 changes: 2 additions & 0 deletions drift_dev/lib/integrations/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import 'package:drift_dev/src/backends/build/preprocess_builder.dart';

Builder preparingBuilder(BuilderOptions options) => PreprocessBuilder();

Builder discover(BuilderOptions options) => DriftDiscover(options);

Builder analyzer(BuilderOptions options) => DriftAnalyzer(options);

Builder driftBuilder(BuilderOptions options) =>
Expand Down
12 changes: 4 additions & 8 deletions drift_dev/lib/src/analysis/driver/cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import 'state.dart';
class DriftAnalysisCache {
final Map<Uri, CachedSerializationResult> serializationCache = {};
final Map<Uri, FileState> knownFiles = {};
final Map<DriftElementId, DiscoveredElement> discoveredElements = {};
final Map<DriftElementId, DriftElementKind> discoveredElements = {};

FileState stateForUri(Uri uri) {
return knownFiles[uri] ?? notifyFileChanged(uri);
Expand All @@ -26,15 +26,11 @@ class DriftAnalysisCache {

void notifyFileDeleted(Uri uri) {}

void postFileDiscoveryResults(FileState state) {
void knowsLocalElements(FileState state) {
discoveredElements.removeWhere((key, _) => key.libraryUri == state.ownUri);

final discovery = state.discovery;
if (discovery != null) {
discoveredElements.addAll({
for (final definedHere in discovery.locallyDefinedElements)
definedHere.ownId: definedHere,
});
for (final (id, kind) in state.definedElements) {
discoveredElements[id] = kind;
}
}

Expand Down
136 changes: 94 additions & 42 deletions drift_dev/lib/src/analysis/driver/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class DriftAnalysisDriver {
final reader = cacheReader;
if (reader == null) return null;

final found = await reader.readCacheFor(uri);
final found = await reader.readElementCacheFor(uri);
if (found == null) return null;

final parsed = json.decode(found) as Map<String, Object?>;
Expand Down Expand Up @@ -140,7 +140,8 @@ class DriftAnalysisDriver {

final cachedImports = cache.serializationCache[state.ownUri]?.cachedImports;
if (cachedImports != null && state.discovery == null) {
state.cachedImports = cachedImports;
state.cachedDiscovery ??= CachedDiscoveryResults(true, cachedImports, {});

for (final import in cachedImports) {
final found = cache.stateForUri(import);

Expand All @@ -156,51 +157,91 @@ class DriftAnalysisDriver {
return allRecovered;
}

/// Runs the first step (element discovery) on a file with the given [uri].
Future<FileState> prepareFileForAnalysis(
Future<void> discoverIfNecessary(
FileState file, {
bool warnIfFileDoesntExist = true,
}) async {
if (file.discovery == null) {
await DiscoverStep(this, file)
.discover(warnIfFileDoesntExist: warnIfFileDoesntExist);
cache.knowsLocalElements(file);
}
}

/// Runs the first step (discovering local elements) on a file with the given
/// [uri].
Future<FileState> findLocalElements(
Uri uri, {
bool needsDiscovery = true,
bool warnIfFileDoesntExist = true,
}) async {
var known = cache.knownFiles[uri] ?? cache.notifyFileChanged(uri);
final known = cache.knownFiles[uri] ?? cache.notifyFileChanged(uri);

if (known.discovery == null && needsDiscovery) {
await DiscoverStep(this, known)
.discover(warnIfFileDoesntExist: warnIfFileDoesntExist);
cache.postFileDiscoveryResults(known);

// todo: Mark elements that need to be analyzed again

// To analyze a drift file, we also need to be able to analyze imports.
final state = known.discovery;
if (state is DiscoveredDriftFile) {
for (final import in state.imports) {
// todo: We shouldn't unconditionally crawl files like this. The build
// backend should emit prepared file results in a previous step which
// should be used here.
final file = await prepareFileForAnalysis(import.importedUri);

if (file.discovery?.isValidImport != true) {
known.errorsDuringDiscovery.add(
DriftAnalysisError.inDriftFile(
import.ast,
'The imported file, `${import.importedUri}`, does not exist or '
"can't be imported.",
),
);
}
if (known.cachedDiscovery != null || known.discovery != null) {
// We already know local elements.
return known;
}

// First, try to read cached results.
final reader = cacheReader;
CachedDiscoveryResults? cached;

if (reader != null) {
cached = await reader.readDiscovery(uri);

if (cached == null && reader.findsLocalElementsReliably) {
// There are no locally defined elements, since otherwise the reader
// would have found them.
cached = CachedDiscoveryResults(false, const [], const {});
}
}

if (cached != null) {
known.cachedDiscovery = cached;
cache.knowsLocalElements(known);
} else {
await discoverIfNecessary(
known,
warnIfFileDoesntExist: warnIfFileDoesntExist,
);
}

return known;
}

Future<void> _findLocalElementsInAllImports(FileState known) async {
// To analyze references in elements, we also need to know locally defined
// elements in all imports.
final state = known.discovery;
if (state is DiscoveredDriftFile) {
for (final import in state.imports) {
// todo: We shouldn't unconditionally crawl files like this. The build
// backend should emit prepared file results in a previous step which
// should be used here.
final file = await findLocalElements(import.importedUri);

if (file.isValidImport != true) {
known.errorsDuringDiscovery.add(
DriftAnalysisError.inDriftFile(
import.ast,
'The imported file, `${import.importedUri}`, does not exist or '
"can't be imported.",
),
);
} else {
await _findLocalElementsInAllImports(file);
}
} else if (state is DiscoveredDartLibrary) {
for (final import in state.importDependencies) {
}
} else {
for (final import in known.imports ?? const <Uri>[]) {
await findLocalElements(
import,
// We might import a generated file that doesn't exist yet, that
// should not be a user-visible error. Users will notice because the
// import is reported as an error by the analyzer either way.
await prepareFileForAnalysis(import, warnIfFileDoesntExist: false);
}
warnIfFileDoesntExist: false,
);
}
}

return known;
}

/// Runs the second analysis step (element analysis) on a file.
Expand Down Expand Up @@ -232,8 +273,6 @@ class DriftAnalysisDriver {
/// necessary work up until that point.
Future<FileState> resolveElements(Uri uri) async {
var known = cache.stateForUri(uri);
await prepareFileForAnalysis(uri, needsDiscovery: false);

if (known.isFullyAnalyzed) {
// Well, there's nothing to do now.
return known;
Expand All @@ -247,8 +286,10 @@ class DriftAnalysisDriver {
}

// We couldn't recover all analyzed elements. Let's run an analysis run
// now then.
await prepareFileForAnalysis(uri, needsDiscovery: true);
// then.
await discoverIfNecessary(known);
await _findLocalElementsInAllImports(known);

await _analyzePrepared(known);
return known;
}
Expand Down Expand Up @@ -299,8 +340,19 @@ class DriftAnalysisDriver {
/// This class is responsible for recovering both assets in a subsequent build-
/// step.
abstract class AnalysisResultCacheReader {
/// Whether [readDiscovery] only returns `null` when the file under the URI
/// is not relevant to drift.
bool get findsLocalElementsReliably;

/// Whether [readElementCacheFor] is guaranteed to return all elements defined
/// in the supplied `uri`, or whether it could be that we just didn't analyze
/// that file yet.
bool get findsResolvedElementsReliably;

Future<CachedDiscoveryResults?> readDiscovery(Uri uri);

Future<LibraryElement?> readTypeHelperFor(Uri uri);
Future<String?> readCacheFor(Uri uri);
Future<String?> readElementCacheFor(Uri uri);
}

/// Thrown by a local element resolver when an element could not be resolved and
Expand Down
38 changes: 36 additions & 2 deletions drift_dev/lib/src/analysis/driver/state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FileState {
final Uri ownUri;

DiscoveredFileState? discovery;
List<Uri>? cachedImports;
CachedDiscoveryResults? cachedDiscovery;

final List<DriftAnalysisError> errorsDuringDiscovery = [];

Expand All @@ -26,7 +26,12 @@ class FileState {

FileState(this.ownUri);

Iterable<Uri>? get imports => discovery?.importDependencies ?? cachedImports;
bool get isValidImport {
return (cachedDiscovery?.isValidImport ?? discovery?.isValidImport) == true;
}

Iterable<Uri>? get imports =>
discovery?.importDependencies ?? cachedDiscovery?.imports;

String get extension => url.extension(ownUri.path);

Expand All @@ -35,6 +40,22 @@ class FileState {
return analyzedElements.any((e) => e is BaseDriftAccessor);
}

Iterable<(DriftElementId, DriftElementKind)> get definedElements sync* {
final discovery = this.discovery;
final cached = cachedDiscovery;

if (discovery != null) {
for (final element in discovery.locallyDefinedElements) {
yield (element.ownId, element.kind);
}
} else if (cached != null) {
for (final MapEntry(:key, :value)
in cached.locallyDefinedElements.entries) {
yield (id(key), value);
}
}
}

/// All analyzed [DriftElement]s found in this library.
@visibleForTesting
Iterable<DriftElement> get analyzedElements {
Expand Down Expand Up @@ -90,6 +111,18 @@ class FileState {
}
}

class CachedDiscoveryResults {
final bool isValidImport;
final List<Uri> imports;
final Map<String, DriftElementKind> locallyDefinedElements;

CachedDiscoveryResults(
this.isValidImport,
this.imports,
this.locallyDefinedElements,
);
}

abstract class DiscoveredFileState {
final List<DiscoveredElement> locallyDefinedElements;

Expand Down Expand Up @@ -156,6 +189,7 @@ class UnknownFile extends DiscoveredFileState {

abstract class DiscoveredElement {
final DriftElementId ownId;
DriftElementKind get kind;

DiscoveredElement(this.ownId);

Expand Down
6 changes: 3 additions & 3 deletions drift_dev/lib/src/analysis/resolver/dart/accessor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ class DartAccessorResolver
} else {
includes.add(import);

final resolved = await resolver.driver.prepareFileForAnalysis(
discovered.ownId.libraryUri.resolveUri(import));
if (resolved.discovery?.isValidImport != true) {
final resolved = await resolver.driver
.findLocalElements(discovered.ownId.libraryUri.resolveUri(import));
if (!resolved.isValidImport) {
reportError(
DriftAnalysisError.forDartElement(
element, '`$value` could not be imported'),
Expand Down
19 changes: 10 additions & 9 deletions drift_dev/lib/src/analysis/resolver/discover.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,17 @@ class DiscoverStep {

imports.add(DriftFileImport(node, uri));
} else if (node is TableInducingStatement) {
pendingElements
.add(DiscoveredDriftTable(_id(node.createdName), node));
pendingElements.add(DiscoveredDriftTable(
_id(node.createdName), DriftElementKind.table, node));
} else if (node is CreateViewStatement) {
pendingElements
.add(DiscoveredDriftView(_id(node.createdName), node));
pendingElements.add(DiscoveredDriftView(
_id(node.createdName), DriftElementKind.view, node));
} else if (node is CreateIndexStatement) {
pendingElements
.add(DiscoveredDriftIndex(_id(node.indexName), node));
pendingElements.add(DiscoveredDriftIndex(
_id(node.indexName), DriftElementKind.dbIndex, node));
} else if (node is CreateTriggerStatement) {
pendingElements
.add(DiscoveredDriftTrigger(_id(node.triggerName), node));
pendingElements.add(DiscoveredDriftTrigger(
_id(node.triggerName), DriftElementKind.trigger, node));
} else if (node is DeclaredStatement) {
String name;

Expand All @@ -137,7 +137,8 @@ class DiscoverStep {
name = '\$drift_${specialQueryNameCount++}';
}

pendingElements.add(DiscoveredDriftStatement(_id(name), node));
pendingElements.add(DiscoveredDriftStatement(
_id(name), DriftElementKind.definedQuery, node));
}
}

Expand Down
Loading

0 comments on commit 788420b

Please sign in to comment.