Skip to content

Commit

Permalink
Merge pull request #3308 from EvilBeaver/feature/double-negations-3271
Browse files Browse the repository at this point in the history
Реализация #3271: Диагностика двойных отрицаний
  • Loading branch information
nixel2007 authored Aug 20, 2024
2 parents ba439b2 + 2dff72e commit 7a7ce89
Show file tree
Hide file tree
Showing 20 changed files with 617 additions and 133 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,123 @@
/*
* 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 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<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 BSLParserBaseVisitor<ParseTree> {

@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;
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* 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;

@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().<UnaryOperationNode>cast();
if (unaryParent.getOperator() == BslOperator.NOT) {
addDiagnostic(node);
}
}

super.visitUnaryOperation(node);
}

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

0 comments on commit 7a7ce89

Please sign in to comment.