Basic support for namespacing get-function calls (#13)

* Add ability to test printed text during migration

* Add basic support for namespacing get-function

Resolves #11.

If the argument passed to get-function is a string literal, we'll add a
namespace if necessary.

For all other get-function calls, we'll emit a warning saying that a
namespace may be required. We might eventually be able to detect some of
these with some more sophisticated logic here, but for now we'll just
handle the basics.

* Use correct syntax for get-function migration

Also changes warning message to use span.message and updates tests to
use only ASCII characters for checking log.

* Quote namespace and add additional tests

* Add comment about module named "true"

lib/src/migrator.dart

@@ -11,6 +11,7 @@ import 'package:sass/src/ast/sass.dart';
 import 'package:sass/src/visitor/recursive_statement.dart';
 import 'package:sass/src/visitor/interface/expression.dart';
 
+import 'package:source_span/source_span.dart';
 import 'package:path/path.dart' as p;
 
 import 'built_in_functions.dart';
@@ -126,10 +127,45 @@ class _Migrator extends RecursiveStatementVisitor implements ExpressionVisitor {
   /// Adds a namespace to any function call that require it.
   void visitFunctionExpression(FunctionExpression node) {
     visitInterpolation(node.name);
+    _patchNamespaceForFunction(node.name.asPlain, (name, namespace) {
+      _currentMigration.patches.add(Patch(node.name.span, "$namespace.$name"));
+    });
     visitArgumentInvocation(node.arguments);
 
-    if (node.name.asPlain == null) return;
-    var name = node.name.asPlain;
+    if (node.name.asPlain == "get-function") {
+      var nameArgument =
+          node.arguments.named['name'] ?? node.arguments.positional.first;
+      if (nameArgument is! StringExpression ||
+          (nameArgument as StringExpression).text.asPlain == null) {
+        print(nameArgument.span.message(
+            "WARNING - get-function call may require \$module parameter"));
+        return;
+      }
+      var fnName = nameArgument as StringExpression;
+      _patchNamespaceForFunction(fnName.text.asPlain, (name, namespace) {
+        var span = fnName.span;
+        if (fnName.hasQuotes) {
+          span = span.file.span(span.start.offset + 1, span.end.offset - 1);
+        }
+        _currentMigration.patches.add(Patch(span, name));
+        var beforeParen = node.span.end.offset - 1;
+        _currentMigration.patches.add(Patch(
+            node.span.file.span(beforeParen, beforeParen),
+            ', \$module: "$namespace"'));
+      });
+    }
+  }
+
+  /// Calls [patcher] when the function [name] requires a namespace and adds a
+  /// new use rule if necessary.
+  ///
+  /// [patcher] takes two arguments: the name used to refer to that function
+  /// when namespaced, and the namespace itself. The name will match the name
+  /// provided to the outer function except for built-in functions whose name
+  /// within a module differs from its original name.
+  void _patchNamespaceForFunction(
+      String name, void patcher(String name, String namespace)) {
+    if (name == null) return;
     if (_localScope?.isLocalFunction(name) ?? false) return;
 
     var namespace = _functions.containsKey(name)
@@ -143,7 +179,7 @@ class _Migrator extends RecursiveStatementVisitor implements ExpressionVisitor {
       name = builtInFunctionNameChanges[name] ?? name;
       _currentMigration.additionalUseRules.add("sass:$namespace");
     }
-    _currentMigration.patches.add(Patch(node.name.span, "$namespace.$name"));
+    if (namespace != null) patcher(name, namespace);
   }
 
   /// Declares the function within the current scope before visiting it.

pubspec.yaml

@@ -12,5 +12,6 @@ dependencies:
   path: "^1.6.0"
 
 dev_dependencies:
+  term_glyph: "^1.1.0"
   test: ">=0.12.29 <2.0.0"
   test_descriptor: "^1.1.1"

test/migrations/README.md

@@ -16,3 +16,6 @@ The migrator will run starting from these entrypoints.
 For each file in `input` that would be modified by this migration, there should
 be a corresponding file in `output` with the migrated contents. `output` should
 not contain a file if it would not be changed by the migrator.
+
+If any warnings should be emitted by this migration, there should be an
+additional file called `log.txt` that contains the expected printed text.

test/migrations/namespace_get_function.hrx

@@ -0,0 +1,51 @@
+<==> input/entrypoint.scss
+@import "library";
+@import "true";
+
+$fn1: get-function(increment);
+// This function's namespace is `true`, which is a boolean when unquoted.
+// This tests that the $module parameter is a quoted string.
+$fn2: get-function($name: "false");
+$fn3: get-function("str-length");
+$fn4: get-function("incre" + "ment");
+
+$x: "increment";
+$fn5: get-function($x);
+
+<==> input/_library.scss
+@function increment($x) {
+  @return $x + 1;
+}
+
+<==> input/_true.scss
+@function false($x) {
+  @return not $x;
+}
+
+<==> output/entrypoint.scss
+@use "sass:meta";
+@use "sass:string";
+@use "library";
+@use "true";
+
+$fn1: meta.get-function(increment, $module: "library");
+// This function's namespace is `true`, which is a boolean when unquoted.
+// This tests that the $module parameter is a quoted string.
+$fn2: meta.get-function($name: "false", $module: "true");
+$fn3: meta.get-function("length", $module: "string");
+$fn4: meta.get-function("incre" + "ment");
+
+$x: "increment";
+$fn5: meta.get-function($x);
+
+<==> log.txt
+line 9, column 20 of $TEST_DIR/entrypoint.scss: WARNING - get-function call may require $module parameter
+  ,
+9 | $fn4: get-function("incre" + "ment");
+  |                    ^^^^^^^^^^^^^^^^
+  '
+line 12, column 20 of $TEST_DIR/entrypoint.scss: WARNING - get-function call may require $module parameter
+   ,
+12 | $fn5: get-function($x);
+   |                    ^^
+   '

test/migrator_test.dart

@@ -9,11 +9,13 @@ import 'dart:io';
 import 'package:sass_module_migrator/src/migrator.dart';
 
 import 'package:path/path.dart' as p;
+import 'package:term_glyph/term_glyph.dart' as glyph;
 import 'package:test/test.dart';
 import 'package:test_descriptor/test_descriptor.dart' as d;
 
 /// Runs all migration tests. See migrations/README.md for details.
 void main() {
+  glyph.ascii = true;
   var migrationTests = Directory("test/migrations");
   for (var file in migrationTests.listSync().whereType<File>()) {
     if (file.path.endsWith(".hrx")) {
@@ -28,7 +30,12 @@ testHrx(File hrxFile) async {
   await files.unpack();
   var entrypoints =
       files.input.keys.where((path) => path.startsWith("entrypoint"));
-  var migrated = migrateFiles(entrypoints, directory: d.sandbox);
+  p.PathMap<String> migrated;
+  var migration = () {
+    migrated = migrateFiles(entrypoints, directory: d.sandbox);
+  };
+  expect(migration,
+      prints(files.expectedLog?.replaceAll("\$TEST_DIR", d.sandbox) ?? ""));
   for (var file in files.input.keys) {
     expect(migrated[p.join(d.sandbox, file)], equals(files.output[file]),
         reason: 'Incorrect migration of $file.');
@@ -38,6 +45,7 @@ testHrx(File hrxFile) async {
 class HrxTestFiles {
   Map<String, String> input = {};
   Map<String, String> output = {};
+  String expectedLog;
 
   HrxTestFiles(String hrxText) {
     // TODO(jathak): Replace this with an actual HRX parser.
@@ -62,6 +70,8 @@ class HrxTestFiles {
       input[filename.substring(6)] = contents;
     } else if (filename.startsWith("output/")) {
       output[filename.substring(7)] = contents;
+    } else if (filename == "log.txt") {
+      expectedLog = contents;
     }
   }