Jefffrey opened a new issue, #18881:
URL: https://github.com/apache/datafusion/issues/18881

   ### Is your feature request related to a problem or challenge?
   
   Lint reference: 
https://rust-lang.github.io/rust-clippy/master/index.html?search=allow_att#allow_attributes
   
   > Checks for usage of the `#[allow]` attribute and suggests replacing it 
with the `#[expect]` attribute (See [RFC 
2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html))
   
   > `#[expect]` attributes suppress the lint emission, but emit a warning, if 
the expectation is unfulfilled. This can be useful to be notified when the lint 
is no longer triggered.
   
   We have a quite a few occurrences of `#[allow]` in our codebase, but should 
prefer `#[expect]` instead for the reasons above.
   
   ### Describe the solution you'd like
   
   Similar to #18503 we should roll this lint our gradually through crates 
before finally enabling at the workspace level so we don't have one big PR.
   
   ### Describe alternatives you've considered
   
   Don't enable this lint if we don't see value or if the false positive rate 
is too high (see below).
   
   ### Additional context
   
   I tested it for some of our current `#[allow]`s and it has some false 
positives, especially for `dead_code` and `deprecated`.
   
   **Example 1**
   
   
https://github.com/apache/datafusion/blob/c8d26ba012471e6aece9430642d6a8a923bc344c/datafusion/core/benches/data_utils/mod.rs#L37-L40
   
   - If I switch this to `#[expect(dead_code)]` then clippy will flag it as 
`this lint expectation is unfulfilled`. But if I remove the lint then the dead 
code warning pops up anyway 🤔 
   
   **Example 2**
   
   
https://github.com/apache/datafusion/blob/c8d26ba012471e6aece9430642d6a8a923bc344c/datafusion/execution/src/disk_manager.rs#L117-L136
   
   - Switching this to `#[expect(deprecated)]` causes similar to above, but 
also flags `NewOS` variant as using deprecated variant. Something to do with 
how `Default` is derived?
   
   (I say false positive but it seems more like a bug in clippy 🤔)
   
   Maybe there are solutions for the above, I haven't looked into it much yet.
   
   Another thing to keep in mind is how features interact, for example here:
   
   
https://github.com/apache/datafusion/blob/c8d26ba012471e6aece9430642d6a8a923bc344c/datafusion/datasource-parquet/src/reader.rs#L290-L304
   
   - This `#[allow]` is for when `parquet_encryption` feature is not enabled, 
but if we change it to `#[expect]` blindly then I believe it'll cause issues 
when `parquet_encryption` _is_ enabled, so we need to be careful


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to