Dart: do not export nested types defined in internal types

In the case of Dart the nested types defined in types annotated as internal were exported. This was invalid behaviour -- such types shall not have been exported. The root cause for that was usage of 'CommonGeneratorPredicates' to check if type is internal. In the case of Dart there is no type nesting and all types are outside. The common version of predicate did not check if any of parent types are internal. This change: - moves Dart specific logic responsible for checking if LimeType is internal from DartVisibilityResolver.kt to DartGeneratorPredicates.kt - injects the predicate to DartVisibilityResolver via constructor in order to preserve the existing behaviour of checking if type is internal - fixes the way how DartGenerator.generateDart() selects types to export by using Dart specific predicate instead of the generic one Note: this change adjusts smoke tests to show, that the internal types like structs, classes and lambdas are no longer exported when their parent type is internal. Signed-off-by: Patryk Wrobel <[email protected]>
heremaps · Oct 24, 2024 · 965271b · 965271b
1 parent 5555379
commit 965271b
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 ### Bug fixes:
  * Added missing generation of conversion functions for lambdas defined in structs for Swift.
+ * Fixed a bug related to exporting nested types defined in a type annotated as internal.
 
 ## 13.9.5
 Release date: 2024-10-22

diff --git a/gluecodium/src/main/java/com/here/gluecodium/generator/dart/DartGenerator.kt b/gluecodium/src/main/java/com/here/gluecodium/generator/dart/DartGenerator.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2019 HERE Europe B.V.
+ * 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.
@@ -129,14 +129,17 @@ internal class DartGenerator : Generator {
             throw GluecodiumExecutionException("Validation errors found, see log for details.")
         }
 
+        val generatorPredicates = DartGeneratorPredicates(dartFilteredModel.referenceMap, activeTags, dartNameResolver)
+        val predicatesMap = generatorPredicates.predicates
+
         val dartResolvers =
             mapOf(
                 "" to dartNameResolver,
                 "Ffi" to CamelCaseNameResolver(ffiNameResolver),
                 "FfiSnakeCase" to ffiNameResolver,
                 "FfiApiTypes" to FfiApiTypeNameResolver(),
                 "FfiDartTypes" to FfiDartTypeNameResolver(),
-                "visibility" to DartVisibilityResolver(dartFilteredModel.referenceMap),
+                "visibility" to DartVisibilityResolver(predicatesMap["isInternal"]!!),
             )
         val ffiCppNameResolver = FfiCppNameResolver(ffiReferenceMap, cppNameRules, rootNamespace, internalNamespace)
         val ffiResolvers =
