Fix apheleia-npx in Yarn PnP projects (#301)

* `apheleia-npx` would use an incorrect path for the Yarn PnP ESM
loader.
* `apheleia-npx` did not correctly guard against word splitting.
* `apheleia-npx` was sometimes not able to find formatters in a Yarn PnP
project if there was also a node_modules folder at the root of the
project. Unfortunately, many tools (including
[Prettier](prettier/prettier#13032)) will
create a cache folder in `node_modules` even in Yarn PnP projects. The
presence of any `node_modules` folders are irrelevant when a `.pnp.cjs`
file is present.

---------

Co-authored-by: Radon Rosborough <[email protected]>

.shellcheckrc

@@ -0,0 +1 @@
+disable=SC2148

CHANGELOG.md

@@ -5,8 +5,23 @@ The format is based on [Keep a Changelog].
 
 ## Unreleased
 ### Formatters
-* [`rubocop`](https://github.com/rubocop/rubocop) changed to use `-a` instead of deprecated `--auto-correct` ([#316]).
+* [`rubocop`](https://github.com/rubocop/rubocop) changed to use `-a`
+  instead of deprecated `--auto-correct` ([#316]).
 
+### Bugs fixed
+* `apheleia-npx` would use an incorrect path for the Yarn PnP ESM
+  loader ([#301]).
+* `apheleia-npx` did not correctly guard against word splitting and
+  would fail when directory names contained spaces ([#301]).
+* `apheleia-npx` was sometimes not able to find formatters in a Yarn
+  PnP project if there was also a `node_modules` folder at the root of
+  the project ([#301]).
+
+## Internal
+* Improvements to formatter test framework, it is now possible to
+  write tests that have additional data files ([#301]).
+
+[#301]: https://github.com/radian-software/apheleia/pull/301
 [#316]: https://github.com/radian-software/apheleia/pull/316
 
 ## 4.2 (released 2024-08-03)
@@ -33,7 +48,6 @@ The format is based on [Keep a Changelog].
   information about errors, is still supported and will continue to be
   called if provided. See [#204].
 
-### Formatters
 ### Bugs fixed
 * The point alignment algorithm, which has been slightly wrong since
   2019, has been fixed to more correctly use dynamic programming to

Makefile

@@ -111,6 +111,10 @@ fmt-changed: ## Get list of changed formatters on this PR
 fmt-test: ## Actually run formatter tests
 	@test/shared/run-func.bash apheleia-ft-test $(APHELEIA_FT)
 
+.PHONY: fmt-emacs  # env var: FILE
+fmt-emacs: ## Start an Emacs instance for testing formatters
+	@emacs -L . -L test/formatters -l apheleia-ft -f apheleia-global-mode $(FILE)
+
 .PHONY: lint-changelog
 lint-changelog: ## Report an error if the changelog wasn't updated
 	@scripts/lint-changelog.bash

apheleia-utils.el

@@ -14,7 +14,8 @@
 (defcustom apheleia-formatters-respect-indent-level t
   "Whether formatters should respect Emacs' indent configuration."
   :type 'boolean
-  :group 'apheleia)
+  :group 'apheleia
+  :safe #'booleanp)
 
 (defun apheleia-formatters-indent (tab-flag indent-flag &optional indent-var)
   "Set flag for indentation.
@@ -73,7 +74,8 @@ always returns nil to defer to the formatter."
 (defcustom apheleia-formatters-respect-fill-column nil
   "Whether formatters should set `fill-column' related flags."
   :type 'boolean
-  :group 'apheleia)
+  :group 'apheleia
+  :safe #'booleanp)
 
 (defun apheleia-formatters-fill-column (fill-flag)
   "Set flag for wrap column.

scripts/formatters/apheleia-npx

@@ -15,7 +15,7 @@ if (( "$#" == 0 )); then
 fi
 
 # location of this script
-scripts_dir="$(cd $(dirname ${BASH_SOURCE[0]}) &>/dev/null && pwd)"
+scripts_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
 pnp_bin="${scripts_dir}/pnp-bin.js"
 
 # This function prints the name of the current directory if it
@@ -36,30 +36,32 @@ find_upwards() {
 dir="$(find_upwards package.json)"
 
 if [[ -d $dir ]]; then
-    cd $dir
+    cd "$dir"
 
-    pnp_root=$(find_upwards '.pnp.cjs')
-    npm_root=$(find_upwards 'node_modules')
+    pnp_root="$(find_upwards .pnp.cjs)"
+    npm_root="$(find_upwards node_modules)"
 
-    if [[ -n ${pnp_root} && ${#pnp_root} -gt ${#npm_root} ]]; then
+    if [[ -n ${pnp_root} ]]; then
         # trying pnp
         pnp_path="${pnp_root}/.pnp.cjs"
-        bin="$(${pnp_bin} ${pnp_path} $1)"
+        bin="$(${pnp_bin} "${pnp_path}" "$1")"
         # note: $bin might not be on the real filesystem,
         #   might be in a zip archive
         if [[ -n $bin ]]; then
-            if [[ -f "${pnp_path}/.pnp.loader.mjs" ]]; then
-                loader_opt="--loader ${pnp_path}/.pnp.loader.mjs"
+            node="$(which node)"
+            if [[ -f "${pnp_root}/.pnp.loader.mjs" ]]; then
+                exec ${node} --require "${pnp_path}" \
+                    --loader "${pnp_root}/.pnp.loader.mjs" "${bin}" "${@:2}"
             fi
-            node=$(which node)
-            command="${node} --require ${pnp_path} ${loader_opt} ${bin} ${@:2}"
-            exec ${command}
+            exec ${node} --require "${pnp_path}" "${bin}" "${@:2}"
         fi
     elif [[ -n ${npm_root} ]]; then
         # trying npm
-        node_modules_paths=(\
-            $(node -e 'console.log(require.resolve.paths("").join("\n"))'))
-        for path in ${node_modules_paths[@]}; do
+        node_modules_paths=()
+        while IFS='' read -r line; do
+            node_modules_paths+=("$line")
+        done < <(node -e 'console.log(require.resolve.paths("").join("\n"))')
+        for path in "${node_modules_paths[@]}"; do
             if [[ -x "${path}/.bin/$1" ]]; then
                 exec "${path}/.bin/$1" "${@:2}"
             fi

test/formatters/apheleia-ft.el

@@ -20,6 +20,10 @@
   "Root directory of the Git repository.
 Guaranteed to be absolute and expanded.")
 
+(defvar apheleia-ft--temp-dir
+  (expand-file-name "apheleia-ft" (temporary-file-directory))
+  "Directory for storing temporary files.")
+
 (defun apheleia-ft--relative-truename (path)
   "Given PATH relative to repo root, resolve symlinks.
 Return another path relative to repo root."
@@ -155,14 +159,68 @@ Return the filename."
 
 (defun apheleia-ft--input-files (formatter)
   "For given FORMATTER, return list of input files used in test cases.
-These are absolute filepaths beginning with \"in.\"."
-  (directory-files
+These are absolute filepaths whose basenames begin with \"in.\".
+FORMATTER is a string."
+  (directory-files-recursively
    (apheleia-ft--path-join
     apheleia-ft--test-dir
     "samplecode" formatter)
-   'full
    "^in\\."))
 
+(defun apheleia-ft--copy-inputs (formatter in-file)
+  "Prepare FORMATTER for testing by copying IN-FILE and related.
+FORMATTER is a string, IN-FILE is an absolute filepath whose
+basename begins with \"in.\". All files from the samplecode
+subdirectory for FORMATTER are copied to the toplevel of
+`apheleia-ft--temp-dir', replicating the directory structure,
+except that all the actual \"in.\" and \"out.\" files are not
+copied, except for IN-FILE, which is left in the corresponding
+place in the directory structure. Any existing files or
+directories in `apheleia-ft--temp-dir' are removed. Return the
+absolute filepath to which IN-FILE was copied in the temporary
+directory."
+  (delete-directory apheleia-ft--temp-dir 'recursive)
+  (unless (zerop
+           (call-process
+            "cp" nil nil nil
+            (apheleia-ft--path-join
+             apheleia-ft--test-dir
+             "samplecode" formatter)
+            apheleia-ft--temp-dir
+            "-RTL"))
+    (error "cp failed"))
+  (with-temp-file (expand-file-name ".dir-locals.el" apheleia-ft--temp-dir)
+    (prin1 '((nil . ((indent-tabs-mode . nil)
+                     (apheleia-formatters-respect-fill-column . nil)
+                     (apheleia-formatters-respect-indent-level . nil))))
+           (current-buffer)))
+  (let ((new-file
+         (expand-file-name
+          (replace-regexp-in-string
+           (concat "^" (regexp-quote
+                        (apheleia-ft--path-join
+                         apheleia-ft--test-dir
+                         "samplecode" formatter))
+                   "/")
+           "" in-file)
+          apheleia-ft--temp-dir)))
+    (prog1 new-file
+      (dolist (fname (directory-files-recursively
+                      apheleia-ft--temp-dir
+                      "^\\(in\\|out\\)\\."))
+        (unless (string= fname new-file)
+          (delete-file fname)))
+      (let ((init-script (expand-file-name
+                          ".apheleia-ft.bash"
+                          (file-name-directory
+                           new-file))))
+        (when (file-exists-p init-script)
+          (let ((default-directory (file-name-directory init-script)))
+            (unless (zerop
+                     (call-process
+                      "bash" nil nil nil init-script))
+              (error "init script failed: %S" init-script))))))))
+
 (defun apheleia-ft--path-join (component &rest components)
   "Join COMPONENT and COMPONENTS together, left to right.
 Return an absolute path."
@@ -242,12 +300,7 @@ involve running any formatters."
                       apheleia-ft--test-dir "samplecode" formatter out-file))
               (error "Input file %s is has no corresponding output file %s"
                      in-file out-file))
-            (push out-file out-files)))
-        (dolist (file all-files)
-          (unless (or (member file in-files)
-                      (member file out-files))
-            (error "Spurious sample code file at samplecode/%s/%s"
-                   formatter file)))))
+            (push out-file out-files)))))
     (dolist (samplecode-dir samplecode-dirs)
       (unless (member samplecode-dir formatters)
         (error
@@ -270,13 +323,11 @@ returned context."
   (interactive
    (unless (or current-prefix-arg noninteractive)
      (list (completing-read "Formatter: " (apheleia-ft--get-formatters)))))
-  (setq-default indent-tabs-mode nil)
   (dolist (formatter (or formatters (apheleia-ft--get-formatters)))
     (dolist (in-file (apheleia-ft--input-files formatter))
       (let* ((extension (file-name-extension in-file))
              (in-text (apheleia-ft--read-file in-file))
-             (in-temp-file (apheleia-ft--write-temp-file
-                            in-text extension))
+             (in-temp-file (apheleia-ft--copy-inputs formatter in-file))
              (out-temp-file nil)
              (command (alist-get (intern formatter) apheleia-formatters))
              (syms nil)
@@ -294,12 +345,18 @@ returned context."
                             ;; Borrowed with love from Magit
                             (let ((load-suffixes '(".el")))
                               (locate-library "apheleia"))))))
-                      exec-path)))
+                      exec-path))
+             (display-fname
+              (replace-regexp-in-string
+               (concat "^" (regexp-quote
+                            (apheleia-ft--path-join
+                             apheleia-ft--test-dir
+                             "samplecode" formatter))
+                       "/")
+               "" in-file)))
+        (when-let ((buf (get-file-buffer in-temp-file)))
+          (kill-buffer buf))
         (with-current-buffer (find-file-noselect in-temp-file)
-          ;; Some formatters use the current file-name or buffer-name to interpret the
-          ;; type of file that is being formatted. Some may not be able to determine
-          ;; this from the contents of the file so we set this to force it.
-          (rename-buffer (file-name-nondirectory in-file))
           (setq stdout-buffer (get-buffer-create
                                (format "*apheleia-ft-stdout-%S%s" formatter extension)))
           (with-current-buffer stdout-buffer
@@ -358,9 +415,12 @@ returned context."
             (apheleia-ft--print-diff
              "expected" expected-out-text
              "actual" out-text)
-            (error "Formatter %s did not format as expected" formatter)))
+            (error "Formatter %s did not format %s as expected"
+                   formatter display-fname)))
         (princ (format
                 "[format-test] success: formatter %s (file %s)\n"
-                formatter (file-name-nondirectory in-file)))))))
+                formatter display-fname))
+        ;; https://stackoverflow.com/a/66558297
+        (set-binary-mode 'stdout nil)))))
 
 (provide 'apheleia-ft)

test/formatters/installers/prettier-ruby.bash

@@ -12,5 +12,4 @@ gem install rbs -v 3.1.3
 gem install prettier_print syntax_tree syntax_tree-haml syntax_tree-rbs
 
 # Install the plugin
-cd /tmp
-npm install --save-dev prettier @prettier/plugin-ruby
+npm install -g prettier @prettier/plugin-ruby

test/formatters/installers/prettier-svelte.bash

@@ -1,2 +1 @@
-cd /tmp
-npm install --save-dev prettier-plugin-svelte prettier
+npm install -g prettier-plugin-svelte prettier

test/formatters/samplecode/prettier-ruby/.apheleia-ft.bash

@@ -0,0 +1,4 @@
+# Fucking Node devs removed NODE_PATH support so now you have to do
+# this bullshit to get import() to work with globally installed
+# packages.
+ln -s /usr/lib/node_modules ./

test/formatters/samplecode/prettier-ruby/.prettierrc.json

@@ -0,0 +1 @@
+{}

test/formatters/samplecode/prettier-svelte/.apheleia-ft.bash

@@ -0,0 +1,4 @@
+# Fucking Node devs removed NODE_PATH support so now you have to do
+# this bullshit to get import() to work with globally installed
+# packages.
+ln -s /usr/lib/node_modules ./

test/formatters/samplecode/prettier/test-finds-config-file/.prettierrc.json

@@ -0,0 +1,3 @@
+{
+  "tabWidth": 3
+}

test/formatters/samplecode/prettier/test-finds-config-file/in.js

@@ -0,0 +1 @@
+../../prettier-javascript/in.js

test/formatters/samplecode/prettier/test-finds-config-file/out.js

@@ -0,0 +1,10 @@
+function HelloWorld({
+   greeting = "hello",
+   greeted = '"World"',
+   silent = false,
+   onMouseOver,
+}) {
+   if (!greeting) {
+      return null;
+   }
+}

test/formatters/samplecode/prettier/test-uses-node-modules/.apheleia-ft.bash

@@ -0,0 +1 @@
+npm install

test/formatters/samplecode/prettier/test-uses-node-modules/.gitignore

@@ -0,0 +1 @@
+node_modules

test/formatters/samplecode/prettier/test-uses-node-modules/in.js

@@ -0,0 +1,6 @@
+// Behavior differs between 2.x and 3.x
+// https://prettier.io/blog/2023/07/05/3.0.0.html
+call(
+  @dec
+  class {},
+);

test/formatters/samplecode/prettier/test-uses-node-modules/out.js

@@ -0,0 +1,8 @@
+// Behavior differs between 2.x and 3.x
+// https://prettier.io/blog/2023/07/05/3.0.0.html
+call(
+  (
+    @dec
+    class {}
+  )
+);

test/formatters/samplecode/prettier/test-uses-node-modules/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "apheleia-ft-prettier-test-uses-node-modules",
+  "version": "1.0.0",
+  "description": "Prettier should be used from node_modules",
+  "main": "index.js",
+  "author": "Radian LLC <[email protected]>",
+  "license": "MIT",
+  "private": true,
+  "dependencies": {
+    "prettier": "^2.8.8"
+  }
+}

test/shared/run-func.bash

@@ -10,5 +10,8 @@ shift
 
 cd "$(dirname "$0")/../.."
 
-exec emacs --batch -L . "$@" \
-     --eval "(setq debug-on-error t)" -f "${func}"
+exec emacs --batch -L . "$@"                  \
+     --eval "(setq debug-on-error t)"         \
+     --eval "(setq backtrace-line-length 0)"  \
+     -f     "${func}"                         \
+     2>&1 | sed -uE 's/^(.{320}).+$/\1...[truncated]/'