Merge pull request #477 from betrusted-io/rustfmt-proposal

RFC: proposed rustfmt settings for the project

.github/workflows/build.yml

@@ -18,15 +18,11 @@ jobs:
           sudo apt update
           sudo apt install -y libxkbcommon-dev
 
-      - name: Install stable toolchain
-        uses: actions-rs/toolchain@v1
-        with:
-          profile: minimal
-          toolchain: "1.75.0"
-          default: true
+      - name: Update to latest toolchain
+        run: rustup update
 
       - name: Checkout sources
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
       - name: Fetch tags
         run: git fetch --prune --unshallow --tags
 
@@ -38,5 +34,5 @@ jobs:
       - name: Clean out old target directory (in case of libstd change)
         run: rm -rf target/*
 
-      - name: Build hosted-ci
+      - name: Build CI targets
         run: cargo xtask ${{ matrix.task }} --no-verify

.github/workflows/rustfmt_check.yml

@@ -0,0 +1,24 @@
+# From https://github.com/samblenny/autofmt-demo/blob/main/.github/workflows/rustfmt_check.yml
+name: rustfmt_check
+on: push
+jobs:
+  rustfmt_check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out branch source
+        uses: actions/checkout@v4
+
+      - name: Install rustfmt from nightly toolchain
+        run: |
+          rustup toolchain install nightly
+          rustup component add rustfmt --toolchain nightly
+
+      - name: Ensure the `main` branch rustfmt is applied for the run
+        run: |
+          git fetch --no-tags --prune --no-recurse-submodules --depth=1 origin main
+          git checkout origin/main rustfmt.toml
+
+      - name: Run cargo +nightly fmt --check
+        run: cargo +nightly fmt --check
+
+      - run: echo "status = ${{ job.status }}"

.github/workflows/trailing_whitespace_check.yml

@@ -0,0 +1,16 @@
+# From https://github.com/samblenny/autofmt-demo/blob/main/.github/workflows/trailing_whitespace_check.yml
+name: trailing_whitespace_check
+on: push
+jobs:
+  trailing_whitespace_check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out branch source
+        uses: actions/checkout@v4
+
+      - name: Check .rs files for trailing whitespace
+        run: >
+          find . -name '*.rs' -print0
+          | xargs -0 grep -c --with-filename -P '[ \t]$'
+          | grep -v ':0$'
+          | perl -e 'while(<>){print;$n+=1};if($n>0){exit 1;};print "PASS\n";'

.vscode/settings.json

@@ -1,20 +1,31 @@
 {
-    "rust-analyzer.cargo.target":"riscv32imac-unknown-xous-elf",
-    "rust-analyzer.check.overrideCommand":[
+    "rust-analyzer.cargo.target": "riscv32imac-unknown-xous-elf",
+    "rust-analyzer.check.overrideCommand": [
         "cargo",
         "check",
-        "--target", "riscv32imac-unknown-xous-elf",
-        "--target-dir", "target/analyzer",
-        "--features", "precursor",
-        "--features", "utralib/precursor",
-        "--features", "utralib/precursor-pvt",
-        "--message-format", "json"
+        "--target",
+        "riscv32imac-unknown-xous-elf",
+        "--target-dir",
+        "target/analyzer",
+        "--features",
+        "precursor",
+        "--features",
+        "utralib/precursor",
+        "--features",
+        "utralib/precursor-pvt",
+        "--message-format",
+        "json"
     ],
     "rust-analyzer.server.extraEnv": {
         "XOUS_SVD_FILE": "../precursors/soc.svd"
     },
-    "rust-analyzer.cargo.features": ["precursor", "mass-storage"],
-    "rust-analyzer.files.excludeDirs": ["./loader/**"],
+    "rust-analyzer.cargo.features": [
+        "precursor",
+        "mass-storage"
+    ],
+    "rust-analyzer.files.excludeDirs": [
+        "./loader/**"
+    ],
     "files.exclude": {
         "**/.git": true,
         "**/.svn": true,
@@ -23,5 +34,10 @@
         "**/.DS_Store": true,
         "**/Thumbs.db": true
     },
+    "rust-analyzer.rustfmt.extraArgs": [
+        "+nightly"
+    ],
+    "files.trimTrailingWhitespace": true,
+    "editor.formatOnSave": true,
     "cSpell.language": "en,fr"
-}
+}

FORMATTING.md

