Skip to content

Commit

Permalink
Fix false positive for duplicate command definition inspection in if/…
Browse files Browse the repository at this point in the history
…else
  • Loading branch information
PHPirates committed Dec 14, 2024
1 parent 99c20e4 commit f78423e
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 55 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Include optional parameters in spellcheck, if it contains 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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*

Expand Down Expand Up @@ -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()
Expand All @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,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),
Expand Down
76 changes: 76 additions & 0 deletions src/nl/hannahsten/texifyidea/util/ConditionalCommands.kt
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

0 comments on commit f78423e

Please sign in to comment.