fix(go): dates are mistreated as strings (#2730)

Implements the correct code generation flow & runtime logic to correctly
handle jsii date values as *time.Time instances.

Fixes #2659



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

gh-pages/content/specification/6-compliance-report.md

@@ -5,7 +5,7 @@
 This section details the current state of each language binding with respect to our standard compliance suite.
 
 
-| number | test | java (100.00%) | golang (70.09%) | Dotnet | Python |
+| number | test | java (100.00%) | golang (73.50%) | Dotnet | Python |
 | ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------- | -------------------------------------------- | ------ | ------ |
 | 1 | asyncOverrides_overrideCallsSuper | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) | ⭕ | ⭕ |
 | 2 | [arrayReturnedByMethodCanBeRead]("Array created in the kernel can be queried for its elements") | 🟢 | 🟢 | ⭕ | ⭕ |
@@ -28,7 +28,7 @@ This section details the current state of each language binding with respect to
 | 19 | [doNotOverridePrivates_method_private]("Non public methods on the guest class do not override methods in the host class") | 🟢 | 🟢 | ⭕ | ⭕ |
 | 20 | [pureInterfacesCanBeUsedTransparently]("Guest implementation of a pure host interface can be used by host consumers accepting that interface") | 🟢 | 🟢 | ⭕ | ⭕ |
 | 21 | [nullShouldBeTreatedAsUndefined]("Null value of target language is treated as undefined by the host") | 🟢 | 🟢 | ⭕ | ⭕ |
-| 22 | [primitiveTypes]("All Primitive types are set and read with their respective types") | 🟢 | [🔴](https://github.com/aws/jsii/issues/2659) | ⭕ | ⭕ |
+| 22 | [primitiveTypes]("All Primitive types are set and read with their respective types") | 🟢 | 🟢  | ⭕ | ⭕ |
 | 23 | [reservedKeywordsAreSlugifiedInClassProperties]("TS code that uses reserved words as class property names get slugified so it is usable in the target language") | 🟢 | ⚪ | ⭕ | ⭕ |
 | 24 | [objectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut]("Ensure the JSII kernel can pass 'this' out to JSII remotes from within the constructor") | 🟢 | 🟢 | ⭕ | ⭕ |
 | 25 | [interfaceBuilder]("Seems to be a duplicate of 'propertyOverrides_interfaces'?") | 🟢 | 🟢 | ⭕ | ⭕ |
@@ -52,7 +52,7 @@ This section details the current state of each language binding with respect to
 | 43 | doNotOverridePrivates_property_getter_public | 🟢 | 🟢 | ⭕ | ⭕ |
 | 44 | equalsIsResistantToPropertyShadowingResultVariable | 🟢 | 🟢 | ⭕ | ⭕ |
 | 45 | listInClassCanBeReadCorrectly | 🟢 | 🟢 | ⭕ | ⭕ |
-| 46 | useNestedStruct | 🟢 | [🔴](https://github.com/aws/jsii/pull/2650) | ⭕ | ⭕ |
+| 46 | useNestedStruct | 🟢 | 🟢  | ⭕ | ⭕ |
 | 47 | testFluentApiWithDerivedClasses | 🟢 | 🟢 | ⭕ | ⭕ |
 | 48 | interfacesCanBeUsedTransparently_WhenAddedToJsiiType | 🟢 | 🟢 | ⭕ | ⭕ |
 | 49 | canOverrideProtectedGetter | 🟢 | [🔴](https://github.com/aws/jsii/issues/2673) | ⭕ | ⭕ |
@@ -69,7 +69,7 @@ This section details the current state of each language binding with respect to
 | 60 | canOverrideProtectedSetter | 🟢 | [🔴](https://github.com/aws/jsii/issues/2673) | ⭕ | ⭕ |
 | 61 | asyncOverrides_callAsyncMethod | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) | ⭕ | ⭕ |
 | 62 | nodeStandardLibrary | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) | ⭕ | ⭕ |
-| 63 | dates | 🟢 | [🔴](https://github.com/aws/jsii/issues/2659) | ⭕ | ⭕ |
+| 63 | dates | 🟢 | 🟢  | ⭕ | ⭕ |
 | 64 | collectionOfInterfaces_ListOfStructs | 🟢 | 🟢 | ⭕ | ⭕ |
 | 65 | objRefsAreLabelledUsingWithTheMostCorrectType | 🟢 | 🔴 | ⭕ | ⭕ |
 | 66 | unionPropertiesWithBuilder | 🟢 | 🟢 | ⭕ | ⭕ |
@@ -86,7 +86,7 @@ This section details the current state of each language binding with respect to
 | 77 | asyncOverrides_overrideThrows | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) | ⭕ | ⭕ |
 | 78 | callMethods | 🟢 | 🟢 | ⭕ | ⭕ |
 | 79 | returnAbstract | 🟢 | 🟢 | ⭕ | ⭕ |
-| 80 | dynamicTypes | 🟢 | [🔴](https://github.com/aws/jsii/issues/2659) | ⭕ | ⭕ |
+| 80 | dynamicTypes | 🟢 | 🟢  | ⭕ | ⭕ |
 | 81 | hashCodeIsResistantToPropertyShadowingResultVariable | 🟢 | ⚪ | ⭕ | ⭕ |
 | 82 | returnSubclassThatImplementsInterface976 | 🟢 | 🟢 | ⭕ | ⭕ |
 | 83 | structs_optionalEquals | 🟢 | ⚪ | ⭕ | ⭕ |

packages/@jsii/go-runtime-test/project/compliance_test.go

@@ -62,21 +62,16 @@ func (suite *ComplianceSuite) TestPrimitiveTypes() {
  types.SetNumberProperty(jsii.Number(1234))
  assert.Equal(float64(1234), *types.NumberProperty())
 
- // // json
+ // json
  mapProp := map[string]interface{}{"Foo": map[string]interface{}{"Bar": 123}}
  types.SetJsonProperty(&mapProp)
  assert.Equal(float64(123), (*types.JsonProperty())["Foo"].(map[string]interface{})["Bar"])
 
- suite.FailTest("Dates are currently treated as strings and fail going through the wire", "https://github.com/aws/jsii/issues/2659")
-
- // whoops - should accept time.Time, not string.
- // date
- types.SetDateProperty(jsii.String("12345"))
- assert.Equal("12345", types.DateProperty())
+ types.SetDateProperty(jsii.Time(time.Unix(0, 123000000)))
+ assert.WithinDuration(time.Unix(0, 123000000), *types.DateProperty(), 0)
 }
 
 func (suite *ComplianceSuite) TestUseNestedStruct() {
- suite.FailTest("Nested types are not namespaced", "https://github.com/aws/jsii/pull/2650")
  jcb.StaticConsumer_Consume(customsubmodulename.NestingClass_NestedStruct{
  Name: jsii.String("Bond, James Bond"),
  })
@@ -128,15 +123,14 @@ func (suite *ComplianceSuite) TestMaps() {
 
 func (suite *ComplianceSuite) TestDates() {
  assert := suite.Assert()
- suite.FailTest("Dates are represented as strings instead of date objects", "https://github.com/aws/jsii/issues/2659")
 
  types := calc.NewAllTypes()
- //types.SetDateProperty(time.Unix(128, 0))
- assert.Equal(time.Unix(128, 0), types.DateProperty())
+ types.SetDateProperty(jsii.Time(time.Unix(128, 0)))
+ assert.WithinDuration(time.Unix(128, 0), *types.DateProperty(), 0)
 
  // weak type
  types.SetAnyProperty(time.Unix(999, 0))
- assert.Equal(time.Unix(999, 0), types.AnyProperty())
+ assert.WithinDuration(time.Unix(999, 0), types.AnyProperty().(time.Time), 0)
 }
 
 func (suite *ComplianceSuite) TestCallMethods() {
@@ -226,9 +220,7 @@ func (suite *ComplianceSuite) TestDynamicTypes() {
 
  // date
  types.SetAnyProperty(time.Unix(1234, 0))
- if types.AnyProperty() != time.Unix(1234, 0) {
- suite.FailTest("Dates not supported", "https://github.com/aws/jsii/issues/2659")
- }
+ assert.WithinDuration(time.Unix(1234, 0), types.AnyProperty().(time.Time), 0)
 }
 
 func (suite *ComplianceSuite) TestArrayReturnedByMethodCanBeRead() {
@@ -1175,7 +1167,6 @@ func (suite *ComplianceSuite) TestUndefinedAndNull() {
 
 func (suite *ComplianceSuite) TestStructs_SerializeToJsii() {
  t := suite.T()
- t.Skip("DateTime fields are not implemented yet")
 
  firstStruct := calclib.MyFirstStruct{
  Astring: jsii.String("FirstString"),
@@ -1186,12 +1177,12 @@ func (suite *ComplianceSuite) TestStructs_SerializeToJsii() {
  doubleTrouble := calc.NewDoubleTrouble()
 
  derivedStruct := calc.DerivedStruct{
- NonPrimitive: doubleTrouble,
- Bool: jsii.Bool(false),
- // TODO: AnotherRequired: time.Now(),
- Astring: jsii.String("String"),
- Anumber: jsii.Number(1234),
- FirstOptional: &[]*string{jsii.String("one"), jsii.String("two")},
+ NonPrimitive:  doubleTrouble,
+ Bool:  jsii.Bool(false),
+ AnotherRequired: jsii.Time(time.Now()),
+ Astring:  jsii.String("String"),
+ Anumber:  jsii.Number(1234),
+ FirstOptional:  &[]*string{jsii.String("one"), jsii.String("two")},
  }
 
  gms := calc.NewGiveMeStructs()
@@ -1334,8 +1325,8 @@ func NewPartiallyInitializedThisConsumerImpl(assert *assert.Assertions) *partial
  return &p
 }
 
-func (p *partiallyInitializedThisConsumerImpl) ConsumePartiallyInitializedThis(obj calc.ConstructorPassesThisOut, dt *string, ev calc.AllTypesEnum) *string {
- epoch := time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC).Format("2006-01-02T15:04:05.000Z07:00")
+func (p *partiallyInitializedThisConsumerImpl) ConsumePartiallyInitializedThis(obj calc.ConstructorPassesThisOut, dt *time.Time, ev calc.AllTypesEnum) *string {
+ epoch := time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC)
 
  p.assert.NotNil(obj)
  p.assert.Equal(epoch, *dt)

packages/@jsii/go-runtime/jsii-runtime-go/internal/api/api.go

@@ -83,6 +83,10 @@ type EnumRef struct {
  MemberFQN string `json:"$jsii.enum"`
 }
 
+type WireDate struct {
+ Timestamp string `json:"$jsii.date"`
+}
+
 type WireMap struct {
  MapData map[string]interface{} `json:"$jsii.map"`
 }

packages/@jsii/go-runtime/jsii-runtime-go/internal/kernel/conversions.go

@@ -2,6 +2,7 @@ package kernel
 
 import (
  "reflect"
+ "time"
 
  "github.com/aws/jsii-runtime-go/internal/api"
 )
@@ -126,6 +127,11 @@ func (c *Client) CastPtrToRef(dataVal reflect.Value) interface{} {
  return nil
  }
 
+ // In case we got a time.Time value (or pointer to one).
+ if wireDate, isDate := castPtrToDate(dataVal); isDate {
+ return wireDate
+ }
+
  switch dataVal.Kind() {
  case reflect.Map:
  result := api.WireMap{MapData: make(map[string]interface{})}
@@ -192,6 +198,24 @@ func (c *Client) CastPtrToRef(dataVal reflect.Value) interface{} {
  return dataVal.Interface()
 }
 
+// castPtrToDate obtains an api.WireDate from the provided reflect.Value if it
+// represents a time.Time or *time.Time value. It accepts both a pointer and
+// direct value as a convenience (when passing time.Time through an interface{}
+// parameter, having to unwrap it as a pointer is annoying and unneeded).
+func castPtrToDate(data reflect.Value) (wireDate api.WireDate, ok bool) {
+ var timestamp *time.Time
+ if timestamp, ok = data.Interface().(*time.Time); !ok {
+ var val time.Time
+ if val, ok = data.Interface().(time.Time); ok {
+ timestamp = &val
+ }
+ }
+ if ok {
+ wireDate.Timestamp = timestamp.Format(time.RFC3339Nano)
+ }
+ return
+}
+
 func castValToRef(data reflect.Value) (ref api.ObjectRef, ok bool) {
  if data.Kind() == reflect.Map {
  for _, k := range data.MapKeys() {
@@ -234,13 +258,14 @@ func castValToRef(data reflect.Value) (ref api.ObjectRef, ok bool) {
 }
 
 // TODO: This should return a time.Time instead
-func castValToDate(data reflect.Value) (date string, ok bool) {
+func castValToDate(data reflect.Value) (date time.Time, ok bool) {
  if data.Kind() == reflect.Map {
  for _, k := range data.MapKeys() {
  v := reflect.ValueOf(data.MapIndex(k).Interface())
  if k.Kind() == reflect.String && k.String() == "$jsii.date" && v.Kind() == reflect.String {
- date = v.String()
- ok = true
+ var err error
+ date, err = time.Parse(time.RFC3339Nano, v.String())
+ ok = (err == nil)
  break
  }
  }

packages/@jsii/go-runtime/jsii-runtime-go/runtime.go

@@ -4,6 +4,7 @@ import (
  "fmt"
  "reflect"
  "strings"
+ "time"
 
  "github.com/aws/jsii-runtime-go/internal/api"
  "github.com/aws/jsii-runtime-go/internal/kernel"
@@ -371,7 +372,14 @@ func Close() {
  kernel.CloseClient()
 }
 
-// Helpers to store primitives and return pointers to them
-func Bool(v bool) *bool { return &v }
-func Number(v float64) *float64 { return &v }
-func String(v string) *string { return &v }
+// Bool obtains a pointer to the provided bool.
+func Bool(v bool) *bool { return &v }
+
+// Number obtains a pointer to the provided float64.
+func Number(v float64) *float64 { return &v }
+
+// String obtains a pointer to the provided string.
+func String(v string) *string { return &v }
+
+// Time obtains a pointer to the provided time.Time.
+func Time(v time.Time) *time.Time { return &v }

packages/jsii-pacmak/lib/targets/go/dependencies.ts

@@ -0,0 +1,17 @@
+/**
+ * Information about a module's dependency on "special" packages (either part of
+ * the go standard library, or generated as part of the current module).
+ */
+export interface SpecialDependencies {
+ /** Whether the jsii runtime library for go is needed */
+ readonly runtime: boolean;
+
+ /** Whether the package's initialization hook is needed */
+ readonly init: boolean;
+
+ /** Whether the internal type aliases package is needed */
+ readonly internal: boolean;
+
+ /** Whether go's standard library "time" module is needed */
+ readonly time: boolean;
+}

packages/jsii-pacmak/lib/targets/go/package.ts

@@ -4,6 +4,7 @@ import { basename, dirname, join } from 'path';
 import * as semver from 'semver';
 
 import { VERSION } from '../../version';
+import { SpecialDependencies } from './dependencies';
 import { EmitContext } from './emit-context';
 import { ReadmeFile } from './readme-file';
 import {
@@ -184,16 +185,18 @@ export abstract class Package {
  code.line();
  }
 
- protected get usesRuntimePackage(): boolean {
- return this.types.some((type) => type.usesRuntimePackage);
- }
-
- protected get usesInitPackage(): boolean {
- return this.types.some((type) => type.usesInitPackage);
- }
-
- protected get usesInternalPackage(): boolean {
- return this.types.some((type) => type.usesInternalPackage);
+ protected get specialDependencies(): SpecialDependencies {
+ return this.types
+ .map((t) => t.specialDependencies)
+ .reduce(
+ (acc, elt) => ({
+ runtime: acc.runtime || elt.runtime,
+ init: acc.init || elt.init,
+ internal: acc.internal || elt.internal,
+ time: acc.time || elt.time,
+ }),
+ { runtime: false, init: false, internal: false, time: false },
+ );
  }
 
  /**
@@ -229,18 +232,24 @@ export abstract class Package {
  private emitImports(code: CodeMaker) {
  const toImport = new Array<ImportedModule>();
 
- if (this.usesRuntimePackage) {
+ const specialDeps = this.specialDependencies;
+
+ if (specialDeps.time) {
+ toImport.push({ module: 'time' });
+ }
+
+ if (specialDeps.runtime) {
  toImport.push(JSII_RT_MODULE);
  }
 
- if (this.usesInitPackage) {
+ if (specialDeps.init) {
  toImport.push({
  alias: JSII_INIT_ALIAS,
  module: `${this.root.goModuleName}/${JSII_INIT_PACKAGE}`,
  });
  }
 
- if (this.usesInternalPackage) {
+ if (specialDeps.internal) {
  toImport.push({
  module: `${this.goModuleName}/${INTERNAL_PACKAGE_NAME}`,
  });
@@ -611,7 +620,9 @@ function importGoModules(code: CodeMaker, modules: readonly ImportedModule[]) {
  }
 
  function isBuiltIn(mod: ImportedModule): boolean {
- return !mod.module.includes('/');
+ // Standard library modules don't have any "." in their path, whereas any
+ // other module has a DNS portion in them, which must include a ".".
+ return !mod.module.includes('.');
  }
 
  function isSpecial(mod: ImportedModule): boolean {