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

Alternative approach for constructing the AST and mod/file mapping #3930

Open
calebcartwright opened this issue Nov 16, 2019 · 1 comment
Open
Assignees
Labels
a-mods Module resolution.

Comments

@calebcartwright
Copy link
Member

There's been a couple issues of late with syntax/parse errors stemming from files that rustfmt was configured to ignore. The underlying cause has been traced to how the complete AST is constructed for the crate up front and the mod nodes in the AST are then associated to the corresponding files.

Based on discussion in some other threads, it should be possible to construct the AST and create the file/mod mapping via a different approach that addresses the issues encountered currently (and also avoids having to parse and construct an AST for files that rustfmt is just going to ignore anyway).

There's a lot more detail from @topecongiro on the underlying cause with the current approach and the new approach in #3920 (comment)

Mark-Simulacrum added a commit to anp/rust that referenced this issue Dec 22, 2019
This replaces cargo-fmt with rustfmt with --skip-children which should
allow us to format code without running into rust-lang/rustfmt#3930.

This also bumps up the version of rustfmt used to a more recent one.
@calebcartwright
Copy link
Member Author

One challenge we may run into with using the ignore list to conditionally construct the AST/file mapping for a crate would be rustfmt potentially ignoring unintentional/unanticipated files that were only imported in an ignored file.

For example:

lib.rs

mod foo;

foo.rs

#[path = "bar.rs"]
mod bar;

fn abc() {
    bar::def();
}

bar.rs

fn def() {
println!("foo bar");
}

and a rustfmt.toml config:

ignore = [ "foo.rs" ]

With this example, I believe the new approach for constructing the ast/file mapping with the current ignore setting would result in rustmt ignoring both bar.rs and foo.rs which I suspect could be confusing/surprising for some users.

That has me wondering if perhaps we should introduce a new config option to allow users to differentiate between files that rustfmt should ignore entirely (i.e. not even try to parse) vs. files that rustfmt can still parse (to be able to discover other mods/files that are imported) but should not format.

So with the above example, if the rustfmt config file was using the theoretical new config option:

parser-ignore = [ "foo.rs" ]

then neither foo.rs nor bar.rs would be formatted but with

ignore = [ "foo.rs" ]

bar.rs would still be formatted

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-mods Module resolution.
Projects
None yet
Development

No branches or pull requests

3 participants