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

Выведение типов #3268

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

nixel2007
Copy link
Member

@nixel2007 nixel2007 commented Apr 24, 2024

Описание

Черновик для вычислителя типов.

На данный момент вычисляет типы на лету, поддерживает вычисление для constValue и оператора Новый.

TODO:

  • Вычисление типа для globalMethodCall (из описания метода)
  • Рекурсивное вычисление выражений на базе expressionTree и обращений через точку

Связанные задачи

Closes

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

Copy link

coderabbitai bot commented Apr 24, 2024

Обзор

Внесены изменения в функциональность сервера языка BSL, включающие добавление провайдера CompletionOptions для обработки автодополнения кода с определёнными символами-триггерами, а также улучшение поддержки типов и референсов.

Изменения

Файл Описание изменений
src/main/java/.../BSLLanguageServer.java Добавлен метод getCompletionProvider() для обработки автодополнения.
src/main/java/.../BSLTextDocumentService.java Добавлен метод completion для обработки запросов на автодополнение, а также соответствующие импорты и поле completionProvider.
src/main/java/.../hover/MarkupContentBuilder.java Изменён метод getContent для добавления параметра Reference.
src/main/java/.../hover/MethodSymbolMarkupContentBuilder.java Добавлен импорт Reference, изменён метод getContent для добавления параметра Reference.
src/main/java/.../hover/VariableSymbolMarkupContentBuilder.java Добавлены импорты Reference, Type, TypeResolver, поле typeResolver и изменён метод getContent для добавления параметра Reference.
src/main/java/.../providers/CompletionProvider.java Добавлены методы getCompletions, getCompletionItemKind, getChildren для обработки автодополнения.
src/main/java/.../types/KnownTypes.java Новый файл, управляющий картой типов и символов, предоставляющий методы для добавления типов и обработки изменений контекста документа.
src/main/java/.../types/Type.java Новый файл, представляющий класс Type с полем name.
src/main/java/.../types/TypeResolver.java Добавлены методы для разрешения типов в BSL коде, включая findTypes, calculateTypes, newTypeName, returnedValue.
src/main/java/.../utils/Trees.java Изменён метод getRootParent для принятия параметра RuleNode вместо BSLParserRuleContext.
src/test/java/.../hover/MethodSymbolMarkupContentBuilderTest.java Добавлен метод getReference для создания объекта Reference и обновлены тесты для использования этого метода.
src/test/java/.../hover/VariableSymbolMarkupContentBuilderTest.java Добавлен метод getReference, обновлены вызовы методов для включения референсов, добавлены соответствующие импорты.
src/test/java/.../providers/CompletionProviderTest.java Новый файл, содержащий тесты для класса CompletionProvider.
src/test/java/.../types/TypeResolverTest.java Добавлены тесты для класса TypeResolver.
src/test/resources/metadata/oslib/Классы/МойКласс.os Новый файл, содержащий экспортируемые и неэкспортируемые переменные, функции и процедуры.
src/test/resources/providers/completion.os Новый файл, содержащий ссылку на модуль metadata/oslib и создание экземпляра класса МойКласс.
src/test/resources/types/TypeResolver.os Новый файл, содержащий объявления переменных, аннотаций, функций и присваиваний в пользовательском языке.

Поэма

В коде BSL новизна пришла,
Автодополнение теперь всегда,
Типы и ссылки мы улучшили,
В тестах новые пути открыли.
Пусть код ваш будет чист и ясен,
С новыми фичами он прекрасен!
🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Apr 24, 2024

Qodana Community for JVM

18 new problems were found

Inspection name Severity Problems
Statement with empty body 🔶 Warning 5
'@Serial' annotation could be used 🔶 Warning 4
Nullability and data flow problems 🔶 Warning 2
Mismatched query and update of collection 🔶 Warning 2
@Value modifiers 🔶 Warning 2
Class is exposed outside of its visibility scope 🔶 Warning 1
Constant values 🔶 Warning 1
Mismatched read and write of array 🔶 Warning 1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

