From 5f728aeca3be364d462e6e20f41cb5cdba5e99d4 Mon Sep 17 00:00:00 2001
From: olabusayoT <50379531+olabusayoT@users.noreply.github.com>
Date: Mon, 14 Oct 2024 20:07:37 -0400
Subject: [PATCH] fixup! Add support for lengthUnit=bits for any type

- remove no-op checkLengthUnits function
- allow lengthUnits bits for binarySeconds/milliSeconds for xs:datetime
- remove SDE for requirement that lengthUnit=bytes for binarySeconds/milliSeconds
- add tests for xs:double/xs:float verifying LU=bits
- add tests for double/float showing runtime lengths not allowed
- add tests for xs:dateTime with binaryCalendarRep set to binarySeconds/milliSeconds
- add test for xs:date with binaryCalendarRep set to binarySeconds/milliSeconds (SDEs)

DAFFODIL-2931
---
 .../grammar/ElementBaseGrammarMixin.scala     |  24 +---
 .../section05/simple_types/SimpleTypes.tdml   |  56 +++++++-
 .../section12/lengthKind/ExplicitTests.tdml   | 133 ++++++++++++++++++
 .../simple_types/TestSimpleTypes.scala        |   3 +
 .../lengthKind/TestLengthKindExplicit.scala   |  21 ++-
 5 files changed, 211 insertions(+), 26 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala
