Convert to intention & Support non-empty constructor #6
+58
−81
Conversation
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
shiraji
force-pushed
the
convert_to_intention
branch
2 times, most recently
from
cbc04b4
to
2e488a2
Compare
shiraji
force-pushed
the
convert_to_intention
branch
from
2e488a2
to
d762b4f
Compare
suusan2go
reviewed
Sep 1, 2018
| result = "$result$parameterString" | ||
| private fun createParameterSetterExpression(element: KtValueArgumentList, parameters: List<ValueParameterDescriptor>) { | ||
| val arguments = element.arguments | ||
| val argumentNames = arguments.map { it.getArgumentName()?.asName?.identifier }.filterNotNull() |
There was a problem hiding this comment.
I think we can simplify this by mapNotNull 👍
suusan2go
reviewed
Sep 1, 2018
| if (arguments.size > index && !arguments[index].isNamed()) return@forEachIndexed | ||
| if (argumentNames.contains(parameter.name.identifier)) return@forEachIndexed | ||
| val newArgument = factory.createArgument( | ||
| expression = factory.createExpression(createDefaultValueFromParameter(parameter)), |
There was a problem hiding this comment.
factory.createExpression throws java.lang.IllegalStateException when createDefaultValueFromParameter returns empty string.
java.lang.IllegalStateException: Failed to create expression from text: ''
at org.jetbrains.kotlin.psi.KtPsiFactory.createExpression(KtPsiFactory.kt:59)
at com.github.suusan2go.kotlinfillclass.intentions.FillClassIntention.createParameterSetterExpression(FillClassIntention.kt:50)
at com.github.suusan2go.kotlinfillclass.intentions.FillClassIntention.applyTo(FillClassIntention.kt:25)
at com.github.suusan2go.kotlinfillclass.intentions.FillClassIntention.applyTo(FillClassIntention.kt:17)
at org.jetbrains.kotlin.idea.intentions.SelfTargetingIntention.invoke(SelfTargetingIntention.kt:107)
at com.intellij.codeInsight.intention.impl.config.IntentionActionWrapper.invoke(IntentionActionWrapper.java:67)
at com.intellij.codeInsight.intention.impl.IntentionActionWithTextCaching$MyIntentionAction.invoke(IntentionActionWithTextCaching.java:173)
at com.intellij.codeInsight.intention.impl.ShowIntentionActionsHandler.lambda$invokeIntention$3(ShowIntentionActionsHandler.java:200)
at com.intellij.openapi.application.WriteAction.run(WriteAction.java:105)
at com.intellij.codeInsight.intention.impl.ShowIntentionActionsHandler.invokeIntention(ShowIntentionActionsHandler.java:202)
at com.intellij.codeInsight.intention.impl.ShowIntentionActionsHandler.lambda$null$1(ShowIntentionActionsHandler.java:177)
at com.intellij.openapi.application.TransactionGuardImpl.runSyncTransaction(TransactionGuardImpl.java:88)
at com.intellij.openapi.application.TransactionGuardImpl.submitTransactionAndWait(TransactionGuardImpl.java:153)
at com.intellij.codeInsight.intention.impl.ShowIntentionActionsHandler.lambda$chooseActionAndInvoke$2(ShowIntentionActionsHandler.java:176)
at com.intellij.openapi.command.impl.CoreCommandProcessor.executeCommand(CoreCommandProcessor.java:139)
at com.intellij.openapi.command.impl.CoreCommandProcessor.executeCommand(CoreCommandProcessor.java:97)
at com.intellij.openapi.command.impl.CoreCommandProcessor.executeCommand(CoreCommandProcessor.java:87)
at com.intellij.openapi.command.impl.CoreCommandProcessor.executeCommand(CoreCommandProcessor.java:73)
at com.intellij.codeInsight.intention.impl.ShowIntentionActionsHandler.chooseActionAndInvoke(ShowIntentionActionsHandler.java:175)
at com.intellij.codeInsight.intention.impl.IntentionListStep.lambda$applyAction$0(IntentionListStep.java:118)
at com.intellij.openapi.application.TransactionGuardImpl.performUserActivity(TransactionGuardImpl.java:195)
at com.intellij.ui.popup.AbstractPopup.lambda$dispose$8(AbstractPopup.java:1405)
at com.intellij.util.ui.UIUtil.invokeLaterIfNeeded(UIUtil.java:3078)
at com.intellij.ide.IdeEventQueue.ifFocusEventsInTheQueue(IdeEventQueue.java:176)
at com.intellij.ide.IdeEventQueue.executeWhenAllFocusEventsLeftTheQueue(IdeEventQueue.java:132)
at com.intellij.openapi.wm.impl.FocusManagerImpl.doWhenFocusSettlesDown(FocusManagerImpl.java:190)
at com.intellij.openapi.wm.impl.IdeFocusManagerImpl.doWhenFocusSettlesDown(IdeFocusManagerImpl.java:58)
at com.intellij.ui.popup.AbstractPopup.dispose(AbstractPopup.java:1399)
at com.intellij.ui.popup.WizardPopup.dispose(WizardPopup.java:160)
at com.intellij.ui.popup.list.ListPopupImpl.dispose(ListPopupImpl.java:307)
at com.intellij.openapi.util.Disposer$1.execute(Disposer.java:48)
at com.intellij.openapi.util.Disposer$1.execute(Disposer.java:44)
at com.intellij.openapi.util.objectTree.ObjectNode$1.execute(ObjectNode.java:138)
at com.intellij.openapi.util.objectTree.ObjectNode$1.execute(ObjectNode.java:107)
at com.intellij.openapi.util.objectTree.ObjectTree.executeActionWithRecursiveGuard(ObjectTree.java:182)
at com.intellij.openapi.util.objectTree.ObjectNode.execute(ObjectNode.java:107)
at com.intellij.openapi.util.objectTree.ObjectTree.executeAll(ObjectTree.java:151)
at com.intellij.openapi.util.Disposer.dispose(Disposer.java:129)
at com.intellij.openapi.util.Disposer.dispose(Disposer.java:125)
at com.intellij.ui.popup.WizardPopup.disposeAllParents(WizardPopup.java:263)
at com.intellij.ui.popup.list.ListPopupImpl.handleNextStep(ListPopupImpl.java:442)
at com.intellij.ui.popup.list.ListPopupImpl._handleSelect(ListPopupImpl.java:396)
at com.intellij.ui.popup.list.ListPopupImpl.handleSelect(ListPopupImpl.java:337)
at com.intellij.ui.popup.list.ListPopupImpl$1.actionPerformed(ListPopupImpl.java:250)
at com.intellij.ui.popup.WizardPopup.proceedKeyEvent(WizardPopup.java:378)
at com.intellij.ui.popup.WizardPopup.dispatch(WizardPopup.java:358)
at com.intellij.ui.popup.PopupDispatcher.dispatchKeyEvent(PopupDispatcher.java:126)
at com.intellij.ui.popup.PopupDispatcher.dispatch(PopupDispatcher.java:160)
at com.intellij.ide.IdePopupManager.dispatch(IdePopupManager.java:86)
at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.java:673)
at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.java:382)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)So I think we have to pass null to expression if createDefaultValueFromParameter returns empty string.
There was a problem hiding this comment.
Changing createDefaultValueFromParameter to return null when not match KotlinBuiltins and handling like this would be better?
val defaultValue = createDefaultValueFromParameter(parameter)
val newArgument = factory.createArgument(
expression = defaultValue?.let { factory.createExpression(it) },
name = parameter.name
)There was a problem hiding this comment.
Do you want me to change the return value in this PR? I prefer not to do that, thou.
The things is I'm not quite sure what do you want to do when createDefaultValueFromParameter returns null.
data class Bar(val value: String)
data class Foo(val value: Bar)
fun foo() {
Foo(value) // compile error
}Instead, I like this one.
data class Bar(val value: String)
data class Foo(val value: Bar)
fun foo() {
Foo(value =
Bar(
value = ""
)
)
}There was a problem hiding this comment.
The point is that this pull-request changes the current filling behavior.
data class UserName(
val value: String
)
data class User(
val id: Int,
val name: UserName,
val age: Int
)// fill class target
val user = User()
// expected
val user = User(id = 0, name =, age = 0)
// got
val user = User(id = 0)I think this is because this exception commented here.
#6 (comment)
I agree with your comment that filling class like this is better and I want to support it in the future, but I have no idea how to implement it now 😅 Do you have any ideas ?
data class Bar(val value: String)
data class Foo(val value: Bar)
fun foo() {
Foo(value =
Bar(
value = ""
)
)
}
|
Sorry for late reply. Thanks for your great pull request 🍺 |
I'm not sure... |
|
@shiraji Thank you for your pull request and I'm sorry for late reply 😓 LGTM ! |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Here is the screenshot