Repository: hannah-sten/texify-idea
Issue: \\ref does not work correctly on custom command with optional parameter

## TeXiFy IDEA version
Latest Alpha

This is a followup to https://github.com/Hannah-Sten/TeXiFy-IDEA/issues/1575

#### Minimal example to reproduce the problem

In the example below the first \\ref will be resolved correctly while the second will show an "unresolved reference" inspection. This is because of the missing optional parameter, which is perfectly valid.

```latex
\\newcommand{\\includenamedimage}[3][]{
    \\begin{figure}
        \\centering
        \\includegraphics[width=#1\\textwidth]{#2}
        \\caption{#3}
        \\label{fig:#2}
    \\end{figure}
    }

    \\includenamedimage[0.5]{test1.png}{fancy caption}
    \\includenamedimage{test2.png}{fancy caption}

    \\ref{fig:test1.png}
    \\ref{fig:test2.png}
```




Changed files:
src/nl/hannahsten/texifyidea/psi/LatexParameterTextUtil.kt
test/nl/hannahsten/texifyidea/inspections/latex/LatexUnresolvedReferenceInspectionTest.kt
src/nl/hannahsten/texifyidea/util/Magic.kt
src/nl/hannahsten/texifyidea/reference/LatexLabelParameterReference.kt
src/nl/hannahsten/texifyidea/util/Labels.kt
test/nl/hannahsten/texifyidea/inspections/latex/LatexDuplicateLabelInspectionTest.kt
src/nl/hannahsten/texifyidea/lang/CommandManager.kt

