Merge pull request #70 from launchbadge/ab/idents-fix

sqlx-macros: fix handling of invalid idents

.github/workflows/rust.yml

@@ -37,11 +37,21 @@ jobs:
 
     # -----------------------------------------------------
 
-    # check: async-std
-    - run: cargo check --no-default-features --features 'chrono uuid postgres mysql macros tls runtime-async-std'
+    # check w/deny warnings in sqlx-core: async-std
+    - working-directory: sqlx-core
+      run: cargo rustc --no-default-features --features 'chrono uuid postgres mysql tls runtime-async-std' -- -D warnings --emit=metadata
 
-    # check: tokio
-    - run: cargo check --no-default-features --features 'chrono uuid postgres mysql macros tls runtime-tokio'
+    # check w/deny warnings in sqlx-core: tokio
+    # `cargo rustc -p sqlx-core` ignores `--no-default-features` and builds with `runtime-async-std` anyway
+    # https://github.com/rust-lang/cargo/issues/5364
+    - working-directory: sqlx-core
+      run: cargo rustc --no-default-features --features 'chrono uuid postgres mysql tls runtime-tokio' -- -D warnings --emit=metadata
+
+    # check w/deny warnings: async-std
+    - run: cargo rustc --no-default-features --features 'chrono uuid postgres mysql macros tls runtime-async-std' -- -D warnings --emit=metadata
+
+    # check w/deny warnings: tokio
+    - run: cargo rustc --no-default-features --features 'chrono uuid postgres mysql macros tls runtime-tokio' -- -D warnings --emit=metadata
 
     # unit test: async-std
     - run: cargo test --manifest-path sqlx-core/Cargo.toml --no-default-features --features 'chrono uuid postgres mysql tls runtime-async-std'

Cargo.lock

