search

R-nvim / R.nvim

Neovim plugin to edit R files
GNU General Public License v3.0
155 stars 16 forks source link

✨ (R.nvim): add feature to convert numbers to explicit integers #153

Closed PMassicotte closed 3 months ago

PMassicotte commented 3 months ago

This PR introduces functionality for converting numbers to explicit integers in the current buffer. This feature is aimed at satisfying linters that recommend explicit integer usage, as detailed here: Implicit Integer Linter.

Key Features:

Maybe the default should be to true?

Documentation

The documentation has been updated to reflect these changes.

jalvesaq commented 3 months ago

Good job!

Do you plan to add other formatting features in the future? If yes, instead of RNumFormat, perhaps the map label could be RFormatNum and the map labels of future formatting features would also start with RFormat too.

Where do the diagnostics in virtual text come from? Are they from the languageserver? If yes, I imagine some people would expect to fix them by calling vim.lsp.buf.code_action().

PMassicotte commented 3 months ago

Do you plan to add other formatting features in the future? If yes, instead of RNumFormat, perhaps the map label could be RFormatNum and the map labels of future formatting features would also start with RFormat too.

Good idea, I will change it, so it will be easier to add more features as you suggest.

Where do the diagnostics in virtual text come from? Are they from the languageserver?

Yes, from the LSP, using a .lintr file globally on in the project directory.

If yes, I imagine some people would expect to fix them by calling vim.lsp.buf.code_action().

