From 6aedb8a193292c99bccd16c1452aecc17467d2ff Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Thu, 9 Jun 2022 22:15:38 +0300 Subject: [PATCH 01/45] =?UTF-8?q?=D0=B7=D0=B0=D0=B3=D0=BE=D1=82=D0=BE?= =?UTF-8?q?=D0=B2=D0=BA=D0=B8=20=D0=BF=D1=80=D0=B0=D0=B2=D0=B8=D0=BB=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 16 ++ docs/en/diagnostics/LostVariable.md | 16 ++ .../diagnostics/LostVariableDiagnostic.java | 256 ++++++++++++++++++ .../LostVariableDiagnostic_en.properties | 2 + .../LostVariableDiagnostic_ru.properties | 2 + .../LostVariableDiagnosticTest.java | 33 +++ .../diagnostics/LostVariableDiagnostic.bsl | 72 +++++ 7 files changed, 397 insertions(+) create mode 100644 docs/diagnostics/LostVariable.md create mode 100644 docs/en/diagnostics/LostVariable.md create mode 100644 src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java create mode 100644 src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties create mode 100644 src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties create mode 100644 src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java create mode 100644 src/test/resources/diagnostics/LostVariableDiagnostic.bsl diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md new file mode 100644 index 00000000000..717340aa793 --- /dev/null +++ b/docs/diagnostics/LostVariable.md @@ -0,0 +1,16 @@ +# (LostVariable) + + +## Описание диагностики + + +## Примеры + + +## Источники + + diff --git a/docs/en/diagnostics/LostVariable.md b/docs/en/diagnostics/LostVariable.md new file mode 100644 index 00000000000..f4023e746ae --- /dev/null +++ b/docs/en/diagnostics/LostVariable.md @@ -0,0 +1,16 @@ +# (LostVariable) + + +## Description + + +## Examples + + +## Sources + + diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java new file mode 100644 index 00000000000..d4b944cd6f2 --- /dev/null +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -0,0 +1,256 @@ +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.context.symbol.variable.VariableKind; +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.references.ReferenceIndex; +import com.github._1c_syntax.bsl.languageserver.references.model.OccurrenceType; +import com.github._1c_syntax.bsl.languageserver.references.model.Reference; +import com.github._1c_syntax.bsl.languageserver.utils.Ranges; +import com.github._1c_syntax.bsl.languageserver.utils.RelatedInformation; +import com.github._1c_syntax.bsl.parser.BSLParser; +import lombok.RequiredArgsConstructor; +import lombok.Value; +import org.antlr.v4.runtime.tree.ParseTree; +import org.eclipse.lsp4j.Location; +import org.eclipse.lsp4j.Position; +import org.eclipse.lsp4j.Range; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +@DiagnosticMetadata( + type = DiagnosticType.CODE_SMELL, + severity = DiagnosticSeverity.MAJOR, + minutesToFix = 2, + tags = { + DiagnosticTag.SUSPICIOUS, + DiagnosticTag.UNPREDICTABLE, + DiagnosticTag.ERROR + } + +) + +@RequiredArgsConstructor +public class LostVariableDiagnostic extends AbstractVisitorDiagnostic { + private final ReferenceIndex referenceIndex; + private List variables; + + private static final Set CHECKING_VARIABLE_KINDS = EnumSet.of( +// VariableKind.MODULE, +// VariableKind.LOCAL, // TODO учесть разные типы переменных + VariableKind.DYNAMIC + ); + @Value + private static class VarData implements Comparable { +// VariableSymbol symbol; + String name; + Range selectionRange; +// Reference definition; + List references; + Status status = Status.NOT_CHECKED; +// List> references; + + @Override + public int compareTo(@NotNull Object o) { + return compare(this.getSelectionRange(), ((VarData)o).getSelectionRange()); + } + +// public VarData(VariableSymbol symbol, List references) { +// this.symbol = symbol; +// this.references = references.stream() +// .filter(ref -> ref.getOccurrenceType() == OccurrenceType.REFERENCE) +// .sorted((o1, o2) -> compare(o1.getSelectionRange().getStart(), o2.getSelectionRange().getStart())) +// .collect(Collectors.toList()); +// } + } + private enum Status { + NOT_CHECKED, + ALL_REFS_INSIDE_ONE_BLOCK, + SOME_REFS_OUTSIDE_ONE_BLOCK + } + + @Override + public ParseTree visitFile(BSLParser.FileContext ctx) { + variables = documentContext.getSymbolTree().getVariables().stream() + .filter(variable -> CHECKING_VARIABLE_KINDS.contains(variable.getKind())) + //.filter(variable -> !variable.isExport()) + .flatMap(variable -> getVarData(variable).stream()) +// .filter(Objects::nonNull) + .sorted() +// .sorted((var1, var2) -> compare(var1.getSymbol().getSelectionRange(), var2.getSymbol().getSelectionRange())) + .collect(Collectors.toUnmodifiableList()); + if (variables.isEmpty()) { + return defaultResult(); + } + return super.visitFile(ctx); + } + + @Nullable + private List getVarData(VariableSymbol variable) { + List allReferences = getSortedReferencesByLocation(variable); + if (allReferences.isEmpty()) { + return Collections.emptyList(); + } + final var possibleLostReferences = hasConsecutiveDefinitions(variable, allReferences); + return possibleLostReferences.stream() + .map(references -> new VarData(variable.getName(), variable.getVariableNameRange(), references)) + .collect(Collectors.toUnmodifiableList()); +// if (possibleLostReferences.isEmpty()) { +// return null; +// } +// return new VarData(variable.getName(), variable.getVariableNameRange(), possibleLostReferences); + } + + private List> hasConsecutiveDefinitions(VariableSymbol variable, List allReferences) { + List> result = new ArrayList<>(); + Reference prev = null; + if (allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION){ + prev = allReferences.get(0); + + final var references = new ArrayList(1 + allReferences.size()); + references.add(Reference.of(variable.getParent().get(), variable, + new Location(documentContext.getUri().toString(), variable.getVariableNameRange()), + OccurrenceType.DEFINITION)); + references.addAll(allReferences); + result.add(references); +// return true; + } + for (int i = 1 ; i < allReferences.size(); i++) { + final var next = allReferences.get(i); + if (next.getOccurrenceType() == OccurrenceType.DEFINITION){ + if (prev != null) { + final var references = new ArrayList(2 + allReferences.size() - i); + references.add(prev); + references.add(next); + if (i < allReferences.size() - 1){ + references.addAll(allReferences.subList(i + 1, allReferences.size())); + } + result.add(references); +// return true; + } + prev = next; + continue; + } + prev = null; + } + return result; + } + + @NotNull + private List getSortedReferencesByLocation(VariableSymbol variable) { + final var references = referenceIndex.getReferencesTo(variable); +// if (references.stream() +// .noneMatch(ref -> ref.getOccurrenceType() == OccurrenceType.REFERENCE)){ +// return null; +// } + return references.stream() + .sorted((o1, o2) -> compare(o1.getSelectionRange(), o2.getSelectionRange())) + .collect(Collectors.toList()); + } + + @Override + public ParseTree visitSubCodeBlock(BSLParser.SubCodeBlockContext ctx) { + final var blockRange = Ranges.create(ctx); + variables.stream() + //.filter(varData -> Ranges.containsRange(blockRange, varData.getSymbol().getSelectionRange())) +// .flatMap(varData -> isLostVars(blockRange, varData).stream()) + .filter(varData -> isLostVars(blockRange, varData)) + .forEach(varData -> fireIssue(varData)); +// .collect(Collectors.toList()); + +// variables.stream() +// .filter(varData -> Ranges.containsRange(blockRange, varData.symbol.getRange())) +// .sorted((o1, o2) -> { +// final var range1 = o1.getSymbol().getRange().getStart(); +// final var range2 = o2.getSymbol().getRange().getStart(); +// return compare(range1, range2); +// }) +// .collect(Collectors.toList()); +// variables.stream() +// .filter(varData -> Ranges.containsRange(blockRange, varData.symbol.getRange())) +// . + return super.visitSubCodeBlock(ctx); + } + + @Override + public ParseTree visitCodeBlock(BSLParser.CodeBlockContext ctx) { + final var parseTree = super.visitCodeBlock(ctx); + return parseTree; + } + + private boolean isLostVars(Range blockRange, VarData varData) { + return varData.references + // TODO избавиться от 1го элемента, т.к. он дублируется в соседних полях VarData + .subList(1, varData.references.size()).stream() + .allMatch(reference -> Ranges.containsRange(blockRange, reference.getSelectionRange())); +// .map(references -> new VarData(varData.getName(), varData.getDefinition(), references)) +// .collect(Collectors.toUnmodifiableList()); +// return false; +// final var isVarDefIntoCodeBlock = Ranges.containsRange(blockRange, varData.getSymbol().getSelectionRange()); +// final var references = varData.references; +//// if (references.get(0).getOccurrenceType() == OccurrenceType.DEFINITION +//// && ){ +//// +//// } +// for (int i = 0; i < references.size(); i++) { +// final var reference = references.get(i); +// if (reference.getOccurrenceType() != OccurrenceType.DEFINITION){ +// continue; +// } +// if (i == references.size() - 1){ +// return true; +// } +// //if (isAllNextRefsIntoCodeBlock(blockRange, reference, references.subList(i + 1, references.size() - 1))) +// } +// return false; +//// return varData.getReferences() +//// .stream() +//// .allMatch(reference -> Ranges.containsRange(blockRange, reference.getSelectionRange())); + } + + private static int compare(Range o1, Range o2) { + return compare(o1.getStart(), o2.getStart()); + } + + public static int compare(Position pos1, Position pos2) { + // 1,1 10,10 + if (pos1.getLine() < pos2.getLine()) { + return -1; + } + // 10,10 1,1 + if (pos1.getLine() > pos2.getLine()) { + return 1; + } + // 1,4 1,9 + if (pos1.getCharacter() < pos2.getCharacter()) { + return -1; + } + // 1,9 1,4 + if (pos1.getCharacter() > pos2.getCharacter()) { + return 1; + } + return 0; + } + + private void fireIssue(VarData varData) { + final var relatedInformationList = varData.getReferences().stream() + .map(context -> RelatedInformation.create( + documentContext.getUri(), + context.getSelectionRange(), + "+1" + )).collect(Collectors.toList()); + final var message = info.getMessage(varData.getName()); + diagnosticStorage.addDiagnostic(varData.getSelectionRange(), message, relatedInformationList); + }} diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties new file mode 100644 index 00000000000..66831715dfd --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties @@ -0,0 +1,2 @@ +diagnosticMessage= +diagnosticName= diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties new file mode 100644 index 00000000000..8fd310cee12 --- /dev/null +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties @@ -0,0 +1,2 @@ +diagnosticMessage=Предыдущее значение <%s> не используется +diagnosticName=<Имя диагностики> diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java new file mode 100644 index 00000000000..acd46923758 --- /dev/null +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -0,0 +1,33 @@ +package com.github._1c_syntax.bsl.languageserver.diagnostics; + +import com.github._1c_syntax.bsl.languageserver.util.CleanupContextBeforeClassAndAfterClass; +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; + +@CleanupContextBeforeClassAndAfterClass +class LostVariableDiagnosticTest extends AbstractDiagnosticTest { + LostVariableDiagnosticTest() { + super(LostVariableDiagnostic.class); + } + + @Test + void test() { + + List diagnostics = getDiagnostics(); + + assertThat(diagnostics, true) + .hasMessageOnRange(getMessage("Значение"), 1, 4, 12) + .hasMessageOnRange(getMessage("МояПеременная"), 4, 4, 17) + .hasMessageOnRange(getMessage("ТекстЗапроса"), 9, 4, 16) + .hasSize(3); + ; + + } + String getMessage(String name){ + return String.format("Предыдущее значение <%s> не используется", name); + } +} diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl new file mode 100644 index 00000000000..b15634e240f --- /dev/null +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -0,0 +1,72 @@ +Процедура Тест1() + Значение = 1; // ошибка + Значение = 2; + + МояПеременная = СтрЗаменить(КакаятоПеременная); // ошибка + МояПеременная = СтрЗаменить(КакаятоДругаяПеременная); +КонецПроцедуры + +Процедура Тест2() + ТекстЗапроса = "Первый"; // ошибка + ТекстЗапроса = "Второй"; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры + +Процедура Тест3() + ТекстЗапроса = "Первый"; // нет ошибки + Если Условие Тогда + ТекстЗапроса = "Второй"; + КонецЕсли; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры + +Процедура Тест4() + ТекстЗапроса = "Первый"; // ошибка + Если Условие Тогда + ТекстЗапроса = "Второй"; + Запрос = Новый Запрос(ТекстЗапроса); + КонецЕсли; +КонецПроцедуры + +Процедура Тест5() + ТекстЗапроса = "Первый"; // ошибка + Если Условие Тогда + ТекстЗапроса = "Второй"; + Если Условие Тогда + Запрос = Новый Запрос(ТекстЗапроса); + КонецЕсли; + КонецЕсли; +КонецПроцедуры + +Процедура Тест6() + Если Условие Тогда + ТекстЗапроса = "Первый"; // не ошибка + Иначе + ТекстЗапроса = "Второй"; + КонецЕсли; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры + +Процедура Тест7() + Если Условие Тогда + ТекстЗапроса = "Первый"; // не ошибка + Иначе + ТекстЗапроса = "Второй"; // ошибка + ТекстЗапроса = "Третий"; + КонецЕсли; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры + +Процедура Тест8() + Значение8 = 1; // TODO не ошибка ? + Значение8 = Значение8; // значение8 не меняется и может быть использовано позже +КонецПроцедуры + +Процедура Тест9() + Попытка + ТекстЗапроса = "Первый"; // не ошибка + Исключение + ТекстЗапроса = "Второй"; + КонецПопытки; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры From 4a558deb30072d9a74b7850a9a04dae16f2b6fbd Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 14 Jun 2022 00:02:20 +0300 Subject: [PATCH 02/45] =?UTF-8?q?=D1=80=D0=B5=D0=B0=D0=BB=D0=B8=D0=B7?= =?UTF-8?q?=D0=B0=D1=86=D0=B8=D1=8F=20+=20=D0=BD=D0=BE=D0=B2=D1=8B=D0=B5?= =?UTF-8?q?=20=D0=BF=D0=B0=D0=B4=D0=B0=D1=8E=D1=89=D0=B8=D0=B5=20=D1=82?= =?UTF-8?q?=D0=B5=D1=81=D1=82-=D0=BA=D0=B5=D0=B9=D1=81=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 244 ++++++++---------- .../bsl/languageserver/utils/Trees.java | 59 +++++ .../LostVariableDiagnosticTest.java | 27 +- .../diagnostics/LostVariableDiagnostic.bsl | 72 +++++- 4 files changed, 258 insertions(+), 144 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index d4b944cd6f2..ec32ac45803 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -1,3 +1,24 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2022 + * 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.symbol.VariableSymbol; @@ -9,24 +30,23 @@ import com.github._1c_syntax.bsl.languageserver.references.ReferenceIndex; import com.github._1c_syntax.bsl.languageserver.references.model.OccurrenceType; import com.github._1c_syntax.bsl.languageserver.references.model.Reference; -import com.github._1c_syntax.bsl.languageserver.utils.Ranges; import com.github._1c_syntax.bsl.languageserver.utils.RelatedInformation; +import com.github._1c_syntax.bsl.languageserver.utils.Trees; import com.github._1c_syntax.bsl.parser.BSLParser; +import com.github._1c_syntax.bsl.parser.BSLParserRuleContext; import lombok.RequiredArgsConstructor; import lombok.Value; import org.antlr.v4.runtime.tree.ParseTree; -import org.eclipse.lsp4j.Location; +import org.antlr.v4.runtime.tree.TerminalNode; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; import java.util.List; -import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -44,101 +64,78 @@ @RequiredArgsConstructor public class LostVariableDiagnostic extends AbstractVisitorDiagnostic { - private final ReferenceIndex referenceIndex; - private List variables; - private static final Set CHECKING_VARIABLE_KINDS = EnumSet.of( + private static final Set CHECKING_VARIABLE_KINDS = EnumSet.of( // VariableKind.MODULE, // VariableKind.LOCAL, // TODO учесть разные типы переменных VariableKind.DYNAMIC ); + private final ReferenceIndex referenceIndex; + @Value - private static class VarData implements Comparable { -// VariableSymbol symbol; + private static class VarData implements Comparable { String name; - Range selectionRange; -// Reference definition; + Range defRange; + Range rewriteRange; List references; - Status status = Status.NOT_CHECKED; -// List> references; @Override - public int compareTo(@NotNull Object o) { - return compare(this.getSelectionRange(), ((VarData)o).getSelectionRange()); + public int compareTo(@NotNull LostVariableDiagnostic.VarData o) { + return compare(this.getDefRange(), o.getDefRange()); } -// public VarData(VariableSymbol symbol, List references) { -// this.symbol = symbol; -// this.references = references.stream() -// .filter(ref -> ref.getOccurrenceType() == OccurrenceType.REFERENCE) -// .sorted((o1, o2) -> compare(o1.getSelectionRange().getStart(), o2.getSelectionRange().getStart())) -// .collect(Collectors.toList()); -// } - } - private enum Status { - NOT_CHECKED, - ALL_REFS_INSIDE_ONE_BLOCK, - SOME_REFS_OUTSIDE_ONE_BLOCK } @Override public ParseTree visitFile(BSLParser.FileContext ctx) { - variables = documentContext.getSymbolTree().getVariables().stream() + documentContext.getSymbolTree().getVariables().stream() .filter(variable -> CHECKING_VARIABLE_KINDS.contains(variable.getKind())) - //.filter(variable -> !variable.isExport()) .flatMap(variable -> getVarData(variable).stream()) -// .filter(Objects::nonNull) - .sorted() -// .sorted((var1, var2) -> compare(var1.getSymbol().getSelectionRange(), var2.getSymbol().getSelectionRange())) - .collect(Collectors.toUnmodifiableList()); - if (variables.isEmpty()) { - return defaultResult(); - } - return super.visitFile(ctx); + .filter(this::isLostVariable) + .forEach(this::fireIssue); + + return defaultResult(); } - @Nullable private List getVarData(VariableSymbol variable) { List allReferences = getSortedReferencesByLocation(variable); if (allReferences.isEmpty()) { return Collections.emptyList(); } - final var possibleLostReferences = hasConsecutiveDefinitions(variable, allReferences); - return possibleLostReferences.stream() - .map(references -> new VarData(variable.getName(), variable.getVariableNameRange(), references)) - .collect(Collectors.toUnmodifiableList()); -// if (possibleLostReferences.isEmpty()) { -// return null; -// } -// return new VarData(variable.getName(), variable.getVariableNameRange(), possibleLostReferences); + return getConsecutiveDefinitions(variable, allReferences); + } + + private List getSortedReferencesByLocation(VariableSymbol variable) { + final var references = referenceIndex.getReferencesTo(variable); + return references.stream() + .sorted((o1, o2) -> compare(o1.getSelectionRange(), o2.getSelectionRange())) + .collect(Collectors.toList()); } - private List> hasConsecutiveDefinitions(VariableSymbol variable, List allReferences) { - List> result = new ArrayList<>(); + private List getConsecutiveDefinitions(VariableSymbol variable, List allReferences) { + List result = new ArrayList<>(); Reference prev = null; - if (allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION){ + if (allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION) { prev = allReferences.get(0); - final var references = new ArrayList(1 + allReferences.size()); - references.add(Reference.of(variable.getParent().get(), variable, - new Location(documentContext.getUri().toString(), variable.getVariableNameRange()), - OccurrenceType.DEFINITION)); - references.addAll(allReferences); - result.add(references); -// return true; + var references = allReferences.subList(1, allReferences.size()); + var varData = new VarData(variable.getName(), variable.getVariableNameRange(), + allReferences.get(0).getSelectionRange(), references); + result.add(varData); } - for (int i = 1 ; i < allReferences.size(); i++) { + for (var i = 1; i < allReferences.size(); i++) { final var next = allReferences.get(i); - if (next.getOccurrenceType() == OccurrenceType.DEFINITION){ + if (next.getOccurrenceType() == OccurrenceType.DEFINITION) { if (prev != null) { - final var references = new ArrayList(2 + allReferences.size() - i); - references.add(prev); - references.add(next); - if (i < allReferences.size() - 1){ - references.addAll(allReferences.subList(i + 1, allReferences.size())); + final List references; + if (i < allReferences.size() - 1) { + references = allReferences.subList(i + 1, allReferences.size()); + } else { + references = Collections.emptyList(); } - result.add(references); -// return true; + var varData = new VarData(variable.getName(), prev.getSelectionRange(), + next.getSelectionRange(), references); + result.add(varData); } prev = next; continue; @@ -148,76 +145,45 @@ private List> hasConsecutiveDefinitions(VariableSymbol variable, return result; } - @NotNull - private List getSortedReferencesByLocation(VariableSymbol variable) { - final var references = referenceIndex.getReferencesTo(variable); -// if (references.stream() -// .noneMatch(ref -> ref.getOccurrenceType() == OccurrenceType.REFERENCE)){ -// return null; -// } - return references.stream() - .sorted((o1, o2) -> compare(o1.getSelectionRange(), o2.getSelectionRange())) - .collect(Collectors.toList()); - } - - @Override - public ParseTree visitSubCodeBlock(BSLParser.SubCodeBlockContext ctx) { - final var blockRange = Ranges.create(ctx); - variables.stream() - //.filter(varData -> Ranges.containsRange(blockRange, varData.getSymbol().getSelectionRange())) -// .flatMap(varData -> isLostVars(blockRange, varData).stream()) - .filter(varData -> isLostVars(blockRange, varData)) - .forEach(varData -> fireIssue(varData)); -// .collect(Collectors.toList()); - -// variables.stream() -// .filter(varData -> Ranges.containsRange(blockRange, varData.symbol.getRange())) -// .sorted((o1, o2) -> { -// final var range1 = o1.getSymbol().getRange().getStart(); -// final var range2 = o2.getSymbol().getRange().getStart(); -// return compare(range1, range2); -// }) -// .collect(Collectors.toList()); -// variables.stream() -// .filter(varData -> Ranges.containsRange(blockRange, varData.symbol.getRange())) -// . - return super.visitSubCodeBlock(ctx); + private boolean isLostVariable(VarData varData) { + // TODO ? быстрее найти сначала метод в дереве, а потом уже переменную в дерере метода? + // чтобы постоянно не искать по всему дереву + final var defNode = Trees.findNodeContainsPosition(documentContext.getAst(), + varData.getDefRange().getStart()); + if (defNode.isEmpty()) { + return false; // вдруг каким-то чудом все-таки не найдем ) + } + var defCodeBlock = getCodeBlock(defNode); + final var rewriteNodeInsideDefCodeBlock = defCodeBlock + .flatMap(context -> Trees.findNodeContainsPosition(context, + varData.rewriteRange.getStart())); + if (rewriteNodeInsideDefCodeBlock.isEmpty()) { + return false; + } + var rewriteCodeBlock = getCodeBlock(rewriteNodeInsideDefCodeBlock); + if (defCodeBlock.get() != rewriteCodeBlock.get() && varData.references.isEmpty()) { + return false; + } + return defCodeBlock.get() == rewriteCodeBlock.get() + || rewriteCodeBlock + .filter(codeBlock -> hasReferenceOutsideRewriteBlock(varData.references, codeBlock)) + .isEmpty(); } - @Override - public ParseTree visitCodeBlock(BSLParser.CodeBlockContext ctx) { - final var parseTree = super.visitCodeBlock(ctx); - return parseTree; + private static Optional getCodeBlock(Optional defNode) { + return defNode + .map(TerminalNode::getParent) + .map(BSLParserRuleContext.class::cast) + .map(node -> Trees.getRootParent(node, BSLParser.RULE_codeBlock)) + .filter(BSLParser.CodeBlockContext.class::isInstance) + .map(BSLParser.CodeBlockContext.class::cast); } - private boolean isLostVars(Range blockRange, VarData varData) { - return varData.references - // TODO избавиться от 1го элемента, т.к. он дублируется в соседних полях VarData - .subList(1, varData.references.size()).stream() - .allMatch(reference -> Ranges.containsRange(blockRange, reference.getSelectionRange())); -// .map(references -> new VarData(varData.getName(), varData.getDefinition(), references)) -// .collect(Collectors.toUnmodifiableList()); -// return false; -// final var isVarDefIntoCodeBlock = Ranges.containsRange(blockRange, varData.getSymbol().getSelectionRange()); -// final var references = varData.references; -//// if (references.get(0).getOccurrenceType() == OccurrenceType.DEFINITION -//// && ){ -//// -//// } -// for (int i = 0; i < references.size(); i++) { -// final var reference = references.get(i); -// if (reference.getOccurrenceType() != OccurrenceType.DEFINITION){ -// continue; -// } -// if (i == references.size() - 1){ -// return true; -// } -// //if (isAllNextRefsIntoCodeBlock(blockRange, reference, references.subList(i + 1, references.size() - 1))) -// } -// return false; -//// return varData.getReferences() -//// .stream() -//// .allMatch(reference -> Ranges.containsRange(blockRange, reference.getSelectionRange())); + private static boolean hasReferenceOutsideRewriteBlock(List references, BSLParserRuleContext codeBlock) { + return references.stream() + .map(Reference::getSelectionRange) + .anyMatch(range -> Trees.findNodeContainsPosition(codeBlock, range.getStart()) + .isEmpty()); } private static int compare(Range o1, Range o2) { @@ -234,14 +200,8 @@ public static int compare(Position pos1, Position pos2) { return 1; } // 1,4 1,9 - if (pos1.getCharacter() < pos2.getCharacter()) { - return -1; - } + return Integer.compare(pos1.getCharacter(), pos2.getCharacter()); // 1,9 1,4 - if (pos1.getCharacter() > pos2.getCharacter()) { - return 1; - } - return 0; } private void fireIssue(VarData varData) { @@ -252,5 +212,7 @@ private void fireIssue(VarData varData) { "+1" )).collect(Collectors.toList()); final var message = info.getMessage(varData.getName()); - diagnosticStorage.addDiagnostic(varData.getSelectionRange(), message, relatedInformationList); - }} + diagnosticStorage.addDiagnostic(varData.getDefRange(), message); +// diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); + } +} diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 5b430da8268..57b83de8a9a 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -29,6 +29,8 @@ import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; import org.antlr.v4.runtime.tree.Tree; +import org.eclipse.lsp4j.Position; +import org.eclipse.lsp4j.util.Positions; import javax.annotation.CheckForNull; import java.util.ArrayList; @@ -408,6 +410,46 @@ public static boolean nodeContains(ParseTree t, ParseTree exclude, Integer... in .anyMatch(i -> nodeContains(t.getChild(i), exclude, index)); } + /** + * Получение ноды в дереве по позиции в документе. + * + * @param tree - дерево, в котором ищем + * @param position - искомая позиция + * @return терминальная нода на указанной позиции, если есть + */ + public static Optional findNodeContainsPosition(BSLParserRuleContext tree, Position position) { + + if (tree.getTokens().isEmpty()) { + return Optional.empty(); + } + + var start = tree.getStart(); + var stop = tree.getStop(); + + if (!(positionIsAfterOrOnToken(position, start) && positionIsBeforeOrOnToken(position, stop))) { + return Optional.empty(); + } + + var children = Trees.getChildren(tree); + + for (Tree child : children) { + if (child instanceof TerminalNode) { + var terminalNode = (TerminalNode) child; + var token = terminalNode.getSymbol(); + if (tokenContainsPosition(token, position)) { + return Optional.of(terminalNode); + } + } else { + Optional node = findNodeContainsPosition((BSLParserRuleContext) child, position); + if (node.isPresent()) { + return node; + } + } + } + + return Optional.empty(); + } + /** * @param tokens - список токенов из DocumentContext * @param token - токен, на строке которого требуется найти висячий комментарий @@ -494,4 +536,21 @@ private static boolean treeContainsErrors(ParseTree tnc, boolean recursive) { && ruleContext.children != null && ruleContext.children.stream().anyMatch(Trees::treeContainsErrors); } + + private static boolean tokenContainsPosition(Token token, Position position) { + var tokenRange = Ranges.create(token); + return Ranges.containsPosition(tokenRange, position); + } + + private static boolean positionIsBeforeOrOnToken(Position position, Token token) { + var tokenRange = Ranges.create(token); + var end = tokenRange.getEnd(); + return Positions.isBefore(position, end) || end.equals(position); + } + + private static boolean positionIsAfterOrOnToken(Position position, Token token) { + var tokenRange = Ranges.create(token); + var start = tokenRange.getStart(); + return Positions.isBefore(start, position) || start.equals(position); + } } diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index acd46923758..ba578deeed0 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -1,3 +1,24 @@ +/* + * This file is a part of BSL Language Server. + * + * Copyright (c) 2018-2022 + * 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.util.CleanupContextBeforeClassAndAfterClass; @@ -23,7 +44,11 @@ void test() { .hasMessageOnRange(getMessage("Значение"), 1, 4, 12) .hasMessageOnRange(getMessage("МояПеременная"), 4, 4, 17) .hasMessageOnRange(getMessage("ТекстЗапроса"), 9, 4, 16) - .hasSize(3); + .hasMessageOnRange(getMessage("ТекстЗапроса"), 23, 4, 16) + .hasMessageOnRange(getMessage("ТекстЗапроса"), 31, 4, 16) + .hasMessageOnRange(getMessage("ТекстЗапроса"), 53, 7, 19) + .hasMessageOnRange(getMessage("ТекстЗапроса"), 74, 2, 14) + .hasSize(7); ; } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index b15634e240f..844ce0aae2f 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -58,8 +58,8 @@ КонецПроцедуры Процедура Тест8() - Значение8 = 1; // TODO не ошибка ? - Значение8 = Значение8; // значение8 не меняется и может быть использовано позже + //Значение8 = 1; // TODO не ошибка ? + //Значение8 = Значение8; // значение8 не меняется и может быть использовано позже КонецПроцедуры Процедура Тест9() @@ -70,3 +70,71 @@ КонецПопытки; Запрос = Новый Запрос(ТекстЗапроса); КонецПроцедуры + +Процедура Тест10() + ТекстЗапроса = "Первый"; // ошибка + ТекстЗапроса = "Второй"; + Если Условие Тогда + Запрос = Новый Запрос(ТекстЗапроса); + КонецЕсли; +КонецПроцедуры + +Процедура Тест11() + ТекстЗапроса = "Первый"; //не ошибка + Если Условие Тогда + ТекстЗапроса = "Второй";// не ошибка + Запрос = Новый Запрос(ТекстЗапроса); + КонецЕсли; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры + +Процедура Тест12() + ТекстЗапроса = "Первый"; //не ошибка + Если Условие Тогда + ТекстЗапроса = "Второй";// не ошибка + Если Условие2 Тогда + ТекстЗапроса = "Третий";// не ошибка + Запрос = Новый Запрос(ТекстЗапроса); + КонецЕсли; + //Запрос = Новый Запрос(ТекстЗапроса); + КонецЕсли; + Запрос = Новый Запрос(ТекстЗапроса); +КонецПроцедуры + +Процедура Тест13() + ТекстЗапроса = "Первый"; // не ошибка + Если Условие Тогда + ТекстЗапроса = "Второй"; + КонецЕсли; +КонецПроцедуры + +//Или другой пример правильного кода (изменение происходит только при определённых условиях): +Процедура ПерезаписьВУсловии() + ЛокальнаяПеременная = ВызовМоейФункции(); // не ошибка + Если Условие Тогда + ЛокальнаяПеременная = 42; + ИначеЕсли ДругоеУсловие Тогда + ЛокальнаяПеременная = ВызовФункции(); + Иначе + ЛокальнаяПеременная = 0; + КонецЕсли; +КонецПроцедуры + +Функция РезультатВыполненияПриПеренаправлении(Знач ЗадачаСсылка) + // в этом методе нет ошибок + Комментарий = "Текст"; + Комментарий = СокрЛП(ЗадачаСсылка.РезультатВыполнения);Комментарий = ?(ПустаяСтрока(Комментарий), "", Комментарий + Символы.ПС); + + ТекстОповещения = "Текст"; + ТекстОповещения = СтроковыеФункцииКлиентСервер.ПодставитьПараметрыВСтроку(ТекстОповещения, Формат); + + ТекстЗапроса = "Текст"; + ТекстЗапроса = СтрЗаменить(ТекстЗапроса, "Текст", ДругойТекстЗапроса); + + ДобавляемыйНомер = 1; + Пока Условие() Цикл + ДобавляемыйНомер = ДобавляемыйНомер + 1; + КонецЦикла; + + Возврат Неопределено; +КонецФункции \ No newline at end of file From d28decb8917cdd06cb7098c4b75441e823b8718e Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 14 Jun 2022 21:52:40 +0300 Subject: [PATCH 03/45] =?UTF-8?q?=D1=80=D0=B5=D0=B0=D0=BB=D0=B8=D0=B7?= =?UTF-8?q?=D0=B0=D1=86=D0=B8=D1=8F=20=D0=BD=D0=BE=D0=B2=D1=8B=D1=85=20?= =?UTF-8?q?=D0=BA=D0=B5=D0=B9=D1=81=D0=BE=D0=B2=20+=20=D0=BD=D0=BE=D0=B2?= =?UTF-8?q?=D1=8B=D0=B5=20=D0=BF=D0=B0=D0=B4=D0=B0=D1=8E=D1=89=D0=B8=D0=B5?= =?UTF-8?q?=20=D1=82=D0=B5=D1=81=D1=82-=D0=BA=D0=B5=D0=B9=D1=81=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 130 ++++++++++++++---- .../LostVariableDiagnostic_en.properties | 4 +- .../LostVariableDiagnostic_ru.properties | 2 +- .../diagnostics/LostVariableDiagnostic.bsl | 89 ++++++++++-- 4 files changed, 184 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index ec32ac45803..9dfdb53658b 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -30,13 +30,14 @@ import com.github._1c_syntax.bsl.languageserver.references.ReferenceIndex; import com.github._1c_syntax.bsl.languageserver.references.model.OccurrenceType; import com.github._1c_syntax.bsl.languageserver.references.model.Reference; +import com.github._1c_syntax.bsl.languageserver.utils.Ranges; import com.github._1c_syntax.bsl.languageserver.utils.RelatedInformation; import com.github._1c_syntax.bsl.languageserver.utils.Trees; import com.github._1c_syntax.bsl.parser.BSLParser; import com.github._1c_syntax.bsl.parser.BSLParserRuleContext; import lombok.RequiredArgsConstructor; import lombok.Value; -import org.antlr.v4.runtime.tree.ParseTree; +import org.antlr.v4.runtime.tree.RuleNode; import org.antlr.v4.runtime.tree.TerminalNode; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; @@ -63,7 +64,7 @@ ) @RequiredArgsConstructor -public class LostVariableDiagnostic extends AbstractVisitorDiagnostic { +public class LostVariableDiagnostic extends AbstractDiagnostic { private static final Set CHECKING_VARIABLE_KINDS = EnumSet.of( // VariableKind.MODULE, @@ -72,6 +73,8 @@ public class LostVariableDiagnostic extends AbstractVisitorDiagnostic { ); private final ReferenceIndex referenceIndex; + // TODO нужна опция для возможности работы правила в модулях форм и модулях объектов, могут быть FP + @Value private static class VarData implements Comparable { String name; @@ -83,18 +86,15 @@ private static class VarData implements Comparable { public int compareTo(@NotNull LostVariableDiagnostic.VarData o) { return compare(this.getDefRange(), o.getDefRange()); } - } @Override - public ParseTree visitFile(BSLParser.FileContext ctx) { + protected void check() { documentContext.getSymbolTree().getVariables().stream() .filter(variable -> CHECKING_VARIABLE_KINDS.contains(variable.getKind())) .flatMap(variable -> getVarData(variable).stream()) .filter(this::isLostVariable) .forEach(this::fireIssue); - - return defaultResult(); } private List getVarData(VariableSymbol variable) { @@ -149,36 +149,114 @@ private boolean isLostVariable(VarData varData) { // TODO ? быстрее найти сначала метод в дереве, а потом уже переменную в дерере метода? // чтобы постоянно не искать по всему дереву final var defNode = Trees.findNodeContainsPosition(documentContext.getAst(), - varData.getDefRange().getStart()); - if (defNode.isEmpty()) { - return false; // вдруг каким-то чудом все-таки не найдем ) - } + varData.getDefRange().getStart()) + .map(TerminalNode::getParent) + .orElseThrow(); +// if (defNode.isEmpty()) { +// return false; // вдруг каким-то чудом все-таки не найдем ) +// } +// var defNode = defNode.orElseThrow(); var defCodeBlock = getCodeBlock(defNode); - final var rewriteNodeInsideDefCodeBlock = defCodeBlock + final var rewriteNodeInsideDefCodeBlockOpt = defCodeBlock .flatMap(context -> Trees.findNodeContainsPosition(context, - varData.rewriteRange.getStart())); - if (rewriteNodeInsideDefCodeBlock.isEmpty()) { + varData.rewriteRange.getStart())) + .map(TerminalNode::getParent); + if (rewriteNodeInsideDefCodeBlockOpt.isEmpty()) { return false; } - var rewriteCodeBlock = getCodeBlock(rewriteNodeInsideDefCodeBlock); - if (defCodeBlock.get() != rewriteCodeBlock.get() && varData.references.isEmpty()) { - return false; + var rewriteNode = rewriteNodeInsideDefCodeBlockOpt.get(); + var rewriteCodeBlock = getCodeBlock(rewriteNode).orElseThrow(); + + var insideOneBlock = defCodeBlock.get() == rewriteCodeBlock; + if (!insideOneBlock) { + if (varData.references.isEmpty()) { + return false; + } + var rewriteStatement = getRootStatement(rewriteNode); + if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { + return false; + } +// return true; } - return defCodeBlock.get() == rewriteCodeBlock.get() - || rewriteCodeBlock - .filter(codeBlock -> hasReferenceOutsideRewriteBlock(varData.references, codeBlock)) - .isEmpty(); + if (insideOneBlock){ + if (!varData.references.isEmpty()) { + var rewriteStatement = getRootStatement(rewriteNode); + if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { + return false; + } + } +// //var defParentExpression = getParentExpression(defNode); +// var rewriteParentExpression = getParentExpression(rewriteNode); +// var noneSelfAssign = rewriteParentExpression.isEmpty(); +// if (noneSelfAssign){ +// return true; +// } +// +// var defParentStatement = getRootStatement(defNode); +// var rewriteParentStatement = getRootStatement(rewriteParentExpression); +// if (defParentStatement != rewriteParentStatement){ +// return true; +// } +// return !isVarNameOnlyIntoExpression(rewriteNode); + } + return insideOneBlock + || !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); +// return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); } - private static Optional getCodeBlock(Optional defNode) { - return defNode - .map(TerminalNode::getParent) + private static Optional getCodeBlock(RuleNode context) { + return getRootNode(context, BSLParser.RULE_codeBlock, BSLParser.CodeBlockContext.class); +// return Optional.of(context) +// .map(BSLParserRuleContext.class::cast) +// .map(node -> Trees.getRootParent(node, BSLParser.RULE_codeBlock)) +// .filter(BSLParser.CodeBlockContext.class::isInstance) +// .map(BSLParser.CodeBlockContext.class::cast); + } + + private static Optional getParentExpression(RuleNode context) { + return getRootNode(context, BSLParser.RULE_expression, BSLParser.ExpressionContext.class); +// return Optional.of(context) +// .map(BSLParserRuleContext.class::cast) +// .map(node -> Trees.getRootParent(node, BSLParser.RULE_expression)) +// .filter(BSLParser.ExpressionContext.class::isInstance) +// .map(BSLParser.ExpressionContext.class::cast) +// .orElseThrow();// TODO падает на Комментарий = 10;Комментарий = 20; (важно, что нет пробела после 10;) + } + + private static Optional getRootNode(RuleNode context, int index, Class klass) { + return Optional.of(context) .map(BSLParserRuleContext.class::cast) - .map(node -> Trees.getRootParent(node, BSLParser.RULE_codeBlock)) - .filter(BSLParser.CodeBlockContext.class::isInstance) - .map(BSLParser.CodeBlockContext.class::cast); + .map(node -> Trees.getRootParent(node, index)) + .filter(klass::isInstance) + .map(klass::cast); } +// private BSLParser.StatementContext getRootStatement(TerminalNode terminalNode) { +// return getRootStatement(terminalNode.getParent()); +// } + + private static BSLParser.StatementContext getRootStatement(RuleNode node) { + return getRootNode(node, BSLParser.RULE_statement, BSLParser.StatementContext.class) +// return Optional.of(node) +// .map(BSLParserRuleContext.class::cast) +// .map(node1 -> Trees.getRootParent(node1, BSLParser.RULE_statement)) +// .filter(BSLParser.StatementContext.class::isInstance) +// .map(BSLParser.StatementContext.class::cast) + .orElseThrow();// TODO падает на Комментарий = 10;Комментарий = 20; (важно, что нет пробела после 10;) + } + + private static boolean isVarNameOnlyIntoExpression(RuleNode context) { + return Optional.of(context) + .filter(BSLParser.ComplexIdentifierContext.class::isInstance) + .map(BSLParser.ComplexIdentifierContext.class::cast) + .filter(node -> node.getChildCount() == 1) + .map(RuleNode::getParent) + .filter(BSLParser.MemberContext.class::isInstance) + .map(RuleNode::getParent) + .filter(expression -> expression.getChildCount() == 1) + .filter(BSLParser.ExpressionContext.class::isInstance) + .isPresent(); + } private static boolean hasReferenceOutsideRewriteBlock(List references, BSLParserRuleContext codeBlock) { return references.stream() .map(Reference::getSelectionRange) diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties index 66831715dfd..772972dd32f 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties @@ -1,2 +1,2 @@ -diagnosticMessage= -diagnosticName= +diagnosticMessage=The previous value <%s> is not used +diagnosticName=Lost variable diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties index 8fd310cee12..118a26fab47 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties @@ -1,2 +1,2 @@ diagnosticMessage=Предыдущее значение <%s> не используется -diagnosticName=<Имя диагностики> +diagnosticName=Потерянная переменная diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 844ce0aae2f..103d9f245c3 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -120,21 +120,86 @@ КонецЕсли; КонецПроцедуры +Процедура Тест14() + ДобавляемыйНомер = 1; + Пока Условие() Цикл + ДобавляемыйНомер = ДобавляемыйНомер + 1; // не ошибка + КонецЦикла; +КонецПроцедуры + +Функция ВыраженияНаОднойСтроке() + //Комментарий = 10;Комментарий = 20; //TODO ошибка (важно, что нет пробела после 10;) + + Возврат Неопределено; +КонецФункции + Функция РезультатВыполненияПриПеренаправлении(Знач ЗадачаСсылка) // в этом методе нет ошибок - Комментарий = "Текст"; - Комментарий = СокрЛП(ЗадачаСсылка.РезультатВыполнения);Комментарий = ?(ПустаяСтрока(Комментарий), "", Комментарий + Символы.ПС); + Комментарий = "Текст"; + //Комментарий = СокрЛП(ЗадачаСсылка.РезультатВыполнения); + Комментарий = ?(ПустаяСтрока(Комментарий), "", Комментарий + Символы.ПС); - ТекстОповещения = "Текст"; - ТекстОповещения = СтроковыеФункцииКлиентСервер.ПодставитьПараметрыВСтроку(ТекстОповещения, Формат); + ТекстОповещения = "Текст"; + ТекстОповещения = СтроковыеФункцииКлиентСервер.ПодставитьПараметрыВСтроку(ТекстОповещения, Формат); - ТекстЗапроса = "Текст"; - ТекстЗапроса = СтрЗаменить(ТекстЗапроса, "Текст", ДругойТекстЗапроса); + ТекстЗапроса = "Текст"; + ТекстЗапроса = СтрЗаменить(ТекстЗапроса, "Текст", ДругойТекстЗапроса); - ДобавляемыйНомер = 1; - Пока Условие() Цикл - ДобавляемыйНомер = ДобавляемыйНомер + 1; - КонецЦикла; + Возврат Неопределено; +КонецФункции + +Процедура Тест15() + // в этом методе нет ошибок - код может быть где угодно + ВидПрава = ВидыПрав.Добавить(); // нет ошибок + + // TODO ВидПрава = ВидыПрав.Добавить(); - Возврат Неопределено; -КонецФункции \ No newline at end of file + НовыйПереход = ТаблицаПереходовНоваяСтрока("Начало", "СтраницаНавигацииНачало"); // нет ошибок + // TODO НовыйПереход = ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки", "СтраницаНавигацииПродолжение"); +КонецПроцедуры + +Процедура Тест15() + // в этом методе нет ошибок + КоличествоОбработанных = ПорцияЭлементовДанных.Количество(); + НачатьТранзакцию(); + Попытка + Блокировка.Заблокировать(); + ЗафиксироватьТранзакцию(); + Исключение + ОтменитьТранзакцию(); + КоличествоОбработанных = 0; // не ошибка + КонецПопытки; + ПорцияЭлементовДанных = Неопределено; + + ТребуетсяПрерватьОбработкуЭлементов(ПараметрыОбновления, КоличествоОбработанных); +КонецПроцедуры + +Процедура УстановитьПометкуУдаленияДляОбъектов(Результат, Знач УдаляемыеСсылки, Знач ПараметрыВыполнения) + Блокировка = Новый БлокировкаДанных; + + НачатьТранзакцию(); + Попытка + ЭтоОшибкаБлокировки = Истина; // TODO не ошибка? + Блокировка.Заблокировать(); + + ЭтоОшибкаБлокировки = Ложь; + ЗафиксироватьТранзакцию(); + Исключение + ОтменитьТранзакцию(); + Если ЭтоОшибкаБлокировки Тогда + КонецЕсли; + КонецПопытки; +КонецПроцедуры + +Процедура ОперативнаяПамятьДоступнаяКлиентскомуПриложению() + // TODO в этом методе нет ошибок + ДоступныйОбъем = 0; +#Если Сервер Или ТолстыйКлиентОбычноеПриложение Тогда + ДоступныйОбъем = 1; +#ИначеЕсли ТонкийКлиент Тогда + ДоступныйОбъем = 2; +#Иначе + ДоступныйОбъем = 0; +#КонецЕсли + Возврат ДоступныйОбъем; +КонецПроцедуры From f9bb3895f17b327e142f44a78e1d26895a843d60 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 14 Jun 2022 22:11:13 +0300 Subject: [PATCH 04/45] =?UTF-8?q?=D0=B2=D1=80=D0=B5=D0=BC=D0=B5=D0=BD?= =?UTF-8?q?=D0=BD=D0=BE=20=D0=B8=D1=81=D0=BA=D0=BB=D1=8E=D1=87=D0=B8=D0=BB?= =?UTF-8?q?=20=D0=BF=D0=B0=D0=B4=D0=B0=D1=8E=D1=89=D0=B8=D0=B5=20=D1=82?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/resources/diagnostics/LostVariableDiagnostic.bsl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 103d9f245c3..6a3f9500959 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -179,7 +179,7 @@ НачатьТранзакцию(); Попытка - ЭтоОшибкаБлокировки = Истина; // TODO не ошибка? + //ЭтоОшибкаБлокировки = Истина; // TODO не ошибка? Блокировка.Заблокировать(); ЭтоОшибкаБлокировки = Ложь; @@ -193,11 +193,11 @@ Процедура ОперативнаяПамятьДоступнаяКлиентскомуПриложению() // TODO в этом методе нет ошибок - ДоступныйОбъем = 0; + //ДоступныйОбъем = 0; #Если Сервер Или ТолстыйКлиентОбычноеПриложение Тогда - ДоступныйОбъем = 1; + //ДоступныйОбъем = 1; #ИначеЕсли ТонкийКлиент Тогда - ДоступныйОбъем = 2; + //ДоступныйОбъем = 2; #Иначе ДоступныйОбъем = 0; #КонецЕсли From 0c27eb96aff1a66c180844c105a99540ebac7f15 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 14 Jun 2022 22:11:52 +0300 Subject: [PATCH 05/45] =?UTF-8?q?Revert=20"=D0=B2=D1=80=D0=B5=D0=BC=D0=B5?= =?UTF-8?q?=D0=BD=D0=BD=D0=BE=20=D0=B8=D1=81=D0=BA=D0=BB=D1=8E=D1=87=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BF=D0=B0=D0=B4=D0=B0=D1=8E=D1=89=D0=B8=D0=B5=20?= =?UTF-8?q?=D1=82=D0=B5=D1=81=D1=82=D1=8B"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit f9bb3895f17b327e142f44a78e1d26895a843d60. --- src/test/resources/diagnostics/LostVariableDiagnostic.bsl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 6a3f9500959..103d9f245c3 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -179,7 +179,7 @@ НачатьТранзакцию(); Попытка - //ЭтоОшибкаБлокировки = Истина; // TODO не ошибка? + ЭтоОшибкаБлокировки = Истина; // TODO не ошибка? Блокировка.Заблокировать(); ЭтоОшибкаБлокировки = Ложь; @@ -193,11 +193,11 @@ Процедура ОперативнаяПамятьДоступнаяКлиентскомуПриложению() // TODO в этом методе нет ошибок - //ДоступныйОбъем = 0; + ДоступныйОбъем = 0; #Если Сервер Или ТолстыйКлиентОбычноеПриложение Тогда - //ДоступныйОбъем = 1; + ДоступныйОбъем = 1; #ИначеЕсли ТонкийКлиент Тогда - //ДоступныйОбъем = 2; + ДоступныйОбъем = 2; #Иначе ДоступныйОбъем = 0; #КонецЕсли From 5838d6aa5bc9d281016fc04f287198fa7a72e2de Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 14 Jun 2022 22:29:40 +0300 Subject: [PATCH 06/45] =?UTF-8?q?=D1=83=D1=82=D0=BE=D1=87=D0=BD=D0=B8?= =?UTF-8?q?=D0=BB=20=D1=82=D0=B5=D0=BA=D1=81=D1=82=20=D0=B7=D0=B0=D0=BC?= =?UTF-8?q?=D0=B5=D1=87=D0=B0=D0=BD=D0=B8=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic_en.properties | 2 +- .../diagnostics/LostVariableDiagnostic_ru.properties | 2 +- .../languageserver/diagnostics/LostVariableDiagnosticTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties index 772972dd32f..9d93b260e8a 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties @@ -1,2 +1,2 @@ -diagnosticMessage=The previous value <%s> is not used +diagnosticMessage=The value <%s> is not used diagnosticName=Lost variable diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties index 118a26fab47..7d1e0f40184 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties @@ -1,2 +1,2 @@ -diagnosticMessage=Предыдущее значение <%s> не используется +diagnosticMessage=Значение <%s> не используется diagnosticName=Потерянная переменная diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index ba578deeed0..24e620b95fd 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -53,6 +53,6 @@ void test() { } String getMessage(String name){ - return String.format("Предыдущее значение <%s> не используется", name); + return String.format("Значение <%s> не используется", name); } } From a1b9d882f1371f5e94e97c467c7c4babe9da66fb Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 14 Jun 2022 22:32:52 +0300 Subject: [PATCH 07/45] =?UTF-8?q?=D1=83=D0=B1=D1=80=D0=B0=D0=BB=20=D0=BD?= =?UTF-8?q?=D0=B5=D0=BD=D1=83=D0=B6=D0=BD=D1=8B=D0=B5=20=D0=B8=D1=81=D0=BA?= =?UTF-8?q?=D0=BB=D1=8E=D1=87=D0=B5=D0=BD=D0=B8=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 9dfdb53658b..20c85b7d6e0 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -66,11 +66,6 @@ @RequiredArgsConstructor public class LostVariableDiagnostic extends AbstractDiagnostic { - private static final Set CHECKING_VARIABLE_KINDS = EnumSet.of( -// VariableKind.MODULE, -// VariableKind.LOCAL, // TODO учесть разные типы переменных - VariableKind.DYNAMIC - ); private final ReferenceIndex referenceIndex; // TODO нужна опция для возможности работы правила в модулях форм и модулях объектов, могут быть FP @@ -91,7 +86,6 @@ public int compareTo(@NotNull LostVariableDiagnostic.VarData o) { @Override protected void check() { documentContext.getSymbolTree().getVariables().stream() - .filter(variable -> CHECKING_VARIABLE_KINDS.contains(variable.getKind())) .flatMap(variable -> getVarData(variable).stream()) .filter(this::isLostVariable) .forEach(this::fireIssue); @@ -291,6 +285,6 @@ private void fireIssue(VarData varData) { )).collect(Collectors.toList()); final var message = info.getMessage(varData.getName()); diagnosticStorage.addDiagnostic(varData.getDefRange(), message); -// diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); +// TODO diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); } } From 42609c8048fb11ffec0d75b070f20e38309486f6 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 15 Jun 2022 17:15:55 +0300 Subject: [PATCH 08/45] =?UTF-8?q?=D0=B8=D1=81=D0=BA=D0=BB=D1=8E=D1=87?= =?UTF-8?q?=D0=B8=D0=BB=20=D0=B4=D1=83=D0=B1=D0=BB=D0=B8=20=D0=B2=D0=BD?= =?UTF-8?q?=D1=83=D1=82=D1=80=D0=B8=20=D0=BF=D1=80=D0=B5=D0=BF=D1=80=D0=BE?= =?UTF-8?q?=D1=86=D0=B5=D1=81=D1=81=D0=BE=D1=80=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 58 +++++++++---------- .../LostVariableDiagnosticTest.java | 4 +- .../diagnostics/LostVariableDiagnostic.bsl | 33 ++++++++++- 3 files changed, 60 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 20c85b7d6e0..e38eb1f1d14 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -162,23 +162,24 @@ private boolean isLostVariable(VarData varData) { var rewriteCodeBlock = getCodeBlock(rewriteNode).orElseThrow(); var insideOneBlock = defCodeBlock.get() == rewriteCodeBlock; - if (!insideOneBlock) { - if (varData.references.isEmpty()) { - return false; - } - var rewriteStatement = getRootStatement(rewriteNode); - if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { - return false; - } -// return true; - } - if (insideOneBlock){ + if (insideOneBlock) { if (!varData.references.isEmpty()) { var rewriteStatement = getRootStatement(rewriteNode); if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { return false; } } + var defStatement = getRootStatement(defNode); + var hasPreprocessorBetween = defStatement.getParent().children.stream() + .filter(BSLParser.StatementContext.class::isInstance) + .map(BSLParser.StatementContext.class::cast) + .dropWhile(statementContext -> statementContext != defStatement) + .skip(1) + .anyMatch(statementContext -> statementContext.preprocessor() != null); + if (hasPreprocessorBetween){ + return false; + } + return true; // //var defParentExpression = getParentExpression(defNode); // var rewriteParentExpression = getParentExpression(rewriteNode); // var noneSelfAssign = rewriteParentExpression.isEmpty(); @@ -192,28 +193,30 @@ private boolean isLostVariable(VarData varData) { // return true; // } // return !isVarNameOnlyIntoExpression(rewriteNode); +// } else { } - return insideOneBlock - || !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); + + if (varData.references.isEmpty()) { + return false; + } + var rewriteStatement = getRootStatement(rewriteNode); + if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { + return false; + } + return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); +// return true; +// } +// return insideOneBlock +// || !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); // return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); } private static Optional getCodeBlock(RuleNode context) { return getRootNode(context, BSLParser.RULE_codeBlock, BSLParser.CodeBlockContext.class); -// return Optional.of(context) -// .map(BSLParserRuleContext.class::cast) -// .map(node -> Trees.getRootParent(node, BSLParser.RULE_codeBlock)) -// .filter(BSLParser.CodeBlockContext.class::isInstance) -// .map(BSLParser.CodeBlockContext.class::cast); } private static Optional getParentExpression(RuleNode context) { return getRootNode(context, BSLParser.RULE_expression, BSLParser.ExpressionContext.class); -// return Optional.of(context) -// .map(BSLParserRuleContext.class::cast) -// .map(node -> Trees.getRootParent(node, BSLParser.RULE_expression)) -// .filter(BSLParser.ExpressionContext.class::isInstance) -// .map(BSLParser.ExpressionContext.class::cast) // .orElseThrow();// TODO падает на Комментарий = 10;Комментарий = 20; (важно, что нет пробела после 10;) } @@ -225,17 +228,8 @@ private static Optional getRootNode(RuleNode .map(klass::cast); } -// private BSLParser.StatementContext getRootStatement(TerminalNode terminalNode) { -// return getRootStatement(terminalNode.getParent()); -// } - private static BSLParser.StatementContext getRootStatement(RuleNode node) { return getRootNode(node, BSLParser.RULE_statement, BSLParser.StatementContext.class) -// return Optional.of(node) -// .map(BSLParserRuleContext.class::cast) -// .map(node1 -> Trees.getRootParent(node1, BSLParser.RULE_statement)) -// .filter(BSLParser.StatementContext.class::isInstance) -// .map(BSLParser.StatementContext.class::cast) .orElseThrow();// TODO падает на Комментарий = 10;Комментарий = 20; (важно, что нет пробела после 10;) } diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 24e620b95fd..2ad75704ef1 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -48,7 +48,9 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 31, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 53, 7, 19) .hasMessageOnRange(getMessage("ТекстЗапроса"), 74, 2, 14) - .hasSize(7); + .hasMessageOnRange(getMessage("Файл"), 116, 4, 8) + .hasMessageOnRange(getMessage("ЭтоОшибкаБлокировки"), 195, 8, 27) + .hasSize(9); ; } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 103d9f245c3..10e68e3f9c7 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -108,6 +108,20 @@ КонецЕсли; КонецПроцедуры +Процедура ПолучитьУжеСуществующиеСнипетыИзОбработок() + Файл = Новый Файл(КаталогФич); // не ошибка + Если Не Файл.Существует() Тогда + Возврат; + КонецЕсли; + + Файл = Новый Файл(КаталогФич); // ошибка + БылиОшибки = Ложь; + НачальныйКаталог = КаталогФич; + КаталогПоиска = НачальныйКаталог; + + Файл = Новый Файл(НачальныйКаталог); +КонецПроцедуры + //Или другой пример правильного кода (изменение происходит только при определённых условиях): Процедура ПерезаписьВУсловии() ЛокальнаяПеременная = ВызовМоейФункции(); // не ошибка @@ -174,12 +188,12 @@ ТребуетсяПрерватьОбработкуЭлементов(ПараметрыОбновления, КоличествоОбработанных); КонецПроцедуры -Процедура УстановитьПометкуУдаленияДляОбъектов(Результат, Знач УдаляемыеСсылки, Знач ПараметрыВыполнения) +Процедура УстановитьПометкуУдаленияДляОбъектов() Блокировка = Новый БлокировкаДанных; НачатьТранзакцию(); Попытка - ЭтоОшибкаБлокировки = Истина; // TODO не ошибка? + ЭтоОшибкаБлокировки = Истина; // ошибка. а вот кейс ниже - не ошибка Блокировка.Заблокировать(); ЭтоОшибкаБлокировки = Ложь; @@ -191,6 +205,20 @@ КонецПопытки; КонецПроцедуры +Процедура ПереУстановкаВБлокеИсключения() + Блокировка = Новый БлокировкаДанных; + + НачатьТранзакцию(); + Попытка + ЭтоОшибкаБлокировки = Истина; // не ошибка + Блокировка.Заблокировать(); + ЗафиксироватьТранзакцию(); + Исключение + ОтменитьТранзакцию(); + ЭтоОшибкаБлокировки = Ложь; + КонецПопытки; +КонецПроцедуры + Процедура ОперативнаяПамятьДоступнаяКлиентскомуПриложению() // TODO в этом методе нет ошибок ДоступныйОбъем = 0; @@ -203,3 +231,4 @@ #КонецЕсли Возврат ДоступныйОбъем; КонецПроцедуры + From f0a5bffbaeda56dcb44d9914b45ade058148cf3d Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 15 Jun 2022 19:55:52 +0300 Subject: [PATCH 09/45] =?UTF-8?q?=D0=94=D0=BE=D0=BA=D1=83=D0=BC=D0=B5?= =?UTF-8?q?=D0=BD=D1=82=D0=B8=D1=80=D0=BE=D0=B2=D0=B0=D0=BB=20=D0=BF=D1=80?= =?UTF-8?q?=D0=B0=D0=B2=D0=B8=D0=BB=D0=BE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 98 ++++++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 5 deletions(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index 717340aa793..c9b0bf939aa 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -3,14 +3,102 @@ ## Описание диагностики +Переменной присваивается значение, которое не используется. Далее переменной еще раз присваивается новое значение. +Подобный код приводит к снижению читабельности кода, также может приводить к снижению производительности. +Может возникать при опечатках, "копипасте" или рефакторинге кода, а также из-за проблем в алгоритме. + +В текущей реализации возможны "ложно-положительные" срабатывания, т.е. могут выдаваться замечания к правильному коду, т.к. некоторые варианты кода сложно отличить от правильного. Подробнее в примерах, пункт "Примеры исключений". ## Примеры +Неверный код - безусловная повторная переустановка значения +```bsl +МояПеременная = СтрЗаменить(Текст, " ", ""); // ошибка +МояПеременная = СтрЗаменить(Текст, ";", ""); +``` +Правильный код +```bsl +МояПеременная = СтрЗаменить(Текст, " ", ""); +МояПеременная = СтрЗаменить(МояПеременная, ";", ""); +``` + +Или другой подозрительный код - повторная переустановка значения при некотором условии и только потом переменная используется +```bsl +МояПеременная = СтрЗаменить(Текст, " ", ""); // ошибка +Если Условие Тогда + МояПеременная = СтрЗаменить(Текст, ";", ""); + Возврат МояПеременная; +КонецЕсли; +``` + +А вот такой слегка переделанный код уже не будет выявляться правилом +- т.к. переустановка выполняется не всегда, а только при выполнения условия. +```bsl +МояПеременная = СтрЗаменить(Текст, " ", ""); +Если Условие Тогда + МояПеременная = СтрЗаменить(Текст, ";", ""); +КонецЕсли; +Возврат МояПеременная; +``` + +Еще подозрительный код - переменная `Файл` меняется несколько раз, и во втором случае ошибочно. +```bsl +Процедура ПолучитьФайлОбработки() + Файл = Новый Файл(КаталогФич); // не ошибка + Если Не Файл.Существует() Тогда + Возврат; + КонецЕсли; + + Файл = Новый Файл(КаталогФич); // ошибка + НачальныйКаталог = КаталогФич; + + Файл = Новый Файл(НачальныйКаталог); +КонецПроцедуры +``` + +**Примеры исключений** + +- 1 в коллекцию добавляются разные значения без явного переиспользования +```bsl +ВидПрава = ВидыПрав.Добавить(); // будет выдано замечание, хотя код валиден +ВидПрава = ВидыПрав.Добавить(); + +НовыйПереход = ТаблицаПереходовНоваяСтрока("Начало"); // будет выдано замечание, хотя код валиден +НовыйПереход = ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки"); +``` +Код выше вполне валиден, но его можно улучшить, упростить, явно выразив свои намерения, избавившись от присваивания переменным и дублирования имен переменных +Исправленный код +```bsl +ВидыПрав.Добавить(); +ВидыПрав.Добавить(); + +ТаблицаПереходовНоваяСтрока("Начало"); +ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки"); +``` + +- 2 обработка переменных в блоках Попытка и Исключение +```bsl +Процедура УстановитьБлокировку() + Блокировка = Новый БлокировкаДанных; + + НачатьТранзакцию(); + Попытка + ЭтоОшибкаБлокировки = Истина; // будет выдано замечание, хотя код валиден + Блокировка.Заблокировать(); // здесь возможно исключение, и поэтому до следующей строки выполнение может не дойти + + ЭтоОшибкаБлокировки = Ложь; + ЗафиксироватьТранзакцию(); + Исключение + ОтменитьТранзакцию(); + Если ЭтоОшибкаБлокировки Тогда + ОбработкаОшибкиБлокировки(); + КонецЕсли; + КонецПопытки; +КонецПроцедуры +``` ## Источники - +* [MITRE, CWE-563 - Assignment to Variable without Use](https://cwe.mitre.org/data/definitions/563.html) +* [MITRE, CWE-1109 - Use of Same Variable for Multiple Purposes](https://cwe.mitre.org/data/definitions/1109.html) +* [PVS-Studio.V763. Parameter is always rewritten in function body before being used.](https://pvs-studio.com/ru/docs/warnings/v763) From 0eb00cf9646e4efb128102068d80e96ec451acab Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 15 Jun 2022 20:17:32 +0300 Subject: [PATCH 10/45] =?UTF-8?q?=D0=A3=D1=82=D0=BE=D1=87=D0=BD=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BE=D0=BF=D0=B8=D1=81=D0=B0=D0=BD=D0=B8=D0=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 35 ++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index c9b0bf939aa..c1dae780010 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -13,7 +13,7 @@ Неверный код - безусловная повторная переустановка значения ```bsl -МояПеременная = СтрЗаменить(Текст, " ", ""); // ошибка +МояПеременная = СтрЗаменить(Текст, " ", ""); // замечание МояПеременная = СтрЗаменить(Текст, ";", ""); ``` Правильный код @@ -24,7 +24,7 @@ Или другой подозрительный код - повторная переустановка значения при некотором условии и только потом переменная используется ```bsl -МояПеременная = СтрЗаменить(Текст, " ", ""); // ошибка +МояПеременная = СтрЗаменить(Текст, " ", ""); // замечание Если Условие Тогда МояПеременная = СтрЗаменить(Текст, ";", ""); Возврат МояПеременная; @@ -44,18 +44,33 @@ Еще подозрительный код - переменная `Файл` меняется несколько раз, и во втором случае ошибочно. ```bsl Процедура ПолучитьФайлОбработки() - Файл = Новый Файл(КаталогФич); // не ошибка + Файл = Новый Файл(КаталогФич); // нет замечания Если Не Файл.Существует() Тогда Возврат; КонецЕсли; - Файл = Новый Файл(КаталогФич); // ошибка + Файл = Новый Файл(КаталогФич); // замечание НачальныйКаталог = КаталогФич; Файл = Новый Файл(НачальныйКаталог); КонецПроцедуры ``` +Пример подозрительного кода - Инициализация переменных в начале метода +```bsl +Процедура ИнициализироватьКэшНастроек(Кэш, ПараметрыИнициализации=Неопределено, Отказ=Ложь) Экспорт + + КэшНастроек = Неопределено; // будет выдано замечание + + Если Инициализировали() Тогда + КэшНастроек = Новый Структура; + ПараметрыНастроек = ПараметрыНастроек(); + КэшНастроек.Вставить("ПараметрыНастроек", ПараметрыНастроек); + Возврат КэшНастроек; + КонецЕсли; +``` +- начальная установка `КэшНастроек` не имеет смысла + **Примеры исключений** - 1 в коллекцию добавляются разные значения без явного переиспользования @@ -97,6 +112,18 @@ КонецПроцедуры ``` +- 3 одно и то же имя счетчика в соседних циклах, причем счетчик цикла в цикле не используется +```bsl +Для НомерЯчейки = 1 По 2 Цикл // будет выдано замечание, хотя код валиден + ТабличныйДокумент.Присоединить(ПустаяЯчейка); +КонецЦикла; + +Для НомерЯчейки = 1 По КоличествоЯчеекДляЗаполнения - 2 Цикл + ПечатьСтроки(ЗначениеЗаполнения); +КонецЦикла; +``` +В этом коде счетчик `НомерЯчейки` фактически используется в обоих циклах + ## Источники * [MITRE, CWE-563 - Assignment to Variable without Use](https://cwe.mitre.org/data/definitions/563.html) From 66363eed72e81f2e2b7af7e5cb542308aa4b8a79 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 15 Jun 2022 20:30:48 +0300 Subject: [PATCH 11/45] =?UTF-8?q?=D0=95=D1=89=D0=B5=20=D1=82=D0=B5=D1=81?= =?UTF-8?q?=D1=82-=D0=BA=D0=B5=D0=B9=D1=81=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 25 +++++-------------- .../LostVariableDiagnosticTest.java | 4 ++- .../diagnostics/LostVariableDiagnostic.bsl | 16 +++++++++--- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index e38eb1f1d14..3ae58e3043c 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -22,7 +22,6 @@ 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.context.symbol.variable.VariableKind; 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; @@ -45,10 +44,8 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.EnumSet; import java.util.List; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; @DiagnosticMetadata( @@ -58,7 +55,7 @@ tags = { DiagnosticTag.SUSPICIOUS, DiagnosticTag.UNPREDICTABLE, - DiagnosticTag.ERROR + DiagnosticTag.BADPRACTICE } ) @@ -69,6 +66,7 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private final ReferenceIndex referenceIndex; // TODO нужна опция для возможности работы правила в модулях форм и модулях объектов, могут быть FP + // TODO нужна опция для возможности работы правила с переменными модуля и глобальными переменными, могут быть FP @Value private static class VarData implements Comparable { @@ -146,10 +144,7 @@ private boolean isLostVariable(VarData varData) { varData.getDefRange().getStart()) .map(TerminalNode::getParent) .orElseThrow(); -// if (defNode.isEmpty()) { -// return false; // вдруг каким-то чудом все-таки не найдем ) -// } -// var defNode = defNode.orElseThrow(); + var defCodeBlock = getCodeBlock(defNode); final var rewriteNodeInsideDefCodeBlockOpt = defCodeBlock .flatMap(context -> Trees.findNodeContainsPosition(context, @@ -176,10 +171,7 @@ private boolean isLostVariable(VarData varData) { .dropWhile(statementContext -> statementContext != defStatement) .skip(1) .anyMatch(statementContext -> statementContext.preprocessor() != null); - if (hasPreprocessorBetween){ - return false; - } - return true; + return !hasPreprocessorBetween; // //var defParentExpression = getParentExpression(defNode); // var rewriteParentExpression = getParentExpression(rewriteNode); // var noneSelfAssign = rewriteParentExpression.isEmpty(); @@ -204,11 +196,6 @@ private boolean isLostVariable(VarData varData) { return false; } return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); -// return true; -// } -// return insideOneBlock -// || !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); -// return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); } private static Optional getCodeBlock(RuleNode context) { @@ -278,7 +265,7 @@ private void fireIssue(VarData varData) { "+1" )).collect(Collectors.toList()); final var message = info.getMessage(varData.getName()); - diagnosticStorage.addDiagnostic(varData.getDefRange(), message); -// TODO diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); +// diagnosticStorage.addDiagnostic(varData.getDefRange(), message); + diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); } } diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 2ad75704ef1..b73f6ef48b9 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -49,8 +49,10 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 53, 7, 19) .hasMessageOnRange(getMessage("ТекстЗапроса"), 74, 2, 14) .hasMessageOnRange(getMessage("Файл"), 116, 4, 8) + .hasMessageOnRange(getMessage("Комментарий"), 144, 4, 15) .hasMessageOnRange(getMessage("ЭтоОшибкаБлокировки"), 195, 8, 27) - .hasSize(9); + .hasMessageOnRange(getMessage("НомерЯчейки"), 236, 8, 19) + .hasSize(11); ; } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 10e68e3f9c7..149daad15e2 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -142,7 +142,7 @@ КонецПроцедуры Функция ВыраженияНаОднойСтроке() - //Комментарий = 10;Комментарий = 20; //TODO ошибка (важно, что нет пробела после 10;) + Комментарий = 10;Комментарий = 20; // ошибка (важно, что нет пробела после 10;) Возврат Неопределено; КонецФункции @@ -205,7 +205,7 @@ КонецПопытки; КонецПроцедуры -Процедура ПереУстановкаВБлокеИсключения() +Процедура ПереустановкаВБлокеИсключения() Блокировка = Новый БлокировкаДанных; НачатьТранзакцию(); @@ -220,7 +220,7 @@ КонецПроцедуры Процедура ОперативнаяПамятьДоступнаяКлиентскомуПриложению() - // TODO в этом методе нет ошибок + // в этом методе нет ошибок ДоступныйОбъем = 0; #Если Сервер Или ТолстыйКлиентОбычноеПриложение Тогда ДоступныйОбъем = 1; @@ -232,3 +232,13 @@ Возврат ДоступныйОбъем; КонецПроцедуры +Процедура ПодготовитьТабличныйДокумент() + ТабличныйДокумент.Вывести(Отступ); + Для НомерЯчейки = 1 По 2 Цикл // ошибка + ТабличныйДокумент.Присоединить(ПустаяЯчейка); + КонецЦикла; + + Для НомерЯчейки = 1 По КоличествоЯчеекДляЗаполнения - 2 Цикл + ПечатьСтроки(ЗначениеЗаполнения); + КонецЦикла; +КонецПроцедуры From 2b99b47333ac746341ae1e97b425ae583ac52a44 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 15 Jun 2022 20:31:06 +0300 Subject: [PATCH 12/45] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BF=D0=BE=D1=8F=D1=81=D0=BD=D0=B5=D0=BD=D0=B8=D0=B5?= =?UTF-8?q?=20=D0=B4=D0=BB=D1=8F=20=D0=B8=D1=81=D0=BF=D1=80=D0=B0=D0=B2?= =?UTF-8?q?=D0=BB=D0=B5=D0=BD=D0=B8=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index c1dae780010..b9a92a466b2 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -122,7 +122,8 @@ ПечатьСтроки(ЗначениеЗаполнения); КонецЦикла; ``` -В этом коде счетчик `НомерЯчейки` фактически используется в обоих циклах +В этом коде счетчик `НомерЯчейки` фактически используется в обоих циклах. +Такой код лучше переписать, используя уникальные имена счетчиков в обоих циклах, что повысит читаемость и упростит сопровождение. ## Источники From b8f9270174e22294af9a1ca017a735ae7f797002 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 13:31:49 +0300 Subject: [PATCH 13/45] =?UTF-8?q?=D0=94=D0=BE=D0=BF=D0=BE=D0=BB=D0=BD?= =?UTF-8?q?=D0=B8=D0=BB=20=D0=BE=D0=BF=D0=B8=D1=81=D0=B0=D0=BD=D0=B8=D0=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 37 +++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index b9a92a466b2..06649dad0d8 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -41,6 +41,15 @@ Возврат МояПеременная; ``` +Часто встречающаяся "ошибка последней строки" +```bsl +ТемаПисьма = ДанныеЗаполнения.Тема;// потерялось +ТекстШаблонаПисьмаHTML = ДанныеЗаполнения.ТекстHTML; +ТекстШаблонаПисьма = ДанныеЗаполнения.Текст; +ТемаПисьма = ДанныеЗаполнения.Тема; // а здесь переопредели +Наименование = ДанныеЗаполнения.Тема; +``` + Еще подозрительный код - переменная `Файл` меняется несколько раз, и во втором случае ошибочно. ```bsl Процедура ПолучитьФайлОбработки() @@ -58,16 +67,14 @@ Пример подозрительного кода - Инициализация переменных в начале метода ```bsl -Процедура ИнициализироватьКэшНастроек(Кэш, ПараметрыИнициализации=Неопределено, Отказ=Ложь) Экспорт +Функция ПолучитьПутьФайлаВРабочемКаталоге(ДанныеФайла) - КэшНастроек = Неопределено; // будет выдано замечание - - Если Инициализировали() Тогда - КэшНастроек = Новый Структура; - ПараметрыНастроек = ПараметрыНастроек(); - КэшНастроек.Вставить("ПараметрыНастроек", ПараметрыНастроек); - Возврат КэшНастроек; - КонецЕсли; + ПутьДляВозврата = ""; + ПолноеИмяФайла = ""; // замечание + ИмяКаталога = РабочийКаталогПользователя(); + + // Сперва пытаемся найти такую запись в регистре сведений. + ПолноеИмяФайла = ДанныеФайла.ПолноеИмяФайлаВРабочемКаталоге; ``` - начальная установка `КэшНастроек` не имеет смысла @@ -81,7 +88,7 @@ НовыйПереход = ТаблицаПереходовНоваяСтрока("Начало"); // будет выдано замечание, хотя код валиден НовыйПереход = ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки"); ``` -Код выше вполне валиден, но его можно улучшить, упростить, явно выразив свои намерения, избавившись от присваивания переменным и дублирования имен переменных +Код выше вполне валиден, но его можно улучшить и упростить, явно выразив свои намерения, избавившись от присваивания переменным и дублирования имен переменных Исправленный код ```bsl ВидыПрав.Добавить(); @@ -91,6 +98,16 @@ ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки"); ``` +Пример полезного замечания для подобных дублей добавления в коллекцию - пример из типовой конфигурации +```bsl +ВидПрава = ВидыПрав.Добавить(); // вот тут ошибка! +ВидПрава = "Просмотр"; // т.к. ниже опечатка, автор кода забыл добавить .Имя +ВидПрава.Интерактивное = Истина; + +ВидПрава = ВидыПрав.Добавить(); +ВидПрава.Имя = "Редактирование"; +``` + - 2 обработка переменных в блоках Попытка и Исключение ```bsl Процедура УстановитьБлокировку() From d6e36cdb8c9c8bf66772e11413a52239685e13a8 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 13:32:59 +0300 Subject: [PATCH 14/45] =?UTF-8?q?=D0=9C=D0=B5=D1=82=D0=BE=D0=B4=20findNode?= =?UTF-8?q?SuchThat?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../bsl/languageserver/utils/Trees.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 57b83de8a9a..a82d5bb78ec 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -26,6 +26,7 @@ import lombok.experimental.UtilityClass; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.Token; +import org.antlr.v4.runtime.misc.Predicate; import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; import org.antlr.v4.runtime.tree.Tree; @@ -33,6 +34,7 @@ import org.eclipse.lsp4j.util.Positions; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -69,6 +71,48 @@ public static Collection findAllRuleNodes(ParseTree t, int ruleIndex) return org.antlr.v4.runtime.tree.Trees.findAllRuleNodes(t, ruleIndex); } + /** + /** + * Находит только первый элемент дерева, подходящий по типу элемента. + * Поиск более ограничен в отличие от findAllRuleNodes, который всегда обегает все дерево + * + * @param t узел дерева + * @param ruleIndex искомый тип элемент + * @return первый подходящий узел, если он найден + */ + public static Optional findNodeSuchThat(BSLParserRuleContext t, int ruleIndex) { + return findNodeSuchThat(t, node -> node.getRuleIndex() == ruleIndex); + } + + /** + * Находит только первый элемент дерева, подходящий по условию поиска. + * Поиск более ограничен в отличие от findAllRuleNodes, который всегда обегает все дерево + * + * @param t узел дерева + * @param pred предикат-условие поиска + * @return первый подходящий узел, если он найден + */ + public static Optional findNodeSuchThat(BSLParserRuleContext t, Predicate pred) { + return Optional.ofNullable(_findNodeSuchThat(t, pred)); + } + + @Nullable + private static BSLParserRuleContext _findNodeSuchThat(BSLParserRuleContext t, Predicate pred) { + if ( pred.eval(t) ) return t; + + if ( t==null ) return null; + + int n = t.getChildCount(); + for (int i = 0 ; i < n ; i++){ + final var child = t.getChild(i); + if (child instanceof BSLParserRuleContext) { + BSLParserRuleContext u = _findNodeSuchThat((BSLParserRuleContext)child, pred); + if ( u != null) return u; + } + } + return null; + } + public static List getChildren(Tree t) { return org.antlr.v4.runtime.tree.Trees.getChildren(t); } @@ -450,6 +494,19 @@ public static Optional findNodeContainsPosition(BSLParserRuleConte return Optional.empty(); } + /** + * Возвращает true, если левый операнд строго слева или выше по тексту, чем правый + * + * @param left узел еслева или впереди + * @param right узел справа или позади + * @return true, если левый операнд строго слева или выше по тексту, чем правый + */ + public boolean isBefore(BSLParserRuleContext left, BSLParserRuleContext right) { + final var leftPos = new Position(left.getStop().getLine(), left.getStop().getCharPositionInLine()); + final var rightPos = new Position(right.getStart().getLine(), right.getStart().getCharPositionInLine()); + return Positions.isBefore(leftPos, rightPos); + } + /** * @param tokens - список токенов из DocumentContext * @param token - токен, на строке которого требуется найти висячий комментарий From ab0e2929c40a7181cbe73ff649a32cc1e5f59b7d Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:13:43 +0300 Subject: [PATCH 15/45] =?UTF-8?q?=D1=82=D0=B8=D0=BF=D0=B8=D0=B7=D0=B8?= =?UTF-8?q?=D1=80=D0=BE=D0=B2=D0=B0=D0=BD=D0=BD=D1=8B=D0=B9=20=D0=BC=D0=B5?= =?UTF-8?q?=D1=82=D0=BE=D0=B4=20getRootNode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit метод findNodeContainsPosition переименовал метод в findTerminalNodeContainsPosition --- .../bsl/languageserver/utils/Trees.java | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index a82d5bb78ec..091612dcfa7 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -28,6 +28,7 @@ import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.misc.Predicate; import org.antlr.v4.runtime.tree.ParseTree; +import org.antlr.v4.runtime.tree.RuleNode; import org.antlr.v4.runtime.tree.TerminalNode; import org.antlr.v4.runtime.tree.Tree; import org.eclipse.lsp4j.Position; @@ -103,7 +104,7 @@ private static BSLParserRuleContext _findNodeSuchThat(BSLParserRuleContext t, Pr if ( t==null ) return null; int n = t.getChildCount(); - for (int i = 0 ; i < n ; i++){ + for (var i = 0 ; i < n ; i++){ final var child = t.getChild(i); if (child instanceof BSLParserRuleContext) { BSLParserRuleContext u = _findNodeSuchThat((BSLParserRuleContext)child, pred); @@ -332,6 +333,26 @@ public static BSLParserRuleContext getRootParent(BSLParserRuleContext tnc) { return tnc; } + /** + * Рекурсивно находит самого верхнего родителя текущей ноды нужного типа + * + * @param context - нода, для которой ищем родителя + * @param index- BSLParser.RULE_* + * @param klass - класс типа + * @param - тип + * @return - если родитель не найден, вернет пустой Optional + */ + public static Optional getRootNode(RuleNode context, int index, Class klass) { + if (klass.isInstance(context)){ + return Optional.of(klass.cast(context)); + } + return Optional.of(context) + .map(BSLParserRuleContext.class::cast) + .map(node -> Trees.getRootParent(node, index)) + .filter(klass::isInstance) + .map(klass::cast); + } + /** * Рекурсивно находит самого верхнего родителя текущей ноды нужного типа * @@ -455,13 +476,13 @@ public static boolean nodeContains(ParseTree t, ParseTree exclude, Integer... in } /** - * Получение ноды в дереве по позиции в документе. + * Получение терминальной ноды в дереве по позиции в документе. * * @param tree - дерево, в котором ищем * @param position - искомая позиция * @return терминальная нода на указанной позиции, если есть */ - public static Optional findNodeContainsPosition(BSLParserRuleContext tree, Position position) { + public static Optional findTerminalNodeContainsPosition(BSLParserRuleContext tree, Position position) { if (tree.getTokens().isEmpty()) { return Optional.empty(); @@ -484,7 +505,7 @@ public static Optional findNodeContainsPosition(BSLParserRuleConte return Optional.of(terminalNode); } } else { - Optional node = findNodeContainsPosition((BSLParserRuleContext) child, position); + Optional node = findTerminalNodeContainsPosition((BSLParserRuleContext) child, position); if (node.isPresent()) { return node; } @@ -495,16 +516,16 @@ public static Optional findNodeContainsPosition(BSLParserRuleConte } /** - * Возвращает true, если левый операнд строго слева или выше по тексту, чем правый + * Получение ноды в дереве по позиции в документе. * - * @param left узел еслева или впереди - * @param right узел справа или позади - * @return true, если левый операнд строго слева или выше по тексту, чем правый + * @param tree - дерево, в котором ищем + * @param position - искомая позиция + * @return нода на указанной позиции, если есть */ - public boolean isBefore(BSLParserRuleContext left, BSLParserRuleContext right) { - final var leftPos = new Position(left.getStop().getLine(), left.getStop().getCharPositionInLine()); - final var rightPos = new Position(right.getStart().getLine(), right.getStart().getCharPositionInLine()); - return Positions.isBefore(leftPos, rightPos); + public static Optional findNodeContainsPosition(BSLParserRuleContext tree, Position position) { + return findTerminalNodeContainsPosition(tree, position) + .map(TerminalNode::getParent) + .map(BSLParserRuleContext.class::cast); } /** From 04c5781c567131c9d2ef7d896ff7fd64ff07ac53 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:15:02 +0300 Subject: [PATCH 16/45] =?UTF-8?q?=D0=BF=D0=BE=D0=B4=D0=B4=D0=B5=D1=80?= =?UTF-8?q?=D0=B6=D0=BA=D0=B0=20=D0=BD=D0=BE=D0=B2=D1=8B=D1=85=20=D1=82?= =?UTF-8?q?=D0=B5=D1=81=D1=82-=D0=BA=D0=B5=D0=B9=D1=81=D0=BE=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ускорение поиска узлов в дереве ast --- .../diagnostics/LostVariableDiagnostic.java | 211 ++++++++++-------- .../LostVariableDiagnosticTest.java | 17 +- .../diagnostics/LostVariableDiagnostic.bsl | 65 ++++-- 3 files changed, 186 insertions(+), 107 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 3ae58e3043c..674bd30b627 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -21,6 +21,7 @@ */ package com.github._1c_syntax.bsl.languageserver.diagnostics; +import com.github._1c_syntax.bsl.languageserver.context.symbol.MethodSymbol; import com.github._1c_syntax.bsl.languageserver.context.symbol.VariableSymbol; import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticMetadata; import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticSeverity; @@ -37,16 +38,19 @@ import lombok.RequiredArgsConstructor; import lombok.Value; import org.antlr.v4.runtime.tree.RuleNode; -import org.antlr.v4.runtime.tree.TerminalNode; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; +import org.eclipse.lsp4j.SymbolKind; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; @DiagnosticMetadata( type = DiagnosticType.CODE_SMELL, @@ -64,9 +68,7 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private final ReferenceIndex referenceIndex; - - // TODO нужна опция для возможности работы правила в модулях форм и модулях объектов, могут быть FP - // TODO нужна опция для возможности работы правила с переменными модуля и глобальными переменными, могут быть FP + private final Map methodsAst = new HashMap<>(); @Value private static class VarData implements Comparable { @@ -74,6 +76,7 @@ private static class VarData implements Comparable { Range defRange; Range rewriteRange; List references; + MethodSymbol method; @Override public int compareTo(@NotNull LostVariableDiagnostic.VarData o) { @@ -83,18 +86,33 @@ public int compareTo(@NotNull LostVariableDiagnostic.VarData o) { @Override protected void check() { - documentContext.getSymbolTree().getVariables().stream() - .flatMap(variable -> getVarData(variable).stream()) + documentContext.getSymbolTree().getMethods().stream() + .flatMap(methodSymbol -> getVarData(methodSymbol).stream()) .filter(this::isLostVariable) .forEach(this::fireIssue); + + methodsAst.clear(); + } + + private List getVarData(MethodSymbol methodSymbol) { + final var variables = methodSymbol.getChildren().stream() + .filter(sourceDefinedSymbol -> sourceDefinedSymbol.getSymbolKind() == SymbolKind.Variable) + .filter(VariableSymbol.class::isInstance) + .map(VariableSymbol.class::cast) + .flatMap(variable -> getVarData(variable, methodSymbol).stream()) + .collect(Collectors.toList()); + if (variables.isEmpty()){ + return Collections.emptyList(); + } + return variables; } - private List getVarData(VariableSymbol variable) { + private List getVarData(VariableSymbol variable, MethodSymbol methodSymbol) { List allReferences = getSortedReferencesByLocation(variable); if (allReferences.isEmpty()) { return Collections.emptyList(); } - return getConsecutiveDefinitions(variable, allReferences); + return getConsecutiveDefinitions(variable, allReferences, methodSymbol); } private List getSortedReferencesByLocation(VariableSymbol variable) { @@ -104,7 +122,8 @@ private List getSortedReferencesByLocation(VariableSymbol variable) { .collect(Collectors.toList()); } - private List getConsecutiveDefinitions(VariableSymbol variable, List allReferences) { + private List getConsecutiveDefinitions(VariableSymbol variable, List allReferences, + MethodSymbol methodSymbol) { List result = new ArrayList<>(); Reference prev = null; if (allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION) { @@ -112,7 +131,7 @@ private List getConsecutiveDefinitions(VariableSymbol variable, List getConsecutiveDefinitions(VariableSymbol variable, List getConsecutiveDefinitions(VariableSymbol variable, List Trees.findNodeContainsPosition(context, - varData.rewriteRange.getStart())) - .map(TerminalNode::getParent); - if (rewriteNodeInsideDefCodeBlockOpt.isEmpty()) { + if (isForInside(defNode)){ return false; } - var rewriteNode = rewriteNodeInsideDefCodeBlockOpt.get(); - var rewriteCodeBlock = getCodeBlock(rewriteNode).orElseThrow(); - - var insideOneBlock = defCodeBlock.get() == rewriteCodeBlock; - if (insideOneBlock) { - if (!varData.references.isEmpty()) { - var rewriteStatement = getRootStatement(rewriteNode); - if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { - return false; - } - } - var defStatement = getRootStatement(defNode); - var hasPreprocessorBetween = defStatement.getParent().children.stream() - .filter(BSLParser.StatementContext.class::isInstance) - .map(BSLParser.StatementContext.class::cast) - .dropWhile(statementContext -> statementContext != defStatement) - .skip(1) - .anyMatch(statementContext -> statementContext.preprocessor() != null); - return !hasPreprocessorBetween; -// //var defParentExpression = getParentExpression(defNode); -// var rewriteParentExpression = getParentExpression(rewriteNode); -// var noneSelfAssign = rewriteParentExpression.isEmpty(); -// if (noneSelfAssign){ -// return true; -// } -// -// var defParentStatement = getRootStatement(defNode); -// var rewriteParentStatement = getRootStatement(rewriteParentExpression); -// if (defParentStatement != rewriteParentStatement){ -// return true; -// } -// return !isVarNameOnlyIntoExpression(rewriteNode); -// } else { - } - if (varData.references.isEmpty()) { + var defCodeBlockOpt = getCodeBlock(defNode); + final var rewriteNodeInsideDefCodeBlockOpt = defCodeBlockOpt + .flatMap(context -> Trees.findNodeContainsPosition(context, + varData.rewriteRange.getStart())); + if (rewriteNodeInsideDefCodeBlockOpt.isEmpty()) { return false; } + + var defCodeBlock = defCodeBlockOpt.orElseThrow(); + + var rewriteNode = rewriteNodeInsideDefCodeBlockOpt.get(); var rewriteStatement = getRootStatement(rewriteNode); - if (Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange())) { - return false; + var rewriteCodeBlock = getCodeBlock(rewriteStatement).orElseThrow(); + + var isInsideSameBlock = defCodeBlock == rewriteCodeBlock; + if (isInsideSameBlock) { + return isLostVariableInSameBlock(varData, defNode, defCodeBlock, rewriteStatement); } - return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); + + return isLostVariableInDifferentBlocks(varData, rewriteStatement, rewriteCodeBlock); } - private static Optional getCodeBlock(RuleNode context) { - return getRootNode(context, BSLParser.RULE_codeBlock, BSLParser.CodeBlockContext.class); + private RuleNode findDefNode(VarData varData) { + // быстрее сначала найти узел метода в дереве, а потом уже узел переменной в дереве метода + // чтобы постоянно не искать по всему дереву файла + final var methodCodeBlockContext = getMethodCodeBlockContext(varData.method); + return Trees.findNodeContainsPosition(methodCodeBlockContext, + varData.getDefRange().getStart()) + .orElseThrow(); } - private static Optional getParentExpression(RuleNode context) { - return getRootNode(context, BSLParser.RULE_expression, BSLParser.ExpressionContext.class); -// .orElseThrow();// TODO падает на Комментарий = 10;Комментарий = 20; (важно, что нет пробела после 10;) + private BSLParser.SubContext getMethodCodeBlockContext(MethodSymbol method) { + return methodsAst.computeIfAbsent(method, methodSymbol -> + Trees.findNodeContainsPosition(documentContext.getAst(), + methodSymbol.getRange().getStart()) + .flatMap(node -> Trees.getRootNode(node, BSLParser.RULE_sub, BSLParser.SubContext.class)) + .orElseThrow()); } - private static Optional getRootNode(RuleNode context, int index, Class klass) { - return Optional.of(context) - .map(BSLParserRuleContext.class::cast) - .map(node -> Trees.getRootParent(node, index)) - .filter(klass::isInstance) - .map(klass::cast); + private static boolean isForInside(RuleNode defNode) { + return defNode instanceof BSLParser.ForStatementContext || defNode instanceof BSLParser.ForEachStatementContext; + } + + private static Optional getCodeBlock(RuleNode context) { + return Trees.getRootNode(context, BSLParser.RULE_codeBlock, BSLParser.CodeBlockContext.class); } private static BSLParser.StatementContext getRootStatement(RuleNode node) { - return getRootNode(node, BSLParser.RULE_statement, BSLParser.StatementContext.class) - .orElseThrow();// TODO падает на Комментарий = 10;Комментарий = 20; (важно, что нет пробела после 10;) + return Trees.getRootNode(node, BSLParser.RULE_statement, BSLParser.StatementContext.class) + .orElseThrow(); + } + + private static boolean isLostVariableInSameBlock(VarData varData, RuleNode defNode, + BSLParser.CodeBlockContext defCodeBlock, + BSLParser.StatementContext rewriteStatement) { + if (!varData.references.isEmpty() && isRewriteAlreadyContainsFirstReference(varData, rewriteStatement)) { + return false; + } + var defStatement = getRootStatement(defNode); + + var defAndRewriteIsInSameLine = defStatement == rewriteStatement; + if (defAndRewriteIsInSameLine){ + return true; + } + var hasPreprocessorBetween = getStatementsBetween(defCodeBlock, defStatement, rewriteStatement) + .anyMatch(statementContext -> statementContext.preprocessor() != null); + if (hasPreprocessorBetween){ + return false; + } + var hasReturnBetween = getStatementsBetween(defCodeBlock, defStatement, rewriteStatement) + .anyMatch(statementContext -> Trees.findNodeSuchThat(statementContext, BSLParser.RULE_returnStatement) + .isPresent()); + return !hasReturnBetween; } - private static boolean isVarNameOnlyIntoExpression(RuleNode context) { - return Optional.of(context) - .filter(BSLParser.ComplexIdentifierContext.class::isInstance) - .map(BSLParser.ComplexIdentifierContext.class::cast) - .filter(node -> node.getChildCount() == 1) - .map(RuleNode::getParent) - .filter(BSLParser.MemberContext.class::isInstance) - .map(RuleNode::getParent) - .filter(expression -> expression.getChildCount() == 1) - .filter(BSLParser.ExpressionContext.class::isInstance) - .isPresent(); + private static Stream getStatementsBetween(BSLParser.CodeBlockContext defCodeBlock, + BSLParser.StatementContext defStatement, + BSLParser.StatementContext rewriteStatement) { + if (defStatement == rewriteStatement){ + return Stream.empty(); + } + return defCodeBlock.children.stream() + .filter(BSLParser.StatementContext.class::isInstance) + .map(BSLParser.StatementContext.class::cast) + .dropWhile(statementContext -> statementContext != defStatement) + .skip(1) + .takeWhile(statementContext -> statementContext != rewriteStatement) + ; } + + private static boolean isLostVariableInDifferentBlocks(VarData varData, + BSLParser.StatementContext rewriteStatement, + BSLParser.CodeBlockContext rewriteCodeBlock) { + if (varData.references.isEmpty()) { + return false; + } + if (isRewriteAlreadyContainsFirstReference(varData, rewriteStatement)) { + return false; + } + return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); + } + + private static boolean isRewriteAlreadyContainsFirstReference(VarData varData, + BSLParser.StatementContext rewriteStatement) { + return Ranges.containsRange(Ranges.create(rewriteStatement), varData.references.get(0).getSelectionRange()); + } + private static boolean hasReferenceOutsideRewriteBlock(List references, BSLParserRuleContext codeBlock) { return references.stream() .map(Reference::getSelectionRange) - .anyMatch(range -> Trees.findNodeContainsPosition(codeBlock, range.getStart()) + .anyMatch(range -> Trees.findTerminalNodeContainsPosition(codeBlock, range.getStart()) .isEmpty()); } @@ -265,7 +303,6 @@ private void fireIssue(VarData varData) { "+1" )).collect(Collectors.toList()); final var message = info.getMessage(varData.getName()); -// diagnosticStorage.addDiagnostic(varData.getDefRange(), message); diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); } } diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index b73f6ef48b9..b21934139f2 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -47,16 +47,19 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 23, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 31, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 53, 7, 19) - .hasMessageOnRange(getMessage("ТекстЗапроса"), 74, 2, 14) - .hasMessageOnRange(getMessage("Файл"), 116, 4, 8) - .hasMessageOnRange(getMessage("Комментарий"), 144, 4, 15) - .hasMessageOnRange(getMessage("ЭтоОшибкаБлокировки"), 195, 8, 27) - .hasMessageOnRange(getMessage("НомерЯчейки"), 236, 8, 19) - .hasSize(11); + .hasMessageOnRange(getMessage("ТекстЗапроса"), 69, 2, 14) + .hasMessageOnRange(getMessage("Файл"), 111, 4, 8) + .hasMessageOnRange(getMessage("Комментарий"), 139, 4, 15) + .hasMessageOnRange(getMessage("ВидПрава"), 159, 4, 12) + .hasMessageOnRange(getMessage("НовыйПереход"), 163, 4, 16) + .hasMessageOnRange(getMessage("ЭтоОшибкаБлокировки"), 188, 8, 27) + .hasMessageOnRange(getMessage("Представление"), 254, 4, 17) + .hasMessageOnRange(getMessage("ЛишниеТэги"), 275, 8, 18) + .hasSize(14); ; } String getMessage(String name){ - return String.format("Значение <%s> не используется", name); + return String.format("Значение переменной <%s> не используется, переменная перезаписывается дальше по коду", name); } } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 149daad15e2..c83d7dd6f58 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -57,11 +57,6 @@ Запрос = Новый Запрос(ТекстЗапроса); КонецПроцедуры -Процедура Тест8() - //Значение8 = 1; // TODO не ошибка ? - //Значение8 = Значение8; // значение8 не меняется и может быть использовано позже -КонецПроцедуры - Процедура Тест9() Попытка ТекстЗапроса = "Первый"; // не ошибка @@ -150,7 +145,6 @@ Функция РезультатВыполненияПриПеренаправлении(Знач ЗадачаСсылка) // в этом методе нет ошибок Комментарий = "Текст"; - //Комментарий = СокрЛП(ЗадачаСсылка.РезультатВыполнения); Комментарий = ?(ПустаяСтрока(Комментарий), "", Комментарий + Символы.ПС); ТекстОповещения = "Текст"; @@ -163,16 +157,15 @@ КонецФункции Процедура Тест15() - // в этом методе нет ошибок - код может быть где угодно - ВидПрава = ВидыПрав.Добавить(); // нет ошибок - - // TODO ВидПрава = ВидыПрав.Добавить(); + ВидПрава = ВидыПрав.Добавить(); // ошибка + ВидПрава = ВидыПрав.Добавить(); + // ошибка НовыйПереход = ТаблицаПереходовНоваяСтрока("Начало", "СтраницаНавигацииНачало"); // нет ошибок - // TODO НовыйПереход = ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки", "СтраницаНавигацииПродолжение"); + НовыйПереход = ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки", "СтраницаНавигацииПродолжение"); КонецПроцедуры -Процедура Тест15() +Процедура Тест16() // в этом методе нет ошибок КоличествоОбработанных = ПорцияЭлементовДанных.Количество(); НачатьТранзакцию(); @@ -234,7 +227,7 @@ Процедура ПодготовитьТабличныйДокумент() ТабличныйДокумент.Вывести(Отступ); - Для НомерЯчейки = 1 По 2 Цикл // ошибка + Для НомерЯчейки = 1 По 2 Цикл // не ошибка - другое правило про итератор ТабличныйДокумент.Присоединить(ПустаяЯчейка); КонецЦикла; @@ -242,3 +235,49 @@ ПечатьСтроки(ЗначениеЗаполнения); КонецЦикла; КонецПроцедуры + +Процедура Событие(Параметры, Отказ) + мСохраненныйДок = Неопределено; // не ошибка + Если Параметры.Свойство("АвтоТест") Тогда + Возврат; + КонецЕсли; + мСохраненныйДок = Параметры.ЗначениеКопирования; +КонецПроцедуры + +&НаКлиенте +Процедура ПредставлениеОтбораПоПериодуНачалоВыбора(Элемент, ДанныеВыбора, СтандартнаяОбработка) + СтандартнаяОбработка = Ложь; // не ошибка +КонецПроцедуры + +&НаСервереБезКонтекста +Функция ПредставлениеЧастиАдреса_ЕДТ(АдресСтруктура, СписокПолей) + + Представление = ""; // не ошибка, если включен флаг Игнорировать типизацию для ЕДТ + СтруктураПолей = Новый Структура(СписокПолей); + ЗаполнитьЗначенияСвойств(СтруктураПолей, АдресСтруктура); + + МодульУправлениеКонтактнойИнформацией = ОбщегоНазначения.ОбщийМодуль("УправлениеКонтактнойИнформацией"); + Представление = МодульУправлениеКонтактнойИнформацией.ПредставлениеКонтактнойИнформации(СтруктураПолей); + + Возврат Представление; + +КонецФункции + +&НаСервере +Функция ЗаполнитьТэгиОбъектов(РеквизитИмяОбъекта, ОбъектыОбработчика, РазметкаКода, ВнешниеТэги = Неопределено, СортироватьПоТэгам = Истина) + + ЭтоИмяПроцедуры = РеквизитИмяОбъекта = "Процедура" ИЛИ РеквизитИмяОбъекта = "Процедура2"; + ТэгиМодуля = РазметкаКода.Тэги.ПоМодулям[РазметкаКода.ИмяМодуля]; + Если ОбъектыОбработчика.Колонки.Найти("Тэги") = Неопределено Тогда + ОбъектыОбработчика.Колонки.Добавить("Тэги", Новый ОписаниеТипов("Строка",,Новый КвалификаторыСтроки(100))); + КонецЕсли; + ЕстьТэгиМодуляОбновления = ОбъектыОбработчика.Колонки.Найти("ТэгиМодуляОбновления") <> Неопределено; + Если ТэгиМодуля = Неопределено Тогда + ЛишниеТэги = Новый Массив; // не ошибка, если включен флаг Игнорировать типизацию для ЕДТ + ЛишниеТэги = ЛишниеТэгиВОбласти(ВнешниеТэги, РазметкаКода); + + Возврат ОбъектыОбработчика; + КонецЕсли; + + Возврат ОбъектыОбработчика; +КонецФункции \ No newline at end of file From 4643e7a3822898c9ddc3a4ca81ec97d013c65fd8 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:15:17 +0300 Subject: [PATCH 17/45] =?UTF-8?q?=D1=83=D1=82=D0=BE=D1=87=D0=BD=D0=B8?= =?UTF-8?q?=D0=BB=20=D1=81=D0=BE=D0=BE=D0=B1=D1=89=D0=B5=D0=BD=D0=B8=D0=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic_en.properties | 2 +- .../diagnostics/LostVariableDiagnostic_ru.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties index 9d93b260e8a..28658461359 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties @@ -1,2 +1,2 @@ -diagnosticMessage=The value <%s> is not used +diagnosticMessage=The value of variable <%s> is not used, the variable is rewritten diagnosticName=Lost variable diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties index 7d1e0f40184..05211d4a92f 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties @@ -1,2 +1,2 @@ -diagnosticMessage=Значение <%s> не используется +diagnosticMessage=Значение переменной <%s> не используется, переменная перезаписывается дальше по коду diagnosticName=Потерянная переменная From df9e488a580e52fdddc1ff665e7db977fba9db84 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:18:25 +0300 Subject: [PATCH 18/45] precommit --- docs/diagnostics/LostVariable.md | 2 +- docs/en/diagnostics/LostVariable.md | 2 +- .../configuration/parameters-schema.json | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index 06649dad0d8..6bfad45ddd1 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -1,4 +1,4 @@ -# (LostVariable) +# Потерянная переменная (LostVariable) ## Описание диагностики diff --git a/docs/en/diagnostics/LostVariable.md b/docs/en/diagnostics/LostVariable.md index f4023e746ae..a8ea9df8e4b 100644 --- a/docs/en/diagnostics/LostVariable.md +++ b/docs/en/diagnostics/LostVariable.md @@ -1,4 +1,4 @@ -# (LostVariable) +# Lost variable (LostVariable) ## Description 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 ee0bede0e41..7189ff8c370 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 @@ -962,6 +962,16 @@ "title": "Using a logical \"OR\" in the \"WHERE\" section of a query", "$id": "#/definitions/LogicalOrInTheWhereSectionOfQuery" }, + "LostVariable": { + "description": "Lost variable", + "default": true, + "type": [ + "boolean", + "object" + ], + "title": "Lost variable", + "$id": "#/definitions/LostVariable" + }, "MagicDate": { "description": "Magic dates", "default": true, From 021870c625bc6e7353c82a302e84ef76f170c901 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:36:58 +0300 Subject: [PATCH 19/45] =?UTF-8?q?=D1=83=D0=B1=D1=80=D0=B0=D0=BB=20=D0=BC?= =?UTF-8?q?=D0=B5=D1=82=D0=BE=D0=B4=D1=8B,=20=D0=BF=D0=B5=D1=80=D0=B5?= =?UTF-8?q?=D1=81=D0=B5=D0=BD=D0=BD=D1=8B=D0=B5=20=D0=B2=20Trees?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit исключил их дублирование --- .../providers/SelectionRangeProvider.java | 58 +------------------ 1 file changed, 1 insertion(+), 57 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java index ad2cdc60dcf..1f0c9af2ab4 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java @@ -27,14 +27,9 @@ import com.github._1c_syntax.bsl.parser.BSLParser; import com.github._1c_syntax.bsl.parser.BSLParserRuleContext; import org.antlr.v4.runtime.ParserRuleContext; -import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.tree.ParseTree; -import org.antlr.v4.runtime.tree.TerminalNode; -import org.antlr.v4.runtime.tree.Tree; -import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.SelectionRange; import org.eclipse.lsp4j.SelectionRangeParams; -import org.eclipse.lsp4j.util.Positions; import org.springframework.stereotype.Component; import javax.annotation.CheckForNull; @@ -91,7 +86,7 @@ public List getSelectionRange(DocumentContext documentContext, S // Result must contains all elements from input return positions.stream() - .map(position -> findNodeContainsPosition(ast, position)) + .map(position -> Trees.findTerminalNodeContainsPosition(ast, position)) .map(terminalNode -> terminalNode.orElse(null)) .map(SelectionRangeProvider::toSelectionRange) .collect(Collectors.toList()); @@ -215,55 +210,4 @@ private static boolean ifBranchMatchesIfStatement(BSLParserRuleContext ctx) { var ifStatement = (BSLParser.IfStatementContext) ifBranch.getParent(); return ifStatement.elseBranch() == null && ifStatement.elsifBranch().isEmpty(); } - - private static Optional findNodeContainsPosition(BSLParserRuleContext tree, Position position) { - - if (tree.getTokens().isEmpty()) { - return Optional.empty(); - } - - var start = tree.getStart(); - var stop = tree.getStop(); - - if (!(positionIsAfterOrOnToken(position, start) && positionIsBeforeOrOnToken(position, stop))) { - return Optional.empty(); - } - - var children = Trees.getChildren(tree); - - for (Tree child : children) { - if (child instanceof TerminalNode) { - var terminalNode = (TerminalNode) child; - var token = terminalNode.getSymbol(); - if (tokenContainsPosition(token, position)) { - return Optional.of(terminalNode); - } - } else { - Optional node = findNodeContainsPosition((BSLParserRuleContext) child, position); - if (node.isPresent()) { - return node; - } - } - } - - return Optional.empty(); - } - - private static boolean tokenContainsPosition(Token token, Position position) { - var tokenRange = Ranges.create(token); - return Ranges.containsPosition(tokenRange, position); - } - - private static boolean positionIsBeforeOrOnToken(Position position, Token token) { - var tokenRange = Ranges.create(token); - var end = tokenRange.getEnd(); - return Positions.isBefore(position, end) || end.equals(position); - } - - private static boolean positionIsAfterOrOnToken(Position position, Token token) { - var tokenRange = Ranges.create(token); - var start = tokenRange.getStart(); - return Positions.isBefore(start, position) || start.equals(position); - } - } From 6a1004bc55882a11841557d27cdbb53eb2a10f48 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:51:12 +0300 Subject: [PATCH 20/45] =?UTF-8?q?=D1=83=D0=B1=D1=80=D0=B0=D0=BB=20=D0=BD?= =?UTF-8?q?=D0=B5=D0=BD=D1=83=D0=B6=D0=BD=D1=83=D1=8E=20=D0=BF=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B5=D1=80=D0=BA=D1=83=20=D0=B2=20findNodeSuchThat=20?= =?UTF-8?q?=D0=BD=D0=B0=20null?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../com/github/_1c_syntax/bsl/languageserver/utils/Trees.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 091612dcfa7..72b2804a89b 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -101,8 +101,6 @@ public static Optional findNodeSuchThat(BSLParserRuleConte private static BSLParserRuleContext _findNodeSuchThat(BSLParserRuleContext t, Predicate pred) { if ( pred.eval(t) ) return t; - if ( t==null ) return null; - int n = t.getChildCount(); for (var i = 0 ; i < n ; i++){ final var child = t.getChild(i); From 067aad30073588d7590fae4ef0d0c304ff7ed5b9 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 15:51:33 +0300 Subject: [PATCH 21/45] =?UTF-8?q?=D1=83=D0=BB=D1=83=D1=87=D1=88=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BF=D0=BE=D0=BA=D1=80=D1=8B=D1=82=D0=B8=D0=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 3 --- .../resources/diagnostics/LostVariableDiagnostic.bsl | 12 +++++++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 674bd30b627..d2206d9621a 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -241,9 +241,6 @@ private static boolean isLostVariableInSameBlock(VarData varData, RuleNode defNo private static Stream getStatementsBetween(BSLParser.CodeBlockContext defCodeBlock, BSLParser.StatementContext defStatement, BSLParser.StatementContext rewriteStatement) { - if (defStatement == rewriteStatement){ - return Stream.empty(); - } return defCodeBlock.children.stream() .filter(BSLParser.StatementContext.class::isInstance) .map(BSLParser.StatementContext.class::cast) diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index c83d7dd6f58..529bbc78565 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -280,4 +280,14 @@ КонецЕсли; Возврат ОбъектыОбработчика; -КонецФункции \ No newline at end of file +КонецФункции + +Процедура ПереборКоллекции(ТабличныйДокумент) + Для Элемент Из Коллекция Цикл // не ошибка - другое правило про итератор + ТабличныйДокумент.Присоединить(ПустаяЯчейка); + КонецЦикла; + + Для Элемент Из Коллекция Цикл // не ошибка - другое правило про итератор + ПечатьСтроки(ЗначениеЗаполнения); + КонецЦикла; +КонецПроцедуры From 78cb724e2b9443c84dd6bc6915d28dcbe1f16ce5 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 16:03:03 +0300 Subject: [PATCH 22/45] =?UTF-8?q?=D1=83=D0=B4=D0=B0=D0=BB=D0=B8=D0=BB=20?= =?UTF-8?q?=D0=BD=D0=B5=D0=B8=D1=81=D0=BF=D0=BE=D0=BB=D1=8C=D0=B7=D1=83?= =?UTF-8?q?=D0=B5=D0=BC=D1=8B=D0=B9=20=D0=BA=D0=BE=D0=B4=20=D0=B8=20=D0=B8?= =?UTF-8?q?=D0=BD=D1=82=D0=B5=D1=80=D1=84=D0=B5=D0=B9=D1=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index d2206d9621a..22a2dfd0479 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -41,7 +41,6 @@ import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; -import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Collections; @@ -71,17 +70,12 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private final Map methodsAst = new HashMap<>(); @Value - private static class VarData implements Comparable { + private static class VarData { String name; Range defRange; Range rewriteRange; List references; MethodSymbol method; - - @Override - public int compareTo(@NotNull LostVariableDiagnostic.VarData o) { - return compare(this.getDefRange(), o.getDefRange()); - } } @Override From 5f0530921a7478701757b51b4818f66ccc06a0d0 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 19 Jun 2022 16:04:40 +0300 Subject: [PATCH 23/45] =?UTF-8?q?=D0=B8=D1=81=D0=BF=D1=80=D0=B0=D0=B2?= =?UTF-8?q?=D0=B8=D0=BB=20=D0=B2=D0=BD=D0=B5=D1=81=D0=B5=D0=BD=D0=BD=D1=83?= =?UTF-8?q?=D1=8E=20=D0=BE=D1=88=D0=B8=D0=B1=D0=BA=D1=83=20=D0=B2=20=D1=82?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D0=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/resources/diagnostics/LostVariableDiagnostic.bsl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 529bbc78565..46578afa14a 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -283,11 +283,11 @@ КонецФункции Процедура ПереборКоллекции(ТабличныйДокумент) - Для Элемент Из Коллекция Цикл // не ошибка - другое правило про итератор + Для Каждого Элемент Из Коллекция Цикл // не ошибка - другое правило про итератор ТабличныйДокумент.Присоединить(ПустаяЯчейка); КонецЦикла; - Для Элемент Из Коллекция Цикл // не ошибка - другое правило про итератор + Для Каждого Элемент Из Коллекция Цикл // не ошибка - другое правило про итератор ПечатьСтроки(ЗначениеЗаполнения); КонецЦикла; КонецПроцедуры From 4adf46adf93e7be7689cde62eec3a1aa4c60e404 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 21 Jun 2022 15:19:24 +0300 Subject: [PATCH 24/45] =?UTF-8?q?=D0=BD=D0=B0=D1=82=D1=83=D1=80=D0=B0?= =?UTF-8?q?=D0=BB=D1=8C=D0=BD=D1=8B=D0=B9=20=D0=BF=D0=BE=D1=80=D1=8F=D0=B4?= =?UTF-8?q?=D0=BE=D0=BA=20=D1=81=D1=80=D0=B0=D0=B2=D0=BD=D0=B5=D0=BD=D0=B8?= =?UTF-8?q?=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Range - Position - SymbolOccurrence --- .../references/model/SymbolOccurrence.java | 15 ++++++- .../bsl/languageserver/utils/Ranges.java | 39 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java index 5e3fa0f8ec2..a0a62a297c1 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java @@ -21,6 +21,7 @@ */ package com.github._1c_syntax.bsl.languageserver.references.model; +import com.github._1c_syntax.bsl.languageserver.utils.Ranges; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Value; @@ -54,6 +55,18 @@ public int compareTo(@NotNull SymbolOccurrence o) { if (this.equals(o)) { return 0; } - return hashCode() > o.hashCode() ? 1 : -1; + final var uriCompare = location.getUri().compareTo(o.location.getUri()); + if (uriCompare != 0){ + return uriCompare; + } + final var rangesCompare = Ranges.compare(location.getRange(), o.location.getRange()); + if (rangesCompare != 0){ + return rangesCompare; + } + final var occurenceCompare = occurrenceType.compareTo(o.occurrenceType); + if (occurenceCompare != 0){ + return occurenceCompare; + } + return symbol.compareTo(o.symbol); } } diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java index 684251f936d..00f0d9b4b9d 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java @@ -145,6 +145,45 @@ public boolean containsPosition(Range range, Position position) { return org.eclipse.lsp4j.util.Ranges.containsPosition(range, position); } + /** + * Натуральный порядок сравнения Range + * + * @param o1 - левый\меньший операнд + * @param o2 - правый\больший операнд + * @return 0 - равно, 1 - больше, -1 - меньше + */ + public int compare(Range o1, Range o2) { + if (o1.equals(o2)){ + return 0; + } + return compare(o1.getStart(), o2.getStart()); + } + + /** + * Натуральный порядок сравнения Position + * + * @param pos1 - левый\меньший операнд + * @param pos2 - правый\больший операнд + * @return 0 - равно, 1 - больше, -1 - меньше + */ + public int compare(Position pos1, Position pos2) { + if (pos1.equals(pos2)){ + return 0; + } + + // 1,1 10,10 + if (pos1.getLine() < pos2.getLine()) { + return -1; + } + // 10,10 1,1 + if (pos1.getLine() > pos2.getLine()) { + return 1; + } + // 1,4 1,9 + return Integer.compare(pos1.getCharacter(), pos2.getCharacter()); + // 1,9 1,4 + } + /** * @deprecated Для совместимости метод оставлен, но будет удален в будущих версиях. * Вместо него стоит использовать метод {@link ModuleSymbol#getSelectionRange()} From 5e028f6824fc3aa9b0f3b599f953e8ad521bbadf Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 21 Jun 2022 17:51:22 +0300 Subject: [PATCH 25/45] =?UTF-8?q?=D0=B4=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BF=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=BA=D1=83=20end?= =?UTF-8?q?=20=D0=B2=20Ranges.compare?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../github/_1c_syntax/bsl/languageserver/utils/Ranges.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java index 00f0d9b4b9d..b9b1d9c9186 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java @@ -156,7 +156,11 @@ public int compare(Range o1, Range o2) { if (o1.equals(o2)){ return 0; } - return compare(o1.getStart(), o2.getStart()); + final var startCompare = compare(o1.getStart(), o2.getStart()); + if (startCompare != 0){ + return startCompare; + } + return compare(o1.getEnd(), o2.getEnd()); } /** From 8829fb4a6fb9d8b35f1c95f7265fc8ddda9d34ed Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 21 Jun 2022 18:09:20 +0300 Subject: [PATCH 26/45] =?UTF-8?q?=D0=BD=D0=B0=D1=82=D1=83=D1=80=D0=B0?= =?UTF-8?q?=D0=BB=D1=8C=D0=BD=D1=8B=D0=B9=20=D0=BF=D0=BE=D1=80=D1=8F=D0=B4?= =?UTF-8?q?=D0=BE=D0=BA=20=D1=81=D0=BE=D1=80=D1=82=D0=B8=D1=80=D0=BE=D0=B2?= =?UTF-8?q?=D0=BA=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit из соседнего ПР --- .../diagnostics/LostVariableDiagnostic.java | 28 +------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 22a2dfd0479..95531bef59f 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -38,7 +38,6 @@ import lombok.RequiredArgsConstructor; import lombok.Value; import org.antlr.v4.runtime.tree.RuleNode; -import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; @@ -102,20 +101,13 @@ private List getVarData(MethodSymbol methodSymbol) { } private List getVarData(VariableSymbol variable, MethodSymbol methodSymbol) { - List allReferences = getSortedReferencesByLocation(variable); + List allReferences = referenceIndex.getReferencesTo(variable); if (allReferences.isEmpty()) { return Collections.emptyList(); } return getConsecutiveDefinitions(variable, allReferences, methodSymbol); } - private List getSortedReferencesByLocation(VariableSymbol variable) { - final var references = referenceIndex.getReferencesTo(variable); - return references.stream() - .sorted((o1, o2) -> compare(o1.getSelectionRange(), o2.getSelectionRange())) - .collect(Collectors.toList()); - } - private List getConsecutiveDefinitions(VariableSymbol variable, List allReferences, MethodSymbol methodSymbol) { List result = new ArrayList<>(); @@ -268,24 +260,6 @@ private static boolean hasReferenceOutsideRewriteBlock(List reference .isEmpty()); } - private static int compare(Range o1, Range o2) { - return compare(o1.getStart(), o2.getStart()); - } - - public static int compare(Position pos1, Position pos2) { - // 1,1 10,10 - if (pos1.getLine() < pos2.getLine()) { - return -1; - } - // 10,10 1,1 - if (pos1.getLine() > pos2.getLine()) { - return 1; - } - // 1,4 1,9 - return Integer.compare(pos1.getCharacter(), pos2.getCharacter()); - // 1,9 1,4 - } - private void fireIssue(VarData varData) { final var relatedInformationList = varData.getReferences().stream() .map(context -> RelatedInformation.create( From cb139844f381647497a9a24d4ae3630fa1d3b9bb Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 21 Jun 2022 18:41:39 +0300 Subject: [PATCH 27/45] =?UTF-8?q?=D1=83=D1=87=D0=B5=D0=BB=20=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B6=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=BF=D0=B5=D1=80=D0=B5?= =?UTF-8?q?=D0=BC=D0=B5=D0=BD=D0=BD=D1=8B=D1=85=20=D0=B2=20=D0=BE=D0=B1?= =?UTF-8?q?=D0=BB=D0=B0=D1=81=D1=82=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 22 ++++++++----------- .../LostVariableDiagnosticTest.java | 3 ++- .../diagnostics/LostVariableDiagnostic.bsl | 10 +++++++++ 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 95531bef59f..ead915eeebf 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -38,8 +38,10 @@ import lombok.RequiredArgsConstructor; import lombok.Value; import org.antlr.v4.runtime.tree.RuleNode; +import org.apache.commons.lang3.tuple.Pair; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; +import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Collections; @@ -79,25 +81,19 @@ private static class VarData { @Override protected void check() { - documentContext.getSymbolTree().getMethods().stream() - .flatMap(methodSymbol -> getVarData(methodSymbol).stream()) + getVariables() .filter(this::isLostVariable) .forEach(this::fireIssue); methodsAst.clear(); } - private List getVarData(MethodSymbol methodSymbol) { - final var variables = methodSymbol.getChildren().stream() - .filter(sourceDefinedSymbol -> sourceDefinedSymbol.getSymbolKind() == SymbolKind.Variable) - .filter(VariableSymbol.class::isInstance) - .map(VariableSymbol.class::cast) - .flatMap(variable -> getVarData(variable, methodSymbol).stream()) - .collect(Collectors.toList()); - if (variables.isEmpty()){ - return Collections.emptyList(); - } - return variables; + @NotNull + private Stream getVariables() { + return documentContext.getSymbolTree().getVariables().stream() + .map(variableSymbol -> Pair.of(variableSymbol.getRootParent(SymbolKind.Method), variableSymbol)) + .filter(pair -> pair.getLeft().isPresent()) + .flatMap(pair -> getVarData(pair.getRight(), (MethodSymbol)pair.getLeft().get()).stream()); } private List getVarData(VariableSymbol variable, MethodSymbol methodSymbol) { diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index b21934139f2..b489ef381ee 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -55,7 +55,8 @@ void test() { .hasMessageOnRange(getMessage("ЭтоОшибкаБлокировки"), 188, 8, 27) .hasMessageOnRange(getMessage("Представление"), 254, 4, 17) .hasMessageOnRange(getMessage("ЛишниеТэги"), 275, 8, 18) - .hasSize(14); + .hasMessageOnRange(getMessage("ТекстЗапроса"), 297, 4, 16) + .hasSize(15); ; } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 46578afa14a..6613fd65660 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -291,3 +291,13 @@ ПечатьСтроки(ЗначениеЗаполнения); КонецЦикла; КонецПроцедуры + +Процедура Тест17() + #Область Первая + #Область Вторая + ТекстЗапроса = "Первый"; // ошибка + ТекстЗапроса = "Второй"; + Запрос = Новый Запрос(ТекстЗапроса); + #КонецОбласти + #КонецОбласти +КонецПроцедуры From fbdbc323ad7f7b9d47aee5f0ec1c480d8c630984 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 21 Jun 2022 20:04:05 +0300 Subject: [PATCH 28/45] =?UTF-8?q?=D1=83=D1=87=D0=B5=D1=82=20=D1=82=D0=B5?= =?UTF-8?q?=D0=BB=D0=B0=20=D0=BC=D0=BE=D0=B4=D1=83=D0=BB=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit учет модуля без методов --- .../diagnostics/LostVariableDiagnostic.java | 89 +++++++++++++++---- .../LostVariableDiagnosticTest.java | 3 +- .../diagnostics/LostVariableDiagnostic.bsl | 4 + 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index ead915eeebf..06d4211fe55 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -22,6 +22,7 @@ package com.github._1c_syntax.bsl.languageserver.diagnostics; import com.github._1c_syntax.bsl.languageserver.context.symbol.MethodSymbol; +import com.github._1c_syntax.bsl.languageserver.context.symbol.SourceDefinedSymbol; import com.github._1c_syntax.bsl.languageserver.context.symbol.VariableSymbol; import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticMetadata; import com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticSeverity; @@ -37,11 +38,12 @@ import com.github._1c_syntax.bsl.parser.BSLParserRuleContext; import lombok.RequiredArgsConstructor; import lombok.Value; +import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.RuleNode; import org.apache.commons.lang3.tuple.Pair; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; -import org.jetbrains.annotations.NotNull; +import org.apache.commons.collections4.map.CaseInsensitiveMap; import java.util.ArrayList; import java.util.Collections; @@ -68,7 +70,8 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private final ReferenceIndex referenceIndex; - private final Map methodsAst = new HashMap<>(); + private final Map astBySymbol = new HashMap<>(); + private Map methodContextsByMethodName = new CaseInsensitiveMap<>(); @Value private static class VarData { @@ -76,27 +79,73 @@ private static class VarData { Range defRange; Range rewriteRange; List references; - MethodSymbol method; + SourceDefinedSymbol method; + boolean isMethod; } @Override protected void check() { + methodContextsByMethodName = getMethodContextsByMethodName(); + getFileCodeBlock() + .ifPresent(fileCodeBlock -> methodContextsByMethodName.put("", fileCodeBlock)); + getVariables() .filter(this::isLostVariable) .forEach(this::fireIssue); - methodsAst.clear(); + astBySymbol.clear(); + methodContextsByMethodName.clear(); + } + + private Map getMethodContextsByMethodName() { + return Optional.ofNullable(documentContext.getAst().subs()) + .map(BSLParser.SubsContext::sub) + .map(subContexts -> subContexts.stream() + .map(LostVariableDiagnostic::getMethodNameAndContext) + .collect(Collectors.toMap(Pair::getLeft, Pair::getRight))) + .orElseGet(Collections::emptyMap); + } + + private static Pair getMethodNameAndContext(BSLParser.SubContext subContext) { + final var subContextOpt = Optional.of(subContext); + final var subName = subContextOpt + .map(BSLParser.SubContext::function) +// .map(functionContext -> Pair.of(functionContext, functionContext.funcDeclaration().subName())) + .map(BSLParser.FunctionContext::funcDeclaration) + .map(BSLParser.FuncDeclarationContext::subName) + .map(BSLParser.SubNameContext::IDENTIFIER) + .map(ParseTree::getText) + .orElseGet(() -> subContextOpt + .map(BSLParser.SubContext::procedure) + .map(BSLParser.ProcedureContext::procDeclaration) + .map(BSLParser.ProcDeclarationContext::subName) + .map(BSLParser.SubNameContext::IDENTIFIER) + .map(ParseTree::getText) + .orElseThrow() + ); + return Pair.of(subName, subContext); + } + + private Optional getFileCodeBlock() { + return Optional.ofNullable(documentContext.getAst().fileCodeBlock()); } - @NotNull private Stream getVariables() { return documentContext.getSymbolTree().getVariables().stream() - .map(variableSymbol -> Pair.of(variableSymbol.getRootParent(SymbolKind.Method), variableSymbol)) - .filter(pair -> pair.getLeft().isPresent()) - .flatMap(pair -> getVarData(pair.getRight(), (MethodSymbol)pair.getLeft().get()).stream()); + .map(variableSymbol -> Pair.of(getRootSymbol(variableSymbol), variableSymbol)) +// .filter(pair -> pair.getLeft().isPresent()) +// .filter(pair -> pair.getLeft().get() instanceof MethodSymbol) +// .filter(pair -> MethodSymbol.class::isInstance(pair.getLeft().get())) + .flatMap(pair -> getVarData(pair.getRight(), pair.getLeft()).stream()); + } + + private static SourceDefinedSymbol getRootSymbol(VariableSymbol variableSymbol) { + return variableSymbol.getRootParent(SymbolKind.Method) + .orElseGet(() -> variableSymbol.getRootParent(SymbolKind.Module) + .orElseThrow()); } - private List getVarData(VariableSymbol variable, MethodSymbol methodSymbol) { + private List getVarData(VariableSymbol variable, SourceDefinedSymbol methodSymbol) { List allReferences = referenceIndex.getReferencesTo(variable); if (allReferences.isEmpty()) { return Collections.emptyList(); @@ -105,7 +154,7 @@ private List getVarData(VariableSymbol variable, MethodSymbol methodSym } private List getConsecutiveDefinitions(VariableSymbol variable, List allReferences, - MethodSymbol methodSymbol) { + SourceDefinedSymbol methodSymbol) { List result = new ArrayList<>(); Reference prev = null; if (allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION) { @@ -113,7 +162,7 @@ private List getConsecutiveDefinitions(VariableSymbol variable, List getConsecutiveDefinitions(VariableSymbol variable, List - Trees.findNodeContainsPosition(documentContext.getAst(), - methodSymbol.getRange().getStart()) - .flatMap(node -> Trees.getRootNode(node, BSLParser.RULE_sub, BSLParser.SubContext.class)) - .orElseThrow()); + private BSLParserRuleContext getMethodCodeBlockContext(SourceDefinedSymbol method, boolean isMethod) { + if (isMethod) { + final var methodName = method.getName(); + return astBySymbol.computeIfAbsent(method, methodSymbol -> + methodContextsByMethodName.get(methodName)); + } + return astBySymbol.computeIfAbsent(method, methodSymbol -> + methodContextsByMethodName.get("")); } private static boolean isForInside(RuleNode defNode) { diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index b489ef381ee..8c062e4ac05 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -56,7 +56,8 @@ void test() { .hasMessageOnRange(getMessage("Представление"), 254, 4, 17) .hasMessageOnRange(getMessage("ЛишниеТэги"), 275, 8, 18) .hasMessageOnRange(getMessage("ТекстЗапроса"), 297, 4, 16) - .hasSize(15); + .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 304, 0, 18) + .hasSize(16); ; } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 6613fd65660..3f5ece881b8 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -301,3 +301,7 @@ #КонецОбласти #КонецОбласти КонецПроцедуры + +ТекстЗапросаВБлоке = "Первый"; // ошибка +ТекстЗапросаВБлоке = "Второй"; +Запрос = Новый Запрос(ТекстЗапросаВБлоке); From 5722023a16fd1babc197923f71ad9222f0591021 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Tue, 21 Jun 2022 20:16:37 +0300 Subject: [PATCH 29/45] =?UTF-8?q?=D0=B7=D0=B0=D0=BC=D0=B5=D1=87=D0=B0?= =?UTF-8?q?=D0=BD=D0=B8=D1=8F=20=D0=A1=D0=BE=D0=BD=D0=B0=D1=80=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../_1c_syntax/bsl/languageserver/utils/Trees.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 72b2804a89b..1d6ffff2bd7 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -94,19 +94,23 @@ public static Optional findNodeSuchThat(BSLParserRuleConte * @return первый подходящий узел, если он найден */ public static Optional findNodeSuchThat(BSLParserRuleContext t, Predicate pred) { - return Optional.ofNullable(_findNodeSuchThat(t, pred)); + return Optional.ofNullable(findNodeSuchThatInner(t, pred)); } @Nullable - private static BSLParserRuleContext _findNodeSuchThat(BSLParserRuleContext t, Predicate pred) { - if ( pred.eval(t) ) return t; + private static BSLParserRuleContext findNodeSuchThatInner(BSLParserRuleContext t, Predicate pred) { + if ( pred.eval(t) ) { + return t; + } int n = t.getChildCount(); for (var i = 0 ; i < n ; i++){ final var child = t.getChild(i); if (child instanceof BSLParserRuleContext) { - BSLParserRuleContext u = _findNodeSuchThat((BSLParserRuleContext)child, pred); - if ( u != null) return u; + BSLParserRuleContext u = findNodeSuchThatInner((BSLParserRuleContext)child, pred); + if ( u != null) { + return u; + } } } return null; From dce636cf1c79baa5031ff417a84a6459f04e9372 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 22 Jun 2022 18:54:25 +0300 Subject: [PATCH 30/45] =?UTF-8?q?=D0=BA=D0=B5=D0=B9=D1=81=20=D0=BB=D0=BE?= =?UTF-8?q?=D0=BA=D0=B0=D0=BB=D1=8C=D0=BD=D0=BE=D0=B9=20=D0=B8=20=D0=B3?= =?UTF-8?q?=D0=BB=D0=BE=D0=B1=D0=B0=D0=BB=D1=8C=D0=BD=D0=BE=D0=B9=20=D0=BF?= =?UTF-8?q?=D0=B5=D1=80=D0=B5=D0=BC=D0=B5=D0=BD=D0=BD=D0=BE=D0=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit исправил и ускорил Trees.findContextContainsPosition --- .../diagnostics/LostVariableDiagnostic.java | 75 ++++++++++++------- .../bsl/languageserver/utils/Trees.java | 37 ++++++++- .../LostVariableDiagnosticTest.java | 7 +- .../diagnostics/LostVariableDiagnostic.bsl | 8 +- 4 files changed, 94 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 06d4211fe55..9bacb197a91 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -24,6 +24,7 @@ import com.github._1c_syntax.bsl.languageserver.context.symbol.MethodSymbol; import com.github._1c_syntax.bsl.languageserver.context.symbol.SourceDefinedSymbol; import com.github._1c_syntax.bsl.languageserver.context.symbol.VariableSymbol; +import com.github._1c_syntax.bsl.languageserver.context.symbol.variable.VariableKind; 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; @@ -41,16 +42,20 @@ import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.RuleNode; import org.apache.commons.lang3.tuple.Pair; +import org.eclipse.lsp4j.DiagnosticRelatedInformation; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; import org.apache.commons.collections4.map.CaseInsensitiveMap; import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -69,17 +74,19 @@ @RequiredArgsConstructor public class LostVariableDiagnostic extends AbstractDiagnostic { + private static final Set GlobalVariableKinds = EnumSet.of(VariableKind.GLOBAL, VariableKind.MODULE); + private final ReferenceIndex referenceIndex; private final Map astBySymbol = new HashMap<>(); private Map methodContextsByMethodName = new CaseInsensitiveMap<>(); @Value private static class VarData { - String name; + VariableSymbol variable; Range defRange; Range rewriteRange; List references; - SourceDefinedSymbol method; + SourceDefinedSymbol parentSymbol; boolean isMethod; } @@ -133,9 +140,6 @@ private Optional getFileCodeBlock() { private Stream getVariables() { return documentContext.getSymbolTree().getVariables().stream() .map(variableSymbol -> Pair.of(getRootSymbol(variableSymbol), variableSymbol)) -// .filter(pair -> pair.getLeft().isPresent()) -// .filter(pair -> pair.getLeft().get() instanceof MethodSymbol) -// .filter(pair -> MethodSymbol.class::isInstance(pair.getLeft().get())) .flatMap(pair -> getVarData(pair.getRight(), pair.getLeft()).stream()); } @@ -157,17 +161,24 @@ private List getConsecutiveDefinitions(VariableSymbol variable, List result = new ArrayList<>(); Reference prev = null; - if (allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION) { + final var isGlobalVar = GlobalVariableKinds.contains(variable.getKind()); + if (!isGlobalVar && allReferences.get(0).getOccurrenceType() == OccurrenceType.DEFINITION) { prev = allReferences.get(0); var references = allReferences.subList(1, allReferences.size()); - var varData = new VarData(variable.getName(), variable.getVariableNameRange(), + var varData = new VarData(variable, variable.getVariableNameRange(), allReferences.get(0).getSelectionRange(), references, methodSymbol, methodSymbol instanceof MethodSymbol); result.add(varData); } - for (var i = 1; i < allReferences.size(); i++) { - final var next = allReferences.get(i); - if (next.getOccurrenceType() == OccurrenceType.DEFINITION) { + final int firstIndex; + if (isGlobalVar){ + firstIndex = 0; + } else { + firstIndex = 1; + } + for (var i = firstIndex; i < allReferences.size(); i++) { + final var current = allReferences.get(i); + if (current.getOccurrenceType() == OccurrenceType.DEFINITION) { if (prev != null) { final List references; if (i < allReferences.size() - 1) { @@ -175,11 +186,11 @@ private List getConsecutiveDefinitions(VariableSymbol variable, List Trees.findNodeContainsPosition(context, - varData.rewriteRange.getStart())); + .flatMap(context -> Trees.findContextContainsPosition(context, varData.rewriteRange.getStart())); if (rewriteNodeInsideDefCodeBlockOpt.isEmpty()) { return false; } @@ -219,22 +229,32 @@ private boolean isLostVariable(VarData varData) { private RuleNode findDefNode(VarData varData) { // быстрее сначала найти узел метода в дереве, а потом уже узел переменной в дереве метода // чтобы постоянно не искать по всему дереву файла - final var methodCodeBlockContext = getMethodCodeBlockContext(varData.method, varData.isMethod); - return Trees.findNodeContainsPosition(methodCodeBlockContext, - varData.getDefRange().getStart()) - .orElseThrow(); + final var parentBlockContext = getMethodCodeBlockContext(varData.parentSymbol, varData.isMethod); + + return Trees.findContextContainsPosition(parentBlockContext, varData.getDefRange().getStart()) + .orElseGet(() -> findMethodByRange(varData.defRange)); } private BSLParserRuleContext getMethodCodeBlockContext(SourceDefinedSymbol method, boolean isMethod) { if (isMethod) { - final var methodName = method.getName(); return astBySymbol.computeIfAbsent(method, methodSymbol -> - methodContextsByMethodName.get(methodName)); + methodContextsByMethodName.get(methodSymbol.getName())); } return astBySymbol.computeIfAbsent(method, methodSymbol -> methodContextsByMethodName.get("")); } + private BSLParserRuleContext findMethodByRange(Range range) { + final var methodContext = documentContext.getSymbolTree().getMethods().stream() + .filter(methodSymbol -> Ranges.containsRange(methodSymbol.getRange(), range)) + .map(methodSymbol -> methodContextsByMethodName.get(methodSymbol.getName())) + .filter(Objects::nonNull) + .findFirst() + .orElseThrow(); + return Trees.findContextContainsPosition(methodContext, range.getStart()) + .orElseThrow(); + } + private static boolean isForInside(RuleNode defNode) { return defNode instanceof BSLParser.ForStatementContext || defNode instanceof BSLParser.ForEachStatementContext; } @@ -308,13 +328,18 @@ private static boolean hasReferenceOutsideRewriteBlock(List reference } private void fireIssue(VarData varData) { - final var relatedInformationList = varData.getReferences().stream() + var resultRefs = new ArrayList(); + resultRefs.add(RelatedInformation.create( + documentContext.getUri(), + varData.rewriteRange, + "+1")); + resultRefs.addAll(varData.getReferences().stream() .map(context -> RelatedInformation.create( documentContext.getUri(), context.getSelectionRange(), "+1" - )).collect(Collectors.toList()); - final var message = info.getMessage(varData.getName()); - diagnosticStorage.addDiagnostic(varData.getDefRange(), message, relatedInformationList); + )).collect(Collectors.toList())); + final var message = info.getMessage(varData.variable.getName()); + diagnosticStorage.addDiagnostic(varData.getDefRange(), message, resultRefs); } } diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 1d6ffff2bd7..5758830bd2c 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -524,10 +524,39 @@ public static Optional findTerminalNodeContainsPosition(BSLParserR * @param position - искомая позиция * @return нода на указанной позиции, если есть */ - public static Optional findNodeContainsPosition(BSLParserRuleContext tree, Position position) { - return findTerminalNodeContainsPosition(tree, position) - .map(TerminalNode::getParent) - .map(BSLParserRuleContext.class::cast); + public static Optional findContextContainsPosition(BSLParserRuleContext tree, Position position) { + if (!nodeContainsPosition(tree, position)) { + return Optional.empty(); + } + return Optional.ofNullable(findContextContainsPositionInner(tree, position)); + } + + private static @Nullable BSLParserRuleContext findContextContainsPositionInner(BSLParserRuleContext tree, Position position) { + + var children = Trees.getChildren(tree); + + var isOneElemSize = children.size() == 1; + for (Tree child : children) { + if ((child instanceof TerminalNode) + || (!isOneElemSize && !nodeContainsPosition((BSLParserRuleContext) child, position))) { + continue; + } + BSLParserRuleContext node = findContextContainsPositionInner((BSLParserRuleContext) child, position); + if (node != null) { + return node; + } + } + return tree; + } + + private static boolean nodeContainsPosition(BSLParserRuleContext tree, Position position) { + var start = tree.getStart(); + var stop = tree.getStop(); + if (start == null || stop == null){ + return false; + } + + return positionIsAfterOrOnToken(position, start) && positionIsBeforeOrOnToken(position, stop); } /** diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 8c062e4ac05..9e3c1c7d2ca 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -41,7 +41,7 @@ void test() { List diagnostics = getDiagnostics(); assertThat(diagnostics, true) - .hasMessageOnRange(getMessage("Значение"), 1, 4, 12) + .hasMessageOnRange(getMessage("Значение"), 2, 4, 12) .hasMessageOnRange(getMessage("МояПеременная"), 4, 4, 17) .hasMessageOnRange(getMessage("ТекстЗапроса"), 9, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 23, 4, 16) @@ -56,8 +56,9 @@ void test() { .hasMessageOnRange(getMessage("Представление"), 254, 4, 17) .hasMessageOnRange(getMessage("ЛишниеТэги"), 275, 8, 18) .hasMessageOnRange(getMessage("ТекстЗапроса"), 297, 4, 16) - .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 304, 0, 18) - .hasSize(16); + .hasMessageOnRange(getMessage("ЗначениеМодуля"), 305, 4, 18) + .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 310, 0, 18) + .hasSize(17); ; } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 3f5ece881b8..713791f6ba3 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -1,7 +1,7 @@ +Перем ЗначениеМодуля; Процедура Тест1() Значение = 1; // ошибка Значение = 2; - МояПеременная = СтрЗаменить(КакаятоПеременная); // ошибка МояПеременная = СтрЗаменить(КакаятоДругаяПеременная); КонецПроцедуры @@ -302,6 +302,12 @@ #КонецОбласти КонецПроцедуры +Процедура Тест18() + ЗначениеМодуля = "Первый"; // ошибка + ЗначениеМодуля = "Второй"; + Запрос = Новый Запрос(ЗначениеМодуля); +КонецПроцедуры + ТекстЗапросаВБлоке = "Первый"; // ошибка ТекстЗапросаВБлоке = "Второй"; Запрос = Новый Запрос(ТекстЗапросаВБлоке); From 3240ea15a4176925dd2de0cf9d7d652a83e3654b Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 22 Jun 2022 21:08:46 +0300 Subject: [PATCH 31/45] =?UTF-8?q?=D0=BD=D0=B5=D0=B1=D0=BE=D0=BB=D1=8C?= =?UTF-8?q?=D1=88=D0=BE=D0=B5=20=D1=83=D1=81=D0=BA=D0=BE=D1=80=D0=B5=D0=BD?= =?UTF-8?q?=D0=B8=D0=B5=20=D1=80=D0=B0=D1=81=D1=87=D0=B5=D1=82=D0=B0=20?= =?UTF-8?q?=D0=B3=D0=BB=D0=BE=D0=B1=D0=B0=D0=BB=D1=8C=D0=BD=D1=8B=D1=85\?= =?UTF-8?q?=D0=BC=D0=BE=D0=B4=D1=83=D0=BB=D1=8C=D0=BD=D1=8B=D1=85=20=D0=BF?= =?UTF-8?q?=D0=B5=D1=80=D0=B5=D0=BC=D0=B5=D0=BD=D0=BD=D1=8B=D1=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 9bacb197a91..cdff511c981 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -45,7 +45,6 @@ import org.eclipse.lsp4j.DiagnosticRelatedInformation; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; -import org.apache.commons.collections4.map.CaseInsensitiveMap; import java.util.ArrayList; import java.util.Collections; @@ -56,6 +55,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeMap; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -78,7 +78,7 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private final ReferenceIndex referenceIndex; private final Map astBySymbol = new HashMap<>(); - private Map methodContextsByMethodName = new CaseInsensitiveMap<>(); + private Map methodContextsByMethodName; @Value private static class VarData { @@ -88,6 +88,7 @@ private static class VarData { List references; SourceDefinedSymbol parentSymbol; boolean isMethod; + boolean isGlobalOrModuleKind; } @Override @@ -105,12 +106,15 @@ protected void check() { } private Map getMethodContextsByMethodName() { + if (documentContext.getSymbolTree().getMethods().isEmpty()){ + return new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + } return Optional.ofNullable(documentContext.getAst().subs()) .map(BSLParser.SubsContext::sub) .map(subContexts -> subContexts.stream() .map(LostVariableDiagnostic::getMethodNameAndContext) .collect(Collectors.toMap(Pair::getLeft, Pair::getRight))) - .orElseGet(Collections::emptyMap); + .orElseThrow(); } private static Pair getMethodNameAndContext(BSLParser.SubContext subContext) { @@ -167,7 +171,8 @@ private List getConsecutiveDefinitions(VariableSymbol variable, List getConsecutiveDefinitions(VariableSymbol variable, List findMethodByRange(varData.defRange)); + .orElseThrow(); } private BSLParserRuleContext getMethodCodeBlockContext(SourceDefinedSymbol method, boolean isMethod) { From b15ba7e8955bce2d9cc0c4b2d2785ba005e99d99 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Wed, 22 Jun 2022 21:24:16 +0300 Subject: [PATCH 32/45] =?UTF-8?q?=D1=80=D0=B5=D1=84=D0=B0=D0=BA=D1=82?= =?UTF-8?q?=D0=BE=D1=80=D0=B8=D0=BD=D0=B3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index cdff511c981..7f1f9220a30 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -89,6 +89,13 @@ private static class VarData { SourceDefinedSymbol parentSymbol; boolean isMethod; boolean isGlobalOrModuleKind; + + private static VarData of(VariableSymbol variable, Range defRange, Reference rewriteReference, + SourceDefinedSymbol methodSymbol, List references) { + return new VarData(variable, defRange, + rewriteReference.getSelectionRange(), references, methodSymbol, methodSymbol instanceof MethodSymbol, + GlobalVariableKinds.contains(variable.getKind())); + } } @Override @@ -161,7 +168,7 @@ private List getVarData(VariableSymbol variable, SourceDefinedSymbol me return getConsecutiveDefinitions(variable, allReferences, methodSymbol); } - private List getConsecutiveDefinitions(VariableSymbol variable, List allReferences, + private static List getConsecutiveDefinitions(VariableSymbol variable, List allReferences, SourceDefinedSymbol methodSymbol) { List result = new ArrayList<>(); Reference prev = null; @@ -170,9 +177,8 @@ private List getConsecutiveDefinitions(VariableSymbol variable, List getConsecutiveDefinitions(VariableSymbol variable, List Date: Thu, 23 Jun 2022 16:48:37 +0300 Subject: [PATCH 33/45] =?UTF-8?q?=D1=83=D1=81=D0=BA=D0=BE=D1=80=D0=B5?= =?UTF-8?q?=D0=BD=20=D0=BC=D0=B5=D1=82=D0=BE=D0=B4=20getReferencesTo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit исключен ненужный расчет символа --- .../references/ReferenceIndex.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java index 0615105e9e8..a6f83bc8d56 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java @@ -89,7 +89,7 @@ public List getReferencesTo(SourceDefinedSymbol symbol) { return symbolOccurrenceRepository.getAllBySymbol(symbolDto) .stream() - .map(this::buildReference) + .map(symbolOccurrence -> buildReference(symbolOccurrence, symbol)) .flatMap(Optional::stream) .collect(Collectors.toList()); } @@ -232,6 +232,16 @@ public void addVariableUsage(URI uri, locationRepository.updateLocation(symbolOccurrence); } + private Optional buildReference(SymbolOccurrence symbolOccurrence, SourceDefinedSymbol symbol) { + var uri = symbolOccurrence.getLocation().getUri(); + var range = symbolOccurrence.getLocation().getRange(); + var occurrenceType = symbolOccurrence.getOccurrenceType(); + + SourceDefinedSymbol from = getFromSymbol(symbolOccurrence); + return Optional.of(new Reference(from, symbol, uri, range, occurrenceType)) + .filter(ReferenceIndex::isReferenceAccessible); + } + private Optional buildReference( SymbolOccurrence symbolOccurrence ) { @@ -280,8 +290,8 @@ private SourceDefinedSymbol getFromSymbol(SymbolOccurrence symbolOccurrence) { .filter(sourceDefinedSymbol -> sourceDefinedSymbol.getSymbolKind() != SymbolKind.Namespace) .filter(symbol -> Ranges.containsPosition(symbol.getRange(), position)) .findFirst() - .or(() -> symbolTree.map(SymbolTree::getModule)) - .orElseThrow(); + .or(() -> symbolTree.map(SymbolTree::getModule)) + .orElseThrow(); } private static boolean isReferenceAccessible(Reference reference) { From 9f1815bf43189b3bcde414abd5930d5352b01e0d Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Thu, 23 Jun 2022 16:50:30 +0300 Subject: [PATCH 34/45] =?UTF-8?q?=D0=BA=D0=B5=D0=B9=D1=81=20=D0=B3=D0=BB?= =?UTF-8?q?=D0=BE=D0=B1=D0=B0=D0=BB=D1=8C=D0=BD=D0=B0=D1=8F=20=D0=BF=D0=B5?= =?UTF-8?q?=D1=80=D0=B5=D0=BC=D0=B5=D0=BD=D0=BD=D0=B0=D1=8F=20=D0=B2=20?= =?UTF-8?q?=D1=82=D0=B5=D0=BB=D0=B5=20=D0=BC=D0=BE=D0=B4=D1=83=D0=BB=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 41 ++++++++++--------- .../LostVariableDiagnosticTest.java | 4 +- .../diagnostics/LostVariableDiagnostic.bsl | 4 ++ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 7f1f9220a30..790429ba051 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -75,6 +75,7 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private static final Set GlobalVariableKinds = EnumSet.of(VariableKind.GLOBAL, VariableKind.MODULE); + private static final String MODULE_SCOPE_NAME = ""; private final ReferenceIndex referenceIndex; private final Map astBySymbol = new HashMap<>(); @@ -102,7 +103,7 @@ private static VarData of(VariableSymbol variable, Range defRange, Reference rew protected void check() { methodContextsByMethodName = getMethodContextsByMethodName(); getFileCodeBlock() - .ifPresent(fileCodeBlock -> methodContextsByMethodName.put("", fileCodeBlock)); + .ifPresent(fileCodeBlock -> methodContextsByMethodName.put(MODULE_SCOPE_NAME, fileCodeBlock)); getVariables() .filter(this::isLostVariable) @@ -238,34 +239,36 @@ private boolean isLostVariable(VarData varData) { } private RuleNode findDefNode(VarData varData) { + final BSLParserRuleContext parentBlockContext; if (varData.isGlobalOrModuleKind) { - return findMethodByRange(varData.defRange); + parentBlockContext = findCodeBlockContextByRange(varData.defRange); + } else { + // быстрее сначала найти узел метода в дереве, а потом уже узел переменной в дереве метода + // чтобы постоянно не искать по всему дереву файла + parentBlockContext = getCodeBlockContextBySymbol(varData.parentSymbol, varData.isMethod); } - // быстрее сначала найти узел метода в дереве, а потом уже узел переменной в дереве метода - // чтобы постоянно не искать по всему дереву файла - final var parentBlockContext = getMethodCodeBlockContext(varData.parentSymbol, varData.isMethod); - return Trees.findContextContainsPosition(parentBlockContext, varData.getDefRange().getStart()) + return Trees.findContextContainsPosition(parentBlockContext, varData.defRange.getStart()) + .orElseThrow(); + } + + private BSLParserRuleContext findCodeBlockContextByRange(Range range) { + final var methodName = documentContext.getSymbolTree().getMethods().stream() + .filter(methodSymbol -> Ranges.containsRange(methodSymbol.getRange(), range)) + .map(MethodSymbol::getName) + .findFirst() + .orElse(MODULE_SCOPE_NAME); + return Optional.ofNullable(methodContextsByMethodName.get(methodName)) + .filter(Objects::nonNull) .orElseThrow(); } - private BSLParserRuleContext getMethodCodeBlockContext(SourceDefinedSymbol method, boolean isMethod) { + private BSLParserRuleContext getCodeBlockContextBySymbol(SourceDefinedSymbol method, boolean isMethod) { if (isMethod) { return astBySymbol.computeIfAbsent(method, methodSymbol -> methodContextsByMethodName.get(methodSymbol.getName())); } return astBySymbol.computeIfAbsent(method, methodSymbol -> - methodContextsByMethodName.get("")); - } - - private BSLParserRuleContext findMethodByRange(Range range) { - final var methodContext = documentContext.getSymbolTree().getMethods().stream() - .filter(methodSymbol -> Ranges.containsRange(methodSymbol.getRange(), range)) - .map(methodSymbol -> methodContextsByMethodName.get(methodSymbol.getName())) - .filter(Objects::nonNull) - .findFirst() - .orElseThrow(); - return Trees.findContextContainsPosition(methodContext, range.getStart()) - .orElseThrow(); + methodContextsByMethodName.get(MODULE_SCOPE_NAME)); } private static boolean isForInside(RuleNode defNode) { diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 9e3c1c7d2ca..30d4efc448a 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -58,8 +58,8 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 297, 4, 16) .hasMessageOnRange(getMessage("ЗначениеМодуля"), 305, 4, 18) .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 310, 0, 18) - .hasSize(17); - ; + .hasMessageOnRange(getMessage("ЗначениеМодуля"), 314, 0, 14) + .hasSize(18); } String getMessage(String name){ diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 713791f6ba3..3ec47088575 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -311,3 +311,7 @@ ТекстЗапросаВБлоке = "Первый"; // ошибка ТекстЗапросаВБлоке = "Второй"; Запрос = Новый Запрос(ТекстЗапросаВБлоке); + +ЗначениеМодуля = "Первый"; // ошибка +ЗначениеМодуля = "Второй"; +Запрос1 = Новый Запрос(ЗначениеМодуля); From e9eb5ade2979876fa6c7a7f0a9f888691df491cb Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Fri, 24 Jun 2022 00:40:40 +0300 Subject: [PATCH 35/45] =?UTF-8?q?=D0=BA=D0=B5=D0=B9=D1=81:=D0=BF=D0=B5?= =?UTF-8?q?=D1=80=D0=B5=D0=BC=D0=B5=D0=BD=D0=BD=D1=8B=D0=B5=20=D0=B8=D1=81?= =?UTF-8?q?=D0=BF=D0=BE=D0=BB=D1=8C=D0=B7=D0=BE=D0=B2=D0=B0=D0=BD=D1=8B=20?= =?UTF-8?q?=D0=B8=20=D0=B7=D0=B0=D0=BD=D0=BE=D0=B2=D0=BE=20=D0=BF=D0=B5?= =?UTF-8?q?=D1=80=D0=B5=D0=BF=D0=B8=D1=81=D0=B0=D0=BD=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit уже без использования - внесен падающий тест для дальнейшей реализации --- .../diagnostics/LostVariableDiagnostic.java | 101 +++++++++++++----- .../LostVariableDiagnostic_en.properties | 2 + .../LostVariableDiagnostic_ru.properties | 2 + .../LostVariableDiagnosticTest.java | 29 ++++- .../diagnostics/LostVariableDiagnostic.bsl | 52 ++++++--- 5 files changed, 139 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 790429ba051..8043f6e6656 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -46,6 +46,7 @@ import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.SymbolKind; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; @@ -68,7 +69,6 @@ DiagnosticTag.UNPREDICTABLE, DiagnosticTag.BADPRACTICE } - ) @RequiredArgsConstructor @@ -76,29 +76,12 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private static final Set GlobalVariableKinds = EnumSet.of(VariableKind.GLOBAL, VariableKind.MODULE); private static final String MODULE_SCOPE_NAME = ""; + private static final String UNUSED_MESSAGE = "unusedMessage"; private final ReferenceIndex referenceIndex; private final Map astBySymbol = new HashMap<>(); private Map methodContextsByMethodName; - @Value - private static class VarData { - VariableSymbol variable; - Range defRange; - Range rewriteRange; - List references; - SourceDefinedSymbol parentSymbol; - boolean isMethod; - boolean isGlobalOrModuleKind; - - private static VarData of(VariableSymbol variable, Range defRange, Reference rewriteReference, - SourceDefinedSymbol methodSymbol, List references) { - return new VarData(variable, defRange, - rewriteReference.getSelectionRange(), references, methodSymbol, methodSymbol instanceof MethodSymbol, - GlobalVariableKinds.contains(variable.getKind())); - } - } - @Override protected void check() { methodContextsByMethodName = getMethodContextsByMethodName(); @@ -207,15 +190,37 @@ private static List getConsecutiveDefinitions(VariableSymbol variable, } prev = null; } + if (!isGlobalVar && variable.getKind() != VariableKind.PARAMETER){ + final var lastDefinition = getLastDefinition(allReferences); + if (lastDefinition != null){ + result.add(VarData.ofFinished(variable, lastDefinition, methodSymbol)); + } + } return result; } + @Nullable + private static Reference getLastDefinition(List referencesTo) { + var reverseIterator = referencesTo.listIterator(referencesTo.size()); + + if(reverseIterator.hasPrevious()) { + final var ref = reverseIterator.previous(); + if (ref.getOccurrenceType() == OccurrenceType.DEFINITION){ + return ref; + } + } + return null; + } + private boolean isLostVariable(VarData varData) { final RuleNode defNode = findDefNode(varData); if (isForInside(defNode)){ return false; } + if (varData.isFinished){ + return true; + } var defCodeBlockOpt = getCodeBlock(defNode); final var rewriteNodeInsideDefCodeBlockOpt = defCodeBlockOpt @@ -344,18 +349,58 @@ private static boolean hasReferenceOutsideRewriteBlock(List reference } private void fireIssue(VarData varData) { - var resultRefs = new ArrayList(); - resultRefs.add(RelatedInformation.create( - documentContext.getUri(), - varData.rewriteRange, - "+1")); - resultRefs.addAll(varData.getReferences().stream() + final String message = getMessage(varData); + diagnosticStorage.addDiagnostic(varData.getDefRange(), message, getDiagnosticReferences(varData)); + } + + private String getMessage(VarData varData) { + if (varData.isFinished){ + return info.getResourceString(UNUSED_MESSAGE, varData.variable.getName()); + } + return info.getMessage(varData.variable.getName()); + } + + private List getDiagnosticReferences(VarData varData) { + final var references = varData.getReferences().stream() .map(context -> RelatedInformation.create( documentContext.getUri(), context.getSelectionRange(), "+1" - )).collect(Collectors.toList())); - final var message = info.getMessage(varData.variable.getName()); - diagnosticStorage.addDiagnostic(varData.getDefRange(), message, resultRefs); + )).collect(Collectors.toList()); + if (varData.isFinished) { + return references; + } + final var result = new ArrayList(); + result.add(RelatedInformation.create( + documentContext.getUri(), + varData.rewriteRange, + "+1")); + result.addAll(references); + return result; + } + + @Value + private static class VarData { + VariableSymbol variable; + Range defRange; + Range rewriteRange; + List references; + SourceDefinedSymbol parentSymbol; + boolean isMethod; + boolean isGlobalOrModuleKind; + boolean isFinished; + + private static VarData of(VariableSymbol variable, Range defRange, Reference rewriteReference, + SourceDefinedSymbol methodSymbol, List references) { + return new VarData(variable, defRange, + rewriteReference.getSelectionRange(), references, methodSymbol, methodSymbol instanceof MethodSymbol, + GlobalVariableKinds.contains(variable.getKind()), false); + } + + private static VarData ofFinished(VariableSymbol variable, Reference lastDefinition, SourceDefinedSymbol methodSymbol) { + return new VarData(variable, lastDefinition.getSelectionRange(), lastDefinition.getSelectionRange(), + Collections.emptyList(), methodSymbol, methodSymbol instanceof MethodSymbol, + GlobalVariableKinds.contains(variable.getKind()), true); + } } } diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties index 28658461359..95061b9eaf2 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties @@ -1,2 +1,4 @@ diagnosticMessage=The value of variable <%s> is not used, the variable is rewritten diagnosticName=Lost variable + +unusedMessage=The value of variable <%s> is not used further diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties index 05211d4a92f..b9801d0beab 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties @@ -1,2 +1,4 @@ diagnosticMessage=Значение переменной <%s> не используется, переменная перезаписывается дальше по коду diagnosticName=Потерянная переменная + +unusedMessage=Значение переменной <%s> не используется далее diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 30d4efc448a..5341542d2bc 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -42,27 +42,48 @@ void test() { assertThat(diagnostics, true) .hasMessageOnRange(getMessage("Значение"), 2, 4, 12) + .hasMessageOnRange(getMessageUnused("Значение"), 3, 4, 12) .hasMessageOnRange(getMessage("МояПеременная"), 4, 4, 17) + .hasMessageOnRange(getMessageUnused("МояПеременная"), 5, 4, 17) .hasMessageOnRange(getMessage("ТекстЗапроса"), 9, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 23, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 31, 4, 16) .hasMessageOnRange(getMessage("ТекстЗапроса"), 53, 7, 19) .hasMessageOnRange(getMessage("ТекстЗапроса"), 69, 2, 14) + .hasMessageOnRange(getMessageUnused("Запрос"), 82, 2, 8) + .hasMessageOnRange(getMessageUnused("Запрос"), 95, 2, 8) + .hasMessageOnRange(getMessageUnused("ТекстЗапроса"), 101, 6, 18) .hasMessageOnRange(getMessage("Файл"), 111, 4, 8) + .hasMessageOnRange(getMessageUnused("Файл"), 116, 4, 8) + .hasMessageOnRange(getMessageUnused("ЛокальнаяПеременная"), 127, 8, 27) .hasMessageOnRange(getMessage("Комментарий"), 139, 4, 15) + .hasMessageOnRange(getMessageUnused("Комментарий"), 139, 21, 32) .hasMessageOnRange(getMessage("ВидПрава"), 159, 4, 12) + .hasMessageOnRange(getMessageUnused("ВидПрава"), 160, 4, 12) .hasMessageOnRange(getMessage("НовыйПереход"), 163, 4, 16) + .hasMessageOnRange(getMessageUnused("НовыйПереход"), 164, 4, 16) .hasMessageOnRange(getMessage("ЭтоОшибкаБлокировки"), 188, 8, 27) + .hasMessageOnRange(getMessageUnused("ЭтоОшибкаБлокировки"), 210, 8, 27) + .hasMessageOnRange(getMessageUnused("мСохраненныйДок"), 243, 4, 19) .hasMessageOnRange(getMessage("Представление"), 254, 4, 17) .hasMessageOnRange(getMessage("ЛишниеТэги"), 275, 8, 18) + .hasMessageOnRange(getMessageUnused("ЛишниеТэги"), 276, 8, 18) .hasMessageOnRange(getMessage("ТекстЗапроса"), 297, 4, 16) .hasMessageOnRange(getMessage("ЗначениеМодуля"), 305, 4, 18) - .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 310, 0, 18) - .hasMessageOnRange(getMessage("ЗначениеМодуля"), 314, 0, 14) - .hasSize(18); - + .hasMessageOnRange(getMessage("ТекстЗапроса"), 311, 4, 16) + .hasMessageOnRange(getMessageUnused("ТекстЗапроса"), 314, 4, 16) + .hasMessageOnRange(getMessage("ЗначениеМодуля"), 318, 4, 18) +// .hasMessageOnRange(getMessageUnused("Значение"), 328, 8, 16) + .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 332, 0, 18) + .hasMessageOnRange(getMessage("ЗначениеМодуля"), 336, 0, 14) + .hasSize(34); } + String getMessage(String name){ return String.format("Значение переменной <%s> не используется, переменная перезаписывается дальше по коду", name); } + + String getMessageUnused(String name){ + return String.format("Значение переменной <%s> не используется далее", name); + } } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 3ec47088575..81d5ebc3373 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -1,9 +1,9 @@ Перем ЗначениеМодуля; Процедура Тест1() Значение = 1; // ошибка - Значение = 2; + Значение = 2; // ошибка МояПеременная = СтрЗаменить(КакаятоПеременная); // ошибка - МояПеременная = СтрЗаменить(КакаятоДругаяПеременная); + МояПеременная = СтрЗаменить(КакаятоДругаяПеременная); // ошибка КонецПроцедуры Процедура Тест2() @@ -80,7 +80,7 @@ ТекстЗапроса = "Второй";// не ошибка Запрос = Новый Запрос(ТекстЗапроса); КонецЕсли; - Запрос = Новый Запрос(ТекстЗапроса); + Запрос = Новый Запрос(ТекстЗапроса); // ошибка КонецПроцедуры Процедура Тест12() @@ -93,13 +93,13 @@ КонецЕсли; //Запрос = Новый Запрос(ТекстЗапроса); КонецЕсли; - Запрос = Новый Запрос(ТекстЗапроса); + Запрос = Новый Запрос(ТекстЗапроса); // ошибка КонецПроцедуры Процедура Тест13() ТекстЗапроса = "Первый"; // не ошибка Если Условие Тогда - ТекстЗапроса = "Второй"; + ТекстЗапроса = "Второй"; // ошибка КонецЕсли; КонецПроцедуры @@ -114,7 +114,7 @@ НачальныйКаталог = КаталогФич; КаталогПоиска = НачальныйКаталог; - Файл = Новый Файл(НачальныйКаталог); + Файл = Новый Файл(НачальныйКаталог); // ошибка КонецПроцедуры //Или другой пример правильного кода (изменение происходит только при определённых условиях): @@ -125,7 +125,7 @@ ИначеЕсли ДругоеУсловие Тогда ЛокальнаяПеременная = ВызовФункции(); Иначе - ЛокальнаяПеременная = 0; + ЛокальнаяПеременная = 0; // ошибка КонецЕсли; КонецПроцедуры @@ -137,7 +137,7 @@ КонецПроцедуры Функция ВыраженияНаОднойСтроке() - Комментарий = 10;Комментарий = 20; // ошибка (важно, что нет пробела после 10;) + Комментарий = 10;Комментарий = 20; // сразу 2 ошибки (важно, что нет пробела после 10;) Возврат Неопределено; КонецФункции @@ -158,7 +158,7 @@ Процедура Тест15() ВидПрава = ВидыПрав.Добавить(); // ошибка - ВидПрава = ВидыПрав.Добавить(); + ВидПрава = ВидыПрав.Добавить(); // ошибка // ошибка НовыйПереход = ТаблицаПереходовНоваяСтрока("Начало", "СтраницаНавигацииНачало"); // нет ошибок @@ -208,7 +208,7 @@ ЗафиксироватьТранзакцию(); Исключение ОтменитьТранзакцию(); - ЭтоОшибкаБлокировки = Ложь; + ЭтоОшибкаБлокировки = Ложь; // ошибка КонецПопытки; КонецПроцедуры @@ -231,17 +231,17 @@ ТабличныйДокумент.Присоединить(ПустаяЯчейка); КонецЦикла; - Для НомерЯчейки = 1 По КоличествоЯчеекДляЗаполнения - 2 Цикл + Для НомерЯчейки = 1 По КоличествоЯчеекДляЗаполнения - 2 Цикл // не ошибка ПечатьСтроки(ЗначениеЗаполнения); КонецЦикла; КонецПроцедуры Процедура Событие(Параметры, Отказ) - мСохраненныйДок = Неопределено; // не ошибка + мСохраненныйДок = Неопределено; // не ошибка Если Параметры.Свойство("АвтоТест") Тогда Возврат; КонецЕсли; - мСохраненныйДок = Параметры.ЗначениеКопирования; + мСохраненныйДок = Параметры.ЗначениеКопирования; // ошибка КонецПроцедуры &НаКлиенте @@ -252,7 +252,7 @@ &НаСервереБезКонтекста Функция ПредставлениеЧастиАдреса_ЕДТ(АдресСтруктура, СписокПолей) - Представление = ""; // не ошибка, если включен флаг Игнорировать типизацию для ЕДТ + Представление = ""; // ошибка СтруктураПолей = Новый Структура(СписокПолей); ЗаполнитьЗначенияСвойств(СтруктураПолей, АдресСтруктура); @@ -274,7 +274,7 @@ ЕстьТэгиМодуляОбновления = ОбъектыОбработчика.Колонки.Найти("ТэгиМодуляОбновления") <> Неопределено; Если ТэгиМодуля = Неопределено Тогда ЛишниеТэги = Новый Массив; // не ошибка, если включен флаг Игнорировать типизацию для ЕДТ - ЛишниеТэги = ЛишниеТэгиВОбласти(ВнешниеТэги, РазметкаКода); + ЛишниеТэги = ЛишниеТэгиВОбласти(ВнешниеТэги, РазметкаКода); // ошибка Возврат ОбъектыОбработчика; КонецЕсли; @@ -308,6 +308,28 @@ Запрос = Новый Запрос(ЗначениеМодуля); КонецПроцедуры +Процедура Тест19() + ТекстЗапроса = "Первый"; // ошибка + ТекстЗапроса = "Второй"; + Запрос = Новый Запрос(ТекстЗапроса); + ТекстЗапроса = "Третий"; // также ошибка +КонецПроцедуры + +Процедура Тест20() + ЗначениеМодуля = "Первый"; // ошибка + ЗначениеМодуля = "Второй"; + Запрос = Новый Запрос(ЗначениеМодуля); + ЗначениеМодуля = "Третий"; // не ошибка +КонецПроцедуры + +Процедура Тест21() + Значение = 10; // не ошибка + Для Каждого Элем ИЗ Коллекция Цикл + Элем.Реквизит = Вычисление(Значение); + Значение = 20; // не ошибка + КонецЦикла; +КонецПроцедуры + ТекстЗапросаВБлоке = "Первый"; // ошибка ТекстЗапросаВБлоке = "Второй"; Запрос = Новый Запрос(ТекстЗапросаВБлоке); From bf542e209741983012d5b0b3b27ba72592a4b7f5 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 24 Jul 2022 14:52:47 +0300 Subject: [PATCH 36/45] =?UTF-8?q?=D0=B8=D1=81=D0=BF=D1=80=D0=B0=D0=B2?= =?UTF-8?q?=D0=BB=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=BD=D0=B5=D1=82=D0=BE=D1=87?= =?UTF-8?q?=D0=BD=D0=BE=D0=B3=D0=BE=20=D1=81=D0=BB=D0=B8=D1=8F=D0=BD=D0=B8?= =?UTF-8?q?=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../bsl/languageserver/utils/Trees.java | 40 ------------------- 1 file changed, 40 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 1be8b091c16..5758830bd2c 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -477,46 +477,6 @@ public static boolean nodeContains(ParseTree t, ParseTree exclude, Integer... in .anyMatch(i -> nodeContains(t.getChild(i), exclude, index)); } - /** - * Получение ноды в дереве по позиции в документе. - * - * @param tree - дерево, в котором ищем - * @param position - искомая позиция - * @return терминальная нода на указанной позиции, если есть - */ - public static Optional findTerminalNodeContainsPosition(BSLParserRuleContext tree, Position position) { - - if (tree.getTokens().isEmpty()) { - return Optional.empty(); - } - - var start = tree.getStart(); - var stop = tree.getStop(); - - if (!(positionIsAfterOrOnToken(position, start) && positionIsBeforeOrOnToken(position, stop))) { - return Optional.empty(); - } - - var children = Trees.getChildren(tree); - - for (Tree child : children) { - if (child instanceof TerminalNode) { - var terminalNode = (TerminalNode) child; - var token = terminalNode.getSymbol(); - if (tokenContainsPosition(token, position)) { - return Optional.of(terminalNode); - } - } else { - Optional node = findTerminalNodeContainsPosition((BSLParserRuleContext) child, position); - if (node.isPresent()) { - return node; - } - } - } - - return Optional.empty(); - } - /** * Получение терминальной ноды в дереве по позиции в документе. * From e15edd84eb507b5ad8749e435a0ec183f3e4c2f5 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sun, 24 Jul 2022 14:53:23 +0300 Subject: [PATCH 37/45] TODO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit для лучшего понимания --- .../languageserver/diagnostics/LostVariableDiagnosticTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 5341542d2bc..545b2538cf2 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -73,7 +73,7 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 311, 4, 16) .hasMessageOnRange(getMessageUnused("ТекстЗапроса"), 314, 4, 16) .hasMessageOnRange(getMessage("ЗначениеМодуля"), 318, 4, 18) -// .hasMessageOnRange(getMessageUnused("Значение"), 328, 8, 16) +// .hasMessageOnRange(getMessageUnused("Значение"), 328, 8, 16) // TODO не должно быть ошибкой .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 332, 0, 18) .hasMessageOnRange(getMessage("ЗначениеМодуля"), 336, 0, 14) .hasSize(34); From 0400d357e64f367d1e4b918c064fea7066cb76ba Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sat, 26 Nov 2022 19:03:10 +0300 Subject: [PATCH 38/45] =?UTF-8?q?=D1=83=D1=82=D0=BE=D1=87=D0=BD=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BE=D0=B1=D1=80=D0=B0=D0=B1=D0=BE=D1=82=D0=BA=D1=83?= =?UTF-8?q?=20=D1=86=D0=B8=D0=BA=D0=BB=D0=BE=D0=B2=20=D0=B8=20=D0=B4=D0=BE?= =?UTF-8?q?=D0=BF.=D0=BA=D0=B5=D0=B9=D1=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ловится переустановка итератора во вложенном цикле - кейс с переустановкой значения --- .../diagnostics/LostVariableDiagnostic.java | 29 ++++++++++++------- .../LostVariableDiagnosticTest.java | 10 ++++--- .../diagnostics/LostVariableDiagnostic.bsl | 11 +++++-- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 8043f6e6656..da2f844ddcf 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -215,10 +215,14 @@ private static Reference getLastDefinition(List referencesTo) { private boolean isLostVariable(VarData varData) { final RuleNode defNode = findDefNode(varData); - if (isForInside(defNode)){ - return false; - } - if (varData.isFinished){ + final var codeBlockForLoopIndex = codeBlockForLoopIndex(defNode); + if (codeBlockForLoopIndex.isPresent()) { + // пропускаю неиспользуемый итератор или счетчик цикла, т.е. есть существующее правило + if (Ranges.compare(varData.defRange, varData.rewriteRange) == 0 && varData.references.isEmpty() + || !Ranges.containsRange(Ranges.create(codeBlockForLoopIndex.get()), varData.rewriteRange)) { + return false; + } + } else if (varData.isFinished()){ return true; } @@ -232,6 +236,7 @@ private boolean isLostVariable(VarData varData) { var defCodeBlock = defCodeBlockOpt.orElseThrow(); var rewriteNode = rewriteNodeInsideDefCodeBlockOpt.get(); + var rewriteStatement = getRootStatement(rewriteNode); var rewriteCodeBlock = getCodeBlock(rewriteStatement).orElseThrow(); @@ -276,8 +281,15 @@ private BSLParserRuleContext getCodeBlockContextBySymbol(SourceDefinedSymbol met methodContextsByMethodName.get(MODULE_SCOPE_NAME)); } - private static boolean isForInside(RuleNode defNode) { - return defNode instanceof BSLParser.ForStatementContext || defNode instanceof BSLParser.ForEachStatementContext; + private static Optional codeBlockForLoopIndex(RuleNode defNode) { + if (defNode instanceof BSLParser.ForStatementContext){ + return Optional.of(((BSLParser.ForStatementContext) defNode) + .codeBlock()); + } else if (defNode instanceof BSLParser.ForEachStatementContext){ + return Optional.of(((BSLParser.ForEachStatementContext) defNode) + .codeBlock()); + } + return Optional.empty(); } private static Optional getCodeBlock(RuleNode context) { @@ -327,10 +339,7 @@ private static Stream getStatementsBetween(BSLParser private static boolean isLostVariableInDifferentBlocks(VarData varData, BSLParser.StatementContext rewriteStatement, BSLParser.CodeBlockContext rewriteCodeBlock) { - if (varData.references.isEmpty()) { - return false; - } - if (isRewriteAlreadyContainsFirstReference(varData, rewriteStatement)) { + if (!varData.references.isEmpty() && isRewriteAlreadyContainsFirstReference(varData, rewriteStatement)) { return false; } return !hasReferenceOutsideRewriteBlock(varData.references, rewriteCodeBlock); diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index 545b2538cf2..f12e51e819f 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -52,6 +52,7 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 69, 2, 14) .hasMessageOnRange(getMessageUnused("Запрос"), 82, 2, 8) .hasMessageOnRange(getMessageUnused("Запрос"), 95, 2, 8) + .hasMessageOnRange(getMessage("ТекстЗапроса"), 99, 2, 14) .hasMessageOnRange(getMessageUnused("ТекстЗапроса"), 101, 6, 18) .hasMessageOnRange(getMessage("Файл"), 111, 4, 8) .hasMessageOnRange(getMessageUnused("Файл"), 116, 4, 8) @@ -73,10 +74,11 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 311, 4, 16) .hasMessageOnRange(getMessageUnused("ТекстЗапроса"), 314, 4, 16) .hasMessageOnRange(getMessage("ЗначениеМодуля"), 318, 4, 18) -// .hasMessageOnRange(getMessageUnused("Значение"), 328, 8, 16) // TODO не должно быть ошибкой - .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 332, 0, 18) - .hasMessageOnRange(getMessage("ЗначениеМодуля"), 336, 0, 14) - .hasSize(34); + .hasMessageOnRange(getMessageUnused("Значение"), 328, 8, 16) + .hasMessageOnRange(getMessage("Элем22"), 333, 16, 22) + .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 339, 0, 18) + .hasMessageOnRange(getMessage("ЗначениеМодуля"), 343, 0, 14) + .hasSize(37); } String getMessage(String name){ diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index 81d5ebc3373..c3d3b6ffd99 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -97,7 +97,7 @@ КонецПроцедуры Процедура Тест13() - ТекстЗапроса = "Первый"; // не ошибка + ТекстЗапроса = "Первый"; // ошибка Если Условие Тогда ТекстЗапроса = "Второй"; // ошибка КонецЕсли; @@ -326,7 +326,14 @@ Значение = 10; // не ошибка Для Каждого Элем ИЗ Коллекция Цикл Элем.Реквизит = Вычисление(Значение); - Значение = 20; // не ошибка + Значение = 20; // ошибка + КонецЦикла; +КонецПроцедуры + +Процедура Тест22() + Для Каждого Элем22 ИЗ Коллекция1 Цикл // ошибка + Для Каждого Элем22 ИЗ Коллекция2 Цикл // не ошибка - есть правило про итератор + КонецЦикла; КонецЦикла; КонецПроцедуры From 6e571b0ce6332c90bb13d5de4d2bb7e6f430ae48 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Mon, 28 Nov 2022 16:39:56 +0300 Subject: [PATCH 39/45] =?UTF-8?q?=D0=BE=D0=B3=D1=80=D0=B0=D0=BD=D0=B8?= =?UTF-8?q?=D1=87=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=BD=D0=B0=20=D1=82=D0=B8?= =?UTF-8?q?=D0=BF=D1=8B=20=D0=BC=D0=BE=D0=B4=D1=83=D0=BB=D0=B5=D0=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit для исключения ФП рефакторинг --- .../diagnostics/LostVariableDiagnostic.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index da2f844ddcf..11fee2d1d32 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -37,6 +37,7 @@ import com.github._1c_syntax.bsl.languageserver.utils.Trees; import com.github._1c_syntax.bsl.parser.BSLParser; import com.github._1c_syntax.bsl.parser.BSLParserRuleContext; +import com.github._1c_syntax.bsl.types.ModuleType; import lombok.RequiredArgsConstructor; import lombok.Value; import org.antlr.v4.runtime.tree.ParseTree; @@ -68,6 +69,15 @@ DiagnosticTag.SUSPICIOUS, DiagnosticTag.UNPREDICTABLE, DiagnosticTag.BADPRACTICE + }, + // TODO сделать флаг для управления работой в модулях объектов и форм? ведь в них могут быть FP + modules = { + ModuleType.CommandModule, + ModuleType.CommonModule, + ModuleType.ManagerModule, + ModuleType.ValueManagerModule, + ModuleType.SessionModule, + ModuleType.UNKNOWN } ) @@ -218,8 +228,9 @@ private boolean isLostVariable(VarData varData) { final var codeBlockForLoopIndex = codeBlockForLoopIndex(defNode); if (codeBlockForLoopIndex.isPresent()) { // пропускаю неиспользуемый итератор или счетчик цикла, т.е. есть существующее правило - if (Ranges.compare(varData.defRange, varData.rewriteRange) == 0 && varData.references.isEmpty() - || !Ranges.containsRange(Ranges.create(codeBlockForLoopIndex.get()), varData.rewriteRange)) { + final var isSameLoop = Ranges.compare(varData.defRange, varData.rewriteRange) == 0 && varData.references.isEmpty(); + final var isInnerLoop = Ranges.containsRange(Ranges.create(codeBlockForLoopIndex.get()), varData.rewriteRange); + if (isSameLoop || !isInnerLoop) { return false; } } else if (varData.isFinished()){ From 3cf5f4834db3fe2aed873fe061ba7f1dc16b1935 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Thu, 1 Dec 2022 10:40:58 +0300 Subject: [PATCH 40/45] =?UTF-8?q?=D1=83=D0=B1=D1=80=D0=B0=D0=BB=20=D0=BD?= =?UTF-8?q?=D0=B5=D0=BD=D1=83=D0=B6=D0=BD=D0=BE=D0=B5=20=D0=B8=D1=81=D0=BA?= =?UTF-8?q?=D0=BB=D1=8E=D1=87=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=B8=D0=B7=20?= =?UTF-8?q?=D0=B4=D0=BE=D0=BA=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index 6bfad45ddd1..0054b81c4cb 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -129,19 +129,6 @@ КонецПроцедуры ``` -- 3 одно и то же имя счетчика в соседних циклах, причем счетчик цикла в цикле не используется -```bsl -Для НомерЯчейки = 1 По 2 Цикл // будет выдано замечание, хотя код валиден - ТабличныйДокумент.Присоединить(ПустаяЯчейка); -КонецЦикла; - -Для НомерЯчейки = 1 По КоличествоЯчеекДляЗаполнения - 2 Цикл - ПечатьСтроки(ЗначениеЗаполнения); -КонецЦикла; -``` -В этом коде счетчик `НомерЯчейки` фактически используется в обоих циклах. -Такой код лучше переписать, используя уникальные имена счетчиков в обоих циклах, что повысит читаемость и упростит сопровождение. - ## Источники * [MITRE, CWE-563 - Assignment to Variable without Use](https://cwe.mitre.org/data/definitions/563.html) From 79a702f1f0cec9f3c4dc966e4cfda71b88a5a8b2 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Thu, 1 Dec 2022 18:54:11 +0300 Subject: [PATCH 41/45] =?UTF-8?q?=D0=B4=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8?= =?UTF-8?q?=D0=BB=20=D0=BF=D0=BE=D0=BB=D0=B5=D0=B7=D0=BD=D1=8B=D0=B9=20?= =?UTF-8?q?=D0=BA=D0=B5=D0=B9=D1=81=20=D0=B2=20=D0=B4=D0=BE=D0=BA=D1=83?= =?UTF-8?q?=D0=BC=D0=B5=D0=BD=D1=82=D0=B0=D1=86=D0=B8=D1=8E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/diagnostics/LostVariable.md | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/diagnostics/LostVariable.md b/docs/diagnostics/LostVariable.md index 0054b81c4cb..4535f1abf60 100644 --- a/docs/diagnostics/LostVariable.md +++ b/docs/diagnostics/LostVariable.md @@ -66,17 +66,36 @@ ``` Пример подозрительного кода - Инициализация переменных в начале метода + +Начальная инициализация может обмануть, т.к. при переустановке переменной значение может получить другой тип. +Или в текущем коде или в будущем, при переделке кода. ```bsl Функция ПолучитьПутьФайлаВРабочемКаталоге(ДанныеФайла) - ПутьДляВозврата = ""; ПолноеИмяФайла = ""; // замечание ИмяКаталога = РабочийКаталогПользователя(); // Сперва пытаемся найти такую запись в регистре сведений. + // тут может быть неточность, т.к. ПолноеИмяФайла = ДанныеФайла.ПолноеИмяФайлаВРабочемКаталоге; ``` -- начальная установка `КэшНастроек` не имеет смысла +или похожий, чуть более сложный кейс +``` + РезультатОбновления = НовыйРезультатОбновления(); + + ДвоичныеДанные = СкачатьФайлОбработкиНаКлиентеНаСервере( + ЭтотОбъект.ПараметрыРаботы.АдресПубликацииВебВитрины, + ЭтотОбъект.ПараметрыОбновления); + + Если Не ЗначениеЗаполнено(ДвоичныеДанные) Тогда + РезультатОбновления.ТекстОшибки = "Не удалось скачать файл обработки."; + Возврат РезультатОбновления; + КонецЕсли; + + // реальный метод ниже может вернуть что угодно сейчас или в будущем + РезультатОбновления = ЭтотОбъект.УстановитьОбработкуНаСервере(ДвоичныеДанные, + ЭтотОбъект.ПараметрыРаботы); +``` **Примеры исключений** From 649bfbfec9048c566942066e75bf9aefe353580d Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sat, 17 Dec 2022 13:19:00 +0300 Subject: [PATCH 42/45] =?UTF-8?q?=D0=B8=D0=BC=D1=8F=20=D1=80=D0=B5=D1=81?= =?UTF-8?q?=D1=83=D1=80=D1=81=D0=B0=20=D1=83=D1=82=D0=BE=D1=87=D0=BD=D0=B5?= =?UTF-8?q?=D0=BD=D0=BE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic_en.properties | 2 +- .../diagnostics/LostVariableDiagnostic_ru.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties index 95061b9eaf2..b04e071cfa9 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_en.properties @@ -1,4 +1,4 @@ diagnosticMessage=The value of variable <%s> is not used, the variable is rewritten diagnosticName=Lost variable -unusedMessage=The value of variable <%s> is not used further +unusedAfterMessage=The value of variable <%s> is not used further diff --git a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties index b9801d0beab..72f6a46da46 100644 --- a/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties +++ b/src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic_ru.properties @@ -1,4 +1,4 @@ diagnosticMessage=Значение переменной <%s> не используется, переменная перезаписывается дальше по коду diagnosticName=Потерянная переменная -unusedMessage=Значение переменной <%s> не используется далее +unusedAfterMessage=Значение переменной <%s> не используется далее From 215f118c54d74b445f3db8ae807c2012026b90cc Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sat, 17 Dec 2022 13:19:21 +0300 Subject: [PATCH 43/45] =?UTF-8?q?=D0=B7=D0=B0=D0=B3=D0=BE=D1=82=D0=BE?= =?UTF-8?q?=D0=B2=D0=BA=D0=B0=20=D0=B8=D1=81=D0=BF=D1=80=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=B4=D0=BB=D1=8F=20=D1=86=D0=B8?= =?UTF-8?q?=D0=BA=D0=BB=D0=BE=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../diagnostics/LostVariableDiagnostic.java | 23 +++++++--- .../LostVariableDiagnosticTest.java | 11 +++-- .../diagnostics/LostVariableDiagnostic.bsl | 43 +++++++++++++++++-- 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java index 11fee2d1d32..f4bcba67bfa 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnostic.java @@ -49,9 +49,11 @@ import javax.annotation.Nullable; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -86,12 +88,21 @@ public class LostVariableDiagnostic extends AbstractDiagnostic { private static final Set GlobalVariableKinds = EnumSet.of(VariableKind.GLOBAL, VariableKind.MODULE); private static final String MODULE_SCOPE_NAME = ""; - private static final String UNUSED_MESSAGE = "unusedMessage"; + private static final String UNUSEDAFTER_MESSAGE = "unusedAfterMessage"; + + private static final Collection EXCLUDED_TOP_RULE_FOR_LOOP = Set.of(BSLParser.RULE_subCodeBlock, BSLParser.RULE_fileCodeBlock); + private static final Collection LOOPS; private final ReferenceIndex referenceIndex; private final Map astBySymbol = new HashMap<>(); private Map methodContextsByMethodName; + static { + final var loops = new HashSet<>(EXCLUDED_TOP_RULE_FOR_LOOP); + loops.addAll(Set.of(BSLParser.RULE_forStatement, BSLParser.RULE_forEachStatement, BSLParser.RULE_whileStatement)); + LOOPS = Set.copyOf(loops); + } + @Override protected void check() { methodContextsByMethodName = getMethodContextsByMethodName(); @@ -233,7 +244,7 @@ private boolean isLostVariable(VarData varData) { if (isSameLoop || !isInnerLoop) { return false; } - } else if (varData.isFinished()){ + } else if (varData.unusedAfter){ return true; } @@ -374,8 +385,8 @@ private void fireIssue(VarData varData) { } private String getMessage(VarData varData) { - if (varData.isFinished){ - return info.getResourceString(UNUSED_MESSAGE, varData.variable.getName()); + if (varData.unusedAfter){ + return info.getResourceString(UNUSEDAFTER_MESSAGE, varData.variable.getName()); } return info.getMessage(varData.variable.getName()); } @@ -387,7 +398,7 @@ private List getDiagnosticReferences(VarData varDa context.getSelectionRange(), "+1" )).collect(Collectors.toList()); - if (varData.isFinished) { + if (varData.unusedAfter) { return references; } final var result = new ArrayList(); @@ -408,7 +419,7 @@ private static class VarData { SourceDefinedSymbol parentSymbol; boolean isMethod; boolean isGlobalOrModuleKind; - boolean isFinished; + boolean unusedAfter; private static VarData of(VariableSymbol variable, Range defRange, Reference rewriteReference, SourceDefinedSymbol methodSymbol, List references) { diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java index f12e51e819f..a92ab25069e 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/LostVariableDiagnosticTest.java @@ -74,10 +74,13 @@ void test() { .hasMessageOnRange(getMessage("ТекстЗапроса"), 311, 4, 16) .hasMessageOnRange(getMessageUnused("ТекстЗапроса"), 314, 4, 16) .hasMessageOnRange(getMessage("ЗначениеМодуля"), 318, 4, 18) - .hasMessageOnRange(getMessageUnused("Значение"), 328, 8, 16) - .hasMessageOnRange(getMessage("Элем22"), 333, 16, 22) - .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 339, 0, 18) - .hasMessageOnRange(getMessage("ЗначениеМодуля"), 343, 0, 14) + //.hasMessageOnRange(getMessageUnused("Значение"), 329, 8, 16) // TODO не ошибка + .hasMessageOnRange(getMessage("Элем22"), 335, 16, 22) + .hasMessageOnRange(getMessage("Значение23"), 344, 12, 22) + .hasMessageOnRange(getMessageUnused("Значение23"), 345, 12, 22) + .hasMessageOnRange(getMessage("Значение24"), 354, 12, 22) + .hasMessageOnRange(getMessage("ТекстЗапросаВБлоке"), 376, 0, 18) + .hasMessageOnRange(getMessage("ЗначениеМодуля"), 380, 0, 14) .hasSize(37); } diff --git a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl index c3d3b6ffd99..a6e738752d7 100644 --- a/src/test/resources/diagnostics/LostVariableDiagnostic.bsl +++ b/src/test/resources/diagnostics/LostVariableDiagnostic.bsl @@ -323,10 +323,12 @@ КонецПроцедуры Процедура Тест21() - Значение = 10; // не ошибка + Значение21 = 10; // не ошибка Для Каждого Элем ИЗ Коллекция Цикл - Элем.Реквизит = Вычисление(Значение); - Значение = 20; // ошибка + //Если ДругоеУсловие() Тогда + Элем.Реквизит = Вычисление(Значение21); + Значение21 = 20; // не ошибка, есть использование + //КонецЕсли; КонецЦикла; КонецПроцедуры @@ -337,6 +339,41 @@ КонецЦикла; КонецПроцедуры +Процедура Тест23() + Пока Условие() Цикл + Если ДругоеУсловие() Тогда + Значение23 = 10; // ошибка + Значение23 = 20; // ошибка + КонецЕсли; + КонецЦикла; +КонецПроцедуры + +Процедура Тест24() + Значение24 = 10; // не ошибка + Пока Условие() Цикл + Если ДругоеУсловие() Тогда + Значение24 = 20; // ошибка, нет использования далее + КонецЕсли; + КонецЦикла; +КонецПроцедуры + +Процедура Тест25() + Значение25 = 10; // не ошибка + Пока Условие() Цикл + Значение25 = 20; // не ошибка, есть использование далее + КонецЦикла; + Метод(Значение25); +КонецПроцедуры + +Процедура Тест26() + ЗначениеМодуля = 10; // не ошибка + Пока Условие() Цикл + Если ДругоеУсловие() Тогда + ЗначениеМодуля = 20; // не ошибка для значения модуля + КонецЕсли; + КонецЦикла; +КонецПроцедуры + ТекстЗапросаВБлоке = "Первый"; // ошибка ТекстЗапросаВБлоке = "Второй"; Запрос = Новый Запрос(ТекстЗапросаВБлоке); From b37656a40fd4d911a3905308da1f6dfe5f4b2806 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sat, 17 Dec 2022 13:43:47 +0300 Subject: [PATCH 44/45] =?UTF-8?q?=D1=83=D0=B1=D1=80=D0=B0=D0=BB=20=D0=BD?= =?UTF-8?q?=D0=B5=D0=B2=D0=B5=D1=80=D0=BD=D1=8B=D0=B9=20=D0=B8=D0=BC=D0=BF?= =?UTF-8?q?=D0=BE=D1=80=D1=82?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../com/github/_1c_syntax/bsl/languageserver/utils/Trees.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index d4625a724de..5758830bd2c 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -23,7 +23,6 @@ import com.github._1c_syntax.bsl.parser.BSLParser; import com.github._1c_syntax.bsl.parser.BSLParserRuleContext; -import edu.umd.cs.findbugs.annotations.Nullable; import lombok.experimental.UtilityClass; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.Token; From 74ac2d1e363cb8f29dd8564d7a8a6ff8f8aba8e0 Mon Sep 17 00:00:00 2001 From: Artur Ayukhanov Date: Sat, 17 Dec 2022 13:52:10 +0300 Subject: [PATCH 45/45] =?UTF-8?q?=D0=B8=D1=81=D0=BF=D1=80=D0=B0=D0=B2?= =?UTF-8?q?=D0=BB=D0=B5=D0=BD=20=D0=BD=D0=B5=D0=B2=D0=B5=D1=80=D0=BD=D1=8B?= =?UTF-8?q?=D0=B9=20=D0=BC=D0=B5=D1=80=D0=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../_1c_syntax/bsl/languageserver/utils/Trees.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java index 5758830bd2c..45d40027c14 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java @@ -431,8 +431,14 @@ private static Stream getChildrenStream(Tree t, Integer[] * Получает дочерние ноды с нужными типами */ public static Collection findAllRuleNodes(ParseTree t, Integer... index) { + return findAllRuleNodes(t, Arrays.asList(index)); + } + + /** + * Получает дочерние ноды с нужными типами + */ + public static Collection findAllRuleNodes(ParseTree t, Collection indexes) { List nodes = new ArrayList<>(); - List indexes = Arrays.asList(index); if (t instanceof ParserRuleContext && indexes.contains(((ParserRuleContext) t).getRuleIndex())) { @@ -440,7 +446,7 @@ public static Collection findAllRuleNodes(ParseTree t, Intege } IntStream.range(0, t.getChildCount()) - .mapToObj(i -> findAllRuleNodes(t.getChild(i), index)) + .mapToObj(i -> findAllRuleNodes(t.getChild(i), indexes)) .forEachOrdered(nodes::addAll); return nodes;