Basic PHP support (AST edges etc.) and DeclarationUsage edge providing #38

max-martynov · 2022-12-16T11:22:29Z

No description provided.

egor-bogomolov · 2022-12-16T11:24:29Z

Begins implementation of #32

egor-bogomolov

Awesome job 🚀 Could you please fix a few minor comments before merging?

egor-bogomolov · 2022-12-16T11:32:24Z

build.gradle.kts

@@ -57,7 +57,7 @@ allprojects {
        type.set(getProperty("platformType"))
        downloadSources.set(getProperty("platformDownloadSources").toBoolean())
        updateSinceUntilBuild.set(true)
-        plugins.set(getProperty("platformPlugins").split(',').map(String::trim).filter(String::isNotEmpty))


Let's avoid duplicating string constants and load them from gradle.properties when possible. In future, we should do the same for different lib versions that are shared in multiple places.

val platformVersion = getProperty("platformVersion") version.set(platformVersion) type.set(getProperty("platformType")) downloadSources.set(getProperty("platformDownloadSources").toBoolean()) updateSinceUntilBuild.set(true) plugins.set( getProperty("platformPlugins").split(',').map(String::trim).filter(String::isNotEmpty) + listOf("com.jetbrains.php:$platformVersion") )

I moved the declaration of this plugin to the gradle.properties, however the version specification is still required. So it builds only when like this:

kotlin.code.style=official

platformType=IU
platformVersion=222.4345.14
platformDownloadSources=true
platformPlugins = com.intellij.java, org.jetbrains.kotlin, android, maven, gradle, Groovy, com.jetbrains.php:222.4345.14
...

Is it okay?

egor-bogomolov · 2022-12-16T11:35:31Z

psiminer-cli/src/main/kotlin/config/StorageConfigs.kt

+        when (language) {
+            Language.Java -> JsonGraphStorage(outputDirectory, JavaGraphMiner())
+            Language.PHP -> JsonGraphStorage(outputDirectory, PhpGraphMiner())
+            else -> throw IllegalArgumentException("rip")


Let's write more elaborate error messages 🤣

Oops 🤪
Wanted to change it but forgot...

egor-bogomolov · 2022-12-16T11:39:49Z

psiminer-core/src/main/kotlin/Common.kt

@@ -24,7 +24,8 @@ enum class Dataset(val folderName: String) {

 enum class Language(val extensions: List<String>) {
    Java(listOf("java")),
-    Kotlin(listOf("kt", "kts"))
+    Kotlin(listOf("kt", "kts")),
+    PHP(listOf("php"))


In theory, there are older PHP extensions such as php3, php4, php5, phps. Will parsing work for them and should we add them to the list?

Well, hard to say because I didn't manage to find any examples of such files. Even the oldest PHP files on GitHub have .php extension so I'd rather not support any other formats. If you have another opinion, we can always change it quickly.

egor-bogomolov · 2022-12-16T20:34:29Z

psiminer-core/src/main/kotlin/psi/method/PHPMethodProvider.kt

+
+    override fun getBodyNode(root: PsiElement): PsiElement? =
+        (root as? MethodImpl)?.lastChild
+            ?: throw IllegalArgumentException("Try to extract name not from the method")


Error message should mention body. Let's move the exceptions to a separate file in order to avoid duplicating and keeping them consistent across languages.

egor-bogomolov · 2022-12-16T20:37:00Z

psiminer-core/src/main/kotlin/psi/method/PHPMethodProvider.kt

+            ?: throw IllegalArgumentException("Try to extract name not from the method")
+
+    override fun getDocComment(root: PsiElement): PsiElement? {
+        return PsiTreeUtil.collectElementsOfType(root, PhpDocCommentImpl::class.java).firstOrNull()


It would be cleaner to use the PhpDocComment interface rather than its implementation, as in the future there might be a different implementation of this interface.

egor-bogomolov · 2022-12-16T20:37:14Z

psiminer-core/src/main/kotlin/psi/method/PHPMethodProvider.kt

+    }
+
+    override fun getNonDocComments(root: PsiElement): Collection<PsiElement> {
+        return PsiTreeUtil.collectElementsOfType(root, PsiComment::class.java).filterNot { it is PhpDocCommentImpl }


Same about PhpDocComment

egor-bogomolov · 2022-12-16T20:39:33Z

psiminer-core/src/main/kotlin/psi/method/PHPMethodProvider.kt

+        (root as? MethodImpl)?.name == "__construct"
+
+    override fun hasModifier(root: PsiElement, modifier: String): Boolean =
+        (root as? MethodImpl)?.modifier?.toString()?.split(' ')?.contains(modifier)


Can you cast to PsiMethod or MethodImpl is strictly necessary here?

I tested casting to PsiMethod and it didn't work :(

Weird, let's put here a comment with TODO to investigate it later

egor-bogomolov · 2022-12-16T20:40:33Z

...iner-core/src/main/kotlin/psi/transformations/excludenode/ExcludePhpSymbolsTransformation.kt

+import psi.transformations.PhpTreeTransformation
+
+class ExcludePhpSymbolsTransformation : PhpTreeTransformation, ExcludeNodeTransformation() {
+    private val skipElementTypes = listOf("(", ")", "{", "}", "arrow", "semicolon", "comma", "dot")


If you want to use in it might be a good idea to make a set rather than a list.

egor-bogomolov

Great, now the PR looks good!

egor-bogomolov · 2023-01-12T23:33:31Z

psiminer-core/src/main/kotlin/psi/method/PHPMethodProvider.kt

+        (root as? MethodImpl)?.name == "__construct"
+
+    override fun hasModifier(root: PsiElement, modifier: String): Boolean =
+        (root as? MethodImpl)?.modifier?.toString()?.split(' ')?.contains(modifier)


Weird, let's put here a comment with TODO to investigate it later

egor-bogomolov · 2023-01-12T23:36:32Z

psiminer-core/src/main/kotlin/psi/method/MethodProvider.kt

@@ -19,4 +19,7 @@ abstract class MethodProvider {
    open fun stringsToCommentString(c: Collection<String>): String {
        return c.filterNot { it.none { s -> s.isLetterOrDigit() } }.flatMap { splitToSubtokens(it) }.joinToString("|")
    }
+
+    open class NotAMethodException(absentToken: String) :
+        IllegalArgumentException("Try to extract $absentToken not from the method")


Let's change wording here. I suggest something like this: "Cannot extract $absentToken : class $className is not a method". For this, you should also pass the encountered class name.

egor-bogomolov · 2023-01-12T23:41:07Z

gradle.properties

@@ -3,7 +3,7 @@ kotlin.code.style=official
 platformType=IU
 platformVersion=222.4345.14
 platformDownloadSources=true
-platformPlugins = com.intellij.java, org.jetbrains.kotlin, android, maven, gradle, Groovy
+platformPlugins = com.intellij.java, org.jetbrains.kotlin, android, maven, gradle, Groovy, com.jetbrains.php:222.4345.14


I did not know it, but gradle.properties actually does not support string interpolation 😞

Let's keep it like this for now, but I guess we should move constants to build file or some separate file in the future.

max-martynov added 6 commits December 7, 2022 23:46

Trying to make php plugin work

9a62cdb

Change platform type to ultimate

3afff92

Merge branch 'ultimate-edition' into php-support

df2fd16

PHP plugin installed

11dae58

Basic php support, start adding data flow edges

2d62e60

Complete providing of declaration edges for php, add tests

7816058

max-martynov requested a review from egor-bogomolov December 16, 2022 11:22

egor-bogomolov reviewed Dec 16, 2022

View reviewed changes

Refactor code

3221b77

egor-bogomolov approved these changes Jan 12, 2023

View reviewed changes

egor-bogomolov merged commit 5ac3e75 into main Jan 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic PHP support (AST edges etc.) and DeclarationUsage edge providing #38

Basic PHP support (AST edges etc.) and DeclarationUsage edge providing #38

max-martynov commented Dec 16, 2022

egor-bogomolov commented Dec 16, 2022

egor-bogomolov left a comment

egor-bogomolov Dec 16, 2022

max-martynov Dec 19, 2022

egor-bogomolov Dec 16, 2022

max-martynov Dec 19, 2022

egor-bogomolov Dec 16, 2022

max-martynov Dec 19, 2022

egor-bogomolov Dec 16, 2022

egor-bogomolov Dec 16, 2022

egor-bogomolov Dec 16, 2022

egor-bogomolov Dec 16, 2022

max-martynov Dec 19, 2022

egor-bogomolov Jan 12, 2023

egor-bogomolov Dec 16, 2022

egor-bogomolov left a comment

egor-bogomolov Jan 12, 2023

egor-bogomolov Jan 12, 2023

egor-bogomolov Jan 12, 2023

Basic PHP support (AST edges etc.) and DeclarationUsage edge providing #38

Basic PHP support (AST edges etc.) and DeclarationUsage edge providing #38

Conversation

max-martynov commented Dec 16, 2022

egor-bogomolov commented Dec 16, 2022

egor-bogomolov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egor-bogomolov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment