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

Доработка ReferenceIndex - Исправлен расчет ссылок на символы-методы + падение анализа #2833

Merged
merged 23 commits into from
Jul 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2161b20
падающий тест для присваивания
artbear Jul 1, 2022
da7b1bd
пропущенное присваивание находится
artbear Jul 1, 2022
ab15179
падающий кейс
artbear Jul 3, 2022
e13337d
исправил расчет ссылок на общие модули
artbear Jul 3, 2022
02bae50
уточнил условие для параметров
artbear Jul 3, 2022
5637ef7
падающий кейс через публичное API RefIndex
artbear Jul 3, 2022
b154583
Sonar - lambda
artbear Jul 3, 2022
6d10965
ускорение + исправление замечанияй ревью
artbear Jul 3, 2022
f32f949
падающий кейс для присванием с менеджерами объектов
artbear Jul 3, 2022
c262312
учел присваивание с менеджерами объектов
artbear Jul 3, 2022
6ba6b84
Merge remote-tracking branch 'origin/develop' into ref-index-fix-2832
artbear Jul 5, 2022
1759f79
исправил merge
artbear Jul 5, 2022
dc9a69e
еще исправление последствий merge
artbear Jul 5, 2022
6136f67
убран фильтр ReferenceIndex::isReferenceAccessible + тест
artbear Jul 5, 2022
98163e4
Revert "убран фильтр ReferenceIndex::isReferenceAccessible + тест"
artbear Jul 5, 2022
6cd23bd
Update src/main/java/com/github/_1c_syntax/bsl/languageserver/referen…
artbear Jul 19, 2022
0e2d0d8
Update src/main/java/com/github/_1c_syntax/bsl/languageserver/referen…
artbear Jul 19, 2022
c1d35c8
Исправлена ошибка падения анализа
artbear Jul 20, 2022
c3c2cc9
Улучшена обработка описаний оповещения
artbear Jul 20, 2022
43676d0
Merge remote-tracking branch 'origin/develop' into ref-index-fix-2832
artbear Jul 20, 2022
01e1b54
Merge remote-tracking branch 'my-artbear/ref-index-fix-2832' into ref…
artbear Jul 20, 2022
3b284d3
убрано лишнее вычисление
artbear Jul 20, 2022
8fae65a
Update src/main/java/com/github/_1c_syntax/bsl/languageserver/referen…
nixel2007 Jul 22, 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
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,23 @@
import com.github._1c_syntax.bsl.types.ModuleType;
import lombok.RequiredArgsConstructor;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.tree.ParseTree;
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.SymbolKind;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;

import javax.annotation.Nullable;
import java.net.URI;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.function.Predicate;

