Skip to content

Commit 3861536

Browse files
authored
Add a warning for keys after the first node which do nothing (#4978)
* add a warning for keys after the first node which do nothing * warnings on stable * better warnings on nightly
1 parent c58526a commit 3861536

File tree

5 files changed

+57
-5
lines changed

5 files changed

+57
-5
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/rsx/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ keywords = ["dom", "ui", "gui", "react"]
1616
proc-macro2 = { workspace = true, features = ["span-locations"] }
1717
proc-macro2-diagnostics = { workspace = true }
1818
quote = { workspace = true }
19+
rustversion.workspace = true
1920
syn = { workspace = true, features = ["full", "extra-traits", "visit", "visit-mut"] }
2021

2122
[features]

packages/rsx/src/diagnostics.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,29 @@ impl ToTokens for Diagnostics {
5353
}
5454
}
5555
}
56+
57+
// TODO: Ideally this would be integrated into the existing diagnostics struct, but currently all fields are public so adding
58+
// new fields would be a breaking change. Diagnostics also doesn't expose the message directly so we can't just modify
59+
// the expansion
60+
pub(crate) mod new_diagnostics {
61+
use proc_macro2_diagnostics::SpanDiagnosticExt;
62+
use std::fmt::Display;
63+
64+
use proc_macro2::{Span, TokenStream as TokenStream2};
65+
use quote::quote_spanned;
66+
67+
pub(crate) fn warning_diagnostic(span: Span, message: impl Display) -> TokenStream2 {
68+
let note = message.to_string();
69+
// If we are compiling on nightly, use diagnostics directly which supports proper warnings through new span apis
70+
if rustversion::cfg!(nightly) {
71+
return span.warning(note).emit_as_item_tokens();
72+
}
73+
quote_spanned! { span =>
74+
const _: () = {
75+
#[deprecated(note = #note)]
76+
struct Warning;
77+
_ = Warning;
78+
};
79+
}
80+
}
81+
}

packages/rsx/src/node.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,14 @@ impl BodyNode {
176176
_ => panic!("Element name not available for this node"),
177177
}
178178
}
179+
180+
pub(crate) fn key(&self) -> Option<&AttributeValue> {
181+
match self {
182+
Self::Element(el) => el.key(),
183+
Self::Component(comp) => comp.get_key(),
184+
_ => None,
185+
}
186+
}
179187
}
180188

181189
#[cfg(test)]

packages/rsx/src/template_body.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ impl ToTokens for TemplateBody {
109109
None => quote! { None },
110110
};
111111

112+
let key_warnings = self.check_for_duplicate_keys();
113+
112114
let roots = node.quote_roots();
113115

114116
// Print paths is easy - just print the paths
@@ -138,6 +140,8 @@ impl ToTokens for TemplateBody {
138140
dioxus_core::Element::Ok({
139141
#diagnostics
140142

143+
#key_warnings
144+
141145
// Components pull in the dynamic literal pool and template in debug mode, so they need to be defined before dynamic nodes
142146
#[cfg(debug_assertions)]
143147
fn __original_template() -> &'static dioxus_core::internal::HotReloadedTemplate {
@@ -273,11 +277,7 @@ impl TemplateBody {
273277
}
274278

275279
pub fn implicit_key(&self) -> Option<&AttributeValue> {
276-
match self.roots.first() {
277-
Some(BodyNode::Element(el)) => el.key(),
278-
Some(BodyNode::Component(comp)) => comp.get_key(),
279-
_ => None,
280-
}
280+
self.roots.first().and_then(BodyNode::key)
281281
}
282282

283283
/// Ensure only one key and that the key is not a static str
@@ -304,6 +304,22 @@ impl TemplateBody {
304304
}
305305
}
306306

307+
fn check_for_duplicate_keys(&self) -> TokenStream2 {
308+
let mut warnings = TokenStream2::new();
309+
310+
// Make sure there are not multiple keys or keys on nodes other than the first in the block
311+
for root in self.roots.iter().skip(1) {
312+
if let Some(key) = root.key() {
313+
warnings.extend(new_diagnostics::warning_diagnostic(
314+
key.span(),
315+
"Keys are only allowed on the first node in the block.",
316+
));
317+
}
318+
}
319+
320+
warnings
321+
}
322+
307323
pub fn get_dyn_node(&self, path: &[u8]) -> &BodyNode {
308324
let mut node = self.roots.get(path[0] as usize).unwrap();
309325
for idx in path.iter().skip(1) {

0 commit comments

Comments
 (0)