Skip to content

Disallow passing functions/mixins across compilations #2544

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
## 1.88.1-dev
## 1.88.0-dev

* Fix a bug when calculating source spans for interpolations.

## 1.88.0
### Dart and JS APIs

* **Potentially breaking bug fix:** Throw an error when passing a function or
mixin object from one compilation to another.

### Dart API

* Deprecate passing a relative URL to `compileString()` and related functions.

Expand Down
26 changes: 25 additions & 1 deletion lib/src/value/function.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:meta/meta.dart';

import '../callable.dart';
import '../exception.dart';
import '../visitor/interface/value.dart';
import '../value.dart';

Expand All @@ -23,14 +24,37 @@ class SassFunction extends Value {
/// synchronous evaluate visitor will crash if this isn't a [Callable].
final AsyncCallable callable;

SassFunction(this.callable);
/// The unique compile context for tracking if this [SassFunction] belongs to
/// the current compilation or not.
///
/// This is `null` for functions defined in plugins' Dart code.
final Object? _compileContext;

SassFunction(this.callable) : _compileContext = null;

@internal
SassFunction.withCompileContext(this.callable, this._compileContext);

/// @nodoc
@internal
T accept<T>(ValueVisitor<T> visitor) => visitor.visitFunction(this);

SassFunction assertFunction([String? name]) => this;

/// Asserts that this SassFunction belongs to [compileContext] and returns it.
///
/// It's checked before evaluating a SassFunction to prevent execution of
/// SassFunction across different compilations.
@internal
SassFunction assertCompileContext(Object compileContext) {
if (_compileContext != null && _compileContext != compileContext) {
throw SassScriptException(
"$this does not belong to current compilation.");
}

return this;
}

bool operator ==(Object other) =>
other is SassFunction && callable == other.callable;

Expand Down
24 changes: 23 additions & 1 deletion lib/src/value/mixin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:meta/meta.dart';

import '../callable.dart';
import '../exception.dart';
import '../visitor/interface/value.dart';
import '../value.dart';

Expand All @@ -25,14 +26,35 @@ final class SassMixin extends Value {
@internal
final AsyncCallable callable;

SassMixin(this.callable);
/// The unique compile context for tracking if this [SassMixin] belongs to the
/// current compilation or not.
final Object? _compileContext;

SassMixin(this.callable) : _compileContext = null;

@internal
SassMixin.withCompileContext(this.callable, this._compileContext);

/// @nodoc
@internal
T accept<T>(ValueVisitor<T> visitor) => visitor.visitMixin(this);

SassMixin assertMixin([String? name]) => this;

/// Asserts that this SassMixin belongs to [compileContext] and returns it.
///
/// It's checked before evaluating a SassMixin to prevent execution of
/// SassMixin across different compilations.
@internal
SassMixin assertCompileContext(Object compileContext) {
if (_compileContext != null && _compileContext != compileContext) {
throw SassScriptException(
"$this does not belong to current compilation.");
}

return this;
}

bool operator ==(Object other) =>
other is SassMixin && callable == other.callable;

Expand Down
27 changes: 20 additions & 7 deletions lib/src/visitor/async_evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ final class _EvaluateVisitor
/// Whether to track source map information.
final bool _sourceMap;

/// The unique compile context for tracking if [SassFunction]s and
/// [SassMixin]s belongs to the current compilation or not.
final Object _compileContext = Object();

/// The current lexical environment.
AsyncEnvironment _environment;

Expand Down Expand Up @@ -433,7 +437,8 @@ final class _EvaluateVisitor

return SassMap({
for (var (name, value) in module.functions.pairs)
SassString(name): SassFunction(value),
SassString(name):
SassFunction.withCompileContext(value, _compileContext),
});
}, url: "sass:meta"),

Expand All @@ -446,7 +451,8 @@ final class _EvaluateVisitor

return SassMap({
for (var (name, value) in module.mixins.pairs)
SassString(name): SassMixin(value),
SassString(name):
SassMixin.withCompileContext(value, _compileContext),
});
}, url: "sass:meta"),

Expand All @@ -462,7 +468,8 @@ final class _EvaluateVisitor
if (module != null) {
throw r"$css and $module may not both be passed at once.";
}
return SassFunction(PlainCssCallable(name.text));
return SassFunction.withCompileContext(
PlainCssCallable(name.text), _compileContext);
}

var callable = _addExceptionSpan(_callableNode!, () {
Expand All @@ -477,7 +484,7 @@ final class _EvaluateVisitor
});
if (callable == null) throw "Function not found: $name";

return SassFunction(callable);
return SassFunction.withCompileContext(callable, _compileContext);
},
url: "sass:meta",
),
Expand All @@ -497,7 +504,7 @@ final class _EvaluateVisitor
);
if (callable == null) throw "Mixin not found: $name";

