Skip to content

Commit

Permalink
Заготовка диагностики и красный тест на построитель выражений
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrey Ovsyankin committed Jun 3, 2024
1 parent d50117c commit c1d49f5
Show file tree
Hide file tree
Showing 8 changed files with 184 additions and 0 deletions.
30 changes: 30 additions & 0 deletions docs/diagnostics/DoubleNegatives.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Двойные отрицания (DoubleNegatives)

<!-- Блоки выше заполняются автоматически, не трогать -->
## Описание диагностики

Использование двойных отрицаний усложняет понимание кода и может приводить к ошибкам, когда вместо истины разработчик "в уме" вычислил Ложь, или наоборот.
Двойные отрицания рекомендуется заменять на выражения условий, которые прямо выражают намерения автора.

## Примеры

### Неправильно

```bsl
Если Не ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") <> Неопределено Тогда
// Сделать действие
КонецЕсли;
```

### Правильно

```bsl
Если ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") = Неопределено Тогда
// Сделать действие
КонецЕсли;
```

## Источники
<!-- Необходимо указывать ссылки на все источники, из которых почерпнута информация для создания диагностики -->

* Источник: [Remove double negative](https://www.refactoring.com/catalog/removeDoubleNegative.html)
28 changes: 28 additions & 0 deletions docs/en/diagnostics/DoubleNegatives.md
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
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.expressiontree.ExpressionParseTreeRewriter;
import com.github._1c_syntax.bsl.parser.BSLParser;
import org.antlr.v4.runtime.tree.ParseTree;

@DiagnosticMetadata(
type = DiagnosticType.CODE_SMELL,
severity = DiagnosticSeverity.MAJOR,
minutesToFix = 3,
tags = {
DiagnosticTag.BRAINOVERLOAD,
DiagnosticTag.BADPRACTICE
}
)
public class DoubleNegativesDiagnostic extends AbstractVisitorDiagnostic {

private static final int MIN_EXPRESSION_SIZE = 3;

@Override
public ParseTree visitExpression(BSLParser.ExpressionContext ctx) {

if (sufficientSize(ctx))
return ctx;

var tree = ExpressionParseTreeRewriter.buildExpressionTree(ctx);

return ctx;
}

private static boolean sufficientSize(BSLParser.ExpressionContext ctx) {
return ctx.children.size() < MIN_EXPRESSION_SIZE;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
diagnosticMessage=Using double negatives complicates understandong of code
diagnosticName=Double negatives
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
diagnosticMessage=Использование двойных отрицаний усложняет понимание кода
diagnosticName=Двойные отрицания
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
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<DoubleNegativesDiagnostic> {
DoubleNegativesDiagnosticTest() {
super(DoubleNegativesDiagnostic.class);
}

@Test
void test() {

List<Diagnostic> diagnostics = getDiagnostics();

assertThat(diagnostics, true)
.hasRange(3, 6, 3, 74)
;

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,25 @@ void realLifeHardExpression() {
assertThat(binary.getRight().<BinaryOperationNode>cast().getOperator()).isEqualTo(BslOperator.EQUAL);
}

@Test
void notOperatorPriority() {
var code = "А = Не Разыменование.Метод() = Неопределено";

var expressionTree = getExpressionTree(code);

assertThat(expressionTree.getNodeType()).isEqualTo(ExpressionNodeType.UNARY_OP);

var unary = expressionTree.<UnaryOperationNode>cast();
assertThat(unary.getOperator()).isEqualTo(BslOperator.NOT);
assertThat(unary.getOperand()).isInstanceOf(BinaryOperationNode.class);

var binary = unary.getOperand().<BinaryOperationNode>cast();
assertThat(binary.getOperator()).isEqualTo(BslOperator.EQUAL);
assertThat(binary.getLeft().getNodeType()).isEqualTo(ExpressionNodeType.CALL);
assertThat(binary.getRight().getNodeType()).isEqualTo(ExpressionNodeType.LITERAL);

}

BslExpression getExpressionTree(String code) {
var expression = parse(code);
return ExpressionParseTreeRewriter.buildExpressionTree(expression);
Expand Down
40 changes: 40 additions & 0 deletions src/test/resources/diagnostics/DoubleNegativesDiagnostic.bsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Выражение в условии
Если Не ТаблицаЗначений.Найти(ИскомоеЗначение, "Колонка") <> Неопределено Тогда
// Сделать действие
КонецЕсли;

// Отрицание с проверкой на литерал

А = Не Отказ = Ложь;
А = Не (Отказ = Ложь);
А = Не Отказ <> Ложь;
А = Не (Отказ <> Ложь);
А = Не НекотороеЗначение() <> Неопределено;
А = Не Неопределено <> НекотороеЗначение();
А = Не<> Неопределено); // срабатывает
А = Не А <> Неопределено И Б = 5; // срабатывает
А = Не<> Неопределено и Б = 5); // не срабатывает
А = Не<> Неопределено или Б = 5); // не срабатывает
А = Не= 5 и А <> Неопределено); // не срабатывает

Пока Не Таблица.Данные <> Неопределено Цикл
КонецЦикла;

Б = Не (Не А = 1 или Б <> Неопределено); // срабатывает на "Не А = 1"
Б = Не<> 1 или Не Б <> Неопределено); // срабатывает на "Не Б <> Неопределено"
Б = Не<> 1 или Не Б = Неопределено); // не срабатывает на "Не Б <> Неопределено" т.к. сравнения вида Не Х = Неопределено популярны

Если Не Т.Найти(Значение) = Неопределено Тогда
// не срабатывает, т.к. популярный код
КонецЕсли;

// Отрицание с проверкой на неравенство нелитералу

А = Не (Отказ <> НеЛитерал); // срабатывает
А = Не СложнаяФункция() <> НеЛитерал; // срабатывает

Б = Не= 1 или Б <> НеЛитерал); // не срабатывает

// Прямое двойное отрицание

Б = Не (Не Значение);

0 comments on commit c1d49f5

Please sign in to comment.