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

Jump to bottom

UsageWriteLogEvent - Исключено FP при использовании глобального объекта ОбработкаОшибок #3010

Merged
merged 4 commits into from
Mar 31, 2023
Merged

UsageWriteLogEvent - Исключено FP при использовании глобального объекта ОбработкаОшибок #3010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71e3e2f
Исключено FP при использовании глобального объекта ОбработкаОшибок
artbear Mar 26, 2023
7e843a5
Update src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnos…
artbear Mar 30, 2023
77f07f3
Update src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnos…
artbear Mar 30, 2023
35031db
исправлены замечания
artbear Mar 30, 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
80 changes: 54 additions & 26 deletions ...va/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsageWriteLogEventDiagnostic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

@DiagnosticMetadata(
type = DiagnosticType.CODE_SMELL,
Expand Down Expand Up @@ -70,6 +71,9 @@ public class UsageWriteLogEventDiagnostic extends AbstractVisitorDiagnostic {
private static final Pattern PATTERN_ERROR = CaseInsensitivePattern.compile(
"ошибка|error"
);
private static final Pattern ERROR_PROCESSING_ERROR = CaseInsensitivePattern.compile(
"обработкаошибок|errorprocessing"
);

private static final int WRITE_LOG_EVENT_METHOD_PARAMS_COUNT = 5;
public static final int COMMENTS_PARAM_INDEX = 4;
Expand All @@ -93,7 +97,7 @@ public ParseTree visitGlobalMethodCall(BSLParser.GlobalMethodCallContext context

private void checkParams(BSLParser.GlobalMethodCallContext context) {
final var callParams = context.doCall().callParamList().callParam();
if (!checkFirstParams(context, callParams)){
if (!checkFirstParams(context, callParams)) {
return;
}

Expand All @@ -112,7 +116,10 @@ private void checkParams(BSLParser.GlobalMethodCallContext context) {
}
}

private boolean checkFirstParams(BSLParser.GlobalMethodCallContext context, List<? extends BSLParser.CallParamContext> callParams) {
private boolean checkFirstParams(
BSLParser.GlobalMethodCallContext context,
List<? extends BSLParser.CallParamContext> callParams
) {
if (callParams.size() < WRITE_LOG_EVENT_METHOD_PARAMS_COUNT) {
fireIssue(context, WRONG_NUMBER_MESSAGE);
return false;
Expand Down Expand Up @@ -176,7 +183,7 @@ private static boolean isCommentCorrect(BSLParser.CallParamContext commentsCtx)
if (hasRaiseStatement(codeBlockContext)) {
return true;
}
return isValidExpression(codeBlockContext, commentsCtx.expression(), true);
return isValidCommentExpression(codeBlockContext, commentsCtx.expression(), true);
}

private static boolean hasRaiseStatement(BSLParser.CodeBlockContext codeBlockContext) {
Expand All @@ -192,57 +199,78 @@ private static boolean hasRaiseStatement(BSLParser.CodeBlockContext codeBlockCon
// то проверим, что в присвоении есть ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()
// если есть какая-то переменная, определенная на уровень выше (например, параметр метода), то не анализируем ее

private static boolean isValidExpression(
private static boolean isValidCommentExpression(
BSLParser.CodeBlockContext codeBlock,
@Nullable BSLParser.ExpressionContext expression,
boolean checkPrevAssignment
) {
if (expression == null) {
return true;
}
final var assignmentGlobalCalls = Trees.findAllRuleNodes(expression, BSLParser.RULE_globalMethodCall);
if (!assignmentGlobalCalls.isEmpty()) {
if (isErrorDescriptionCallCorrect(assignmentGlobalCalls)) {
final var methodCalls = Trees.findAllRuleNodes(expression,
List.of(BSLParser.RULE_globalMethodCall, BSLParser.RULE_methodCall)).stream()
.filter(BSLParserRuleContext.class::isInstance)
.map(BSLParserRuleContext.class::cast)
.collect(Collectors.toList());
if (!methodCalls.isEmpty()) {
if (isErrorDescriptionCallCorrect(methodCalls)) {
return true;
}
if (hasSimpleErrorDescription(assignmentGlobalCalls) || hasBriefErrorDescription(assignmentGlobalCalls)) {
if (hasSimpleErrorDescription(methodCalls) || hasBriefErrorDescription(methodCalls)) {
return false;
}
}

return isValidExpression(expression, codeBlock, checkPrevAssignment);
}

private static boolean isErrorDescriptionCallCorrect(Collection<ParseTree> globalCalls) {
return globalCalls.stream()
.filter(context -> context instanceof BSLParser.GlobalMethodCallContext)
.map(BSLParser.GlobalMethodCallContext.class::cast)
.filter(context -> isAppropriateName(context, PATTERN_DETAIL_ERROR_DESCRIPTION))
.anyMatch(UsageWriteLogEventDiagnostic::hasFirstDescendantGlobalCall);
private static boolean isErrorDescriptionCallCorrect(Collection<BSLParserRuleContext> calls) {
return calls.stream()
.filter(context -> isAppropriateMethodName(context, PATTERN_DETAIL_ERROR_DESCRIPTION))
.filter(context -> context instanceof BSLParser.GlobalMethodCallContext
|| (context instanceof BSLParser.MethodCallContext && isErrorProcessingCall((BSLParser.MethodCallContext) context)))
.anyMatch(UsageWriteLogEventDiagnostic::hasFirstDescendantGlobalCallWithPatternError);
}

private static boolean isAppropriateName(
BSLParser.GlobalMethodCallContext context,
private static boolean isAppropriateMethodName(
BSLParserRuleContext context,
Pattern patternDetailErrorDescription
) {
return patternDetailErrorDescription.matcher(context.methodName().getText()).matches();
BSLParser.MethodNameContext methodNameContext = context.getRuleContext(BSLParser.MethodNameContext.class, 0);
return patternDetailErrorDescription.matcher(methodNameContext.getText()).matches();
}

private static boolean isErrorProcessingCall(BSLParser.MethodCallContext methodCallContext) {
return Optional.of(methodCallContext)
.map(BSLParserRuleContext::getParent)
.filter(context -> context instanceof BSLParser.AccessCallContext)
.map(BSLParserRuleContext::getParent)
.filter(context -> context instanceof BSLParser.ModifierContext)
.map(BSLParserRuleContext::getParent)
.filter(context -> context instanceof BSLParser.ComplexIdentifierContext)
.map(BSLParser.ComplexIdentifierContext.class::cast)
.map(BSLParser.ComplexIdentifierContext::IDENTIFIER)
.filter(terminalNode -> ERROR_PROCESSING_ERROR.matcher(terminalNode.getText()).matches())
.isPresent();
}

private static boolean hasFirstDescendantGlobalCall(BSLParser.GlobalMethodCallContext globalCallCtx) {
private static boolean hasFirstDescendantGlobalCallWithPatternError(BSLParserRuleContext globalCallCtx) {
return Trees.findAllRuleNodes(globalCallCtx, BSLParser.RULE_globalMethodCall).stream()
.map(BSLParser.GlobalMethodCallContext.class::cast)
.anyMatch(context -> isAppropriateName(context, PATTERN_ERROR_INFO));
.anyMatch(context -> isAppropriateMethodName(context, PATTERN_ERROR_INFO));
}

private static boolean hasSimpleErrorDescription(Collection<ParseTree> globalCalls) {
private static boolean hasSimpleErrorDescription(Collection<BSLParserRuleContext> globalCalls) {
return globalCalls.stream()
.filter(context -> context instanceof BSLParser.GlobalMethodCallContext)
.anyMatch(context -> isAppropriateName((BSLParser.GlobalMethodCallContext) context, PATTERN_SIMPLE_ERROR_DESCRIPTION));
.anyMatch(context -> isAppropriateMethodName(context, PATTERN_SIMPLE_ERROR_DESCRIPTION));
}

private static boolean hasBriefErrorDescription(Collection<ParseTree> globalCalls) {
return globalCalls.stream()
.filter(context -> context instanceof BSLParser.GlobalMethodCallContext)
.anyMatch(context -> isAppropriateName((BSLParser.GlobalMethodCallContext) context, PATTERN_BRIEF_ERROR_DESCRIPTION));
private static boolean hasBriefErrorDescription(Collection<BSLParserRuleContext> calls) {
return calls.stream()
.filter(context -> isAppropriateMethodName(context, PATTERN_BRIEF_ERROR_DESCRIPTION))
.anyMatch(context -> context instanceof BSLParser.GlobalMethodCallContext
|| (context instanceof BSLParser.MethodCallContext && isErrorProcessingCall((BSLParser.MethodCallContext) context)));
}

private static boolean isValidExpression(BSLParser.ExpressionContext context, BSLParser.CodeBlockContext codeBlock,
Expand Down Expand Up @@ -272,7 +300,7 @@ private static boolean isValidVarAssignment(
String varName = identifierContext.getText();
return getAssignment(varName, codeBlock)
.map(BSLParser.AssignmentContext::expression)
.map(expression -> isValidExpression(codeBlock, expression, false))
.map(expression -> isValidCommentExpression(codeBlock, expression, false))
.orElse(true);
}

Expand Down
7 changes: 6 additions & 1 deletion ...om/github/_1c_syntax/bsl/languageserver/diagnostics/UsageWriteLogEventDiagnosticTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ void test() {
.hasRange(204, 6, 206,22)
.hasRange(219, 6, 221,22)
.hasRange(286, 12, 291,39)
.hasSize(14)

.hasRange(354, 6, 356,73)
.hasRange(368, 6, 370,22)
.hasRange(383, 6, 385,22)
.hasRange(439, 12, 444,39)
.hasSize(18)
;

}
Expand Down
153 changes: 153 additions & 0 deletions src/test/resources/diagnostics/UsageWriteLogEventDiagnostic.bsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,156 @@
КороткийТекстСообщения); // ошибка
КонецПопытки;
КонецПроцедуры

&НаСервере
Процедура Тест2()

ЗаписьЖурналаРегистрации("Событие",
УровеньЖурналаРегистрации.Ошибка, , ,
ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));

Попытка
ПравильноеИсключениеВКодеСервер();
Исключение
ЗаписьЖурналаРегистрации("Событие", УровеньЖР,
, , ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));
КонецПопытки;

ЗаписьЖурналаРегистрации(НСтр("ru = 'Мой механизм.Действие с возможной ошибкой'", КодОсновногоЯзыка),
УровеньЖурналаРегистрации.Ошибка, , ,
НСтр("ru = 'Во время выполнения действия произошла неизвестная ошибка.'") + Символы.ПС +
ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));

Попытка
ПравильноеИсключениеВКодеСервер();
Исключение

ТекстОшибки = ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке());
ЗаписьЖурналаРегистрации("Событие", УровеньЖурналаРегистрации.Ошибка, , ,
ТекстОшибки);
КонецПопытки;

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

&НаКлиенте
Процедура ПравильноеИсключениеВКодеКлиент_ОбработкаОшибок()
Попытка
ИсключениеВКодеСервер();
Исключение
ТекстСообщения = ОбработкаОшибок.КраткоеПредставлениеОшибки(ИнформацияОбОшибке());
ПоказатьПредупреждение(,НСтр("ru = 'Операция не может быть выполнена по причине:'") + Символы.ПС + ТекстСообщения);
КонецПопытки;
КонецПроцедуры

&НаКлиенте
Процедура ПравильноСоздатьФайлНаДиске_ОбработкаОшибок()
Попытка
// клиентский код, приводящий к вызову исключения
СоздатьФайлНаДиске();
Исключение
ТекстСообщения = ОбработкаОшибок.КраткоеПредставлениеОшибки(ИнформацияОбОшибке());
ПоказатьПредупреждение(,НСтр("ru = 'Операция не может быть выполнена по причине:'") + Символы.ПС + ТекстСообщения);
ЗаписатьОшибкуРаботыСФайлами(ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));
КонецПопытки;
КонецПроцедуры

&НаКлиенте
Процедура ВИсключенииЕстьКраткоеПредставлениеНоНетПолного_ОбработкаОшибок()
Попытка
// клиентский код, приводящий к вызову исключения
СоздатьФайлНаДиске();
Исключение
ПоказатьПредупреждение(,НСтр("ru = 'Операция не может быть выполнена по причине:'") + Символы.ПС + ТекстСообщения);
ЗаписьЖурналаРегистрации(НСтр("ru = 'Выполнение операции'"),
УровеньЖурналаРегистрации.Ошибка,,,
ОбработкаОшибок.КраткоеПредставлениеОшибки(ИнформацияОбОшибке())); // ошибка
КонецПопытки;
КонецПроцедуры

&НаКлиенте
Процедура ВИсключенииЕстьКраткоеПредставлениеНоНетПолного1_ОбработкаОшибок()
Попытка
// клиентский код, приводящий к вызову исключения
СоздатьФайлНаДиске();
Исключение
ТекстСообщения = ОбработкаОшибок.КраткоеПредставлениеОшибки(ИнформацияОбОшибке());
ПоказатьПредупреждение(,НСтр("ru = 'Операция не может быть выполнена по причине:'") + Символы.ПС + ТекстСообщения);
ЗаписьЖурналаРегистрации(НСтр("ru = 'Выполнение операции'"),
УровеньЖурналаРегистрации.Ошибка,,,
ТекстСообщения); // ошибка
КонецПопытки;
КонецПроцедуры

&НаКлиенте
Процедура ВИсключенииЕстьКраткоеПредставлениеНоНетПолного2_ОбработкаОшибок()
Попытка
// клиентский код, приводящий к вызову исключения
СоздатьФайлНаДиске();
Исключение
ТекстСообщения = ОбработкаОшибок.КраткоеПредставлениеОшибки(ИнформацияОбОшибке());
ДругойТекстСообщения = ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке());
ПоказатьПредупреждение(,НСтр("ru = 'Операция не может быть выполнена по причине:'") + Символы.ПС + ТекстСообщения);
ЗаписьЖурналаРегистрации(НСтр("ru = 'Выполнение операции'"),
УровеньЖурналаРегистрации.Ошибка,,,
ТекстСообщения); // ошибка
КонецПопытки;
КонецПроцедуры

Процедура СложныйМетодСИспользованиемПодробногоПредставленияВнутриИсключения_ОбработкаОшибок(Знач ИмяСобытия, Знач ОписаниеОшибки, Знач Ответ, Знач СсылкаНаДанные = Неопределено) Экспорт
Попытка
Блокировка.Заблокировать();
Исключение
ТекстСообщения = СтроковыеФункцииКлиентСервер.ПодставитьПараметрыВСтроку(
НСтр("ru = 'Не удалось обработать график работы по причине:
|%2'"),
ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));

ЗаписьЖурналаРегистрации(
ИмяСобытия,
УровеньЖурналаРегистрации.Ошибка,
,
СсылкаНаДанные,
ТекстСообщения); // не ошибка
КонецПопытки;
КонецПроцедуры

Процедура СложныйМетодСИспользованиемПодробногоПредставленияВнутриИсключенияВВыражении_ОбработкаОшибок(Знач Выборка, Знач Блокировка) Экспорт
Попытка
Блокировка.Заблокировать();
Исключение
ТекстСообщения =
НСтр("ru = 'Не удалось установить разделение сеанса. Область данных'") + " = "
+ Формат(Выборка.ОбластьДанных, "ЧГ=0")
+ Символы.ПС + ОбработкаОшибок.ПодробноеПредставлениеОшибки(ИнформацияОбОшибке());

ЗаписьЖурналаРегистрации(
ИмяСобытия,
УровеньЖурналаРегистрации.Ошибка,
,
СсылкаНаДанные,
ТекстСообщения); // не ошибка

ЗаписьЖурналаРегистрации(
ИмяСобытия,
УровеньОшибки(),
,
СсылкаНаДанные,
ТекстСообщения); // не ошибка

КонецПопытки;
КонецПроцедуры

Процедура Метод2_ОбработкаОшибок(Знач СсылкаНаДанные, Знач Блокировка)
Попытка
Блокировка.Заблокировать();
Исключение
КороткийТекстСообщения = ОбработкаОшибок.КраткоеПредставлениеОшибки(ИнформацияОбОшибке()) + ОписаниеОшибки();

ЗаписьЖурналаРегистрации(
ИмяСобытия,
УровеньЖурналаРегистрации.Ошибка,
,
СсылкаНаДанные,
КороткийТекстСообщения); // ошибка
КонецПопытки;
КонецПроцедуры