eslint: Use cwd from executable location to fix nested projects (#3222)

* Split FindNearestExecutable from FindExecutable

The path searching in ale#node#FindExecutable() will be useful for
eslint.  Refactor it into a separate function so it can be used without
regard for the state of the _use_global and _executable variables.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

* eslint: Set project root from local executable

Using the nearest directory with node_modules does not work correctly
for nested projects where the eslint dependencies are in the outer
project.  For example:
https://github.com/dense-analysis/ale/issues/3143#issuecomment-652452362

Adopt the behavior of SublimeLinter, which runs from project_root
determined by the presence of the eslint executable in node_modules/.bin
(or eslint in dependencies/devDependencies of package.json, which we can
add later as necessary).  See [NodeLinter#find_local_executable].

[NodeLinter#find_local_executable]: https://github.com/SublimeLinter/SublimeLinter/blob/056e6f6/lint/base_linter/node_linter.py#L109

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

autoload/ale/fixers/eslint.vim

@@ -53,7 +53,8 @@ function! ale#fixers#eslint#ApplyFixForVersion(buffer, version) abort
     " Use --fix-to-stdout with eslint_d
     if l:executable =~# 'eslint_d$' && ale#semver#GTE(a:version, [3, 19, 0])
         return {
-        \   'command': ale#node#Executable(a:buffer, l:executable)
+        \   'command': ale#handlers#eslint#GetCdString(a:buffer)
+        \       . ale#node#Executable(a:buffer, l:executable)
         \       . ale#Pad(l:options)
         \       . ' --stdin-filename %s --stdin --fix-to-stdout',
         \   'process_with': 'ale#fixers#eslint#ProcessEslintDOutput',
@@ -63,7 +64,8 @@ function! ale#fixers#eslint#ApplyFixForVersion(buffer, version) abort
     " 4.9.0 is the first version with --fix-dry-run
     if ale#semver#GTE(a:version, [4, 9, 0])
         return {
-        \   'command': ale#node#Executable(a:buffer, l:executable)
+        \   'command': ale#handlers#eslint#GetCdString(a:buffer)
+        \       . ale#node#Executable(a:buffer, l:executable)
         \       . ale#Pad(l:options)
         \       . ' --stdin-filename %s --stdin --fix-dry-run --format=json',
         \   'process_with': 'ale#fixers#eslint#ProcessFixDryRunOutput',
@@ -71,7 +73,8 @@ function! ale#fixers#eslint#ApplyFixForVersion(buffer, version) abort
     endif
 
     return {
-    \   'command': ale#node#Executable(a:buffer, l:executable)
+    \   'command': ale#handlers#eslint#GetCdString(a:buffer)
+    \       . ale#node#Executable(a:buffer, l:executable)
     \       . ale#Pad(l:options)
     \       . (!empty(l:config) ? ' -c ' . ale#Escape(l:config) : '')
     \       . ' --fix %t',

autoload/ale/handlers/eslint.vim

@@ -1,6 +1,11 @@
 " Author: w0rp <devw0rp@gmail.com>
 " Description: Functions for working with eslint, for checking or fixing files.
 
+let s:executables = [
+\   'node_modules/.bin/eslint_d',
+\   'node_modules/eslint/bin/eslint.js',
+\   'node_modules/.bin/eslint',
+\]
 let s:sep = has('win32') ? '\' : '/'
 
 call ale#Set('javascript_eslint_options', '')
@@ -30,11 +35,31 @@ function! ale#handlers#eslint#FindConfig(buffer) abort
 endfunction
 
 function! ale#handlers#eslint#GetExecutable(buffer) abort
-    return ale#node#FindExecutable(a:buffer, 'javascript_eslint', [
-    \   'node_modules/.bin/eslint_d',
-    \   'node_modules/eslint/bin/eslint.js',
-    \   'node_modules/.bin/eslint',
-    \])
+    return ale#node#FindExecutable(a:buffer, 'javascript_eslint', s:executables)
+endfunction
+
+" Given a buffer, return a command prefix string which changes directory
+" as necessary for running ESLint.
+function! ale#handlers#eslint#GetCdString(buffer) abort
+    " ESLint 6 loads plugins/configs/parsers from the project root
+    " By default, the project root is simply the CWD of the running process.
+    " https://github.com/eslint/rfcs/blob/master/designs/2018-simplified-package-loading/README.md
+    " https://github.com/dense-analysis/ale/issues/2787
+    "
+    " If eslint is installed in a directory which contains the buffer, assume
+    " it is the ESLint project root.  Otherwise, use nearest node_modules.
+    " Note: If node_modules not present yet, can't load local deps anyway.
+    let l:executable = ale#node#FindNearestExecutable(a:buffer, s:executables)
+
+    if !empty(l:executable)
+        let l:nmi = strridx(l:executable, 'node_modules')
+        let l:project_dir = l:executable[0:l:nmi - 2]
+    else
+        let l:modules_dir = ale#path#FindNearestDirectory(a:buffer, 'node_modules')
+        let l:project_dir = !empty(l:modules_dir) ? fnamemodify(l:modules_dir, ':h:h') : ''
+    endif
+
+    return !empty(l:project_dir) ? ale#path#CdString(l:project_dir) : ''
 endfunction
 
 function! ale#handlers#eslint#GetCommand(buffer) abort
@@ -42,17 +67,7 @@ function! ale#handlers#eslint#GetCommand(buffer) abort
 
     let l:options = ale#Var(a:buffer, 'javascript_eslint_options')
 
-    " ESLint 6 loads plugins/configs/parsers from the project root
-    " By default, the project root is simply the CWD of the running process.
-    " https://github.com/eslint/rfcs/blob/master/designs/2018-simplified-package-loading/README.md
-    " https://github.com/dense-analysis/ale/issues/2787
-    " Identify project root from presence of node_modules dir.
-    " Note: If node_modules not present yet, can't load local deps anyway.
-    let l:modules_dir = ale#path#FindNearestDirectory(a:buffer, 'node_modules')
-    let l:project_dir = !empty(l:modules_dir) ? fnamemodify(l:modules_dir, ':h:h') : ''
-    let l:cd_command = !empty(l:project_dir) ? ale#path#CdString(l:project_dir) : ''
-
-    return l:cd_command
+    return ale#handlers#eslint#GetCdString(a:buffer)
     \   . ale#node#Executable(a:buffer, l:executable)
     \   . (!empty(l:options) ? ' ' . l:options : '')
     \   . ' -f json --stdin --stdin-filename %s'

autoload/ale/node.vim

@@ -12,6 +12,18 @@ function! ale#node#FindExecutable(buffer, base_var_name, path_list) abort
         return ale#Var(a:buffer, a:base_var_name . '_executable')
     endif
 
+    let l:nearest = ale#node#FindNearestExecutable(a:buffer, a:path_list)
+
+    if !empty(l:nearest)
+        return l:nearest
+    endif
+
+    return ale#Var(a:buffer, a:base_var_name . '_executable')
+endfunction
+
+" Given a buffer number, a base variable name, and a list of paths to search
+" for in ancestor directories, detect the executable path for a Node program.
+function! ale#node#FindNearestExecutable(buffer, path_list) abort
     for l:path in a:path_list
         let l:executable = ale#path#FindNearestFile(a:buffer, l:path)
 
@@ -20,7 +32,7 @@ function! ale#node#FindExecutable(buffer, base_var_name, path_list) abort
         endif
     endfor
 
-    return ale#Var(a:buffer, a:base_var_name . '_executable')
+    return ''
 endfunction
 
 " Create a executable string which executes a Node.js script command with a

test/fixers/test_eslint_fixer_callback.vader

@@ -13,7 +13,9 @@ Execute(The executable path should be correct):
   AssertFixer
   \ {
   \   'read_temporary_file': 1,
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
   \     . ' --fix %t',
@@ -150,7 +152,9 @@ Execute(The lower priority configuration file in a nested directory should be pr
   AssertFixer
   \ {
   \   'read_temporary_file': 1,
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/subdir-with-config/.eslintrc'))
   \     . ' --fix %t',
@@ -164,7 +168,9 @@ Execute(--config in options should override configuration file detection for old
   AssertFixer
   \ {
   \   'read_temporary_file': 1,
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' --config /foo.cfg'
   \     . ' --fix %t',
@@ -175,7 +181,9 @@ Execute(--config in options should override configuration file detection for old
   AssertFixer
   \ {
   \   'read_temporary_file': 1,
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' -c /foo.cfg'
   \     . ' --fix %t',
@@ -187,7 +195,9 @@ Execute(package.json should be used as a last resort):
   AssertFixer
   \ {
   \   'read_temporary_file': 1,
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/.eslintrc.js'))
   \     . ' --fix %t',
@@ -199,7 +209,8 @@ Execute(package.json should be used as a last resort):
   \ {
   \   'read_temporary_file': 1,
   \   'command':
-  \     ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/node_modules/.bin/eslint'))
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files'))
+  \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/node_modules/.bin/eslint'))
   \     . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/package.json'))
   \     . ' --fix %t',
   \ }
@@ -214,7 +225,9 @@ Execute(The version check should be correct):
   \   . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \   . ' --version',
   \ {
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' --stdin-filename %s --stdin --fix-dry-run --format=json',
   \   'process_with': 'ale#fixers#eslint#ProcessFixDryRunOutput',
@@ -223,7 +236,9 @@ Execute(The version check should be correct):
 
   AssertFixer [
   \ {
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' --stdin-filename %s --stdin --fix-dry-run --format=json',
   \   'process_with': 'ale#fixers#eslint#ProcessFixDryRunOutput',
@@ -236,7 +251,9 @@ Execute(--fix-dry-run should be used for 4.9.0 and up):
   GivenCommandOutput ['4.9.0']
   AssertFixer
   \ {
-  \   'command': (has('win32') ? 'node.exe ' : '')
+  \   'command':
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app'))
+  \     . (has('win32') ? 'node.exe ' : '')
   \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
   \     . ' --stdin-filename %s --stdin --fix-dry-run --format=json',
   \   'process_with': 'ale#fixers#eslint#ProcessFixDryRunOutput',
@@ -249,7 +266,8 @@ Execute(--fix-to-stdout should be used for eslint_d):
   \ {
   \   'read_temporary_file': 1,
   \   'command':
-  \     ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d'))
+  \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
   \     . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/package.json'))
   \     . ' --fix %t',
   \ }
@@ -260,7 +278,8 @@ Execute(--fix-to-stdout should be used for eslint_d):
   AssertFixer
   \ {
   \   'command':
-  \     ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d'))
+  \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
   \     . ' --stdin-filename %s --stdin --fix-to-stdout',
   \   'process_with': 'ale#fixers#eslint#ProcessEslintDOutput',
   \ }
@@ -270,7 +289,8 @@ Execute(--fix-to-stdout should be used for eslint_d):
   AssertFixer
   \ {
   \   'command':
-  \     ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
+  \     ale#path#CdString(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d'))
+  \     . ale#Escape(ale#path#Simplify(g:dir . '/../eslint-test-files/app-with-eslint-d/node_modules/.bin/eslint_d'))
   \     . ' --stdin-filename %s --stdin --fix-to-stdout',
   \   'process_with': 'ale#fixers#eslint#ProcessEslintDOutput',
   \ }

test/test_eslint_executable_detection.vader

@@ -70,6 +70,25 @@ Execute(eslint.js executables should be run with node on Windows):
     \ ale#handlers#eslint#GetCommand(bufnr(''))
   endif
 
+Execute(eslint.js should be run from containing project with eslint):
+  call ale#test#SetFilename('eslint-test-files/react-app/subdir-with-package-json/testfile.js')
+
+  " We have to execute the file with node.
+  if has('win32')
+    AssertEqual
+    \ ale#path#CdString(ale#path#Simplify(g:dir . '/eslint-test-files/react-app'))
+    \   . ale#Escape('node.exe') . ' '
+    \   . ale#Escape(ale#path#Simplify(g:dir . '/eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+    \   . ' -f json --stdin --stdin-filename %s',
+    \ ale#handlers#eslint#GetCommand(bufnr(''))
+  else
+    AssertEqual
+    \ ale#path#CdString(ale#path#Simplify(g:dir . '/eslint-test-files/react-app'))
+    \   . ale#Escape(ale#path#Simplify(g:dir . '/eslint-test-files/react-app/node_modules/eslint/bin/eslint.js'))
+    \   . ' -f json --stdin --stdin-filename %s',
+    \ ale#handlers#eslint#GetCommand(bufnr(''))
+  endif
+
 Execute(eslint.js executables can be run outside project dir):
   " Set filename above eslint-test-files (which contains node_modules)
   call ale#test#SetFilename('testfile.js')