Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FP в RefOveruse для для табличных частей #2808

Merged
merged 2 commits into from
Jun 22, 2022
Merged

FP в RefOveruse для для табличных частей #2808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01edbe2
ФП для табличных частей
artbear Jun 10, 2022
3766393
кейс с падением анализа
artbear Jun 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 81 additions & 55 deletions src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 = &ИНН"; // ошибка
КонецПроцедуры