From 7ac55fb6edeb6188a239acd2a410dfb5cf690edb Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 17 Dec 2023 02:10:02 +0000 Subject: [PATCH] Add mcdev support for Translatable.allowArbitraryArgs, for the 1.20.3 update --- .../minecraft/network/chat/annotations.xml | 6 + .../net/minecraft/text/annotations.xml | 6 + .../mcp/mappings/HardcodedYarnToMojmap.kt | 14 +- .../translations/TranslationConstants.kt | 1 + .../identification/TranslationIdentifier.kt | 32 +++- .../identification/TranslationInstance.kt | 1 + .../WrongTypeInTranslationArgsInspection.kt | 145 ++++++++++++++++++ src/main/kotlin/util/call-utils.kt | 17 +- src/main/resources/META-INF/plugin.xml | 7 + 9 files changed, 215 insertions(+), 14 deletions(-) create mode 100644 src/main/kotlin/translations/inspections/WrongTypeInTranslationArgsInspection.kt diff --git a/externalAnnotations/net/minecraft/network/chat/annotations.xml b/externalAnnotations/net/minecraft/network/chat/annotations.xml index 514952729..3fbb18cd9 100644 --- a/externalAnnotations/net/minecraft/network/chat/annotations.xml +++ b/externalAnnotations/net/minecraft/network/chat/annotations.xml @@ -39,6 +39,12 @@ + + + + + + diff --git a/externalAnnotations/net/minecraft/text/annotations.xml b/externalAnnotations/net/minecraft/text/annotations.xml index 374328686..7d6201e6d 100644 --- a/externalAnnotations/net/minecraft/text/annotations.xml +++ b/externalAnnotations/net/minecraft/text/annotations.xml @@ -41,6 +41,12 @@ + + + + + + diff --git a/src/main/kotlin/platform/mcp/mappings/HardcodedYarnToMojmap.kt b/src/main/kotlin/platform/mcp/mappings/HardcodedYarnToMojmap.kt index 7dfd4ea7c..246dbd92a 100644 --- a/src/main/kotlin/platform/mcp/mappings/HardcodedYarnToMojmap.kt +++ b/src/main/kotlin/platform/mcp/mappings/HardcodedYarnToMojmap.kt @@ -20,6 +20,7 @@ package com.demonwav.mcdev.platform.mcp.mappings +import com.demonwav.mcdev.util.MemberReference import com.google.common.collect.ImmutableBiMap /** @@ -30,9 +31,20 @@ object HardcodedYarnToMojmap { ImmutableBiMap.ofEntries( "net.minecraft.item.ItemStack" mapTo "net.minecraft.world.item.ItemStack", "net.minecraft.util.Formatting" mapTo "net.minecraft.ChatFormatting", + "net.minecraft.text.Text" mapTo "net.minecraft.network.chat.Component", ), ImmutableBiMap.ofEntries(), - ImmutableBiMap.ofEntries(), + ImmutableBiMap.ofEntries( + MemberReference( + owner = "net.minecraft.util.Text", + name = "stringifiedTranslatable", + descriptor = "(Ljava/lang/String;[Ljava/lang/Object;)Lnet/minecraft/text/MutableText;" + ) mapTo MemberReference( + owner = "net.minecraft.network.chat.Component", + name = "translatableEscape", + descriptor = "(Ljava/lang/String;[Ljava/lang/Object;)Lnet/minecraft/network/chat/MutableComponent;" + ) + ), hashMapOf(), false, ) diff --git a/src/main/kotlin/translations/TranslationConstants.kt b/src/main/kotlin/translations/TranslationConstants.kt index 440bb785e..b5e4ea536 100644 --- a/src/main/kotlin/translations/TranslationConstants.kt +++ b/src/main/kotlin/translations/TranslationConstants.kt @@ -25,6 +25,7 @@ object TranslationConstants { const val TRANSLATABLE_ANNOTATION = "com.demonwav.mcdev.annotations.Translatable" const val REQUIRED = "required" const val FOLD_METHOD = "foldMethod" + const val ALLOW_ARBITRARY_ARGS = "allowArbitraryArgs" const val PREFIX = "prefix" const val SUFFIX = "suffix" } diff --git a/src/main/kotlin/translations/identification/TranslationIdentifier.kt b/src/main/kotlin/translations/identification/TranslationIdentifier.kt index 52be6f702..d80756023 100644 --- a/src/main/kotlin/translations/identification/TranslationIdentifier.kt +++ b/src/main/kotlin/translations/identification/TranslationIdentifier.kt @@ -20,18 +20,23 @@ package com.demonwav.mcdev.translations.identification +import com.demonwav.mcdev.platform.mcp.mappings.getMappedClass +import com.demonwav.mcdev.platform.mcp.mappings.getMappedMethod import com.demonwav.mcdev.translations.TranslationConstants import com.demonwav.mcdev.translations.identification.TranslationInstance.Companion.FormattingError import com.demonwav.mcdev.translations.index.TranslationIndex import com.demonwav.mcdev.translations.index.merge import com.demonwav.mcdev.util.constantStringValue import com.demonwav.mcdev.util.constantValue +import com.demonwav.mcdev.util.descriptor import com.demonwav.mcdev.util.extractVarArgs +import com.demonwav.mcdev.util.findModule import com.demonwav.mcdev.util.referencedMethod import com.intellij.codeInsight.AnnotationUtil import com.intellij.codeInspection.dataFlow.CommonDataflow import com.intellij.openapi.project.Project import com.intellij.psi.CommonClassNames +import com.intellij.psi.JavaPsiFacade import com.intellij.psi.PsiCall import com.intellij.psi.PsiCallExpression import com.intellij.psi.PsiElement @@ -79,6 +84,12 @@ abstract class TranslationIdentifier { val required = translatableAnnotation.findAttributeValue(TranslationConstants.REQUIRED)?.constantValue as? Boolean ?: true + val isPreEscapeException = + method.containingClass?.qualifiedName?.startsWith("net.minecraft.") == true && + isPreEscapeMcVersion(project, element) + val allowArbitraryArgs = isPreEscapeException || translatableAnnotation.findAttributeValue( + TranslationConstants.ALLOW_ARBITRARY_ARGS + )?.constantValue as? Boolean ?: false val translationKey = CommonDataflow.computeValue(element) as? String ?: return null @@ -90,7 +101,8 @@ abstract class TranslationIdentifier { referenceElement, TranslationInstance.Key(prefix, translationKey, suffix), null, - required + required, + allowArbitraryArgs, ) val foldMethod = @@ -126,6 +138,7 @@ abstract class TranslationIdentifier { TranslationInstance.Key(prefix, translationKey, suffix), formatted, required, + allowArbitraryArgs, if (superfluousParams >= 0) FormattingError.SUPERFLUOUS else null, superfluousParams, ) @@ -137,6 +150,7 @@ abstract class TranslationIdentifier { TranslationInstance.Key(prefix, translationKey, suffix), translation, required, + allowArbitraryArgs, FormattingError.MISSING, ) } @@ -147,6 +161,7 @@ abstract class TranslationIdentifier { val paramCount = STRING_FORMATTING_PATTERN.findAll(format).count() val varargs = call.extractVarArgs(method.parameterList.parametersCount - 1, true, true) + ?: return null val varargStart = if (varargs.size > paramCount) { method.parameterList.parametersCount - 1 + paramCount } else { @@ -162,6 +177,21 @@ abstract class TranslationIdentifier { } } + private fun isPreEscapeMcVersion(project: Project, contextElement: PsiElement): Boolean { + val module = contextElement.findModule() ?: return false + val componentClassName = module.getMappedClass("net.minecraft.network.chat.Component") + val componentClass = JavaPsiFacade.getInstance(project) + .findClass(componentClassName, contextElement.resolveScope) ?: return false + val translatableEscapeName = module.getMappedMethod( + "net.minecraft.network.chat.Component", + "translatableEscape", + "(Ljava/lang/String;[Ljava/lang/Object;)Lnet/minecraft/network/chat/Component;" + ) + return componentClass.findMethodsByName(translatableEscapeName, false).any { method -> + method.descriptor?.startsWith("(Ljava/lang/String;[Ljava/lang/Object;)") == true + } + } + private val NUMBER_FORMATTING_PATTERN = Regex("%(\\d+\\$)?[\\d.]*[df]") private val STRING_FORMATTING_PATTERN = Regex("[^%]?%(?:\\d+\\$)?s") } diff --git a/src/main/kotlin/translations/identification/TranslationInstance.kt b/src/main/kotlin/translations/identification/TranslationInstance.kt index 2e8774f2e..e72e2c650 100644 --- a/src/main/kotlin/translations/identification/TranslationInstance.kt +++ b/src/main/kotlin/translations/identification/TranslationInstance.kt @@ -29,6 +29,7 @@ data class TranslationInstance( val key: Key, val text: String?, val required: Boolean, + val allowArbitraryArgs: Boolean, val formattingError: FormattingError? = null, val superfluousVarargStart: Int = -1, ) { diff --git a/src/main/kotlin/translations/inspections/WrongTypeInTranslationArgsInspection.kt b/src/main/kotlin/translations/inspections/WrongTypeInTranslationArgsInspection.kt new file mode 100644 index 000000000..58b969a7b --- /dev/null +++ b/src/main/kotlin/translations/inspections/WrongTypeInTranslationArgsInspection.kt @@ -0,0 +1,145 @@ +/* + * Minecraft Development for IntelliJ + * + * https://mcdev.io/ + * + * Copyright (C) 2023 minecraft-dev + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation, version 3.0 only. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package com.demonwav.mcdev.translations.inspections + +import com.demonwav.mcdev.platform.mcp.mappings.getMappedClass +import com.demonwav.mcdev.platform.mcp.mappings.getMappedMethod +import com.demonwav.mcdev.translations.identification.TranslationInstance +import com.demonwav.mcdev.util.findModule +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.LocalQuickFixOnPsiElement +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.project.Project +import com.intellij.psi.CommonClassNames +import com.intellij.psi.JavaElementVisitor +import com.intellij.psi.JavaPsiFacade +import com.intellij.psi.PsiCall +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.PsiEllipsisType +import com.intellij.psi.PsiFile +import com.intellij.psi.PsiLiteralExpression +import com.intellij.psi.PsiManager +import com.intellij.psi.PsiMethodCallExpression +import com.intellij.psi.PsiReferenceExpression +import com.intellij.psi.PsiType +import com.siyeh.ig.psiutils.CommentTracker +import com.siyeh.ig.psiutils.MethodCallUtils + +class WrongTypeInTranslationArgsInspection : TranslationInspection() { + override fun getStaticDescription() = "Detect wrong argument types in translation arguments" + + override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = Visitor(holder) + + private class Visitor(private val holder: ProblemsHolder) : JavaElementVisitor() { + override fun visitReferenceExpression(expression: PsiReferenceExpression) { + doCheck(expression) + } + + override fun visitLiteralExpression(expression: PsiLiteralExpression) { + doCheck(expression) + } + + private fun doCheck(element: PsiElement) { + val result = TranslationInstance.find(element) + if (result == null || result.foldingElement !is PsiCall || result.allowArbitraryArgs) { + return + } + + val args = result.foldingElement.argumentList ?: return + + if (!MethodCallUtils.isVarArgCall(result.foldingElement)) { + return + } + + val resolvedMethod = result.foldingElement.resolveMethod() ?: return + if ((resolvedMethod.parameterList.parameters.lastOrNull()?.type as? PsiEllipsisType) + ?.componentType?.equalsToText(CommonClassNames.JAVA_LANG_OBJECT) != true + ) { + return + } + val module = element.findModule() ?: return + val componentName = module.getMappedClass("net.minecraft.network.chat.Component") + val translatableName = module.getMappedMethod( + "net.minecraft.network.chat.Component", + "translatable", + "(Ljava/lang/String;[Ljava/lang/Object;)Lnet/minecraft/network/chat/MutableComponent;" + ) + val isComponentTranslatable = resolvedMethod.name == translatableName && + resolvedMethod.containingClass?.qualifiedName == componentName + + val booleanType = + PsiType.getTypeByName(CommonClassNames.JAVA_LANG_BOOLEAN, holder.project, element.resolveScope) + val numberType = + PsiType.getTypeByName(CommonClassNames.JAVA_LANG_NUMBER, holder.project, element.resolveScope) + val stringType = PsiType.getJavaLangString(PsiManager.getInstance(holder.project), element.resolveScope) + val componentType = PsiType.getTypeByName(componentName, holder.project, element.resolveScope) + for (arg in args.expressions.drop(resolvedMethod.parameterList.parametersCount - 1)) { + val type = arg.type ?: continue + if (!booleanType.isAssignableFrom(type) && + !numberType.isAssignableFrom(type) && + !stringType.isAssignableFrom(type) && + !componentType.isAssignableFrom(type) + ) { + var fixes = arrayOf(WrapWithStringValueOfFix(arg)) + if (isComponentTranslatable && result.foldingElement is PsiMethodCallExpression) { + val referenceName = result.foldingElement.methodExpression.referenceNameElement + if (referenceName != null) { + fixes = arrayOf(ReplaceWithTranslatableEscapedFix(referenceName)) + fixes + } + } + holder.registerProblem( + arg, + "Translation argument is not a 'String', 'Number', 'Boolean' or 'Component'", + *fixes + ) + } + } + } + } + + private class ReplaceWithTranslatableEscapedFix( + referenceName: PsiElement + ) : LocalQuickFixOnPsiElement(referenceName) { + override fun getFamilyName() = "Replace with 'Component.translatableEscaped'" + override fun getText() = "Replace with 'Component.translatableEscaped'" + + override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) { + val module = startElement.findModule() ?: return + val newMethodName = module.getMappedMethod( + "net.minecraft.network.chat.Component", + "translatableEscape", + "(Ljava/lang/String;[Ljava/lang/Object;)Lnet/minecraft/network/chat/MutableComponent;" + ) + startElement.replace(JavaPsiFacade.getElementFactory(project).createIdentifier(newMethodName)) + } + } + + private class WrapWithStringValueOfFix(element: PsiElement) : LocalQuickFixOnPsiElement(element) { + override fun getFamilyName() = "Wrap with 'String.valueOf()'" + override fun getText() = "Wrap with 'String.valueOf()'" + + override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) { + val ct = CommentTracker() + ct.replace(startElement, "String.valueOf(${ct.text(startElement)})") + } + } +} diff --git a/src/main/kotlin/util/call-utils.kt b/src/main/kotlin/util/call-utils.kt index c5ec67e29..893e11860 100644 --- a/src/main/kotlin/util/call-utils.kt +++ b/src/main/kotlin/util/call-utils.kt @@ -27,7 +27,6 @@ import com.intellij.psi.PsiMethodCallExpression import com.intellij.psi.PsiNewExpression import com.intellij.psi.PsiSubstitutor import com.intellij.psi.PsiType -import com.intellij.psi.PsiTypeCastExpression val PsiCall.referencedMethod: PsiMethod? get() = when (this) { @@ -36,7 +35,7 @@ val PsiCall.referencedMethod: PsiMethod? else -> null } -fun PsiCall.extractVarArgs(index: Int, allowReferences: Boolean, allowTranslations: Boolean): Array { +fun PsiCall.extractVarArgs(index: Int, allowReferences: Boolean, allowTranslations: Boolean): Array? { val method = this.referencedMethod val args = this.argumentList?.expressions ?: return emptyArray() if (method == null || args.size < (index + 1)) { @@ -56,24 +55,18 @@ private fun extractVarArgs( elements: List, allowReferences: Boolean, allowTranslations: Boolean, -): Array { - tailrec fun resolveReference(expression: PsiExpression): Array { - if (expression is PsiTypeCastExpression && expression.operand != null) { - return resolveReference(expression.operand!!) - } - return arrayOf(expression.evaluate(allowTranslations, allowReferences)) - } - +): Array? { return if (elements[0].type == type) { - // We're dealing with an array initializer, let's analyse it! val initializer = elements[0] if (initializer is PsiNewExpression && initializer.arrayInitializer != null) { + // We're dealing with an array initializer, let's analyse it! initializer.arrayInitializer!!.initializers .asSequence() .map { it.evaluate(allowReferences, allowTranslations) } .toTypedArray() } else { - resolveReference(initializer) + // We're dealing with a more complex expression that results in an array, give up + return null } } else { elements.asSequence().map { it.evaluate(allowReferences, allowTranslations) }.toTypedArray() diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 482aabcbe..a51407441 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -510,6 +510,13 @@ level="WARNING" hasStaticDescription="true" implementationClass="com.demonwav.mcdev.translations.inspections.SuperfluousFormatInspection"/> +