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/eslint cwd strategy #4781

Merged
merged 3 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 1 addition & 25 deletions autoload/ale/handlers/eslint.vim
Original file line number Diff line number Diff line change
Expand Up @@ -45,31 +45,7 @@ endfunction

" Given a buffer, return an appropriate working directory for ESLint.
function! ale#handlers#eslint#GetCwd(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#path#FindNearestExecutable(a:buffer, s:executables)

if !empty(l:executable)
let l:modules_index = strridx(l:executable, 'node_modules')
let l:modules_root = l:modules_index > -1 ? l:executable[0:l:modules_index - 2] : ''

let l:sdks_index = strridx(l:executable, ale#path#Simplify('.yarn/sdks'))
let l:sdks_root = l:sdks_index > -1 ? l:executable[0:l:sdks_index - 2] : ''
else
let l:modules_dir = ale#path#FindNearestDirectory(a:buffer, 'node_modules')
let l:modules_root = !empty(l:modules_dir) ? fnamemodify(l:modules_dir, ':h:h') : ''

let l:sdks_dir = ale#path#FindNearestDirectory(a:buffer, ale#path#Simplify('.yarn/sdks'))
let l:sdks_root = !empty(l:sdks_dir) ? fnamemodify(l:sdks_dir, ':h:h:h') : ''
endif

return strlen(l:modules_root) > strlen(l:sdks_root) ? l:modules_root : l:sdks_root
return ale#path#Dirname(ale#handlers#eslint#FindConfig(a:buffer))
endfunction

function! ale#handlers#eslint#GetCommand(buffer) abort
Expand Down
43 changes: 5 additions & 38 deletions test/fixers/test_eslint_fixer_callback.vader
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ Execute(The lower priority configuration file in a nested directory should be pr
AssertFixer
\ {
\ 'read_temporary_file': 1,
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app'),
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/subdir-with-config'),
\ 'command': (has('win32') ? 'node.exe ' : '')
\ . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/node_modules/eslint/bin/eslint.js'))
\ . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/subdir-with-config/.eslintrc'))
Expand All @@ -182,7 +182,7 @@ Execute(--config in options should override configuration file detection for old
AssertFixer
\ {
\ 'read_temporary_file': 1,
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app'),
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/subdir-with-config'),
\ 'command': (has('win32') ? 'node.exe ' : '')
\ . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/node_modules/eslint/bin/eslint.js'))
\ . ' --config /foo.cfg'
Expand All @@ -194,7 +194,7 @@ Execute(--config in options should override configuration file detection for old
AssertFixer
\ {
\ 'read_temporary_file': 1,
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app'),
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/subdir-with-config'),
\ 'command': (has('win32') ? 'node.exe ' : '')
\ . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/node_modules/eslint/bin/eslint.js'))
\ . ' -c /foo.cfg'
Expand Down Expand Up @@ -235,7 +235,7 @@ Execute(The version check should be correct):
\ . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/node_modules/eslint/bin/eslint.js'))
\ . ' --version',
\ {
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app'),
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/subdir-with-config'),
\ 'command': (has('win32') ? 'node.exe ' : '')
\ . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/node_modules/eslint/bin/eslint.js'))
\ . ' --stdin-filename %s --stdin --fix-dry-run --format=json',
Expand All @@ -245,7 +245,7 @@ Execute(The version check should be correct):

AssertFixer [
\ {
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app'),
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/subdir-with-config'),
\ 'command': (has('win32') ? 'node.exe ' : '')
\ . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/react-app/node_modules/eslint/bin/eslint.js'))
\ . ' --stdin-filename %s --stdin --fix-dry-run --format=json',
Expand All @@ -266,39 +266,6 @@ Execute(--fix-dry-run should be used for 4.9.0 and up):
\ 'process_with': 'ale#fixers#eslint#ProcessFixDryRunOutput',
\ }

Execute(--fix-to-stdout should be used for eslint_d):
call ale#test#SetFilename('../test-files/eslint/app-with-eslint-d/testfile.js')

AssertFixer
\ {
\ 'read_temporary_file': 1,
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d'),
\ 'command': ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d/node_modules/.bin/eslint_d'))
\ . ' -c ' . ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/package.json'))
\ . ' --fix %t',
\ }

" The option should be used when eslint_d is new enough.
" We look at the ESLint version instead of the eslint_d version.
GivenCommandOutput ['v3.19.0 (eslint_d v4.2.0)']
AssertFixer
\ {
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d'),
\ 'command': ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d/node_modules/.bin/eslint_d'))
\ . ' --stdin-filename %s --stdin --fix-to-stdout',
\ 'process_with': 'ale#fixers#eslint#ProcessEslintDOutput',
\ }

" The option should be used for new versions too.
GivenCommandOutput ['4.9.0']
AssertFixer
\ {
\ 'cwd': ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d'),
\ 'command': ale#Escape(ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d/node_modules/.bin/eslint_d'))
\ . ' --stdin-filename %s --stdin --fix-to-stdout',
\ 'process_with': 'ale#fixers#eslint#ProcessEslintDOutput',
\ }

Execute(The --fix-dry-run post-processor should handle JSON output correctly):
AssertEqual
\ [],
Expand Down
7 changes: 0 additions & 7 deletions test/linter/test_eslint.vader
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,6 @@ Execute(use-global should override other app directories):
AssertLinterCwd ale#path#Simplify(g:dir . '/../test-files/eslint')
AssertLinter b:executable, ale#Escape(b:executable) . b:args

Execute(eslint_d should be detected correctly):
call ale#test#SetFilename('../test-files/eslint/app-with-eslint-d/testfile.js')

let b:executable = ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d/node_modules/.bin/eslint_d')
AssertLinterCwd ale#path#Simplify(g:dir . '/../test-files/eslint/app-with-eslint-d')
AssertLinter b:executable, ale#Escape(b:executable) . b:args

Execute(eslint.js executables should be run with node on Windows):
call ale#test#SetFilename('../test-files/eslint/react-app/subdir/testfile.js')

Expand Down