dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.57k stars 1.44k forks source link

`shfmt` options on `.editorconfig` not being applied #4233

Closed hbarcelos closed 2 years ago

hbarcelos commented 2 years ago

Information

VIM version

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Jun 07 2022 00:51:41)
Included fixes: 1-5046

Operating System: Linux - Manjaro

What went wrong

shfmt added support for configuration through .editorconfig a while ago. The problem is that if I run :ALEFix shfmt in any supported file type, it apparently ignores any config set in .editorconfig and simply sticks with the default.

Reproducing the bug

Create an .editorconfig file like:

# top-most EditorConfig file
root = true

[*.sh]
switch_case_indent = true

Starting with the following file:

while getopts_long "$optspec" OPT; do
    case "$OPT" in
    'h' | 'help')
        echo -e "$(usage)"
        exit 0
        ;;
done

If I run shfmt standalone (shfmt -w script.sh), I get:

while getopts_long "$optspec" OPT; do
    case "$OPT" in
        'h' | 'help')
            echo -e "$(usage)"
            exit 0
            ;;
done

If I run :ALEFix shfmt, I get:

while getopts_long "$optspec" OPT; do
    case "$OPT" in
    'h' | 'help')
        echo -e "$(usage)"
        exit 0
        ;;
done

:ALEInfo

 Current Filetype: sh
Available Linters: ['bashate', 'cspell', 'language_server', 'shell', 'shellcheck']
  Enabled Linters: ['bashate', 'cspell', 'language_server', 'shell', 'shellcheck']
  Ignored Linters: []
 Suggested Fixers:
  'remove_trailing_lines' - Remove all blank lines at the end of a file.
  'shfmt' - Fix sh files with shfmt.
  'trim_whitespace' - Remove all trailing whitespace characters at the end of every line.
 Linter Variables:

let g:ale_sh_bashate_executable = 'bashate'
let g:ale_sh_bashate_options = ''
let g:ale_sh_language_server_executable = 'bash-language-server'
let g:ale_sh_language_server_use_global = 0
let g:ale_sh_shell_default_shell = 'zsh'
let g:ale_sh_shellcheck_change_directory = 1
let g:ale_sh_shellcheck_dialect = 'auto'
let g:ale_sh_shellcheck_exclusions = ''
let g:ale_sh_shellcheck_executable = 'shellcheck'
let g:ale_sh_shellcheck_options = ''
let g:ale_sh_shfmt_executable = 'shfmt'
let g:ale_sh_shfmt_options = ''
 Global Variables:

let g:ale_cache_executable_check_failures = v:null
let g:ale_change_sign_column_color = 0
let g:ale_command_wrapper = ''
let g:ale_completion_delay = v:null
let g:ale_completion_enabled = 0
let g:ale_completion_max_suggestions = v:null
let g:ale_disable_lsp = 0
let g:ale_echo_cursor = 1
let g:ale_echo_msg_error_str = 'Error'
let g:ale_echo_msg_format = '%code: %%s'
let g:ale_echo_msg_info_str = 'Info'
let g:ale_echo_msg_warning_str = 'Warning'
let g:ale_enabled = 1
let g:ale_fix_on_save = 0
let g:ale_fixers = {'less': ['prettier', 'stylelint'], 'scss': ['prettier', 'stylelint'], 'solidity': ['prettier'], 'javascriptreact': ['prettier', 'eslint', 'prettier-eslint'], 'sh': ['shfmt'], 'html': ['prettier'], 'typescript': ['prettier', 'eslint', 'prettier-eslint'], 'markdown': ['prettier'], 'vue': ['prettier', 'eslint', 'prettier-eslint'], 'bash': ['shfmt'], 'python': ['autopep8', 'yapf'], 'c': ['clang-format'], 'xml': ['prettier', 'xmllint'], 'typescriptreact': ['prettier', 'eslint', 'prettier-eslint'], 'zsh': ['shfmt'], 'toml': ['prettier'], 'javascript': ['prettier', 'eslint', 'prettier-eslint'], 'svg': ['prettier'], 'jsonc': ['jq', 'prettier'], 'json': ['jq', 'prettier'], 'cpp': ['clang-format'], 'css': ['prettier', 'stylelint']}
let g:ale_history_enabled = 1
let g:ale_history_log_output = 1
let g:ale_keep_list_window_open = 0
let g:ale_lint_delay = 200
let g:ale_lint_on_enter = 1
let g:ale_lint_on_filetype_changed = 1
let g:ale_lint_on_insert_leave = 1
let g:ale_lint_on_save = 1
let g:ale_lint_on_text_changed = 'normal'
let g:ale_linter_aliases = {}
let g:ale_linters = {'less': ['stylelint'], 'scss': ['stylelint'], 'solidity': ['solhint', 'solium'], 'javascriptreact': ['prettier', 'eslint', 'prettier-eslint'], 'html': ['prettier'], 'typescript': ['prettier', 'eslint', 'prettier-eslint', 'tsserver'], 'vue': ['prettier', 'eslint', 'prettier-eslint'], 'python': ['flake8', 'pylint'], 'vim': ['vint'], 'c': ['clang'], 'xml': ['xmllint'], 'typescriptreact': ['prettier', 'eslint', 'prettier-eslint', 'tsserver'], 'javascript': ['prettier', 'eslint', 'prettier-eslint'], 'svg': ['prettier'], 'jsonc': ['jq', 'jsonlint'], 'json': ['jq', 'jsonlint'], 'cpp': ['clang++', 'g++'], 'css': ['stylelint']}
let g:ale_linters_explicit = 0
let g:ale_linters_ignore = {}
let g:ale_list_vertical = 0
let g:ale_list_window_size = 10
let g:ale_loclist_msg_format = '%code: %%s'
let g:ale_max_buffer_history_size = 20
let g:ale_max_signs = -1
let g:ale_maximum_file_size = v:null
let g:ale_open_list = 0
let g:ale_pattern_options = v:null
let g:ale_pattern_options_enabled = v:null
let g:ale_root = {}
let g:ale_set_balloons = 0
let g:ale_set_highlights = 1
let g:ale_set_loclist = 1
let g:ale_set_quickfix = 0
let g:ale_set_signs = 1
let g:ale_sign_column_always = 0
let g:ale_sign_error = ''
let g:ale_sign_info = ''
let g:ale_sign_offset = 1000000
let g:ale_sign_style_error = ''
let g:ale_sign_style_warning = ''
let g:ale_sign_warning = ''
let g:ale_sign_highlight_linenrs = 0
let g:ale_statusline_format = v:null
let g:ale_type_map = {}
let g:ale_use_global_executables = v:null
let g:ale_virtualtext_cursor = 0
let g:ale_warn_about_trailing_blank_lines = 1
let g:ale_warn_about_trailing_whitespace = 1
  Command History:

