src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/FlowGraphLinkException.java (1)

24-28: Добавьте serialVersionUID и конструкторы с cause для улучшения диагностики

Многие анализаторы (SpotBugs/IDEA) предупреждают об отсутствующем serialVersionUID у Serializable-исключений; дополнительные конструкторы упростят проброс первопричины.

 public class FlowGraphLinkException extends RuntimeException {
+  private static final long serialVersionUID = 1L;
   public FlowGraphLinkException(String message) {
     super(message);
   }
+  public FlowGraphLinkException(String message, Throwable cause) {
+    super(message, cause);
+  }
+  public FlowGraphLinkException(Throwable cause) {
+    super(cause);
+  }
 }

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/BasicBlockVertex.java (1)

51-57: Nit: обрамите if фигурными скобками для единообразия и безопасности будущих правок

Стиль кода в файле преимущественно со скобками; так меньше шансов на ошибку при добавлении строк.

   @Override
   public String toString() {
-    if (statements.isEmpty())
-      return "<empty block>";
+    if (statements.isEmpty()) {
+      return "<empty block>";
+    }
 
     return super.toString();
   }

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ConditionalVertex.java (1)

57-65: Проверяйте тип ребра до вызова super.onConnectOutgoing — fail-fast и отсутствие побочных эффектов

