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

Clippy reports issues from generated code #3346

Closed
autarch opened this issue Apr 19, 2023 · 3 comments · Fixed by #3355
Closed

Clippy reports issues from generated code #3346

autarch opened this issue Apr 19, 2023 · 3 comments · Fixed by #3355

Comments

@autarch
Copy link
Contributor

autarch commented Apr 19, 2023

If I run clippy against a crate that contains the module generated by icu4x-datagen I get a whole bunch of clippy errors. Here's a sample:

$> cargo clippy --locked --all-targets --all-features --workspace -- -D clippy::all

Checking omegasort-rs v0.1.0 (/home/autarch/projects/omegasort-rs)
error: module has the same name as its containing module
 --> src/./icu-data/mod.rs:2:1
  |
2 | mod collator ; mod fallback ; mod normalizer ; use :: icu_provider :: prelude :: * ; # [doc = r" Implement [`DataProvider<M>`] on the given str...
  | ^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
  = note: `-D clippy::module-inception` implied by `-D clippy::all`

error: unnecessary closure used with `bool::then`
 --> src/./icu-data/collator/jamo_v1/mod.rs:2:253
  |
2 | ... 'static DataStruct > { locale . is_empty () . then (|| & UND) } static UND : DataStruct = include ! ("und.rs.data") ;
  |                            ^^^^^^^^^^^^^^^^^^^^^^^---------------
  |                                                   |
  |                                                   help: use `then_some(..)` instead: `then_some(& UND)`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
  = note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D clippy::all`

I note that the generated code has some #[allow (clippy :: octal_escapes)] bits in it, but that's not helping with this.

I think it'd be best to just #[allow(clippy)]. Otherwise every time clippy adds a new lint that causes a lint failure you need to update the icu4x-datagen crate and I need to regenerate the generated code.

@sffc
Copy link
Member

sffc commented Apr 19, 2023

CC @robertbastian

Separately: if you pass --pretty to icu4x-datagen, it will invoke rustfmt for you, which may help a bit in the readability of these warnings.

@robertbastian
Copy link
Member

Hmm, yeah we should probably add #![allow(clippy::all)]. I do want to have clippy during development though.

@robertbastian
Copy link
Member

I found the #[clippy::msrv] attribute which we can set. Note that your first clippy finding is actually due to how you named the enclosing module, and I think that's a valid point.

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

Successfully merging a pull request may close this issue.

3 participants