(executable check - failure) cspell
(executable check - failure) bash-language-server
(finished - exit code 0) ['/bin/zsh', '-c', 'bash -n ''/tmp/vZ4cRfE/77/script.sh''']

<<<NO OUTPUT RETURNED>>>

(executable check - failure) shellcheck
(finished - exit code 0) ['/bin/zsh', '-c', '''shfmt'' -i 2 < ''/tmp/vZ4cRfE/78/script.sh''']
(executable check - failure) bashate
(executable check - failure) cspell
(executable check - failure) bash-language-server
(finished - exit code 0) ['/bin/zsh', '-c', 'bash -n ''/tmp/vZ4cRfE/79/script.sh''']

<<<NO OUTPUT RETURNED>>>

(executable check - failure) shellcheck
(executable check - failure) bashate
(executable check - failure) cspell
(executable check - failure) bash-language-server
(finished - exit code 0) ['/bin/zsh', '-c', 'bash -n ''/tmp/vZ4cRfE/80/script.sh''']

<<<NO OUTPUT RETURNED>>>

(executable check - failure) shellcheck
(executable check - failure) bashate
(executable check - failure) cspell
(executable check - failure) bash-language-server
(finished - exit code 0) ['/bin/zsh', '-c', 'bash -n ''/tmp/vZ4cRfE/90/script.sh''']

<<<NO OUTPUT RETURNED>>>

(executable check - failure) shellcheck
hsanson commented 2 years ago

From the ALEInfo I see ALE uses this command:

shfmt -i 2 < /path/to/script.sh

That is different from what you use stand alone:

shfmt -w /path/to/script.sh

Not familiar with shfmt but maybe the .editorconfig won't work if the linter is invoked using stdin as input? or if the -w option is not used?

If we can figure out what command ALE needs to execute to get the functionality you need then it should be easy to modify the fixer to use that command.

hbarcelos commented 2 years ago

So it looks like shfmt will ignore .editorconfig if any command line arguments are used.

Can we drop the -i 2 flag from the fixer invocation? If we do so, then if there's no .editorconfig, it will simply use whatever the default is for shfmt. Users willing to customize it can simply create the file.

hsanson commented 2 years ago

Should be fairly easy to do modifying the fixer itself: autoload/ale/fixers/shfmt.vim. Feel free to submit an MR with the modifications.

hbarcelos commented 2 years ago

Apparently there is an incompatibility between ALE and shfmt.

AFAIK, ALE copies the buffer contents to a temp file, formats it and replace the current buffer with the result.

If ALE is running the following command:

shfmt < /tmp/vZ4cRfE/78/script.sh

It won't work well with .editorconfig because shfmt is not aware of $PWD, they use the path of the file passed as parameter to recursively look for the config file.

This gives us 2 problems:

  1. Since we're using stdin, there's no file path to inspect to look for the config.
  2. Even if we don't use stdin, the file path would be in /tmp/<some_dir>/... and there wouldn't be any config files there.

From this we have 2 options:

  1. Copy the .editorconfig to the same temp directory as the file being formatted.
  2. Use the original file path.

Option 1 seems pretty clunky, but I'm not sure option 2 is feasible with the current design.

I'd appreciate any guidance here.

hsanson commented 2 years ago

You can try adding %s to the fixer command. ALE will replace that placeholder with the full path of the file being fixed (see :h ale-command-format-strings).

Another option is if shfmt supports some command line argument to explicitly set the configuration (e.g. -w .editorconfig) then you can try using ale#path#FindNearestFile() function to find the nearest .editorconfig file to the current buffer and build the fixer command so it explicitly sets it.

For better results do both, set explictly the config file if found and use %s to set full path of the file being fixed.

Check rubocop.vim for example using %s and astyle.vim for an example of finding configuration file.

hbarcelos commented 2 years ago

You can try adding %s to the fixer command. ALE will replace that placeholder with the full path of the file being fixed (see :h ale-command-format-strings).

From what I hear from shfmt author, that's the only viable option.

Will work on a MR. Thanks for the pointers.