var maybeDescription = describableSymbol.getDescription();
if (maybeDescription.isPresent()) {
var description = maybeDescription.get();
if (description instanceof VariableDescription variableDescription) {

Check warning

Code scanning / QDJVMC

Statement with empty body

'if' statement has empty body

@Value
public class Type {
private final String name;

Check warning

Code scanning / QDJVM

@Value modifiers Warning

@Value already marks non-static fields final.

@Value
public class Type {
private final String name;

Check warning

Code scanning / QDJVM

@Value modifiers Warning

@Value already marks non-static, package-local fields private.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Comment on lines 74 to 101
private List<Type> calculateTypes(SourceDefinedSymbol symbol) {

// variable description resolver
if (symbol instanceof Describable describableSymbol) {
var maybeDescription = describableSymbol.getDescription();
if (maybeDescription.isPresent()) {
var description = maybeDescription.get();
if (description instanceof VariableDescription variableDescription) {
// TODO: use new type information from new bsp-parser
var purposeDescription = variableDescription.getPurposeDescription();
var typeName = Pattern.compile("^(\\S+)").matcher(purposeDescription).results()
.findFirst()
.map(MatchResult::group)
.orElse("");

if (!typeName.isEmpty()) {
return List.of(new Type(typeName));
}
}
}
}
Copy link

Choose a reason for hiding this comment

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

Метод calculateTypes содержит сложную логику разрешения типов. Рассмотрите возможность рефакторинга для улучшения читаемости и использования предварительно скомпилированных шаблонов для регулярных выражений.

Comment on lines 114 to 150
private List<Type> calculateTypes(URI uri, Reference reference) {

// source defined symbol resolver
if (reference.isSourceDefinedSymbolReference()) {
return calculateTypes(reference.getSourceDefinedSymbol().orElseThrow());
}

// expression tree resolver
if (reference.getOccurrenceType() == OccurrenceType.DEFINITION) {
var document = serverContext.getDocument(uri);
var ast = document.getAst();
var position = reference.getSelectionRange().getStart();
return calculateTypes(uri, ast, position);
}

// no-op
return Collections.emptyList();
}
Copy link

Choose a reason for hiding this comment

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

Метод calculateTypes корректно обрабатывает различные типы ссылок. Однако, следует обратить внимание на возможное возникновение NullPointerException при вызове getAst. Рекомендуется добавить проверку на null.

- var ast = document.getAst();
+ var ast = document != null ? document.getAst() : null;
+ if (ast == null) return Collections.emptyList();

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
private List<Type> calculateTypes(URI uri, Reference reference) {
// source defined symbol resolver
if (reference.isSourceDefinedSymbolReference()) {
return calculateTypes(reference.getSourceDefinedSymbol().orElseThrow());
}
// expression tree resolver
if (reference.getOccurrenceType() == OccurrenceType.DEFINITION) {
var document = serverContext.getDocument(uri);
var ast = document.getAst();
var position = reference.getSelectionRange().getStart();
return calculateTypes(uri, ast, position);
}
// no-op
return Collections.emptyList();
}
private List<Type> calculateTypes(URI uri, Reference reference) {
// source defined symbol resolver
if (reference.isSourceDefinedSymbolReference()) {
return calculateTypes(reference.getSourceDefinedSymbol().orElseThrow());
}
// expression tree resolver
if (reference.getOccurrenceType() == OccurrenceType.DEFINITION) {
var document = serverContext.getDocument(uri);
var ast = document != null ? document.getAst() : null;
if (ast == null) return Collections.emptyList();
var position = reference.getSelectionRange().getStart();
return calculateTypes(uri, ast, position);
}
// no-op
return Collections.emptyList();
}

// expression tree resolver
if (reference.getOccurrenceType() == OccurrenceType.DEFINITION) {
var document = serverContext.getDocument(uri);
var ast = document.getAst();

Check warning

Code scanning / QDJVM

Nullability and data flow problems Warning

Method invocation getAst may produce NullPointerException
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4


// TODO: Create LRU cache for calculated types.

// TODO: Use reference instead of symbol. Refactor hover provider to pass references to markup content builders.
Copy link

Choose a reason for hiding this comment

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

TODO: Использовать ссылку вместо символа. Рефакторинг поставщика всплывающих подсказок для передачи ссылок в построители содержимого разметки.

Могу ли я помочь с реализацией этого изменения или открыть задачу на GitHub для его отслеживания?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Copy link

github-actions bot commented Jun 4, 2024

Qodana for JVM

27 new problems were found

Inspection name Severity Problems
Incorrect autowiring in Spring bean components 🔴 Failure 2
Incorrectly referenced bean in @Lookup annotation of Spring component 🔴 Failure 1
Invalid properties configuration 🔶 Warning 6
Statement with empty body 🔶 Warning 5
'@Serial' annotation could be used 🔶 Warning 4
Nullability and data flow problems 🔶 Warning 2
@Value modifiers 🔶 Warning 2
Class is exposed outside of its visibility scope 🔶 Warning 1
Constant values 🔶 Warning 1
Mismatched read and write of array 🔶 Warning 1
Non recommended 'field' injections ◽️ Notice 2

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Detected 99 dependencies

Third-party software list

This page lists the third-party software dependencies used in bsl-language-server

Dependency Version Licenses
aho-corasick-double-array-trie 1.2.2 Apache-2.0
aspectjrt 1.9.19 Apache-2.0
checker-qual 3.33.0 MIT
classgraph 4.8.147 MIT
commons-beanutils 1.9.4 Apache-2.0
commons-collections 3.2.2 Apache-2.0
commons-collections4 4.4 Apache-2.0
commons-digester 2.1 Apache-2.0
commons-exec 1.3 Apache-2.0
commons-io 2.13.0 Apache-2.0
commons-lang3 3.12.0 Apache-2.0
commons-logging 1.2 Apache-2.0
commons-pool2 2.12.0 Apache-2.0
commons-text 1.10.0 Apache-2.0
commons-validator 1.7 Apache-2.0
emoji-java 5.1.1 MIT
error_prone_annotations 2.18.0 Apache-2.0
grpc-netty-shaded 1.50.2 Apache-2.0
gson 2.10.1 Apache-2.0
guava 32.0.1-jre Apache-2.0
hunspell 2.1.1 Apache-2.0
istack-commons-runtime 4.1.2 BSD-3-Clause
j2objc-annotations 2.8 Apache-2.0
jackson-annotations 2.15.4 Apache-2.0
jackson-core 2.15.4 Apache-2.0
jackson-databind 2.15.4 Apache-2.0
jackson-datatype-jdk8 2.15.4 Apache-2.0
jackson-datatype-jsr310 2.15.4 Apache-2.0
jackson-module-parameter-names 2.15.4 Apache-2.0
jakarta.activation-api 2.1.3 BSD-3-Clause
jakarta.annotation-api 2.1.1 Classpath-exception-2.0
EPL-2.0
GPL-2.0-only
jakarta.websocket-api 2.1.1 Classpath-exception-2.0
EPL-2.0
GPL-2.0-only
jakarta.xml.bind-api 4.0.2 BSD-3-Clause
java-sarif 2.0 MIT
javax.activation-api 1.2.0 CDDL-1.1
GPL-2.0-only
javax.annotation-api 1.3.2 CDDL-1.0
GPL-2.0-only
ORACLE-OPENJDK-EXCEPTION-2.0
jaxb-api 2.3.0 CDDL-1.1
GPL-2.0-only
ORACLE-OPENJDK-EXCEPTION-2.0
jaxb-core 4.0.5 BSD-3-Clause
jaxb-runtime 4.0.5 BSD-3-Clause
jgrapht-core 1.5.2 EPL-1.0
LGPL-2.0-or-later
jline 3.21.0 BSD-3-Clause
jna-platform 5.12.1 Apache-2.0
jna 5.12.1 Apache-2.0
jsr305 3.0.2 Apache-2.0
jul-to-slf4j 2.0.13 MIT
language-detector 0.6 Apache-2.0
language-en 6.1 LGPL-2.0-or-later
language-ru 6.1 LGPL-2.0-or-later
languagetool-core 6.1 LGPL-2.0-or-later
log4j-api 2.21.1 Apache-2.0
log4j-to-slf4j 2.21.1 Apache-2.0
lombok edge-snapshot BSD-3-CLAUSE-NO-TRADEMARK
MIT
lucene-backward-codecs 5.5.5 Apache-2.0
lucene-core 5.5.5 Apache-2.0
micrometer-commons 1.12.5 Apache-2.0
micrometer-core 1.12.5 Apache-2.0
micrometer-observation 1.12.5 Apache-2.0
micrometer-registry-prometheus 1.12.5 Apache-2.0
mxparser 1.2.2 xpp
opennlp-chunk-models 1.5 Apache-2.0
opennlp-postag-models 1.5 Apache-2.0
opennlp-tokenize-models 1.5 Apache-2.0
opennlp-tools 1.9.4 Apache-2.0
openregex 1.1.1 LGPL-2.0-or-later
progressbar 0.9.5 MIT
proto-google-common-protos 2.9.0 Apache-2.0
segment 2.0.1 MIT
simpleclient 0.16.0 Apache-2.0
simpleclient_common 0.16.0 Apache-2.0
simpleclient_guava 0.16.0 Apache-2.0
simpleclient_tracer_common 0.16.0 Apache-2.0
simpleclient_tracer_otel 0.16.0 Apache-2.0
simpleclient_tracer_otel_agent 0.16.0 Apache-2.0
slf4j-api 2.0.13 MIT
snakeyaml 2.2 Apache-2.0
spring-aop 6.1.6 Apache-2.0
spring-beans 6.1.6 Apache-2.0
spring-boot-autoconfigure 3.2.5 Apache-2.0
spring-boot-starter-json 3.2.5 Apache-2.0
spring-boot-starter-logging 3.2.5 Apache-2.0
spring-boot-starter-tomcat 3.2.5 Apache-2.0
spring-boot-starter-web 3.2.5 Apache-2.0
spring-boot-starter-websocket 3.2.5 Apache-2.0
spring-boot-starter 3.2.5 Apache-2.0
spring-boot 3.2.5 Apache-2.0
spring-context 6.1.6 Apache-2.0
spring-core 6.1.6 Apache-2.0
spring-expression 6.1.6 Apache-2.0
spring-jcl 6.1.6 Apache-2.0
spring-messaging 6.1.6 Apache-2.0
spring-web 6.1.6 Apache-2.0
spring-webmvc 6.1.6 Apache-2.0
spring-websocket 6.1.6 Apache-2.0
tomcat-embed-core 10.1.20 Apache-2.0
CDDL-1.0
PROPRIETARY-LICENSE
tomcat-embed-el 10.1.20 Apache-2.0
tomcat-embed-websocket 10.1.20 Apache-2.0
txw2 4.0.5 BSD-3-Clause
vavr-match 0.10.2 Apache-2.0
vavr 0.10.2 Apache-2.0
Contact Qodana team

Contact us at [email protected]

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7b21327 and 3cb6bbf.

Files selected for processing (18)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (3 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java (4 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.java (2 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java (2 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (5 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/KnownTypes.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/Type.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeResolver.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (2 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.java (5 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java (7 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.java (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeResolverTest.java (1 hunks)
  • src/test/resources/metadata/oslib/Классы/МойКласс.os (1 hunks)
  • src/test/resources/providers/completion.os (1 hunks)
  • src/test/resources/types/TypeResolver.os (1 hunks)
Files skipped from review due to trivial changes (1)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeResolverTest.java
Additional comments not posted (19)
src/test/resources/providers/completion.os (1)

1-4: Код корректно импортирует необходимый модуль и создает экземпляр класса для тестирования функциональности автодополнения. Рекомендуется добавить комментарий к незавершенной строке, чтобы пояснить ее назначение в контексте теста.

src/test/resources/metadata/oslib/Классы/МойКласс.os (1)

1-17: Структура класса корректно разделяет экспортные и неэкспортные методы и процедуры. Убедитесь, что экспортные элементы используются в соответствии с требованиями проекта.

src/test/resources/types/TypeResolver.os (1)

1-34: Файл корректно демонстрирует объявления переменных и определения функций для тестирования разрешения типов. Рекомендуется улучшить четкость аннотаций для лучшего понимания их назначения.

src/main/java/com/github/_1c_syntax/bsl/languageserver/types/Type.java (1)

28-28: Учитывая предыдущие комментарии, аннотация @Value уже обеспечивает необходимые модификаторы для поля name. Убедитесь, что это соответствует вашим требованиям к иммутабельности и доступности поля.

src/main/java/com/github/_1c_syntax/bsl/languageserver/types/KnownTypes.java (1)

13-38: Класс корректно управляет типами и символами с использованием потокобезопасной коллекции. Рекомендуется перенести логику из метода обработчика событий в более подходящее место, как указано в комментарии TODO.

src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.java (1)

Line range hint 25-42: Добавление параметра Reference в метод getContent улучшает функциональность построения контента для всплывающих окон, позволяя интегрировать больше контекста о символе. Убедитесь, что это изменение корректно интегрировано в остальную часть системы.

src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.java (2)

19-20: Класс корректно аннотирован для интеграционного тестирования.


37-55: Тест completionAfterDotOnOSClass корректно проверяет функциональность автодополнения. Проверки адекватны ожидаемому поведению.

src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.java (1)

55-55: Метод getHover корректно обрабатывает ссылки и генерирует содержимое всплывающей подсказки. Использование Optional и лямбда-выражений улучшает читаемость и структуру кода.

src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java (1)

33-67: Метод getCompletions эффективно использует TypeResolver и KnownTypes для определения типов и генерации предложений автодополнения. Код хорошо структурирован и использует возможности Java для работы со стримами и опциональными значениями.

src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (1)

Line range hint 53-81: Метод getContent корректно генерирует разметку для символов переменных, включая сигнатуру, местоположение, описание и типы. Использование вспомогательных методов и TypeResolver улучшает читаемость и функциональность кода.

src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.java (2)

67-70: Тесты testContentFromDirectFile и testContentFromManagerModule адекватно проверяют генерацию содержимого для символов методов из разных модулей. Проверки охватывают структуру и специфические секции содержимого.

Also applies to: 134-134


156-159: Тест testMethodsFromCommonModule корректно проверяет генерацию содержимого для методов из общего модуля. Проверки охватывают структуру и специфические секции содержимого.

src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java (1)

68-71: Тесты testFileVarContentFromDirectFile_NoComments и testContentFromObjectModule адекватно проверяют генерацию содержимого для переменных из разных контекстов. Проверки охватывают структуру и специфические секции содержимого.

Also applies to: 201-201

src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java (1)

67-67: Добавлен параметр Reference в метод getContent. Убедитесь, что все вызовы этого метода обновлены с учетом нового параметра.

Verification successful

Добавлен параметр Reference в метод getContent. Убедитесь, что все вызовы этого метода обновлены с учетом нового параметра.

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.java: вызов метода getContent обновлен.
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.java: вызовы метода getContent обновлены.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Описание: Проверка всех вызовов функции `getContent` на соответствие новой сигнатуре.

# Тест: Поиск использования функции. Ожидаем: Только вхождения с новой сигнатурой.
rg --type java $'getContent'

Length of output: 4422

src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (1)

342-348: Добавлен метод getCompletionProvider для настройки параметров автодополнения. Убедитесь, что эти параметры соответствуют требованиям вашего проекта.

src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (1)

290-297: Изменение сигнатуры метода getRootParent на принятие RuleNode вместо BSLParserRuleContext может повлиять на использование этого метода в кодовой базе.

src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java (2)

35-35: Добавлены новые импорты, связанные с функциональностью автодополнения, что соответствует целям PR.

Also applies to: 64-66


378-389: Метод completion корректно реализован и соответствует целям PR, обрабатывая запросы автодополнения.

private final ReferenceResolver referenceResolver;
private final ReferenceIndex referenceIndex;

// TODO: Create LRU cache for calculated types.
Copy link

@coderabbitai coderabbitai bot Jun 4, 2024

Choose a reason for hiding this comment

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

TODO: Создать кэш LRU для вычисленных типов.

Хотите, чтобы я сгенерировал код для реализации этого кэша или создал задачу на GitHub для отслеживания этой задачи?

Copy link
Member Author

Choose a reason for hiding this comment

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

Да, попробуй создать кэш

Copy link

Choose a reason for hiding this comment

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

@nixel2007, я предлагаю использовать библиотеку Guava для реализации LRU кэша, так как она предоставляет простой и надежный API для создания и управления кэшем. Вот пример кода, который можно добавить в класс TypeResolver для интеграции LRU кэша:

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.Cache;

@Component
@RequiredArgsConstructor
public class TypeResolver {

    private final ServerContext serverContext;
    private final ReferenceResolver referenceResolver;
    private final ReferenceIndex referenceIndex;

    // Создаем кэш с максимальным размером 100 и автоматическим удалением записей после 10 минут неактивности
    private final Cache<String, List<Type>> typesCache = CacheBuilder.newBuilder()
        .maximumSize(100)
        .expireAfterAccess(10, TimeUnit.MINUTES)
        .build();

    // Пример метода, использующего кэш
    public List<Type> calculateTypes(URI uri, BSLParser.ExpressionContext expression) {
        String key = uri.toString() + expression.getText();
        return typesCache.get(key, () -> computeTypes(uri, expression));
    }

    // Метод для вычисления типов, который ранее был реализован
    private List<Type> computeTypes(URI uri, BSLParser.ExpressionContext expression) {
        // Логика вычисления типов
    }
}

Этот код добавляет кэш для хранения вычисленных типов, где ключом является уникальное сочетание URI и текста выражения. Элементы кэша автоматически удаляются после 10 минут неактивности или когда количество элементов превышает 100. Это должно значительно улучшить производительность при повторных вычислениях типов для одних и тех же выражений.

Пожалуйста, дайте знать, если нужно что-то изменить или добавить.

Copy link
Member Author

Choose a reason for hiding this comment

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

Не хочу использовать guava. Если что-то из библиотек apache?

Copy link

Choose a reason for hiding this comment

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

@nixel2007, для реализации LRU кэша без использования Guava, можно использовать LRUMap из библиотеки Apache Commons Collections. Вот пример интеграции LRUMap в класс TypeResolver:

import org.apache.commons.collections4.map.LRUMap;
import com.github._1c_syntax.bsl.languageserver.context.ServerContext;
import com.github._1c_syntax.bsl.languageserver.context.symbol.Describable;
import com.github._1c_syntax.bsl.languageserver.context.symbol.SourceDefinedSymbol;
import com.github._1c_syntax.bsl.languageserver.context.symbol.Symbol;
import com.github._1c_syntax.bsl.languageserver.context.symbol.description.MethodDescription;
import com.github._1c_syntax.bsl.languageserver.context.symbol.description.TypeDescription;
import com.github._1c_syntax.bsl.languageserver.context.symbol.variable.VariableDescription;
import com.github._1c_syntax.bsl.languageserver.references.ReferenceIndex;
import com.github._1c_syntax.bsl.languageserver.references.ReferenceResolver;
import com.github._1c_syntax.bsl.languageserver.references.model.OccurrenceType;
import com.github._1c_syntax.bsl.languageserver.references.model.Reference;
import com.github._1c_syntax.bsl.languageserver.utils.Ranges;
import com.github._1c_syntax.bsl.languageserver.utils.Trees;
import com.github._1c_syntax.bsl.languageserver.utils.bsl.Constructors;
import com.github._1c_syntax.bsl.parser.BSLParser;
import lombok.RequiredArgsConstructor;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.eclipse.lsp4j.Position;
import org.springframework.stereotype.Component;

import java.net.URI;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.regex.MatchResult;
import java.util.regex.Pattern;
import java.util.stream.Stream;

@Component
@RequiredArgsConstructor
public class TypeResolver {

    private final ServerContext serverContext;
    private final ReferenceResolver referenceResolver;
    private final ReferenceIndex referenceIndex;

    // Создаем LRU кэш с максимальным размером 100
    private final LRUMap<String, List<Type>> typesCache = new LRUMap<>(100);

    // Пример метода, использующего кэш
    public List<Type> calculateTypes(URI uri, BSLParser.ExpressionContext expression) {
        String key = uri.toString() + expression.getText();
        return typesCache.computeIfAbsent(key, k -> computeTypes(uri, expression));
    }

    // Метод для вычисления типов, который ранее был реализован
    private List<Type> computeTypes(URI uri, BSLParser.ExpressionContext expression) {
        // Логика вычисления типов
    }
}

Этот код использует LRUMap для хранения вычисленных типов, где ключом является уникальное сочетание URI и текста выражения. Элементы кэша автоматически удаляются, когда количество элементов превышает 100. Это должно улучшить производительность при повторных вычислениях типов для одних и тех же выражений.

Пожалуйста, дайте знать, если нужно что-то изменить или добавить.

Copy link

Choose a reason for hiding this comment

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

@nixel2007, LRUMap из Apache Commons Collections по умолчанию не является потокобезопасным. Это означает, что при использовании в многопоточной среде, как в вашем случае с TypeResolver в качестве синглтона, необходимо предпринять дополнительные шаги для обеспечения потокобезопасности.

Один из подходов — использовать Collections.synchronizedMap для обертывания LRUMap, что обеспечит базовую синхронизацию. Вот пример:

import org.apache.commons.collections4.map.LRUMap;
import java.util.Collections;
import java.util.Map;

// Создаем потокобезопасный LRU кэш
private final Map<String, List<Type>> typesCache = Collections.synchronizedMap(new LRUMap<>(100));

Однако, если вы ищете более современное и эффективное решение, рекомендую рассмотреть использование библиотеки Caffeine, которая предоставляет высокопроизводительные потокобезопасные кэши. Caffeine предназначена для использования в многопоточных приложениях и предлагает гибкие стратегии управления кэшем.

Если вам интересно, я могу предоставить пример интеграции Caffeine в ваш TypeResolver.

Copy link
Member Author

Choose a reason for hiding this comment

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

Да, интересно, покажи

Copy link

Choose a reason for hiding this comment

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

@nixel2007, вот пример интеграции библиотеки Caffeine для потокобезопасного кэширования в классе TypeResolver. Caffeine предоставляет высокопроизводительные кэши, подходящие для использования в многопоточных приложениях, и поддерживает различные стратегии управления кэшем.

import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;

@Component
@RequiredArgsConstructor
public class TypeResolver {

    private final ServerContext serverContext;
    private final ReferenceResolver referenceResolver;
    private final ReferenceIndex referenceIndex;

    // Создаем кэш с максимальным размером 100 и автоматическим удалением записей после 10 минут неактивности
    private final Cache<String, List<Type>> typesCache = Caffeine.newBuilder()
        .maximumSize(100)
        .expireAfterAccess(10, TimeUnit.MINUTES)
        .build();

    // Пример метода, использующего кэш
    public List<Type> calculateTypes(URI uri, BSLParser.ExpressionContext expression) {
        String key = uri.toString() + expression.getText();
        return typesCache.get(key, k -> computeTypes(uri, expression));
    }

    // Метод для вычисления типов, который ранее был реализован
    private List<Type> computeTypes(URI uri, BSLParser.ExpressionContext expression) {
        // Логика вычисления типов
    }
}

Этот код использует Caffeine для создания кэша, который автоматически удаляет элементы после 10 минут неактивности или когда количество элементов превышает 100. Кэш потокобезопасен и идеально подходит для использования в многопоточной среде вашего TypeResolver.

Пожалуйста, дайте знать, если нужно что-то изменить или добавить.

Copy link
Member Author

Choose a reason for hiding this comment

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

Есть ли у Caffeine какая-то интеграция со spring boot? Или его можно свободно использовать через builder?

Copy link

Choose a reason for hiding this comment

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

@nixel2007, Caffeine does not have a dedicated Spring Boot starter or automatic configuration integration like some other libraries. However, it can be seamlessly integrated into a Spring Boot application by manually configuring the cache using its builder.

Here's a basic example of how you can define a Caffeine cache bean in a Spring Boot application:

import com.github.benmanes.caffeine.cache.Caffeine;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class CacheConfig {

    @Bean
    public Cache<String, List<Type>> caffeineCache() {
        return Caffeine.newBuilder()
            .expireAfterWrite(10, TimeUnit.MINUTES)
            .maximumSize(100)
            .build();
    }
}

This configuration class defines a bean for the cache which you can autowire and use throughout your application. This approach gives you the flexibility to customize the cache settings as per your requirements.

Please let me know if you need further details or assistance with setting this up!

var maybeDescription = describableSymbol.getDescription();
if (maybeDescription.isPresent()) {
var description = maybeDescription.get();
if (description instanceof VariableDescription variableDescription) {
Copy link

Choose a reason for hiding this comment

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

Пустое тело условного оператора 'if'.

- if (description instanceof VariableDescription variableDescription) {
+ if (description instanceof VariableDescription variableDescription) {
+   // TODO: Добавить логику обработки
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (description instanceof VariableDescription variableDescription) {
if (description instanceof VariableDescription variableDescription) {
// TODO: Добавить логику обработки
}

// expression tree resolver
if (reference.getOccurrenceType() == OccurrenceType.DEFINITION) {
var document = serverContext.getDocument(uri);
var ast = document.getAst();
Copy link

Choose a reason for hiding this comment

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

В методе calculateTypes вызов getAst может привести к NullPointerException. Рекомендуется добавить проверку на null перед вызовом этого метода.

- var ast = document.getAst();
+ var ast = document != null ? document.getAst() : null;
+ if (ast == null) return Collections.emptyList();
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
var ast = document.getAst();
var ast = document != null ? document.getAst() : null;
if (ast == null) return Collections.emptyList();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant