diff --git a/docs/diagnostics/UselessTernaryOperator.md b/docs/diagnostics/UselessTernaryOperator.md new file mode 100644 index 00000000000..d02e8e27ef0 --- /dev/null +++ b/docs/diagnostics/UselessTernaryOperator.md @@ -0,0 +1,24 @@ +# Бесполезный тернарный оператор (UselessTernaryOperator) + + +## Описание диагностики +Размещение в тернарном операторе булевых констант "Истина" или "Ложь" указывает на плохую продуманность кода. + +## Примеры +Бессмысленные операторы + +```Bsl +А = ?(Б = 1, Истина, Ложь); +``` +```Bsl +А = ?(Б = 0, False, True); +``` + +Подозрительные операторы + +```Bsl +А = ?(Б = 1, True, Истина); +``` +```Bsl +А = ?(Б = 0, 0, False); +``` diff --git a/docs/en/diagnostics/UselessTernaryOperator.md b/docs/en/diagnostics/UselessTernaryOperator.md new file mode 100644 index 00000000000..47a5ce94609 --- /dev/null +++ b/docs/en/diagnostics/UselessTernaryOperator.md @@ -0,0 +1,24 @@ +# Useless ternary operator (UselessTernaryOperator) + + +## Description +The placement of Boolean constants "True" or "False" in the ternary operator indicates poor code thoughtfulness. + +## Examples +Useless operators + +```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); +``` 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 92ff349be38..2a8f473ed66 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 @@ -23,7 +23,6 @@ import com.github._1c_syntax.bsl.languageserver.context.symbol.SourceDefinedSymbol; import com.github._1c_syntax.bsl.languageserver.utils.Ranges; -import org.jspecify.annotations.Nullable; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.tree.ParseTree; @@ -32,6 +31,7 @@ import org.eclipse.lsp4j.DiagnosticCodeDescription; import org.eclipse.lsp4j.DiagnosticRelatedInformation; import org.eclipse.lsp4j.Range; +import org.jspecify.annotations.Nullable; import java.util.ArrayList; import java.util.List; @@ -74,6 +74,17 @@ protected void addDiagnostic(ParserRuleContext node) { ); } + protected void addDiagnostic(ParserRuleContext node, DiagnosticAdditionalData data) { + if (node.exception != null) { + return; + } + + addDiagnostic( + Ranges.create(node), + data + ); + } + protected void addDiagnostic(ParserRuleContext node, String diagnosticMessage) { if (node.exception != null) { return; @@ -98,6 +109,14 @@ protected void addDiagnostic(Range range) { ); } + protected void addDiagnostic(Range range, DiagnosticAdditionalData data) { + addDiagnostic( + range, + data, + diagnostic.getInfo().getMessage() + ); + } + protected void addDiagnostic(Range range, String diagnosticMessage) { addDiagnostic( range, @@ -106,6 +125,15 @@ protected void addDiagnostic(Range range, String diagnosticMessage) { ); } + protected void addDiagnostic(Range range, DiagnosticAdditionalData data, String diagnosticMessage) { + addDiagnostic( + range, + data, + diagnosticMessage, + null + ); + } + protected void addDiagnostic(Token token) { addDiagnostic( Ranges.create(token) @@ -202,6 +230,20 @@ public void addDiagnostic( String diagnosticMessage, @Nullable List relatedInformation ) { + addDiagnostic( + range, + null, + diagnosticMessage, + relatedInformation + ); + } + + public void addDiagnostic( + Range range, + @Nullable DiagnosticAdditionalData data, + String diagnosticMessage, + @Nullable List relatedInformation + ) { if (Ranges.isEmpty(range)) { return; @@ -210,6 +252,7 @@ public void addDiagnostic( diagnosticList.add(createDiagnostic( diagnostic, range, + data, diagnosticMessage, relatedInformation )); @@ -234,9 +277,20 @@ protected void addDiagnostic(SourceDefinedSymbol sourceDefinedSymbol) { addDiagnostic(sourceDefinedSymbol.getSelectionRange()); } + /** + * Создает доп данные для диагностики на основании строки + * + * @param string Некая строка для помещения в доп данные диагностики + * @return Допданные диагностики + */ + public static DiagnosticAdditionalData createAdditionalData(String string) { + return new DiagnosticAdditionalData(string); + } + private static Diagnostic createDiagnostic( BSLDiagnostic bslDiagnostic, Range range, + @Nullable DiagnosticAdditionalData data, String diagnosticMessage, @Nullable List relatedInformation ) { @@ -258,6 +312,21 @@ private static Diagnostic createDiagnostic( if (relatedInformation != null) { diagnostic.setRelatedInformation(relatedInformation); } + + if (data != null) { + diagnostic.setData(data); + } return diagnostic; } + + /** + * Служебный класс для хранения вспомогательной информации диагностики, которая может использоваться + * например в квикфиксах. + * Пока реализация примитивная под конкретную задачу + * + * @param string Некая строка + */ + public record DiagnosticAdditionalData(String string) { + + } } 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..420d8805176 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java @@ -0,0 +1,129 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2025 + * Alexey Sosnoviy , Nikita Fedkin 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.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 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; +import java.util.Optional; + +@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 { + + private static final int SKIPPED_RULE_INDEX = 0; + + @Override + public ParseTree visitTernaryOperator(BSLParser.TernaryOperatorContext ctx) { + var exp = ctx.expression(); + + if (exp != null && exp.size() >= 3) { + var condition = getBooleanToken(exp.get(0)); + var trueBranch = getBooleanToken(exp.get(1)); + var falseBranch = getBooleanToken(exp.get(2)); + + if (condition != SKIPPED_RULE_INDEX) { + diagnosticStorage.addDiagnostic(ctx); + } else if (trueBranch == BSLParser.TRUE && falseBranch == BSLParser.FALSE) { + diagnosticStorage.addDiagnostic(ctx, DiagnosticStorage.createAdditionalData(exp.get(0).getText())); + } else if (trueBranch == BSLParser.FALSE && falseBranch == BSLParser.TRUE) { + diagnosticStorage.addDiagnostic(ctx, + DiagnosticStorage.createAdditionalData(getAdaptedText(exp.get(0).getText()))); + } else if (trueBranch != SKIPPED_RULE_INDEX || falseBranch != SKIPPED_RULE_INDEX) { + 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 data = diagnostic.getData(); + if (data instanceof DiagnosticStorage.DiagnosticAdditionalData additionalData) { + var textEdit = new TextEdit(range, additionalData.string()); + textEdits.add(textEdit); + } + }); + + return CodeActionProvider.createCodeActions( + textEdits, + info.getResourceString("quickFixMessage"), + documentContext.getUri(), + diagnostics + ); + + } + + private String getAdaptedText(String text) { + return info.getResourceString("quickFixAdaptedText", text); + } + + private int getBooleanToken(BSLParser.ExpressionContext expCtx) { + + var tmpCtx = Optional.of(expCtx) + .filter(ctx -> ctx.children.size() == 1) + .map(ctx -> ctx.member(0)) + .map(ctx -> ctx.getChild(0)) + .filter(BSLParser.ConstValueContext.class::isInstance) + .map(BSLParser.ConstValueContext.class::cast); + + return tmpCtx + .map(ctx -> ctx.getToken(BSLParser.TRUE, 0)) + .map(ctx -> BSLParser.TRUE) + .or(() -> tmpCtx + .map(ctx -> ctx.getToken(BSLParser.FALSE, 0)) + .map(ctx -> BSLParser.FALSE) + ) + .orElse(SKIPPED_RULE_INDEX); + } +} 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 c44f41b0e97..377c1d5cbcd 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 @@ -2108,6 +2108,16 @@ "title": "Use of system information", "$id": "#/definitions/UseSystemInformation" }, + "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 4887be622ca..595516dd8d2 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 @@ -491,6 +491,9 @@ "UseSystemInformation": { "$ref": "parameters-schema.json#/definitions/UseSystemInformation" }, + "UselessTernaryOperator": { + "$ref": "parameters-schema.json#/definitions/UselessTernaryOperator" + }, "UsingCancelParameter": { "$ref": "parameters-schema.json#/definitions/UsingCancelParameter" }, @@ -924,15 +927,30 @@ "properties": { "type": { "type": "string", - "enum": ["ERROR", "CODE_SMELL", "VULNERABILITY", "SECURITY_HOTSPOT"] + "enum": [ + "ERROR", + "CODE_SMELL", + "VULNERABILITY", + "SECURITY_HOTSPOT" + ] }, "severity": { "type": "string", - "enum": ["INFO", "MINOR", "MAJOR", "CRITICAL", "BLOCKER"] + "enum": [ + "INFO", + "MINOR", + "MAJOR", + "CRITICAL", + "BLOCKER" + ] }, "scope": { "type": "string", - "enum": ["ALL", "BSL", "OS"] + "enum": [ + "ALL", + "BSL", + "OS" + ] }, "modules": { "type": "array", @@ -963,7 +981,12 @@ }, "lspSeverity": { "type": "string", - "enum": ["Error", "Warning", "Information", "Hint"], + "enum": [ + "Error", + "Warning", + "Information", + "Hint" + ], "description": "LSP severity level (Error, Warning, Information, Hint). If not specified, calculated automatically based on type and severity." } } 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..29034b3540b --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_en.properties @@ -0,0 +1,4 @@ +diagnosticMessage=Useless ternary operator +diagnosticName=Useless ternary operator +quickFixMessage=Fix some useless ternary operators +quickFixAdaptedText=NOT (%s) \ 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..f8bffd5612c --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic_ru.properties @@ -0,0 +1,4 @@ +diagnosticMessage=Бесполезный тернарный оператор +diagnosticName=Бесполезный тернарный оператор +quickFixMessage=Исправить некоторые бесполезные тернарные операторы +quickFixAdaptedText=НЕ (%s) \ 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..ffbf3d97c4b --- /dev/null +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java @@ -0,0 +1,86 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2025 + * Alexey Sosnoviy , Nikita Fedkin 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 org.eclipse.lsp4j.CodeAction; +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); + + } + + @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("НЕ (Б=0)"); + } + +} 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); + +// валидный +ОбластьМакета.Параметры.ДебетСубСчета = ОбластьМакета.Параметры.ДебетСубСчета + + ?(ПустаяСтрока(ОбластьМакета.Параметры.ДебетСубСчета), "", ", ") + + СчетДт;