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.

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 #2825

Merged
merged 28 commits into from
Jun 3, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7cf2091
refOverUse тест с фп
asosnoviy Jun 24, 2022
5a195a0
refOverUse тест с fp
asosnoviy Jun 24, 2022
b43f436
новые методы поиска
artbear Jun 24, 2022
b88f851
исправление FP
artbear Jun 24, 2022
7bbdff9
уточнил комментарии к методу
artbear Jun 24, 2022
fd1a831
refOverUse Fn тест
asosnoviy Jun 24, 2022
d1e3bd1
пропускались вложенные запросы
artbear Jun 24, 2022
28bc2da
исправил еще небольшое ФП
artbear Jun 24, 2022
aa7d88e
refOfuse fn test
asosnoviy Jun 24, 2022
e7586a1
учитываются поля без псевдонимов таблиц
artbear Jun 25, 2022
b171d5f
refOverUse fp test
asosnoviy Jun 27, 2022
9a6bdd1
FP полное имя таблицы
artbear Jun 27, 2022
fc91b7f
refOverUse fp test
asosnoviy Jun 28, 2022
1a3931e
ФП обращение к инлайн-таблице
artbear Jun 28, 2022
2b4f1d1
refOveruse fn fp tests
asosnoviy Jun 29, 2022
7bef41a
исправил FP
artbear Jul 1, 2022
caa878e
refOveruse fp
asosnoviy Jul 5, 2022
07f6ab2
Merge remote-tracking branch 'origin/develop' into fix/refOveruse
artbear Jul 18, 2022
262d231
заготовка под проверку метаданных
artbear Jul 18, 2022
74f66d2
CleanupContextBeforeClassAndAfterEachTestMethod
artbear Jul 19, 2022
834682a
Merge branch 'develop' into fix/refOveruse
artbear Dec 17, 2022
3e73343
Заготовка исправления
artbear Dec 24, 2022
b5f8661
убрал ненужные тесты
artbear Dec 24, 2022
7d7f3a0
вернул прохождение тестов
artbear Dec 24, 2022
7837e2a
заготовка анализа таблиц метаданных
artbear Dec 24, 2022
fe24ac3
форматирование + исправлены замечания
artbear Mar 31, 2023
1e9d3e7
возвращен неверно отправленный код
artbear Mar 31, 2023
7466713
переименовал метод + исправил имя параметра
artbear Mar 31, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,26 @@
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.utils.Ranges;
import com.github._1c_syntax.bsl.languageserver.utils.Trees;
import com.github._1c_syntax.bsl.parser.BSLParserRuleContext;
import com.github._1c_syntax.bsl.parser.SDBLParser;
import com.github._1c_syntax.bsl.parser.SDBLParser.DataSourceContext;
import com.github._1c_syntax.utils.CaseInsensitivePattern;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.tree.ParseTree;
import org.eclipse.lsp4j.Range;

