fix(rustfmt): remove edition in default arguments

[zmtq05]

The Rust 2021 edition has been released. So I think it would be better to use 2021 edition for rustfmt.

related: rust-lang/rustfmt#4847

[syphar]

Shouldn't the correct thing be to omit the edition here?

From the docs it looks like the edition is read from the projects Cargo.toml when formatting is run through cargo fmt.

[zmtq05]

Yes. I don't think many people use rust without cargo.

cargo fmt format all files in workspace.
I think rustfmt is more suitable when formatting single file. and it's default value is 2015.

What do you think?

[syphar]

valid point... I'm not sure how to solve it best.

We can keep it just using 2021 for now.

Perhaps I'll try to find a way with cargo fmt or reading the toml

[zmtq05]

Here is example code to get edition from Cargo.toml.
I hope this helps.

[jose-elias-alvarez]

I'm not really familiar with Rust or its ecosystem, but we generally target the latest version unless there's a compelling and widespread reason not to update. With that in mind, is this OK to merge?

[syphar]

I'm not really familiar with Rust or its ecosystem, but we generally target the latest version unless there's a compelling and widespread reason not to update.

Rust editions are only for backwards incompatible changes, other than the rust stable versions. For example introducing new keywords lead to a new edition (like for 2018, and 2021)

You totally can have 2015-edition crates used as a dependency for 2021-edition crates. So while many packages just upgrade the editions, because it's easy, it might be more work for some.

With that in mind, is this OK to merge?

Not all changes in an edition also change formatting, also it's definitely not worse than before :)

This is the PR that added 2021 support to rustfmt: rust-lang/rustfmt#4618

So it's definitely not worse than before, but also not that big of an issue IMHO.

Perhaps I'll give it a shot when something doesn't work :)

[jose-elias-alvarez]

I see, so this has been merged upstream since January but it's not guaranteed that projects will be using it.

Still, I think it makes sense to merge this change here as long as there's a workaround for users on older versions (even though that's not exactly what's going on here, the same idea applies). Users could either modify the arguments directly or try to set the edition dynamically with something like this (totally untested):

local rustfmt = null_ls.builtins.formatting.rustfmt.with({
    args = function(params)
        -- you'd probably want to cache this to avoid having to scan Cargo.toml on every run
        local default_args = { "--emit=stdout", "--edition=2015" } -- or whatever you want to default to

        local cargo_toml = params.root .. "/" .. "Cargo.toml"
        local fd = vim.loop.fs_open(cargo_toml, "r", 438)
        if not fd then
            return default_args
        end

        local stat = vim.loop.fs_fstat(fd)
        local data = vim.loop.fs_read(fd, stat.size, 0)
        vim.loop.fs_close(fd)
        for _, line in ipairs(vim.split(data, "\n")) do
            local edition = line:match("edition-regex-goes-here")
            if edition then
                return { "--emit=stdout", "--edition=" .. edition }
            end
        end

        return default_args
    end,
})

Edit: If the above snippet makes sense and if someone wants to test it out and make it actually work, I'd be happy to add it to the documentation.

[syphar]

I can check out some options only in the next days, IMHO this looks more complex than it should be. Perhaps there is an easier way.

Another thought:
you can also define the edition in a project-specific rustfmt.toml or .rustfmt.toml, which many projects already have.

I need to check which takes precedence between config and argument, and which edition is default, but perhaps we only have to add a hint about us using 2021, while every can just add different editions in the custom rustfmt.toml.

[jose-elias-alvarez]

If there's a dedicated config file and the formatter will automatically pick up on it, that definitely sounds like the way to go.

[zmtq05]

Another thought: you can also define the edition in a project-specific rustfmt.toml or .rustfmt.toml, which many projects already have.

I love this.

What do you think about removing --edition part from default, then add script that read edition from Cargo.toml (or rustfmt.toml if arguments is precedence) to documentation?

[syphar]

I believe we still should have a sensible default, and IMHO that's 2021. So removing the argument is only correct when the rustfmt default is 2021.

Question is if rustfmt still uses an edition config in rustfmt.toml. For reading rustfmt.toml there is also no code required since that's builtin into rustfmt

[zmtq05]

Question is if rustfmt still uses an edition config in rustfmt.toml. For reading rustfmt.toml there is also no code required since that's builtin into rustfmt

Yes. Here is my output.

$ cat rustfmt.toml
edition = "2021"

$ rustfmt --print-config=current src/main.rs | rg edition
edition = "2021"

without grep

