Resolve remaining source parity issues

Author: Milky2018

composer.mbt

@@ -169,6 +169,7 @@ priv struct WgslComposeSession {
   modules : @hashmap.HashMap[String, String]
   redirects : Array[WgslSymbolRedirect]
   additional_imports : Array[ImportDefinition]
+  redirect_match_counts : @hashmap.HashMap[String, Int]
   visited : @hashmap.HashMap[String, Bool]
   imported_item_names : @hashmap.HashMap[String, String]
   active_rel_paths : Array[String]
@@ -299,6 +300,7 @@ fn Composer::new_wgsl_compose_session(
     modules: self.registry.copy_import_module_paths(),
     redirects: options.redirects.copy(),
     additional_imports: copy_import_definitions(options.additional_imports),
+    redirect_match_counts: @hashmap.HashMap::new(),
     visited: @hashmap.HashMap::new(),
     imported_item_names: @hashmap.HashMap::new(),
     active_rel_paths: [],
@@ -332,6 +334,27 @@ fn collect_wgsl_cached_import_redirects(
   count
 }
 
+///|
+fn record_wgsl_redirect_matches(
+  source : String,
+  session : WgslComposeSession,
+) -> Unit {
+  for redirect in session.redirects {
+    let from_name = redirect.from_name.trim().to_string()
+    let to_name = redirect.to_name.trim().to_string()
+    if from_name == "" || to_name == "" || from_name == to_name {
+      continue
+    }
+    if wgsl_source_contains_identifier(source, from_name) {
+      let current = match session.redirect_match_counts.get(from_name) {
+        Some(count) => count
+        None => 0
+      }
+      session.redirect_match_counts.set(from_name, current + 1)
+    }
+  }
+}
+
 ///|
 fn composer_remove_composable_module_recursive(
   composer : Composer,
@@ -799,6 +822,7 @@ fn prepare_wgsl_compose_source(
   source : String,
   session : WgslComposeSession,
 ) -> WgslPreparedComposeSource {
+  record_wgsl_redirect_matches(source, session)
   let redirected_source = rewrite_wgsl_symbol_redirects(
     source,
     session.redirects,
@@ -1352,7 +1376,9 @@ pub fn Composer::compose_wgsl(
   options : WgslComposeOptions,
 ) -> String raise ComposerError {
   let session = self.new_wgsl_compose_session(options)
-  self.load_root_wgsl_preprocessed_into_session(rel, session)
+  sanitize_wgsl_invalid_identifiers(
+    self.load_root_wgsl_preprocessed_into_session(rel, session),
+  )
 }
 
 ///|
@@ -1362,10 +1388,12 @@ pub fn Composer::compose_wgsl_source(
   options : WgslComposeOptions,
 ) -> String raise ComposerError {
   let session = self.new_wgsl_compose_session(options)
-  self.preprocess_wgsl_source_with_path(
-    "",
-    prepend_wgsl_additional_imports(source, session.additional_imports),
-    session,
+  sanitize_wgsl_invalid_identifiers(
+    self.preprocess_wgsl_source_with_path(
+      "",
+      prepend_wgsl_additional_imports(source, session.additional_imports),
+      session,
+    ),
   )
 }
 

docs/moon_wgsl-issue-tracker.md

@@ -36,15 +36,13 @@ Last updated: 2026-04-29
 | `WGSL-021` | GitHub issue #3 | Mixed Bevy PBR import graphs exposed missing import-only entry preservation and nested alias/type rewrites for source-level composition. | `DONE` | Fixed by preserving import-only WGSL entry items and expanding the Bevy mixed full/item import regression suite. Released across `0.1.4` and `0.1.5`; verified with `moon test` on 2026-04-29. |
 | `WGSL-022` | GitHub issue #4 | Repeated full-module imports under a nested alias skipped cached alias redirects, leaving expressions such as `view_bindings::view` unresolved. | `DONE` | Fixed by reusing cached alias redirects when a full-module import has already been visited. Released in `0.1.6`; verified with `moon test` on 2026-04-29 (`85/85` passing). |
 | `WGSL-023` | Upstream naga_oil parity audit | The upstream compose test surface was only partially represented in local tests and not tracked as an explicit coverage matrix. | `DONE` | Added `docs/naga_oil-parity.md` with the upstream compose test/fixture matrix, source-level compatibility boundary, blocked cases, and the next architecture priorities. |
-| `WGSL-024` | Architecture review | Import syntax parsing/planning is still split between preprocessor metadata parsing and composer/module-graph import target parsing, so alias/group syntax can drift between APIs. | `TODO` | Consolidate `parse_imports` and `collect_wgsl_import_targets` around one canonical import AST/parser, then route both metadata output and recursive compose planning through that representation. |
+| `WGSL-024` | Architecture review | Import syntax parsing/planning is still split between preprocessor metadata parsing and composer/module-graph import target parsing, so alias/group syntax can drift between APIs. | `DONE` | Consolidated metadata parsing and composer import-target planning around the same tokenizer-based import target parser, while keeping the existing public `collect_wgsl_import_targets` API. Verified with `moon test` on 2026-04-29. |
 | `WGSL-025` | Upstream `additional_import` parity | Descriptor-level `additional_imports` exists, but source-level `compose_wgsl` / `export_wgsl_with_options` has no stable root-request API for injecting additional imports. | `DONE` | Added root-only `WgslComposeOptions.additional_imports`, wired descriptor-level additional imports into registered composable module source, and added compose/export regressions based on the upstream `add_imports` fixture shape. Full `virtual`/`override` overlay semantics remain part of `WGSL-012` because they require Naga. |
-| `WGSL-026` | Upstream `bad_identifiers` parity | Naga can sanitize invalid/reserved identifiers during IR writeback, but this source-level library currently lacks a defined identifier-sanitization layer. | `TODO` | Implement a limited source-level sanitizer for declarations/references the local analyzer can identify, and document that full parser/IR-equivalent sanitization remains outside source-only scope. |
-| `WGSL-027` | Upstream `invalid_override` parity | Redirect/override-style source-level APIs do not yet diagnose redirects that never match a local declaration, while upstream catches invalid override cases through Naga. | `TODO` | Add explicit redirect diagnostics, likely through export diagnostics or a checked compose API, without pretending to validate full Naga `virtual`/`override` semantics. |
+| `WGSL-026` | Upstream `bad_identifiers` parity | Naga can sanitize invalid/reserved identifiers during IR writeback, but this source-level library currently lacks a defined identifier-sanitization layer. | `DONE` | Added a root-output source-level sanitizer for invalid/reserved top-level declaration names and function parameters, plus upstream-inspired `invalid_identifiers` parity fixtures. Struct-member sanitization remains intentionally outside source-only scope. Verified with `moon test` on 2026-04-29. |
+| `WGSL-027` | Upstream `invalid_override` parity | Redirect/override-style source-level APIs do not yet diagnose redirects that never match a local declaration, while upstream catches invalid override cases through Naga. | `DONE` | Added export diagnostics for source-level redirects that never match any source identifier. Full Naga `virtual`/`override` validation remains part of `WGSL-012`. Verified with `moon test` on 2026-04-29. |
 | `WGSL-028` | Upstream `test_shader` parity | The local parity corpus lacks a simple compute shader smoke test corresponding to upstream `compute_test.wgsl`. | `DONE` | Added `testdata/upstream_compose/compute_test` and an export smoke test that preserves the compute entry point plus imported module dependency. Verified with `moon test` on 2026-04-29. |
 
 ## Current work queue
 
-- `WGSL-024`: consolidate import parsing and planning around one canonical import AST.
-- `WGSL-026`: add a documented source-level identifier sanitizer where local parsing is sufficient.
-- `WGSL-027`: add checked redirect diagnostics for source-level override/redirect usage.
+- No active `TODO` or `IN_PROGRESS` items.
 - `WGSL-012` remains intentionally `BLOCKED` on true Naga/runtime-backed parity coverage.

docs/naga_oil-parity.md

@@ -53,8 +53,8 @@ The parity target is therefore:
 | `glsl_const_import`, `glsl_wgsl_const_import`, `wgsl_glsl_const_import`, `glsl_const_import/` | Blocked | Requires GLSL parsing plus constant import/writeback semantics. |
 | `test_raycasts`, `raycast/` | Blocked | Requires Naga/wgpu-style shader validation or runtime execution behavior. |
 | `additional_import`, `add_imports/` | Covered source-level subset | Root compose/export requests and registered composable modules can inject additional imports. Upstream `virtual`/`override` overlay semantics still require Naga and remain blocked. |
-| `invalid_override` | TODO | Local redirect diagnostics should pin invalid redirect/override behavior explicitly. |
-| `bad_identifiers`, `invalid_identifiers/` | TODO | A limited source-level sanitizer is feasible for locally parsed declarations; full Naga writeback parity is not. |
+| `invalid_override` | Covered source-level subset | Export diagnostics now warn when a source-level redirect never matches. Full Naga `virtual`/`override` validation remains blocked. |
+| `bad_identifiers`, `invalid_identifiers/` | Covered source-level subset | Top-level declaration names and function parameters are sanitized in final composed/exported source. Struct-member rewriteback remains blocked without a real parser/IR. |
 | `test_shader`, `compute_test.wgsl` | Covered source-level subset | Local export smoke coverage preserves the compute entry point and imported module dependency. Upstream runtime execution remains outside source-only scope. |
 
 ## Architecture Priorities

export.mbt

@@ -79,6 +79,73 @@ fn build_wgsl_symbol_redirect_map(
   redirect_map
 }
 
+///|
+fn wgsl_identifier_is_reserved_or_invalid(ident : String) -> Bool {
+  if ident == "" {
+    return false
+  }
+  ident == "alias" ||
+  ident == "break" ||
+  ident == "case" ||
+  ident == "const" ||
+  ident == "const_assert" ||
+  ident == "continue" ||
+  ident == "continuing" ||
+  ident == "default" ||
+  ident == "diagnostic" ||
+  ident == "discard" ||
+  ident == "else" ||
+  ident == "enable" ||
+  ident == "false" ||
+  ident == "fn" ||
+  ident == "for" ||
+  ident == "if" ||
+  ident == "let" ||
+  ident == "loop" ||
+  ident == "override" ||
+  ident == "requires" ||
+  ident == "return" ||
+  ident == "struct" ||
+  ident == "switch" ||
+  ident == "true" ||
+  ident == "var" ||
+  ident == "while" ||
+  ident == "in" ||
+  ident.has_suffix("_") ||
+  starts_with_text(ident, "__")
+}
+
+///|
+fn sanitize_wgsl_identifier(ident : String) -> String {
+  let mut out = ident
+  while out.has_suffix("_") {
+    out = out[:out.length() - 1].to_string()
+  }
+  while starts_with_text(out, "__") {
+    out = out[1:out.length()].to_string()
+  }
+  if out == "" {
+    out = "wgsl"
+  }
+  if wgsl_identifier_is_reserved_or_invalid(out) {
+    out = "\{out}_wgsl"
+  }
+  out
+}
+
+///|
+fn wgsl_redirect_map_set_sanitized(
+  redirect_map : @hashmap.HashMap[String, String],
+  ident : String,
+) -> Unit {
+  if wgsl_identifier_is_reserved_or_invalid(ident) {
+    let sanitized = sanitize_wgsl_identifier(ident)
+    if sanitized != ident {
+      redirect_map.set(ident, sanitized)
+    }
+  }
+}
+
 ///|
 fn rewrite_wgsl_declaration_text_with_map(
   source : String,
@@ -240,6 +307,80 @@ fn rewrite_wgsl_declaration_text_with_renamed_declarations(
   rewrite_wgsl_declaration_text_with_map(source, redirect_map, true)
 }
 
+///|
+fn collect_wgsl_function_parameter_sanitizers(
+  block_text : String,
+  redirect_map : @hashmap.HashMap[String, String],
+) -> Unit {
+  let header_end = match block_text.find("{") {
+    Some(index) => index
+    None => block_text.length()
+  }
+  let header = block_text[:header_end].to_string()
+  guard header.find("fn ") is Some(fn_index) else { return }
+  let fn_header = header[fn_index:header.length()].to_string()
+  guard fn_header.find("(") is Some(open_index) else { return }
+  guard fn_header.find(")") is Some(close_index) else { return }
+  if close_index <= open_index {
+    return
+  }
+  let params = fn_header[open_index + 1:close_index].to_string()
+  for param_item in params.split(",") {
+    let param = param_item.to_string().trim().to_string()
+    if param == "" {
+      continue
+    }
+    let mut end = 0
+    while end < param.length() &&
+          wgsl_identifier_char(param.code_unit_at(end).to_int()) {
+      end = end + 1
+    }
+    if end > 0 {
+      wgsl_redirect_map_set_sanitized(redirect_map, param[:end].to_string())
+    }
+  }
+}
+
+///|
+fn sanitize_wgsl_invalid_identifiers(source : String) -> String {
+  let graph = build_wgsl_declaration_graph(source)
+  let global_redirects : @hashmap.HashMap[String, String] = @hashmap.HashMap::new()
+  for block in graph.declaration_blocks {
+    wgsl_redirect_map_set_sanitized(global_redirects, block.name)
+  }
+  if global_redirects.is_empty() {
+    let mut has_parameter_sanitizers = false
+    for block in graph.declaration_blocks {
+      let block_redirects : @hashmap.HashMap[String, String] = @hashmap.HashMap::new()
+      collect_wgsl_function_parameter_sanitizers(block.text, block_redirects)
+      if !block_redirects.is_empty() {
+        has_parameter_sanitizers = true
+      }
+    }
+    if !has_parameter_sanitizers {
+      return source
+    }
+  }
+  let mut sanitized = ""
+  for import_block in graph.import_blocks {
+    sanitized = sanitized + import_block.text
+  }
+  for block in graph.declaration_blocks {
+    let block_redirects : @hashmap.HashMap[String, String] = @hashmap.HashMap::new()
+    for entry in global_redirects.iter() {
+      let (from_name, to_name) = entry
+      block_redirects.set(from_name, to_name)
+    }
+    collect_wgsl_function_parameter_sanitizers(block.text, block_redirects)
+    sanitized = sanitized +
+      rewrite_wgsl_declaration_text_with_renamed_declarations(
+        block.text,
+        block_redirects,
+      )
+  }
+  sanitized
+}
+
 ///|
 pub fn rewrite_wgsl_symbol_redirects(
   source : String,
@@ -438,22 +579,55 @@ fn serialize_wgsl_export_output(
   WgslExportOutput::{ source, source_catalog, source_map, diagnostics }
 }
 
+///|
+fn append_wgsl_unused_redirect_diagnostics(
+  diagnostics : Array[WgslDiagnostic],
+  session : WgslComposeSession,
+) -> Unit {
+  for redirect in session.redirects {
+    let from_name = redirect.from_name.trim().to_string()
+    let to_name = redirect.to_name.trim().to_string()
+    if from_name == "" || to_name == "" || from_name == to_name {
+      continue
+    }
+    let matched = match session.redirect_match_counts.get(from_name) {
+      Some(count) => count > 0
+      None => false
+    }
+    if !matched {
+      push_wgsl_diagnostic(
+        diagnostics,
+        WgslDiagnosticSeverity::Warn,
+        "symbol redirect did not match any source identifier",
+        None,
+        Some(from_name),
+        None,
+        None,
+      )
+    }
+  }
+}
+
 ///|
 fn Composer::export_wgsl_from_session(
   self : Composer,
   rel : String,
   session : WgslComposeSession,
   options : WgslExportOptions,
 ) -> WgslExportOutput raise ComposerError {
-  let source = self.load_root_wgsl_preprocessed_into_session(rel, session)
+  let source = sanitize_wgsl_invalid_identifiers(
+    self.load_root_wgsl_preprocessed_into_session(rel, session),
+  )
   let graph = build_wgsl_declaration_graph(source)
-  serialize_wgsl_export_output(
+  let output = serialize_wgsl_export_output(
     graph,
     copy_wgsl_source_files_from_registry(session.resolved_source_files),
     options,
     session.defines,
     session.value_defines,
   )
+  append_wgsl_unused_redirect_diagnostics(output.diagnostics, session)
+  output
 }
 
 ///|

export_test.mbt

@@ -281,3 +281,29 @@ test "naga_oil: export_wgsl_with_redirects rewrites dependencies before tree-sha
   debug_inspect(output.diagnostics.length(), content="0")
   @moon_wgsl.clear_registered_wgsl_source_registry()
 }
+
+///|
+test "naga_oil: export warns when a source-level redirect never matches" {
+  register_redirect_export_test_shaders()
+  let output = export_test_global_registry_composer().export_wgsl_with_options(
+    "shaders/effects/redirect.wgsl",
+    export_test_compose_options(export_test_empty_defines(), [
+      { from_name: "missing_shadow", to_name: "build_color" },
+    ]),
+    { root_items: ["shade"] },
+  ) catch {
+    err =>
+      abort("expected redirect diagnostic export success: \{err.message()}")
+  }
+  debug_inspect(output.diagnostics.length(), content="1")
+  debug_inspect(output.diagnostics[0].severity, content="Warn")
+  debug_inspect(
+    output.diagnostics[0].message,
+    content="\"symbol redirect did not match any source identifier\"",
+  )
+  debug_inspect(
+    output.diagnostics[0].symbol_name,
+    content="Some(\"missing_shadow\")",
+  )
+  @moon_wgsl.clear_registered_wgsl_source_registry()
+}