return SassMixin(callable);
return SassMixin.withCompileContext(callable, _compileContext);
}, url: "sass:meta"),

AsyncBuiltInCallable.function("call", r"$function, $args...", (
Expand Down Expand Up @@ -541,7 +548,10 @@ final class _EvaluateVisitor
return await expression.accept(this);
}

var callable = function.assertFunction("function").callable;
var callable = function
.assertFunction("function")
.assertCompileContext(_compileContext)
.callable;
// ignore: unnecessary_type_check
if (callable is AsyncCallable) {
return await _runFunctionCallable(
Expand Down Expand Up @@ -608,7 +618,10 @@ final class _EvaluateVisitor
rest: ValueExpression(args, callableNode.span),
);

var callable = mixin.assertMixin("mixin").callable;
var callable = mixin
.assertMixin("mixin")
.assertCompileContext(_compileContext)
.callable;
var content = _environment.content;

// ignore: unnecessary_type_check
Expand Down
29 changes: 21 additions & 8 deletions lib/src/visitor/evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 05d8589b401932198e1f52434066ea4d6cbf3756
// Checksum: 6e5710daa106ed0b9b684af8bc61ce9cc233a10b
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -187,6 +187,10 @@ final class _EvaluateVisitor
/// Whether to track source map information.
final bool _sourceMap;

/// The unique compile context for tracking if [SassFunction]s and
/// [SassMixin]s belongs to the current compilation or not.
final Object _compileContext = Object();

/// The current lexical environment.
Environment _environment;

Expand Down Expand Up @@ -441,7 +445,8 @@ final class _EvaluateVisitor

return SassMap({
for (var (name, value) in module.functions.pairs)
SassString(name): SassFunction(value),
SassString(name):
SassFunction.withCompileContext(value, _compileContext),
});
}, url: "sass:meta"),

Expand All @@ -454,7 +459,8 @@ final class _EvaluateVisitor

return SassMap({
for (var (name, value) in module.mixins.pairs)
SassString(name): SassMixin(value),
SassString(name):
SassMixin.withCompileContext(value, _compileContext),
});
}, url: "sass:meta"),

Expand All @@ -470,7 +476,8 @@ final class _EvaluateVisitor
if (module != null) {
throw r"$css and $module may not both be passed at once.";
}
return SassFunction(PlainCssCallable(name.text));
return SassFunction.withCompileContext(
PlainCssCallable(name.text), _compileContext);
}

var callable = _addExceptionSpan(_callableNode!, () {
Expand All @@ -485,7 +492,7 @@ final class _EvaluateVisitor
});
if (callable == null) throw "Function not found: $name";

return SassFunction(callable);
return SassFunction.withCompileContext(callable, _compileContext);
},
url: "sass:meta",
),
Expand All @@ -505,7 +512,7 @@ final class _EvaluateVisitor
);
if (callable == null) throw "Mixin not found: $name";

return SassMixin(callable);
return SassMixin.withCompileContext(callable, _compileContext);
}, url: "sass:meta"),

BuiltInCallable.function("call", r"$function, $args...", (
Expand Down Expand Up @@ -549,7 +556,10 @@ final class _EvaluateVisitor
return expression.accept(this);
}

var callable = function.assertFunction("function").callable;
var callable = function
.assertFunction("function")
.assertCompileContext(_compileContext)
.callable;
// ignore: unnecessary_type_check
if (callable is Callable) {
return _runFunctionCallable(
Expand Down Expand Up @@ -616,7 +626,10 @@ final class _EvaluateVisitor
rest: ValueExpression(args, callableNode.span),
);

var callable = mixin.assertMixin("mixin").callable;
var callable = mixin
.assertMixin("mixin")
.assertCompileContext(_compileContext)
.callable;
var content = _environment.content;

// ignore: unnecessary_type_check
Expand Down
6 changes: 1 addition & 5 deletions pkg/sass_api/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
## 15.5.1-dev

* No user-visible changes.

## 15.5.0
## 15.5.0-dev

* No user-visible changes.

Expand Down
4 changes: 2 additions & 2 deletions pkg/sass_api/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ name: sass_api
# Note: Every time we add a new Sass AST node, we need to bump the *major*
# version because it's a breaking change for anyone who's implementing the
# visitor interface(s).
version: 15.5.1-dev
version: 15.5.0-dev
description: Additional APIs for Dart Sass.
homepage: https://github.com/sass/dart-sass

environment:
sdk: ">=3.6.0 <4.0.0"

dependencies:
sass: 1.88.1
sass: 1.88.0

dev_dependencies:
dartdoc: ^8.0.14
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass
version: 1.88.1-dev
version: 1.88.0-dev
description: A Sass implementation in Dart.
homepage: https://github.com/sass/dart-sass

Expand Down