Skip to content

Commit

Permalink
Merge pull request #2808 from artbear/ref-overuse-fix
Browse files Browse the repository at this point in the history 
FP в RefOveruse для для табличных частей
  • Loading branch information
@nixel2007
nixel2007 authored Jun 22, 2022
2 parents 7c5594e + 3766393 commit 1ec2ab9
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 59 deletions.
136 changes: 81 additions & 55 deletions src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@
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.Objects;
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,
Expand All @@ -51,53 +53,92 @@
)
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<ParseTree> dataSourceCollection = new ArrayList<>();
public static final int COUNT_OF_TABLE_DOT_REF_DOT_REF = 5;
private Map<String, Boolean> dataSourcesWithTabularFlag = Collections.emptyMap();

@Override
public ParseTree visitQuery(SDBLParser.QueryContext ctx) {
checkQuery(ctx).forEach(diagnosticStorage::addDiagnostic);
return ctx;
}

private Stream<BSLParserRuleContext> 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<String, Boolean> dataSourcesWithTabularSection(SDBLParser.QueryContext ctx) {
return findAllDataSourceWithoutInnerQueries(ctx)
.collect(Collectors.toMap(RefOveruseDiagnostic::getTableNameOrAlias,
RefOveruseDiagnostic::isTableWithTabularSection));
}

private static Stream<? extends SDBLParser.DataSourceContext> findAllDataSourceWithoutInnerQueries(
SDBLParser.QueryContext ctx) {
if (ctx.from == null){
return Stream.empty();
}
return Stream.concat(
ctx.from.dataSource().stream(),
ctx.from.dataSource().stream()
.flatMap(dataSourceContext -> dataSourceContext.joinPart().stream())
.map(SDBLParser.JoinPartContext::dataSource)
.filter(Objects::nonNull)
);
}

@Override
public ParseTree visitSelectQuery(SDBLParser.SelectQueryContext ctx) {
this.dataSourceCollection = Trees.findAllRuleNodes(ctx, SDBLParser.RULE_dataSource);
return super.visitSelectQuery(ctx);
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("");
}

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<ParseTree> columnsCollection) {
columnsCollection.stream()
private static Stream<BSLParserRuleContext> getSimpleOverused(Collection<ParseTree> 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<String> tableNames) {
private Stream<BSLParserRuleContext> getOverused(Collection<ParseTree> 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:
//
Expand All @@ -111,39 +152,24 @@ private void checkColumnNode(SDBLParser.ColumnContext ctx, Set<String> 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<String> 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;
}
}
8 changes: 5 additions & 3 deletions ...t/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnosticTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ void test() {

List<Diagnostic> diagnostics = getDiagnostics();

assertThat(diagnostics).hasSize(12);
assertThat(diagnostics, true)
.hasRange(3, 28, 3, 45)
.hasRange(13, 8, 13, 34)
Expand All @@ -51,7 +50,10 @@ 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)
.hasRange(216, 13, 37)
.hasRange(226, 13, 37)
.hasRange(238, 13, 38)
.hasSize(15);
}
}
54 changes: 53 additions & 1 deletion src/test/resources/diagnostics/RefOveruseDiagnostic.bsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,62 @@
| Контрагенты.Ссылка.ИНН = &ИНН"; // ошибка
КонецПроцедуры

Процедура Тест17()
Процедура Тест16()
ТекстЗапроса =
"ВЫБРАТЬ
| Артикул КАК Артикул // не задано имя таблицы
|ИЗ
| Справочник.Номенклатура КАК Номенклатура";
КонецПроцедуры

Процедура Тест17()
ТекстЗапроса =
"ВЫБРАТЬ
| Таблица.Ссылка КАК Ссылка
|ИЗ
| Документ.Документ1.ТабличнаяЧасть1 КАК Таблица
|ГДЕ
| Таблица.Ссылка.Реквизит1 = &ИНН"; // не ошибка
КонецПроцедуры

Процедура Тест18()
ТекстЗапроса =
"ВЫБРАТЬ
| Таблица.Ссылка КАК Ссылка
|ИЗ
| РегистрСведений.РегистрСведений1.СрезПоследних КАК Таблица
|ГДЕ
| Таблица.Ссылка.Реквизит1 = &ИНН"; // не ошибка
КонецПроцедуры

Процедура Тест19()
ТекстЗапроса =
"ВЫБРАТЬ
| Таблица.Ссылка КАК Ссылка
|ИЗ
| (ВЫБРАТЬ Таблица.Ссылка ИЗ Таблица КАК Таблица) КАК Таблица // было падение анализа
|ГДЕ
| Таблица.Ссылка.Реквизит1 = &ИНН"; // ошибка
КонецПроцедуры

Процедура Тест20()
ТекстЗапроса =
"ВЫБРАТЬ
| Таблица.Ссылка КАК Ссылка
|ИЗ
| (ВЫБРАТЬ Таблица.Ссылка ИЗ Таблица) КАК Таблица // было падение анализа
|ГДЕ
| Таблица.Ссылка.Реквизит1 = &ИНН"; // ошибка
КонецПроцедуры

Процедура Тест21()
ТекстЗапроса =
"ВЫБРАТЬ
| Таблица.Ссылка КАК Ссылка
|ИЗ
| Таблица КАК Таблица
| ЛЕВОЕ СОЕДИНЕНИЕ (ВЫБРАТЬ Таблица.Ссылка ИЗ Таблица) // было падение анализа
| КАК Таблица2 ПО Истина
|ГДЕ
| Таблица2.Ссылка.Реквизит1 = &ИНН"; // ошибка
КонецПроцедуры

0 comments on commit 1ec2ab9

Please sign in to comment.