Замечания сонара

1c-syntax · Jun 13, 2024 · 656b264 · 656b264
1 parent 7ad9766
commit 656b264
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 80 deletions.
diff --git a/...om/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractExpressionTreeDiagnostic.java b/...om/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractExpressionTreeDiagnostic.java
@@ -58,11 +58,11 @@ public final List<Diagnostic> getDiagnostics(DocumentContext documentContext) {
    * При входе в выражение вызывается данный метод.
    * Переопределяя его можно оценить - имеет ли смысл строить дерево выражения, или данное выражение не подходит.
    * Позволяет сократить время на построение дерева, если это не требуется для данного AST.
+   *
    * @param ctx - выражение, которое в данный момент посещается.
-   * @return
-   *   - если надо прекратить обход в глубину и построить Expression Tree на данном выражении - надо вернуть ACCEPT
-   *   - если надо пройти дальше и посетить дочерние выражения, не затрагивая данное - надо вернуть VISIT_CHILDREN
-   *   - если надо пропустить выражение, не ходить глубже и не строить Expression Tree - надо вернуть SKIP
+   * @return - если надо прекратить обход в глубину и построить Expression Tree на данном выражении - надо вернуть ACCEPT
+   * - если надо пройти дальше и посетить дочерние выражения, не затрагивая данное - надо вернуть VISIT_CHILDREN
+   * - если надо пропустить выражение, не ходить глубже и не строить Expression Tree - надо вернуть SKIP
    */
   protected ExpressionVisitorDecision onExpressionEnter(BSLParser.ExpressionContext ctx) {
     return ExpressionVisitorDecision.ACCEPT;
@@ -96,17 +96,19 @@ 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 ACCEPT -> processExpression(ctx);
         case VISIT_CHILDREN -> super.visitChildren(ctx);
       };
     }
+
+    private BSLParser.ExpressionContext processExpression(BSLParser.ExpressionContext ctx) {
+      super.visitExpression(ctx);
+      var expressionTree = getExpressionTree();
+      if (expressionTree != null) // нашлись выражения в предложенном файле
+        visitTopLevelExpression(expressionTree);
+
+      return ctx;
+    }
   }
 
 }
diff --git a/.../java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java b/.../java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java
@@ -44,11 +44,6 @@
 )
 public class DoubleNegativesDiagnostic extends AbstractExpressionTreeDiagnostic {
 
-  @Override
-  protected ExpressionVisitorDecision onExpressionEnter(BSLParser.ExpressionContext ctx) {
-    return super.onExpressionEnter(ctx);
-  }
-
   @Override
   protected void visitBinaryOperation(BinaryOperationNode node) {
 
@@ -64,9 +59,7 @@ protected void visitBinaryOperation(BinaryOperationNode node) {
       return;
     }
 
-    if (node.getOperator() == BslOperator.NOT_EQUAL) {
-      addDiagnostic(node);
-    } else if (isBooleanLiteral(node.getLeft()) || isBooleanLiteral(node.getRight())) {
+    if (node.getOperator() == BslOperator.NOT_EQUAL || isBooleanLiteral(node.getLeft()) || isBooleanLiteral(node.getRight())) {
       addDiagnostic(node);
     }
 
@@ -76,8 +69,8 @@ protected void visitBinaryOperation(BinaryOperationNode node) {
   @Override
   protected void visitUnaryOperation(UnaryOperationNode node) {
     if (node.getOperator() == BslOperator.NOT &&
-        node.getParent() != null &&
-        node.getParent().getNodeType() == ExpressionNodeType.UNARY_OP) {
+      node.getParent() != null &&
+      node.getParent().getNodeType() == ExpressionNodeType.UNARY_OP) {
 
       var unaryParent = node.getParent().<UnaryOperationNode>cast();
       if (unaryParent.getOperator() == BslOperator.NOT) {
@@ -88,28 +81,44 @@ protected void visitUnaryOperation(UnaryOperationNode node) {
     super.visitUnaryOperation(node);
   }
 
-  private boolean isBooleanLiteral(BslExpression node) {
-    if (node.getNodeType() != ExpressionNodeType.LITERAL)
+  private static 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;
+    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();
+    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();
+    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/.../com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java b/.../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.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;
 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;
@@ -228,52 +224,6 @@ private static void fillTokens(BslExpression node, List<Token> collection) {
     }
   }
 
-  private static List<BinaryOperationNode> flattenBinaryOperations(BslExpression tree) {
-    var list = new ArrayList<BinaryOperationNode>();
-    gatherBinaryOperations(list, tree);
-    return list;
-  }
-
-  private static void gatherBinaryOperations(List<BinaryOperationNode> list, BslExpression tree) {
-    switch (tree.getNodeType()) {
-      case CALL:
-        for (var expr : tree.<AbstractCallNode>cast().arguments()) {
-          gatherBinaryOperations(list, expr);
-        }
-        break;
-      case UNARY_OP:
-        gatherBinaryOperations(list, tree.<UnaryOperationNode>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/...hub/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java b/...hub/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
@@ -59,6 +59,7 @@ public int getPriority() {
 
   /**
    * Хелпер построения дерева выражения на основе готового AST выражения
+   *
    * @param ctx AST выражения
    * @return дерево вычисления выражения
    */
@@ -201,8 +202,9 @@ private void processOperation(OperatorInCode operator) {
 
   private boolean hasHigherPriorityOperatorsInFly(OperatorInCode operator) {
     var lastSeenOperator = operatorsInFly.peek();
-    if (lastSeenOperator == null)
+    if (lastSeenOperator == null) {
       return false;
+    }
 
     return lastSeenOperator.getPriority() > operator.getPriority();
   }