-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[kotlin] K2 J2K: Move VarToValProcessing LocalVarToValInspectionBasedProcessing and fixValToVarDiagnosticBasedProcessing to JKTree #2743
base: master
Are you sure you want to change the base?
Conversation
This case can be handled separately, eg., you can use
Tests seem to be failing because of comparing declarations by name in |
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt
Show resolved
Hide resolved
still having an issue with also Otherwise, this seems to be working ! |
java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java
Outdated
Show resolved
Hide resolved
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/conversions/ClassMemberConversion.kt
Outdated
Show resolved
Hide resolved
@@ -1,5 +1,6 @@ | |||
class TestFieldInitializer(string: String) { | |||
var string: String = "" | |||
private set |
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.
Why this changed?
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.
In java, this string is private, but the class is public. Since it's a var
it should have a private setter or it can be changed from anywhere
java/java-analysis-impl/src/com/intellij/codeInspection/localCanBeFinal/LocalCanBeFinal.java
Outdated
Show resolved
Hide resolved
java/java-analysis-impl/src/com/intellij/codeInspection/localCanBeFinal/LocalCanBeFinal.java
Outdated
Show resolved
Hide resolved
java/java-analysis-impl/src/com/intellij/codeInspection/localCanBeFinal/LocalCanBeFinal.java
Show resolved
Hide resolved
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin/j2k/shared/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt
Outdated
Show resolved
Hide resolved
small changes Update ControlFlowUtil.java
ae90e1f
to
c5f0cff
Compare
back down to 2 updates Update LocalCanBeFinal.java extract immutableVariables back down to 2
(continuation of #2702 and #2726)
@darthorimar requested that I move the process of determining mutability earlier as it will be faster in the psi/java side of things. However, after completing it in this pull request I feel like this is actually impossible. There are many such cases where the code changes after the
JKBuilder
phase that would change mutability. For example:s1
ands2
are initially stub expressions and are only defined once in the class. This would indicate that they can beval
. However, bykt-19340.kt
,s1
ands2
are set equal to null initially, and thus have their values changed. There are many more such instances of this.additionally, per slack discussion we have noted that
nameConflict4
andnameConflict5
should be in a run block. However, the j2k is not creating these run blocks, hence those tests failing here.Thanks in advance for any feedback!
@abelkov @darthorimar @ermattt