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.

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

Fix apheleia-npx in Yarn PnP projects #301

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .shellcheckrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
disable=SC2148
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ The format is based on [Keep a Changelog].
calling major-mode. Also includes setting for `indent-tabs-mode` ([#286]).
* [Formatter scripts](scripts/formatters) will now work on Windows if Emacs
can find the executable defined in the shebang.
* `apheleia-npx` would use an incorrect path for the Yarn PnP ESM
loader. ([#301])
* `apheleia-npx` did not correctly guard against word splitting.
([#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
* Major internal refactoring has occurred to make it possible to write
Expand All @@ -49,6 +56,7 @@ The format is based on [Keep a Changelog].
[#286]: https://github.com/radian-software/apheleia/pull/286
[#285]: https://github.com/radian-software/apheleia/issues/285
[#290]: https://github.com/radian-software/apheleia/pull/290
[#301]: https://github.com/radian-software/apheleia/pull/301
[#302]: https://github.com/radian-software/apheleia/issues/302

## 4.1 (released 2024-02-25)
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions apheleia-utils.el
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
30 changes: 16 additions & 14 deletions scripts/formatters/apheleia-npx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
94 changes: 75 additions & 19 deletions test/formatters/apheleia-ft.el
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -155,14 +159,66 @@ 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"
apheleia-ft--temp-dir)))
(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."
Expand Down Expand Up @@ -242,12 +298,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
Expand All @@ -270,13 +321,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)
Expand All @@ -294,12 +343,16 @@ 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)))
(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
Expand Down Expand Up @@ -358,9 +411,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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"tabWidth": 3
}
10 changes: 10 additions & 0 deletions test/formatters/samplecode/prettier/test-finds-config-file/out.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
function HelloWorld({
greeting = "hello",
greeted = '"World"',
silent = false,
onMouseOver,
}) {
if (!greeting) {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
npm install
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
node_modules
Original file line number Diff line number Diff line change
@@ -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 {},
);
Original file line number Diff line number Diff line change
@@ -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 {}
)
);

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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 <contact+apheleia@radian.codes>",
"license": "MIT",
"private": true,
"dependencies": {
"prettier": "^2.8.8"
}
}
7 changes: 5 additions & 2 deletions test/shared/run-func.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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]/'