Skip to content

Commit

Permalink
ФП на Где Поле Есть NULL
Browse files Browse the repository at this point in the history 
удалил закомментированные тесты

1c-syntax#1867
  • Loading branch information
@artbear
artbear committed Oct 12, 2021
1 parent 0e4b396 commit a63d59a
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 69 deletions.
70 changes: 37 additions & 33 deletions ...hub/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,6 @@
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.BSLParserRuleContext;
import com.github._1c_syntax.bsl.parser.SDBLParser;
import com.github._1c_syntax.bsl.parser.SDBLParser.BuiltInFunctionsContext;
import com.github._1c_syntax.bsl.parser.SDBLParser.ColumnContext;
import com.github._1c_syntax.bsl.parser.SDBLParser.DataSourceContext;
import com.github._1c_syntax.bsl.parser.SDBLParser.IsNullPredicateContext;
import com.github._1c_syntax.bsl.parser.SDBLParser.JoinPartContext;
import com.github._1c_syntax.bsl.parser.SDBLParser.QueryContext;
import com.github._1c_syntax.bsl.parser.SDBLParser.SearchConditionContext;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.eclipse.lsp4j.DiagnosticRelatedInformation;
Expand All @@ -53,9 +45,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static com.github._1c_syntax.bsl.parser.SDBLParser.RULE_builtInFunctions;
import static com.github._1c_syntax.bsl.parser.SDBLParser.RULE_query;
import static com.github._1c_syntax.bsl.parser.SDBLParser.RULE_searchCondition;
import static com.github._1c_syntax.bsl.parser.SDBLParser.*;

@DiagnosticMetadata(
type = DiagnosticType.ERROR,
Expand All @@ -70,13 +60,14 @@
)
public class FieldsFromJoinsWithoutIsNullDiagnostic extends AbstractSDBLVisitorDiagnostic {

private static final Integer SELECT_ROOT = SDBLParser.RULE_selectedField;
private static final Integer SELECT_ROOT = RULE_selectedField;
private static final Collection<Integer> SELECT_STATEMENTS = Set.of(SELECT_ROOT, RULE_builtInFunctions);

private static final Integer WHERE_ROOT = RULE_searchCondition;
private static final Collection<Integer> WHERE_STATEMENTS = Set.of(WHERE_ROOT, RULE_builtInFunctions);
private static final Collection<Integer> WHERE_STATEMENTS = Set.of(WHERE_ROOT, RULE_builtInFunctions,
RULE_isNullPredicate);

private static final Integer JOIN_ROOT = SDBLParser.RULE_joinPart;
private static final Integer JOIN_ROOT = RULE_joinPart;
private static final Collection<Integer> JOIN_STATEMENTS = Set.of(JOIN_ROOT, RULE_builtInFunctions);

public static final Collection<Integer> RULES_OF_PARENT_FOR_SEARCH_CONDITION = Set.of(RULE_searchCondition, RULE_query);
Expand Down Expand Up @@ -106,7 +97,7 @@ private static Stream<String> joinedTables(JoinPartContext joinPartCtx) {
.filter(Objects::nonNull)
.map(DataSourceContext::alias)
.filter(Objects::nonNull)
.map(SDBLParser.AliasContext::identifier)
.map(AliasContext::identifier)
.map(BSLParserRuleContext::getText);
}

Expand Down Expand Up @@ -137,9 +128,9 @@ private void checkQuery(String joinedTableName, JoinPartContext joinPartCtx) {
});
}