index 43589cd096..c9ca196cd2 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala
@@ -110,7 +110,6 @@ trait ElementBaseGrammarMixin
   lazy val prefixedLengthElementDecl: PrefixLengthQuasiElementDecl =
     LV('prefixedLengthElementDecl) {
       Assert.invariant(lengthKind == LengthKind.Prefixed)
-      checkLengthUnits()
       val detachedNode =
         <element name={name + " (prefixLength)"} type={prefixLengthType.toQNameString}/>
           .copy(scope = prefixLengthTypeGSTD.xml.scope)
@@ -591,7 +590,6 @@ trait ElementBaseGrammarMixin
     val lengthFromProp: JLong = repElement.lengthEv.optConstant.get
     val nbits = repElement.lengthUnits match {
       case LengthUnits.Bits =>
-        checkLengthUnits(repElement)
         lengthFromProp.longValue()
       case LengthUnits.Bytes => lengthFromProp.longValue() * 8
       case LengthUnits.Characters =>
@@ -1069,10 +1067,8 @@ trait ElementBaseGrammarMixin
         (primType, binaryCalendarRep) match {
           case (PrimType.DateTime, BinaryCalendarRep.BinarySeconds) =>
             (lengthUnits, binaryNumberKnownLengthInBits) match {
-              case (LengthUnits.Bytes, 32) =>
+              case (LengthUnits.Bytes | LengthUnits.Bits, 32) =>
                 new ConvertBinaryDateTimeSecMilliPrim(this, binaryNumberKnownLengthInBits)
-              case (_, 32) =>
-                SDE("lengthUnits must be 'bytes' when binaryCalendarRep='binarySeconds'")
               case (_, n) =>
                 SDE(
                   "binary xs:dateTime must be 32 bits when binaryCalendarRep='binarySeconds'. Length in bits was %s.",
@@ -1083,10 +1079,8 @@ trait ElementBaseGrammarMixin
             SDE("binaryCalendarRep='binarySeconds' is not allowed with type %s", primType.name)
           case (PrimType.DateTime, BinaryCalendarRep.BinaryMilliseconds) =>
             (lengthUnits, binaryNumberKnownLengthInBits) match {
-              case (LengthUnits.Bytes, 64) =>
+              case (LengthUnits.Bytes | LengthUnits.Bits, 64) =>
                 new ConvertBinaryDateTimeSecMilliPrim(this, binaryNumberKnownLengthInBits)
-              case (_, 64) =>
-                SDE("lengthUnits must be 'bytes' when binaryCalendarRep='binaryMilliseconds'")
               case (_, n) =>
                 SDE(
                   "binary xs:dateTime must be 64 bits when binaryCalendarRep='binaryMilliseconds'. Length in bits was %s.",
@@ -1713,18 +1707,4 @@ trait ElementBaseGrammarMixin
     prod("mandatoryTextAlignment", impliedRepresentation eq Representation.Text) {
       mtaBase
     }
-
-  private def checkLengthUnits(elem: ElementBase = context): Unit = {
-    elem.lengthUnits match {
-      case LengthUnits.Bits =>
-        elem.optPrimType match {
-          case Some(_) => {
-            // allow all types to use lengthUnits bits
-            // PORTABILITY: See https://github.com/OpenGridForum/DFDL/issues/12
-          }
-          case None => // do nothing
-        }
-      case _ =>
-    }
-  }
 }
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
index 8593f015de..91ad8fb287 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section05/simple_types/SimpleTypes.tdml
@@ -276,6 +276,12 @@
       dfdl:binaryCalendarRep="binarySeconds" dfdl:binaryCalendarEpoch="01-01-2000T00:00:00"/>
     <xs:element name="dateTimeBin12" type="xs:dateTime" dfdl:lengthKind="delimited" dfdl:terminator=";"
       dfdl:binaryCalendarRep="binarySeconds" dfdl:binaryCalendarEpoch="1977-01-01T00:00:07"/>
+    <xs:element name="dateTimeBin13" type="xs:dateTime" dfdl:lengthKind="explicit" dfdl:length="{ 32 }"
+                dfdl:lengthUnits="bits" dfdl:binaryCalendarRep="binarySeconds" dfdl:binaryCalendarEpoch="2018-01-01T09:13:42+09:00"/>
+    <xs:element name="dateTimeBin14" type="xs:dateTime" dfdl:lengthKind="explicit" dfdl:length="{ 64 }"
+                dfdl:lengthUnits="bits" dfdl:binaryCalendarRep="binaryMilliseconds" dfdl:binaryCalendarEpoch="2000-06-15T03:25:19"/>
+    <xs:element name="dateTimeBin15" type="xs:date" dfdl:lengthKind="explicit" dfdl:length="{ 64 }"
+                dfdl:lengthUnits="bits" dfdl:binaryCalendarRep="binaryMilliseconds" dfdl:binaryCalendarEpoch="2000-06-15T03:25:19"/>
 
     <xs:element name="dateBinBCD" type="xs:date" dfdl:calendarPattern="MMddyy" dfdl:calendarPatternKind="explicit"
       dfdl:lengthKind="explicit" dfdl:length="{ 3 }" dfdl:lengthUnits="bytes" dfdl:binaryCalendarRep="bcd" />
@@ -2413,9 +2419,11 @@
     <tdml:document>
       <tdml:documentPart type="bits">00000000 00000000 00000000 00111101</tdml:documentPart>
     </tdml:document>
-    <tdml:errors>
-      <tdml:error>Schema Definition Error: lengthUnits must be 'bytes' when binaryCalendarRep='binarySeconds'</tdml:error>
-    </tdml:errors>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:dateTimeBin2 xmlns:ex="http://example.com">1970-01-01T00:01:01</ex:dateTimeBin2>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
   </tdml:parserTestCase>
 
   <tdml:parserTestCase name="dateTimeBin3" root="dateTimeBin3"
@@ -2700,6 +2708,48 @@
     </tdml:errors>
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="dateTimeBin23" root="dateTimeBin13"
+                       model="SimpleTypes-binary" description="binarySeconds with LengthUnits=bits"
+                       roundTrip="true">
+
+    <tdml:document>
+      <tdml:documentPart type="bits">00000000 00000000 00000000 00000001</tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <dateTimeBin13>2018-01-01T09:13:43+09:00</dateTimeBin13>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="dateTimeBin24" root="dateTimeBin14"
+                       model="SimpleTypes-binary" description="binaryMilliSeconds with lengthUnits=bits"
+                       roundTrip="true">
+
+    <tdml:document>
+      <tdml:documentPart type="bits">00000000 00000000 00000000 00000000 11111111 11111111 11111111 11111111</tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <dateTimeBin14>2000-08-03T20:28:06.295000</dateTimeBin14>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="dateTimeBin25" root="dateTimeBin15"
+                       model="SimpleTypes-binary" description="binaryMilliSeconds with lengthUnits=bits"
+                       roundTrip="true">
+
+    <tdml:document>
+      <tdml:documentPart type="bits">00000000 00000000 00000000 00000000 11111111 11111111 11111111 11111111</tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>binaryCalendarRep='binaryMilliseconds'</tdml:error>
+      <tdml:error>not allowed</tdml:error>
+      <tdml:error>type date</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
   <tdml:parserTestCase name="dateBinBCD" root="dateBinBCD"
     model="SimpleTypes-binary" description="Section 5 Schema types-dateTime - DFDL-5-016R"
     roundTrip="true">
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/ExplicitTests.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/ExplicitTests.tdml
index 2af41931b8..d55a92948c 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/ExplicitTests.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/ExplicitTests.tdml
@@ -1050,6 +1050,139 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+<!--
+     Test Name: lengthUnitsBitsForFloat_explicit
+        Schema: lengthUnitsBitsForFloat_explicit
+          Root: r1
+       Purpose: This test checks that no warning is emitted when a float/double is used when lengthUnits='bits'.
+-->
+  <tdml:defineSchema name="lengthUnitsBitsForFloatDouble_explicit">
+    <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:defineFormat name="Binary">
+      <dfdl:format ref="ex:GeneralFormat" representation="binary"/>
+    </dfdl:defineFormat>
+    <dfdl:format ref="ex:Binary" />
+
+    <xs:element name="r1" dfdl:lengthKind="explicit" type="xs:float"
+                dfdl:lengthUnits="bits" dfdl:length="32"/>
+    <xs:element name="r2" dfdl:lengthKind="explicit" type="xs:double"
+                dfdl:lengthUnits="bits" dfdl:length="64"/>
+    <xs:element name="r3" dfdl:lengthKind="explicit" type="xs:float"
+                dfdl:lengthUnits="bits" dfdl:length="8"/>
+    <xs:element name="r4" dfdl:lengthKind="explicit" type="xs:double"
+                dfdl:lengthUnits="bits" dfdl:length="32"/>
+    <xs:element name="r5">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="len" type="xs:int" dfdl:inputValueCalc="{ 32 }"/>
+          <xs:element name="e1" dfdl:lengthKind="explicit" type="xs:float"
+                      dfdl:lengthUnits="bits" dfdl:length="{ ../ex:len }"/>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+    <xs:element name="r6">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="len" type="xs:int" dfdl:inputValueCalc="{ 64 }"/>
+          <xs:element name="e1" dfdl:lengthKind="explicit" type="xs:double"
+                      dfdl:lengthUnits="bits" dfdl:length="{ ../ex:len }"/>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="lengthUnitsBitsForFloat_explicit1" root="r1" model="lengthUnitsBitsForFloatDouble_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:float'"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
+      <tdml:documentPart type="byte">
+        42 f6 e9 79
+      </tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <r1>123.456</r1>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="lengthUnitsBitsForFloat_explicit2" root="r3" model="lengthUnitsBitsForFloatDouble_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:float'"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
+      <tdml:documentPart type="byte">
+        42 f6 e9 79
+      </tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>must be 32 bits</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="lengthUnitsBitsForFloat_explicit3" root="r5" model="lengthUnitsBitsForFloatDouble_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:float'"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
+      <tdml:documentPart type="byte">
+        42 f6 e9 79
+      </tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>floating point binary numbers</tdml:error>
+      <tdml:error>may not have runtime-specified lengths</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="lengthUnitsBitsForDouble_explicit1" root="r2" model="lengthUnitsBitsForFloatDouble_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:double'"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
+      <tdml:documentPart type="byte">
+        40 5e dd 2f 1a 9f be 77
+      </tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <r2>123.456</r2>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="lengthUnitsBitsForDouble_explicit2" root="r4" model="lengthUnitsBitsForFloatDouble_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:double'"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
+      <tdml:documentPart type="byte">
+        40 5e dd 2f 1a 9f be 77
+      </tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>must be 64 bits</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="lengthUnitsBitsForDouble_explicit3" root="r6" model="lengthUnitsBitsForFloatDouble_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:double'"
+                       ignoreUnexpectedWarnings="false">
+
+    <tdml:document>
+      <tdml:documentPart type="byte">
+        40 5e dd 2f 1a 9f be 77
+      </tdml:documentPart>
+    </tdml:document>
+    <tdml:errors>
+      <tdml:error>floating point binary numbers</tdml:error>
+      <tdml:error>may not have runtime-specified lengths</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
   <!--
     These tests check for the condition where the bit length exceeds the width of the
     specified type when the value is a constant. All numeric signed and unsigned types are checked
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala
index f2997d9500..126a11df4a 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section05/simple_types/TestSimpleTypes.scala
@@ -311,6 +311,9 @@ class TestSimpleTypes {
   @Test def test_dateTimeBin20(): Unit = { runner.runOneTest("dateTimeBin20") }
   @Test def test_dateTimeBin21(): Unit = { runner.runOneTest("dateTimeBin21") }
   @Test def test_dateTimeBin22(): Unit = { runner.runOneTest("dateTimeBin22") }
+  @Test def test_dateTimeBin23(): Unit = { runner.runOneTest("dateTimeBin23") }
+  @Test def test_dateTimeBin24(): Unit = { runner.runOneTest("dateTimeBin24") }
+  @Test def test_dateTimeBin25(): Unit = { runner.runOneTest("dateTimeBin25") }
   @Test def test_dateBinBCD(): Unit = { runner.runOneTest("dateBinBCD") }
   @Test def test_dateBinBCD2(): Unit = { runner.runOneTest("dateBinBCD2") }
   @Test def test_dateBinBCD3(): Unit = { runner.runOneTest("dateBinBCD3") }
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindExplicit.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindExplicit.scala
index c51a40abf7..f8a62cb6de 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindExplicit.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindExplicit.scala
@@ -98,15 +98,34 @@ class TestLengthKindExplicit {
     runner.runOneTest("denseBit_lengthKind_explicit")
   }
 
+  // DFDL-2931
   @Test def test_lengthUnitsBitsForInteger_explicit(): Unit = {
     runner.runOneTest("lengthUnitsBitsForInteger_explicit")
   }
   @Test def test_lengthUnitsBitsForInteger_explicit2(): Unit = {
     runner.runOneTest("lengthUnitsBitsForInteger_explicit2")
   }
-  @Test def lengthUnitsBitsForDecimal_explicit(): Unit = {
+  @Test def test_lengthUnitsBitsForDecimal_explicit(): Unit = {
     runner.runOneTest("lengthUnitsBitsForDecimal_explicit")
   }
+  @Test def test_lengthUnitsBitsForFloat_explicit1(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForFloat_explicit1")
+  }
+  @Test def test_lengthUnitsBitsForFloat_explicit2(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForFloat_explicit2")
+  }
+  @Test def test_lengthUnitsBitsForFloat_explicit3(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForFloat_explicit3")
+  }
+  @Test def test_lengthUnitsBitsForDouble_explicit1(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForDouble_explicit1")
+  }
+  @Test def test_lengthUnitsBitsForDouble_explicit2(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForDouble_explicit2")
+  }
+  @Test def test_lengthUnitsBitsForDouble_explicit3(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForDouble_explicit3")
+  }
 
   @Test def test_invalidUnsignedLongBitLength(): Unit = {
     runner.runOneTest("invalidUnsignedLongBitLength")