diff --git a/CHANGELOG.md b/CHANGELOG.md index 674866aa9..93d0ec636 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspection.kt b/src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspection.kt index e348ea6f1..02128df21 100644 --- a/src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspection.kt +++ b/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,6 +33,7 @@ open class LatexDuplicateDefinitionInspection : TexifyInspectionBase() { val defined = HashMultiset.create() 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) } @@ -39,6 +41,7 @@ open class LatexDuplicateDefinitionInspection : TexifyInspectionBase() { // Go monkeys. file.definitions() .forEach { + if (isInConditionalBranch(it)) return@forEach val definedCmd = it.definedCommandName() ?: return@forEach if (defined.count(definedCmd) > 1) { descriptors.add( diff --git a/src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateLabelInspection.kt b/src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateLabelInspection.kt index a93c42d9e..de900a7d0 100644 --- a/src/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateLabelInspection.kt +++ b/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 - } } diff --git a/src/nl/hannahsten/texifyidea/lang/commands/LatexGenericRegularCommand.kt b/src/nl/hannahsten/texifyidea/lang/commands/LatexGenericRegularCommand.kt index 83522ac09..ae9658d81 100644 --- a/src/nl/hannahsten/texifyidea/lang/commands/LatexGenericRegularCommand.kt +++ b/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), diff --git a/src/nl/hannahsten/texifyidea/util/ConditionalCommands.kt b/src/nl/hannahsten/texifyidea/util/ConditionalCommands.kt new file mode 100644 index 000000000..b6d29ec88 --- /dev/null +++ b/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()?.firstParentOfType()?.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 +} \ No newline at end of file diff --git a/test/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspectionTest.kt b/test/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspectionTest.kt index 89212e026..f73104920 100644 --- a/test/nl/hannahsten/texifyidea/inspections/latex/redundancy/LatexDuplicateDefinitionInspectionTest.kt +++ b/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() + } } \ No newline at end of file