Skip to content

Commit 5ac4bf9

Browse files
cristianocclaude
andcommitted
Print exotic import-attribute keys in escaped form
Escaped identifiers also arrive as Lident, so keys like \"some-identifier" are stored verbatim and must print escaped again to be writable source; only special-casing the type_ -> type normalization was insufficient. Use the printer's identifier classification, and pin the display with a fixture (registered in the catalog). Import attributes on relative-path modules never reach spec comparison - the relative flag deliberately erases the primitive in signatures - so the fixture uses a package module name. Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
1 parent 4d6c249 commit 5ac4bf9

5 files changed

Lines changed: 36 additions & 6 deletions

File tree

compiler/syntax/src/res_outcome_printer.ml

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -513,11 +513,15 @@ let print_external_module_doc (emn : External_ffi_types.external_module_name) =
513513
let with_fields =
514514
import_attributes
515515
|> List.map (fun (k, v) ->
516-
(* digestion stores the source key [type_] as [type]; map it
517-
back so the printed record is writable source *)
518-
let k = if k = "type" then "type_" else k in
519-
Doc.concat
520-
[Doc.text k; Doc.text ": "; print_string_literal_doc v])
516+
(* digestion stores the source key [type_] as [type]; other
517+
keys are stored as written, including exotic ones from
518+
escaped idents (\"some-identifier"), which must print
519+
escaped again to be writable source *)
520+
let key =
521+
if k = "type" then Doc.text "type_"
522+
else Res_printer.print_ident_like k
523+
in
524+
Doc.concat [key; Doc.text ": "; print_string_literal_doc v])
521525
in
522526
Doc.concat
523527
[

tests/ERROR_VARIANTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ Wrapper symptoms attached to inclusion failures. Source: [includemod.ml:23](../c
357357
| Variant | Status | Fixture | Notes |
358358
|---|---|---|---|
359359
| `Missing_field` || `super_errors_multi/Iface_missing_value` | |
360-
| `Value_descriptions` || `super_errors_multi/Iface_value_descriptions`, `super_errors_multi/Iface_value_arity_mismatch`, `super_errors_multi/Smoke_interface_mismatch`, `super_errors_multi/Cross_external_spec_mismatch`, `super_errors_multi/Cross_external_payload_name`, `module_sig_value_arity_mismatch*.res` | Arity mismatches print a dedicated hint (implementation vs interface argument counts), including through aliases and nested function types. |
360+
| `Value_descriptions` || `super_errors_multi/Iface_value_descriptions`, `super_errors_multi/Iface_value_arity_mismatch`, `super_errors_multi/Smoke_interface_mismatch`, `super_errors_multi/Cross_external_spec_mismatch`, `super_errors_multi/Cross_external_payload_name`, `super_errors_multi/Cross_external_import_attrs`, `module_sig_value_arity_mismatch*.res` | Arity mismatches print a dedicated hint (implementation vs interface argument counts), including through aliases and nested function types. |
361361
| `Type_declarations` || `super_errors_multi/Iface_type_decl_record`, `super_errors_multi/Iface_type_decl_variant`, `RecordInclusion.res`, `type_decl_function_arity_mismatch.res` | |
362362
| `Extension_constructors` || `super_errors_multi/Iface_extension_constructors` | |
363363
| `Module_types` || `super_errors_multi/Iface_module_types` | |
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
===== Importattrs.res =====
2+
3+
We've found a bug for you!
4+
/.../fixtures/Cross_external_import_attrs/Importattrs.res:1:1-2:30
5+
6+
1 │ @module({from: "somepkg", with: {type_: "json", \"some-identifier": "nop
7+
│ e"}})
8+
2 │ external data: int = "default"
9+
3 │
10+
11+
The implementation /.../fixtures/Cross_external_import_attrs/Importattrs.res
12+
does not match the interface /.../fixtures/Cross_external_import_attrs/importattrs.cmi:
13+
Values do not match:
14+
@val @module({from: "somepkg", with: {type_: "json", \"some-identifier": "nope"}}) external data: int =
15+
"default"
16+
is not included in
17+
@val @module({from: "somepkg", with: {type_: "json", \"some-identifier": "yep"}}) external data: int =
18+
"default"
19+
/.../fixtures/Cross_external_import_attrs/Importattrs.resi:1:1-2:30:
20+
Expected declaration
21+
/.../fixtures/Cross_external_import_attrs/Importattrs.res:1:1-2:30:
22+
Actual declaration
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
@module({from: "somepkg", with: {type_: "json", \"some-identifier": "nope"}})
2+
external data: int = "default"
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
@module({from: "somepkg", with: {type_: "json", \"some-identifier": "yep"}})
2+
external data: int = "default"

0 commit comments

Comments
 (0)