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

chore: add deprecation notice for type ascription use #2483

Conversation

saiintbrisson
Copy link
Contributor

@saiintbrisson saiintbrisson commented May 2, 2023

Adds a deprecation notice when type ascription pattern is used, preparing grounds for when
we drop support for the pattern, keeping compatibility with Rust's supported syntax, as decided
on RFC 3307.

This change was created as discussed in #2482.

Example

The following code uses type ascription for the second argument.

    let _ = sqlx::query!(
        r#"SELECT $1 as a, $2 as b"#,
        foo_bar as i32,
        (6 + 8): <Foo as Deref>::Target
    );

Generating a warning that looks as follows:

warning: use of deprecated constant `main::{closure#0}::warning_arg1`: 
                 Type ascription pattern is deprecated, prefer casting
                 Try changing from
                     `(6 + 8) : < Foo as Deref > :: Target`
                 to
                     `(6 + 8) as < Foo as Deref > :: Target`
         
                 See <https://github.com/rust-lang/rfcs/pull/3307> for more information
  --> src/main.rs:16:9
   |
16 |         (6 + 8): <Foo as Deref>::Target
   |         ^^^^^^^

@saiintbrisson
Copy link
Contributor Author

saiintbrisson commented May 2, 2023

The formatting is a bit clunky with all the spaces, but this is how syn prints it and there isn't much we can do. Though I don't see this as a crucial problem.

EDIT: I also couldn't get the span to wrap the entirety of the expression for some reason. It completely ignores the type override part.

\t\tSee <https://github.com/rust-lang/rfcs/pull/3307> for more information
"
);
let name = Ident::new(&format!("warning_{name}"), span);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The names won't conflict as the consts are created inside the invocation's local scope.

@saiintbrisson saiintbrisson force-pushed the chore/macros/type-ascription-deprecation-notice branch 3 times, most recently from 6be6874 to 40ade0b Compare May 2, 2023 13:39
@saiintbrisson
Copy link
Contributor Author

saiintbrisson commented May 2, 2023

@abonander should I enable the missing syn features for sqlx-macros on this PR or will there be a separate PR for this? Referring to the 0.6.3 changelog.

@abonander
Copy link
Collaborator

We enable the correct features in sqlx-macros-core so enabling them in sqlx-macros isn't strictly necessary. I would only enable the features that sqlx-macros specifically uses.

@saiintbrisson saiintbrisson force-pushed the chore/macros/type-ascription-deprecation-notice branch from 40ade0b to 505d0fa Compare May 2, 2023 23:39
@saiintbrisson
Copy link
Contributor Author

Done, I've enabled both "parsing" and "proc-macro", which were missing and causing my local test to fail while building without syn on the build-dependencies.

@abonander abonander merged commit 4095ac4 into launchbadge:main May 4, 2023
@saiintbrisson saiintbrisson deleted the chore/macros/type-ascription-deprecation-notice branch May 4, 2023 20:03
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 this pull request may close these issues.

2 participants