Реализация #3271: Диагностика двойных отрицаний

docs/diagnostics/DoubleNegatives.md

@@ -0,0 +1,30 @@
+# Двойные отрицания (DoubleNegatives)
+
+<!-- Блоки выше заполняются автоматически, не трогать -->
+## Описание диагностики
+
+Использование двойных отрицаний усложняет понимание кода и может приводить к ошибкам, когда вместо истины разработчик "в уме" вычислил Ложь, или наоборот.
+Двойные отрицания рекомендуется заменять на выражения условий, которые прямо выражают намерения автора.
+
+## Примеры
+
+### Неправильно
+
+```bsl
+Если Не ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") <> Неопределено Тогда
+    // Сделать действие
+КонецЕсли;
+```
+
+### Правильно
+
+```bsl
+Если ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") = Неопределено Тогда
+    // Сделать действие
+КонецЕсли;
+```
+
+## Источники
+<!-- Необходимо указывать ссылки на все источники, из которых почерпнута информация для создания диагностики -->
+
+* Источник: [Remove double negative](https://www.refactoring.com/catalog/removeDoubleNegative.html)

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)

src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractExpressionTreeDiagnostic.java

@@ -0,0 +1,112 @@
+/*
+ * This file is a part of BSL Language Server.
+ *
+ * Copyright (c) 2018-2024
+ * Alexey Sosnoviy <[email protected]>, Nikita Fedkin <[email protected]> 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 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<Diagnostic> 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 ExpressionTreeBuildingVisitor {
+    @Override
+    public ParseTree visitExpression(BSLParser.ExpressionContext ctx) {
+
+      var result = onExpressionEnter(ctx);
+      return switch (result) {
+        case SKIP -> ctx;
+        case ACCEPT -> {
+          super.visitExpression(ctx);
+          var expressionTree = getExpressionTree();
+          if (expressionTree != null) // нашлись выражения в предложенном файле
+            visitTopLevelExpression(expressionTree);
+
+          yield ctx;
+        }
+        case VISIT_CHILDREN -> super.visitChildren(ctx);
+      };
+    }
+  }
+
+}

src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java

@@ -0,0 +1,116 @@
+/*
+ * This file is a part of BSL Language Server.
+ *
+ * Copyright (c) 2018-2024
+ * Alexey Sosnoviy <[email protected]>, Nikita Fedkin <[email protected]> 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;
+import com.github._1c_syntax.bsl.parser.BSLParser;
+
+@DiagnosticMetadata(
+  type = DiagnosticType.CODE_SMELL,
+  severity = DiagnosticSeverity.MAJOR,
+  minutesToFix = 3,
+  tags = {
+    DiagnosticTag.BRAINOVERLOAD,
+    DiagnosticTag.BADPRACTICE
+  }
+)
+public class DoubleNegativesDiagnostic extends AbstractExpressionTreeDiagnostic {
+
+  @Override
+  protected ExpressionVisitorDecision onExpressionEnter(BSLParser.ExpressionContext ctx) {
+    return super.onExpressionEnter(ctx);
+  }
+
+  @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);
+    } else if (isBooleanLiteral(node.getLeft()) || isBooleanLiteral(node.getRight())) {
+      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().<UnaryOperationNode>cast();
+      if (unaryParent.getOperator() == BslOperator.NOT) {
+        addDiagnostic(node);
+      }
+    }
+
+    super.visitUnaryOperation(node);
+  }
+
+  private boolean isBooleanLiteral(BslExpression node) {
+    if (node.getNodeType() != ExpressionNodeType.LITERAL)
+      return false;
+
+    var constant = (BSLParser.ConstValueContext) node.getRepresentingAst();
+    return constant.TRUE() != null || constant.FALSE() != null;
+  }
+
+  private static boolean isNegationOperator(BslExpression parent) {
+    return parent.getNodeType() == ExpressionNodeType.UNARY_OP && parent.<UnaryOperationNode>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);
+  }
+}

src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java

@@ -34,7 +34,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.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.NodeEqualityComparer;
 import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.TernaryOperatorNode;
 import com.github._1c_syntax.bsl.languageserver.utils.expressiontree.TransitiveOperationsIgnoringComparer;
@@ -62,7 +62,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 +73,10 @@ public class IdenticalExpressionsDiagnostic extends AbstractVisitorDiagnostic {
   )
   private Set<String> popularDivisors = parseCommaSeparatedSet(POPULAR_DIVISORS_DEFAULT_VALUE);
   private final FormatProvider formatProvider;
-
+
+  private final List<BinaryOperationNode> binaryOperations = new ArrayList<>();
+  private BSLParser.ExpressionContext expressionContext;
+
   private static Set<String> parseCommaSeparatedSet(String values) {
     if (values.trim().isEmpty()) {
       return Collections.emptySet();
@@ -95,28 +98,41 @@ public void configure(Map<String, Object> 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;
+    }
+
+    // одинаковые умножения и сложения - не считаем, см. тесты
+    if (operator != BslOperator.ADD && operator != BslOperator.MULTIPLY) {
+      binaryOperations.add(node);
+    }
 
-    return ctx;
+    super.visitBinaryOperation(node);
   }
 
   private boolean checkEquality(NodeEqualityComparer comparer, BinaryOperationNode node) {

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);