From ea41b08dcfbac5bb1702ca75b72bffe2ef96fd2e Mon Sep 17 00:00:00 2001
From: Anton Lavrik <alavrik@piqi.org>
Date: Sat, 28 Sep 2019 00:16:42 +0100
Subject: [PATCH] Fix parsing record fields with .piq-positional

See allow_omit_label_invalid.piq for details
---
 piqilib/piqobj_of_piq.ml               | 13 +++++--
 tests/piq/Makefile                     |  3 ++
 tests/piq/allow_omit_label_invalid.piq | 27 ++++++++++++++
 tests/piq/allow_omit_label_valid.piq   | 50 ++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 tests/piq/allow_omit_label_invalid.piq
 create mode 100644 tests/piq/allow_omit_label_valid.piq

diff --git a/piqilib/piqobj_of_piq.ml b/piqilib/piqobj_of_piq.ml
index c2756f63..5d5b8a98 100644
--- a/piqilib/piqobj_of_piq.ml
+++ b/piqilib/piqobj_of_piq.ml
@@ -653,12 +653,17 @@ and parse_option_by_type ~try_mode o x =
           | _ ->
               None
         )
-    | _, Some t ->
+    (* TODO: do not support this behavior by default, only when
+     * piq-allow-omit-label is specified for options
+     *
+     * TODO: unify .piq-allow-omit-label and .piq-positional
+     *)
+    | None, Some t when o.piq_alias = None ->
         let do_parse () =
           let obj = Some (parse_obj t x ?piq_format:o.piq_format) in
           Some O.({t = o; obj = obj})
         in
-        match C.unalias t, x with
+        (match C.unalias t, x with
           | `bool, `bool _
 
           | `int, `int _
@@ -693,7 +698,9 @@ and parse_option_by_type ~try_mode o x =
           | `binary, `string (s, _) when Piq_lexer.is_ascii_string s -> do_parse ()
           | _ ->
               None
-
+        )
+    | _, Some _ ->  (* either name or piq_alias are defined *)
+        None
 
 
 and parse_name_option o name =
diff --git a/tests/piq/Makefile b/tests/piq/Makefile
index 2b8f03d6..0ea4d221 100644
--- a/tests/piq/Makefile
+++ b/tests/piq/Makefile
@@ -18,6 +18,9 @@ test:
 	$(PIQI) check --type options/v options_invalid.piq; test $$? -eq 1
 	$(PIQI) check --type options/v options_valid.piq
 
+	$(PIQI) check allow_omit_label_invalid.piq; test $$? -eq 1
+	$(PIQI) check allow_omit_label_valid.piq
+
 
 clean:
 	rm -f *.pib *.json
diff --git a/tests/piq/allow_omit_label_invalid.piq b/tests/piq/allow_omit_label_invalid.piq
new file mode 100644
index 00000000..eab6bb20
--- /dev/null
+++ b/tests/piq/allow_omit_label_invalid.piq
@@ -0,0 +1,27 @@
+:piqi [
+    .module m
+
+    .record [
+        .name r
+
+        .field [
+            .name f
+            .type v
+            .piq-positional true
+        ]
+    ]
+
+    .variant [
+        .name v
+
+        .option [
+            .piq-alias oo
+            .type string
+        ]
+    ]
+]
+
+:m/r [
+    % this is invalid, because there is .piq-alias in option definition
+    "foo"
+]
diff --git a/tests/piq/allow_omit_label_valid.piq b/tests/piq/allow_omit_label_valid.piq
new file mode 100644
index 00000000..1f9cacfa
--- /dev/null
+++ b/tests/piq/allow_omit_label_valid.piq
@@ -0,0 +1,50 @@
+:piqi [
+    .module m
+
+    .record [
+        .name r
+
+        .field [
+            .name f
+            .type v
+            .piq-positional true
+        ]
+    ]
+
+    .record [
+        .name rr
+
+        .field [
+            .name f
+            .type vv
+            .piq-positional true
+        ]
+    ]
+
+    .variant [
+        .name v
+
+        .option [
+            .piq-alias oo
+            .type string
+        ]
+    ]
+
+    .variant [
+        .name vv
+
+        .option [
+            .type string
+        ]
+    ]
+]
+
+:m/r [
+    % this is valid, because of .piq-positional
+    .oo "foo"
+]
+
+:m/rr [
+    % this is valid, because of a) .piq-positional and b) there is no .name nor .piq-alias in option definition
+    "foo"
+]