Merge pull request #43 from JetBrains-Research/php-small-fixes

Php small fixes
JetBrains-Research · Feb 6, 2023 · 6b647ea · 6b647ea
2 parents bee0af8 + fea1827
commit 6b647ea
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 17 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -24,27 +24,24 @@ allprojects {
     repositories {
         mavenCentral()
         maven(url = "https://packages.jetbrains.team/maven/p/astminer/astminer")
+        maven("https://packages.jetbrains.team/maven/p/big-code/bigcode")
     }
 
     dependencies {
         implementation("org.jetbrains.kotlin:kotlin-stdlib:1.7.21")
         implementation("org.jetbrains.kotlinx:kotlinx-serialization-core:1.4.1")
         implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.4.1")
         implementation("it.unimi.dsi:fastutil:8.5.9")
+        implementation("org.jetbrains.research:plugin-utilities-core:1.0") {
+            exclude("org.slf4j", "slf4j-simple")
+            exclude("org.slf4j", "slf4j-api")
+            exclude("org.slf4j", "slf4j")
+        }
 
         testImplementation(platform("org.junit:junit-bom:5.9.0"))
         testImplementation("org.junit.jupiter:junit-jupiter:5.9.0")
 
         detektPlugins("io.gitlab.arturbosch.detekt:detekt-formatting:1.18.1")
-
-        implementation("${getProperty("utilitiesProjectName")}:plugin-utilities-core") {
-            version {
-                branch = getProperty("utilitiesBranch")
-            }
-            exclude("org.slf4j", "slf4j-simple")
-            exclude("org.slf4j", "slf4j-api")
-            exclude("org.slf4j", "slf4j")
-        }
     }
 
     configurations.all {

diff --git a/psiminer-cli/build.gradle.kts b/psiminer-cli/build.gradle.kts
@@ -15,7 +15,8 @@ tasks {
         args = listOfNotNull("psiminer", dataset, output, config)
         jvmArgs = listOf(
             "-Djava.awt.headless=true", "-Djdk.module.illegalAccess.silent=true",
-            "--add-exports", "java.base/jdk.internal.vm=ALL-UNNAMED", "-Xmx32G"
+            "--add-exports", "java.base/jdk.internal.vm=ALL-UNNAMED", "-Xmx32G",
+            "-Didea.is.internal=false", "-Dlog4j.configurationFile=psiminer-cli/src/main/resources/log4j.properties"
         )
         maxHeapSize = "32g"
     }

diff --git a/psiminer-cli/src/main/resources/log4j.properties b/psiminer-cli/src/main/resources/log4j.properties
@@ -1,5 +1,5 @@
 # Define the root logger with appender file
-log4j.rootLogger=WARN, FILE
+log4j.rootLogger=SEVERE, FILE
 
 # Define the file appender
 log4j.appender.FILE=org.apache.log4j.FileAppender

diff --git a/psiminer-core/src/main/kotlin/psi/graphs/edgeProviders/php/PhpControlFlowEdgeProvider.kt b/psiminer-core/src/main/kotlin/psi/graphs/edgeProviders/php/PhpControlFlowEdgeProvider.kt
@@ -3,7 +3,6 @@ package psi.graphs.edgeProviders.php
 import com.intellij.psi.PsiElement
 import com.intellij.psi.controlFlow.*
 import com.jetbrains.php.codeInsight.controlFlow.PhpControlFlow
-import com.jetbrains.php.codeInsight.controlFlow.instructions.PhpExitPointInstruction
 import com.jetbrains.php.codeInsight.controlFlow.instructions.PhpInstruction
 import com.jetbrains.php.lang.psi.elements.ControlStatement
 import com.jetbrains.php.lang.psi.elements.PhpReturn
@@ -77,7 +76,6 @@ class PhpControlFlowEdgeProvider : EdgeProvider(
 
     private fun provideEdgesForInstruction(
         index: Int,
-        instructions: List<PhpInstruction>,
         elements: List<PsiElement?>,
         successors: Array<out Set<Int>>,
         nextNonBranchingInstructions: Array<out Set<Int>>,
@@ -101,19 +99,19 @@ class PhpControlFlowEdgeProvider : EdgeProvider(
                 newEdges.add(Edge(element, toElement, EdgeType.ControlElement))
             }
         }
-        if (element is PhpReturn || isTerminalInstruction(index, instructions, successors)) {
+        if (isTerminalInstruction(index, elements, successors)) {
             addReturnsToEdge(element, elements, newEdges)
         }
         return newEdges
     }
 
     private fun isTerminalInstruction(
         index: Int,
-        instructions: List<PhpInstruction>,
+        elements: List<PsiElement?>,
         successors: Array<out Set<Int>>,
     ): Boolean =
         successors[index].isEmpty() ||
-                successors[index].map { instructions[it] }.any { it is PhpExitPointInstruction }
+                successors[index].map { elements[it] }.all { it == null }
 
     private fun addReturnsToEdge(
         element: PsiElement,
@@ -139,7 +137,7 @@ class PhpControlFlowEdgeProvider : EdgeProvider(
         val nextNonBranchingInstructions = computeNextNonBranchingInstructions(instructions, elements, successors)
         instructions.indices.forEach { index ->
             newEdges.addAll(
-                provideEdgesForInstruction(index, instructions, elements, successors, nextNonBranchingInstructions)
+                provideEdgesForInstruction(index, elements, successors, nextNonBranchingInstructions)
             )
         }
     }

diff --git a/psiminer-core/src/test/kotlin/psi/graphs/edgeProviders/php/PhpControlFlowEdgeProviderTest.kt b/psiminer-core/src/test/kotlin/psi/graphs/edgeProviders/php/PhpControlFlowEdgeProviderTest.kt
@@ -64,6 +64,34 @@ internal class PhpControlFlowEdgeProviderTest : PhpGraphTest("PhpFlowMethods") {
         }
     }
 
+    @ParameterizedTest
+    @ValueSource(
+        strings = [
+            "straightWriteMethod",
+            "multipleReturns",
+            "forWithReturn"
+        ]
+    )
+    fun `test returns extraction from PHP methods`(methodName: String) {
+        val psiRoot = getMethod(methodName)
+        val graphMiner = PhpGraphMiner()
+        ReadAction.run<Exception> {
+            val codeGraph = graphMiner.mine(psiRoot)
+            val controlFlowEdges =
+                codeGraph.edges.withType(EdgeType.ReturnsTo).flatMap { (_, edges) -> edges.forward() }
+            assertContainsElements(
+                countIncomingEdges(controlFlowEdges).entries,
+                correctNumberOfReturnsToEdges.incoming[methodName]?.entries
+                    ?: throw CorrectValueNotProvidedException(methodName, "returnsTo")
+            )
+            assertContainsElements(
+                countOutgoingEdges(controlFlowEdges).entries,
+                correctNumberOfReturnsToEdges.outgoing[methodName]?.entries
+                    ?: throw CorrectValueNotProvidedException(methodName, "returnsTo")
+            )
+        }
+    }
+
     companion object {
 
         val correctNumberOfControlFlowEdges = CorrectNumberOfIncomingAndOutgoingEdges(
@@ -131,5 +159,33 @@ internal class PhpControlFlowEdgeProviderTest : PhpGraphTest("PhpFlowMethods") {
                 )
             )
         )
+
+        val correctNumberOfReturnsToEdges = CorrectNumberOfIncomingAndOutgoingEdges(
+            incoming = mapOf(
+                "straightWriteMethod" to mapOf(
+                    Vertex("MethodImpl: straightWriteMethod", Pair(0, 20)) to 1
+                ),
+                "multipleReturns" to mapOf(
+                    Vertex("MethodImpl: multipleReturns", Pair(0, 20)) to 3
+                ),
+                "forWithReturn" to mapOf(
+                    Vertex("MethodImpl: forWithReturn", Pair(0, 20)) to 2
+                )
+            ),
+            outgoing = mapOf(
+                "straightWriteMethod" to mapOf(
+                    Vertex("VariableImpl: c", Pair(4, 8)) to 1
+                ),
+                "multipleReturns" to mapOf(
+                    Vertex("PhpExpressionImpl: 0", Pair(4, 19)) to 1,
+                    Vertex("PhpExpressionImpl: 1", Pair(8, 23)) to 1,
+                    Vertex("PhpExpressionImpl: 2", Pair(11, 15)) to 1
+                ),
+                "forWithReturn" to mapOf(
+                    Vertex("Return", Pair(4, 16)) to 1,
+                    Vertex("VariableImpl: e", Pair(7, 8)) to 1,
+                )
+            )
+        )
     }
 }
diff --git a/psiminer-core/src/test/resources/data/php/PhpFlowMethods.php b/psiminer-core/src/test/resources/data/php/PhpFlowMethods.php
@@ -82,4 +82,14 @@ public function multipleReturns(): int
         }
         return 2;
     }
+
+    public function forWithReturn()
+    {
+        for ($i = 0; $i < 2; $i++) {
+            if ($i == 1) {
+                return;
+            }
+        }
+        $e = 5;
+    }
 }