This is a good suggestion. It looks like it is supported by languageserver (https://github.com/REditorSupport/languageserver/tree/9b300181f2588462cb5a6f7ecd55fcbeb083d028?tab=readme-ov-file#services-implemented). I never tried to implement such service (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction), but I can try. If you have any idea where to start, please let me know.

PMassicotte commented 3 months ago

Looks like the only option/action provided is to add nolint (https://github.com/REditorSupport/languageserver/blob/9b300181f2588462cb5a6f7ecd55fcbeb083d028/R/code_action.R#L52). Can we implement an answer/action to take on our side or it must be implemented within r languageserver?

jalvesaq commented 3 months ago

I didn't investigate the possibilities, but I guess that we can use vim.lsp.buf.code_action() only if the diagnostics come from a language server. The options, then, become: contribute to the existing R's languageserver package, create our own language server, or forget about vim.lsp.buf.code_action().

The combination of nvimcom and rnvimserver already works like a language server (for completion of code, displaying documentation, formatting selected code), but it also has features that aren't in the language server protocol (Object Browser, opening the Examples section of documentation as a normal R script). The communication between nvimcom+rnvimserver and Neovim doesn't follow the language server protocol.

We use cache files to do completion with no noticeable delay and we can complete objects from the .GlobalEnv. As far as I know, the languageserver team doesn't want to use cache files, and completion of .GlobalEnv objects isn't a goal.

In summary, I think that, ideally, we should avoid duplication of efforts in the free software community and collaborate with the languageserver project when the goals overlap. However, it's more frequently easier to create something anew than adapt to an existing project.

Can we implement an answer/action to take on our side

It was a lot of work to convert Nvim-R's VimScript into R.nvim's Lua. I will not try to convert part of R.nvim into a language server. So, my answer is: Yes, it's possible. But I can't work on this because it would require rewriting most of nvimcom and nclientserver and I don't have time for such a big change.

or it must be implemented within r languageserver ?

It would be more under users' expectations on how diagnostics are displayed and fixed, but you have already developed the feature to R.nvim and it is ready to be merged. I think that we can simply merge your pull request. R.nvim is free software and anyone can adapt the code to the languageserver.

Maybe I should not have made my initial comment...

PMassicotte commented 3 months ago

I agree with all you said. R.nvim works just perfectly as it is now. We can add more goodies as we go ;)

PMassicotte commented 3 months ago

I have made the changes. Maybe we could eventually document on how to have the lintr working. 

  lintr.linter_file = "~/.lintr",
linters: linters_with_defaults(
  T_and_F_symbol_linter(),
  absolute_path_linter(lax = TRUE),
  any_duplicated_linter(),
  any_is_na_linter(),
  backport_linter(),
  boolean_arithmetic_linter(),
  class_equals_linter(),
  commented_code_linter(),
  condition_message_linter(),
  conjunct_test_linter(),
  consecutive_assertion_linter(),
  cyclocomp_linter(),
  cyclocomp_linter(50),
  duplicate_argument_linter(),
  empty_assignment_linter(),
  expect_comparison_linter(),
  expect_length_linter(),
  expect_named_linter(),
  expect_not_linter(),
  expect_null_linter(),
  expect_s3_class_linter(),
  expect_s4_class_linter(),
  expect_true_false_linter(),
  expect_type_linter(),
  extraction_operator_linter(),
  fixed_regex_linter(),
  for_loop_index_linter(),
  function_argument_linter(),
  function_return_linter(),
  ifelse_censor_linter(),
  implicit_assignment_linter(),
  implicit_integer_linter(allow_colon = TRUE),
  inner_combine_linter(),
  is_numeric_linter(),
  length_levels_linter(),
  lengths_linter(),
  library_call_linter(),
  line_length_linter(150),
  literal_coercion_linter(),
  missing_argument_linter(),
  missing_package_linter(),
  namespace_linter(),
  nested_ifelse_linter(),
  nonportable_path_linter(),
  nonportable_path_linter(lax = TRUE),
  numeric_leading_zero_linter(),
  object_length_linter(30),
  object_usage_linter = NULL,
  outer_negation_linter(),
  package_hooks_linter(),
  paste_linter(),
  pipe_call_linter(),
  redundant_equals_linter(),
  redundant_ifelse_linter(),
  regex_subset_linter(),
  routine_registration_linter(),
  scalar_in_linter(),
  semicolon_linter(allow_compound = TRUE),
  seq_linter(),
  sort_linter(),
  sprintf_linter(),
  string_boundary_linter(),
  strings_as_factors_linter(),
  system_file_linter(),
  undesirable_function_linter(all_undesirable_functions),
  undesirable_operator_linter(all_undesirable_operators),
  unnecessary_concatenation_linter(),
  unnecessary_lambda_linter(),
  unnecessary_nested_if_linter(),
  unnecessary_placeholder_linter(),
  unreachable_code_linter(),
  vector_logic_linter(),
  yoda_test_linter(),
  undesirable_function_linter(
    fun = c(
      # Base messaging
      "message" = "use cli::cli_inform()",
      "warning" = "use cli::cli_warn()",
      "stop" = "use cli::cli_abort()",
      # rlang messaging
      "inform" = "use cli::cli_inform()",
      "warn" = "use cli::cli_warn()",
      "abort" = "use cli::cli_abort()",
      # older cli
      "cli_alert_danger" = "use cli::cli_inform()",
      "cli_alert_info" = "use cli::cli_inform()",
      "cli_alert_success" = "use cli::cli_inform()",
      "cli_alert_warning" = "use cli::cli_inform()",
      # fs
      "file.path" = "use path()",
      "dir" = "use dir_ls()",
      "dir.create" = "use dir_create()",
      "file.copy" = "use file_copy()",
      "file.create" = "use file_create()",
      "file.exists" = "use file_exists()",
      "file.info" = "use file_info()",
      "normalizePath" = "use path_real()",
      "unlink" = "use file_delete()",
      "basename" = "use path_file()",
      "dirname" = "use path_dir()",
      # i/o
      "readLines" = "use read_lines()",
      "writeLines" = "use write_lines()"
    ),
    symbol_is_undesirable = FALSE
  )
  )

vim.g.LanguageClient_serverCommands = {
    r = { "R", "--slave", "-e", "languageserver::run()" },
 }