-
Notifications
You must be signed in to change notification settings - Fork 45
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
Replace Kotlin getters/setters with property access in codegen #496 #1002
Merged
volivan239
merged 9 commits into
main
from
volivan239/fix_kotlin_property_access_rendering
Sep 27, 2022
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
5950012
Replace Kotlin getters/setters with property access in codegen
volivan239 3a93809
Corrected behavior for static fields
volivan239 4a46a78
Always use safe call (?.) in Kotlin codegen
volivan239 2143680
Fix calling wrong unary plus
volivan239 c8739aa
Fixed test crashing due to nullability problems
volivan239 c8a8407
Treat java-accessible fields as accessible properties in Kotlin
volivan239 2245a09
Fix false-positive canBeSetIn result for private fields with getters …
volivan239 569e581
Add comments
volivan239 38fb0d0
Minor changes fixing issues found in review
volivan239 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 50 additions & 3 deletions
53
utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,70 @@ | ||
| package org.utbot.framework.codegen.model.util | ||
|
|
||
| import org.utbot.framework.codegen.model.constructor.context.CgContext | ||
| import org.utbot.framework.plugin.api.CodegenLanguage | ||
| import org.utbot.framework.plugin.api.FieldId | ||
| import org.utbot.framework.plugin.api.MethodId | ||
| import org.utbot.framework.plugin.api.util.voidClassId | ||
|
|
||
| /** | ||
| * For now we will count field accessible if it is not private and its class is also accessible, | ||
| * because we generate tests in the same package with the class under test, | ||
| * which means we can access public, protected and package-private fields | ||
| * | ||
| * @param packageName name of the package we check accessibility from | ||
| * @param context context in which code is generated (it is needed because the method needs to know package and language) | ||
| */ | ||
| infix fun FieldId.isAccessibleFrom(packageName: String): Boolean { | ||
| // TODO: change parameter from packageName: String to context: CgContext in ClassId.isAccessibleFrom and ExecutableId.isAccessibleFrom ? | ||
| private fun FieldId.isAccessibleFrom(context: CgContext): Boolean { | ||
| val packageName = context.testClassPackageName | ||
| val isClassAccessible = declaringClass.isAccessibleFrom(packageName) | ||
| val isAccessibleByVisibility = isPublic || (declaringClass.packageName == packageName && (isPackagePrivate || isProtected)) | ||
| val isAccessibleFromPackageByModifiers = isAccessibleByVisibility && !isSynthetic | ||
|
|
||
| return isClassAccessible && isAccessibleFromPackageByModifiers | ||
| } | ||
|
|
||
| private fun FieldId.canBeReadViaGetterFrom(context: CgContext): Boolean = | ||
| declaringClass.allMethods.contains(getter) && getter.isAccessibleFrom(context.testClassPackageName) | ||
|
|
||
| /** | ||
| * Returns whether you can read field's value without reflection | ||
| */ | ||
| internal infix fun FieldId.canBeReadFrom(context: CgContext): Boolean { | ||
| if (context.codegenLanguage == CodegenLanguage.KOTLIN) { | ||
| // Kotlin will allow direct field access for non-static fields with accessible getter | ||
| if (!isStatic && canBeReadViaGetterFrom(context)) | ||
| return true | ||
| } | ||
|
|
||
| return isAccessibleFrom(context) | ||
| } | ||
|
|
||
| private fun FieldId.canBeSetViaSetterFrom(context: CgContext): Boolean = | ||
| declaringClass.allMethods.contains(setter) && setter.isAccessibleFrom(context.testClassPackageName) | ||
|
|
||
| /** | ||
| * Whether or not a field can be set without reflection | ||
| */ | ||
| fun FieldId.canBeSetIn(packageName: String): Boolean = isAccessibleFrom(packageName) && !isFinal | ||
| internal fun FieldId.canBeSetFrom(context: CgContext): Boolean { | ||
| if (context.codegenLanguage == CodegenLanguage.KOTLIN) { | ||
| // Kotlin will allow direct write access if both getter and setter is defined | ||
| // !isAccessibleFrom(context) is important here because above rule applies to final fields only if they are not accessible in Java terms | ||
| if (!isAccessibleFrom(context) && !isStatic && canBeReadViaGetterFrom(context) && canBeSetViaSetterFrom(context)) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return isAccessibleFrom(context) && !isFinal | ||
| } | ||
|
|
||
| /** | ||
| * The default getter method for field (the one which is generated by Kotlin compiler) | ||
| */ | ||
| val FieldId.getter: MethodId | ||
| get() = MethodId(declaringClass, "get${name.replaceFirstChar { it.uppercase() } }", type, emptyList()) | ||
|
|
||
| /** | ||
| * The default setter method for field (the one which is generated by Kotlin compiler) | ||
| */ | ||
| val FieldId.setter: MethodId | ||
| get() = MethodId(declaringClass, "set${name.replaceFirstChar { it.uppercase() } }", voidClassId, listOf(type)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I don't understand this change, but can believe that is is possible :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, this method can't return null (at least, no compilation error is produced after this change), this may help to avoid unnecessary
?.-calls