Detect a common syntax error case for diagnostic_directive (#6718)

Co-authored-by: Erich Gubler <[email protected]>

CHANGELOG.md

@@ -156,6 +156,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
 
 - Show types of LHS and RHS in binary operation type mismatch errors. By @ErichDonGubler in [#6450](https://github.com/gfx-rs/wgpu/pull/6450).
 - The GLSL parser now uses less expressions for function calls. By @magcius in [#6604](https://github.com/gfx-rs/wgpu/pull/6604).
+- Add a note to help with a common syntax error case for global diagnostic filter directives. By @e-hat in [#6718](https://github.com/gfx-rs/wgpu/pull/6718)
 
 #### General
 

naga/src/front/wgsl/error.rs

@@ -303,7 +303,7 @@ pub(crate) enum Error<'a> {
         spans: Vec<Span>,
     },
     DiagnosticAttributeNotSupported {
-        on_what_plural: &'static str,
+        on_what: DiagnosticAttributeNotSupportedPosition,
         spans: Vec<Span>,
     },
 }
@@ -314,6 +314,19 @@ impl From<ConflictingDiagnosticRuleError> for Error<'_> {
     }
 }
 
+/// Used for diagnostic refinement in [`Error::DiagnosticAttributeNotSupported`].
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum DiagnosticAttributeNotSupportedPosition {
+    SemicolonInModulePosition,
+    Other { display_plural: &'static str },
+}
+
+impl From<&'static str> for DiagnosticAttributeNotSupportedPosition {
+    fn from(display_plural: &'static str) -> Self {
+        Self::Other { display_plural }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub(crate) struct AutoConversionError {
     pub dest_span: Span,
@@ -1080,27 +1093,55 @@ impl<'a> Error<'a> {
                     "so they can prioritize it!"
                 ))],
             },
-            Error::DiagnosticAttributeNotSupported {
-                on_what_plural,
-                ref spans,
-            } => ParseError {
-                message: format!(
-                    "`@diagnostic(…)` attribute(s) on {on_what_plural} are not supported",
-                ),
-                labels: spans
-                    .iter()
-                    .cloned()
-                    .map(|span| (span, "".into()))
-                    .collect(),
-                notes: vec![
-                    concat!(
-                        "`@diagnostic(…)` attributes are only permitted on `fn`s, ",
-                        "some statements, and `switch`/`loop` bodies."
-                    )
-                    .into(),
-                    "These attributes are well-formed, you likely just need to move them.".into(),
-                ],
-            },
+            Error::DiagnosticAttributeNotSupported { on_what, ref spans } => {
+                // In this case the user may have intended to create a global diagnostic filter directive,
+                // so display a note to them suggesting the correct syntax.
+                let intended_diagnostic_directive = match on_what {
+                    DiagnosticAttributeNotSupportedPosition::SemicolonInModulePosition => true,
+                    DiagnosticAttributeNotSupportedPosition::Other { .. } => false,
+                };
+                let on_what_plural = match on_what {
+                    DiagnosticAttributeNotSupportedPosition::SemicolonInModulePosition => {
+                        "semicolons"
+                    }
+                    DiagnosticAttributeNotSupportedPosition::Other { display_plural } => {
+                        display_plural
+                    }
+                };
+                ParseError {
+                    message: format!(
+                        "`@diagnostic(…)` attribute(s) on {on_what_plural} are not supported",
+                    ),
+                    labels: spans
+                        .iter()
+                        .cloned()
+                        .map(|span| (span, "".into()))
+                        .collect(),
+                    notes: vec![
+                        concat!(
+                            "`@diagnostic(…)` attributes are only permitted on `fn`s, ",
+                            "some statements, and `switch`/`loop` bodies."
+                        )
+                        .into(),
+                        {
+                            if intended_diagnostic_directive {
+                                concat!(
+                                    "If you meant to declare a diagnostic filter that ",
+                                    "applies to the entire module, move this line to ",
+                                    "the top of the file and remove the `@` symbol."
+                                )
+                                .into()
+                            } else {
+                                concat!(
+                                    "These attributes are well-formed, ",
+                                    "you likely just need to move them."
+                                )
+                                .into()
+                            }
+                        },
+                    ],
+                }
+            }
         }
     }
 }

naga/src/front/wgsl/parse/mod.rs

@@ -2,7 +2,7 @@ use crate::diagnostic_filter::{
     self, DiagnosticFilter, DiagnosticFilterMap, DiagnosticFilterNode, FilterableTriggeringRule,
     ShouldConflictOnFullDuplicate, StandardFilterableTriggeringRule,
 };
-use crate::front::wgsl::error::{Error, ExpectedToken};
+use crate::front::wgsl::error::{DiagnosticAttributeNotSupportedPosition, Error, ExpectedToken};
 use crate::front::wgsl::parse::directive::enable_extension::{
     EnableExtension, EnableExtensions, UnimplementedEnableExtension,
 };