diff --git a/src/nl/hannahsten/texifyidea/lang/CommandManager.kt b/src/nl/hannahsten/texifyidea/lang/CommandManager.kt
index 823291a94..69d580190 100644
--- a/src/nl/hannahsten/texifyidea/lang/CommandManager.kt
+++ b/src/nl/hannahsten/texifyidea/lang/CommandManager.kt
@@ -283,12 +283,15 @@ object CommandManager : Iterable<String?>, Serializable {
                 val definedCommand = commandDefinition.requiredParameter(0) ?: return@forEach
                 if (definedCommand.isBlank()) return@forEach
 
+                val isFirstParameterOptional = commandDefinition.optionalParameters.size > 1
+
                 val parameterCommands = commandDefinition.requiredParameters().getOrNull(1)
                     ?.requiredParamContentList
                     ?.flatMap { it.childrenOfType(LatexCommands::class) }
                     ?.asSequence()
                     ?.filterNotNull()
 
+                // Positions of label parameters in the custom commands (starting from 0)
                 val positions = parameterCommands
                     ?.filter { it.name in Magic.Command.labelDefinitionsWithoutCustomCommands }
                     ?.mapNotNull { it.requiredParameter(0) }
@@ -301,6 +304,8 @@ object CommandManager : Iterable<String?>, Serializable {
                     ?.map(Character::getNumericValue)
                     // LaTeX starts from 1, we from 0 (consistent with how we count required parameters)
                     ?.map { it - 1 }
+                    // For the moment we only consider required parameters and ignore the optional one
+                    ?.map { if (isFirstParameterOptional) it - 1 else it }
                     ?.filter { it >= 0 }
                     ?.toList() ?: return@forEach
                 if (positions.isEmpty()) return@forEach
diff --git a/src/nl/hannahsten/texifyidea/psi/LatexParameterTextUtil.kt b/src/nl/hannahsten/texifyidea/psi/LatexParameterTextUtil.kt
index 768247e2c..b39e2399c 100644
--- a/src/nl/hannahsten/texifyidea/psi/LatexParameterTextUtil.kt
+++ b/src/nl/hannahsten/texifyidea/psi/LatexParameterTextUtil.kt
@@ -50,11 +50,13 @@ fun getReference(element: LatexParameterText): PsiReference? {
 fun getNameIdentifier(element: LatexParameterText): PsiElement? {
     // Because we do not want to trigger the NonAsciiCharactersInspection when the LatexParameterText is not an identifier
     // (think non-ASCII characters in a \\section command), we return null here when the element is not an identifier
+    // It is important not to return null for any identifier, otherwise exceptions like "Throwable: null byMemberInplaceRenamer" may occur
     val name = element.firstParentOfType(LatexCommands::class)?.name
     if (!Magic.Command.labelReferenceWithoutCustomCommands.contains(name) &&
         !Magic.Command.labelDefinitionsWithoutCustomCommands.contains(name) &&
         !Magic.Command.bibliographyReference.contains(name) &&
-        element.firstParentOfType(LatexEndCommand::class) == null) {
+        element.firstParentOfType(LatexEndCommand::class) == null &&
+        element.firstParentOfType(LatexBeginCommand::class) == null) {
         return null
     }
     return element
diff --git a/src/nl/hannahsten/texifyidea/reference/LatexLabelParameterReference.kt b/src/nl/hannahsten/texifyidea/reference/LatexLabelParameterReference.kt
index 5c931fc86..9590d2faa 100644
--- a/src/nl/hannahsten/texifyidea/reference/LatexLabelParameterReference.kt
+++ b/src/nl/hannahsten/texifyidea/reference/LatexLabelParameterReference.kt
@@ -3,9 +3,9 @@ package nl.hannahsten.texifyidea.reference
 import com.intellij.psi.*
 import com.intellij.util.containers.toArray
 import nl.hannahsten.texifyidea.psi.LatexParameterText
+import nl.hannahsten.texifyidea.util.extractLabelElement
 import nl.hannahsten.texifyidea.util.extractLabelName
 import nl.hannahsten.texifyidea.util.findLatexLabelPsiElementsInFileSetAsSequence
-import nl.hannahsten.texifyidea.util.firstChildOfType
 
 /**
  * The difference with [LatexLabelReference] is that this reference works on parameter text, i.e. the actual label parameters.
@@ -39,8 +39,7 @@ class LatexLabelParameterReference(element: LatexParameterText) : PsiReferenceBa
                     // We cannot just resolve to the label command itself, because for Find Usages IJ will get the name of the element
                     // under the cursor and use the words scanner to look for it (and then check if the elements found are references to the element under the cursor)
                     // but only the label text itself will have the correct name for that.
-                    val labelText = it.firstChildOfType(LatexParameterText::class) ?: return@mapNotNull null
-                    PsiElementResolveResult(labelText)
+                    PsiElementResolveResult(it.extractLabelElement() ?: return@mapNotNull null)
                 }
                 .toList()
                 .toArray(emptyArray())
diff --git a/src/nl/hannahsten/texifyidea/util/Labels.kt b/src/nl/hannahsten/texifyidea/util/Labels.kt
index 47c609a12..157484187 100644
--- a/src/nl/hannahsten/texifyidea/util/Labels.kt
+++ b/src/nl/hannahsten/texifyidea/util/Labels.kt
@@ -7,9 +7,7 @@ import nl.hannahsten.texifyidea.index.BibtexEntryIndex
 import nl.hannahsten.texifyidea.index.LatexCommandsIndex
 import nl.hannahsten.texifyidea.index.LatexParameterLabeledEnvironmentsIndex
 import nl.hannahsten.texifyidea.lang.CommandManager
-import nl.hannahsten.texifyidea.psi.BibtexEntry
-import nl.hannahsten.texifyidea.psi.LatexCommands
-import nl.hannahsten.texifyidea.psi.LatexEnvironment
+import nl.hannahsten.texifyidea.psi.*
 import nl.hannahsten.texifyidea.util.files.commandsInFile
 import nl.hannahsten.texifyidea.util.files.commandsInFileSet
 
@@ -164,14 +162,31 @@ fun PsiElement.extractLabelName(): String {
             val info = CommandManager.labelAliasesInfo.getOrDefault(name, null)
             val position = info?.positions?.firstOrNull() ?: 0
             val prefix = info?.prefix ?: ""
-            // Optional and required parameters both count as parameters
-            prefix + this.parameterList.getOrNull(position)?.text?.drop(1)?.dropLast(1)
+            // Skip optional parameters for now (also below and in
+            prefix + this.requiredParameter(position)
         }
         is LatexEnvironment -> this.label ?: ""
         else -> text
     }
 }
 
+/**
+ * Extracts the label element (so the element that should be resolved to) from the PsiElement given that the PsiElement represents a label.
+ */
+fun PsiElement.extractLabelElement(): PsiElement? {
+    return when (this) {
+        is BibtexEntry -> firstChildOfType(BibtexId::class)
+        is LatexCommands -> {
+            // For now just take the first label name (may be multiple for user defined commands)
+            val info = CommandManager.labelAliasesInfo.getOrDefault(name, null)
+            val position = info?.positions?.firstOrNull() ?: 0
+            // Skip optional parameters for now
+            this.parameterList.mapNotNull { it.requiredParam }.getOrNull(position)?.firstChildOfType(LatexParameterText::class)
+        }
+        else -> null
+    }
+}
+
 /**
  * Finds all section marker commands (as defined in [Magic.Command.sectionMarkers]) in the project.
  *
diff --git a/src/nl/hannahsten/texifyidea/util/Magic.kt b/src/nl/hannahsten/texifyidea/util/Magic.kt
index 7b5c14a32..528618ab7 100644
--- a/src/nl/hannahsten/texifyidea/util/Magic.kt
+++ b/src/nl/hannahsten/texifyidea/util/Magic.kt
@@ -585,7 +585,7 @@ object Magic {
          * All commands that at first glance look like \\if-esque commands, but that actually aren't.
          */
         @JvmField
-        val ignoredIfs = hashSetOf("\\\\newif", "\\\\iff", "\\\\ifthenelse", "\\\\iftoggle", "\\\\ifoot")
+        val ignoredIfs = hashSetOf("\\\\newif", "\\\\iff", "\\\\ifthenelse", "\\\\iftoggle", "\\\\ifoot", "\\\\ifcsvstrcmp")
 
         /**
          * List of all TeX style primitives.
diff --git a/test/nl/hannahsten/texifyidea/inspections/latex/LatexDuplicateLabelInspectionTest.kt b/test/nl/hannahsten/texifyidea/inspections/latex/LatexDuplicateLabelInspectionTest.kt
index 11502e413..54e01e440 100644
--- a/test/nl/hannahsten/texifyidea/inspections/latex/LatexDuplicateLabelInspectionTest.kt
+++ b/test/nl/hannahsten/texifyidea/inspections/latex/LatexDuplicateLabelInspectionTest.kt
@@ -2,6 +2,7 @@ package nl.hannahsten.texifyidea.inspections.latex
 
 import nl.hannahsten.texifyidea.file.LatexFileType
 import nl.hannahsten.texifyidea.inspections.TexifyInspectionTestBase
+import nl.hannahsten.texifyidea.lang.CommandManager
 
 class LatexDuplicateLabelInspectionTest : TexifyInspectionTestBase(LatexDuplicateLabelInspection()) {
     fun testWarning() {
@@ -11,4 +12,25 @@ class LatexDuplicateLabelInspectionTest : TexifyInspectionTestBase(LatexDuplicat
         """.trimIndent())
         myFixture.checkHighlighting()
     }
+
+    fun testFigureReferencedCustomCommandOptionalParameter() {
+        myFixture.configureByText(LatexFileType, """
+            \\newcommand{\\includenamedimage}[3][]{
+            \\begin{figure}
+                \\centering
+                \\includegraphics[width=#1\\textwidth]{#2}
+                \\caption{#3}
+                \\label{fig:#2}
+            \\end{figure}
+            }
+        
+            \\includenamedimage[0.5]{test.png}{fancy caption}
+            \\includenamedimage{test2.png}{fancy caption}
+        
+            some text~\\ref{fig:test.png} more text.
+            some text~\\ref{fig:test2.png} more text.
+        """.trimIndent())
+        CommandManager.updateAliases(setOf("\\\\label"), project)
+        myFixture.checkHighlighting()
+    }
 }
\\ No newline at end of file
diff --git a/test/nl/hannahsten/texifyidea/inspections/latex/LatexUnresolvedReferenceInspectionTest.kt b/test/nl/hannahsten/texifyidea/inspections/latex/LatexUnresolvedReferenceInspectionTest.kt
index 5c4c8dcab..73d57236e 100644
--- a/test/nl/hannahsten/texifyidea/inspections/latex/LatexUnresolvedReferenceInspectionTest.kt
+++ b/test/nl/hannahsten/texifyidea/inspections/latex/LatexUnresolvedReferenceInspectionTest.kt
@@ -59,6 +59,27 @@ class LatexUnresolvedReferenceInspectionTest : TexifyInspectionTestBase(LatexUnr
     //     myFixture.checkHighlighting()
     // }
 
+    fun testFigureReferencedCustomCommandOptionalParameter() {
+        myFixture.configureByText(LatexFileType, """
+            \\newcommand{\\includenamedimage}[3][]{
+            \\begin{figure}
+                \\centering
+                \\includegraphics[width=#1\\textwidth]{#2}
+                \\caption{#3}
+                \\label{fig:#2}
+            \\end{figure}
+            }
+        
+            \\includenamedimage[0.5]{test.png}{fancy caption}
+            \\includenamedimage{test2.png}{fancy caption}
+        
+            some text~\\ref{fig:test.png} more text.
+            some text~\\ref{fig:test2.png} more text.
+        """.trimIndent())
+        CommandManager.updateAliases(setOf("\\\\label"), project)
+        myFixture.checkHighlighting()
+    }
+
     fun testComma() {
         myFixture.configureByText(LatexFileType, """\\input{name,with,.tex}""")
         myFixture.checkHighlighting()