From 0e041364e7b8354e046e1898884e5d73554dd8b5 Mon Sep 17 00:00:00 2001
From: olabusayoT <50379531+olabusayoT@users.noreply.github.com>
Date: Mon, 23 Sep 2024 13:29:48 -0400
Subject: [PATCH] Add support for lengthUnit=bits for any type

- verify test that uses lengthUnit=bits with calendar type no longer produces warning
- verify test that uses lengthUnit=bits with integer type no longer produces warning
- verify test that uses lengthUnit=bits with decimal type no longer produces warning
- deprecate allowBigIntegerBits tunable and the DeprecatedBigIntegerBits WarnID

Deprecation/Compatibility
- allowBigIntegerBits tunable no longer has any effect. All types are allowed to use lengthUnit=bits.
- WarnID DeprecatedBigIntegerBits has been deprecated. It is no longer in use in the codebase.

DAFFODIL-2931
---
 .../grammar/ElementBaseGrammarMixin.scala     | 32 ++-------
 .../org/apache/daffodil/xsd/dafext.xsd        | 15 +++--
 .../section05/simple_types/SimpleTypes.tdml   | 17 +++++
 .../section12/lengthKind/ExplicitTests.tdml   | 65 +++++++++----------
 .../section12/lengthKind/PrefixedTests.tdml   | 12 ++--
 .../simple_types/TestSimpleTypes.scala        |  1 +
 .../lengthKind/TestLengthKindExplicit.scala   | 12 ++--
 .../lengthKind/TestLengthKindPrefixed.scala   |  4 +-
 8 files changed, 76 insertions(+), 82 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 d230c94f05..43589cd096 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
@@ -1714,35 +1714,15 @@ trait ElementBaseGrammarMixin
       mtaBase
     }
 