@@ -589,6 +589,11 @@ dependencies = [
  "wasi 0.9.0+wasi-snapshot-preview1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "glob"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "h2"
 version = "0.1.26"
@@ -1362,6 +1367,7 @@ dependencies = [
  "sqlx-core 0.2.4",
  "sqlx-macros 0.2.3",
  "tokio 0.2.9 (registry+https://github.com/rust-lang/crates.io-index)",
+ "trybuild 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -1683,11 +1689,32 @@ dependencies = [
  "tokio-executor 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "toml"
+version = "0.5.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "serde 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "try-lock"
 version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "trybuild"
+version = "1.0.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "serde 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)",
+ "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)",
+ "termcolor 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "toml 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "typenum"
 version = "1.11.2"
@@ -1879,6 +1906,7 @@ dependencies = [
 "checksum futures-util-preview 0.3.0-alpha.19 (registry+https://github.com/rust-lang/crates.io-index)" = "5ce968633c17e5f97936bd2797b6e38fb56cf16a7422319f7ec2e30d3c470e8d"
 "checksum generic-array 0.12.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c68f0274ae0e023facc3c97b2e00f076be70e254bc851d972503b328db79b2ec"
 "checksum getrandom 0.1.14 (registry+https://github.com/rust-lang/crates.io-index)" = "7abc8dd8451921606d809ba32e95b6111925cd2906060d2dcc29c070220503eb"
+"checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
 "checksum h2 0.1.26 (registry+https://github.com/rust-lang/crates.io-index)" = "a5b34c246847f938a410a03c5458c7fee2274436675e76d8b903c08efc29c462"
 "checksum hermit-abi 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "eff2656d88f158ce120947499e971d743c05dbcbed62e5bd2f38f1698bbc3772"
 "checksum hmac 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "5dcb5e64cda4c23119ab41ba960d1e170a774c8e4b9d9e6a9bc18aabf5e59695"
@@ -1989,7 +2017,9 @@ dependencies = [
 "checksum tokio-tcp 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "1d14b10654be682ac43efee27401d792507e30fd8d26389e1da3b185de2e4119"
 "checksum tokio-threadpool 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)" = "f0c32ffea4827978e9aa392d2f743d973c1dfa3730a2ed3f22ce1e6984da848c"
 "checksum tokio-timer 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)" = "1739638e364e558128461fc1ad84d997702c8e31c2e6b18fb99842268199e827"
+"checksum toml 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)" = "ffc92d160b1eef40665be3a05630d003936a3bc7da7421277846c2613e92c71a"
 "checksum try-lock 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e604eb7b43c06650e854be16a2a03155743d3752dd1c943f6829e26b7a36e382"
+"checksum trybuild 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)" = "987d6fdc45ddd7f3be5aa7386c8c8a844d1655c95b9ed948a9cd9cded8f2b79f"
 "checksum typenum 1.11.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6d2783fe2d6b8c1101136184eb41be8b1ad379e4657050b8aaff0c79ee7575f9"
 "checksum unicode-bidi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "49f2bd0c6468a8230e1db229cff8029217cf623c767ea5d60bfbd42729ea54d5"
 "checksum unicode-normalization 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "b561e267b2326bb4cebfc0ef9e68355c7abe6c6f522aeac2f5bf95d56c59bdcf"

Cargo.toml

@@ -55,6 +55,7 @@ env_logger = "0.7.1"
 async-std = { version = "1.4.0", features = [ "attributes" ] }
 tokio = { version = "0.2.9", features = [ "full" ] }
 dotenv = "0.15.0"
+trybuild = "1.0"
 
 [[test]]
 name = "postgres-macros"

sqlx-core/src/io/tls.rs

@@ -1,5 +1,4 @@
 use std::io;
-use std::io::{IoSlice, IoSliceMut};
 use std::net::Shutdown;
 use std::pin::Pin;
 use std::task::{Context, Poll};
@@ -94,7 +93,7 @@ impl AsyncRead for MaybeTlsStream {
     fn poll_read_vectored(
         mut self: Pin<&mut Self>,
         cx: &mut Context,
-        bufs: &mut [IoSliceMut],
+        bufs: &mut [std::io::IoSliceMut],
     ) -> Poll<io::Result<usize>> {
         forward_pin!(self.poll_read_vectored(cx, bufs))
     }
@@ -127,7 +126,7 @@ impl AsyncWrite for MaybeTlsStream {
     fn poll_write_vectored(
         mut self: Pin<&mut Self>,
         cx: &mut Context,
-        bufs: &[IoSlice],
+        bufs: &[std::io::IoSlice],
     ) -> Poll<io::Result<usize>> {
         forward_pin!(self.poll_write_vectored(cx, bufs))
     }

sqlx-core/src/postgres/arguments.rs

@@ -1,6 +1,6 @@
 use byteorder::{ByteOrder, NetworkEndian};
 
-use crate::arguments::{Arguments, IntoArguments};
+use crate::arguments::Arguments;
 use crate::encode::{Encode, IsNull};
 use crate::io::BufMut;
 use crate::types::HasSqlType;

sqlx-core/src/postgres/protocol/close.rs

@@ -1,3 +1,4 @@
+#![allow(dead_code)]
 use crate::io::BufMut;
 use crate::postgres::protocol::Encode;
 use byteorder::NetworkEndian;

sqlx-core/src/postgres/protocol/mod.rs

@@ -19,6 +19,7 @@ mod parse;
 mod password_message;
 mod query;
 mod sasl;
+#[cfg_attr(not(feature = "tls"), allow(unused_imports, dead_code))]
 mod ssl_request;
 mod startup_message;
 mod statement;

sqlx-core/src/postgres/protocol/ssl_request.rs

@@ -1,6 +1,7 @@
-use crate::io::{Buf, BufMut};
 use byteorder::NetworkEndian;
 
+use crate::io::BufMut;
+
 pub struct SslRequest;
 
 impl SslRequest {

sqlx-macros/src/lib.rs

@@ -8,8 +8,6 @@ use proc_macro::TokenStream;
 
 use quote::quote;
 
-use syn::parse_macro_input;
-
 #[cfg(feature = "runtime-async-std")]
 use async_std::task::block_on;
 
@@ -102,7 +100,7 @@ macro_rules! async_macro (
                 if let Some(parse_err) = e.downcast_ref::<syn::Error>() {
                     macro_result(parse_err.to_compile_error())
                 } else {
-                    let msg = format!("{:?}", e);
+                    let msg = e.to_string();
                     macro_result(quote!(compile_error!(#msg)))
                 }
             }

sqlx-macros/src/query_macros/args.rs

@@ -1,6 +1,5 @@
 use proc_macro2::TokenStream;
 use quote::{quote, quote_spanned, ToTokens};
-use syn::spanned::Spanned;
 use syn::Expr;
 
 use sqlx::describe::Describe;

sqlx-macros/src/query_macros/input.rs

@@ -1,6 +1,6 @@
 use std::env;
 
-use proc_macro2::{Ident, Span, TokenStream};
+use proc_macro2::{Ident, Span};
 use sqlx::runtime::fs;
 use syn::parse::{Parse, ParseStream};
 use syn::punctuated::Punctuated;
@@ -9,7 +9,7 @@ use syn::token::Group;
 use syn::{Expr, ExprLit, ExprPath, Lit};
 use syn::{ExprGroup, Token};
 
-use quote::{format_ident, quote, ToTokens};
+use quote::{format_ident, ToTokens};
 
 use sqlx::describe::Describe;
 use sqlx::Connection;

sqlx-macros/src/query_macros/output.rs

@@ -22,8 +22,7 @@ pub fn columns_to_rust<DB: DatabaseExt>(describe: &Describe<DB>) -> crate::Resul
                 .as_deref()
                 .ok_or_else(|| format!("column at position {} must have a name", i))?;
 
-            let ident = syn::parse_str::<Ident>(name)
-                .map_err(|_| format!("{:?} is not a valid Rust identifier", name))?;
+            let ident = parse_ident(name)?;
 
             let type_ = <DB as DatabaseExt>::return_type_for_id(&column.type_info)
                 .ok_or_else(|| format!("unknown type: {}", &column.type_info))?
@@ -61,3 +60,18 @@ pub fn quote_query_as<DB: DatabaseExt>(
         })
     }
 }
+
+fn parse_ident(name: &str) -> crate::Result<Ident> {
+    // workaround for the following issue (it's semi-fixed but still spits out extra diagnostics)
+    // https://github.com/dtolnay/syn/issues/749#issuecomment-575451318
+
+    let is_valid_ident = name.chars().all(|c| c.is_alphanumeric() || c == '_');
+
+    if is_valid_ident {
+        if let Ok(ident) = syn::parse_str(name) {
+            return Ok(ident);
+        }
+    }
+
+    Err(format!("{:?} is not a valid Rust identifier", name).into())
+}

src/lib.rs

@@ -31,7 +31,6 @@ pub use sqlx_core::postgres::{self, PgConnection, PgPool, Postgres};
 pub extern crate sqlx_macros;
 
 #[cfg(feature = "macros")]
-#[macro_export]
 mod macros;
 
 // macro support

tests/ui-tests.rs

@@ -0,0 +1,14 @@
+#[test]
+fn ui_tests() {
+    let t = trybuild::TestCases::new();
+
+    if cfg!(feature = "postgres") {
+        t.compile_fail("tests/ui/postgres/*.rs");
+    }
+
+    if cfg!(feature = "mysql") {
+        t.compile_fail("tests/ui/mysql/*.rs");
+    }
+
+    t.compile_fail("tests/ui/*.rs");
+}

tests/ui/postgres/issue_30.rs

@@ -0,0 +1,3 @@
+fn main() {
+    let query = sqlx::query!("select 1 as \"'1\"");
+}

tests/ui/postgres/issue_30.stderr

@@ -0,0 +1,7 @@
+error: "\'1" is not a valid Rust identifier
+ --> $DIR/issue_30.rs:2:17
+  |
+2 |     let query = sqlx::query!("select 1 as \"'1\"");
+  |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |
+  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)