Fix false positive for duplicate command definition inspection in if/else

CHANGELOG.md

@@ -22,7 +22,7 @@
 * Inspections can now be suppressed for any single line, or block of text
 
 ### Fixed
-
+* Fix false positive for duplicate command definition inspection in if/else
 * Fix LaTeX files not showing up when choosing main file in run configuration
 * Fix various issues with the Grazie implementation, in particular default rules for Grazie Pro
 

src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspection.kt

@@ -7,10 +7,11 @@ import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.psi.PsiFile
 import nl.hannahsten.texifyidea.inspections.InsightGroup
 import nl.hannahsten.texifyidea.inspections.TexifyInspectionBase
-import nl.hannahsten.texifyidea.util.parser.definedCommandName
 import nl.hannahsten.texifyidea.util.files.definitions
 import nl.hannahsten.texifyidea.util.files.definitionsInFileSet
+import nl.hannahsten.texifyidea.util.isInConditionalBranch
 import nl.hannahsten.texifyidea.util.magic.CommandMagic
+import nl.hannahsten.texifyidea.util.parser.definedCommandName
 
 /**
  * Warns for commands that are defined twice in the same fileset.
@@ -32,13 +33,15 @@ open class LatexDuplicateDefinitionInspection : TexifyInspectionBase() {
         val defined = HashMultiset.create<String>()
         val definitions = file.definitionsInFileSet().filter { it.name in CommandMagic.regularStrictCommandDefinitions }
         for (command in definitions) {
+            if (isInConditionalBranch(command)) continue
             val name = command.definedCommandName() ?: continue
             defined.add(name)
         }
 
         // Go monkeys.
         file.definitions()
             .forEach {
+                if (isInConditionalBranch(it)) return@forEach
                 val definedCmd = it.definedCommandName() ?: return@forEach
                 if (defined.count(definedCmd) > 1) {
                     descriptors.add(

src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateLabelInspection.kt

@@ -6,18 +6,23 @@ import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiFile
-import com.intellij.psi.util.elementType
 import com.intellij.psi.util.startOffset
 import nl.hannahsten.texifyidea.inspections.InsightGroup
 import nl.hannahsten.texifyidea.inspections.TexifyInspectionBase
 import nl.hannahsten.texifyidea.lang.alias.CommandManager
 import nl.hannahsten.texifyidea.lang.magic.MagicCommentScope
-import nl.hannahsten.texifyidea.psi.*
+import nl.hannahsten.texifyidea.psi.LatexCommands
+import nl.hannahsten.texifyidea.psi.LatexEnvironment
+import nl.hannahsten.texifyidea.psi.LatexParameter
+import nl.hannahsten.texifyidea.psi.getEnvironmentName
+import nl.hannahsten.texifyidea.util.isInConditionalBranch
 import nl.hannahsten.texifyidea.util.labels.findBibitemCommands
 import nl.hannahsten.texifyidea.util.labels.findLatexLabelingElementsInFileSet
 import nl.hannahsten.texifyidea.util.magic.CommandMagic
 import nl.hannahsten.texifyidea.util.magic.EnvironmentMagic
-import nl.hannahsten.texifyidea.util.parser.*
+import nl.hannahsten.texifyidea.util.parser.isDefinitionOrRedefinition
+import nl.hannahsten.texifyidea.util.parser.parentOfType
+import nl.hannahsten.texifyidea.util.parser.requiredParameter
 import java.lang.Integer.max
 import java.util.*
 
@@ -125,7 +130,7 @@ open class LatexDuplicateLabelInspection : TexifyInspectionBase() {
             // When the label is defined in a command definition ignore it, because there could be more than one with #1 as parameter
             if (command.parentOfType(LatexCommands::class).isDefinitionOrRedefinition()) return@mapNotNull null
             // If the command is within an \if branch, ignore it because it will may appear in multiple branches of which only one will be present during compilation
-            if (isPreviousConditionalStart(command) && isNextConditionalEnd(command)) {
+            if (isInConditionalBranch(command)) {
                 return@mapNotNull null
             }
             command.getLabelDescriptor()
@@ -151,54 +156,4 @@ open class LatexDuplicateLabelInspection : TexifyInspectionBase() {
             isOntheFly
         )
     }
-
-    /**
-     * If the next relevant command is a \fi
-     */
-    private fun isNextConditionalEnd(current: PsiElement): Boolean {
-        return isEndConditional(nextConditionalCommand(current, searchBackwards = false) ?: return false)
-    }
-
-    /**
-     * If the previous relevant command is an \if
-     */
-    private fun isPreviousConditionalStart(current: PsiElement): Boolean {
-        return isStartConditional(nextConditionalCommand(current, searchBackwards = true) ?: return false)
-    }
-
-    /**
-     * Next relevant command. There are  many ways in which this does not work, but since this is just an inspection this is much safer than trying to parse user defined \if commands in the parser, which is impossiblee
-     */
-    private fun nextConditionalCommand(element: PsiElement, searchBackwards: Boolean): PsiElement? {
-        var current = element.parentOfType(LatexNoMathContent::class)
-        while (current != null && !isConditional(current)) {
-            current = if (!searchBackwards) {
-                current.nextSibling?.nextSiblingOfType(LatexNoMathContent::class)
-            }
-            else {
-                current.prevSibling?.previousSiblingOfType(LatexNoMathContent::class)
-            }
-        }
-        return current
-    }
-
-    private fun isConditional(element: PsiElement): Boolean {
-        return isStartConditional(element) || isEndConditional(element)
-    }
-
-    private fun isStartConditional(rootElement: PsiElement): Boolean {
-        // To keep it simple, only look one level down
-        for (element in rootElement.children + listOf(rootElement)) {
-            if (element is LatexCommands && element.name?.startsWith("\\if") == true) return true
-            if (element.elementType == LatexTypes.START_IF) return true
-        }
-        return false
-    }
-
-    private fun isEndConditional(rootElement: PsiElement): Boolean {
-        for (element in rootElement.children + listOf(rootElement)) {
-            if (element.firstChild?.elementType in setOf(LatexTypes.ELSE, LatexTypes.END_IF)) return true
-        }
-        return false
-    }
 }

src/nl/hannahsten/texifyidea/lang/commands/LatexGenericRegularCommand.kt

@@ -142,6 +142,7 @@ enum class LatexGenericRegularCommand(
     HYPERREF("hyperref", "options".asOptional(), "label".asRequired(Argument.Type.TEXT), dependency = LatexPackage.HYPERREF),
     HYPHENATION("hyphenation", "words".asRequired(Argument.Type.TEXT)),
     I("i", display = "i (dotless)"),
+    IFTHENELSE("ifthenelse", "test".asRequired(), "then clause".asRequired(), "else clause".asRequired()),
     IMPORT("import", RequiredFolderArgument("absolute path"), RequiredFileArgument("filename", false, false, "tex"), dependency = LatexPackage.IMPORT),
     INCLUDE("include", RequiredFileArgument("sourcefile", false, false, "tex")),
     INCLUDEFROM("includefrom", RequiredFolderArgument("absolute path"), RequiredFileArgument("filename", false, false, "tex"), dependency = LatexPackage.IMPORT),

src/nl/hannahsten/texifyidea/util/ConditionalCommands.kt

@@ -0,0 +1,76 @@
+package nl.hannahsten.texifyidea.util
+
+import com.intellij.psi.PsiElement
+import com.intellij.psi.util.elementType
+import nl.hannahsten.texifyidea.lang.commands.LatexGenericRegularCommand
+import nl.hannahsten.texifyidea.psi.LatexCommands
+import nl.hannahsten.texifyidea.psi.LatexNoMathContent
+import nl.hannahsten.texifyidea.psi.LatexParameter
+import nl.hannahsten.texifyidea.psi.LatexTypes
+import nl.hannahsten.texifyidea.util.parser.firstParentOfType
+import nl.hannahsten.texifyidea.util.parser.nextSiblingOfType
+import nl.hannahsten.texifyidea.util.parser.parentOfType
+import nl.hannahsten.texifyidea.util.parser.previousSiblingOfType
+
+/**
+ * If the element is in an if/else construct
+ */
+fun isInConditionalBranch(element: PsiElement): Boolean {
+    // \ifthenelse{condition}{true}{false}
+    if (element.firstParentOfType<LatexParameter>()?.firstParentOfType<LatexCommands>()?.name == LatexGenericRegularCommand.IFTHENELSE.commandWithSlash) {
+        return true
+    }
+
+    // Check for an \if...\fi combination
+    return isPreviousConditionalStart(element) && isNextConditionalEnd(element)
+}
+
+/**
+ * If the next relevant command is a \fi
+ */
+private fun isNextConditionalEnd(current: PsiElement): Boolean {
+    return isEndConditional(nextConditionalCommand(current, searchBackwards = false) ?: return false)
+}
+
+/**
+ * If the previous relevant command is an \if
+ */
+private fun isPreviousConditionalStart(current: PsiElement): Boolean {
+    return isStartConditional(nextConditionalCommand(current, searchBackwards = true) ?: return false)
+}
+
+/**
+ * Next relevant command. There are  many ways in which this does not work, but since this is just an inspection this is much safer than trying to parse user defined \if commands in the parser, which is impossiblee
+ */
+private fun nextConditionalCommand(element: PsiElement, searchBackwards: Boolean): PsiElement? {
+    var current = element.parentOfType(LatexNoMathContent::class)
+    while (current != null && !isConditional(current)) {
+        current = if (!searchBackwards) {
+            current.nextSibling?.nextSiblingOfType(LatexNoMathContent::class)
+        }
+        else {
+            current.prevSibling?.previousSiblingOfType(LatexNoMathContent::class)
+        }
+    }
+    return current
+}
+
+private fun isConditional(element: PsiElement): Boolean {
+    return isStartConditional(element) || isEndConditional(element)
+}
+
+private fun isStartConditional(rootElement: PsiElement): Boolean {
+    // To keep it simple, only look one level down
+    for (element in rootElement.children + listOf(rootElement)) {
+        if (element is LatexCommands && element.name?.startsWith("\\if") == true) return true
+        if (element.elementType == LatexTypes.START_IF) return true
+    }
+    return false
+}
+
+private fun isEndConditional(rootElement: PsiElement): Boolean {
+    for (element in rootElement.children + listOf(rootElement)) {
+        if (element.firstChild?.elementType in setOf(LatexTypes.ELSE, LatexTypes.END_IF)) return true
+    }
+    return false
+}

test/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspectionTest.kt

@@ -39,4 +39,19 @@ class LatexDuplicateDefinitionInspectionTest : TexifyInspectionTestBase(LatexDup
         )
         myFixture.checkHighlighting()
     }
+
+    fun testIfthenelse() {
+        myFixture.configureByText(
+            LatexFileType,
+            """
+            \usepackage{ifthen}
+            \ifthenelse {\equal{\venue}{1}}{
+                \newcommand\examLocation{Building 1}
+            }{
+                \newcommand\examLocation{Building 2}
+            }
+            """.trimIndent()
+        )
+        myFixture.checkHighlighting()
+    }
 }