private static boolean haveExprNotIsNullInsideWhere(@Nullable SDBLParser.SearchConditionsContext whereCtx) {
private static boolean haveExprNotIsNullInsideWhere(@Nullable SearchConditionsContext whereCtx) {
return Optional.ofNullable(whereCtx)
.stream().flatMap(ctx -> Trees.findAllRuleNodes(ctx, SDBLParser.RULE_isNullPredicate).stream())
.stream().flatMap(ctx -> Trees.findAllRuleNodes(ctx, RULE_isNullPredicate).stream())
.map(IsNullPredicateContext.class::cast)
.anyMatch(isNullPredicateCtx ->
haveFirstIsThenNotThenNull(isNullPredicateCtx)
Expand All @@ -159,7 +150,7 @@ private static boolean haveExprNotIsNullInsideWhere(IsNullPredicateContext isNul
}

private static boolean isTerminalNodeNOT(ParseTree node) {
return node instanceof TerminalNode && ((TerminalNode) node).getSymbol().getType() == SDBLParser.NOT;
return node instanceof TerminalNode && ((TerminalNode) node).getSymbol().getType() == NOT;
}

private static boolean haveExprNotWithParens(SearchConditionContext ctx) {
Expand All @@ -170,42 +161,55 @@ private static boolean haveExprNotWithParens(SearchConditionContext ctx) {
return rootCtx.getChildCount() == NOT_WITH_PARENS_EXPR_MEMBERS_COUNT && isTerminalNodeNOT(rootCtx.getChild(0));
}

private void checkSelect(String tableName, SDBLParser.SelectedFieldsContext columns) {
checkStatements(tableName, columns, SELECT_STATEMENTS, SELECT_ROOT);
private void checkSelect(String tableName, SelectedFieldsContext columns) {
checkStatements(tableName, columns, SELECT_STATEMENTS, SELECT_ROOT, false);
}

private void checkStatements(String tableName, BSLParserRuleContext expression, Collection<Integer> statements,
Integer rootForStatement) {
Integer rootForStatement, boolean checkIsNullOperator) {

Trees.findAllRuleNodes(expression, SDBLParser.RULE_column).stream()
Trees.findAllRuleNodes(expression, RULE_column).stream()
.filter(Objects::nonNull)
.filter(ColumnContext.class::isInstance)
.map(ColumnContext.class::cast)
.filter(columnContext -> checkColumn(tableName, columnContext, statements, rootForStatement))
.filter(columnContext -> checkColumn(tableName, columnContext, statements, rootForStatement, checkIsNullOperator))
.collect(Collectors.toCollection(() -> nodesForIssues));
}

private static boolean checkColumn(String tableName, ColumnContext columnCtx,
Collection<Integer> statements, Integer rootForStatement) {
Collection<Integer> statements, Integer rootForStatement,
boolean checkIsNullOperator) {
return Optional.of(columnCtx)
.filter(columnContext -> columnContext.mdoName != null)
.filter(columnContext -> columnContext.mdoName.getText().equalsIgnoreCase(tableName))
.filter(columnContext -> !haveIsNullInsideExprForColumn(columnContext, statements, rootForStatement))
.filter(columnContext -> !haveIsNullInsideExprForColumn(columnContext, statements, rootForStatement,
checkIsNullOperator))
.isPresent();
}

private static boolean haveIsNullInsideExprForColumn(BSLParserRuleContext ctx, Collection<Integer> statements,
Integer rootForStatement) {
Integer rootForStatement, boolean checkIsNullOperator) {
var selectStatement = Trees.getRootParent(ctx, statements);
if (selectStatement == null || selectStatement.getChildCount() == 0
|| rootForStatement == selectStatement.getRuleIndex()) {
return false;
}
return haveIsNullExpression(selectStatement)
|| haveIsNullInsideExprForColumn(selectStatement, statements, rootForStatement);
if (checkIsNullOperator && haveIsNullOperator(selectStatement)){
return true;
}
return haveIsNullFunction(selectStatement)
|| haveIsNullInsideExprForColumn(selectStatement, statements, rootForStatement, checkIsNullOperator);
}

private static boolean haveIsNullOperator(BSLParserRuleContext ctx) {
return Optional.of(ctx)
.filter(IsNullPredicateContext.class::isInstance)
.map(IsNullPredicateContext.class::cast)
.filter(isNullPredicateContext -> isNullPredicateContext.NOT() == null)
.isPresent();
}

private static boolean haveIsNullExpression(BSLParserRuleContext ctx) {
private static boolean haveIsNullFunction(BSLParserRuleContext ctx) {
return Optional.of(ctx)
.filter(BuiltInFunctionsContext.class::isInstance)
.map(BuiltInFunctionsContext.class::cast)
Expand All @@ -214,21 +218,21 @@ private static boolean haveIsNullExpression(BSLParserRuleContext ctx) {
.isPresent();
}

private void checkWhere(String tableName, @Nullable SDBLParser.SearchConditionsContext where) {
private void checkWhere(String tableName, @Nullable SearchConditionsContext where) {
Optional.ofNullable(where)
.stream().flatMap(searchConditionsContext -> searchConditionsContext.condidions.stream())
.forEach(searchConditionContext -> checkStatements(tableName, searchConditionContext,
WHERE_STATEMENTS, WHERE_ROOT));
WHERE_STATEMENTS, WHERE_ROOT, true));
}

private void checkAllJoins(String tableName, JoinPartContext currentJoinPart) {
Optional.ofNullable(Trees.getRootParent(currentJoinPart, SDBLParser.RULE_dataSource))
Optional.ofNullable(Trees.getRootParent(currentJoinPart, RULE_dataSource))
.filter(DataSourceContext.class::isInstance)
.stream().flatMap(ctx -> ((DataSourceContext) ctx).joinPart().stream())
.filter(joinPartContext -> joinPartContext != currentJoinPart)
.map(JoinPartContext::searchConditions)
.forEach(searchConditionsContext -> checkStatements(tableName, searchConditionsContext,
JOIN_STATEMENTS, JOIN_ROOT));
JOIN_STATEMENTS, JOIN_ROOT, false));
}

private List<DiagnosticRelatedInformation> getRelatedInformation(JoinPartContext self) {
Expand Down
31 changes: 25 additions & 6 deletions ..._1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnosticTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,9 @@ void test() {

checkContent(
diagnostics.get(8),
Ranges.create(194, 5, 195, 50),
Arrays.asList(
Ranges.create(192, 13, 32),
Ranges.create(196, 9, 30)
));
Ranges.create(177, 5, 178, 50),
Ranges.create(175, 13, 32)
);

}

Expand Down Expand Up @@ -187,7 +185,7 @@ void testWithNotIsNullInPairsInsideExpression() {
}

@Test
void testWithIsNullInsideExpression() {
void testWithIsNullInsideWhere() {
var sample =
" Запрос = Новый Запрос;\n" +
" Запрос.Текст =\n" +
Expand All @@ -204,6 +202,27 @@ void testWithIsNullInsideExpression() {
assertThat(diagnostics).hasSize(1);
}

@Test
void testWithIsNullInsideWhereButNonTableFieldRequest() {
var sample =
"Процедура Тест15_в_ГДЕ_Есть_NULL_НоНетОбращенийКПолямТаблицы()\n" +
"\n" +
" Запрос = Новый Запрос;\n" +
" Запрос.Текст =\n" +
" \"ВЫБРАТЬ Истина\n" +
" |ИЗ Справочник.Склады КАК Склады15\n" +
" |ЛЕВОЕ СОЕДИНЕНИЕ Справочник.Сотрудники КАК Сотрудники15\n" +
" |ПО Склады15.Кладовщик = Сотрудники15.Ссылка\n" +
" |ГДЕ Сотрудники15.Реквизит ЕСТЬ NULL // не ошибка\n" +
" |\";\n" +
"КонецПроцедуры";

var documentContext = TestUtils.getDocumentContext(sample);
var diagnostics = getDiagnostics(documentContext);

assertThat(diagnostics).isEmpty();
}

@Test
void tesEqualTableNamesInUnion() {
var sample =
Expand Down
30 changes: 0 additions & 30 deletions src/test/resources/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.bsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,23 +169,6 @@

КонецПроцедуры

Процедура Тест12_СоединениеСВременнойТаблицейБезОшибки()

// Запрос = Новый Запрос;
// Запрос.Текст =
// "ВЫБРАТЬ
// | Таблица12.Ссылка КАК Ссылка,
// | Контрагенты12.Ссылка КАК Ссылка1 // не ошибка
// |ПОМЕСТИТЬ ВТ
// |ИЗ
// | &Таблица КАК Таблица12
// | ЛЕВОЕ СОЕДИНЕНИЕ Справочник.Контрагенты КАК Контрагенты12
// | ПО Таблица12.Ссылка = Контрагенты12.Ссылка
// |ГДЕ (НЕ (Контрагенты12.Реквизит ЕСТЬ NULL)) // TODO (Контрагенты12.Реквизит ЕСТЬ НЕ NULL)
// |";

// КонецПроцедуры

Процедура Тест13_Есть_NULL_ЛевоеСоединение()

Запрос = Новый Запрос;
Expand All @@ -211,16 +194,3 @@
|";

КонецПроцедуры

//Процедура Тест15_в_ГДЕ_Есть_NULL_НоНетОбращенийКПолямТаблицы()

// Запрос = Новый Запрос;
// Запрос.Текст =
// "ВЫБРАТЬ Истина
// |ИЗ Справочник.Склады КАК Склады15
// |ЛЕВОЕ СОЕДИНЕНИЕ Справочник.Сотрудники КАК Сотрудники15
// |ПО Склады15.Кладовщик = Сотрудники15.Ссылка
// |ГДЕ Сотрудники15.Реквизит ЕСТЬ NULL // не ошибка
// |";

// КонецПроцедуры

0 comments on commit a63d59a

Please sign in to comment.