import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
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.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -55,89 +63,159 @@
public class RefOveruseDiagnostic extends AbstractSDBLVisitorDiagnostic {

private static final Pattern REF_PATTERN = CaseInsensitivePattern.compile("Ссылка|Reference");
private static final int BAD_CHILD_COUNT = 3;
private static final int COUNT_OF_TABLE_DOT_REF = 3;
private static final int LAST_INDEX_OF_TABLE_DOT_REF = COUNT_OF_TABLE_DOT_REF - 1;
private static final int COUNT_OF_TABLE_DOT_REF_DOT_REF = 5;
private static final Set<Integer> RULE_COLUMNS = Set.of(SDBLParser.RULE_column, SDBLParser.RULE_query);
private static final Set<Integer> METADATA_TYPES = Set.of(
SDBLParser.BUSINESS_PROCESS_TYPE,
SDBLParser.CATALOG_TYPE,
SDBLParser.DOCUMENT_TYPE,
SDBLParser.INFORMATION_REGISTER_TYPE,
SDBLParser.CONSTANT_TYPE,
SDBLParser.FILTER_CRITERION_TYPE,
SDBLParser.EXCHANGE_PLAN_TYPE,
SDBLParser.SEQUENCE_TYPE,
SDBLParser.DOCUMENT_JOURNAL_TYPE,
SDBLParser.ENUM_TYPE,
SDBLParser.CHART_OF_CHARACTERISTIC_TYPES_TYPE,
SDBLParser.CHART_OF_ACCOUNTS_TYPE,
SDBLParser.CHART_OF_CALCULATION_TYPES_TYPE,
SDBLParser.ACCUMULATION_REGISTER_TYPE,
SDBLParser.ACCOUNTING_REGISTER_TYPE,
SDBLParser.CALCULATION_REGISTER_TYPE,
SDBLParser.TASK_TYPE,
SDBLParser.EXTERNAL_DATA_SOURCE_TYPE);
private static final Collection<Integer> EXCLUDED_COLUMNS_ROOT =
Set.of(SDBLParser.RULE_inlineTableField, SDBLParser.RULE_query);
private Map<String, Boolean> dataSourcesWithTabularFlag = Collections.emptyMap();
private Map<String, Boolean> prevDataSourcesWithTabularFlag = Collections.emptyMap();
@Nullable private Range prevQueryRange;

@Override
public ParseTree visitQueryPackage(SDBLParser.QueryPackageContext ctx) {
var result = super.visitQueryPackage(ctx);
prevQueryRange = null;
prevDataSourcesWithTabularFlag = Collections.emptyMap();
dataSourcesWithTabularFlag = Collections.emptyMap();
return result;
}

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

private Stream<BSLParserRuleContext> checkQuery(SDBLParser.QueryContext ctx) {
var columnsCollection = Trees.findAllRuleNodes(ctx, SDBLParser.RULE_column);
var columns = Trees.findAllTopLevelRuleNodes(ctx, RULE_COLUMNS).stream()
.filter(parserRuleContext -> parserRuleContext.getRuleIndex() == SDBLParser.RULE_column)
.filter(parserRuleContext -> Trees.getRootParent((BSLParserRuleContext) parserRuleContext, EXCLUDED_COLUMNS_ROOT)
.getRuleIndex() == SDBLParser.RULE_query)
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed

Check warning

Code scanning / QDJVMC

Nullability and data flow problems

Method invocation 'getRuleIndex' may produce 'NullPointerException'
.collect(Collectors.toList());

if (columnsCollection.isEmpty()) {
if (columns.isEmpty()) {
return Stream.empty();
}

dataSourcesWithTabularFlag = dataSourcesWithTabularSection(ctx);
if (dataSourcesWithTabularFlag.isEmpty()) {
return getSimpleOverused(columnsCollection);
return getSimpleOverused(columns);
}

return getOverused(columnsCollection);
return getOverused(columns);
}

private static Map<String, Boolean> dataSourcesWithTabularSection(SDBLParser.QueryContext ctx) {
return findAllDataSourceWithoutInnerQueries(ctx)
private Map<String, Boolean> dataSourcesWithTabularSection(SDBLParser.QueryContext ctx) {
var newResult = findAllDataSourceWithoutInnerQueries(ctx)
.collect(Collectors.toMap(
RefOveruseDiagnostic::getTableNameOrAlias,
RefOveruseDiagnostic::isTableWithTabularSection,
this::isTableWithTabularSection,
(existing, replacement) -> existing,
HashMap::new));

var queryRange = Ranges.create(ctx);

final Map<String, Boolean> result;
if (prevQueryRange == null || !Ranges.containsRange(prevQueryRange, queryRange)){
result = newResult;
prevDataSourcesWithTabularFlag = result;
prevQueryRange = queryRange;
} else {
result = new HashMap<>(newResult);
result.putAll(prevDataSourcesWithTabularFlag);
}
return result;
}

private static Stream<? extends SDBLParser.DataSourceContext> findAllDataSourceWithoutInnerQueries(
private static Stream<? extends 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)
.flatMap(dataSourceContext -> getInnerDataSource(dataSourceContext).stream())
);
}

private static String getTableNameOrAlias(SDBLParser.DataSourceContext dataSource) {
private static Collection<DataSourceContext> getInnerDataSource(DataSourceContext dataSourceContext) {
var result = new ArrayList<DataSourceContext>();
Optional.ofNullable(dataSourceContext.dataSource())
.map(RefOveruseDiagnostic::getInnerDataSource)
.ifPresent(result::addAll);

var joinDataSources = dataSourceContext.joinPart().stream()
.map(SDBLParser.JoinPartContext::dataSource)
.filter(Objects::nonNull)
.collect(Collectors.toList());
result.addAll(joinDataSources);

var dataSourcesFromJoins = joinDataSources.stream()
.flatMap(dataSourceContext1 -> getInnerDataSource(dataSourceContext1).stream())
.collect(Collectors.toList());

result.addAll(dataSourcesFromJoins);
return result;
}


private static String getTableNameOrAlias(DataSourceContext dataSource) {
final var value = Optional.of(dataSource);
return value
.map(SDBLParser.DataSourceContext::alias)
.map(DataSourceContext::alias)
.map(alias -> (ParseTree)alias.name)
.or(() -> value
.map(SDBLParser.DataSourceContext::table)
.map(DataSourceContext::table)
.map(tableContext -> (ParseTree)tableContext.tableName))
.or(() -> value
.map(SDBLParser.DataSourceContext::parameterTable)
.map(DataSourceContext::parameterTable)
.map(tableContext -> (ParseTree)tableContext.parameter()))
.map(ParseTree::getText)
.orElse("");
}

private static boolean isTableWithTabularSection(SDBLParser.DataSourceContext dataSourceContext) {
private boolean isTableWithTabularSection(DataSourceContext dataSourceContext) {
final var table = dataSourceContext.table();
if (table == null) {
return dataSourceContext.virtualTable() != null;
}
return table.tableName != null || table.objectTableName != null;
}

private static Stream<BSLParserRuleContext> getSimpleOverused(Collection<ParseTree> columnsCollection) {
private static Stream<BSLParserRuleContext> getSimpleOverused(List<ParserRuleContext> columnsCollection) {
return columnsCollection.stream()
.filter(columnNode -> columnNode.getChildCount() > BAD_CHILD_COUNT)
.filter(columnNode -> columnNode.getChildCount() > COUNT_OF_TABLE_DOT_REF)
.map(column -> column.getChild(column.getChildCount() - 1))
.filter(lastChild -> REF_PATTERN.matcher(lastChild.getText()).matches())
.map(BSLParserRuleContext.class::cast);
}

private Stream<BSLParserRuleContext> getOverused(Collection<ParseTree> columnsCollection) {
private Stream<BSLParserRuleContext> getOverused(List<ParserRuleContext> columnsCollection) {
return columnsCollection.stream()
.map(SDBLParser.ColumnContext.class::cast)
.filter(column -> column.getChildCount() >= BAD_CHILD_COUNT)
.filter(column -> column.getChildCount() >= COUNT_OF_TABLE_DOT_REF)
.filter(this::isOveruse)
.map(BSLParserRuleContext.class::cast);
}
Expand All @@ -154,26 +232,43 @@ private boolean isOveruse(SDBLParser.ColumnContext ctx) {
// ^ ^ ^
// 0 1 2

final int childCount = ctx.children.size();

// dots are also children of ColumnContext,
// that is why -3 must be an index of penultimate identifier
var penultimateChild = ctx.getChild(childCount - BAD_CHILD_COUNT);
var children = extractFirstMetadataTypeName(ctx);
var refIndex = findLastRef(children);

String penultimateIdentifierName = penultimateChild.getText();
final int childCount = children.size();
final var lastIndex = childCount - 1;
if (refIndex == lastIndex) {
var penultimateIdentifierName = children.get(lastIndex - LAST_INDEX_OF_TABLE_DOT_REF).getText();
return dataSourcesWithTabularFlag.get(penultimateIdentifierName) == null;
}
if (refIndex < LAST_INDEX_OF_TABLE_DOT_REF){
return false;
}
if (refIndex > LAST_INDEX_OF_TABLE_DOT_REF){
return true;
}
var tabName = children.get(0).getText();
return !dataSourcesWithTabularFlag.getOrDefault(tabName, false);
}

if (REF_PATTERN.matcher(penultimateIdentifierName).matches()) {
if (childCount < COUNT_OF_TABLE_DOT_REF_DOT_REF){
return true;
private static int findLastRef(List<ParseTree> children) {
for (int i = children.size() - 1; i >= 0 ; i--) {
final var child = children.get(i);
final var childText = child.getText();
if (REF_PATTERN.matcher(childText).matches()) {
return i;
}
var prevChildID = ctx.getChild(childCount - COUNT_OF_TABLE_DOT_REF_DOT_REF).getText();
return !dataSourcesWithTabularFlag.getOrDefault(prevChildID, false);
}
var lastChild = ctx.getChild(childCount - 1);
String lastIdentifierName = lastChild.getText();
if (REF_PATTERN.matcher(lastIdentifierName).matches()) {
return dataSourcesWithTabularFlag.get(penultimateIdentifierName) == null;
return -1;
}

private static List<ParseTree> extractFirstMetadataTypeName(SDBLParser.ColumnContext ctx) {
final var mdoName = ctx.mdoName;
final var children = ctx.children;
if (mdoName == null || children.size() < COUNT_OF_TABLE_DOT_REF_DOT_REF
|| !METADATA_TYPES.contains(ctx.mdoName.getStart().getType())){
return children;
}
return false;
return children.subList(1, children.size() - 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -360,21 +360,69 @@ private static Stream<BSLParserRuleContext> getChildrenStream(Tree t, Integer[]

/**
* Получает дочерние ноды с нужными типами
*
* @param t - начальный узел дерева
* @param index - массив индексов
* @return найденные узлы
*/
public static Collection<ParserRuleContext> findAllRuleNodes(ParseTree t, Integer... index) {
List<ParserRuleContext> nodes = new ArrayList<>();
List<Integer> indexes = Arrays.asList(index);
return findAllRuleNodes(t, Arrays.asList(index));
}

/**
* Получает дочерние ноды с нужными типами
*
* @param t - начальный узел дерева
* @param indexes - коллекция индексов
* @return найденные узлы
*/
public static Collection<ParserRuleContext> findAllRuleNodes(ParseTree t, Collection<Integer> indexes) {
List<ParserRuleContext> result = new ArrayList<>();

if (t instanceof ParserRuleContext
&& indexes.contains(((ParserRuleContext) t).getRuleIndex())) {
nodes.add((ParserRuleContext) t);
result.add((ParserRuleContext) t);
}

IntStream.range(0, t.getChildCount())
.mapToObj(i -> findAllRuleNodes(t.getChild(i), index))
.forEachOrdered(nodes::addAll);
.mapToObj(i -> findAllRuleNodes(t.getChild(i), indexes))
.forEachOrdered(result::addAll);

return nodes;
return result;
}

/**
* Получает "первые" дочерние ноды с нужными типами
* ВАЖНО: поиск вглубь найденной ноды с нужными индексами не выполняется
* Например, если указать RULE_codeBlock, то найдется только самый верхнеуровневый блок кода, все вложенные найдены не будут
* ВАЖНО: начальная нода не проверяется на условие, т.к. тогда она единственная и вернется в результате
*
* @param t - начальный узел дерева
* @param indexes - коллекция индексов
* @return найденные узлы
*/
public static Collection<ParserRuleContext> findAllTopLevelRuleNodes(ParserRuleContext t, Collection<Integer> indexes) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

просьба, без односимвольных параметров

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Плбс плохое название метода. Оно ищет не все верхнеуровневые узлы, а среди потомков.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Плбс плохое название метода. Оно ищет не все верхнеуровневые узлы, а среди потомков.

исправил название на findAllTopLevelDescendantNodes

также исправил односимвольное имя параметра на root

@nixel2007 @theshadowco

var result = new ArrayList<ParserRuleContext>();

t.children.stream()
.map(node -> findAllTopLevelRuleNodesInner(node, indexes))
.forEachOrdered(result::addAll);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а сортировка зачем?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а сортировка зачем?

да, сортировка не нужна. убрал


return result;
}

private static Collection<ParserRuleContext> findAllTopLevelRuleNodesInner(ParseTree t, Collection<Integer> indexes) {
if (t instanceof ParserRuleContext
&& indexes.contains(((ParserRuleContext) t).getRuleIndex())) {
return List.of((ParserRuleContext) t);
}

List<ParserRuleContext> result = new ArrayList<>();
IntStream.range(0, t.getChildCount())
.mapToObj(i -> findAllTopLevelRuleNodesInner(t.getChild(i), indexes))
.forEachOrdered(result::addAll);

return result;
}

/**
Expand Down
Loading