diff --git a/docs/diagnostics/DoubleNegatives.md b/docs/diagnostics/DoubleNegatives.md new file mode 100644 index 00000000000..fda21235024 --- /dev/null +++ b/docs/diagnostics/DoubleNegatives.md @@ -0,0 +1,30 @@ +# Двойные отрицания (DoubleNegatives) + + +## Описание диагностики + +Использование двойных отрицаний усложняет понимание кода и может приводить к ошибкам, когда вместо истины разработчик "в уме" вычислил Ложь, или наоборот. +Двойные отрицания рекомендуется заменять на выражения условий, которые прямо выражают намерения автора. + +## Примеры + +### Неправильно + +```bsl +Если Не ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") <> Неопределено Тогда + // Сделать действие +КонецЕсли; +``` + +### Правильно + +```bsl +Если ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") = Неопределено Тогда + // Сделать действие +КонецЕсли; +``` + +## Источники + + +* Источник: [Remove double negative](https://www.refactoring.com/catalog/removeDoubleNegative.html) diff --git a/docs/en/diagnostics/DoubleNegatives.md b/docs/en/diagnostics/DoubleNegatives.md new file mode 100644 index 00000000000..3a6b3059d40 --- /dev/null +++ b/docs/en/diagnostics/DoubleNegatives.md @@ -0,0 +1,28 @@ +# Double negatives (DoubleNegatives) + + +## Description + +Using double negatives complicates the understanding of the code and can lead to errors when instead of truth the developer "in his mind" calculated False, or vice versa. It is recommended to replace double negatives with conditions that directly express the author's intentions. + +## Examples + +### Wrong + +```bsl +If Not ValueTable.Find(ValueToSearch, "Column") <> Undefined Тогда + // Act +EndIf; +``` + +### Correct + +```bsl +If ValueTable.Find(ValueToSearch, "Column") = Undefined Тогда + // Act +EndIf; +``` + +## Sources + +* Источник: [Remove double negative](https://www.refactoring.com/catalog/removeDoubleNegative.html) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractExpressionTreeDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractExpressionTreeDiagnostic.java new file mode 100644 index 00000000000..dbbcc1c9c50 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractExpressionTreeDiagnostic.java @@ -0,0 +1,123 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2024 + * Alexey Sosnoviy , Nikita Fedkin and contributors + * + * SPDX-License-Identifier: LGPL-3.0-or-later + * + * BSL Language Server is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3.0 of the License, or (at your option) any later version. + * + * BSL Language Server is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with BSL Language Server. + */ +package com.github._1c_syntax.bsl.languageserver.diagnostics; + +import com.github._1c_syntax.bsl.languageserver.context.DocumentContext; +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticInfo; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionTreeBuildingVisitor; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionTreeVisitor; +import com.github._1c_syntax.bsl.parser.BSLParser; +import com.github._1c_syntax.bsl.parser.BSLParserBaseVisitor; +import lombok.Getter; +import lombok.Setter; +import org.antlr.v4.runtime.tree.ParseTree; +import org.eclipse.lsp4j.Diagnostic; + +import java.util.List; + +/** + * Диагностика, анализирующая выражения BSL и предоставляющая для этого Expression Tree + */ +public abstract class AbstractExpressionTreeDiagnostic extends ExpressionTreeVisitor implements BSLDiagnostic { + @Getter + @Setter + protected DiagnosticInfo info; + protected final DiagnosticStorage diagnosticStorage = new DiagnosticStorage(this); + protected DocumentContext documentContext; + + @Override + public final List getDiagnostics(DocumentContext documentContext) { + this.documentContext = documentContext; + diagnosticStorage.clearDiagnostics(); + + var expressionTreeBuilder = new ExpressionTreeBuilder(); + expressionTreeBuilder.visitFile(documentContext.getAst()); + + return diagnosticStorage.getDiagnostics(); + } + + /** + * При входе в выражение вызывается данный метод. + * Переопределяя его можно оценить - имеет ли смысл строить дерево выражения, или данное выражение не подходит. + * Позволяет сократить время на построение дерева, если это не требуется для данного AST. + * + * @param ctx - выражение, которое в данный момент посещается. + * @return - флаг дальнейшего поведения. + * - если надо прекратить обход в глубину и построить Expression Tree на данном выражении - надо вернуть ACCEPT + * - если надо пройти дальше и посетить дочерние выражения, не затрагивая данное - надо вернуть VISIT_CHILDREN + * - если надо пропустить выражение, не ходить глубже и не строить Expression Tree - надо вернуть SKIP + */ + protected ExpressionVisitorDecision onExpressionEnter(BSLParser.ExpressionContext ctx) { + return ExpressionVisitorDecision.ACCEPT; + } + + /** + * Стратегия по построению дерева выражения на основе выражения AST + */ + protected enum ExpressionVisitorDecision { + + /** + * Не обрабатывать выражение + */ + SKIP, + + /** + * Обработать данное выражение (построить для него ExpressionTree) + */ + ACCEPT, + + /** + * Пропустить данное выражение и обойти вложенные в него выражения + */ + VISIT_CHILDREN + } + + private class ExpressionTreeBuilder extends BSLParserBaseVisitor { + + @Override + public ParseTree visitExpression(BSLParser.ExpressionContext ctx) { + + var treeBuildingVisitor = new ExpressionTreeBuildingVisitor(); + + var result = onExpressionEnter(ctx); + return switch (result) { + case SKIP -> ctx; + case ACCEPT -> processExpression(treeBuildingVisitor, ctx); + case VISIT_CHILDREN -> treeBuildingVisitor.visitChildren(ctx); + }; + } + + private BSLParser.ExpressionContext processExpression( + ExpressionTreeBuildingVisitor treeBuildingVisitor, + BSLParser.ExpressionContext ctx + ) { + treeBuildingVisitor.visitExpression(ctx); + var expressionTree = treeBuildingVisitor.getExpressionTree(); + if (expressionTree != null) { // нашлись выражения в предложенном файле + visitTopLevelExpression(expressionTree); + } + + return ctx; + } + } + +} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java new file mode 100644 index 00000000000..9e895698adc --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java @@ -0,0 +1,115 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2024 + * Alexey Sosnoviy , Nikita Fedkin and contributors + * + * SPDX-License-Identifier: LGPL-3.0-or-later + * + * BSL Language Server is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3.0 of the License, or (at your option) any later version. + * + * BSL Language Server is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with BSL Language Server. + */ +package com.github._1c_syntax.bsl.languageserver.diagnostics; + +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticMetadata; +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticSeverity; +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticTag; +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticType; +import com.github._1c_syntax.bsl.languageserver.utils.Trees; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BinaryOperationNode; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslExpression; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslOperator; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionNodeType; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.UnaryOperationNode; + +@DiagnosticMetadata( + type = DiagnosticType.CODE_SMELL, + severity = DiagnosticSeverity.MAJOR, + minutesToFix = 3, + tags = { + DiagnosticTag.BRAINOVERLOAD, + DiagnosticTag.BADPRACTICE + } +) +public class DoubleNegativesDiagnostic extends AbstractExpressionTreeDiagnostic { + + @Override + protected void visitBinaryOperation(BinaryOperationNode node) { + + if (node.getOperator() != BslOperator.EQUAL && node.getOperator() != BslOperator.NOT_EQUAL) { + super.visitBinaryOperation(node); + return; + } + + var parent = node.getParent(); + + if (parent == null || !isNegationOperator(parent)) { + super.visitBinaryOperation(node); + return; + } + + if (node.getOperator() == BslOperator.NOT_EQUAL) { + addDiagnostic(node); + } + + super.visitBinaryOperation(node); + } + + @Override + protected void visitUnaryOperation(UnaryOperationNode node) { + if (node.getOperator() == BslOperator.NOT && + node.getParent() != null && + node.getParent().getNodeType() == ExpressionNodeType.UNARY_OP) { + + var unaryParent = node.getParent().cast(); + if (unaryParent.getOperator() == BslOperator.NOT) { + addDiagnostic(node); + } + } + + super.visitUnaryOperation(node); + } + + private static boolean isNegationOperator(BslExpression parent) { + return parent.getNodeType() == ExpressionNodeType.UNARY_OP + && parent.cast().getOperator() == BslOperator.NOT; + } + + private void addDiagnostic(BinaryOperationNode node) { + var startToken = Trees.getTokens(node.getParent().getRepresentingAst()) + .stream() + .findFirst() + .orElseThrow(); + + var endToken = Trees.getTokens(node.getRight().getRepresentingAst()) + .stream() + .reduce((one, two) -> two) + .orElseThrow(); + + diagnosticStorage.addDiagnostic(startToken, endToken); + } + + private void addDiagnostic(UnaryOperationNode node) { + var startToken = Trees.getTokens(node.getParent().getRepresentingAst()) + .stream() + .findFirst() + .orElseThrow(); + + var endToken = Trees.getTokens(node.getOperand().getRepresentingAst()) + .stream() + .reduce((one, two) -> two) + .orElseThrow(); + + diagnosticStorage.addDiagnostic(startToken, endToken); + } +} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java index 5b81dee5aa8..6a9a19338f2 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java @@ -29,20 +29,16 @@ import com.github._1c_syntax.bsl.languageserver.providers.FormatProvider; import com.github._1c_syntax.bsl.languageserver.utils.Ranges; import com.github._1c_syntax.bsl.languageserver.utils.Trees; -import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.AbstractCallNode; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BinaryOperationNode; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslExpression; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslOperator; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionNodeType; -import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionParseTreeRewriter; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.NodeEqualityComparer; -import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.TernaryOperatorNode; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.TransitiveOperationsIgnoringComparer; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.UnaryOperationNode; import com.github._1c_syntax.bsl.parser.BSLParser; import lombok.RequiredArgsConstructor; import org.antlr.v4.runtime.Token; -import org.antlr.v4.runtime.tree.ParseTree; import org.eclipse.lsp4j.FormattingOptions; import java.util.ArrayList; @@ -62,7 +58,7 @@ } ) @RequiredArgsConstructor -public class IdenticalExpressionsDiagnostic extends AbstractVisitorDiagnostic { +public class IdenticalExpressionsDiagnostic extends AbstractExpressionTreeDiagnostic { private static final int MIN_EXPRESSION_SIZE = 3; private static final String POPULAR_DIVISORS_DEFAULT_VALUE = "60, 1024"; @@ -73,7 +69,10 @@ public class IdenticalExpressionsDiagnostic extends AbstractVisitorDiagnostic { ) private Set popularDivisors = parseCommaSeparatedSet(POPULAR_DIVISORS_DEFAULT_VALUE); private final FormatProvider formatProvider; - + + private final List binaryOperations = new ArrayList<>(); + private BSLParser.ExpressionContext expressionContext; + private static Set parseCommaSeparatedSet(String values) { if (values.trim().isEmpty()) { return Collections.emptySet(); @@ -95,28 +94,41 @@ public void configure(Map configuration) { } @Override - public ParseTree visitExpression(BSLParser.ExpressionContext ctx) { - - if (sufficientSize(ctx)) { - return ctx; - } - - var tree = ExpressionParseTreeRewriter.buildExpressionTree(ctx); + protected ExpressionVisitorDecision onExpressionEnter(BSLParser.ExpressionContext ctx) { + expressionContext = ctx; + return sufficientSize(ctx)? ExpressionVisitorDecision.SKIP : ExpressionVisitorDecision.ACCEPT; + } - var binariesList = flattenBinaryOperations(tree); - if (binariesList.isEmpty()) { - return ctx; - } + @Override + protected void visitTopLevelExpression(BslExpression node) { + binaryOperations.clear(); + super.visitTopLevelExpression(node); var comparer = new TransitiveOperationsIgnoringComparer(); comparer.logicalOperationsAsTransitive(true); - binariesList + binaryOperations .stream() .filter(x -> checkEquality(comparer, x)) - .forEach(x -> diagnosticStorage.addDiagnostic(ctx, + .forEach(x -> diagnosticStorage.addDiagnostic(expressionContext, info.getMessage(x.getRepresentingAst().getText(), getOperandText(x)))); + } + + @Override + protected void visitBinaryOperation(BinaryOperationNode node) { + var operator = node.getOperator(); + + // разыменования отбросим, хотя comparer их и не зачтет, но для производительности + // лучше выкинем их сразу + if (operator == BslOperator.DEREFERENCE || operator == BslOperator.INDEX_ACCESS) { + return; + } - return ctx; + // одинаковые умножения и сложения - не считаем, см. тесты + if (operator != BslOperator.ADD && operator != BslOperator.MULTIPLY) { + binaryOperations.add(node); + } + + super.visitBinaryOperation(node); } private boolean checkEquality(NodeEqualityComparer comparer, BinaryOperationNode node) { @@ -212,52 +224,6 @@ private static void fillTokens(BslExpression node, List collection) { } } - private static List flattenBinaryOperations(BslExpression tree) { - var list = new ArrayList(); - gatherBinaryOperations(list, tree); - return list; - } - - private static void gatherBinaryOperations(List list, BslExpression tree) { - switch (tree.getNodeType()) { - case CALL: - for (var expr : tree.cast().arguments()) { - gatherBinaryOperations(list, expr); - } - break; - case UNARY_OP: - gatherBinaryOperations(list, tree.cast().getOperand()); - break; - case TERNARY_OP: - var ternary = (TernaryOperatorNode) tree; - gatherBinaryOperations(list, ternary.getCondition()); - gatherBinaryOperations(list, ternary.getTruePart()); - gatherBinaryOperations(list, ternary.getFalsePart()); - break; - case BINARY_OP: - var binary = (BinaryOperationNode) tree; - var operator = binary.getOperator(); - - // разыменования отбросим, хотя comparer их и не зачтет, но для производительности - // лучше выкинем их сразу - if (operator == BslOperator.DEREFERENCE || operator == BslOperator.INDEX_ACCESS) { - return; - } - - // одинаковые умножения и сложения - не считаем, см. тесты - if (operator != BslOperator.ADD && operator != BslOperator.MULTIPLY) { - list.add(binary); - } - - gatherBinaryOperations(list, binary.getLeft()); - gatherBinaryOperations(list, binary.getRight()); - break; - - default: - break; // для спокойствия сонара - } - } - private static boolean isComplementary(BinaryOperationNode binary) { var operator = binary.getOperator(); if ((operator == BslOperator.OR || operator == BslOperator.AND) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BinaryOperationNode.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BinaryOperationNode.java index acde5b6f19b..31549e9b98f 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BinaryOperationNode.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BinaryOperationNode.java @@ -22,16 +22,16 @@ package com.github._1c_syntax.bsl.languageserver.utils.expressiontree; import lombok.EqualsAndHashCode; +import lombok.Getter; import lombok.ToString; -import lombok.Value; import org.antlr.v4.runtime.tree.ParseTree; -@Value +@Getter @EqualsAndHashCode(callSuper = true) @ToString(callSuper = true) public class BinaryOperationNode extends BslOperationNode { - BslExpression left; - BslExpression right; + private final BslExpression left; + private final BslExpression right; private BinaryOperationNode(BslOperator operator, BslExpression left, BslExpression right, ParseTree actualSourceCode) { super(ExpressionNodeType.BINARY_OP, operator, actualSourceCode); diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java index 76bc9dc1da0..72a98f95e9e 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java @@ -24,18 +24,22 @@ import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Data; -import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; +import lombok.Setter; +import lombok.ToString; import org.antlr.v4.runtime.tree.ParseTree; @Data @RequiredArgsConstructor(access = AccessLevel.PROTECTED) @AllArgsConstructor(access = AccessLevel.PROTECTED) -@NoArgsConstructor(access = AccessLevel.PRIVATE, force = true) public abstract class BslExpression { private final ExpressionNodeType nodeType; private ParseTree representingAst; + @ToString.Exclude + @Setter(AccessLevel.PACKAGE) + private BslExpression parent; + /** * Синтаксический-помощник для более удобных downcast-ов * @param тип, к которому надо привести данный узел diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslOperationNode.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslOperationNode.java index 8afabd15956..fa065e25bae 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslOperationNode.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslOperationNode.java @@ -36,7 +36,7 @@ public abstract class BslOperationNode extends BslExpression { BslOperator operator; protected BslOperationNode(ExpressionNodeType type, BslOperator operator, ParseTree sourceCodeOperator) { - super(type, sourceCodeOperator); + super(type, sourceCodeOperator, null); this.operator = operator; } diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionParseTreeRewriter.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionParseTreeRewriter.java deleted file mode 100644 index c580191eb8d..00000000000 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionParseTreeRewriter.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * This file is a part of BSL Language Server. - * - * Copyright (c) 2018-2024 - * Alexey Sosnoviy , Nikita Fedkin and contributors - * - * SPDX-License-Identifier: LGPL-3.0-or-later - * - * BSL Language Server is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3.0 of the License, or (at your option) any later version. - * - * BSL Language Server is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with BSL Language Server. - */ -package com.github._1c_syntax.bsl.languageserver.utils.expressiontree; - -import com.github._1c_syntax.bsl.parser.BSLParser; - - -/** - * Преобразователь выражения в дерево вычисления. - */ -public final class ExpressionParseTreeRewriter { - - private ExpressionParseTreeRewriter(){ - } - - /** - * @return результирующее выражение в виде дерева вычисления операций - */ - public static BslExpression buildExpressionTree(BSLParser.ExpressionContext expression) { - var visitor = new ExpressionTreeBuildingVisitor(); - visitor.visitExpression(expression); - return visitor.getExpressionTree(); - } - -} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java index 178e5d79883..cf39d3ed566 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java @@ -33,13 +33,12 @@ import java.util.Deque; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; /** - * внутренний, неэкспортируемый класс. + * Посетитель AST, который находит выражения и преобразует их в Expression Tree */ -class ExpressionTreeBuildingVisitor extends BSLParserBaseVisitor { +public final class ExpressionTreeBuildingVisitor extends BSLParserBaseVisitor { @Value private static class OperatorInCode { @@ -57,6 +56,18 @@ public int getPriority() { private BslExpression resultExpression; private int recursionLevel = -1; + /** + * Хелпер построения дерева выражения на основе готового AST выражения + * + * @param ctx AST выражения + * @return дерево вычисления выражения + */ + public static BslExpression buildExpressionTree(BSLParser.ExpressionContext ctx) { + var instance = new ExpressionTreeBuildingVisitor(); + instance.visitExpression(ctx); + return instance.getExpressionTree(); + } + /** * @return результирующее выражение в виде дерева вычисления операций */ @@ -149,10 +160,6 @@ public ParseTree visitMember(BSLParser.MemberContext ctx) { dispatchChild.accept(this); } - if (unaryModifier != null) { - buildOperation(); - } - return ctx; } @@ -185,14 +192,22 @@ private void processOperation(OperatorInCode operator) { return; } - var lastSeenOperator = operatorsInFly.peek(); - if (lastSeenOperator.getPriority() > operator.getPriority()) { + while (hasHigherPriorityOperatorsInFly(operator)) { buildOperation(); } operatorsInFly.push(operator); } + private boolean hasHigherPriorityOperatorsInFly(OperatorInCode operator) { + var lastSeenOperator = operatorsInFly.peek(); + if (lastSeenOperator == null) { + return false; + } + + return lastSeenOperator.getPriority() > operator.getPriority(); + } + private static BslOperator getOperator(BSLParser.OperationContext ctx) { if (ctx.PLUS() != null) { return BslOperator.ADD; @@ -301,7 +316,7 @@ public ParseTree visitNewExpression(BSLParser.NewExpressionContext ctx) { if (typeName == null) { // function style var typeNameArg = args.get(0); - args = args.stream().skip(1).collect(Collectors.toList()); + args = args.stream().skip(1).toList(); callNode = ConstructorCallNode.createDynamic(makeSubexpression(typeNameArg.expression())); } else { // static style @@ -362,7 +377,7 @@ public ParseTree visitTernaryOperator(BSLParser.TernaryOperatorContext ctx) { } private static BslExpression makeSubexpression(BSLParser.ExpressionContext ctx) { - return ExpressionParseTreeRewriter.buildExpressionTree(ctx); + return buildExpressionTree(ctx); } private static void addCallArguments(AbstractCallNode callNode, List args) { @@ -387,11 +402,16 @@ private void buildOperation() { var operand = operands.pop(); var operation = UnaryOperationNode.create(operator.getOperator(), operand, operator.getActualSourceCode()); + operand.setParent(operation); operands.push(operation); } else { var right = operands.pop(); var left = operands.pop(); var binaryOp = BinaryOperationNode.create(operator.getOperator(), left, right, operator.getActualSourceCode()); + + left.setParent(binaryOp); + right.setParent(binaryOp); + operands.push(binaryOp); } } diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeVisitor.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeVisitor.java new file mode 100644 index 00000000000..951ac2b76d9 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeVisitor.java @@ -0,0 +1,74 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2024 + * Alexey Sosnoviy , Nikita Fedkin and contributors + * + * SPDX-License-Identifier: LGPL-3.0-or-later + * + * BSL Language Server is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3.0 of the License, or (at your option) any later version. + * + * BSL Language Server is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with BSL Language Server. + */ +package com.github._1c_syntax.bsl.languageserver.utils.expressiontree; + +/** + * Обходчик дерева выражений + */ +public class ExpressionTreeVisitor { + + private void visit(BslExpression node) { + switch (node.getNodeType()) { + case CALL: + visitAbstractCall((AbstractCallNode) node); + break; + case UNARY_OP: + visitUnaryOperation((UnaryOperationNode) node); + break; + case TERNARY_OP: + var ternary = (TernaryOperatorNode) node; + visitTernaryOperator(ternary); + break; + case BINARY_OP: + visitBinaryOperation((BinaryOperationNode)node); + break; + + default: + break; // для спокойствия сонара + } + } + + protected void visitTopLevelExpression(BslExpression node) { + visit(node); + } + + protected void visitAbstractCall(AbstractCallNode node) { + for (var expr : node.arguments()) { + visit(expr); + } + } + + protected void visitUnaryOperation(UnaryOperationNode node) { + visit(node.getOperand()); + } + + protected void visitBinaryOperation(BinaryOperationNode node) { + visit(node.getLeft()); + visit(node.getRight()); + } + + protected void visitTernaryOperator(TernaryOperatorNode node) { + visit(node.getCondition()); + visit(node.getTruePart()); + visit(node.getFalsePart()); + } +} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/TerminalSymbolNode.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/TerminalSymbolNode.java index dae120ca7b3..e32ae01ee08 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/TerminalSymbolNode.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/TerminalSymbolNode.java @@ -30,7 +30,7 @@ */ public class TerminalSymbolNode extends BslExpression { private TerminalSymbolNode(ExpressionNodeType type, ParseTree representingAst) { - super(type, representingAst); + super(type, representingAst, null); } /** diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json index 749df4a1160..917a458b09c 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json @@ -526,6 +526,16 @@ "title": "Disable safe mode", "$id": "#/definitions/DisableSafeMode" }, + "DoubleNegatives": { + "description": "Double negatives", + "default": true, + "type": [ + "boolean", + "object" + ], + "title": "Double negatives", + "$id": "#/definitions/DoubleNegatives" + }, "DuplicateRegion": { "description": "Duplicate regions", "default": true, diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json index 96fdadfbdff..e22c02dd4b2 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json @@ -149,6 +149,9 @@ "DisableSafeMode": { "$ref": "parameters-schema.json#/definitions/DisableSafeMode" }, + "DoubleNegatives": { + "$ref": "parameters-schema.json#/definitions/DoubleNegatives" + }, "DuplicateRegion": { "$ref": "parameters-schema.json#/definitions/DuplicateRegion" }, diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic_en.properties new file mode 100644 index 00000000000..b40f038bd92 --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic_en.properties @@ -0,0 +1,2 @@ +diagnosticMessage=Using double negatives complicates understanding of code +diagnosticName=Double negatives diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic_ru.properties new file mode 100644 index 00000000000..06fb1ed09ea --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic_ru.properties @@ -0,0 +1,2 @@ +diagnosticMessage=Использование двойных отрицаний усложняет понимание кода +diagnosticName=Двойные отрицания diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnosticTest.java new file mode 100644 index 00000000000..00311548343 --- /dev/null +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnosticTest.java @@ -0,0 +1,57 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2024 + * Alexey Sosnoviy , Nikita Fedkin and contributors + * + * SPDX-License-Identifier: LGPL-3.0-or-later + * + * BSL Language Server is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3.0 of the License, or (at your option) any later version. + * + * BSL Language Server is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with BSL Language Server. + */ +package com.github._1c_syntax.bsl.languageserver.diagnostics; + +import org.eclipse.lsp4j.Diagnostic; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static com.github._1c_syntax.bsl.languageserver.util.Assertions.assertThat; + +class DoubleNegativesDiagnosticTest extends AbstractDiagnosticTest { + DoubleNegativesDiagnosticTest() { + super(DoubleNegativesDiagnostic.class); + } + + @Test + void test() { + + List diagnostics = getDiagnostics(); + + assertThat(diagnostics, true) + .hasRange(1, 5, 1, 73) + .hasRange(5, 4, 5, 20) + .hasRange(6, 4, 6, 21) + .hasRange(7, 4, 7, 42) + .hasRange(8, 4, 8, 42) + .hasRange(9, 4, 9, 25) + .hasRange(10, 4, 10, 24) + .hasRange(15, 5, 15, 38) + .hasRange(19, 19, 19, 39) + .hasRange(28, 4, 28, 26) + .hasRange(29, 4, 29, 36) + .hasRange(35, 4, 35, 19) + ; + + } +} diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionParseTreeRewriterTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionParseTreeRewriterTest.java index 01b2415ae2d..c97ae2dd585 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionParseTreeRewriterTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionParseTreeRewriterTest.java @@ -28,7 +28,7 @@ import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslOperator; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ConstructorCallNode; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionNodeType; -import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionParseTreeRewriter; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionTreeBuildingVisitor; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.MethodCallNode; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.SkippedCallArgumentNode; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.UnaryOperationNode; @@ -146,6 +146,21 @@ void booleanPriority() { } + @Test + void booleanNotPriority() { + var code = "Рез = Не Б <> Неопределено И Ложь"; + var expressionTree = getExpressionTree(code); + var binary = (BinaryOperationNode) expressionTree; + + assertThat(binary.getOperator()).isEqualTo(BslOperator.AND); + + var negation = binary.getLeft().cast(); + assertThat(negation.getNodeType()).isEqualTo(ExpressionNodeType.UNARY_OP); + assertThat(negation.getOperator()).isEqualTo(BslOperator.NOT); + + assertThat((binary.getRight()).getNodeType()).isEqualTo(ExpressionNodeType.LITERAL); + } + @Test void dereferenceOfProperty() { var code = "Рез = Структура.Свойство"; @@ -317,8 +332,46 @@ void realLifeHardExpression() { assertThat(binary.getRight().cast().getOperator()).isEqualTo(BslOperator.EQUAL); } + @Test + void notOperatorPriority() { + var code = "А = Не Разыменование.Метод() = Неопределено"; + + var expressionTree = getExpressionTree(code); + + assertThat(expressionTree.getNodeType()).isEqualTo(ExpressionNodeType.UNARY_OP); + + var unary = expressionTree.cast(); + assertThat(unary.getOperator()).isEqualTo(BslOperator.NOT); + assertThat(unary.getOperand()).isInstanceOf(BinaryOperationNode.class); + + var binary = unary.getOperand().cast(); + assertThat(binary.getOperator()).isEqualTo(BslOperator.EQUAL); + assertThat(binary.getLeft().cast().getOperator()).isEqualTo(BslOperator.DEREFERENCE); + assertThat(binary.getRight().getNodeType()).isEqualTo(ExpressionNodeType.LITERAL); + + } + + @Test + void notOperatorPriority_with_parenthesis() { + var code = "А = Не (Разыменование.Метод() = Неопределено)"; + + var expressionTree = getExpressionTree(code); + + assertThat(expressionTree.getNodeType()).isEqualTo(ExpressionNodeType.UNARY_OP); + + var unary = expressionTree.cast(); + assertThat(unary.getOperator()).isEqualTo(BslOperator.NOT); + assertThat(unary.getOperand()).isInstanceOf(BinaryOperationNode.class); + + var binary = unary.getOperand().cast(); + assertThat(binary.getOperator()).isEqualTo(BslOperator.EQUAL); + assertThat(binary.getLeft().cast().getOperator()).isEqualTo(BslOperator.DEREFERENCE); + assertThat(binary.getRight().getNodeType()).isEqualTo(ExpressionNodeType.LITERAL); + + } + BslExpression getExpressionTree(String code) { var expression = parse(code); - return ExpressionParseTreeRewriter.buildExpressionTree(expression); + return ExpressionTreeBuildingVisitor.buildExpressionTree(expression); } } \ No newline at end of file diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionTreeComparersTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionTreeComparersTest.java index 0a654d5fdb9..375ee3cb2f2 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionTreeComparersTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/utils/ExpressionTreeComparersTest.java @@ -26,7 +26,7 @@ import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslExpression; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.BslOperator; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.DefaultNodeEqualityComparer; -import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionParseTreeRewriter; +import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.ExpressionTreeBuildingVisitor; import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.TransitiveOperationsIgnoringComparer; import com.github._1c_syntax.bsl.parser.BSLParser; import org.junit.jupiter.api.Test; @@ -72,7 +72,7 @@ BSLParser.ExpressionContext parse(String code) { BslExpression getExpressionTree(String code) { var expression = parse(code); - return ExpressionParseTreeRewriter.buildExpressionTree(expression); + return ExpressionTreeBuildingVisitor.buildExpressionTree(expression); } } diff --git a/src/test/resources/diagnostics/DoubleNegativesDiagnostic.bsl b/src/test/resources/diagnostics/DoubleNegativesDiagnostic.bsl new file mode 100644 index 00000000000..b28fe9f6ca5 --- /dev/null +++ b/src/test/resources/diagnostics/DoubleNegativesDiagnostic.bsl @@ -0,0 +1,41 @@ +// Выражение в условии +Если Не ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") <> Неопределено Тогда + // Сделать действие +КонецЕсли; + +А = Не Отказ <> Ложь; +А = Не (Отказ <> Ложь); +А = Не НекотороеЗначение() <> Неопределено; +А = Не Неопределено <> НекотороеЗначение(); +А = Не (А <> Неопределено); // срабатывает +А = Не А <> Неопределено И Б = 5; // срабатывает +А = Не (А <> Неопределено и Б = 5); // не срабатывает +А = Не (А <> Неопределено или Б = 5); // не срабатывает +А = Не (Б = 5 и А <> Неопределено); // не срабатывает + +Пока Не Таблица.Данные <> Неопределено Цикл +КонецЦикла; + +Б = Не (Не А = 1 или Б <> Неопределено); // не срабатывает на "Не А = 1" +Б = Не (А <> 1 или Не Б <> Неопределено); // срабатывает на "Не Б <> Неопределено" +Б = Не (А <> 1 или Не Б = Неопределено); // не срабатывает на "Не Б <> Неопределено" т.к. сравнения вида Не Х = Неопределено популярны + +Если Не Т.Найти(Значение) = Неопределено Тогда + // не срабатывает, т.к. популярный код +КонецЕсли; + +// Отрицание с проверкой на неравенство нелитералу + +А = Не (Отказ <> НеЛитерал); // срабатывает +А = Не СложнаяФункция() <> НеЛитерал; // срабатывает + +Б = Не (А = 1 или Б <> НеЛитерал); // не срабатывает + +// Прямое двойное отрицание + +Б = Не (Не Значение); +Б = Не (Не Значение И ДругоеЗначение); // не срабатывает + +// NoSuchElementException +Запись = РегистрыСведений.ЗаданияКПересчетуСтатуса.СоздатьМенеджерЗаписи(); +Запись.Записать(Истина);