Skip to content
Snippets Groups Projects
Commit 052756a1 authored by Natalie Weizenbaum's avatar Natalie Weizenbaum
Browse files

Fix an edge case in the dependency computer.

When a package both transformed itself and had a dev transformer (that
is, a transformer that only runs on non-public files), *and* that
package's self-transformed code was imported by another transformer, the
dependency computer could return the dev transformer in its results.

Closes #1290
Closes #1291

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1220223008.
parent 8884f572
No related branches found
No related tags found
No related merge requests found
...@@ -172,9 +172,15 @@ class DependencyComputer { ...@@ -172,9 +172,15 @@ class DependencyComputer {
for (var config in phase) { for (var config in phase) {
var id = config.id; var id = config.id;
if (id.isBuiltInTransformer) continue; if (id.isBuiltInTransformer) continue;
if (packageName != _graph.entrypoint.root.name &&
!config.canTransformPublicFiles) {
continue;
}
if (_loadingPackageComputers.contains(id.package)) { if (_loadingPackageComputers.contains(id.package)) {
throw new CycleException("$packageName is transformed by $id"); throw new CycleException("$packageName is transformed by $id");
} }
results.add(id); results.add(id);
} }
} }
...@@ -264,9 +270,9 @@ class _PackageDependencyComputer { ...@@ -264,9 +270,9 @@ class _PackageDependencyComputer {
// [_transformersNeededByLibraries] while [_applicableTransformers] is // [_transformersNeededByLibraries] while [_applicableTransformers] is
// smaller. // smaller.
for (var phase in _package.pubspec.transformers) { for (var phase in _package.pubspec.transformers) {
for (var config in phase) { _applicableTransformers.addAll(phase.where((config) {
// Ignore non-root transformers on non-public files. // Ignore non-root transformers on non-public files.
if (!isRootPackage && !config.canTransformPublicFiles) continue; if (!isRootPackage && !config.canTransformPublicFiles) return false;
var id = config.id; var id = config.id;
try { try {
...@@ -287,13 +293,14 @@ class _PackageDependencyComputer { ...@@ -287,13 +293,14 @@ class _PackageDependencyComputer {
} on CycleException catch (error) { } on CycleException catch (error) {
throw error.prependStep("$packageName is transformed by $id"); throw error.prependStep("$packageName is transformed by $id");
} }
}
return true;
}));
// Clear the cached imports and exports because the new transformers may // Clear the cached imports and exports because the new transformers may
// start transforming a library whose directives were previously // start transforming a library whose directives were previously
// statically analyzable. // statically analyzable.
_transitiveExternalDirectives.clear(); _transitiveExternalDirectives.clear();
_applicableTransformers.addAll(phase);
} }
} }
......
...@@ -88,4 +88,40 @@ void main() { ...@@ -88,4 +88,40 @@ void main() {
expectDependencies({"foo": []}); expectDependencies({"foo": []});
}); });
// Regression test for #1291
integration("doesn't return a dependency's transformer that can't run on lib "
"when the app's transformer imports the dependency's", () {
d.dir(appPath, [
d.pubspec({
"name": "myapp",
"dependencies": {"foo": {"path": "../foo"}},
"transformers": ["myapp"]
}),
d.dir("lib", [
d.file("myapp.dart", transformer(['package:foo/foo.dart']))
])
]).create();
d.dir("foo", [
d.pubspec({
"name": "foo",
"version": "1.0.0",
"transformers": [
["foo/bar"],
[{"foo": {"\$include": "test/foo_test.dart"}}]
]
}),
d.dir("lib", [
d.file("foo.dart", transformer()),
d.file("bar.dart", transformer())
]),
d.dir("test", [d.file("foo_test.dart", "")])
]).create();
expectDependencies({
'foo/bar': [],
'myapp': ['foo/bar']
});
});
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment