Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dart: enable custom fields with defaults that do not provide const constructor to work with 'PositionalDefaults' #1632

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Gluecodium project Release Notes

## Unreleased
### Bug fixes:
* Dart: fixed a bug related to compilation error caused by usage of 'PositionalDefaults' and default value for a field that uses type, which does not provide const constructor.

## 13.10.1
Release date 2024-12-12
### Bug fixes:
Expand Down
6 changes: 6 additions & 0 deletions docs/lime_attributes.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ name was defined through command-line parameters. If the tag is not present, the
positional parameters in Dart. Can only be applied to a struct that has at least one field with a default value. The
positional defaults constructor will be generated with a `@Deprecated` annotation, if _DeprecationMessage_ is
specified.
> **Important:** if the constructor used for the default value is non-const, then the generated constructor will use
> optional value to prevent compilation error. However, this implies certain limitation for default values of nullable
> types, which do not provide const constructor (i.a. Blob type or custom structures that are not annotated as `@Immutable`).
>
> If the field is nullable and its type does not provide const constructor then the only default value that is accepted
> can be null. If any other value is used then the generator will raise validation error.
* **Attribute** **=** **"**_Annotation_**"**: marks an element to be marked with the given annotation in Dart
generated code. _Annotation_ does not need to be prepended with `@`. _Annotation_ can contain parameters, e.g.
`@Dart(Attribute="Deprecated(\"It's deprecated.\")")`. If some of the parameters are string literals, their enclosing
Expand Down
43 changes: 43 additions & 0 deletions functional-tests/functional/dart/test/Defaults_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
//
// -------------------------------------------------------------------------------------------------

