Deprecating old feature names

[npapapietro]

PR addressing #1636

• Had to add #![allow(deprecated)] or builds will fail
• Adding listed features with deprecated message but allow build
• Life cycle of this, messages should be updated when the release is known for removing the deprecated feature all together.

[weiznich]

This shouldn't be required. I would prefer to add a conditional (#[cfg_attr]) scoped allow attribute to the exact locations where the error is emitted instead.

[npapapietro]

I'm unsure if this is allowed.

#[cfg_attr(feature="huge-tables", allow(deprecated), deprecated="`huge-tables` is deprecated in favor of `64-column-tables`")]
#[cfg_attr(feature="large-tables", allow(deprecated), deprecated="`large-tables` is deprecated in favor of `32-column-tables`")]
#[macro_export]
macro_rules! table {
...

Still throws

error: use of deprecated macro `table`: `large-tables` is deprecated in favor of `32-column-tables`
   --> diesel/src/pg/metadata_lookup.rs:153:1
    |
153 | table! {
    | ^^^^^
    |
note: the lint level is defined here
   --> diesel/src/lib.rs:97:9
    |
97  | #![deny(warnings)]
    |         ^^^^^^^^
    = note: `#[deny(deprecated)]` implied by `#[deny(warnings)]`

[weiznich]

I think this should be fixable by putting #[cfg_attr(any(feature="huge-tables" feature="large-tables"), allow(deprecated)] directly on the table! call mentioned in the error message. (If that does not work, just putting it as inner attribute into the corresponding (metadata_lookup) module should work)

[npapapietro]

Having issues with this even injecting into the inner attributes. Doing some reading around maybe we should take this approach and deny specific warnings? https://github.com/rust-unofficial/patterns/blob/main/anti_patterns/deny-warnings.md

[npapapietro]

To expand, I've tried

diesel/src/lib.rs

#[macro_use]
#[doc(hidden)]
#[allow(deprecated)]
pub mod macros;

diesel/src/macros/mod.rs

#![allow(warnings)]
#![allow(unused_parens)] // FIXME: Remove this attribute once false positive is resolved.
#![cfg_attr(rustfmt, rustfmt_skip)] // https://github.com/rust-lang-nursery/rustfmt/issues/2755

diesel/src/macros/mod.rs

#[cfg_attr(any(feature="huge-tables" feature="large-tables"), allow(deprecated)]
#[cfg_attr(feature="huge-tables", deprecated="`huge-tables` is deprecated in favor of `64-column-tables`")]
#[cfg_attr(feature="large-tables", deprecated="`large-tables` is deprecated in favor of `32-column-tables`")]
#[macro_export]
macro_rules! table {

And I've gotten the same build error due to deny on warnings in src/lib.rs

[weiznich]

Deprecated warnings are issued on places where a deprecated item is used, not where it is defined. That means we need to suppress the warning at the usage location, not at the definition side in src/macros/mod.rs
I think the only usage of the table! macro in diesel is here:
https://github.com/diesel-rs/diesel/blob/master/diesel/src/pg/metadata_lookup.rs#L153-L167

Adding the corresponding allow annotation there should fix the compilation error.

[npapapietro]

This approach doesn't work either. I've tried tons of placement options.

I'm pretty sure the #![deny(warnings] blocks all warnings even if allowed as well turns unblocked warnings into build errors. As an experiment I removed it in favor of all the warnings we would normally block

#![deny(
    missing_debug_implementations,
    missing_copy_implementations,
    missing_docs,
    clippy::unwrap_used,
    clippy::print_stdout,
    clippy::wrong_pub_self_convention,
    clippy::mut_mut,
    clippy::non_ascii_literal,
    clippy::similar_names,
    clippy::unicode_not_nfc,
    clippy::enum_glob_use,
    clippy::if_not_else,
    clippy::items_after_statements,
    clippy::used_underscore_binding,
   // rest of general rust lint warnings
)]

and this produces the intended result. Additionally I propose adding a void, empty function that is called in metadata_lookup.rs that is well named so the warnings are direct and not scattered.

[weiznich]

Surprising that putting the #[allow(deprecated)] did not work as expected. I've experimented a bit with this and pushed a more minimal version that only applies #[allow(deprecated)] where it is really needed and put that behind the deprecated feature flags itself.

[npapapietro]

I've pushed an alternate proposal