Skip to content

Commit

Permalink
Improve fabric.mod.json entrypoints insight
Browse files Browse the repository at this point in the history 
- Recognize entrypoints declared in object form
- Add more conditions to the inspection
- Add some tests to cover the inspection

Fixes #2296
  • Loading branch information
@RedNesto
RedNesto committed Jul 2, 2024
1 parent 6de3376 commit 9da3841
Show file tree
Hide file tree
Showing 8 changed files with 396 additions and 45 deletions.
2 changes: 2 additions & 0 deletions build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ repositories {
maven("https://maven.fabricmc.net/") {
content {
includeModule("net.fabricmc", "mapping-io")
includeModule("net.fabricmc", "fabric-loader")
}
}
mavenCentral()
Expand Down Expand Up @@ -119,6 +120,7 @@ dependencies {
classifier = "shaded"
}
}
testLibs(libs.test.fabricloader)
testLibs(libs.test.nbt) {
artifact {
extension = "nbt"
Expand Down
1 change: 1 addition & 0 deletions gradle/libs.versions.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ fuel-coroutines = { module = "com.github.kittinunf.fuel:fuel-coroutines", versio
test-mockJdk = "org.jetbrains.idea:mock-jdk:1.7-4d76c50"
test-mixin = "org.spongepowered:mixin:0.8.5"
test-spongeapi = "org.spongepowered:spongeapi:7.4.0"
test-fabricloader = "net.fabricmc:fabric-loader:0.15.11"
test-nbt = "com.demonwav.mcdev:all-types-nbt:1.0"

junit-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" }
Expand Down
48 changes: 41 additions & 7 deletions src/main/kotlin/platform/fabric/inspection/FabricEntrypointsInspection.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ package com.demonwav.mcdev.platform.fabric.inspection

import com.demonwav.mcdev.platform.fabric.reference.EntryPointReference
import com.demonwav.mcdev.platform.fabric.util.FabricConstants
import com.demonwav.mcdev.util.equivalentTo
import com.intellij.codeInspection.InspectionManager
import com.intellij.codeInspection.LocalInspectionTool
import com.intellij.codeInspection.ProblemDescriptor
import com.intellij.codeInspection.ProblemHighlightType
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.json.psi.JsonArray
import com.intellij.json.psi.JsonElementVisitor
import com.intellij.json.psi.JsonLiteral
import com.intellij.json.psi.JsonProperty
import com.intellij.json.psi.JsonStringLiteral
import com.intellij.psi.JavaPsiFacade
Expand Down Expand Up @@ -79,8 +82,7 @@ class FabricEntrypointsInspection : LocalInspectionTool() {
val element = resolved.singleOrNull()?.element
when {
element is PsiClass && !literal.text.contains("::") -> {
val propertyKey = literal.parentOfType<JsonProperty>()?.name
val expectedType = propertyKey?.let { FabricConstants.ENTRYPOINT_BY_TYPE[it] }
val (propertyKey, expectedType) = findEntrypointKeyAndType(literal)
if (propertyKey != null && expectedType != null &&
!isEntrypointOfCorrectType(element, propertyKey)
) {
Expand Down Expand Up @@ -111,21 +113,43 @@ class FabricEntrypointsInspection : LocalInspectionTool() {
reference.rangeInElement,
)
}

if (!element.hasModifierProperty(PsiModifier.PUBLIC)) {
holder.registerProblem(
literal,
"Entrypoint method must be public",
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
reference.rangeInElement,
)
}

if (!element.hasModifierProperty(PsiModifier.STATIC)) {
val clazz = element.containingClass
if (clazz != null && clazz.constructors.isNotEmpty() &&
clazz.constructors.find { !it.hasParameters() } == null
) {
holder.registerProblem(
literal,
"Entrypoint instance method class must have an empty constructor",
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
reference.rangeInElement,
)
}
}
}

element is PsiField -> {
if (!element.hasModifierProperty(PsiModifier.STATIC)) {
if (!element.hasModifierProperty(PsiModifier.PUBLIC)) {
holder.registerProblem(
literal,
"Entrypoint field must be static",
"Entrypoint field must be public",
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
reference.rangeInElement,
)
}

val propertyKey = literal.parentOfType<JsonProperty>()?.name
val (propertyKey, expectedType) = findEntrypointKeyAndType(literal)
val fieldTypeClass = (element.type as? PsiClassType)?.resolve()
val expectedType = propertyKey?.let { FabricConstants.ENTRYPOINT_BY_TYPE[it] }
if (propertyKey != null && fieldTypeClass != null && expectedType != null &&
!isEntrypointOfCorrectType(fieldTypeClass, propertyKey)
) {
Expand All @@ -141,11 +165,21 @@ class FabricEntrypointsInspection : LocalInspectionTool() {
}
}

private fun findEntrypointKeyAndType(literal: JsonLiteral): Pair<String?, String?> {
val propertyKey = when (val parent = literal.parent) {
is JsonArray -> (parent.parent as? JsonProperty)?.name
is JsonProperty -> parent.parentOfType<JsonProperty>()?.name
else -> null
}
val expectedType = propertyKey?.let { FabricConstants.ENTRYPOINT_BY_TYPE[it] }
return propertyKey to expectedType
}

private fun isEntrypointOfCorrectType(element: PsiClass, type: String): Boolean {
val entrypointClass = FabricConstants.ENTRYPOINT_BY_TYPE[type]
?: return false
val clazz = JavaPsiFacade.getInstance(element.project).findClass(entrypointClass, element.resolveScope)
return clazz != null && element.isInheritor(clazz, true)
return clazz != null && (element.equivalentTo(clazz) || element.isInheritor(clazz, true))
}
}
}
54 changes: 27 additions & 27 deletions src/main/kotlin/platform/fabric/reference/EntryPointReference.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import com.demonwav.mcdev.util.fullQualifiedName
import com.demonwav.mcdev.util.manipulator
import com.demonwav.mcdev.util.reference.InspectionReference
import com.intellij.codeInsight.completion.JavaLookupElementBuilder
import com.intellij.json.psi.JsonArray
import com.intellij.json.psi.JsonProperty
import com.intellij.json.psi.JsonStringLiteral
import com.intellij.openapi.util.TextRange
import com.intellij.psi.JavaPsiFacade
Expand All @@ -40,7 +42,9 @@ import com.intellij.psi.PsiReference
import com.intellij.psi.PsiReferenceBase
import com.intellij.psi.PsiReferenceProvider
import com.intellij.psi.ResolveResult
import com.intellij.psi.search.GlobalSearchScope
import com.intellij.psi.search.searches.ClassInheritorsSearch
import com.intellij.psi.util.parentOfType
import com.intellij.util.ArrayUtil
import com.intellij.util.IncorrectOperationException
import com.intellij.util.ProcessingContext
Expand Down Expand Up @@ -136,27 +140,17 @@ object EntryPointReference : PsiReferenceProvider() {

fun isEntryPointReference(reference: PsiReference) = reference is Reference

fun isValidEntrypointClass(element: PsiClass): Boolean {
val psiFacade = JavaPsiFacade.getInstance(element.project)
var inheritsEntrypointInterface = false
for (entrypoint in FabricConstants.ENTRYPOINTS) {
val entrypointClass = psiFacade.findClass(entrypoint, element.resolveScope)
?: continue
if (element.isInheritor(entrypointClass, true)) {
inheritsEntrypointInterface = true
break
}
}
return inheritsEntrypointInterface
fun isValidEntrypointClass(element: PsiClass, entrypointClass: PsiClass): Boolean {
return element.isInheritor(entrypointClass, true)
}

fun isValidEntrypointField(field: PsiField): Boolean {
fun isValidEntrypointField(field: PsiField, entrypointClass: PsiClass): Boolean {
if (!field.hasModifierProperty(PsiModifier.PUBLIC) || !field.hasModifierProperty(PsiModifier.STATIC)) {
return false
}

val fieldTypeClass = (field.type as? PsiClassType)?.resolve()
return fieldTypeClass != null && isValidEntrypointClass(fieldTypeClass)
return fieldTypeClass != null && isValidEntrypointClass(fieldTypeClass, entrypointClass)
}

fun isValidEntrypointMethod(method: PsiMethod): Boolean {
Expand Down Expand Up @@ -228,30 +222,36 @@ object EntryPointReference : PsiReferenceProvider() {
val text = element.text.substring(range.startOffset, range.endOffset)
val parts = text.split("::", limit = 2)

val psiFacade = JavaPsiFacade.getInstance(element.project)
val entrypointType = getEntrypointType()?.let(FabricConstants.ENTRYPOINT_BY_TYPE::get)
?: return ArrayUtil.EMPTY_OBJECT_ARRAY
val entrypointClass = psiFacade.findClass(entrypointType, element.resolveScope)
?: return ArrayUtil.EMPTY_OBJECT_ARRAY

val variants = mutableListOf<Any>()
if (!isMemberReference) {
val psiFacade = JavaPsiFacade.getInstance(element.project)
for (entrypoint in FabricConstants.ENTRYPOINTS) {
val entrypointClass = psiFacade.findClass(entrypoint, element.resolveScope)
?: continue
ClassInheritorsSearch.search(entrypointClass, true)
.mapNotNullTo(variants) {
val shortName = it.name ?: return@mapNotNullTo null
val fqName = it.fullQualifiedName ?: return@mapNotNullTo null
JavaLookupElementBuilder.forClass(it, fqName, true).withPresentableText(shortName)
}
}
val scope = element.resolveScope.intersectWith(GlobalSearchScope.projectScope(element.project))
ClassInheritorsSearch.search(entrypointClass, scope, true)
.mapNotNullTo(variants) {
val shortName = it.name ?: return@mapNotNullTo null
val fqName = it.fullQualifiedName ?: return@mapNotNullTo null
JavaLookupElementBuilder.forClass(it, fqName, true).withPresentableText(shortName)
}
} else if (parts.size >= 2) {
val psiFacade = JavaPsiFacade.getInstance(element.project)
val className = parts[0].replace('$', '.')
val clazz = psiFacade.findClass(className, element.resolveScope)
if (clazz != null) {
clazz.fields.filterTo(variants, ::isValidEntrypointField)
clazz.fields.filterTo(variants) { isValidEntrypointField(it, entrypointClass) }
clazz.methods.filterTo(variants, ::isValidEntrypointMethod)
}
}

return variants.toTypedArray()
}

private fun getEntrypointType(): String? {
val entrypointsProperty = element.parentOfType<JsonArray>()?.parent as? JsonProperty
return entrypointsProperty?.name
}
}
}
26 changes: 17 additions & 9 deletions src/main/kotlin/platform/fabric/reference/FabricReferenceContributor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ package com.demonwav.mcdev.platform.fabric.reference
import com.demonwav.mcdev.platform.fabric.util.FabricConstants
import com.demonwav.mcdev.util.isPropertyValue
import com.intellij.json.psi.JsonArray
import com.intellij.json.psi.JsonElement
import com.intellij.json.psi.JsonObject
import com.intellij.json.psi.JsonStringLiteral
import com.intellij.patterns.PlatformPatterns
import com.intellij.patterns.StandardPatterns
import com.intellij.psi.PsiReferenceContributor
import com.intellij.psi.PsiReferenceRegistrar

Expand All @@ -34,19 +36,25 @@ class FabricReferenceContributor : PsiReferenceContributor() {
val stringInModJson = PlatformPatterns.psiElement(JsonStringLiteral::class.java)
.inVirtualFile(PlatformPatterns.virtualFile().withName(FabricConstants.FABRIC_MOD_JSON))

val entryPointPattern = stringInModJson.withParent(
PlatformPatterns.psiElement(JsonArray::class.java)
.withSuperParent(
2,
PlatformPatterns.psiElement(JsonObject::class.java).isPropertyValue("entrypoints"),
),
)
val entrypointsArray = PlatformPatterns.psiElement(JsonArray::class.java)
.withSuperParent(2, PlatformPatterns.psiElement(JsonObject::class.java).isPropertyValue("entrypoints"))
val entryPointSimplePattern = stringInModJson.withParent(entrypointsArray)
val entryPointObjectPattern = stringInModJson.isPropertyValue("value")
.withSuperParent(2, PlatformPatterns.psiElement(JsonObject::class.java).withParent(entrypointsArray))
val entryPointPattern = StandardPatterns.or(entryPointSimplePattern, entryPointObjectPattern)
registrar.registerReferenceProvider(entryPointPattern, EntryPointReference)

val mixinConfigPattern = stringInModJson.withParent(
val mixinConfigSimplePattern = stringInModJson.withParent(
PlatformPatterns.psiElement(JsonArray::class.java).isPropertyValue("mixins"),
)
registrar.registerReferenceProvider(mixinConfigPattern, ResourceFileReference("mixin config '%s'"))
val mixinsConfigArray = PlatformPatterns.psiElement(JsonArray::class.java).isPropertyValue("mixins")
val mixinConfigObjectPattern = stringInModJson.isPropertyValue("config")
.withSuperParent(2, PlatformPatterns.psiElement(JsonElement::class.java).withParent(mixinsConfigArray))
val mixinConfigPattern = StandardPatterns.or(mixinConfigSimplePattern, mixinConfigObjectPattern)
registrar.registerReferenceProvider(
mixinConfigPattern,
ResourceFileReference("mixin config '%s'", Regex("(.+)\\.mixins\\.json"))
)

registrar.registerReferenceProvider(
stringInModJson.isPropertyValue("accessWidener"),
Expand Down
40 changes: 38 additions & 2 deletions src/main/kotlin/platform/fabric/reference/ResourceFileReference.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ import com.demonwav.mcdev.util.manipulator
import com.demonwav.mcdev.util.mapFirstNotNull
import com.demonwav.mcdev.util.reference.InspectionReference
import com.intellij.json.psi.JsonStringLiteral
import com.intellij.openapi.application.runReadAction
import com.intellij.openapi.module.Module
import com.intellij.openapi.module.ModuleManager
import com.intellij.openapi.project.rootManager
import com.intellij.openapi.roots.ModuleRootManager
import com.intellij.openapi.vfs.findPsiFile
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiFile
import com.intellij.psi.PsiManager
Expand All @@ -37,13 +41,17 @@ import com.intellij.psi.PsiReferenceBase
import com.intellij.psi.PsiReferenceProvider
import com.intellij.util.IncorrectOperationException
import com.intellij.util.ProcessingContext
import org.jetbrains.jps.model.java.JavaResourceRootType

class ResourceFileReference(private val description: String) : PsiReferenceProvider() {
class ResourceFileReference(
private val description: String,
private val filenamePattern: Regex? = null
) : PsiReferenceProvider() {
override fun getReferencesByElement(element: PsiElement, context: ProcessingContext): Array<PsiReference> {
return arrayOf(Reference(description, element as JsonStringLiteral))
}

private class Reference(desc: String, element: JsonStringLiteral) :
private inner class Reference(desc: String, element: JsonStringLiteral) :
PsiReferenceBase<JsonStringLiteral>(element),
InspectionReference {
override val description = desc
Expand All @@ -61,6 +69,9 @@ class ResourceFileReference(private val description: String) : PsiReferenceProvi
?: ModuleRootManager.getInstance(module)
.getDependencies(false)
.mapFirstNotNull(::findFileIn)
?: ModuleManager.getInstance(element.project)
.getModuleDependentModules(module)
.mapFirstNotNull(::findFileIn)
}

override fun bindToElement(newTarget: PsiElement): PsiElement? {
Expand All @@ -70,5 +81,30 @@ class ResourceFileReference(private val description: String) : PsiReferenceProvi
val manipulator = element.manipulator ?: return null
return manipulator.handleContentChange(element, manipulator.getRangeInElement(element), newTarget.name)
}

override fun getVariants(): Array<out Any?> {
if (filenamePattern == null) {
return emptyArray()
}

val module = element.findModule() ?: return emptyArray()
val variants = mutableListOf<Any>()
val relevantModules = ModuleManager.getInstance(element.project).getModuleDependentModules(module) + module
runReadAction {
val relevantRoots = relevantModules.flatMap {
it.rootManager.getSourceRoots(JavaResourceRootType.RESOURCE)
}
for (roots in relevantRoots) {
for (child in roots.children) {
val relativePath = child.path.removePrefix(roots.path)
val testRelativePath = "/$relativePath"
if (testRelativePath.matches(filenamePattern)) {
variants.add(child.findPsiFile(element.project) ?: relativePath)
}
}
}
}
return variants.toTypedArray()
}
}
}
6 changes: 6 additions & 0 deletions src/test/kotlin/framework/ProjectBuilder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ class ProjectBuilder(private val fixture: JavaCodeInsightTestFixture, private va
configure: Boolean = true,
allowAst: Boolean = false,
) = file(path, code, ".nbtt", configure, allowAst)
fun json(
path: String,
@Language("JSON") code: String,
configure: Boolean = true,
allowAst: Boolean = false,
) = file(path, code, ".json", configure, allowAst)

inline fun dir(path: String, block: ProjectBuilder.() -> Unit) {
val oldIntermediatePath = intermediatePath
Expand Down
Loading

0 comments on commit 9da3841

Please sign in to comment.