import 'dart:typed_data';
import "package:test/test.dart";
import "package:functional/test.dart";
import "../test_suite.dart";
Expand Down Expand Up @@ -103,6 +104,48 @@ void main() {
expect(result.intField, 13);
expect(result.stringField, "foobar");
});
_testSuite.test("Check positional defaults for non-const constructible types", () {
// Case 1: all defaults.
final first = PosDefaultStructWithCustomStructsFields();
expect(first.nonConstCtorField0.intField, 42);
expect(first.nonConstCtorField1.someField1.intField, 42);
expect(first.nonConstCtorField2.stringField, "Some string");
expect(first.nonConstCtorField3.nullableListField, null);
expect(first.nonConstCtorField5, Uint8List.fromList([]));
expect(first.nonConstCtorField6, Uint8List.fromList([222, 173, 190, 239]));
expect(first.nonConstCtorField7, null);

// Case 2: custom values.
final second = PosDefaultStructWithCustomStructsFields(
// Fields with const constructors.
AnotherImmutableStructWithDefaults.withDefaults(),
null,
[],
null,
0,
0.0,
null,
null,
// Fields without const constructor.
StructWithAllDefaults(21, "ABC"),
PosDefaultStructWithFieldUsingImmutableStruct(),
SomeMutableCustomStructWithDefaults(21, "Another string", [7, 7, 7]),
StructWithNullableCollectionDefaults(),
StructWithAllDefaults(44, "DEF"),
Uint8List.fromList([1, 2, 3]),
Uint8List.fromList([4, 5, 6]),
Uint8List.fromList([7, 8, 9])
);

expect(second.nonConstCtorField0.intField, 21);
expect(second.nonConstCtorField1.someField1.intField, 42);
expect(second.nonConstCtorField2.stringField, "Another string");
expect(second.nonConstCtorField3.nullableListField, null);
expect(second.nonConstCtorField4?.stringField, "DEF");
expect(second.nonConstCtorField5, Uint8List.fromList([1, 2, 3]));
expect(second.nonConstCtorField6, Uint8List.fromList([4, 5, 6]));
expect(second.nonConstCtorField7, Uint8List.fromList([7, 8, 9]));
});
_testSuite.test("Check positional enumerator defaults", () {
final result = StructWithEnums();

Expand Down
30 changes: 30 additions & 0 deletions functional-tests/functional/input/lime/PositionalDefaults.lime
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,36 @@ struct PosDefaultStructWithFieldUsingImmutableStruct {
someField1: AnotherImmutableStructWithDefaults = {}
}

@Dart(PositionalDefaults)
@Java(Skip) @Swift(Skip)
struct SomeMutableCustomStructWithDefaults {
intField: Int = 77
stringField: String = "Some string"
listField: List<Int> = [1, 2, 3]
}

@Dart(PositionalDefaults)
@Java(Skip) @Swift(Skip)
struct PosDefaultStructWithCustomStructsFields {
constCtorField0: AnotherImmutableStructWithDefaults = {}
constCtorField1: AnotherImmutableStructWithDefaults? = {}
constCtorField2: List<String> = ["abc", "def", "ghi"]
constCtorField3: Map<String, String>? = null
constCtorField4: Int = 77
constCtorField5: Double = 77.77
constCtorField6: AnotherImmutableStructWithDefaults? = {}
constCtorField7: AnotherImmutableStructWithDefaults? = null

nonConstCtorField0: StructWithAllDefaults = {}
nonConstCtorField1: PosDefaultStructWithFieldUsingImmutableStruct = {}
nonConstCtorField2: SomeMutableCustomStructWithDefaults = {}
nonConstCtorField3: StructWithNullableCollectionDefaults = {}
nonConstCtorField4: StructWithAllDefaults? = null
nonConstCtorField5: Blob = []
nonConstCtorField6: Blob = [222, 173, 190, 239]
nonConstCtorField7: Blob? = null
}

@Dart(PositionalDefaults)
@Java(Skip) @Swift(Skip)
struct PosDefaultsWithDuration {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (C) 2016-2024 HERE Europe B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/

package com.here.gluecodium.generator.dart

import com.here.gluecodium.common.LimeLogger
import com.here.gluecodium.model.lime.LimeAttributeType.DART
import com.here.gluecodium.model.lime.LimeAttributeType.IMMUTABLE
import com.here.gluecodium.model.lime.LimeAttributeValueType
import com.here.gluecodium.model.lime.LimeBasicType
import com.here.gluecodium.model.lime.LimeElement
import com.here.gluecodium.model.lime.LimeStruct
import com.here.gluecodium.model.lime.LimeValue

class DartDefaultValuesValidator(private val logger: LimeLogger) {
fun validate(referenceMap: Map<String, LimeElement>): Boolean {
val allStructs = referenceMap.values.filterIsInstance<LimeStruct>()
return !allStructs.map { validatePositionalDefaults(it) }.contains(false)
}

private fun validatePositionalDefaults(limeStruct: LimeStruct): Boolean {
if (!limeStruct.attributes.have(DART, LimeAttributeValueType.POSITIONAL_DEFAULTS)) {
return true
}

var result = true
for (field in limeStruct.initializedFields) {
// Types other than BLOB and custom LimeStructs provide const constructors.
val basicType = field.typeRef.type as? LimeBasicType
if (field.typeRef.type !is LimeStruct && (basicType?.typeId != LimeBasicType.TypeId.BLOB)) {
continue
}

// Custom immutable types provide const constructors.
if (field.typeRef.type.attributes.have(IMMUTABLE)) {
continue
}

// If the type does not provide const constructor, then generated 'PositionalDefaults' constructor
// will utilize optional field to avoid compilation errors.
// However, if the user uses 'nullable type' and wants to set value that is different from 'null' as the
// default one, then if somebody passed 'null' to the generated constructor, then it wouldn't be honored.
//
// Do not allow users to do that.
if (field.typeRef.isNullable && field.defaultValue!! !is LimeValue.Null) {
logger.error(field, "$NULLABLE_ERROR_REASON; please check $DOCS_LINK")
result = false
}
}

return result
}

companion object {
const val NULLABLE_ERROR_REASON =
"For '@PositionalDefaults' structs, nullable fields that do not provide" +
" const constructors must use 'null' default value"
const val DOCS_LINK = "https://github.com/heremaps/gluecodium/blob/master/docs/lime_attributes.md#dart-specific-attributes"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,12 @@ internal class DartGenerator : Generator {
val dartNameResolver = DartNameResolver(ffiReferenceMap, nameRules, limeLogger, commentsProcessor)
val ffiNameResolver = FfiNameResolver(ffiReferenceMap, nameRules, internalPrefix)

val validationResult =
val overloadsValidationResult =
DartOverloadsValidator(dartNameResolver, limeLogger, overloadsWerror)
.validate(dartFilteredModel.referenceMap.values)
if (!validationResult) {
val defaultValuesValidationResult =
DartDefaultValuesValidator(limeLogger).validate(dartFilteredModel.referenceMap)
if (!overloadsValidationResult || !defaultValuesValidationResult) {
throw GluecodiumExecutionException("Validation errors found, see log for details.")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import com.here.gluecodium.generator.common.CommonGeneratorPredicates
import com.here.gluecodium.model.lime.LimeAttributeType.DART
import com.here.gluecodium.model.lime.LimeAttributeType.IMMUTABLE
import com.here.gluecodium.model.lime.LimeAttributeValueType.POSITIONAL_DEFAULTS
import com.here.gluecodium.model.lime.LimeBasicType
import com.here.gluecodium.model.lime.LimeContainer
import com.here.gluecodium.model.lime.LimeElement
import com.here.gluecodium.model.lime.LimeExternalDescriptor
import com.here.gluecodium.model.lime.LimeField
import com.here.gluecodium.model.lime.LimeNamedElement
import com.here.gluecodium.model.lime.LimeStruct
import com.here.gluecodium.model.lime.LimeType
Expand All @@ -45,6 +47,23 @@ internal class DartGeneratorPredicates(
"allFieldsCtorIsPublic" to { limeStruct: Any ->
limeStruct is LimeStruct && allFieldsCtorIsPublic(limeStruct)
},
"fieldHasConstCtor" to { limeField: Any ->
if (limeField is LimeField) {
when (limeField.typeRef.type) {
is LimeBasicType -> {
val basicType = limeField.typeRef.type as LimeBasicType
basicType.typeId != LimeBasicType.TypeId.BLOB
}
is LimeStruct -> limeField.typeRef.type.attributes.have(IMMUTABLE)
else -> true
}
} else {
false
}
},
"fieldHasDefaultValue" to { limeField: Any ->
limeField is LimeField && limeField.defaultValue != null
},
"isInternal" to { element: Any ->
when (element) {
// Dart has no type nesting, so all types are "outside" and have to check for an internal outer type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,27 @@
{{/instanceOf}}{{!!
}} {{#if attributes.immutable}}const {{/if}}{{resolveName}}{{#if external.dart.converter}}Internal{{/if}}({{!!
}}{{#uninitializedFields}}{{resolveName typeRef}} {{resolveName}}, {{/uninitializedFields}}{{!!
}}[{{#initializedFields}}{{resolveName typeRef}} {{resolveName}} = {{>constPrefix}}{{resolveName defaultValue}}{{#if iter.hasNext}}, {{/if}}{{/initializedFields}}])
: {{#fields}}{{resolveName "visibility"}}{{resolveName}} = {{resolveName}}{{#if iter.hasNext}}, {{/if}}{{/fields}};
}}[{{#initializedFields}}{{!!
}}{{#ifPredicate "fieldHasConstCtor"}}{{!!
}}{{resolveName typeRef}} {{resolveName}} = {{>constPrefix}}{{resolveName defaultValue}}{{#if iter.hasNext}}, {{/if}}{{!!
}}{{/ifPredicate}}{{!!
}}{{#unlessPredicate "fieldHasConstCtor"}}{{!!
}}{{resolveName typeRef.asNullable}} {{resolveName}} = null{{#if iter.hasNext}}, {{/if}}{{!!
}}{{/unlessPredicate}}{{!!
}}{{/initializedFields}}])
: {{#fields}}{{!!
}}{{#ifPredicate "fieldHasDefaultValue"}}{{!!
}}{{#ifPredicate "fieldHasConstCtor"}}{{!!
}}{{resolveName "visibility"}}{{resolveName}} = {{resolveName}}{{#if iter.hasNext}}, {{/if}}{{!!
}}{{/ifPredicate}}{{!!
}}{{#unlessPredicate "fieldHasConstCtor"}}{{!!
}}{{resolveName "visibility"}}{{resolveName}} = {{resolveName}} ?? {{resolveName defaultValue}}{{#if iter.hasNext}}, {{/if}}{{!!
}}{{/unlessPredicate}}{{!!
}}{{/ifPredicate}}{{!!
}}{{#unlessPredicate "fieldHasDefaultValue"}}{{!!
}}{{resolveName "visibility"}}{{resolveName}} = {{resolveName}}{{#if iter.hasNext}}, {{/if}}{{!!
}}{{/unlessPredicate}}{{!!
}}{{/fields}};
{{/if}}{{!!

}}{{#unless attributes.dart.positionalDefaults initializedFields}}{{!!
Expand Down
Loading
Loading