Skip to content

Commit 7303fe7

Browse files
authored
Add additional registered properties checks (#703)
* Disallow nullable registration again * Fix error message * Update error message * Add exported property type checks * Add property type checks * Fix node type and refcounted type checks * Remove useless new ide checks as they were already implemented * Do not register rid * Revert "Do not register rid" This reverts commit 06f01d8. * Make RID extend the core type interface again * Fix rebase
1 parent 8a2a9d1 commit 7303fe7

File tree

12 files changed

+101
-14
lines changed

12 files changed

+101
-14
lines changed

harness/tests/scripts/godot/tests/Invocation.gdj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ properties = [
4343
nav_meshes_dictionary,
4444
nullable_dictionary,
4545
color,
46-
rid,
4746
packed_byte_array,
4847
packed_int32_array,
4948
packed_float64_array,

kt/entry-generation/godot-entry-generator/src/main/kotlin/godot/entrygenerator/EntryGenerator.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import godot.entrygenerator.checks.ConstructorOverloadingCheck
55
import godot.entrygenerator.checks.DefaultConstructorCheck
66
import godot.entrygenerator.checks.FunctionArgCountCheck
77
import godot.entrygenerator.checks.LateinitPropertyCheck
8+
import godot.entrygenerator.checks.NullablePropertyCheck
89
import godot.entrygenerator.checks.PropertyMutablilityCheck
10+
import godot.entrygenerator.checks.PropertyTypeCheck
911
import godot.entrygenerator.checks.RpcCheck
1012
import godot.entrygenerator.checks.SignalTypeCheck
1113
import godot.entrygenerator.exceptions.ChecksFailedException
@@ -112,8 +114,10 @@ object EntryGenerator {
112114

113115
SignalTypeCheck(logger, sourceFiles).execute(),
114116

117+
PropertyTypeCheck(logger, sourceFiles).execute(),
115118
PropertyMutablilityCheck(logger, sourceFiles).execute(),
116119
LateinitPropertyCheck(logger, sourceFiles).execute(),
120+
NullablePropertyCheck(logger, sourceFiles).execute(),
117121

118122
RpcCheck(logger, sourceFiles).execute(),
119123
).any { hasIssue -> hasIssue }
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package godot.entrygenerator.checks
2+
3+
import godot.entrygenerator.ext.isCoreType
4+
import godot.entrygenerator.ext.isGodotPrimitive
5+
import godot.entrygenerator.model.RegisterPropertyAnnotation
6+
import godot.entrygenerator.model.SourceFile
7+
import godot.entrygenerator.utils.Logger
8+
9+
class NullablePropertyCheck(logger: Logger, sourceFiles: List<SourceFile>): BaseCheck(logger, sourceFiles) {
10+
override fun execute(): Boolean {
11+
var hasIssue = false
12+
sourceFiles
13+
.flatMap { it.registeredClasses }
14+
.flatMap { it.properties }
15+
.filter { registeredProperty -> registeredProperty.annotations.filterIsInstance<RegisterPropertyAnnotation>().isNotEmpty() }
16+
.forEach { exportedProperty ->
17+
if (exportedProperty.type.isNullable && (exportedProperty.type.isCoreType() || exportedProperty.type.isGodotPrimitive())) {
18+
hasIssue = true
19+
logger.error(exportedProperty, "Registered property which is a Kotlin/Java primitive or Godot core type cannot be nullable. Assign a default value")
20+
}
21+
}
22+
return hasIssue
23+
}
24+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package godot.entrygenerator.checks
2+
3+
import godot.entrygenerator.ext.isCoreType
4+
import godot.entrygenerator.ext.isEnum
5+
import godot.entrygenerator.ext.isGodotPrimitive
6+
import godot.entrygenerator.ext.isKotlinCollection
7+
import godot.entrygenerator.ext.isNodeType
8+
import godot.entrygenerator.ext.isRefCounted
9+
import godot.entrygenerator.model.RegisterPropertyAnnotation
10+
import godot.entrygenerator.model.SourceFile
11+
import godot.entrygenerator.utils.Logger
12+
13+
class PropertyTypeCheck(logger: Logger, sourceFiles: List<SourceFile>) : BaseCheck(logger, sourceFiles) {
14+
override fun execute(): Boolean {
15+
var hasIssue = false
16+
sourceFiles
17+
.flatMap { it.registeredClasses }
18+
.flatMap { it.properties }
19+
.filter { registeredProperty ->
20+
registeredProperty.annotations.filterIsInstance<RegisterPropertyAnnotation>().isNotEmpty()
21+
}
22+
.forEach { exportedProperty ->
23+
if (
24+
!exportedProperty.type.isGodotPrimitive()
25+
&& !exportedProperty.type.isCoreType()
26+
&& !exportedProperty.type.isNodeType()
27+
&& !exportedProperty.type.isRefCounted()
28+
&& !exportedProperty.type.isKotlinCollection()
29+
&& !exportedProperty.type.isEnum()
30+
) {
31+
hasIssue = true
32+
logger.error(
33+
exportedProperty,
34+
"Registered property can only be of type primitive, core type, node type or ref counted"
35+
)
36+
}
37+
}
38+
return hasIssue
39+
}
40+
}

kt/entry-generation/godot-entry-generator/src/main/kotlin/godot/entrygenerator/ext/TypeExtensions.kt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package godot.entrygenerator.ext
22

33
import com.squareup.kotlinpoet.ClassName
44
import com.squareup.kotlinpoet.TypeName
5+
import godot.core.CoreType
56
import godot.entrygenerator.model.Type
7+
import godot.entrygenerator.model.TypeKind
68
import godot.tools.common.constants.GodotKotlinJvmTypes
79
import godot.tools.common.constants.GodotTypes
810
import godot.tools.common.constants.VARIANT_CASTER_ANY
@@ -35,6 +37,7 @@ import godot.tools.common.constants.VARIANT_PARSER__RID
3537
import godot.tools.common.constants.godotApiPackage
3638
import godot.tools.common.constants.godotCorePackage
3739
import godot.tools.common.constants.godotUtilPackage
40+
import godot.tools.common.constants.kotlinCollectionsPackage
3841
import godot.tools.common.constants.variantParserPackage
3942
import godot.tools.common.extensions.convertToCamelCase
4043
import java.util.*
@@ -93,10 +96,7 @@ fun Type?.toGodotVariantType(): ClassName = this?.let {
9396
} ?: toKtVariantType()
9497

9598
fun Type.isCoreType(): Boolean {
96-
return listOf(
97-
*GodotTypes.coreTypes.map { "$godotCorePackage.$it" }.toTypedArray(),
98-
"$godotCorePackage.${GodotKotlinJvmTypes.variantArray}"
99-
).contains(fqName)
99+
return supertypes.any { it.fqName == CoreType::class.qualifiedName || it.allSuperTypes.any { superType -> superType.fqName == CoreType::class.qualifiedName } }
100100
}
101101

102102
fun Type.isNodeType(): Boolean {
@@ -121,7 +121,11 @@ fun Type.isCompatibleList(): Boolean = when (fqName) {
121121
else -> allSuperTypes.any { it.fqName == "$godotCorePackage.${GodotKotlinJvmTypes.variantArray}" }
122122
}
123123

124-
fun Type.isReference(): Boolean = fqName == "$godotApiPackage.${GodotKotlinJvmTypes.refCounted}" || this
124+
fun Type.isKotlinCollection(): Boolean = fqName.contains(kotlinCollectionsPackage)
125+
126+
fun Type.isEnum(): Boolean = kind == TypeKind.ENUM_CLASS
127+
128+
fun Type.isRefCounted(): Boolean = fqName == "$godotApiPackage.${GodotKotlinJvmTypes.refCounted}" || this
125129
.allSuperTypes
126130
.any { supertype -> supertype.fqName == "$godotApiPackage.${GodotKotlinJvmTypes.refCounted}" }
127131

kt/entry-generation/godot-entry-generator/src/main/kotlin/godot/entrygenerator/generator/hintstring/PropertyHintStringGeneratorProvider.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package godot.entrygenerator.generator.hintstring
33
import godot.entrygenerator.EntryGenerator
44
import godot.entrygenerator.ext.isCompatibleList
55
import godot.entrygenerator.ext.isNodeType
6-
import godot.entrygenerator.ext.isReference
6+
import godot.entrygenerator.ext.isRefCounted
77
import godot.entrygenerator.model.ColorNoAlphaHintAnnotation
88
import godot.entrygenerator.model.DirHintAnnotation
99
import godot.entrygenerator.model.EnumFlagHintStringAnnotation
@@ -43,7 +43,7 @@ object PropertyHintStringGeneratorProvider {
4343
is RangeHintAnnotation<*> -> RangeHintStringGenerator(registeredProperty)
4444
null -> when {
4545
registeredProperty.type.isNodeType() -> NodeTypeHintStringGenerator(registeredProperty)
46-
registeredProperty.type.isReference() -> ResourceHintStringGenerator(registeredProperty)
46+
registeredProperty.type.isRefCounted() -> ResourceHintStringGenerator(registeredProperty)
4747
registeredProperty.type.isCompatibleList() -> ArrayHintStringGenerator(registeredProperty)
4848
else -> object : PropertyHintStringGenerator<PropertyHintAnnotation>(registeredProperty) {
4949
override fun getHintString(): String {

kt/entry-generation/godot-entry-generator/src/main/kotlin/godot/entrygenerator/generator/typehint/PropertyTypeHintProvider.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
package godot.entrygenerator.generator.typehint
22

33
import com.squareup.kotlinpoet.ClassName
4-
import godot.entrygenerator.ext.*
4+
import godot.entrygenerator.ext.hasAnnotation
5+
import godot.entrygenerator.ext.isCompatibleList
6+
import godot.entrygenerator.ext.isCoreType
7+
import godot.entrygenerator.ext.isNodeType
8+
import godot.entrygenerator.ext.isRefCounted
59
import godot.entrygenerator.generator.typehint.array.JvmArrayTypeHintGenerator
610
import godot.entrygenerator.generator.typehint.coretypes.JvmCoreTypeTypeHintGenerator
711
import godot.entrygenerator.generator.typehint.primitives.JvmPrimitivesTypeHintGenerator
@@ -53,7 +57,7 @@ object PropertyTypeHintProvider {
5357
registeredProperty
5458
).getPropertyTypeHint()
5559

56-
registeredProperty.type.isReference() -> ClassName(
60+
registeredProperty.type.isRefCounted() -> ClassName(
5761
"$godotApiPackage.${GodotTypes.propertyHint}",
5862
"PROPERTY_HINT_RESOURCE_TYPE"
5963
)

kt/godot-library/src/main/kotlin/godot/core/bridge/RID.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class RID(
77
* Returns the ID of the referenced low-level resource.
88
*/
99
val id: Long
10-
) : Comparable<RID> {
10+
) : Comparable<RID>, CoreType {
1111

1212

1313
//CONSTRUCTOR

kt/plugins/godot-intellij-plugin/src/main/kotlin/godot/intellij/plugin/annotator/property/RegisterPropertiesAnnotator.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import godot.intellij.plugin.quickfix.PropertyNotRegisteredQuickFix
1616
import godot.intellij.plugin.quickfix.PropertyRemoveExportAnnotationQuickFix
1717
import godot.intellij.plugin.quickfix.RegisterPropertyMutabilityQuickFix
1818
import godot.tools.common.constants.GodotKotlinJvmTypes
19-
import godot.tools.common.constants.GodotTypes
2019
import godot.tools.common.constants.godotAnnotationPackage
2120
import godot.tools.common.constants.godotApiPackage
2221
import godot.tools.common.constants.godotCorePackage
@@ -49,6 +48,7 @@ class RegisterPropertiesAnnotator : Annotator {
4948
checkMutability(element, holder)
5049
checkRegisteredType(element, holder)
5150
lateinitChecks(element, holder)
51+
nullableChecks(element, holder)
5252
}
5353
// outside to check if the property is also registered
5454
propertyHintAnnotationChecker.checkPropertyHintAnnotations(element, holder)
@@ -133,6 +133,18 @@ class RegisterPropertiesAnnotator : Annotator {
133133
}
134134
}
135135

136+
private fun nullableChecks(ktProperty: KtProperty, holder: AnnotationHolder) {
137+
if (
138+
ktProperty.type()?.isNullable() == true
139+
&& (ktProperty.type()?.isCoreType() == true || ktProperty.type()?.isGodotPrimitive() == true)
140+
) {
141+
holder.registerProblem(
142+
message = GodotPluginBundle.message("problem.property.nullable"),
143+
errorLocation = ktProperty.nameIdentifier ?: ktProperty.navigationElement,
144+
)
145+
}
146+
}
147+
136148
private fun getInitializerProblemLocation(ktProperty: KtProperty) =
137149
ktProperty.initializer?.psiOrParent ?: ktProperty.nameIdentifier ?: ktProperty.navigationElement
138150

kt/plugins/godot-intellij-plugin/src/main/kotlin/godot/intellij/plugin/extension/kotlinTypeExt.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import godot.tools.common.constants.godotCorePackage
66
import godot.tools.common.constants.godotUtilPackage
77
import org.jetbrains.kotlin.idea.base.utils.fqname.fqName
88
import org.jetbrains.kotlin.js.descriptorUtils.getKotlinTypeFqName
9-
109
import org.jetbrains.kotlin.types.KotlinType
1110
import org.jetbrains.kotlin.types.typeUtil.supertypes
1211

@@ -28,7 +27,6 @@ fun KotlinType.isCoreType(): Boolean = getKotlinTypeFqName(false)
2827
.removeSuffix("?") == "$godotCorePackage.${GodotTypes.coreType}"
2928
|| supertypes().any { supertype -> supertype.isCoreType() }
3029

31-
3230
fun KotlinType.isGodotPrimitive(): Boolean = when (this.fqName?.asString()?.removeSuffix("?")) {
3331
Int::class.qualifiedName,
3432
"$godotUtilPackage.${GodotKotlinJvmTypes.naturalT}",

0 commit comments

Comments
 (0)