@@ -160,8 +163,6 @@ internal class DartGenerator : Generator {
         val importsCollector = DartImportsCollector(importResolver)
         val declarationImportsCollector = GenericImportsCollector(declarationImportResolver, collectOwnImports = true)
 
-        val generatorPredicates = DartGeneratorPredicates(dartFilteredModel.referenceMap, activeTags, dartNameResolver)
-        val predicatesMap = generatorPredicates.predicates
         val includeResolver = FfiCppIncludeResolver(ffiReferenceMap, cppNameRules, internalNamespace)
         val includeCollector =
             GenericIncludesCollector(
@@ -234,9 +235,11 @@ internal class DartGenerator : Generator {
         val filePath = "$packagePath/$fileName"
         val relativePath = "$SRC_DIR_SUFFIX/$filePath.dart"
 
+        val isInternal = predicates["isInternal"]!!
+
         val allTypes = LimeTypeHelper.getAllTypes(rootElement).filterNot { it is LimeTypeAlias }
         val nonExternalTypes = allTypes.filter { it.external?.dart == null }
-        val allSymbols = nonExternalTypes.filterNot { CommonGeneratorPredicates.isInternal(it, DART) }
+        val allSymbols = nonExternalTypes.filterNot { isInternal(it) }
         if (allSymbols.isNotEmpty()) {
             val allNames = allSymbols.map { dartNameResolver.resolveName(it) }
             val testNames =

diff --git a/gluecodium/src/main/java/com/here/gluecodium/generator/dart/DartGeneratorPredicates.kt b/gluecodium/src/main/java/com/here/gluecodium/generator/dart/DartGeneratorPredicates.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2021 HERE Europe B.V.
+ * 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.
@@ -19,6 +19,7 @@
 
 package com.here.gluecodium.generator.dart
 
+import com.here.gluecodium.cli.GluecodiumExecutionException
 import com.here.gluecodium.common.LimeModelSkipPredicates
 import com.here.gluecodium.generator.common.CommonGeneratorPredicates
 import com.here.gluecodium.model.lime.LimeAttributeType.DART
@@ -44,6 +45,18 @@ internal class DartGeneratorPredicates(
             "allFieldsCtorIsPublic" to { limeStruct: Any ->
                 limeStruct is LimeStruct && allFieldsCtorIsPublic(limeStruct)
             },
+            "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.
+                    is LimeType -> {
+                        generateSequence(element) {
+                            limeReferenceMap[it.path.parent.toString()] as? LimeType
+                        }.any { CommonGeneratorPredicates.isInternal(it, DART) }
+                    }
+                    is LimeNamedElement -> CommonGeneratorPredicates.isInternal(element, DART)
+                    else -> throw GluecodiumExecutionException("Unsupported element type ${element.javaClass.name}")
+                }
+            },
             "hasAnyComment" to { CommonGeneratorPredicates.hasAnyComment(it, "Dart") },
             "hasImmutableFields" to { CommonGeneratorPredicates.hasImmutableFields(it) },
             "hasSingleConstructor" to { limeContainer: Any ->

diff --git a/gluecodium/src/main/java/com/here/gluecodium/generator/dart/DartVisibilityResolver.kt b/gluecodium/src/main/java/com/here/gluecodium/generator/dart/DartVisibilityResolver.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2022 HERE Europe B.V.
+ * 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.
@@ -19,30 +19,8 @@
 
 package com.here.gluecodium.generator.dart
 
-import com.here.gluecodium.cli.GluecodiumExecutionException
-import com.here.gluecodium.generator.common.CommonGeneratorPredicates
 import com.here.gluecodium.generator.common.NameResolver
-import com.here.gluecodium.generator.common.ReferenceMapBasedResolver
-import com.here.gluecodium.model.lime.LimeAttributeType.DART
-import com.here.gluecodium.model.lime.LimeElement
-import com.here.gluecodium.model.lime.LimeNamedElement
-import com.here.gluecodium.model.lime.LimeType
 
-internal class DartVisibilityResolver(limeReferenceMap: Map<String, LimeElement>) :
-    ReferenceMapBasedResolver(limeReferenceMap), NameResolver {
-    override fun resolveName(element: Any): String =
-        when (element) {
-            // Dart has no type nesting, so all types are "outside" and have to check for an internal outer type.
-            is LimeType -> {
-                val isInternal =
-                    generateSequence(element) {
-                        limeReferenceMap[it.path.parent.toString()] as? LimeType
-                    }.any { CommonGeneratorPredicates.isInternal(it, DART) }
-                getVisibilityPrefix(isInternal)
-            }
-            is LimeNamedElement -> getVisibilityPrefix(CommonGeneratorPredicates.isInternal(element, DART))
-            else -> throw GluecodiumExecutionException("Unsupported element type ${element.javaClass.name}")
-        }
-
-    private fun getVisibilityPrefix(isInternal: Boolean) = if (isInternal) "_" else ""
+internal class DartVisibilityResolver(val dartIsInternalPredicate: (Any) -> Boolean) : NameResolver {
+    override fun resolveName(element: Any) = if (dartIsInternalPredicate(element)) "_" else ""
 }
diff --git a/gluecodium/src/test/resources/smoke/visibility_attribute/output/dart/lib/smoke.dart b/gluecodium/src/test/resources/smoke/visibility_attribute/output/dart/lib/smoke.dart
@@ -1,8 +1,6 @@
 
 
 export 'src/smoke/internal_property_only.dart' show InternalPropertyOnly;
-export 'src/smoke/outer_class_with_internal_attribute.dart' show OuterClassWithInternalAttribute_ClassNestedInInternalClass, OuterClassWithInternalAttribute_LambdaNestedInInternalClass, OuterClassWithInternalAttribute_StructNestedInInternalClass;
-export 'src/smoke/outer_struct_with_internal_attribute.dart' show OuterStructWithInternalAttribute_ClassNestedInInternalStruct, OuterStructWithInternalAttribute_LambdaNestedInInternalStruct, OuterStructWithInternalAttribute_StructNestedInInternalStruct;
 export 'src/smoke/public_class.dart' show PublicClass, PublicClass_PublicStruct, PublicClass_PublicStructWithInternalDefaults;
 export 'src/smoke/public_interface.dart' show PublicInterface;
 export 'src/smoke/public_struct_with_non_default_internal_field.dart' show PublicStructWithNonDefaultInternalField;