[CHORE] Add rustfmt config file and run formatter

[raunakab]

Overview

The only file that I added was the rustfmt.toml. In that file, I updated the styles on how imports should be ordered ("StdExternalCrate"). Then I ran the formatter. The formatter changed updated all of our rust files to follow this convention.

This PR is obviously huge, but the only changes that are observed are after running cargo fmt on the original code. I have not sneaked anything else into here.

You can verify this via 2 ways:

1. Manually inspecting each file to make sure that only the imports have been rearranged (obviously tiring and cumbersome).
2. Checking out the prior commit, adding the exact same rustfmt.toml file, running cargo fmt, and checking to make sure your formatted code has NO diffs against my commit. (Make sure your rust toolchain is updated to the latest release!).

[samster25]

Since this is machine generated, let's hold off on merging this until we get all the other PRs in to avoid merge conflicts.

[codspeed-hq]

CodSpeed Performance Report

Merging #2807 will improve performances by 56.03%

_{Comparing ronnie/rustfmt (38aa90a) with main (4ddc283)}

Summary

⚡ 1 improvements
✅ 15 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	ronnie/rustfmt	Change
⚡	test_show[100 Small Files]	313.2 ms	200.7 ms	+56.03%

[raunakab]

Note:

The local pre-commit hook runs cargo fmt. cargo fmt only formats files that are in the crate hierarchy. If the file is not in the crate hierarchy, cargo will not format it (even if that file is physically located inside of that crate's src directory).

However, the GitHub action runs pre-commit run --all-files which runs the formatter on all the rust files. This includes files that are not included in the crate hierarchy.

Therefore, there will be a discrepancy between your local pre-commit hook and GitHub's action. This may mean that you will get "formatting failed" errors (e.g., something like fmt hook failed because some files were modified). If that happens, it means that you have some dead rust files in the project which were not formatted locally, but were caught by the GitHub Action.

In that case, please consider removing those files.

This PR has run into that exact issue. The files that I removed were:

Builds still succeeded after the removal of those dead files.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.08%. Comparing base (4ddc283) to head (38aa90a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2807      +/-   ##
==========================================
- Coverage   64.09%   64.08%   -0.01%     
==========================================
  Files        1005     1005              
  Lines      113072   113071       -1     
==========================================
- Hits        72473    72463      -10     
- Misses      40599    40608       +9     

Files with missing lines	Coverage Δ
src/common/arrow-ffi/src/lib.rs	99.23% <ø> (ø)
src/common/daft-config/src/lib.rs	82.14% <ø> (ø)
src/common/daft-config/src/python.rs	70.62% <ø> (ø)
src/common/display/src/mermaid.rs	97.67% <ø> (ø)
src/common/error/src/python.rs	66.66% <ø> (ø)
src/common/file-formats/src/file_format.rs	32.00% <ø> (ø)
src/common/file-formats/src/file_format_config.rs	45.69% <ø> (ø)
src/common/file-formats/src/lib.rs	75.00% <ø> (ø)
src/common/io-config/src/azure.rs	20.51% <ø> (ø)
src/common/io-config/src/config.rs	3.12% <ø> (ø)
... and 278 more

... and 90 files with indirect coverage changes

[kevinzwang]

LGTM! Glad to have enforced import styling in Rust now.

I remember Sammy talking about something along the lines of preserving the original git blame for the formatted lines? Just curious what the conclusion was for that

[raunakab]

LGTM! Glad to have enforced import styling in Rust now.

I remember Sammy talking about something along the lines of preserving the original git blame for the formatted lines? Just curious what the conclusion was for that

@kevinzwang Yup! I actually will need to preserve that. I know of the git command to do that; I just need to find some time to walk through the steps. I also need to add a couple of ignores to external crates that we've brought into version-control (since we don't want to continuously reformat them for every new fresh copy).

[raunakab]

Synced with @samster25 offline. He mentioned that src/arrow2 is a clone of an existing codebase. As such, I configured rustfmt.toml to ignore that entire directory during its formatting.

Therefore, those arrow2 files above that were deleted are now added back. They are not deleted from version control anymore. However, the dead src/daft-csv/src/compression.rs has been deleted, since we wrote that and have primary control over that project.

[samster25]

looks like src/arrow2 still has a huge diff. I thought we said to ignore it

[samster25]

LGTM! Let's make sure this doesn't affect the git blame when we merge!

[raunakab]

LGTM! Let's make sure this doesn't affect the git blame when we merge!

I took a brief look into the suggestions that you made.

This will affect the git blame history (not sure if there's a way around that), but git exposes this feature to include a ".git-blame-ignore-revs" file which git will pick up and modify the way it displays git-blames to the end-user. The file needs to be included in tree for it to work.

We could either check that file into source-control (either in this PR or in a follow-up), or have users manually touch that file (may cause a poor DX, the more that I think about it...). Any preferences?

Reference: https://graphite.dev/guides/git-blame-ignore-revs.

[raunakab]

The other option is to run git blame --ignore-rev $COMMIT_HASH. This can be cumbersome, especially if you don't remember which commit hash this major automated refactor was from.