$ rustfmt --print-config=current src/main.rs
max_width = 100
hard_tabs = false
tab_spaces = 4
newline_style = "Auto"
indent_style = "Block"
use_small_heuristics = "Default"
fn_call_width = 60
attr_fn_like_width = 70
struct_lit_width = 18
struct_variant_width = 35
array_width = 60
chain_width = 60
single_line_if_else_max_width = 50
wrap_comments = false
format_code_in_doc_comments = false
comment_width = 80
normalize_comments = false
normalize_doc_attributes = false
license_template_path = ""
format_strings = false
format_macro_matchers = false
format_macro_bodies = true
empty_item_single_line = true
struct_lit_single_line = true
fn_single_line = false
where_single_line = false
imports_indent = "Block"
imports_layout = "Mixed"
imports_granularity = "Preserve"
group_imports = "Preserve"
reorder_imports = true
reorder_modules = true
reorder_impl_items = false
type_punctuation_density = "Wide"
space_before_colon = false
space_after_colon = true
spaces_around_ranges = false
binop_separator = "Front"
remove_nested_parens = true
combine_control_expr = true
overflow_delimited_expr = false
struct_field_align_threshold = 0
enum_discrim_align_threshold = 0
match_arm_blocks = true
match_arm_leading_pipes = "Never"
force_multiline_blocks = false
fn_args_layout = "Tall"
brace_style = "SameLineWhere"
control_brace_style = "AlwaysSameLine"
trailing_semicolon = true
trailing_comma = "Vertical"
match_block_trailing_comma = false
blank_lines_upper_bound = 1
blank_lines_lower_bound = 0
edition = "2021"
version = "One"
inline_attribute_width = 0
merge_derives = true
use_try_shorthand = false
use_field_init_shorthand = false
force_explicit_abi = true
condense_wildcard_suffixes = false
color = "Auto"
required_version = "1.4.37"
unstable_features = false
disable_all_formatting = false
skip_children = false
hide_parse_errors = false
error_on_line_overflow = false
error_on_unformatted = false
report_todo = "Never"
report_fixme = "Never"
ignore = []
emit_mode = "Files"
make_backup = false

Arguments seem to take precedence (not exact).

$ cat rustfmt.toml
edition = "2021"

$ rustfmt --edition=2018 --print-config=current src/main.rs | rg edition
edition = "2018"

[syphar]

Looking at this, wouldn't the best option be to remove the argument completely, and adding a comment how to globally change the default edition? Probably via global rustfmt config so it can be changed by project?

[jose-elias-alvarez]

Looking at this, wouldn't the best option be to remove the argument completely, and adding a comment how to globally change the default edition? Probably via global rustfmt config so it can be changed by project?

I agree with this, though I wonder why the edition was specified in the first place. I see that #209 fixed a mismatch between the documentation and the actual arguments. What is the behavior when the edition is not specified? Unless it doesn't run at all, it seems best to leave it out and let rustfmt infer it from local / global configuration.

[syphar]

So, I did some short research.

This seems to be an open issue especially with editor-plugins that try to format files. Related issues:

• Consider CLI option for cargo.toml path & pull settings out of cargo.toml rust-lang/rustfmt#4074
• Allow cargo-fmt to handle formatting individual files. rust-lang/rustfmt#5071

From a short scan the culprit is that for most users this is a problem only for editor-plugins. Also some projects already define the edition in a rustfmt.toml, which would be correctly picked up.

The more global cargo fmt would more or less run rustfmt --edition in the background, adding the edition from Cargo.toml.

Tricky thing is still that the default edition is 2015, while in rustfmt 2.0 it will be 2018.

I'm not sure which is the best option here...

1. We drop the explicit edition from args and we write code to read the edition from Cargo.toml, if we find it in the project root. This is probably most consistent with what users expect. Downside: more code to maintain, default for formatting outside of rust project changes.
2. we keep the explicit (then 2021) edition but provide code how to read from Cargo.toml. Downside: confusing behavior in projects that uses older editions. Of course can be solved with providing the best snippet.
3. drop the default, and add documentation how to override the edition via global or project-specific rustfmt config.

[zmtq05]

I think it's good to read Cargo.toml in rustfmt, but I don't think it will be implemented.

In rust-analyzer, it reads Cargo.toml edition when formatting(lua vim.lsp.buf.formatting_sync()).
Run rustfmt with respect to Cargo.toml edition rust-analyzer/rust-analyzer#2477

If you want, you can use 'rust-analyzer' instead of 'null-ls' to format.
How about removing the default and adding a script for those who don't use rust-analyzer?

I used the above script and it seems to work fine.

// src/lib.rs
async fn foo() { 0; }

// format with null-ls
// edition is specified in Cargo.toml
// 2015
async fn foo() { 0; }

// 2018
async fn foo() {
    0;
}

[jose-elias-alvarez]

Alright, so how about we remove the edition in this PR, then add something like this to the documentation:

- To automatically configure `--edition`, see the [wiki](wiki-link-goes-here). 
- Otherwise, `--edition` will default to $WHATEVER_THE_DEFAULT_IS. (2015?)

The wiki link could then describe creating local / global config files or reading the version from Cargo.toml, as discussed here. Since this isn't a Rust-centric project, maintaining these workarounds is out-of-scope, so I'd rather keep them out of the main documentation if possible.

Let me know how that sounds.

[zmtq05]

I agree.

[syphar]

Sounds good to me.
Default edition in the default shipped rustfmt is 2015 currently

I would propose also provide a simple snippet just to override args with a newer version by default. But I'm happy to add that if needed.

[zmtq05]

https://github.com/jose-elias-alvarez/null-ls.nvim/blob/main/doc/BUILTINS.md#arguments

null_ls.builtins.formatting.rustfmt.with({
  extra_args = { "--edition=2021" }

  -- or

  extra_args = function(param)
    -- read Cargo.toml
    return { "--edition="..EDITION_FROM_CARGO }
  end
})

This may help if remove the edition from the default.

[jose-elias-alvarez]

Alright, I added a link to a new wiki page that I set up to document workarounds for specific sources. Feel free to add the information there. Thanks again for the thorough review / discussion!

[zmtq05]

I cannot edit wiki page.

[jose-elias-alvarez]

@zmtq05 my bad, should be open now!