diff --git a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java index 68698099706..4617c0d4147 100644 --- a/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java +++ b/src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java @@ -32,12 +32,13 @@ import com.github._1c_syntax.utils.CaseInsensitivePattern; import org.antlr.v4.runtime.tree.ParseTree; -import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; @DiagnosticMetadata( type = DiagnosticType.CODE_SMELL, @@ -51,53 +52,79 @@ ) public class RefOveruseDiagnostic extends AbstractSDBLVisitorDiagnostic { - private static final String REF_REGEX = "Ссылка|Reference"; - private static final Pattern REF_PATTERN = CaseInsensitivePattern.compile(REF_REGEX); + private static final Pattern REF_PATTERN = CaseInsensitivePattern.compile("Ссылка|Reference"); private static final int BAD_CHILD_COUNT = 3; - private Collection dataSourceCollection = new ArrayList<>(); + public static final int COUNT_OF_TABLE_DOT_REF_DOT_REF = 5; + private Map dataSourcesWithTabularFlag = Collections.emptyMap(); @Override public ParseTree visitQuery(SDBLParser.QueryContext ctx) { + checkQuery(ctx).forEach(diagnosticStorage::addDiagnostic); + return ctx; + } + + private Stream checkQuery(SDBLParser.QueryContext ctx) { var columnsCollection = Trees.findAllRuleNodes(ctx, SDBLParser.RULE_column); - if (columnsCollection.isEmpty() - || dataSourceCollection.stream().anyMatch(Trees::treeContainsErrors)) { - return ctx; + if (columnsCollection.isEmpty()) { + return Stream.empty(); } - if (dataSourceCollection.isEmpty()) { - performSimpleCheck(columnsCollection); - return ctx; + dataSourcesWithTabularFlag = dataSourcesWithTabularSection(ctx); + if (dataSourcesWithTabularFlag.isEmpty()) { + return getSimpleOverused(columnsCollection); } - var tableNames = dataSourceCollection.stream() - .map(RefOveruseDiagnostic::getTableNameOrAlias) - .collect(Collectors.toSet()); + return getOverused(columnsCollection); + } - columnsCollection.forEach(column -> checkColumnNode((SDBLParser.ColumnContext) column, tableNames)); - return ctx; + private static Map dataSourcesWithTabularSection(SDBLParser.QueryContext ctx) { + return Trees.findAllRuleNodes(ctx, SDBLParser.RULE_dataSource).stream() + .map(SDBLParser.DataSourceContext.class::cast) + .collect(Collectors.toMap(RefOveruseDiagnostic::getTableNameOrAlias, + RefOveruseDiagnostic::isTableWithTabularSection)); + } + private static String getTableNameOrAlias(SDBLParser.DataSourceContext dataSource) { + final var value = Optional.of(dataSource); + return value + .map(SDBLParser.DataSourceContext::alias) + .map(alias -> (ParseTree)alias.name) + .or(() -> value + .map(SDBLParser.DataSourceContext::table) + .map(tableContext -> (ParseTree)tableContext.tableName)) + .or(() -> value + .map(SDBLParser.DataSourceContext::parameterTable) + .map(tableContext -> (ParseTree)tableContext.parameter())) + .map(ParseTree::getText) + .orElse(""); } - @Override - public ParseTree visitSelectQuery(SDBLParser.SelectQueryContext ctx) { - this.dataSourceCollection = Trees.findAllRuleNodes(ctx, SDBLParser.RULE_dataSource); - return super.visitSelectQuery(ctx); + private static boolean isTableWithTabularSection(SDBLParser.DataSourceContext dataSourceContext) { + final var table = dataSourceContext.table(); + if (table == null) { + return dataSourceContext.virtualTable() != null; + } + return table.tableName != null || table.objectTableName != null; } - private void performSimpleCheck(Collection columnsCollection) { - columnsCollection.stream() + private static Stream getSimpleOverused(Collection columnsCollection) { + return columnsCollection.stream() .filter(columnNode -> columnNode.getChildCount() > BAD_CHILD_COUNT) .map(column -> column.getChild(column.getChildCount() - 1)) .filter(lastChild -> REF_PATTERN.matcher(lastChild.getText()).matches()) - .forEach(node -> diagnosticStorage.addDiagnostic((BSLParserRuleContext) node)); + .map(BSLParserRuleContext.class::cast); } - private void checkColumnNode(SDBLParser.ColumnContext ctx, Set tableNames) { + private Stream getOverused(Collection columnsCollection) { + return columnsCollection.stream() + .map(SDBLParser.ColumnContext.class::cast) + .filter(column -> column.getChildCount() >= BAD_CHILD_COUNT) + .filter(this::isOveruse) + .map(BSLParserRuleContext.class::cast); + } - if (ctx.children == null) { - return; - } + private boolean isOveruse(SDBLParser.ColumnContext ctx) { // children: // @@ -111,39 +138,24 @@ private void checkColumnNode(SDBLParser.ColumnContext ctx, Set tableName final int childCount = ctx.children.size(); - if (childCount < 3) { - return; - } - - var lastChild = ctx.getChild(childCount - 1); // dots are also children of ColumnContext, // that is why -3 must be an index of penultimate identifier - var penultimateChild = ctx.getChild(childCount - 3); + var penultimateChild = ctx.getChild(childCount - BAD_CHILD_COUNT); - String lastIdentifierName = lastChild.getText(); String penultimateIdentifierName = penultimateChild.getText(); - if (REF_PATTERN.matcher(penultimateIdentifierName).matches() - || (REF_PATTERN.matcher(lastIdentifierName).matches() - && !tableNames.contains(penultimateIdentifierName))) { - diagnosticStorage.addDiagnostic(ctx); + if (REF_PATTERN.matcher(penultimateIdentifierName).matches()) { + if (childCount < COUNT_OF_TABLE_DOT_REF_DOT_REF){ + return true; + } + var prevChildID = ctx.getChild(childCount - COUNT_OF_TABLE_DOT_REF_DOT_REF).getText(); + return !dataSourcesWithTabularFlag.getOrDefault(prevChildID, false); } - } - - private static String getTableNameOrAlias(ParseTree dataSource) { - return Optional.of(dataSource) - .flatMap(dataSrc -> extractTextFromChild(dataSrc, SDBLParser.RULE_alias)) - .or(() -> Optional.of(dataSource) - .flatMap(dataSrc -> extractTextFromChild(dataSrc, SDBLParser.RULE_table))) - .or(() -> Optional.of(dataSource) - .flatMap(dataSrc -> extractTextFromChild(dataSrc, SDBLParser.RULE_parameterTable))) - .orElse(""); - } - - private static Optional extractTextFromChild(ParseTree parseTree, int childRuleType) { - return Optional.of(parseTree) - .flatMap(tree -> Trees.getFirstChild(tree, childRuleType)) - .flatMap(child -> Trees.getFirstChild(child, SDBLParser.RULE_identifier)) - .map(BSLParserRuleContext::getText); + var lastChild = ctx.getChild(childCount - 1); + String lastIdentifierName = lastChild.getText(); + if (REF_PATTERN.matcher(lastIdentifierName).matches()) { + return dataSourcesWithTabularFlag.get(penultimateIdentifierName) == null; + } + return false; } } diff --git a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnosticTest.java b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnosticTest.java index 030a0dc03f3..04d28823289 100644 --- a/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnosticTest.java +++ b/src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnosticTest.java @@ -38,7 +38,6 @@ void test() { List diagnostics = getDiagnostics(); - assertThat(diagnostics).hasSize(12); assertThat(diagnostics, true) .hasRange(3, 28, 3, 45) .hasRange(13, 8, 13, 34) @@ -51,7 +50,7 @@ void test() { .hasRange(92, 8, 29) .hasRange(153, 13, 153, 41) .hasRange(164, 13, 164, 53) - .hasRange(178, 13, 178, 35); - + .hasRange(178, 13, 178, 35) + .hasSize(12); } } diff --git a/src/test/resources/diagnostics/RefOveruseDiagnostic.bsl b/src/test/resources/diagnostics/RefOveruseDiagnostic.bsl index 02b3e329232..bc968112e77 100644 --- a/src/test/resources/diagnostics/RefOveruseDiagnostic.bsl +++ b/src/test/resources/diagnostics/RefOveruseDiagnostic.bsl @@ -179,10 +179,30 @@ | Контрагенты.Ссылка.ИНН = &ИНН"; // ошибка КонецПроцедуры -Процедура Тест17() +Процедура Тест16() ТекстЗапроса = "ВЫБРАТЬ | Артикул КАК Артикул // не задано имя таблицы |ИЗ | Справочник.Номенклатура КАК Номенклатура"; КонецПроцедуры + +Процедура Тест17() + ТекстЗапроса = + "ВЫБРАТЬ + | Таблица.Ссылка КАК Ссылка + |ИЗ + | Документ.Документ1.ТабличнаяЧасть1 КАК Таблица + |ГДЕ + | Таблица.Ссылка.Реквизит1 = &ИНН"; // не ошибка +КонецПроцедуры + +Процедура Тест18() + ТекстЗапроса = + "ВЫБРАТЬ + | Таблица.Ссылка КАК Ссылка + |ИЗ + | РегистрСведений.РегистрСведений1.СрезПоследних КАК Таблица + |ГДЕ + | Таблица.Ссылка.Реквизит1 = &ИНН"; // не ошибка +КонецПроцедуры