@@ -2465,17 +2465,16 @@ impl Parser {
             unresolved: &mut dependencies,
         };
         let mut diagnostic_filters = DiagnosticFilterMap::new();
-        let ensure_no_diag_attrs =
-            |on_what_plural, filters: DiagnosticFilterMap| -> Result<(), Error> {
-                if filters.is_empty() {
-                    Ok(())
-                } else {
-                    Err(Error::DiagnosticAttributeNotSupported {
-                        on_what_plural,
-                        spans: filters.spans().collect(),
-                    })
-                }
-            };
+        let ensure_no_diag_attrs = |on_what, filters: DiagnosticFilterMap| -> Result<(), Error> {
+            if filters.is_empty() {
+                Ok(())
+            } else {
+                Err(Error::DiagnosticAttributeNotSupported {
+                    on_what,
+                    spans: filters.spans().collect(),
+                })
+            }
+        };
 
         self.push_rule_span(Rule::Attribute, lexer);
         while lexer.skip(Token::Attribute) {
@@ -2562,22 +2561,25 @@ impl Parser {
         let start = lexer.start_byte_offset();
         let kind = match lexer.next() {
             (Token::Separator(';'), _) => {
-                ensure_no_diag_attrs("semicolons", diagnostic_filters)?;
+                ensure_no_diag_attrs(
+                    DiagnosticAttributeNotSupportedPosition::SemicolonInModulePosition,
+                    diagnostic_filters,
+                )?;
                 None
             }
             (Token::Word(word), directive_span) if DirectiveKind::from_ident(word).is_some() => {
                 return Err(Error::DirectiveAfterFirstGlobalDecl { directive_span });
             }
             (Token::Word("struct"), _) => {
-                ensure_no_diag_attrs("`struct`s", diagnostic_filters)?;
+                ensure_no_diag_attrs("`struct`s".into(), diagnostic_filters)?;
 
                 let name = lexer.next_ident()?;
 
                 let members = self.struct_body(lexer, &mut ctx)?;
                 Some(ast::GlobalDeclKind::Struct(ast::Struct { name, members }))
             }
             (Token::Word("alias"), _) => {
-                ensure_no_diag_attrs("`alias`es", diagnostic_filters)?;
+                ensure_no_diag_attrs("`alias`es".into(), diagnostic_filters)?;
 
                 let name = lexer.next_ident()?;
 
@@ -2587,7 +2589,7 @@ impl Parser {
                 Some(ast::GlobalDeclKind::Type(ast::TypeAlias { name, ty }))
             }
             (Token::Word("const"), _) => {
-                ensure_no_diag_attrs("`const`s", diagnostic_filters)?;
+                ensure_no_diag_attrs("`const`s".into(), diagnostic_filters)?;
 
                 let name = lexer.next_ident()?;
 
@@ -2605,7 +2607,7 @@ impl Parser {
                 Some(ast::GlobalDeclKind::Const(ast::Const { name, ty, init }))
             }
             (Token::Word("override"), _) => {
-                ensure_no_diag_attrs("`override`s", diagnostic_filters)?;
+                ensure_no_diag_attrs("`override`s".into(), diagnostic_filters)?;
 
                 let name = lexer.next_ident()?;
 
@@ -2631,7 +2633,7 @@ impl Parser {
                 }))
             }
             (Token::Word("var"), _) => {
-                ensure_no_diag_attrs("`var`s", diagnostic_filters)?;
+                ensure_no_diag_attrs("`var`s".into(), diagnostic_filters)?;
 
                 let mut var = self.variable_decl(lexer, &mut ctx)?;
                 var.binding = binding.take();
@@ -2662,7 +2664,7 @@ impl Parser {
                 }))
             }
             (Token::Word("const_assert"), _) => {
-                ensure_no_diag_attrs("`const_assert`s", diagnostic_filters)?;
+                ensure_no_diag_attrs("`const_assert`s".into(), diagnostic_filters)?;
 
                 // parentheses are optional
                 let paren = lexer.skip(Token::Paren('('));

naga/src/front/wgsl/tests.rs

@@ -660,6 +660,27 @@ fn parse_missing_workgroup_size() {
 }
 
 mod diagnostic_filter {
+    use crate::front::wgsl::assert_parse_err;
+
+    #[test]
+    fn intended_global_directive() {
+        let shader = "@diagnostic(off, my.lint);";
+        assert_parse_err(
+            shader,
+            "\
+error: `@diagnostic(…)` attribute(s) on semicolons are not supported
+  ┌─ wgsl:1:1
+  │
+1 │ @diagnostic(off, my.lint);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^
+  │
+  = note: `@diagnostic(…)` attributes are only permitted on `fn`s, some statements, and `switch`/`loop` bodies.
+  = note: If you meant to declare a diagnostic filter that applies to the entire module, move this line to the top of the file and remove the `@` symbol.
+
+"
+        );
+    }
+
     mod parse_sites_not_yet_supported {
         use crate::front::wgsl::assert_parse_err;