-  val allowedBitTypes = Set[PrimType](
-    PrimType.Boolean,
-    PrimType.Byte,
-    PrimType.Short,
-    PrimType.Int,
-    PrimType.Long,
-    PrimType.UnsignedByte,
-    PrimType.UnsignedShort,
-    PrimType.UnsignedInt,
-    PrimType.UnsignedLong
-  )
-  val allowedBitTypesText = allowedBitTypes.map("xs:" + _.toString).toList.sorted.mkString(", ")
-
   private def checkLengthUnits(elem: ElementBase = context): Unit = {
     elem.lengthUnits match {
-      case LengthUnits.Bits if elem.representation == Representation.Binary =>
+      case LengthUnits.Bits =>
         elem.optPrimType match {
-          case Some(primType) =>
-            if (!allowedBitTypes.contains(primType))
-              if (tunable.allowBigIntegerBits)
-                elem.SDW(
-                  WarnID.DeprecatedBigIntegerBits,
-                  s"In a future release, lengthUnits='bits' will only be supported for the following types: $allowedBitTypesText"
-                )
-              else
-                elem.SDE(
-                  "lengthUnits='bits' is only supported for the following types: $allowedBitTypesText"
-                )
-          case None =>
+          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-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd b/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd
index 3d89c56617..1f776029d9 100644
--- a/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd
+++ b/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd
@@ -104,12 +104,8 @@
         <xs:element name="allowBigIntegerBits" type="xs:boolean" default="true" minOccurs="0">
           <xs:annotation>
             <xs:documentation>
-              Previous Daffodil releases let schemas define every type's length using "bits" as the length unit
-              even though the specification allows bit length units only for a specific set of types' binary
-              representations and does not allow bit length units for any other type's binary representation
-              or any type's text representation.  When this tunable is true, a deprecation warning is issued
-              when bit length units are incorrectly used.  When this tunable is false, a schema definition
-              error will be issued instead.
+              Deprecated. This tunable no longer has any affect and is only kept for
+              backwards compatability.
             </xs:documentation>
           </xs:annotation>
         </xs:element>
@@ -696,7 +692,12 @@
           <xs:enumeration value="appinfoNoSource" />
           <xs:enumeration value="choiceInsideHiddenGroup" />
           <xs:enumeration value="codeGenerator" />
-          <xs:enumeration value="deprecatedBigIntegerBits" />
+          <!-- deprecated -->
+          <xs:enumeration value="deprecatedBigIntegerBits">
+            <xs:annotation>
+              <xs:documentation>Deprecated.</xs:documentation>
+            </xs:annotation>
+          </xs:enumeration>
           <xs:enumeration value="deprecatedBuiltInFormats" />
           <xs:enumeration value="deprecatedEncodingNameUSASCII7BitPacked" />
           <xs:enumeration value="deprecatedExpressionResultCoercion" />
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 d3562569ba..8593f015de 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
@@ -338,6 +338,9 @@
       dfdl:lengthKind="explicit" dfdl:length="{ 3 }" dfdl:lengthUnits="bytes" dfdl:binaryCalendarRep="ibm4690Packed" />
     <xs:element name="dateBinIBM4690Packed2" type="xs:date" dfdl:calendarPattern="MMddyy" dfdl:calendarPatternKind="explicit"
       dfdl:lengthKind="explicit" dfdl:length="{ 18 }" dfdl:lengthUnits="bits" dfdl:binaryCalendarRep="ibm4690Packed"/>
+    <xs:element name="dateBinIBM4690Packed3" type="xs:date" dfdl:calendarPattern="MMddyy" dfdl:calendarPatternKind="explicit"
+                dfdl:lengthKind="explicit" dfdl:length="{ 24 }" dfdl:lengthUnits="bits" dfdl:binaryCalendarRep="ibm4690Packed" />
+
 
     <xs:element name="dateTimeBinIBM4690Packed">
       <xs:complexType>
@@ -2992,6 +2995,20 @@
     </tdml:errors>
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="dateBinIBM4690Packed4" root="dateBinIBM4690Packed3"
+                       model="SimpleTypes-binary" description="binary packed calendar representation with lengthUnit=bits"
+                       roundTrip="true" ignoreUnexpectedWarnings="false">
+
+    <tdml:document>
+      <tdml:documentPart type="byte">12 14 23</tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <dateBinIBM4690Packed3>2023-12-14</dateBinIBM4690Packed3>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
   <tdml:parserTestCase name="dateTimeBinIBM4690Packed" root="dateTimeBinIBM4690Packed"
     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 b622a67eb2..2af41931b8 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
@@ -962,12 +962,12 @@
   </tdml:parserTestCase>
 
 <!--
-     Test Name: invalidLengthUnitsIntegerWarning_explicit
-        Schema: invalidLengthUnitsInteger_explicit
+     Test Name: lengthUnitsBitsForInteger_explicit
+        Schema: lengthUnitsBitsForInteger_explicit
           Root: r1
-       Purpose: This test checks that a warning is emitted when a bigint is used when lengthUnits='bits'.
+       Purpose: This test checks that no warning is emitted when a bigint is used when lengthUnits='bits'.
 -->
-  <tdml:defineSchema name="invalidLengthUnitsInteger_explicit">
+  <tdml:defineSchema name="lengthUnitsBitsForInteger_explicit">
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:defineFormat name="Binary">
       <dfdl:format ref="ex:GeneralFormat" representation="binary"/>
@@ -977,51 +977,52 @@
     <xs:element name="r1" dfdl:lengthKind="explicit" type="xs:integer" dfdl:lengthUnits="bits" dfdl:length="8"/>
   </tdml:defineSchema>
 
-  <tdml:parserTestCase name="invalidLengthUnitsIntegerWarning_explicit" root="r1" model="invalidLengthUnitsInteger_explicit"
-                       description="lengthKind='explicit' lengthUnits='bits' type='xs:integer'">
+  <tdml:parserTestCase name="lengthUnitsBitsForInteger_explicit" root="r1" model="lengthUnitsBitsForInteger_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:integer'"
+                       ignoreUnexpectedWarnings="false">
     <tdml:document>
       <tdml:documentPart type="byte">
         0f
       </tdml:documentPart>
     </tdml:document>
     <tdml:infoset>
-      <tdml:dfdlInfoset>f
+      <tdml:dfdlInfoset>
         <r1>15</r1>
       </tdml:dfdlInfoset>
     </tdml:infoset>
-    <tdml:warnings>
-      <tdml:warning>lengthUnits='bits'</tdml:warning>
-      <tdml:warning>will only be supported</tdml:warning>
-    </tdml:warnings>
   </tdml:parserTestCase>
 
 <!--
-     Test Name: invalidLengthUnitsIntegerError_explicit
-        Schema: invalidLengthUnitsInteger_explicit
+     Test Name: lengthUnitsBitsForInteger_explicit2
+        Schema: lengthUnitsBitsForInteger_explicit
           Root: r1
-       Purpose: This test checks that an error is raised when a bigint is used when lengthUnits='bits'.
+       Purpose: This test verifies that no error is raised when a bigint is used when lengthUnits='bits'
+       and to show that the allowBigIntegerBits tunable has no effect.
 -->
-  <tdml:parserTestCase name="invalidLengthUnitsIntegerError_explicit" root="r1" model="invalidLengthUnitsInteger_explicit"
+  <tdml:parserTestCase name="lengthUnitsBitsForInteger_explicit2" root="r1" model="lengthUnitsBitsForInteger_explicit"
                        description="lengthKind='explicit' lengthUnits='bits' type='xs:integer' error"
-                       config="enforceBigIntegerBits">
-    <tdml:document>
+                       config="enforceBigIntegerBits"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
       <tdml:documentPart type="byte">
         0f
       </tdml:documentPart>
     </tdml:document>
-    <tdml:errors>
-      <tdml:error>lengthUnits='bits'</tdml:error>
-      <tdml:error>is only supported</tdml:error>
-    </tdml:errors>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <r1>15</r1>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
   </tdml:parserTestCase>
 
 <!--
-     Test Name: invalidLengthUnitsDecimalWarning_explicit
-        Schema: invalidLengthUnitsDecimal_explicit
+     Test Name: lengthUnitsBitsForDecimal_explicit
+        Schema: lengthUnitsBitsForDecimal_explicit
           Root: r1
-       Purpose: This test checks that a warning is emitted when a decimal is used when lengthUnits='bits'.
+       Purpose: This test checks that no warning is emitted when a decimal is used when lengthUnits='bits'.
 -->
-  <tdml:defineSchema name="invalidLengthUnitsDecimal_explicit">
+  <tdml:defineSchema name="lengthUnitsBitsForDecimal_explicit">
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:defineFormat name="Binary">
       <dfdl:format ref="ex:GeneralFormat" representation="binary"/>
@@ -1033,22 +1034,20 @@
                 dfdl:binaryDecimalVirtualPoint="2"/>
   </tdml:defineSchema>
 
-  <tdml:parserTestCase name="invalidLengthUnitsDecimalWarning_explicit" root="r1" model="invalidLengthUnitsDecimal_explicit"
-                       description="lengthKind='explicit' lengthUnits='bits' type='xs:decimal'">
-    <tdml:document>
+  <tdml:parserTestCase name="lengthUnitsBitsForDecimal_explicit" root="r1" model="lengthUnitsBitsForDecimal_explicit"
+                       description="lengthKind='explicit' lengthUnits='bits' type='xs:decimal'"
+                       ignoreUnexpectedWarnings="false">
+
+  <tdml:document>
       <tdml:documentPart type="byte">
         0f
       </tdml:documentPart>
     </tdml:document>
     <tdml:infoset>
-      <tdml:dfdlInfoset>f
+      <tdml:dfdlInfoset>
         <r1>0.15</r1>
       </tdml:dfdlInfoset>
     </tdml:infoset>
-    <tdml:warnings>
-      <tdml:warning>lengthUnits='bits'</tdml:warning>
-      <tdml:warning>will only be supported</tdml:warning>
-    </tdml:warnings>
   </tdml:parserTestCase>
 
   <!--
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/PrefixedTests.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/PrefixedTests.tdml
index 82d201a360..9752067ce2 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/PrefixedTests.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section12/lengthKind/PrefixedTests.tdml
@@ -2560,7 +2560,7 @@
     </tdml:errors>
   </tdml:parserTestCase>
 
-  <tdml:defineSchema name="invalidLengthUnits_prefixed">
+  <tdml:defineSchema name="lengthUnitsBitsForNonNegativeInteger_prefixed">
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:defineFormat name="Binary">
       <dfdl:format ref="ex:GeneralFormat" representation="binary"/>
@@ -2579,9 +2579,9 @@
                 dfdl:lengthUnits="bits"/>
   </tdml:defineSchema>
 
-  <tdml:parserTestCase name="invalidLengthUnits_prefixed" root="r1" model="invalidLengthUnits_prefixed"
-                       description="">
-    <tdml:document>
+  <tdml:parserTestCase name="lengthUnitsBitsForNonNegativeInteger_prefixed" root="r1" model="lengthUnitsBitsForNonNegativeInteger_prefixed"
+                       ignoreUnexpectedWarnings="false">
+  <tdml:document>
       <tdml:documentPart type="byte">08 ff</tdml:documentPart>
     </tdml:document>
     <tdml:infoset>
@@ -2589,10 +2589,6 @@
         <r1>255</r1>
       </tdml:dfdlInfoset>
     </tdml:infoset>
-    <tdml:warnings>
-      <tdml:warning>lengthUnits='bits'</tdml:warning>
-      <tdml:warning>will only be supported</tdml:warning>
-    </tdml:warnings>
   </tdml:parserTestCase>
 
   <!--
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 e9b8ce259d..f2997d9500 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
@@ -334,6 +334,7 @@ class TestSimpleTypes {
   @Test def test_dateBinIBM4690Packed(): Unit = { runner.runOneTest("dateBinIBM4690Packed") }
   @Test def test_dateBinIBM4690Packed2(): Unit = { runner.runOneTest("dateBinIBM4690Packed2") }
   @Test def test_dateBinIBM4690Packed3(): Unit = { runner.runOneTest("dateBinIBM4690Packed3") }
+  @Test def test_dateBinIBM4690Packed4(): Unit = { runner.runOneTest("dateBinIBM4690Packed4") }
   @Test def test_dateTimeBinIBM4690Packed(): Unit = {
     runner.runOneTest("dateTimeBinIBM4690Packed")
   }
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 e85cabf018..c51a40abf7 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,14 +98,14 @@ class TestLengthKindExplicit {
     runner.runOneTest("denseBit_lengthKind_explicit")
   }
 
-  @Test def test_invalidLengthUnitsIntegerWarning_explicit(): Unit = {
-    runner.runOneTest("invalidLengthUnitsIntegerWarning_explicit")
+  @Test def test_lengthUnitsBitsForInteger_explicit(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForInteger_explicit")
   }
-  @Test def test_invalidLengthUnitsIntegerError_explicit(): Unit = {
-    runner.runOneTest("invalidLengthUnitsIntegerError_explicit")
+  @Test def test_lengthUnitsBitsForInteger_explicit2(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForInteger_explicit2")
   }
-  @Test def test_invalidLengthUnitsDecimalWarning_explicit(): Unit = {
-    runner.runOneTest("invalidLengthUnitsDecimalWarning_explicit")
+  @Test def lengthUnitsBitsForDecimal_explicit(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForDecimal_explicit")
   }
 
   @Test def test_invalidUnsignedLongBitLength(): Unit = {
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala
index 99e0cf1c98..b2a7124fd2 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section12/lengthKind/TestLengthKindPrefixed.scala
@@ -280,8 +280,8 @@ class TestLengthKindPrefixed {
   @Test def test_pl_complexContentLengthCharacters_utf8_1() = {
     runner.runOneTest("pl_complexContentLengthCharacters_utf8_1")
   }
-  @Test def test_invalidLengthUnits_prefixed(): Unit = {
-    runner.runOneTest("invalidLengthUnits_prefixed")
+  @Test def lengthUnitsBitsForNonNegativeInteger_prefixed(): Unit = {
+    runner.runOneTest("lengthUnitsBitsForNonNegativeInteger_prefixed")
   }
 
   @Test def test_invalidUnsignedLongBitLength(): Unit = {