diff --git a/build.gradle.kts b/build.gradle.kts index 6f64a88e434..d9056fbc834 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -11,12 +11,12 @@ plugins { signing id("org.cadixdev.licenser") version "0.6.1" id("org.sonarqube") version "5.1.0.4882" - id("io.freefair.lombok") version "8.7.1" - id("io.freefair.javadoc-links") version "8.7.1" - id("io.freefair.javadoc-utf-8") version "8.7.1" - id("io.freefair.aspectj.post-compile-weaving") version "8.7.1" - id("io.freefair.maven-central.validate-poms") version "8.7.1" - id("me.qoomon.git-versioning") version "6.4.3" + id("io.freefair.lombok") version "8.10" + id("io.freefair.javadoc-links") version "8.10" + id("io.freefair.javadoc-utf-8") version "8.10" + id("io.freefair.aspectj.post-compile-weaving") version "8.10" + id("io.freefair.maven-central.validate-poms") version "8.10" + id("me.qoomon.git-versioning") version "6.4.4" id("com.github.ben-manes.versions") version "0.51.0" id("org.springframework.boot") version "3.2.5" id("io.spring.dependency-management") version "1.1.6" 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..65eea5b15f4 --- /dev/null +++ b/docs/en/diagnostics/DoubleNegatives.md @@ -0,0 +1,5 @@ +# Double negatives (DoubleNegatives) + + + +* Description 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 fda40aa8eb9..394f2b27da4 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 +Запись = РегистрыСведений.ЗаданияКПересчетуСтатуса.СоздатьМенеджерЗаписи(); +Запись.Записать(Истина);