[New] allow .nvmrc files to support comments

In theory, `npx nvmrc` can now be used to validate an `.nvmrc` file that `nvm` will support. Allowances have been made for future extensibility, and aliases may no longer contain a `#`. Fixes nvm-sh#3336. Closes nvm-sh#2288. Co-authored-by: Jordan Harband <[email protected]> Co-authored-by: Yash Singh <[email protected]>
ljharb · Jun 7, 2024 · 29dce5e · 29dce5e
1 parent 95081f0
commit 29dce5e
Show file tree

Hide file tree

Showing 8 changed files with 278 additions and 3 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -26,3 +26,10 @@ insert_final_newline = off
 
 [Makefile]
 indent_style = tab
+
+[test/fixtures/nvmrc/**]
+indent_style = off
+insert_final_newline = off
+
+[test/fixtures/actual/alias/empty]
+insert_final_newline = off
diff --git a/.gitmodules b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "test/fixtures/nvmrc"]
+  path = test/fixtures/nvmrc
+  url = [email protected]:nvm-sh/nvmrc.git
diff --git a/README.md b/README.md
@@ -298,6 +298,13 @@ To install a specific version of node:
 nvm install 14.7.0 # or 16.3.0, 12.22.1, etc
 ```
 
+To set an alias:
+
+```sh
+nvm alias my_alias v14.4.0
+```
+Make sure that your alias does not contain any spaces or slashes.
+
 The first version installed becomes the default. New shells will start with the default version of node (e.g., `nvm alias default`).
 
 You can list available versions using `ls-remote`:
@@ -563,7 +570,11 @@ Now using node v5.9.1 (npm v3.7.3)
 
 `nvm use` et. al. will traverse directory structure upwards from the current directory looking for the `.nvmrc` file. In other words, running `nvm use` et. al. in any subdirectory of a directory with an `.nvmrc` will result in that `.nvmrc` being utilized.
 
-The contents of a `.nvmrc` file **must** be the `<version>` (as described by `nvm --help`) followed by a newline. No trailing spaces are allowed, and the trailing newline is required.
+The contents of a `.nvmrc` file **must** contain precisely one `<version>` (as described by `nvm --help`) followed by a newline. `.nvmrc` files may also have comments. The comment delimiter is `#`, and it and any text after it, as well as blank lines, and leading and trailing white space, will be ignored when parsing.
+
+Key/value pairs using `=` are also allowed and ignored, but are reserved for future use, and may cause validation errors in the future.
+
+Run [`npx nvmrc`](https://npmjs.com/nvmrc) to validate an `.nvmrc` file. If that tool’s results do not agree with nvm, one or the other has a bug - please file an issue.
 
 ### Deeper Shell Integration
 

diff --git a/nvm.sh b/nvm.sh
@@ -467,7 +467,89 @@ nvm_find_nvmrc() {
   fi
 }
 
-# Obtain nvm version from rc file
+nvm_nvmrc_invalid_msg() {
+  local error_text
+  error_text="invalid .nvmrc!
+all non-commented content (anything after # is a comment) must be either:
+  - a single bare nvm-recognized version-ish
+  - or, multiple distinct key-value pairs, each key/value separated by a single equals sign (=)
+
+additionally, a single bare nvm-recognized version-ish must be present (after stripping comments)."
+
+  local warn_text
+  warn_text="non-commented content parsed:
+${1}"
+
+  nvm_err "$(nvm_wrap_with_color_code r "${error_text}")
+
+$(nvm_wrap_with_color_code y "${warn_text}")"
+}
+
+nvm_process_nvmrc() {
+  local NVMRC_PATH="$1"
+  local lines
+  local unpaired_line
+
+  lines=$(command sed 's/#.*//' "$NVMRC_PATH" | command sed 's/^[[:space:]]*//;s/[[:space:]]*$//' | nvm_grep -v '^$')
+
+  if [ -z "$lines" ]; then
+    nvm_nvmrc_invalid_msg "${lines}"
+    return 1
+  fi
+
+  # Initialize key-value storage
+  local keys=''
+  local values=''
+
+  while IFS= read -r line; do
+    if [ -z "${line}" ]; then
+      continue
+    elif [ -z "${line%%=*}" ]; then
+      if [ -n "${unpaired_line}" ]; then
+        nvm_nvmrc_invalid_msg "${lines}"
+        return 1
+      fi
+      unpaired_line="${line}"
+    elif case "$line" in *'='*) true;; *) false;; esac; then
+      key="${line%%=*}"
+      value="${line#*=}"
+
+      # Trim whitespace around key and value
+      key=$(nvm_echo "${key}" | command sed 's/^[[:space:]]*//;s/[[:space:]]*$//')
+      value=$(nvm_echo "${value}" | command sed 's/^[[:space:]]*//;s/[[:space:]]*$//')
+
+      # Check for invalid key "node"
+      if [ "${key}" = 'node' ]; then
+        nvm_nvmrc_invalid_msg "${lines}"
+        return 1
+      fi
+
+      # Check for duplicate keys
+      if nvm_echo "${keys}" | nvm_grep -q -E "(^| )${key}( |$)"; then
+        nvm_nvmrc_invalid_msg "${lines}"
+        return 1
+      fi
+      keys="${keys} ${key}"
+      values="${values} ${value}"
+    else
+      if [ -n "${unpaired_line}" ]; then
+        nvm_nvmrc_invalid_msg "${lines}"
+        return 1
+      fi
+      unpaired_line="${line}"
+    fi
+  done <<EOF
+$lines
+EOF
+
+  if [ -z "${unpaired_line}" ]; then
+    nvm_nvmrc_invalid_msg "${lines}"
+    return 1
+  fi
+
+  nvm_echo "${unpaired_line}"
+}
+
 nvm_rc_version() {
   export NVM_RC_VERSION=''
   local NVMRC_PATH
@@ -478,7 +560,12 @@ nvm_rc_version() {
     fi
     return 1
   fi
-  NVM_RC_VERSION="$(command head -n 1 "${NVMRC_PATH}" | command tr -d '\r')" || command printf ''
+
+
+  if ! NVM_RC_VERSION="$(nvm_process_nvmrc "${NVMRC_PATH}")"; then
+    return 1
+  fi
+
   if [ -z "${NVM_RC_VERSION}" ]; then
     if [ "${NVM_SILENT:-0}" -ne 1 ]; then
       nvm_err "Warning: empty .nvmrc file found at \"${NVMRC_PATH}\""
@@ -4058,6 +4145,9 @@ nvm() {
         # so, unalias it.
         nvm unalias "${ALIAS}"
         return $?
+      elif echo "${ALIAS}" | grep -q "#"; then
+        nvm_err 'Aliases with a comment delimiter (#) are not supported.'
+        return 1
       elif [ "${TARGET}" != '--' ]; then
         # a target was passed: create an alias
         if [ "${ALIAS#*\/}" != "${ALIAS}" ]; then
@@ -4271,6 +4361,7 @@ nvm() {
         nvm_get_colors nvm_set_colors nvm_print_color_code nvm_wrap_with_color_code nvm_format_help_message_colors \
         nvm_echo_with_colors nvm_err_with_colors \
         nvm_get_artifact_compression nvm_install_binary_extract nvm_extract_tarball \
+        nvm_process_nvmrc nvm_nvmrc_invalid_msg \
         >/dev/null 2>&1
       unset NVM_RC_VERSION NVM_NODEJS_ORG_MIRROR NVM_IOJS_ORG_MIRROR NVM_DIR \
         NVM_CD_FLAGS NVM_BIN NVM_INC NVM_MAKE_JOBS \

diff --git a/test/common.sh b/test/common.sh
@@ -101,3 +101,105 @@ watch() {
   kill %2;
   return $EXIT_CODE
 }
+
+parse_json() {
+  local json
+  json="$1"
+  local key
+  key=""
+  local value
+  value=""
+  local output
+  output=""
+  local in_key
+  in_key=0
+  local in_value
+  in_value=0
+  local in_string
+  in_string=0
+  local escaped
+  escaped=0
+  local buffer
+  buffer=""
+  local char
+  local len
+  len=${#json}
+  local arr_index
+  arr_index=0
+  local in_array
+  in_array=0
+
+  for ((i = 0; i < len; i++)); do
+    char="${json:i:1}"
+
+    if [ "$in_string" -eq 1 ]; then
+      if [ "$escaped" -eq 1 ]; then
+        buffer="$buffer$char"
+        escaped=0
+      elif [ "$char" = "\\" ]; then
+        escaped=1
+      elif [ "$char" = "\"" ]; then
+        in_string=0
+        if [ "$in_key" -eq 1 ]; then
+          key="$buffer"
+          buffer=""
+          in_key=0
+        elif [ "$in_value" -eq 1 ]; then
+          value="$buffer"
+          buffer=""
+          output="$output$key=\"$value\"\n"
+          in_value=0
+        elif [ "$in_array" -eq 1 ]; then
+          value="$buffer"
+          buffer=""
+          output="$output$arr_index=\"$value\"\n"
+          arr_index=$((arr_index + 1))
+        fi
+      else
+        buffer="$buffer$char"
+      fi
+      continue
+    fi
+
+    case "$char" in
+      "\"")
+        in_string=1
+        buffer=""
+        if [ "$in_value" -eq 0 ] && [ "$in_array" -eq 0 ]; then
+          in_key=1
+        fi
+        ;;
+      ":")
+        in_value=1
+        ;;
+      ",")
+        if [ "$in_value" -eq 1 ]; then
+          in_value=0
+        fi
+        ;;
+      "[")
+        in_array=1
+        ;;
+      "]")
+        in_array=0
+        ;;
+      "{" | "}")
+        ;;
+      *)
+        if [ "$in_value" -eq 1 ] && [ "$char" != " " ] && [ "$char" != "\n" ] && [ "$char" != "\t" ]; then
+          buffer="$buffer$char"
+        fi
+        ;;
+    esac
+  done
+
+  printf "%b" "$output"
+}
+
+extract_value() {
+  local key
+  key="$1"
+  local parsed
+  parsed="$2"
+  echo "$parsed" | grep "^$key=" | cut -d'=' -f2 | tr -d '"'
+}
diff --git a/test/fast/Aliases/'nvm alias' should not accept aliases with a hash b/test/fast/Aliases/'nvm alias' should not accept aliases with a hash
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+\. ../../../nvm.sh
+
+die () { echo "$@" ; exit 1; }
+
+OUTPUT="$(nvm alias foo#bar baz 2>&1)"
+EXPECTED_OUTPUT="Aliases with a comment delimiter (#) are not supported."
+[ "$OUTPUT" = "$EXPECTED_OUTPUT" ] || die "trying to create an alias with a hash should fail with '$EXPECTED_OUTPUT', got '$OUTPUT'"
+
+EXIT_CODE="$(nvm alias foo#bar baz >/dev/null 2>&1 ; echo $?)"
+[ "$EXIT_CODE" = "1" ] || die "trying to create an alias with a hash should fail with code 1, got '$EXIT_CODE'"
+
+OUTPUT="$(nvm alias foo# baz 2>&1)"
+EXPECTED_OUTPUT="Aliases with a comment delimiter (#) are not supported."
+[ "$OUTPUT" = "$EXPECTED_OUTPUT" ] || die "trying to create an alias ending with a hash should fail with '$EXPECTED_OUTPUT', got '$OUTPUT'"
+
+EXIT_CODE="$(nvm alias foo# baz >/dev/null 2>&1 ; echo $?)"
+[ "$EXIT_CODE" = "1" ] || die "trying to create an alias ending with a hash should fail with code 1, got '$EXIT_CODE'"
+
+OUTPUT="$(nvm alias \#bar baz 2>&1)"
+EXPECTED_OUTPUT="Aliases with a comment delimiter (#) are not supported."
+[ "$OUTPUT" = "$EXPECTED_OUTPUT" ] || die "trying to create an alias starting with a hash should fail with '$EXPECTED_OUTPUT', got '$OUTPUT'"
+
+EXIT_CODE="$(nvm alias \#bar baz >/dev/null 2>&1 ; echo $?)"
+[ "$EXIT_CODE" = "1" ] || die "trying to create an alias starting with a hash should fail with code 1, got '$EXIT_CODE'"
diff --git a/test/fast/Unit tests/nvm_process_nvmrc b/test/fast/Unit tests/nvm_process_nvmrc
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+die () { echo "$@" ; cleanup ; exit 1; }
+
+cleanup() {
+  echo 'cleaned up'
+}
+
+\. ../../../nvm.sh
+
+\. ../../common.sh
+
+for f in ../../../test/fixtures/nvmrc/test/fixtures/valid/*; do
+  STDOUT="$(nvm_process_nvmrc $f/.nvmrc 2>/dev/null)"
+  EXIT_CODE="$(nvm_process_nvmrc $f/.nvmrc >/dev/null 2>/dev/null; echo $?)"
+
+  EXPECTED="$(extract_value node "$(parse_json "$(cat "$f/expected.json")")")"
+
+  [ "${EXIT_CODE}" = "0" ] || die "$(basename "${f}"): expected exit code of 0 but got ${EXIT_CODE}"
+
+  [ "${STDOUT}" = "${EXPECTED}" ] || die "$(basename "${f}"): expected STDOUT of \`${EXPECTED}\` but got \`${STDOUT}\`"
+done
+
+for f in ../../../test/fixtures/nvmrc/test/fixtures/invalid/*; do
+  STDOUT="$(nvm_process_nvmrc $f/.nvmrc 2>/dev/null)"
+  STDERR="$(nvm_process_nvmrc $f/.nvmrc 2>&1 >/dev/null | awk '{if(NR > 8) print $0}' | strip_colors)"
+  EXIT_CODE="$(nvm_process_nvmrc $f/.nvmrc >/dev/null 2>/dev/null; echo $?)"
+
+  EXPECTED="$(parse_json "$(cat "$f/expected.json")" | sed 's/^[0-9]*="//;s/"$//')"
+
+  [ "${EXIT_CODE}" != "0" ] || die "$(basename "${f}"): expected exit code of 'not 0' but got ${EXIT_CODE}"
+
+  [ "${STDERR}" = "${EXPECTED}" ] || die "$(basename "${f}"): expected STDERR of \`${EXPECTED}\` but got \`${STDERR}\`"
+done
diff --git a/test/fixtures/nvmrc b/test/fixtures/nvmrc