Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Another implicit dependency fix #1352

Merged
merged 2 commits into from Jun 11, 2021
Merged

Another implicit dependency fix #1352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5bc0904
Another implicit dependency fix
jathak Jun 11, 2021
1601f8a
Code review
jathak Jun 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
@@ -1,7 +1,7 @@
## 1.34.2

* Fix a bug that could prevent some members from being found in certain files
that use a mix of imports and the module system.
* Fix a couple bugs that could prevent some members from being found in certain
files that use a mix of imports and the module system.

## 1.34.1

Expand Down
115 changes: 46 additions & 69 deletions lib/src/async_environment.dart
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@

import 'dart:collection';

import 'package:collection/collection.dart';
import 'package:path/path.dart' as p;
import 'package:source_span/source_span.dart';

Expand Down Expand Up @@ -43,23 +44,21 @@ class AsyncEnvironment {
/// modules were originally loaded.
final Map<String, AstNode> _namespaceNodes;

/// The namespaceless modules used in the current scope.
final Set<Module> _globalModules;

/// A map from modules in [_globalModules] to the nodes whose spans
/// indicate where those modules were originally loaded.
final Map<Module, AstNode> _globalModuleNodes;

/// The modules forwarded by this module.
/// A map from namespaceless modules to the `@use` rules whose spans indicate
/// where those modules were originally loaded.
///
/// This is `null` if there are no forwarded modules.
Set<Module>? _forwardedModules;
/// This does not include modules that were imported into the current scope.
final Map<Module, AstNode> _globalModules;

/// A map from modules in [_forwardedModules] to the nodes whose spans
/// A map from modules that were imported into the current scope to the nodes
/// whose spans indicate where those modules were originally loaded.
final Map<Module, AstNode> _importedModules;

/// A map from modules forwarded by this module to the nodes whose spans
/// indicate where those modules were originally forwarded.
///
/// This is `null` if there are no forwarded modules.
Map<Module, AstNode>? _forwardedModuleNodes;
Map<Module, AstNode>? _forwardedModules;

/// Modules forwarded by nested imports at each lexical scope level *beneath
/// the global scope*.
Expand Down Expand Up @@ -152,9 +151,8 @@ class AsyncEnvironment {
: _modules = {},
_namespaceNodes = {},
_globalModules = {},
_globalModuleNodes = {},
_importedModules = {},
_forwardedModules = null,
_forwardedModuleNodes = null,
_nestedForwardedModules = null,
_allModules = [],
_variables = [{}],
Expand All @@ -169,9 +167,8 @@ class AsyncEnvironment {
this._modules,
this._namespaceNodes,
this._globalModules,
this._globalModuleNodes,
this._importedModules,
this._forwardedModules,
this._forwardedModuleNodes,
this._nestedForwardedModules,
this._allModules,
this._variables,
Expand All @@ -196,9 +193,8 @@ class AsyncEnvironment {
_modules,
_namespaceNodes,
_globalModules,
_globalModuleNodes,
_importedModules,
_forwardedModules,
_forwardedModuleNodes,
_nestedForwardedModules,
_allModules,
_variables.toList(),
Expand All @@ -215,15 +211,8 @@ class AsyncEnvironment {
AsyncEnvironment forImport() => AsyncEnvironment._(
{},
{},
{
for (var module in _globalModules)
if (module is ForwardedModuleView) module
},
{
for (var entry in _globalModuleNodes.entries)
if (entry.key is ForwardedModuleView) entry.key: entry.value
},
null,
{},
_importedModules,
null,
null,
[],
Expand All @@ -245,8 +234,7 @@ class AsyncEnvironment {
/// with the same name as a variable defined in this environment.
void addModule(Module module, AstNode nodeWithSpan, {String? namespace}) {
if (namespace == null) {
_globalModules.add(module);
_globalModuleNodes[module] = nodeWithSpan;
_globalModules[module] = nodeWithSpan;
_allModules.add(module);

for (var name in _variables.first.keys) {
Expand Down Expand Up @@ -275,10 +263,9 @@ class AsyncEnvironment {
/// defined in this module, according to the modifications defined by [rule].
void forwardModule(Module module, ForwardRule rule) {
var forwardedModules = (_forwardedModules ??= {});
var forwardedModuleNodes = (_forwardedModuleNodes ??= {});

var view = ForwardedModuleView.ifNecessary(module, rule);
for (var other in forwardedModules) {
for (var other in forwardedModules.keys) {
_assertNoConflicts(
view.variables, other.variables, view, other, "variable");
_assertNoConflicts(
Expand All @@ -291,8 +278,7 @@ class AsyncEnvironment {
// `==`. This is safe because upstream modules are only used for collating
// CSS, not for the members they expose.
_allModules.add(module);
forwardedModules.add(view);
forwardedModuleNodes[view] = rule;
forwardedModules[view] = rule;
}

/// Throws a [SassScriptException] if [newMembers] from [newModule] has any
Expand Down Expand Up @@ -324,7 +310,7 @@ class AsyncEnvironment {
}

if (type == "variable") name = "\$$name";
var span = _forwardedModuleNodes?[oldModule]?.span;
var span = _forwardedModules?[oldModule]?.span;
throw MultiSpanSassScriptException(
'Two forwarded modules both define a $type named $name.',
"new @forward",
Expand All @@ -346,69 +332,56 @@ class AsyncEnvironment {
var forwardedModules = _forwardedModules;
if (forwardedModules != null) {
forwarded = {
for (var module in forwarded)
if (!forwardedModules.contains(module) ||
!_globalModules.contains(module))
module
for (var entry in forwarded.entries)
if (!forwardedModules.containsKey(entry.key) ||
!_globalModules.containsKey(entry.key))
entry.key: entry.value,
};
} else {
forwardedModules = _forwardedModules ??= {};
}

var forwardedModuleNodes = _forwardedModuleNodes ??= {};

var forwardedVariableNames =
forwarded.expand((module) => module.variables.keys).toSet();
forwarded.keys.expand((module) => module.variables.keys).toSet();
var forwardedFunctionNames =
forwarded.expand((module) => module.functions.keys).toSet();
forwarded.keys.expand((module) => module.functions.keys).toSet();
var forwardedMixinNames =
forwarded.expand((module) => module.mixins.keys).toSet();
forwarded.keys.expand((module) => module.mixins.keys).toSet();

if (atRoot) {
// Hide members from modules that have already been imported or
// forwarded that would otherwise conflict with the @imported members.
for (var module in _globalModules.toList()) {
for (var entry in _importedModules.entries.toList()) {
var module = entry.key;
var shadowed = ShadowedModuleView.ifNecessary(module,
variables: forwardedVariableNames,
mixins: forwardedMixinNames,
functions: forwardedFunctionNames);
if (shadowed != null) {
_globalModules.remove(module);

if (!shadowed.isEmpty) {
_globalModules.add(shadowed);
_globalModuleNodes[shadowed] = _globalModuleNodes.remove(module)!;
}
_importedModules.remove(module);
if (!shadowed.isEmpty) _importedModules[shadowed] = entry.value;
}
}

for (var module in forwardedModules.toList()) {
for (var entry in forwardedModules.entries.toList()) {
var module = entry.key;
var shadowed = ShadowedModuleView.ifNecessary(module,
variables: forwardedVariableNames,
mixins: forwardedMixinNames,
functions: forwardedFunctionNames);
if (shadowed != null) {
forwardedModules.remove(module);

if (!shadowed.isEmpty) {
forwardedModules.add(shadowed);
forwardedModuleNodes[shadowed] =
forwardedModuleNodes.remove(module)!;
}
if (!shadowed.isEmpty) forwardedModules[shadowed] = entry.value;
}
}

_globalModules.addAll(forwarded);
_globalModuleNodes
.addAll(module._environment._forwardedModuleNodes ?? const {});
_importedModules.addAll(forwarded);
forwardedModules.addAll(forwarded);
forwardedModuleNodes
.addAll(module._environment._forwardedModuleNodes ?? const {});
} else {
(_nestedForwardedModules ??=
List.generate(_variables.length - 1, (_) => []))
.last
.addAll(forwarded);
.addAll(forwarded.keys);
}

// Remove existing member definitions that are now shadowed by the
Expand Down Expand Up @@ -514,7 +487,7 @@ class AsyncEnvironment {
AstNode? _getVariableNodeFromGlobalModule(String name) {
// We don't need to worry about multiple modules defining the same variable,
// because that's already been checked by [getVariable].
for (var module in _globalModules) {
for (var module in _importedModules.keys.followedBy(_globalModules.keys)) {
var value = module.variableNodes[name];
if (value != null) return value;
}
Expand Down Expand Up @@ -837,7 +810,7 @@ class AsyncEnvironment {
Module toModule(CssStylesheet css, ExtensionStore extensionStore) {
assert(atRoot);
return _EnvironmentModule(this, css, extensionStore,
forwarded: _forwardedModules);
forwarded: _forwardedModules.andThen((modules) => MapKeySet(modules)));
}

/// Returns a module with the same members and upstream modules as [this], but
Expand All @@ -852,7 +825,7 @@ class AsyncEnvironment {
CssStylesheet(const [],
SourceFile.decoded(const [], url: "<dummy module>").span(0)),
ExtensionStore.empty,
forwarded: _forwardedModules);
forwarded: _forwardedModules.andThen((modules) => MapKeySet(modules)));
}

/// Returns the module with the given [namespace], or throws a
Expand All @@ -866,7 +839,7 @@ class AsyncEnvironment {
}

/// Returns the result of [callback] if it returns non-`null` for exactly one
/// module in [_globalModules] *or* for any module in
/// module in [_globalModules] *or* for any module in [_importedModules] or
/// [_nestedForwardedModules].
///
/// Returns `null` if [callback] returns `null` for all modules. Throws an
Expand All @@ -886,10 +859,14 @@ class AsyncEnvironment {
}
}
}
for (var module in _importedModules.keys) {
var value = callback(module);
if (value != null) return value;
}

T? value;
Object? identity;
for (var module in _globalModules) {
for (var module in _globalModules.keys) {
var valueInModule = callback(module);
if (valueInModule == null) continue;

Expand All @@ -899,7 +876,7 @@ class AsyncEnvironment {
if (identityFromModule == identity) continue;

if (value != null) {
var spans = _globalModuleNodes.entries.map(
var spans = _globalModules.entries.map(
(entry) => callback(entry.key).andThen((_) => entry.value.span));

throw MultiSpanSassScriptException(
Expand Down