From 732b5ac9fdde0c11e0e10c42f3ee97e277987818 Mon Sep 17 00:00:00 2001 From: EvgSychev Date: Thu, 16 Sep 2021 20:35:11 +0300 Subject: [PATCH 1/7] Add useless ternary operator diagnostic --- docs/diagnostics/UselessTernaryOperator.md | 47 ++++++++ docs/diagnostics/index.md | 5 +- docs/en/diagnostics/UselessTernaryOperator.md | 47 ++++++++ docs/en/diagnostics/index.md | 5 +- .../diagnostics/DiagnosticStorage.java | 84 ++++++------- .../UselessTernaryOperatorDiagnostic.java | 110 ++++++++++++++++++ ...essTernaryOperatorDiagnostic_en.properties | 3 + ...essTernaryOperatorDiagnostic_ru.properties | 3 + .../UselessTernaryOperatorDiagnosticTest.java | 33 ++++++ .../UselessTernaryOperatorDiagnostic.bsl | 14 +++ 10 files changed, 308 insertions(+), 43 deletions(-) create mode 100644 docs/diagnostics/UselessTernaryOperator.md create mode 100644 docs/en/diagnostics/UselessTernaryOperator.md create mode 100644 src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java create mode 100644 src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_en.properties create mode 100644 src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_ru.properties create mode 100644 src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java create mode 100644 src/test/resources/diagnostics/UselessTernaryOperatorDiagnostic.bsl diff --git a/docs/diagnostics/UselessTernaryOperator.md b/docs/diagnostics/UselessTernaryOperator.md new file mode 100644 index 00000000000..3bd335b8982 --- /dev/null +++ b/docs/diagnostics/UselessTernaryOperator.md @@ -0,0 +1,47 @@ +# Бесполезный тернарный оператор (UselessTernaryOperator) + +| Тип | Поддерживаются
языки | Важность | Включена
по умолчанию | Время на
исправление (мин) | Теги | +|:-------------:|:-----------------------------:|:----------------:|:------------------------------:|:-----------------------------------:|:-----------------------------------:| +| `Дефект кода` | `BSL` | `Информационный` | `Да` | `1` | `badpractice`
`suspicious` | + + +## Описание диагностики +Размещение в тернарном операторе булевых констант "Истина" или "Ложь" указывает на плохую продуманность кода. + +## Примеры +Бессмысленные операторы + +```Bsl +А = ?(Истина, 1, 0); +``` +```Bsl +А = ?(Б = 1, Истина, Ложь); +``` +```Bsl +А = ?(Б = 0, False, True); +``` + +Подозрительные операторы + +```Bsl +А = ?(Б = 1, True, Истина); +``` +```Bsl +А = ?(Б = 0, 0, False); +``` + +## Сниппеты + + +### Экранирование кода + +```bsl +// BSLLS:UselessTernaryOperator-off +// BSLLS:UselessTernaryOperator-on +``` + +### Параметр конфигурационного файла + +```json +"UselessTernaryOperator": false +``` diff --git a/docs/diagnostics/index.md b/docs/diagnostics/index.md index 1b41768f33c..55e8b3e70ca 100644 --- a/docs/diagnostics/index.md +++ b/docs/diagnostics/index.md @@ -8,12 +8,12 @@ ## Список реализованных диагностик -Общее количество: **145** +Общее количество: **146** * Потенциальная уязвимость: **4** * Уязвимость: **4** * Ошибка: **44** -* Дефект кода: **93** +* Дефект кода: **94** | Ключ | Название | Включена по умолчанию | Важность | Тип | Тэги | @@ -146,6 +146,7 @@ [UnusedParameters](UnusedParameters.md) | Неиспользуемый параметр | Да | Важный | Дефект кода | `design`
`unused` [UsageWriteLogEvent](UsageWriteLogEvent.md) | Неверное использование метода "ЗаписьЖурналаРегистрации" | Да | Информационный | Дефект кода | `standard`
`badpractice` [UseLessForEach](UseLessForEach.md) | Бесполезный перебор коллекции | Да | Критичный | Ошибка | `clumsy` + [UselessTernaryOperator](UselessTernaryOperator.md) | Бесполезный тернарный оператор | Да | Информационный | Дефект кода | `badpractice`
`suspicious` [UsingCancelParameter](UsingCancelParameter.md) | Работа с параметром "Отказ" | Да | Важный | Дефект кода | `standard`
`badpractice` [UsingExternalCodeTools](UsingExternalCodeTools.md) | Использование возможностей выполнения внешнего кода | Да | Критичный | Потенциальная уязвимость | `standard`
`design` [UsingFindElementByString](UsingFindElementByString.md) | Использование методов "НайтиПоНаименованию" и "НайтиПоКоду" | Да | Важный | Дефект кода | `standard`
`badpractice`
`performance` diff --git a/docs/en/diagnostics/UselessTernaryOperator.md b/docs/en/diagnostics/UselessTernaryOperator.md new file mode 100644 index 00000000000..37d47671a28 --- /dev/null +++ b/docs/en/diagnostics/UselessTernaryOperator.md @@ -0,0 +1,47 @@ +# Useless ternary operator (UselessTernaryOperator) + +| Type | Scope | Severity | Activated
by default | Minutes
to fix | Tags | +|:------------:|:-----:|:--------:|:-----------------------------:|:-----------------------:|:-----------------------------------:| +| `Code smell` | `BSL` | `Info` | `Yes` | `1` | `badpractice`
`suspicious` | + + +## Description +The placement of Boolean constants "True" or "False" in the ternary operator indicates poor code thoughtfulness. + +## Examples +Useless operators + +```Bsl +A = ?(True, 1, 0); +``` +```Bsl +A = ?(B = 1, True, False); +``` +```Bsl +A = ?(B = 0, False, True); +``` + +Suspicious operators + +```Bsl +A = ?(B = 1, True, True); +``` +```Bsl +A = ?(B = 0, 0, False); +``` + +## Snippets + + +### Diagnostic ignorance in code + +```bsl +// BSLLS:UselessTernaryOperator-off +// BSLLS:UselessTernaryOperator-on +``` + +### Parameter for config + +```json +"UselessTernaryOperator": false +``` diff --git a/docs/en/diagnostics/index.md b/docs/en/diagnostics/index.md index 4427c56ec4f..d1293662f82 100644 --- a/docs/en/diagnostics/index.md +++ b/docs/en/diagnostics/index.md @@ -8,12 +8,12 @@ To escape individual sections of code or files from triggering diagnostics, you ## Implemented diagnostics -Total: **145** +Total: **146** * Security Hotspot: **4** * Vulnerability: **4** * Error: **44** -* Code smell: **93** +* Code smell: **94** | Key | Name| Enabled by default | Severity | Type | Tags | @@ -146,6 +146,7 @@ Total: **145** [UnusedParameters](UnusedParameters.md) | Unused parameter | Yes | Major | Code smell | `design`
`unused` [UsageWriteLogEvent](UsageWriteLogEvent.md) | Incorrect use of the method | Yes | Info | Code smell | `standard`
`badpractice` [UseLessForEach](UseLessForEach.md) | Useless collection iteration | Yes | Critical | Error | `clumsy` + [UselessTernaryOperator](UselessTernaryOperator.md) | Useless ternary operator | Yes | Info | Code smell | `badpractice`
`suspicious` [UsingCancelParameter](UsingCancelParameter.md) | Using parameter "Cancel" | Yes | Major | Code smell | `standard`
`badpractice` [UsingExternalCodeTools](UsingExternalCodeTools.md) | Using external code tools | Yes | Critical | Security Hotspot | `standard`
`design` [UsingFindElementByString](UsingFindElementByString.md) | Using FindByName and FindByCode | Yes | Major | Code smell | `standard`
`badpractice`
`performance` diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java index 48d63b9e798..9f23e570dac 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java @@ -34,6 +34,7 @@ import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; @@ -56,150 +57,155 @@ public void clearDiagnostics() { diagnosticList.clear(); } - protected void addDiagnostic(BSLParserRuleContext node) { + protected Optional addDiagnostic(BSLParserRuleContext node) { if (node.exception != null) { - return; + return Optional.empty(); } - addDiagnostic( + return addDiagnostic( Ranges.create(node) ); } - protected void addDiagnostic(BSLParserRuleContext node, String diagnosticMessage) { + protected Optional addDiagnostic(BSLParserRuleContext node, String diagnosticMessage) { if (node.exception != null) { - return; + return Optional.empty(); } - addDiagnostic( + return addDiagnostic( Ranges.create(node), diagnosticMessage ); } - protected void addDiagnostic(int startLine, int startChar, int endLine, int endChar) { - addDiagnostic( + protected Optional addDiagnostic(int startLine, int startChar, int endLine, int endChar) { + return addDiagnostic( Ranges.create(startLine, startChar, endLine, endChar) ); } - protected void addDiagnostic(Range range) { - addDiagnostic( + protected Optional addDiagnostic(Range range) { + return addDiagnostic( range, diagnostic.getInfo().getMessage() ); } - protected void addDiagnostic(Range range, String diagnosticMessage) { - addDiagnostic( + protected Optional addDiagnostic(Range range, String diagnosticMessage) { + return addDiagnostic( range, diagnosticMessage, null ); } - protected void addDiagnostic(Token token) { - addDiagnostic( + protected Optional addDiagnostic(Token token) { + return addDiagnostic( Ranges.create(token) ); } - protected void addDiagnostic(Token startToken, Token endToken) { - addDiagnostic( + protected Optional addDiagnostic(Token startToken, Token endToken) { + return addDiagnostic( Ranges.create(startToken, endToken) ); } - protected void addDiagnostic(Token token, String diagnosticMessage) { - addDiagnostic( + protected Optional addDiagnostic(Token token, String diagnosticMessage) { + return addDiagnostic( Ranges.create(token), diagnosticMessage ); } - protected void addDiagnostic(TerminalNode terminalNode) { - addDiagnostic(terminalNode.getSymbol()); + protected Optional addDiagnostic(TerminalNode terminalNode) { + return addDiagnostic(terminalNode.getSymbol()); } - protected void addDiagnostic(TerminalNode terminalNode, String diagnosticMessage) { - addDiagnostic(terminalNode.getSymbol(), diagnosticMessage); + protected Optional addDiagnostic(TerminalNode terminalNode, String diagnosticMessage) { + return addDiagnostic(terminalNode.getSymbol(), diagnosticMessage); } - protected void addDiagnostic(TerminalNode startTerminalNode, TerminalNode stopTerminalNode) { - addDiagnostic(startTerminalNode.getSymbol(), stopTerminalNode.getSymbol()); + protected Optional addDiagnostic(TerminalNode startTerminalNode, TerminalNode stopTerminalNode) { + return addDiagnostic(startTerminalNode.getSymbol(), stopTerminalNode.getSymbol()); } - protected void addDiagnostic(BSLParserRuleContext node, List relatedInformation) { + protected Optional addDiagnostic(BSLParserRuleContext node, List relatedInformation) { if (node.exception != null) { - return; + return Optional.empty(); } - addDiagnostic( + return addDiagnostic( node, diagnostic.getInfo().getMessage(), relatedInformation ); } - public void addDiagnostic(Token token, List relatedInformation) { - addDiagnostic( + public Optional addDiagnostic(Token token, List relatedInformation) { + return addDiagnostic( token, diagnostic.getInfo().getMessage(), relatedInformation ); } - public void addDiagnostic( + public Optional addDiagnostic( BSLParserRuleContext node, String diagnosticMessage, List relatedInformation ) { if (node.exception != null) { - return; + return Optional.empty(); } - addDiagnostic( + return addDiagnostic( Ranges.create(node), diagnosticMessage, relatedInformation ); } - public void addDiagnostic( + public Optional addDiagnostic( Token token, String diagnosticMessage, List relatedInformation ) { - addDiagnostic( + return addDiagnostic( Ranges.create(token), diagnosticMessage, relatedInformation ); } - public void addDiagnostic( + public Optional addDiagnostic( Range range, List relatedInformation ) { - addDiagnostic( + return addDiagnostic( range, diagnostic.getInfo().getMessage(), relatedInformation ); } - public void addDiagnostic( + public Optional addDiagnostic( Range range, String diagnosticMessage, @Nullable List relatedInformation ) { - diagnosticList.add(createDiagnostic( + + var dgs = createDiagnostic( diagnostic, range, diagnosticMessage, relatedInformation - )); + ); + + diagnosticList.add(dgs); + + return Optional.of(dgs); } public void addDiagnostic(ParseTree tree) { diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java new file mode 100644 index 00000000000..e03f233b2e6 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -0,0 +1,110 @@ +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.DiagnosticMetadata; +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticScope; +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.providers.CodeActionProvider; +import com.github._1c_syntax.bsl.parser.BSLParser; +import com.github._1c_syntax.mdclasses.mdo.MDLanguage; +import org.antlr.v4.runtime.tree.ParseTree; +import org.eclipse.lsp4j.CodeAction; +import org.eclipse.lsp4j.CodeActionParams; +import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.TextEdit; + +import java.util.ArrayList; +import java.util.List; + +@DiagnosticMetadata( + type = DiagnosticType.CODE_SMELL, + severity = DiagnosticSeverity.INFO, + scope = DiagnosticScope.BSL, + minutesToFix = 1, + tags = { + DiagnosticTag.BADPRACTICE, + DiagnosticTag.SUSPICIOUS + } + +) +public class UselessTernaryOperatorDiagnostic extends AbstractVisitorDiagnostic implements QuickFixProvider { + + @Override + public ParseTree visitTernaryOperator(BSLParser.TernaryOperatorContext ctx){ + + var emptyToken = 0; + var exp = ctx.expression(); + var condition = getBooleanToken(exp.get(0)); + var trueBranch = getBooleanToken(exp.get(1)); + var falseBranch = getBooleanToken(exp.get(2)); + + if (condition != emptyToken) { + diagnosticStorage.addDiagnostic(ctx); + } else if (trueBranch == BSLParser.TRUE && falseBranch == BSLParser.FALSE){ + var dgs = diagnosticStorage.addDiagnostic(ctx); + dgs.ifPresent(diagnostic -> diagnostic.setData(exp.get(0).getText())); + } else if (trueBranch == BSLParser.FALSE && falseBranch == BSLParser.TRUE){ + var dgs = diagnosticStorage.addDiagnostic(ctx); + if (dgs.isPresent()) { + if(documentContext.getServerContext().getConfiguration().getDefaultLanguage() == MDLanguage.ENGLISH) { + dgs.get().setData("NOT (" + exp.get(0).getText() + ")"); + } else { + dgs.get().setData("НЕ (" + exp.get(0).getText() + ")"); + } + } + } else if (trueBranch != emptyToken){ + diagnosticStorage.addDiagnostic(ctx); + } else if (falseBranch != emptyToken){ + diagnosticStorage.addDiagnostic(ctx); + } + + return super.visitTernaryOperator(ctx); + } + + @Override + public List getQuickFixes( + List diagnostics, + CodeActionParams params, + DocumentContext documentContext + ) { + + List textEdits = new ArrayList<>(); + + diagnostics.forEach((Diagnostic diagnostic) -> { + var range = diagnostic.getRange(); + var textEdit = new TextEdit(range,(String) diagnostic.getData()); + textEdits.add(textEdit); + }); + + return CodeActionProvider.createCodeActions( + textEdits, + info.getResourceString("quickFixMessage"), + documentContext.getUri(), + diagnostics + ); + + } + + private int getBooleanToken(BSLParser.ExpressionContext expCtx){ + + if (expCtx.children.size() == 1){ + var mmbCtx = (BSLParser.MemberContext) expCtx.getChild(0); + var parseTree = mmbCtx.getChild(0); + if (parseTree instanceof BSLParser.ConstValueContext){ + var constValue = (BSLParser.ConstValueContext) parseTree; + var tokenTrue = constValue.getToken(BSLParser.TRUE, 0); + if (tokenTrue != null) { + return BSLParser.TRUE; + } + var tokenFalse = constValue.getToken(BSLParser.FALSE, 0); + if (tokenFalse != null) { + return BSLParser.FALSE; + } + } + } + return 0; + } + +} diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_en.properties new file mode 100644 index 00000000000..28d413d8b06 --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_en.properties @@ -0,0 +1,3 @@ +diagnosticMessage=Useless ternary operator +diagnosticName=Useless ternary operator +quickFixMessage=Fix some useless ternary operators \ No newline at end of file diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_ru.properties new file mode 100644 index 00000000000..250bcee805a --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_ru.properties @@ -0,0 +1,3 @@ +diagnosticMessage=Бесполезный тернарный оператор +diagnosticName=Бесполезный тернарный оператор +quickFixMessage=Исправить некоторые бесполезные тернарные операторы \ No newline at end of file diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java new file mode 100644 index 00000000000..4869b146c96 --- /dev/null +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java @@ -0,0 +1,33 @@ +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 UselessTernaryOperatorDiagnosticTest extends AbstractDiagnosticTest { + + UselessTernaryOperatorDiagnosticTest() { + super(UselessTernaryOperatorDiagnostic.class); + } + + @Test + void test() { + + List diagnostics = getDiagnostics(); + + assertThat(diagnostics).hasSize(8); + assertThat(diagnostics, true) + .hasRange(1, 4, 1, 26) + .hasRange(2, 4, 2, 25) + .hasRange(3, 4, 3, 26) + .hasRange(4, 4, 4, 25) + .hasRange(5, 4, 5, 21) + .hasRange(6, 4, 6, 22) + .hasRange(7, 4, 7, 19) + .hasRange(8, 4, 8, 18); + + } +} diff --git a/src/test/resources/diagnostics/UselessTernaryOperatorDiagnostic.bsl b/src/test/resources/diagnostics/UselessTernaryOperatorDiagnostic.bsl new file mode 100644 index 00000000000..21c447838b3 --- /dev/null +++ b/src/test/resources/diagnostics/UselessTernaryOperatorDiagnostic.bsl @@ -0,0 +1,14 @@ +// Бессмысленные тернарники +А = ?(Б = 1, Истина, Ложь);// прямой, фиксится в А = Б = 1; +А = ?(Б = 0, False, True);// обратный, фиксится в А = НЕ (Б = 0); +А = ?(Б = 1, True, Истина); +А = ?(Б = 0, Ложь, False); +А = ?(Б = 1, True, 1); +А = ?(Б = 0, 0, False); +А = ?(истина, 1, 0); +А = ?(false, 0, 1); + +// валидный +ОбластьМакета.Параметры.ДебетСубСчета = ОбластьМакета.Параметры.ДебетСубСчета + + ?(ПустаяСтрока(ОбластьМакета.Параметры.ДебетСубСчета), "", ", ") + + СчетДт; From f103c75439cb3a1a8cc7ca569d033f02f04e3021 Mon Sep 17 00:00:00 2001 From: EvgSychev Date: Sat, 18 Sep 2021 08:20:44 +0300 Subject: [PATCH 2/7] precommit done --- .../UselessTernaryOperatorDiagnostic.java | 21 +++++++++++++++++++ .../configuration/parameters-schema.json | 10 +++++++++ .../languageserver/configuration/schema.json | 3 +++ .../UselessTernaryOperatorDiagnosticTest.java | 21 +++++++++++++++++++ 4 files changed, 55 insertions(+) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java index e03f233b2e6..96e44efe53b 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -1,3 +1,24 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2021 + * Alexey Sosnoviy , Nikita Gryzlov 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; diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json index 3bc5a8c04f7..05dc15cc911 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json @@ -1676,6 +1676,16 @@ "title": "Useless collection iteration", "$id": "#/definitions/UseLessForEach" }, + "UselessTernaryOperator": { + "description": "Useless ternary operator", + "default": true, + "type": [ + "boolean", + "object" + ], + "title": "Useless ternary operator", + "$id": "#/definitions/UselessTernaryOperator" + }, "UsingCancelParameter": { "description": "Using parameter \"Cancel\"", "default": true, diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json index 986f3105d67..78852847537 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json @@ -407,6 +407,9 @@ "UseLessForEach": { "$ref": "parameters-schema.json#/definitions/UseLessForEach" }, + "UselessTernaryOperator": { + "$ref": "parameters-schema.json#/definitions/UselessTernaryOperator" + }, "UsingCancelParameter": { "$ref": "parameters-schema.json#/definitions/UsingCancelParameter" }, diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java index 4869b146c96..805f53fd17e 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java @@ -1,3 +1,24 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2021 + * Alexey Sosnoviy , Nikita Gryzlov 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 org.eclipse.lsp4j.Diagnostic; From cd8ba4dc613555ddeba91a9ef1eebd9c717b2429 Mon Sep 17 00:00:00 2001 From: EvgSychev Date: Tue, 21 Sep 2021 21:48:40 +0300 Subject: [PATCH 3/7] suggested changes have been made --- .../UselessTernaryOperatorDiagnostic.java | 61 ++++++++++--------- .../UselessTernaryOperatorDiagnosticTest.java | 32 ++++++++++ 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java index 96e44efe53b..627aee33710 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; @DiagnosticMetadata( type = DiagnosticType.CODE_SMELL, @@ -52,32 +53,25 @@ ) public class UselessTernaryOperatorDiagnostic extends AbstractVisitorDiagnostic implements QuickFixProvider { + private static final int SKIPPED_RULE_INDEX = 0; + @Override public ParseTree visitTernaryOperator(BSLParser.TernaryOperatorContext ctx){ - var emptyToken = 0; var exp = ctx.expression(); var condition = getBooleanToken(exp.get(0)); var trueBranch = getBooleanToken(exp.get(1)); var falseBranch = getBooleanToken(exp.get(2)); - if (condition != emptyToken) { + if (condition != SKIPPED_RULE_INDEX) { diagnosticStorage.addDiagnostic(ctx); } else if (trueBranch == BSLParser.TRUE && falseBranch == BSLParser.FALSE){ var dgs = diagnosticStorage.addDiagnostic(ctx); dgs.ifPresent(diagnostic -> diagnostic.setData(exp.get(0).getText())); } else if (trueBranch == BSLParser.FALSE && falseBranch == BSLParser.TRUE){ var dgs = diagnosticStorage.addDiagnostic(ctx); - if (dgs.isPresent()) { - if(documentContext.getServerContext().getConfiguration().getDefaultLanguage() == MDLanguage.ENGLISH) { - dgs.get().setData("NOT (" + exp.get(0).getText() + ")"); - } else { - dgs.get().setData("НЕ (" + exp.get(0).getText() + ")"); - } - } - } else if (trueBranch != emptyToken){ - diagnosticStorage.addDiagnostic(ctx); - } else if (falseBranch != emptyToken){ + dgs.ifPresent(diagnostic -> diagnostic.setData(getAdaptedText(exp.get(0).getText()))); + } else if (trueBranch != SKIPPED_RULE_INDEX || falseBranch != SKIPPED_RULE_INDEX){ diagnosticStorage.addDiagnostic(ctx); } @@ -108,24 +102,33 @@ public List getQuickFixes( } - private int getBooleanToken(BSLParser.ExpressionContext expCtx){ - - if (expCtx.children.size() == 1){ - var mmbCtx = (BSLParser.MemberContext) expCtx.getChild(0); - var parseTree = mmbCtx.getChild(0); - if (parseTree instanceof BSLParser.ConstValueContext){ - var constValue = (BSLParser.ConstValueContext) parseTree; - var tokenTrue = constValue.getToken(BSLParser.TRUE, 0); - if (tokenTrue != null) { - return BSLParser.TRUE; - } - var tokenFalse = constValue.getToken(BSLParser.FALSE, 0); - if (tokenFalse != null) { - return BSLParser.FALSE; - } - } + private String getAdaptedText(String text) { + if(documentContext.getServerContext().getConfiguration().getDefaultLanguage() == MDLanguage.ENGLISH) { + return "NOT (" + text + ")"; + } else { + return "НЕ (" + text + ")"; } - return 0; } + private int getBooleanToken(BSLParser.ExpressionContext expCtx){ + + return Optional.of(expCtx) + .filter(ctx -> ctx.children.size() == 1) + .map(ctx -> (BSLParser.MemberContext) ctx.getChild(0)) + .map(ctx -> ctx.getChild(0)) + .filter(BSLParser.ConstValueContext.class::isInstance) + .map(BSLParser.ConstValueContext.class::cast) + .map(ctx -> ctx.getToken(BSLParser.TRUE, 0)) + .map(ctx -> BSLParser.TRUE) + .or(() -> Optional.of(expCtx) + .filter(ctx -> ctx.children.size() == 1) + .map(ctx -> (BSLParser.MemberContext) ctx.getChild(0)) + .map(ctx -> ctx.getChild(0)) + .filter(BSLParser.ConstValueContext.class::isInstance) + .map(BSLParser.ConstValueContext.class::cast) + .map(ctx -> ctx.getToken(BSLParser.FALSE, 0)) + .map(ctx -> BSLParser.FALSE) + ) + .orElse(SKIPPED_RULE_INDEX); + } } diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java index 805f53fd17e..7680c07b506 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java @@ -21,6 +21,8 @@ */ package com.github._1c_syntax.bsl.languageserver.diagnostics; +import com.github._1c_syntax.bsl.languageserver.context.DocumentContext; +import org.eclipse.lsp4j.CodeAction; import org.eclipse.lsp4j.Diagnostic; import org.junit.jupiter.api.Test; @@ -51,4 +53,34 @@ void test() { .hasRange(8, 4, 8, 18); } + + @Test + void testQuickFix() { + + final DocumentContext documentContext = getDocumentContext(); + List diagnostics = getDiagnostics(); + + final Diagnostic directDiagnostic = diagnostics.get(0); + List directQuickFixes = getQuickFixes(directDiagnostic); + assertThat(directQuickFixes).hasSize(1); + final CodeAction directQuickFix = directQuickFixes.get(0); + assertThat(directQuickFix) + .of(diagnosticInstance) + .in(documentContext) + .fixes(directDiagnostic) + .hasChanges(1) + .hasNewText("Б=1"); + + final Diagnostic reversDiagnostic = diagnostics.get(1); + List reversQuickFixes = getQuickFixes(reversDiagnostic); + assertThat(reversQuickFixes).hasSize(1); + final CodeAction reversQuickFix = reversQuickFixes.get(0); + assertThat(reversQuickFix) + .of(diagnosticInstance) + .in(documentContext) + .fixes(reversDiagnostic) + .hasChanges(1) + .hasNewText("NOT (Б=0)"); + } + } From 541b137c70db63e3d0169918d820c61ed44c3f6e Mon Sep 17 00:00:00 2001 From: EvgSychev Date: Thu, 23 Sep 2021 20:12:34 +0300 Subject: [PATCH 4/7] Update UselessTernaryOperatorDiagnostic.java --- .../UselessTernaryOperatorDiagnostic.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java index 627aee33710..c97531eab95 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -112,21 +112,20 @@ private String getAdaptedText(String text) { private int getBooleanToken(BSLParser.ExpressionContext expCtx){ - return Optional.of(expCtx) + var tmpCtx = Optional.of(expCtx) .filter(ctx -> ctx.children.size() == 1) .map(ctx -> (BSLParser.MemberContext) ctx.getChild(0)) .map(ctx -> ctx.getChild(0)) .filter(BSLParser.ConstValueContext.class::isInstance) - .map(BSLParser.ConstValueContext.class::cast) - .map(ctx -> ctx.getToken(BSLParser.TRUE, 0)) + .map(BSLParser.ConstValueContext.class::cast); + + return Optional.of(tmpCtx) + .filter(Optional::isPresent) + .map(ctx -> ctx.get().getToken(BSLParser.TRUE, 0)) .map(ctx -> BSLParser.TRUE) - .or(() -> Optional.of(expCtx) - .filter(ctx -> ctx.children.size() == 1) - .map(ctx -> (BSLParser.MemberContext) ctx.getChild(0)) - .map(ctx -> ctx.getChild(0)) - .filter(BSLParser.ConstValueContext.class::isInstance) - .map(BSLParser.ConstValueContext.class::cast) - .map(ctx -> ctx.getToken(BSLParser.FALSE, 0)) + .or(() -> Optional.of(tmpCtx) + .filter(Optional::isPresent) + .map(ctx -> ctx.get().getToken(BSLParser.FALSE, 0)) .map(ctx -> BSLParser.FALSE) ) .orElse(SKIPPED_RULE_INDEX); From 811023ae7fdec2132af3c871c69ff3603d0f162c Mon Sep 17 00:00:00 2001 From: EvgSychev Date: Sat, 25 Sep 2021 08:57:11 +0300 Subject: [PATCH 5/7] Update UselessTernaryOperatorDiagnostic.java --- .../diagnostics/UselessTernaryOperatorDiagnostic.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java index c97531eab95..ef098707008 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -119,13 +119,11 @@ private int getBooleanToken(BSLParser.ExpressionContext expCtx){ .filter(BSLParser.ConstValueContext.class::isInstance) .map(BSLParser.ConstValueContext.class::cast); - return Optional.of(tmpCtx) - .filter(Optional::isPresent) - .map(ctx -> ctx.get().getToken(BSLParser.TRUE, 0)) + return tmpCtx + .map(ctx -> ctx.getToken(BSLParser.TRUE, 0)) .map(ctx -> BSLParser.TRUE) - .or(() -> Optional.of(tmpCtx) - .filter(Optional::isPresent) - .map(ctx -> ctx.get().getToken(BSLParser.FALSE, 0)) + .or(() -> tmpCtx + .map(ctx -> ctx.getToken(BSLParser.FALSE, 0)) .map(ctx -> BSLParser.FALSE) ) .orElse(SKIPPED_RULE_INDEX); From 0c78fb21087d679d9905fa3b10a62cbbcfc6fa36 Mon Sep 17 00:00:00 2001 From: Nikita Gryzlov Date: Thu, 7 Oct 2021 00:27:37 +0300 Subject: [PATCH 6/7] Apply suggestions from code review --- .../bsl/languageserver/diagnostics/DiagnosticStorage.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java index 9f23e570dac..9bf9f20ff23 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DiagnosticStorage.java @@ -196,16 +196,16 @@ public Optional addDiagnostic( @Nullable List relatedInformation ) { - var dgs = createDiagnostic( + var diagnostic = createDiagnostic( diagnostic, range, diagnosticMessage, relatedInformation ); - diagnosticList.add(dgs); + diagnosticList.add(diagnostic); - return Optional.of(dgs); + return Optional.of(diagnostic); } public void addDiagnostic(ParseTree tree) { From bac62f77db1a46781238fde32a3c4234db548057 Mon Sep 17 00:00:00 2001 From: Nikita Gryzlov Date: Thu, 7 Oct 2021 00:44:43 +0300 Subject: [PATCH 7/7] Update src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java --- .../diagnostics/UselessTernaryOperatorDiagnostic.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java index ef098707008..3cbdacdc18b 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -89,7 +89,7 @@ public List getQuickFixes( diagnostics.forEach((Diagnostic diagnostic) -> { var range = diagnostic.getRange(); - var textEdit = new TextEdit(range,(String) diagnostic.getData()); + var textEdit = new TextEdit(range, (String) diagnostic.getData()); textEdits.add(textEdit); });