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

Convert function to property shouldn't insert explicit type if it was inferred previously #1011

Merged
merged 1 commit into from
May 19, 2017

Conversation

cypressious
Copy link
Contributor

Fixes #KT-14820

elementsToShorten.add(property.typeReference!!)

if (!hasExplicitType) {
val linebreak = property.getter?.node?.treePrev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch. This should be handled by a formatter rule, not by AST manipulations.

Copy link
Contributor Author

@cypressious cypressious Jan 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this isn't pretty. Currently, the formatter allows both, getters on the same line and on the next line. Forcing one of them could be to strict.

Maybe the KtPsiFactory could get an option to create a property with the getter on the same line. What do you think @yole?

Edit: Maybe that's already possible and we should just use the PsiFactory to create the whole thing instead of reconstructing the KtNamedFunction part by part?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually yes, the way this intention replaces the internal child nodes of KtFunction is quite ugly. I think it would be much nicer to use KtPsiFactory.CallableBuilder to construct the property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason for the current way is because it preserves annotations, modifiers, receiver type and other things I probably forgot. If we redid it, we would have to manually take care of it.

@yole yole self-assigned this Jan 27, 2017
@cypressious
Copy link
Contributor Author

@yole I reworked the code and added some more tests. I think it's cleaner now.

… to property shouldn't insert explicit type if it was inferred previously'

Fixes #KT-14820
@yole yole merged commit 4cbdbaa into JetBrains:master May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants