Skip to content

Commit

Permalink
Fix/llamas expressions fixes (#2324)
Browse files Browse the repository at this point in the history 
* MixinExtras: Fix completion confidence.

* Expressions: Autocomplete `method`s and `field`s using flows not instructions.

* Expressions: Autocomplete `method`s for method references.

* Expressions: A class constant is an expression.

* Expressions: Show instantiation desc in autocomplete.

* Expressions: Make completions always unique to stop them being filtered.
We filter duplicates ourselves.

* Expressions: Overhaul array completions.
Make the preview text more representative of the actual completion and support array literals which were previously missing.

* Expressions: Fix super call completions.

* Expressions: Confidently suggest completions after `::`.

* Expressions: Add intention action to define unresolved identifiers.

* Expressions: Fix `@Local`s not properly handling compound insns.
Also adapt to related MixinExtras changes.

* Refactor: Add `project` as parameter to `MEExpressionCompletionUtil.addDefinition`
  • Loading branch information
@LlamaLad7
LlamaLad7 authored Jul 9, 2024
1 parent f652895 commit eeb4dd5
Show file tree
Hide file tree
Showing 9 changed files with 216 additions and 81 deletions.
2 changes: 1 addition & 1 deletion build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ dependencies {
implementation(files(Jvm.current().toolsJar))

// TODO: temporary waiting for a release
fun mixinExtras(variant: String) = "com.github.LlamaLad7.MixinExtras:mixinextras-$variant:4d2e01e"
fun mixinExtras(variant: String) = "com.github.LlamaLad7.MixinExtras:mixinextras-$variant:2ad48e8"

implementation(mixinExtras("expressions"))
testLibs(mixinExtras("common"))
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/platform/mixin/completion/MixinCompletionConfidence.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class MixinCompletionConfidence : CompletionConfidence() {
PsiJavaPatterns.psiAnnotation().qName(
StandardPatterns.or(
StandardPatterns.string().startsWith(MixinConstants.PACKAGE),
StandardPatterns.string().startsWith(MixinConstants.MixinExtras.PACKAGE),
StandardPatterns.string()
.oneOf(MixinAnnotationHandler.getBuiltinHandlers().map { it.first }.toList()),
)
Expand Down
51 changes: 51 additions & 0 deletions src/main/kotlin/platform/mixin/expression/MEExpressionAnnotator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,24 @@ import com.demonwav.mcdev.platform.mixin.expression.gen.psi.METype
import com.demonwav.mcdev.platform.mixin.expression.psi.METypeUtil
import com.demonwav.mcdev.platform.mixin.util.MixinConstants
import com.demonwav.mcdev.util.findMultiInjectionHost
import com.intellij.codeInsight.AutoPopupController
import com.intellij.codeInspection.InspectionManager
import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement
import com.intellij.codeInspection.ProblemHighlightType
import com.intellij.codeInspection.RemoveAnnotationQuickFix
import com.intellij.lang.annotation.AnnotationBuilder
import com.intellij.lang.annotation.AnnotationHolder
import com.intellij.lang.annotation.Annotator
import com.intellij.lang.annotation.HighlightSeverity
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.editor.colors.TextAttributesKey
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiAnnotation
import com.intellij.psi.PsiDocumentManager
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiFile
import com.intellij.psi.PsiModifierListOwner
import com.intellij.psi.impl.source.tree.injected.InjectedLanguageEditorUtil
import com.intellij.psi.search.searches.ReferencesSearch
import com.intellij.psi.util.TypeConversionUtil
import com.intellij.psi.util.parentOfType
Expand Down Expand Up @@ -275,6 +283,7 @@ class MEExpressionAnnotator : Annotator {
)
.range(type)
.highlightType(ProblemHighlightType.LIKE_UNKNOWN_SYMBOL)
.withDefinitionFix(type)
.create()
} else {
holder.newSilentAnnotation(HighlightSeverity.TEXT_ATTRIBUTES)
Expand Down Expand Up @@ -306,6 +315,7 @@ class MEExpressionAnnotator : Annotator {
)
.range(variable)
.highlightType(ProblemHighlightType.LIKE_UNKNOWN_SYMBOL)
.withDefinitionFix(variable)
.create()
} else {
holder.newSilentAnnotation(HighlightSeverity.TEXT_ATTRIBUTES)
Expand All @@ -314,4 +324,45 @@ class MEExpressionAnnotator : Annotator {
.create()
}
}

private fun AnnotationBuilder.withDefinitionFix(name: MEName) =
withFix(AddDefinitionInspection(name))

private class AddDefinitionInspection(name: MEName) : LocalQuickFixAndIntentionActionOnPsiElement(name) {
private val id = name.text

override fun getFamilyName(): String = "Add @Definition"

override fun getText(): String = "$familyName(id = \"$id\")"

override fun invoke(
project: Project,
file: PsiFile,
editor: Editor?,
startElement: PsiElement,
endElement: PsiElement
) {
if (editor == null) {
MEExpressionCompletionUtil.addDefinition(
project,
startElement,
id,
""
)
return
}
val annotation = MEExpressionCompletionUtil.addDefinition(
project,
startElement,
id,
"dummy"
) ?: return
val dummy = annotation.findAttribute("dummy") as? PsiElement ?: return
val hostEditor = InjectedLanguageEditorUtil.getTopLevelEditor(editor)
hostEditor.caretModel.moveToOffset(dummy.textOffset)
PsiDocumentManager.getInstance(project).doPostponedOperationsAndUnblockDocument(hostEditor.document)
hostEditor.document.replaceString(dummy.textRange.startOffset, dummy.textRange.endOffset, "")
AutoPopupController.getInstance(project).autoPopupMemberLookup(hostEditor, null)
}
}
}
137 changes: 76 additions & 61 deletions src/main/kotlin/platform/mixin/expression/MEExpressionCompletionUtil.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,11 +691,12 @@ object MEExpressionCompletionUtil {
canCompleteExprs: Boolean,
canCompleteTypes: Boolean
): List<EliminableLookup> {
val flow = flows[insn]
when (insn.insn) {
is LdcInsnNode -> {
when (val cst = insn.insn.cst) {
is Type -> {
if (canCompleteTypes && cst.isAccessibleFrom(mixinClass)) {
if (canCompleteExprs && cst.isAccessibleFrom(mixinClass)) {
return listOf(
createTypeLookup(cst)
.withTailText(".class")
Expand Down Expand Up @@ -727,7 +728,7 @@ object MEExpressionCompletionUtil {
is FieldInsnNode -> {
if (canCompleteExprs) {
val definitionValue = "field = \"L${insn.insn.owner};${insn.insn.name}:${insn.insn.desc}\""
var lookup = LookupElementBuilder.create(insn.insn.name.toValidIdentifier())
var lookup = createUniqueLookup(insn.insn.name.toValidIdentifier())
.withIcon(PlatformIcons.FIELD_ICON)
.withPresentableText(insn.insn.owner.substringAfterLast('/') + "." + insn.insn.name)
.withTypeText(Type.getType(insn.insn.desc).presentableName())
Expand All @@ -743,12 +744,10 @@ object MEExpressionCompletionUtil {
is MethodInsnNode -> {
if (canCompleteExprs) {
val definitionValue = "method = \"L${insn.insn.owner};${insn.insn.name}${insn.insn.desc}\""
var lookup = LookupElementBuilder.create(insn.insn.name.toValidIdentifier())
var lookup = createUniqueLookup(insn.insn.name.toValidIdentifier())
.withIcon(PlatformIcons.METHOD_ICON)
.withPresentableText(insn.insn.owner.substringAfterLast('/') + "." + insn.insn.name)
.withTailText(
"(" + Type.getArgumentTypes(insn.insn.desc).joinToString { it.presentableName() } + ")"
)
.withDescTailText(insn.insn.desc)
.withTypeText(Type.getReturnType(insn.insn.desc).presentableName())
.withDefinitionAndFold(insn.insn.name.toValidIdentifier(), "method", definitionValue)
if (insn.insn.opcode == Opcodes.INVOKESTATIC) {
Expand All @@ -766,25 +765,21 @@ object MEExpressionCompletionUtil {
val lookup = createTypeLookup(type)
when (insn.insn.opcode) {
Opcodes.ANEWARRAY -> {
return listOf(
lookup.withTail(
BracketsTailType(
1,
flows[insn]?.hasDecoration(FlowDecorations.ARRAY_CREATION_INFO) == true,
)
)
.createEliminable("new [${insn.insn.desc}")
)
val arrayType = Type.getType('[' + Type.getObjectType(insn.insn.desc).descriptor)
return createNewArrayCompletion(flow, arrayType)
}
Opcodes.NEW -> {
val initCall = flows[insn]
val initCall = flow
?.getDecoration<InstantiationInfo>(FlowDecorations.INSTANTIATION_INFO)
?.initCall
?.virtualInsnOrNull
?.insn as MethodInsnNode?
?.insn as? MethodInsnNode
?: return emptyList()
return listOf(
lookup.withTail(ParenthesesTailType(initCall?.desc?.startsWith("()") == false))
.createEliminable("new ${insn.insn.desc}${initCall?.desc}")
lookup
.withDescTailText(initCall.desc)
.withTail(ParenthesesTailType(!initCall.desc.startsWith("()")))
.createEliminable("new ${insn.insn.desc}${initCall.desc}")
)
}
else -> return listOf(lookup.createEliminable("type ${insn.insn.desc}"))
Expand All @@ -794,52 +789,35 @@ object MEExpressionCompletionUtil {
is IntInsnNode -> {
if (insn.insn.opcode == Opcodes.NEWARRAY) {
if (canCompleteTypes) {
val type = when (insn.insn.operand) {
Opcodes.T_BOOLEAN -> "boolean"
Opcodes.T_CHAR -> "char"
Opcodes.T_FLOAT -> "float"
Opcodes.T_DOUBLE -> "double"
Opcodes.T_BYTE -> "byte"
Opcodes.T_SHORT -> "short"
Opcodes.T_INT -> "int"
Opcodes.T_LONG -> "long"
else -> "unknown" // wtf?
}
return listOf(
LookupElementBuilder.create(type)
.withIcon(PlatformIcons.CLASS_ICON)
.withTail(
BracketsTailType(
1,
flows[insn]?.hasDecoration(FlowDecorations.ARRAY_CREATION_INFO) == true,
)
)
.createEliminable("new $type[]")
val arrayType = Type.getType(
when (insn.insn.operand) {
Opcodes.T_BOOLEAN -> "[B"
Opcodes.T_CHAR -> "[C"
Opcodes.T_FLOAT -> "[F"
Opcodes.T_DOUBLE -> "[D"
Opcodes.T_BYTE -> "[B"
Opcodes.T_SHORT -> "[S"
Opcodes.T_INT -> "[I"
Opcodes.T_LONG -> "[J"
else -> "[Lnull;" // wtf?
}
)
return createNewArrayCompletion(flow, arrayType)
}
}
}
is MultiANewArrayInsnNode -> {
if (canCompleteTypes) {
val type = Type.getType(insn.insn.desc)
return listOf(
createTypeLookup(type.elementType)
.withTail(
BracketsTailType(
type.dimensions,
flows[insn]?.hasDecoration(FlowDecorations.ARRAY_CREATION_INFO) == true
)
)
.createEliminable("new ${insn.insn.desc}")
)
val arrayType = Type.getType(insn.insn.desc)
return createNewArrayCompletion(flow, arrayType)
}
}
is InsnNode -> {
when (insn.insn.opcode) {
Opcodes.ARRAYLENGTH -> {
if (canCompleteExprs) {
return listOf(
LookupElementBuilder.create("length")
createUniqueLookup("length")
.withIcon(PlatformIcons.FIELD_ICON)
.withTypeText("int")
.createEliminable("arraylength")
Expand Down Expand Up @@ -868,7 +846,7 @@ object MEExpressionCompletionUtil {
)
} else {
return listOf(
LookupElementBuilder.create(handle.name.toValidIdentifier())
createUniqueLookup(handle.name.toValidIdentifier())
.withIcon(PlatformIcons.METHOD_ICON)
.withPresentableText(handle.owner.substringAfterLast('/') + "." + handle.name)
.withTypeText(Type.getReturnType(handle.desc).presentableName())
Expand Down Expand Up @@ -935,7 +913,7 @@ object MEExpressionCompletionUtil {
private fun createTypeLookup(type: Type): LookupElementBuilder {
val definitionId = type.typeNameToInsert()

val lookupElement = LookupElementBuilder.create(definitionId)
val lookupElement = createUniqueLookup(definitionId)
.withIcon(PlatformIcons.CLASS_ICON)
.withPresentableText(type.presentableName())

Expand All @@ -946,6 +924,22 @@ object MEExpressionCompletionUtil {
}
}

private fun createNewArrayCompletion(flow: FlowValue?, arrayType: Type): List<EliminableLookup> {
val hasInitializer = flow?.hasDecoration(FlowDecorations.ARRAY_CREATION_INFO) == true
val initializerText = if (hasInitializer) "{}" else ""
return listOf(
createTypeLookup(arrayType.elementType)
.withTailText("[]".repeat(arrayType.dimensions) + initializerText)
.withTail(
BracketsTailType(
arrayType.dimensions,
hasInitializer,
)
)
.createEliminable("new ${arrayType.descriptor}$initializerText")
)
}

private fun createLocalVariableLookups(
project: Project,
targetClass: ClassNode,
Expand Down Expand Up @@ -997,7 +991,7 @@ object MEExpressionCompletionUtil {
val ordinal = localsOfMyType.indexOf(localVariable)
val isImplicit = localsOfMyType.size == 1
val localName = localVariable.name.toValidIdentifier()
LookupElementBuilder.create(localName)
createUniqueLookup(localName)
.withIcon(PlatformIcons.VARIABLE_ICON)
.withTypeText(localPsiType.presentableText)
.withLocalDefinition(
Expand All @@ -1020,14 +1014,19 @@ object MEExpressionCompletionUtil {
val localName = localType.typeNameToInsert().replace("[]", "Array") + (ordinal + 1)
val isImplicit = localTypes.count { it == localType } == 1
return listOf(
LookupElementBuilder.create(localName)
createUniqueLookup(localName)
.withIcon(PlatformIcons.VARIABLE_ICON)
.withTypeText(localType.presentableName())
.withLocalDefinition(localName, localType, ordinal, isArgsOnly, isImplicit, mixinClass)
.createEliminable("local $localName", if (isImplicit) -1 else 0)
)
}

private fun LookupElementBuilder.withDescTailText(desc: String) =
withTailText(
Type.getArgumentTypes(desc).joinToString(prefix = "(", postfix = ")") { it.presentableName() }
)

private fun LookupElement.withTail(tailType: TailType?) = object : TailTypeDecorator<LookupElement>(this) {
override fun computeTailType(context: InsertionContext?) = tailType
}
Expand Down Expand Up @@ -1179,6 +1178,10 @@ object MEExpressionCompletionUtil {

private fun addDefinition(context: InsertionContext, id: String, definitionValue: String): PsiAnnotation? {
val contextElement = context.file.findElementAt(context.startOffset) ?: return null
return addDefinition(context.project, contextElement, id, definitionValue)
}

fun addDefinition(project: Project, contextElement: PsiElement, id: String, definitionValue: String): PsiAnnotation? {
val injectionHost = contextElement.findMultiInjectionHost() ?: return null
val expressionAnnotation = injectionHost.parentOfType<PsiAnnotation>() ?: return null
if (!expressionAnnotation.hasQualifiedName(MixinConstants.MixinExtras.EXPRESSION)) {
Expand All @@ -1196,14 +1199,14 @@ object MEExpressionCompletionUtil {
}

// create and add the new @Definition annotation
var newAnnotation = JavaPsiFacade.getElementFactory(context.project).createAnnotationFromText(
var newAnnotation = JavaPsiFacade.getElementFactory(project).createAnnotationFromText(
"@${MixinConstants.MixinExtras.DEFINITION}(id = \"$id\", $definitionValue)",
modifierList,
)
var anchor = modifierList.annotations.lastOrNull { it.hasQualifiedName(MixinConstants.MixinExtras.DEFINITION) }
if (anchor == null) {
val definitionPosRelativeToExpression =
MinecraftProjectSettings.getInstance(context.project).definitionPosRelativeToExpression
MinecraftProjectSettings.getInstance(project).definitionPosRelativeToExpression
if (definitionPosRelativeToExpression == BeforeOrAfter.AFTER) {
anchor = expressionAnnotation
}
Expand All @@ -1212,11 +1215,11 @@ object MEExpressionCompletionUtil {

// add imports and reformat
newAnnotation =
JavaCodeStyleManager.getInstance(context.project).shortenClassReferences(newAnnotation) as PsiAnnotation
JavaCodeStyleManager.getInstance(context.project).optimizeImports(modifierList.containingFile)
JavaCodeStyleManager.getInstance(project).shortenClassReferences(newAnnotation) as PsiAnnotation
JavaCodeStyleManager.getInstance(project).optimizeImports(modifierList.containingFile)
val annotationIndex = modifierList.annotations.indexOf(newAnnotation)
val formattedModifierList =
CodeStyleManager.getInstance(context.project).reformat(modifierList) as PsiModifierList
CodeStyleManager.getInstance(project).reformat(modifierList) as PsiModifierList
return formattedModifierList.annotations.getOrNull(annotationIndex)
}

Expand Down Expand Up @@ -1271,13 +1274,25 @@ object MEExpressionCompletionUtil {
val fixedNewArrayExpr = factory.createExpression("new ?[?]") as MENewExpression
fixedNewArrayExpr.type!!.replace(type)
variants += fixedNewArrayExpr

val arrayLitExpr = factory.createExpression("new ?[]{?}") as MENewExpression
arrayLitExpr.type!!.replace(type)
variants += arrayLitExpr
}
}
is MESuperCallExpression -> {
// Might be missing its parentheses
val callExpr = factory.createExpression("super.?()") as MESuperCallExpression
expression.memberName?.let { callExpr.memberName!!.replace(it) }
variants += callExpr
}
}

return variants
}

private fun createUniqueLookup(text: String) = LookupElementBuilder.create(Any(), text)

private fun LookupElement.createEliminable(uniquenessKey: String, priority: Int = 0) =
EliminableLookup(uniquenessKey, this, priority)

Expand Down
Loading

0 comments on commit eeb4dd5

Please sign in to comment.