diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d430a121..c5ae69c7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,31 @@ * Add support for nesting in plain CSS files. This is not processed by Sass at all; it's emitted exactly as-is in the CSS. +* In certain circumstances, the current working directory was unintentionally + being made available as a load path. This is now deprecated. Anyone relying on + this should explicitly pass in `.` as a load path or `FilesystemImporter('.')` + as the current importer. + * Add linux-riscv64 and windows-arm64 releases. +### Command-Line Interface + +* Fix a bug where absolute `file:` URLs weren't loaded for files compiled via + the command line unless an unrelated load path was also passed. + +* Fix a bug where `--update` would always update files that were specified via + absolute path unless an unrelated load path was also passed. + +### Dart API + +* Add `FilesystemImporter.noLoadPath`, which is a `FilesystemImporter` that can + load absolute `file:` URLs and resolve URLs relative to the base file but + doesn't load relative URLs from a load path. + +* `FilesystemImporter.cwd` is now deprecated. Either use + `FilesystemImporter.noLoadPath` if you weren't intending to rely on the load + path, or `FilesystemImporter('.')` if you were. + ## 1.72.0 * Support adjacent `/`s without whitespace in between when parsing plain CSS diff --git a/bin/sass.dart b/bin/sass.dart index 67b1782ed..d24439eb9 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -13,6 +13,7 @@ import 'package:sass/src/executable/options.dart'; import 'package:sass/src/executable/repl.dart'; import 'package:sass/src/executable/watch.dart'; import 'package:sass/src/import_cache.dart'; +import 'package:sass/src/importer/filesystem.dart'; import 'package:sass/src/io.dart'; import 'package:sass/src/logger/deprecation_handling.dart'; import 'package:sass/src/stylesheet_graph.dart'; @@ -45,7 +46,7 @@ Future main(List args) async { } var graph = StylesheetGraph(ImportCache( - importers: options.pkgImporters, + importers: [...options.pkgImporters, FilesystemImporter.noLoadPath], loadPaths: options.loadPaths, // This logger is only used for handling fatal/future deprecations // during parsing, and is re-used across parses, so we don't want to diff --git a/lib/src/deprecation.dart b/lib/src/deprecation.dart index 2724d0afe..3f8540634 100644 --- a/lib/src/deprecation.dart +++ b/lib/src/deprecation.dart @@ -69,6 +69,11 @@ enum Deprecation { deprecatedIn: '1.62.3', description: 'Passing null as alpha in the ${isJS ? 'JS' : 'Dart'} API.'), + fsImporterCwd('fs-importer-cwd', + deprecatedIn: '1.73.0', + description: + 'Using the current working directory as an implicit load path.'), + @Deprecated('This deprecation name was never actually used.') calcInterp('calc-interp', deprecatedIn: null), diff --git a/lib/src/evaluation_context.dart b/lib/src/evaluation_context.dart index 5c1b074a9..30b3852f9 100644 --- a/lib/src/evaluation_context.dart +++ b/lib/src/evaluation_context.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:source_span/source_span.dart'; import 'deprecation.dart'; +import 'logger.dart'; /// An interface that exposes information about the current Sass evaluation. /// @@ -17,13 +18,16 @@ abstract interface class EvaluationContext { /// /// Throws [StateError] if there isn't a Sass stylesheet currently being /// evaluated. - static EvaluationContext get current { - if (Zone.current[#_evaluationContext] case EvaluationContext context) { - return context; - } else { - throw StateError("No Sass stylesheet is currently being evaluated."); - } - } + static EvaluationContext get current => + currentOrNull ?? + (throw StateError("No Sass stylesheet is currently being evaluated.")); + + /// The current evaluation context, or `null` if none exists. + static EvaluationContext? get currentOrNull => + switch (Zone.current[#_evaluationContext]) { + EvaluationContext context => context, + _ => null + }; /// Returns the span for the currently executing callable. /// @@ -50,13 +54,20 @@ abstract interface class EvaluationContext { /// This may only be called within a custom function or importer callback. /// {@category Compile} void warn(String message, {bool deprecation = false}) => - EvaluationContext.current - .warn(message, deprecation ? Deprecation.userAuthored : null); + switch (EvaluationContext.currentOrNull) { + var context? => + context.warn(message, deprecation ? Deprecation.userAuthored : null), + _ when deprecation => (const Logger.stderr()) + .warnForDeprecation(Deprecation.userAuthored, message), + _ => (const Logger.stderr()).warn(message) + }; /// Prints a deprecation warning with [message] of type [deprecation]. -void warnForDeprecation(String message, Deprecation deprecation) { - EvaluationContext.current.warn(message, deprecation); -} +void warnForDeprecation(String message, Deprecation deprecation) => + switch (EvaluationContext.currentOrNull) { + var context? => context.warn(message, deprecation), + _ => (const Logger.stderr()).warnForDeprecation(deprecation, message) + }; /// Runs [callback] with [context] as [EvaluationContext.current]. /// diff --git a/lib/src/executable/compile_stylesheet.dart b/lib/src/executable/compile_stylesheet.dart index 160a01d9a..cd121a6f5 100644 --- a/lib/src/executable/compile_stylesheet.dart +++ b/lib/src/executable/compile_stylesheet.dart @@ -73,8 +73,8 @@ Future _compileStylesheetWithoutErrorHandling(ExecutableOptions options, try { if (source != null && destination != null && - !graph.modifiedSince( - p.toUri(source), modificationTime(destination), importer)) { + !graph.modifiedSince(p.toUri(p.absolute(source)), + modificationTime(destination), importer)) { return; } } on FileSystemException catch (_) { diff --git a/lib/src/importer/filesystem.dart b/lib/src/importer/filesystem.dart index 47b0ae288..cb23d3095 100644 --- a/lib/src/importer/filesystem.dart +++ b/lib/src/importer/filesystem.dart @@ -5,30 +5,86 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; +import '../deprecation.dart'; +import '../evaluation_context.dart'; import '../importer.dart'; import '../io.dart' as io; import '../syntax.dart'; import '../util/nullable.dart'; import 'utils.dart'; -/// An importer that loads files from a load path on the filesystem. +/// An importer that loads files from a load path on the filesystem, either +/// relative to the path passed to [FilesystemImporter.new] or absolute `file:` +/// URLs. +/// +/// Use [FilesystemImporter.noLoadPath] to _only_ load absolute `file:` URLs and +/// URLs relative to the current file. /// /// {@category Importer} @sealed class FilesystemImporter extends Importer { /// The path relative to which this importer looks for files. - final String _loadPath; + /// + /// If this is `null`, this importer will _only_ load absolute `file:` URLs + /// and URLs relative to the current file. + final String? _loadPath; + + /// Whether loading from files from this importer's [_loadPath] is deprecated. + final bool _loadPathDeprecated; /// Creates an importer that loads files relative to [loadPath]. - FilesystemImporter(String loadPath) : _loadPath = p.absolute(loadPath); + FilesystemImporter(String loadPath) + : _loadPath = p.absolute(loadPath), + _loadPathDeprecated = false; + + FilesystemImporter._deprecated(String loadPath) + : _loadPath = p.absolute(loadPath), + _loadPathDeprecated = true; + + /// Creates an importer that _only_ loads absolute `file:` URLs and URLs + /// relative to the current file. + FilesystemImporter._noLoadPath() + : _loadPath = null, + _loadPathDeprecated = false; - /// Creates an importer relative to the current working directory. - static final cwd = FilesystemImporter('.'); + /// A [FilesystemImporter] that loads files relative to the current working + /// directory. + /// + /// Historically, this was the best default for supporting `file:` URL loads + /// when the load path didn't matter. However, adding the current working + /// directory to the load path wasn't always desirable, so it's no longer + /// recommended. Instead, either use [FilesystemImporter.noLoadPath] if the + /// load path doesn't matter, or `FilesystemImporter('.')` to explicitly + /// preserve the existing behavior. + @Deprecated( + "Use FilesystemImporter.noLoadPath or FilesystemImporter('.') instead.") + static final cwd = FilesystemImporter._deprecated('.'); + + /// Creates an importer that _only_ loads absolute `file:` URLsand URLs + /// relative to the current file. + static final noLoadPath = FilesystemImporter._noLoadPath(); Uri? canonicalize(Uri url) { - if (url.scheme != 'file' && url.scheme != '') return null; - return resolveImportPath(p.join(_loadPath, p.fromUri(url))) - .andThen((resolved) => p.toUri(io.canonicalize(resolved))); + String? resolved; + if (url.scheme == 'file') { + resolved = resolveImportPath(p.fromUri(url)); + } else if (url.scheme != '') { + return null; + } else if (_loadPath case var loadPath?) { + resolved = resolveImportPath(p.join(loadPath, p.fromUri(url))); + + if (resolved != null && _loadPathDeprecated) { + warnForDeprecation( + "Using the current working directory as an implicit load path is " + "deprecated. Either add it as an explicit load path or importer, or " + "load this stylesheet from a different URL.", + Deprecation.fsImporterCwd); + } + } else { + return null; + } + + return resolved.andThen((resolved) => p.toUri(io.canonicalize(resolved))); } ImporterResult? load(Uri url) { @@ -53,5 +109,5 @@ class FilesystemImporter extends Importer { basename == p.url.withoutExtension(canonicalBasename); } - String toString() => _loadPath; + String toString() => _loadPath ?? ''; } diff --git a/test/cli/shared/update.dart b/test/cli/shared/update.dart index 4fc7bfbc1..73b8a21df 100644 --- a/test/cli/shared/update.dart +++ b/test/cli/shared/update.dart @@ -2,6 +2,7 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_process/test_process.dart'; @@ -148,6 +149,18 @@ void sharedTests(Future runSass(Iterable arguments)) { await d.file("out.css", "x {y: z}").validate(); }); + // Regression test for #2203 + test("whose sources weren't modified with an absolute path", () async { + await d.file("test.scss", "a {b: c}").create(); + await d.file("out.css", "x {y: z}").create(); + + var sass = await update(["${p.absolute(d.path('test.scss'))}:out.css"]); + expect(sass.stdout, emitsDone); + await sass.shouldExit(0); + + await d.file("out.css", "x {y: z}").validate(); + }); + test("whose sibling was modified", () async { await d.file("test1.scss", "a {b: c}").create(); await d.file("out1.css", "x {y: z}").create(); diff --git a/test/dart_api/logger_test.dart b/test/dart_api/logger_test.dart index a1c0ec93f..979b2ba11 100644 --- a/test/dart_api/logger_test.dart +++ b/test/dart_api/logger_test.dart @@ -227,10 +227,6 @@ void main() { mustBeCalled(); })); }); - - test("throws an error outside a callback", () { - expect(() => warn("heck"), throwsStateError); - }); }); }