@Component
Expand Down Expand Up @@ -82,6 +89,19 @@ public void fill(DocumentContext documentContext) {
private class MethodSymbolReferenceIndexFinder extends BSLParserBaseVisitor<BSLParserRuleContext> {

private final DocumentContext documentContext;
private Set<String> commonModuleMdoRefFromSubParams = Collections.emptySet();

@Override
public BSLParserRuleContext visitProcDeclaration(BSLParser.ProcDeclarationContext ctx) {
commonModuleMdoRefFromSubParams = calcParams(ctx.paramList());
return super.visitProcDeclaration(ctx);
}

@Override
public BSLParserRuleContext visitFuncDeclaration(BSLParser.FuncDeclarationContext ctx) {
commonModuleMdoRefFromSubParams = calcParams(ctx.paramList());
return super.visitFuncDeclaration(ctx);
}

@Override
public BSLParserRuleContext visitCallStatement(BSLParser.CallStatementContext ctx) {
Expand Down Expand Up @@ -156,11 +176,28 @@ public BSLParserRuleContext visitNewExpression(BSLParser.NewExpressionContext ct
return super.visitNewExpression(ctx);
}

@Override
public BSLParserRuleContext visitLValue(BSLParser.LValueContext ctx) {
final var identifier = ctx.IDENTIFIER();
if (identifier != null){
final List<? extends BSLParser.ModifierContext> modifiers = Optional.ofNullable(ctx.acceptor())
.map(BSLParser.AcceptorContext::modifier)
.orElseGet(Collections::emptyList);
String mdoRef = MdoRefBuilder.getMdoRef(documentContext, identifier, modifiers);
if (!mdoRef.isEmpty()) {
Methods.getMethodName(ctx).ifPresent(methodName -> checkCall(mdoRef, methodName));
}
}
return super.visitLValue(ctx);
}

private void checkCall(String mdoRef, Token methodName) {
var methodNameText = Strings.trimQuotes(methodName.getText());
Map<ModuleType, URI> modules = documentContext.getServerContext().getConfiguration().getModulesByMDORef(mdoRef);
final var configuration = documentContext.getServerContext().getConfiguration();
Map<ModuleType, URI> modules = configuration.getModulesByMDORef(mdoRef);
for (ModuleType moduleType : modules.keySet()) {
if (!DEFAULT_MODULE_TYPES.contains(moduleType)) {
if (!DEFAULT_MODULE_TYPES.contains(moduleType)
|| (moduleType == ModuleType.CommonModule && commonModuleMdoRefFromSubParams.contains(mdoRef))) {
continue;
}
addMethodCall(mdoRef, moduleType, methodNameText, Ranges.create(methodName));
Expand All @@ -172,6 +209,10 @@ private void addMethodCall(String mdoRef, ModuleType moduleType, String methodNa
}

private void addCallbackMethodCall(BSLParser.CallParamContext methodName, String mdoRef) {
// todo: move this out of method
if (mdoRef.isEmpty()){
Copy link
Member

Choose a reason for hiding this comment

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

а в каком случае сюда может прилететь пустой mdoRef?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

а в каком случае сюда может прилететь пустой mdoRef?

в этот параметр передается результат getModule, который теперь может возвращать пустую строку, если не может получить модуль.

  • важно, чтобы случайно не обработать неподдерживаемую ссылку виды ОбщийМодуль("Имя") или ОбщегоНазначения.ОбщийМодуль("Имя")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

мне самому не нравится проверка mdoRef на пустоту.
но другой вариант с выносом этой проверки на уровень выше также не очень - дубль проверки на пустоту будет.

Copy link
Member

Choose a reason for hiding this comment

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

Не совсем понимаю. Где? В какой строке? Я не могу припомнить ситуации, чтобы мдореф был пуст. Если MdoRefBuilder не может построить mdoRef, он возвращает uri

Copy link
Contributor Author

Choose a reason for hiding this comment

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

посмотри код вызова вызовов getModule и сам метод getModule и увидишь.

Copy link
Member

Choose a reason for hiding this comment

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

Посмотрел. мне не нравится, что у метода addCallbackMethodCall появилось право не добавлять вызов метода. лучше все же продублировать проверку на пустоту, но не размазывать ответственность. Либо сделать, чтобы getModule возвращал Optional, например.

nixel2007 marked this conversation as resolved.
Show resolved Hide resolved
return;
}
Methods.getMethodName(methodName).ifPresent((Token methodNameToken) -> {
if (!mdoRef.equals(MdoRefBuilder.getMdoRef(documentContext))) {
checkCall(mdoRef, methodNameToken);
Expand All @@ -187,13 +228,34 @@ private void addCallbackMethodCall(BSLParser.CallParamContext methodName, String
}

private String getModule(BSLParser.CallParamContext callParamContext) {
return NotifyDescription.getFirstMember(callParamContext)
final var complexIdentifierContext1 = NotifyDescription.getFirstMember(callParamContext)
.map(BSLParser.MemberContext::complexIdentifier)
.filter(complexIdentifierContext -> complexIdentifierContext.IDENTIFIER() != null)
.filter(complexIdentifierContext -> complexIdentifierContext.modifier().isEmpty());
if (complexIdentifierContext1.isEmpty()){
return "";
}
return complexIdentifierContext1
.filter(Predicate.not(Modules::isThisObject))
.map(complexIdentifier -> MdoRefBuilder.getMdoRef(documentContext, complexIdentifier))
.orElse(MdoRefBuilder.getMdoRef(documentContext));
}

private Set<String> calcParams(@Nullable BSLParser.ParamListContext paramList) {
if (paramList == null) {
return Collections.emptySet();
}
final var configuration = documentContext.getServerContext().getConfiguration();
return paramList.param().stream()
.map(BSLParser.ParamContext::IDENTIFIER)
.filter(Objects::nonNull)
.map(ParseTree::getText)
.map(configuration::getCommonModule)
.filter(Optional::isPresent)
.flatMap(Optional::stream)
.map(mdCommonModule -> mdCommonModule.getMdoReference().getMdoRef())
.collect(Collectors.toSet());
}
}

@RequiredArgsConstructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,10 @@ public static Optional<Token> getMethodName(BSLParser.CallParamContext callParam
.map(BSLParser.StringContext::getStart);
}

public static Optional<Token> getMethodName(BSLParser.LValueContext lValueContext) {
return Optional.ofNullable(lValueContext.acceptor())
.map(BSLParser.AcceptorContext::modifier)
.flatMap(Methods::getMethodName);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,15 @@
public class Modules {

private static final Pattern THIS_OBJECT_PATTERN = CaseInsensitivePattern.compile(
"^(ЭтотОбъект|ThisObject)$"
"ЭтотОбъект|ThisObject"
);

public static boolean isThisObject(BSLParser.ComplexIdentifierContext complexIdentifier) {
return THIS_OBJECT_PATTERN.matcher(complexIdentifier.IDENTIFIER().getText()).find();
final var identifier = complexIdentifier.IDENTIFIER();
if (identifier == null){
return false;
}
return THIS_OBJECT_PATTERN.matcher(identifier.getText()).matches();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
import com.github._1c_syntax.bsl.types.ModuleType;
import org.eclipse.lsp4j.Location;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.SymbolKind;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;

import javax.annotation.PostConstruct;
import java.nio.file.Paths;
import java.util.stream.Collectors;

import static com.github._1c_syntax.bsl.languageserver.util.TestUtils.PATH_TO_METADATA;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -158,6 +160,95 @@ void testGetReferenceToCommonModuleMethod() {
assertThat(reference.getUri()).isEqualTo(uri);
}

@Test
void getReferencesToCommonModuleMethodFromAssignment() {
// given
var documentContext = TestUtils.getDocumentContextFromFile(PATH_TO_FILE);
var methodSymbol = documentContext.getSymbolTree().getMethodSymbol("Тест_Присваивание").orElseThrow();
var commonModuleContext = serverContext.getDocument("CommonModule.ПервыйОбщийМодуль", ModuleType.CommonModule).orElseThrow();
var calledMethodSymbol = commonModuleContext.getSymbolTree().getMethodSymbol("НеУстаревшаяФункция").orElseThrow();

var uri = documentContext.getUri();

// when
final var referencesTo = referenceIndex.getReferencesTo(calledMethodSymbol).stream()
.filter(reference -> reference.getUri().equals(uri))
.filter(reference -> Ranges.containsRange(methodSymbol.getRange(), reference.toLocation().getRange()))
.collect(Collectors.toList());

// then
var reference = referencesTo.get(0);
assertThat(reference.getFrom()).isEqualTo(methodSymbol);
assertThat(reference.getSymbol()).isEqualTo(calledMethodSymbol);
assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(8, 26, 45));
assertThat(reference.getUri()).isEqualTo(uri);

reference = referencesTo.get(1);
assertThat(reference.getFrom()).isEqualTo(methodSymbol);
assertThat(reference.getSymbol()).isEqualTo(calledMethodSymbol);
assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(9, 26, 45));
assertThat(reference.getUri()).isEqualTo(uri);

reference = referencesTo.get(2);
assertThat(reference.getFrom()).isEqualTo(methodSymbol);
assertThat(reference.getSymbol()).isEqualTo(calledMethodSymbol);
assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(10, 22, 41));
assertThat(reference.getUri()).isEqualTo(uri);

assertThat(referencesTo).hasSize(3);
}

@Test
void getReferencesToFullPathModuleMethodFromAssignment() {
// given
var documentContext = TestUtils.getDocumentContextFromFile(PATH_TO_FILE);
var methodSymbol = documentContext.getSymbolTree().getMethodSymbol("Тест_ВызовЧерезПолноеИмяОбъекта").orElseThrow();
var commonModuleContext = serverContext.getDocument("InformationRegister.РегистрСведений1", ModuleType.ManagerModule).orElseThrow();
var calledMethodSymbol = commonModuleContext.getSymbolTree().getMethodSymbol("НеУстаревшаяФункция").orElseThrow();

var uri = documentContext.getUri();

// when
final var referencesTo = referenceIndex.getReferencesTo(calledMethodSymbol).stream()
.filter(reference -> reference.getUri().equals(uri))
.filter(reference -> Ranges.containsRange(methodSymbol.getRange(), reference.toLocation().getRange()))
.collect(Collectors.toList());

// then
var reference = referencesTo.get(0);
assertThat(reference.getFrom()).isEqualTo(methodSymbol);
assertThat(reference.getSymbol()).isEqualTo(calledMethodSymbol);
assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(22, 42, 61));
assertThat(reference.getUri()).isEqualTo(uri);

reference = referencesTo.get(1);
assertThat(reference.getFrom()).isEqualTo(methodSymbol);
assertThat(reference.getSymbol()).isEqualTo(calledMethodSymbol);
assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(23, 42, 61));
assertThat(reference.getUri()).isEqualTo(uri);

reference = referencesTo.get(2);
assertThat(reference.getFrom()).isEqualTo(methodSymbol);
assertThat(reference.getSymbol()).isEqualTo(calledMethodSymbol);
assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(24, 38, 57));
assertThat(reference.getUri()).isEqualTo(uri);

assertThat(referencesTo).hasSize(3);
}

@Test
void getReferencesToCommonModuleMethodWithEqualNameWitMethodParam() {

var documentContext = TestUtils.getDocumentContextFromFile(PATH_TO_FILE);
var methodSymbol = documentContext.getSymbolTree().getMethodSymbol("Тест_ИмяПараметр").orElseThrow();

final var referencesFromLocationRepo = referenceIndex.getReferencesFrom(documentContext.getUri(), SymbolKind.Method).stream()
.filter(reference -> Ranges.containsRange(methodSymbol.getRange(), reference.getSelectionRange()))
.collect(Collectors.toList());

assertThat(referencesFromLocationRepo).isEmpty();
}

@Test
void testCantGetReferenceToNonExportCommonModuleMethod() {
// given
Expand Down
20 changes: 20 additions & 0 deletions src/test/resources/references/ReferenceIndex.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,23 @@
РегистрыСведений.РегистрСведений1.УстаревшаяПроцедура();
ПервыйОбщийМодуль.Тест();
КонецПроцедуры

Процедура Тест_Присваивание()
А = ПервыйОбщийМодуль.НеУстаревшаяФункция();
Б = ПервыйОбщийМодуль.НеУстаревшаяФункция().Добавить();
ПервыйОбщийМодуль.НеУстаревшаяФункция().Реквизит = 10;
КонецПроцедуры

Процедура Тест_ИмяПараметр(ПервыйОбщийМодуль)
nixel2007 marked this conversation as resolved.
Show resolved Hide resolved
ПервыйОбщийМодуль.НеУстаревшаяПроцедура(1, 2); // ошибка
А = ПервыйОбщийМодуль.НеУстаревшаяФункция(); // ошибка
ПервыйОбщийМодуль.НеУстаревшаяФункция().Добавить(); // ошибка
ПервыйОбщийМодуль.НеУстаревшаяФункция().Реквизит = 10; // ошибка
Б = ПервыйОбщийМодуль.НеУстаревшаяФункция().Добавить(); // ошибка
КонецПроцедуры

Процедура Тест_ВызовЧерезПолноеИмяОбъекта()
А = РегистрыСведений.РегистрСведений1.НеУстаревшаяФункция();
Б = РегистрыСведений.РегистрСведений1.НеУстаревшаяФункция().Добавить();
РегистрыСведений.РегистрСведений1.НеУстаревшаяФункция().Реквизит = 10;
КонецПроцедуры
22 changes: 20 additions & 2 deletions src/test/resources/references/ReferenceIndexNotifyDescription.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,28 @@
ЭтотОбъект
);

// Такое объявление не ловим
ОписаниеОповещения2 = Новый ОписаниеОповещения(
"ОбработчикОписаниеОповещения",
ОбщийМодуль("Имя"),
,
"ОшибкаОписаниеОповещения",
ОбщегоНазначения.ОбщийМодуль("Имя")
);

// Такое объявление не ловим
ОписаниеОповещения3 = Новый ОписаниеОповещения(
"ОбработчикОписаниеОповещения",
"ИмяМодуля",
,
"ОшибкаОписаниеОповещения",
"ДругоеИмяМодуля"
);

// Такое объявление не ловим
ОбработчикОписаниеОповещения = "ОбработчикОписаниеОповещенияВПеременной";
ОшибкаОписаниеОповещения = "ОшибкаОписаниеОповещенияВПеременной";
ОписаниеОповещения2 = Новый ОписаниеОповещения(
ОписаниеОповещения4 = Новый ОписаниеОповещения(
ОбработчикОписаниеОповещения,
ЭтотОбъект,
,
Expand All @@ -26,7 +44,7 @@
ПараметрыОбработчика[1] = ЭтотОбъект;
ПараметрыОбработчика[3] = "ОшибкаОписаниеОповещения";
ПараметрыОбработчика[4] = ЭтотОбъект;
ОписаниеОповещения3 = Новый("ОписаниеОповещения", ПараметрыОбработчика);
ОписаниеОповещения5 = Новый("ОписаниеОповещения", ПараметрыОбработчика);

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

Expand Down