Так мы раньше отфильтруем недопустимые типы и избежим лишней работы/побочных эффектов базовой реализации.

   @Override
   protected void onConnectOutgoing(ControlFlowGraph graph, CfgVertex target, CfgEdge edge) {
-    super.onConnectOutgoing(graph, target, edge);
-
-    if (edge.getType() != CfgEdgeType.TRUE_BRANCH && edge.getType() != CfgEdgeType.FALSE_BRANCH) {
+    if (edge.getType() != CfgEdgeType.TRUE_BRANCH && edge.getType() != CfgEdgeType.FALSE_BRANCH) {
       throw new FlowGraphLinkException("Can't add edge " + this + "->"+target + "\n"
         +"Edge type " + edge.getType() + " is forbidden here.");
     }
+    super.onConnectOutgoing(graph, target, edge);
   }

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ExitVertex.java (1)

25-28: Сделайте сообщение исключения более информативным (тип ребра и целевая вершина)

Это облегчит диагностику при нарушении инварианта.

   @Override
   protected void onConnectOutgoing(ControlFlowGraph graph, CfgVertex target, CfgEdge edge) {
-    throw new FlowGraphLinkException("ExitNode can't have outgoing edges");
+    throw new FlowGraphLinkException(
+      "ExitVertex can't have outgoing edges: attempted " + edge.getType() + " to " + target
+    );
   }

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.java (1)

48-50: Унифицировать проверку уникальности с walkNext(edgeType)

Сейчас множественность проверяется только для DIRECT; имеет смысл так же контролировать уникальность для произвольного типа в walkNext(CfgEdgeType), либо вынести общую логику в приватный хелпер.

Набросок приватного хелпера и использование в обоих методах:

private Optional<CfgEdge> uniqueEdgeByType(CfgEdgeType edgeType) {
  return availableRoutes().stream()
    .filter(x -> x.getType() == edgeType)
    .reduce((a, b) -> { throw new IllegalStateException("Multiple " + edgeType + " outgoing edges in " + currentNode); });
}

public CfgEdge walkNext() {
  var edgeOrNot = uniqueEdgeByType(CfgEdgeType.DIRECT);
  if (edgeOrNot.isPresent()) {
    currentNode = graph.getEdgeTarget(edgeOrNot.get());
    return edgeOrNot.get();
  }
  throw new IllegalStateException("DIRECT edge is not found for node " + currentNode);
}

public CfgEdge walkNext(CfgEdgeType edgeType) {
  var edgeOrNot = uniqueEdgeByType(edgeType);
  if (edgeOrNot.isPresent()) {
    currentNode = graph.getEdgeTarget(edgeOrNot.get());
    return edgeOrNot.get();
  }
  throw new IllegalStateException("Edge is not found for node " + currentNode);
}

src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AllFunctionPathMustHaveReturnDiagnostic.java (1)

120-125: Избегайте Collector с поставщиком уже существующего списка

Передача поставщика, возвращающего уже созданный список, нарушает контракт Collector и может повести себя неожиданно при изменениях режима выполнения стрима. Проще и безопаснее использовать forEach/addAll.

Предлагаю заменить на прямое добавление:

-    incomingVertices.stream()
-      .map(vertex -> RelatedInformation.create(documentContext.getUri(),
-        Ranges.create(vertex),
-        info.getMessage()))
-      .collect(Collectors.toCollection(() -> listOfMessages));
+    incomingVertices.forEach(vertex ->
+      listOfMessages.add(
+        RelatedInformation.create(documentContext.getUri(), Ranges.create(vertex), info.getMessage())
+      )
+    );

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/CfgVertex.java (2)

30-31: Состояние isConnected — ок как микро-оптимизация, но проверьте жизненный цикл вершин

Флаг позволяет избежать первого обращения к графу, но делает поведение зависящим от жизненного цикла вершины (повторное использование объекта вершины в другом графе теоретически может повлиять). Если такие переиспользования исключены конвенцией — ок; иначе стоит пересмотреть на graph.outgoingEdgesOf(this).isEmpty().

Хотите, подготовлю проверку по коду на предмет переиспользования экземпляров CfgVertex между графами?

---

59-62: Нит: метод duplicateLinkError бросает исключение сам и при этом используется в выражении throw

Сейчас duplicateLinkError сам делает throw, а снаружи используется как throw duplicateLinkError(...). Это нетипично и сбивает с толку. Лучше пусть метод возвращает исключение, а бросание остается в месте вызова.

-  private FlowGraphLinkException duplicateLinkError(ControlFlowGraph graph, CfgVertex target, CfgEdge edge) {
-    throw new FlowGraphLinkException("Can't add edge " + this + "->"+target + "\n"
-      +"Source vertex " + this + " already has "+edge.getType()+" edge " + graph.edgePresentation(edge));
-  }
+  private FlowGraphLinkException duplicateLinkError(ControlFlowGraph graph, CfgVertex target, CfgEdge edge) {
+    return new FlowGraphLinkException(
+      "Can't add edge " + this + "->" + target + "\n" +
+      "Source vertex " + this + " already has " + edge.getType() + " edge " + graph.edgePresentation(edge)
+    );
+  }

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java (1)

42-44: Нит: несоответствие возвращаемых типов addEdge-оберток

Сейчас есть void-обертка addEdge(source, target, CfgEdgeType), которая игнорирует boolean-результат базового addEdge. Это консистентно со старым API, но в перспективе можно вернуть boolean для унификации (осторожно: возможен breaking change).

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/CfgBuildingParseTreeVisitor.java (4)

70-81: Верхнеуровневый препроцессор: сейчас обрабатывается только ближайший блок; стоит поддержать несколько подряд

Если перед телом модуля идут несколько верхнеуровневых препроцессорных конструкций подряд, текущая логика посетит только ближайшую к FileCodeBlock. Предлагаю пройтись по всем предыдущим узлам RULE_preprocessor и вызвать accept в порядке исходника.

Пример правки:

-    if (producePreprocessorConditionsEnabled) {
-      // Если это тело модуля, то самую первую инструкцию препроцессора сожрет грамматика file
-      // надо ее тоже посетить принудительно.
-      var parent = block.getParent();
-      if (parent instanceof BSLParser.FileCodeBlockContext fileBlock) {
-        var probablyPreprocessor = Trees.getPreviousNode(fileBlock.getParent(), fileBlock , BSLParser.RULE_preprocessor);
-        if (probablyPreprocessor != fileBlock) {
-          hasTopLevelPreprocessor = true;
-          probablyPreprocessor.accept(this);
-        }
-      }
-    }
+    if (producePreprocessorConditionsEnabled) {
+      // Если это тело модуля, грамматика file «съедает» верхнеуровневые препроцессоры.
+      // Пройдем все предыдущие препроцессоры и посетим их в порядке исходника.
+      var parent = block.getParent();
+      if (parent instanceof BSLParser.FileCodeBlockContext fileBlock) {
+        var container = fileBlock.getParent();
+        var node = Trees.getPreviousNode(container, fileBlock, BSLParser.RULE_preprocessor);
+        java.util.Deque<org.antlr.v4.runtime.tree.ParseTree> stack = new java.util.ArrayDeque<>();
+        while (node != fileBlock) {
+          stack.push(node);
+          node = Trees.getPreviousNode(container, node, BSLParser.RULE_preprocessor);
+        }
+        while (!stack.isEmpty()) {
+          hasTopLevelPreprocessor = true;
+          stack.pop().accept(this);
+        }
+      }
+    }

---

402-405: Защититься от NPE в проверке уровня препроцессора

В вызове isStatementLevelPreproc(ctx) возможен NPE при нетипичном дереве (если у узла не окажется одного из родителей). Редкий случай, но лучше сделать метод null-safe.

Предлагаемая правка метода (вне текущего диапазона строк):

private static boolean isStatementLevelPreproc(BSLParserRuleContext ctx) {
  var parent = ctx.getParent();
  if (parent == null) {
    return false;
  }
  var grandparent = parent.getParent();
  return grandparent != null && grandparent.getRuleIndex() == BSLParser.RULE_statement;
}

---

492-526: preproc_endif: корректная логика FALSE_BRANCH; возможно избыточный addVertex для хвостов

Логика mustAddFalseBranch устраняет необходимость принудительно добавлять FALSE_BRANCH, когда альтернативы уже заданы ранее — это правильно. Небольшая придирка: вызов graph.addVertex(blockTail) перед добавлением ребра, скорее всего, избыточен — хвост, как правило, уже присутствует в графе. Если ControlFlowGraph не гарантирует идемпотентность addVertex, это может быть источником ошибок; если гарантирует — всё ок.

Мини-правка:

-      graph.addVertex(blockTail);
       graph.addEdge(blockTail, upperBlock.end());

Дополнительно: стоит убедиться, что тесты покрывают оба случая — без else/elsif (добавляется FALSE_BRANCH) и с альтернативами (ветка не добавляется).

---

589-599: Перекладка входящих ребер: устранен CME; сохранить тип ребра до удаления и проверить базовую JDK

Текущий подход с предварительным копированием списка ребер предотвращает ConcurrentModificationException — это плюс. Для наглядности и на всякий случай лучше сохранить тип ребра в локальную переменную до удаления ребра из графа. Также используется Stream.toList(), требующий JDK 16+; проверьте, соответствует ли это целевой версии проекта.

Предлагаемая правка:

-      var incoming = graph.incomingEdgesOf(currentTail).stream().toList();
+      var incoming = graph.incomingEdgesOf(currentTail).stream().toList();
       for (var edge : incoming) {
         // ребра смежности не переключаем, т.к. текущий блок удаляется
         if (edge.getType() == CfgEdgeType.ADJACENT_CODE) {
           continue;
         }

-        var source = graph.getEdgeSource(edge);
-        graph.removeEdge(edge);
-        graph.addEdge(source, vertex, edge.getType());
+        var source = graph.getEdgeSource(edge);
+        var type = edge.getType();
+        graph.removeEdge(edge);
+        graph.addEdge(source, vertex, type);
       }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: nixel2007
PR: 1c-syntax/bsl-language-server#3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.

src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.java (2)

44-58: Добавить precondition: walker должен быть запущен (start())

Сейчас при вызове walkNext() без start() будет NPE внутри outgoingEdgesOf. Предлагаю явную проверку, чтобы сообщение об ошибке было предсказуемым.

Применить в этом методе:

   public CfgEdge walkNext() {
+    ensureStarted();
     var edges = availableRoutes();

И аналогично в availableRoutes() и walkNext(CfgEdgeType) (см. ниже).

Добавить вспомогательный метод в класс:

// вне выбранного диапазона, вставить в класс
private void ensureStarted() {
  if (currentNode == null) {
    throw new IllegalStateException("Walker is not started. Call start() first.");
  }
}

---

60-71: Симметрия и явная диагностика: проверять уникальность и для типизированных рёбер

Граф теперь сам запрещает дубликаты, но для симметрии с walkNext() и более явной диагностики багов графа лучше применить тот же подход в walkNext(CfgEdgeType) и уточнить текст ошибки.

-  public CfgEdge walkNext(CfgEdgeType edgeType) {
-    var edgeOrNot = availableRoutes().stream()
-      .filter(x -> x.getType() == edgeType)
-      .findAny();
-
-    if (edgeOrNot.isPresent()) {
-      currentNode = graph.getEdgeTarget(edgeOrNot.get());
-      return edgeOrNot.get();
-    }
-
-    throw new IllegalStateException("Edge is not found for node " + currentNode);
-  }
+  public CfgEdge walkNext(CfgEdgeType edgeType) {
+    ensureStarted();
+    var edgeOrNot = availableRoutes().stream()
+      .filter(x -> x.getType() == edgeType)
+      .reduce((CfgEdge a, CfgEdge b) -> {
+        throw new IllegalStateException("Multiple " + edgeType + " outgoing edges in " + currentNode);
+      });
+
+    if (edgeOrNot.isPresent()) {
+      currentNode = graph.getEdgeTarget(edgeOrNot.get());
+      return edgeOrNot.get();
+    }
+
+    throw new IllegalStateException("Edge of type " + edgeType + " is not found for node " + currentNode);
+  }

Дополнительно предложено уточнение сообщения об отсутствии ребра: включить тип ребра в текст.

src/test/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphBuilderTest.java (3)

513-531: Усилить проверку топового препроцессора

Сейчас проверяем только факт ветвления. Рекомендую дополнить проверкой точного числа исходящих рёбер и их типов — это повысит надежность теста.

   walker.start();
 
-  assertThat(walker.isOnBranch()).isTrue();
+  assertThat(walker.isOnBranch()).isTrue();
+  assertThat(walker.availableRoutes()).hasSize(2);
+  assertThat(walker.availableRoutes())
+    .extracting(CfgEdge::getType)
+    .containsExactlyInAnyOrder(CfgEdgeType.TRUE_BRANCH, CfgEdgeType.FALSE_BRANCH);

---

532-553: Уточнить ожидаемое стартовое положение

Чтобы зафиксировать, что препроцессор в разделе переменных игнорируется, можно явно проверить первую операцию.

   walker.start();
 
-  assertThat(walker.isOnBranch()).isFalse();
+  assertThat(walker.isOnBranch()).isFalse();
+  assertThat(textOfCurrentNode(walker)).isEqualTo("А=8");

---

554-578: Топовый препроцессор в теле модуля — добавить проверку численности исходящих

Для полноты картины полезно проверить, что у узла-препроцессора два исходящих ребра.

   walker.start();
 
-  assertThat(walker.isOnBranch()).isTrue();
+  assertThat(walker.isOnBranch()).isTrue();
+  assertThat(walker.availableRoutes()).hasSize(2);
   walker.walkNext(CfgEdgeType.TRUE_BRANCH);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.