diff --git a/docs/diagnostics/UnusedLocalVariable.md b/docs/diagnostics/UnusedLocalVariable.md new file mode 100644 index 00000000000..f5a7ac41551 --- /dev/null +++ b/docs/diagnostics/UnusedLocalVariable.md @@ -0,0 +1,28 @@ +# Неиспользуемая локальная переменная (UnusedLocalVariable) + +| Тип | Поддерживаются
языки | Важность | Включена
по умолчанию | Время на
исправление (мин) | Тэги | +| :-: | :-: | :-: | :-: | :-: | :-: | +| `Дефект кода` | `BSL`
`OS` | `Важный` | `Да` | `1` | `brainoverload`
`badpractice` | + + +## Описание диагностики +Программные модули не должны иметь неиспользуемых переменных. + +Если локальная переменная объявлена, но не используется, это мертвый код, который следует удалить. +Это улучшит удобство обслуживания, поскольку разработчики не будут удивляться, для чего используется переменная. + +## Сниппеты + + +### Экранирование кода + +```bsl +// BSLLS:UnusedLocalVariable-off +// BSLLS:UnusedLocalVariable-on +``` + +### Параметр конфигурационного файла + +```json +"UnusedLocalVariable": false +``` diff --git a/docs/diagnostics/index.md b/docs/diagnostics/index.md index 09c6b1a555e..b946df0b40d 100644 --- a/docs/diagnostics/index.md +++ b/docs/diagnostics/index.md @@ -8,9 +8,9 @@ ## Список реализованных диагностик -Общее количество: **111** +Общее количество: **112** -* Дефект кода: **70** +* Дефект кода: **71** * Уязвимость: **3** * Ошибка: **34** * Потенциальная уязвимость: **4** @@ -112,6 +112,7 @@ | [UnreachableCode](UnreachableCode.md) | Недостижимый код | Да | Незначительный | Ошибка | `design`
`suspicious` | | [UnsafeSafeModeMethodCall](UnsafeSafeModeMethodCall.md) | Небезопасное использование функции БезопасныйРежим() | Да | Блокирующий | Ошибка | `deprecated`
`error` | | [UnusedLocalMethod](UnusedLocalMethod.md) | Неиспользуемый локальный метод | Да | Важный | Дефект кода | `standard`
`suspicious` | +| [UnusedLocalVariable](UnusedLocalVariable.md) | Неиспользуемая локальная переменная | Да | Важный | Дефект кода | `brainoverload`
`badpractice` | | [UnusedParameters](UnusedParameters.md) | Неиспользуемый параметр | Да | Важный | Дефект кода | `design` | | [UseLessForEach](UseLessForEach.md) | Бесполезный перебор коллекции | Да | Критичный | Ошибка | `clumsy` | | [UsingCancelParameter](UsingCancelParameter.md) | Работа с параметром "Отказ" | Да | Важный | Дефект кода | `standard`
`badpractice` | diff --git a/docs/en/diagnostics/UnusedLocalVariable.md b/docs/en/diagnostics/UnusedLocalVariable.md new file mode 100644 index 00000000000..7643d47b9c4 --- /dev/null +++ b/docs/en/diagnostics/UnusedLocalVariable.md @@ -0,0 +1,28 @@ +# Unused local variable (UnusedLocalVariable) + +| Type | Scope | Severity | Activated
by default | Minutes
to fix | Tags | +| :-: | :-: | :-: | :-: | :-: | :-: | +| `Code smell` | `BSL`
`OS` | `Major` | `Yes` | `1` | `brainoverload`
`badpractice` | + + +## Description +Unused local variables should be removed + +If a local variable is declared but not used, it is dead code and should be removed. +Doing so will improve maintainability because developers will not wonder what the variable is used for. + +## Snippets + + +### Diagnostic ignorance in code + +```bsl +// BSLLS:UnusedLocalVariable-off +// BSLLS:UnusedLocalVariable-on +``` + +### Parameter for config + +```json +"UnusedLocalVariable": false +``` diff --git a/docs/en/diagnostics/index.md b/docs/en/diagnostics/index.md index 627d965dc73..2a9a0f100f9 100644 --- a/docs/en/diagnostics/index.md +++ b/docs/en/diagnostics/index.md @@ -8,10 +8,10 @@ To escape individual sections of code or files from triggering diagnostics, you ## Implemented diagnostics -Total: **111** +Total: **112** * Error: **34** -* Code smell: **70** +* Code smell: **71** * Vulnerability: **3** * Security Hotspot: **4** @@ -112,6 +112,7 @@ Total: **111** | [UnreachableCode](UnreachableCode.md) | Unreachable Code | Yes | Minor | Error | `design`
`suspicious` | | [UnsafeSafeModeMethodCall](UnsafeSafeModeMethodCall.md) | Unsafe SafeMode method call | Yes | Blocker | Error | `deprecated`
`error` | | [UnusedLocalMethod](UnusedLocalMethod.md) | Unused local method | Yes | Major | Code smell | `standard`
`suspicious` | +| [UnusedLocalVariable](UnusedLocalVariable.md) | Unused local variable | Yes | Major | Code smell | `brainoverload`
`badpractice` | | [UnusedParameters](UnusedParameters.md) | Unused parameter | Yes | Major | Code smell | `design` | | [UseLessForEach](UseLessForEach.md) | Useless collection iteration | Yes | Critical | Error | `clumsy` | | [UsingCancelParameter](UsingCancelParameter.md) | Using parameter "Cancel" | Yes | Major | Code smell | `standard`
`badpractice` | diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java index b24204ab837..6edaf6abbbf 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java @@ -22,7 +22,9 @@ package com.github._1c_syntax.bsl.languageserver.context.computer; import com.github._1c_syntax.bsl.languageserver.context.DocumentContext; +import com.github._1c_syntax.bsl.languageserver.context.symbol.Usage; import com.github._1c_syntax.bsl.languageserver.context.symbol.VariableSymbol; +import com.github._1c_syntax.bsl.languageserver.context.symbol.VariableUsage; import com.github._1c_syntax.bsl.languageserver.context.symbol.variable.VariableDescription; import com.github._1c_syntax.bsl.languageserver.context.symbol.variable.VariableKind; import com.github._1c_syntax.bsl.languageserver.utils.Ranges; @@ -42,6 +44,9 @@ public class VariableSymbolComputer extends BSLParserBaseVisitor impl private final DocumentContext documentContext; private final List variables = new ArrayList<>(); + private ArrayList currentMethodParameters = new ArrayList<>(); + private Range currentMethodRange; + public VariableSymbolComputer(DocumentContext documentContext) { this.documentContext = documentContext; @@ -62,6 +67,21 @@ public ParseTree visitModuleVarDeclaration(BSLParser.ModuleVarDeclarationContext return ctx; } + @Override + public ParseTree visitSub(BSLParser.SubContext ctx) { + currentMethodRange = Ranges.create(ctx); + ParseTree tree = super.visitSub(ctx); + currentMethodRange = null; + currentMethodParameters.clear(); + return tree; + } + + @Override + public ParseTree visitParam(BSLParser.ParamContext ctx) { + currentMethodParameters.add(ctx.getText()); + return ctx; + } + @Override public ParseTree visitSubVarDeclaration(BSLParser.SubVarDeclarationContext ctx) { var symbol = createVariableSymbol(ctx, ctx.var_name(), false, VariableKind.LOCAL); @@ -70,6 +90,55 @@ public ParseTree visitSubVarDeclaration(BSLParser.SubVarDeclarationContext ctx) return ctx; } + @Override + public ParseTree visitLValue(BSLParser.LValueContext ctx) { + if (ctx.getChildCount() > 1) { + return ctx; + } + + if (notParameter(ctx.getText()) && notRegistered(ctx.getText())) { + VariableSymbol symbol = VariableSymbol.builder() + .name(ctx.getText()) + .range(Ranges.create(ctx)) + .variableNameRange(Ranges.create(ctx)) + .export(false) + .kind(VariableKind.LOCAL) + .description(createDescription(getTokenToSearchComments(ctx))) + .build(); + variables.add(symbol); + } + return ctx; + } + + @Override + public ParseTree visitCallStatement(BSLParser.CallStatementContext ctx) { + if(ctx.getStart().getType() == BSLParser.IDENTIFIER) { + findVariableSymbol(ctx.getStart().getText()).ifPresent(symbol -> { + Usage usage = VariableUsage.builder() + .range(Ranges.create(ctx.getStart())) + .kind(Usage.Kind.OBJECT) + .build(); + symbol.addUsage(usage); + }); + } + return super.visitCallStatement(ctx); + } + + @Override + public ParseTree visitComplexIdentifier(BSLParser.ComplexIdentifierContext ctx) { + if (ctx.getStart().getType() == BSLParser.IDENTIFIER) { + findVariableSymbol(ctx.getStart().getText()).ifPresent(symbol -> { + Usage usage = VariableUsage.builder() + .range(Ranges.create(ctx.getStart())) + .kind(Usage.Kind.OTHER) + .build(); + symbol.addUsage(usage); + }); + } + + return super.visitComplexIdentifier(ctx); + } + private VariableSymbol createVariableSymbol( BSLParserRuleContext ctx, BSLParser.Var_nameContext varName, @@ -134,4 +203,25 @@ private static Range getRangeForDescription(List tokens) { return Ranges.create(firstElement, lastElement); } + private boolean notParameter(String variableName) { + return !currentMethodParameters.contains(variableName); + } + + private boolean notRegistered(String variableName) { + return variables.stream() + .filter(v -> v.getKind() == VariableKind.MODULE + || currentMethodRange == null + || Ranges.containsRange(currentMethodRange, v.getRange())) + .noneMatch(v -> v.getName().equals(variableName)); + } + + private Optional findVariableSymbol(String variableName) { + return variables.stream() + .filter(v -> v.getKind() == VariableKind.MODULE + || currentMethodRange == null + || Ranges.containsRange(currentMethodRange, v.getRange())) + .filter(v -> v.getName().equals(variableName)) + .findFirst(); + } + } diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/Usage.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/Usage.java new file mode 100644 index 00000000000..b47bcc27c90 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/Usage.java @@ -0,0 +1,37 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright © 2018-2020 + * 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.context.symbol; + +import org.eclipse.lsp4j.Range; + +public interface Usage { + + Range getRange(); + + Kind getKind(); + + enum Kind { + OTHER, + OBJECT + } + +} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java index 9eea0ffbcc6..ba54e64f6b3 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java @@ -32,6 +32,7 @@ import lombok.experimental.NonFinal; import org.eclipse.lsp4j.Range; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -57,4 +58,10 @@ public class VariableSymbol implements Symbol { VariableKind kind; boolean export; Optional description; + + ArrayList usages = new ArrayList<>(); + + public void addUsage(Usage usage) { + usages.add(usage); + } } diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableUsage.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableUsage.java new file mode 100644 index 00000000000..654ff9c4242 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableUsage.java @@ -0,0 +1,35 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright © 2018-2020 + * 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.context.symbol; + +import lombok.Builder; +import lombok.Value; +import org.eclipse.lsp4j.Range; + +@Value +@Builder +public class VariableUsage implements Usage { + + Range range; + Kind kind; + +} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java new file mode 100644 index 00000000000..518e6eea5f7 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java @@ -0,0 +1,64 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright © 2018-2020 + * 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.symbol.VariableSymbol; +import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticInfo; +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.mdclasses.metadata.additional.ModuleType; + +@DiagnosticMetadata( + type = DiagnosticType.CODE_SMELL, + severity = DiagnosticSeverity.MAJOR, + minutesToFix = 1, + tags = { + DiagnosticTag.BRAINOVERLOAD, + DiagnosticTag.BADPRACTICE + }, + modules = { + ModuleType.CommandModule, + ModuleType.CommonModule, + ModuleType.ManagerModule, + ModuleType.ValueManagerModule, + ModuleType.SessionModule, + } +) +public class UnusedLocalVariableDiagnostic extends AbstractDiagnostic { + + public UnusedLocalVariableDiagnostic(DiagnosticInfo info) { + super(info); + } + + @Override + protected void check() { + documentContext + .getSymbolTree() + .getChildrenFlat(VariableSymbol.class) + .stream() + .filter(v -> v.getUsages().isEmpty()) + .filter(v -> !v.isExport()) + .forEach(v -> diagnosticStorage.addDiagnostic(v.getRange())); + } +} 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 e73a1d1552b..3203a586c69 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 @@ -1216,6 +1216,16 @@ "title": "Unused local method", "$id": "#/definitions/UnusedLocalMethod" }, + "UnusedLocalVariable": { + "description": "Unused local variable", + "default": true, + "type": [ + "boolean", + "object" + ], + "title": "Unused local variable", + "$id": "#/definitions/UnusedLocalVariable" + }, "UnusedParameters": { "description": "Unused parameter", "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 af050b93a31..d18c56ba348 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 @@ -308,6 +308,9 @@ "UnusedLocalMethod": { "$ref": "parameters-schema.json#/definitions/UnusedLocalMethod" }, + "UnusedLocalVariable": { + "$ref": "parameters-schema.json#/definitions/UnusedLocalVariable" + }, "UnusedParameters": { "$ref": "parameters-schema.json#/definitions/UnusedParameters" }, diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic_en.properties new file mode 100644 index 00000000000..f3cb9a175e8 --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic_en.properties @@ -0,0 +1,2 @@ +diagnosticMessage=Remove unused variable +diagnosticName=Unused local variable diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic_ru.properties new file mode 100644 index 00000000000..88c989b0ad0 --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic_ru.properties @@ -0,0 +1,2 @@ +diagnosticMessage=Удалите не используемую переменную +diagnosticName=Неиспользуемая локальная переменная diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnosticTest.java new file mode 100644 index 00000000000..9dd57e3f681 --- /dev/null +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnosticTest.java @@ -0,0 +1,49 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright © 2018-2020 + * 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; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static com.github._1c_syntax.bsl.languageserver.util.Assertions.assertThat; + +class UnusedLocalVariableDiagnosticTest extends AbstractDiagnosticTest { + UnusedLocalVariableDiagnosticTest() { + super(UnusedLocalVariableDiagnostic.class); + } + + @Test + void test() { + + List diagnostics = getDiagnostics(); + assertThat(diagnostics).hasSize(5); + assertThat(diagnostics, true) + .hasRange(0, 6, 0, 36) + .hasRange(14, 10, 14, 35) + .hasRange(14, 37, 14, 63) + .hasRange(19, 4, 19, 28) + .hasRange(41, 0, 41, 25); + + } +} diff --git a/src/test/resources/diagnostics/UnusedLocalVariableDiagnostic.bsl b/src/test/resources/diagnostics/UnusedLocalVariableDiagnostic.bsl new file mode 100644 index 00000000000..4c81fdcef53 --- /dev/null +++ b/src/test/resources/diagnostics/UnusedLocalVariableDiagnostic.bsl @@ -0,0 +1,44 @@ +Перем ПеременнаяМодуляНеИспользуемая; // Тут ошибка +Перем ПеременнаяМодуляНеИспользуемаяЭкспортная Экспорт; // Тут думаю ошибка не нужно, возможно ради поддержания интефейса +Перем ПеременнаяМодуляИспользуемая; // Тут без ошибок +Перем ПеременнаяМодуляИспользуемаяЭкспортная Экспорт; // Тут без ошибок + +Функция Первая() + + ПеременнаяМодуляИспользуемая = ДействиеСРезультатомЧисло(); + ДействиеСПараметром(ПеременнаяМодуляИспользуемая); + ДействиеСПараметром2(ПеременнаяМодуляИспользуемаяЭкспортная); + +КонецФункции + +Функция Вторая() + Перем ЛокальнаяБезИспользования, ТолькоСПрисвоениемЗначения, ЛокальнаяСИспользованием; + + ЛокальнаяСИспользованием = 40; + ТолькоСПрисвоениемЗначения = ВыполнитьДействие(ЛокальнаяСИспользованием); + ВПроцедуреИспользуемая = Проверка(); + ВПроцедуреНеИспользуемая = Проверка(); + + Если ВПроцедуреИспользуемая = Истина Тогда + + ТолькоСПрисвоениемЗначения = 39; + + КонецЕсли; + + ПеременнаяОбъектСИспользованием = Обработки.Проверка.Создать(); + ПеременнаяОбъектСИспользованием.Выполнить(); + + ВПроцедуреИспользуемая2 = Новый Файл(ОбъединитьПути(".", "test_versions.mxl")); + Ожидаем.Что(ВПроцедуреИспользуемая2.Существует(), "Файл отчета не был создан").ЭтоИстина(); + +КонецФункции + +Функция Третья(ЭтоПараметр) + + ЭтоПараметр = Новый Массив(); + +КонецФункции + +ВнеПроцедурНеИспользуемая = 30; +ВнеПроцедурИспользуемая = 40; +ДействиеСПараметром(ВнеПроцедурИспользуемая); \ No newline at end of file