@@ -0,0 +1,50 @@
+# Formatting Guidelines
+
+All contributions to Xous must comply with two formatting guidelines:
+
+1. Run `rustfmt` using our `rustfmt.toml` with a nightly toolchain
+2. Remove trailing whitespaces
+
+## Resources
+
+Our `rustfmt` requires a Rust nightly toolchain.
+
+Install a nightly toolchain with
+
+`rustup toolchain install nightly`
+
+From here, you may run `rustfmt` on the command line with
+
+`cargo +nightly fmt -p <crate you are editing>`
+
+Unfortunately, one cannot simply run `rustfmt` on a single file with nightly from the command line without changing the default toolchain. However, there are various plugins one can use to help with this.
+
+### vscode
+
+If you are using `vscode`, add these lines to your `settings.json` file:
+
+```json
+"rust-analyzer.rustfmt.extraArgs": [
+        "+nightly"
+    ],
+"files.trimTrailingWhitespace": true,
+"editor.formatOnSave": true,
+```
+
+The default `settings.json` that comes with the repo already has these built in.
+
+### `pre-commit`
+
+[pre-commit](https://pre-commit.com/) is a tool that can be configured to run github actions locally.
+
+The development workflow looks like this:
+
+ 1. the developer installs pre-commit on their machine and runs pre-commit install from within the xous-core directory: it'll set up the necessary dependencies, and sets up a local git pre-commit hook to run them
+ 2. development flows as expected
+ 3. at commit time, one of two things can happen
+    a. code is formatted correctly already, commit gets written
+    b. code is not formatted correctly, commit is aborted, code gets formatted automatically
+
+### Other tools
+
+If you have your own workflow, please contribute a hint to this document so that others can benefit from it!

RELEASE-v0.9.md

@@ -462,6 +462,8 @@ perform the Xous firmware upgrade. This requires running manual update commands,
 - `sigchat` can now link up to an existing account - thanks to huge PR by @nhoj
 - `xtask` can accept out-of-tree apps using a syntax like `cargo xtask app-image sigchat:path-to-elf-file` thanks to PR by @kotval. This allows apps like `sigchat` to have AGPL licensing without contaminating `xous-core`
 - `sigchat` moved to its own repo with AGPL licensing: https://github.com/betrusted-io/sigchat
+- Formatting and contribution standards have been modified. Formatting with `rustfmt` and trailing white space removal is now mandatory for all Xous contributions, see [#477](https://github.com/betrusted-io/xous-core/pull/477) for a discussion of how we got there and why.
+- The repo has gone through a "flag day" where all the crates have been formatted, which means commits before the flag day may be more difficult to undo. The changes are committed on a crate-by-crate basis, so if something is really broken we can undo the formatting for the crate and add an exception to the rustfmt rules.
 
 
 ## Roadmap

rustfmt.toml

@@ -1,20 +1,39 @@
-# if you want to format 'em, then you can maintain 'em!
-ignore = [
-  "services/codec",
-  "services/com",
-  "services/engine-25519",
-  "services/gam",
-  "services/graphics-server",
-  "services/ime-frontend",
-  "services/ime-plugin-shell",
-  "services/jtag",
-  "services/keyboard",
-  "services/llio",
-  "services/root-keys",
-  "services/rtc",
-  "services/shellchat",
-  "services/spinor",
-  "services/susres",
-]
-# Note: run this on Linux to fix Windows changing .rs files to +x:
-# find . -name "*.rs" -exec chmod -x {} \;
+# Notes:
+# - @generated in the first five lines of a file suppresses formatting for generated files!
+# - run this on Linux to fix Windows changing .rs files to +x:
+#      `find . -name "*.rs" -exec chmod -x {} \;`
+
+# I can't rely on contributors using .editorconfig
+newline_style = "Unix"
+# require the shorthand instead of it being optional
+use_field_init_shorthand = true
+# outdated default — `?` was unstable at the time
+# additionally the `try!` macro is deprecated now
+use_try_shorthand = true
+# Computers in the 90's had 132 column displays[1], and the displays were 1/6th the area.
+# 110 columns doesn't seem like a big stretch...
+# [1] https://en.wikipedia.org/wiki/VGA_text_mode
+max_width = 110
+# Max to use the max width for everything or Default. See https://rust-lang.github.io/rustfmt/?version=v1.4.38&search=#use_small_heuristics
+use_small_heuristics = "Max"
+
+# Unstable features below
+unstable_features = true
+version = "Two"
+# code can be 110 characters, why not comments?
+comment_width = 110
+# quicker manual lookup
+group_imports = "StdExternalCrate"
+# why use an attribute if a normal doc comment would suffice?
+normalize_doc_attributes = true
+# why not?
+format_code_in_doc_comments = true
+format_macro_bodies = true
+format_macro_matchers = true
+format_strings = false
+wrap_comments = true
+fn_single_line = true
+reorder_impl_items = true
+# this helps with merges, even though it destroys logical cohesion
+reorder_imports = true
+reorder_modules = true