Skip to content

Commit bc94050

Browse files
authored
Emit better errors for builds with bad conditional imports (#4393)
Conditional imports are represented as `NamespaceDirective` objects in the analyzer, but we weren't crawling those when generating error messages. Fixes #4297
1 parent 516634a commit bc94050

File tree

4 files changed

+70
-6
lines changed

4 files changed

+70
-6
lines changed

build_modules/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 5.1.9
2+
3+
- Emit build errors for bad conditional imports.
4+
15
## 5.1.8
26

37
- Generate synthetic `package_config.json` in the root of its synthetic test directory. Fixes sources in `web/` and related directories not obeying version constraints.

build_modules/lib/src/errors.dart

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,26 @@ Future<String?> _missingImportMessage(
107107
final import = parsed.directives
108108
.whereType<UriBasedDirective>()
109109
.firstWhereOrNull((directive) {
110-
final uriString = directive.uri.stringValue;
111-
if (uriString == null) return false;
112-
if (uriString.startsWith('dart:')) return false;
113-
final id = AssetId.resolve(Uri.parse(uriString), from: sourceId);
114-
return id == missingId;
110+
final uris = <String?>[
111+
directive.uri.stringValue,
112+
// Conditional imports are represented as NamespaceDirectives.
113+
if (directive is NamespaceDirective)
114+
for (final config in directive.configurations)
115+
config.uri.stringValue,
116+
];
117+
118+
for (final uriString in uris) {
119+
if (uriString == null) continue;
120+
if (uriString.startsWith('dart:')) continue;
121+
122+
try {
123+
final id = AssetId.resolve(Uri.parse(uriString), from: sourceId);
124+
if (id == missingId) return true;
125+
} on FormatException {
126+
// Ignore format exceptions since we're assembling an error string.
127+
}
128+
}
129+
return false;
115130
});
116131
if (import == null) return null;
117132
final lineInfo = parsed.lineInfo.getLocation(import.offset);

build_modules/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: build_modules
2-
version: 5.1.8
2+
version: 5.1.9
33
description: >-
44
Builders to analyze and split Dart code into individually compilable modules
55
based on imports.

build_modules/test/modules_test.dart

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,51 @@ Please check the following imports:
132132
);
133133
});
134134

135+
test('missing conditional imports yield helpful errors', () async {
136+
await testBuilder(
137+
TestBuilder(
138+
buildExtensions: {
139+
'lib/a${moduleExtension(platform)}': ['.transitive'],
140+
},
141+
build: expectAsync2((buildStep, _) async {
142+
await expectLater(
143+
() => rootModule.computeTransitiveDependencies(buildStep),
144+
throwsA(
145+
isA<MissingModulesException>().having(
146+
(e) => e.message,
147+
'message',
148+
contains('''
149+
Unable to find modules for some sources, this is usually the result of either a
150+
bad import, a missing dependency in a package (or possibly a dev_dependency
151+
needs to move to a real dependency), or a build failure (if importing a
152+
generated file).
153+
154+
Please check the following imports:
155+
156+
`import 'some_other_dep.dart' if (dart.library.js_interop) 'src/dep.dart';` from b|lib/b.dart at 1:1
157+
'''),
158+
),
159+
),
160+
);
161+
}),
162+
),
163+
{
164+
'a|lib/a${moduleExtension(platform)}': jsonEncode(
165+
rootModule.toJson(),
166+
),
167+
'a|lib/src/dep${moduleExtension(platform)}': jsonEncode(
168+
directDepModule.toJson(),
169+
),
170+
'b|lib/b${moduleExtension(platform)}': jsonEncode(
171+
transitiveDepModule.toJson(),
172+
),
173+
// No module for b|lib/src/dep.dart
174+
'b|lib/b.dart':
175+
'import \'some_other_dep.dart\' if (dart.library.js_interop) \'src/dep.dart\';',
176+
},
177+
);
178+
});
179+
135180
group('unsupported modules', () {
136181
test('are not allowed as the root', () async {
137182
final unsupportedRootModule = Module(

0 commit comments

Comments
 (0)