google / wasefire

Secure firmware framework focusing on developer experience
https://google.github.io/wasefire/
Apache License 2.0
86 stars 20 forks source link

Fix all lints currently disabled in scripts/sync.sh #565

Open ia0 opened 3 months ago

ia0 commented 3 months ago

We want to enable the following lints:

devesh-2002 commented 3 months ago

May I take this issue?

ia0 commented 3 months ago

Of course, that would be great. That's a good first issue. You can start by changing those 2 lines:

https://github.com/google/wasefire/blob/5da7a5e0de8545825ce6ca023c389f2e35926c02/scripts/sync.sh#L33-L34

into (I'm removing missing-debug-implementations because I'm not yet convinced it's a good thing to have):

  case $crate in
    prelude) add_lint $file warn rust.elided-lifetimes-in-paths ;;
  esac

Then run ./scripts/sync.sh (note that you might need to run ./scripts/setup.sh the very first time you clone the repository). This will add the lint to the prelude Cargo.toml. Then you can run ./test.sh from the crates/prelude directory and fix any warning.

Once there are no warnings, you can create your first PR.

Then, you can add more and more crates to the case pattern, like:

  case $crate in
    board|prelude) add_lint $file warn rust.elided-lifetimes-in-paths ;;
  esac

And create one PR for each non-trivial crate (you can group multiple crates together if there's nothing or almost nothing to change).

The final state (i.e. when adding the last remaning crate) should simply be:

  add_lint $file warn rust.elided-lifetimes-in-paths

Thanks! Feel free to ask questions if you're blocked.

devesh-2002 commented 3 months ago

Thank you @ia0 for the detailed instructions! I'll keep these in mind

devesh-2002 commented 3 months ago

@ia0 , Hello. I ran ./scripts/setup.sh and then ./scripts/sync.sh. But it gives me a duplicate key error. Would you please guide me in resolving this error?

 error: invalid table header
duplicate key `lints` in document root
  --> crates/xtask/Cargo.toml:26:1
   |
26 | [lints]
   | ^
   |
ia0 commented 3 months ago

To debug:

I suspect that one of those 2 lines doesn't behave as expected in your case:

https://github.com/google/wasefire/blob/d32a3d98046b15e2295a71334462b1e5b12cd923/scripts/sync.sh#L30-L31