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

Feature/os class module type #3113

Draft
wants to merge 23 commits into
base: develop
Choose a base branch
from
Draft

Conversation

nixel2007
Copy link
Member

@nixel2007 nixel2007 commented Jul 24, 2023

Описание

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

Closes

Чеклист

Общие

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

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

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

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

Summary by CodeRabbit

  • New Features

    • Introduced a new component for finding references to OScript entities.
    • Implemented a selection range provider with additional methods.
    • Added version 8.3.21 to compatibility mode options.
    • Enhanced variable symbols to include type information.
  • Enhancements

    • Improved readability and formatting across various diagnostic checks.
    • Streamlined code by adopting modern Java features like var keyword and toList() method.
  • Bug Fixes

    • Fixed conditional checks and control flow logic in several diagnostic methods.
    • Corrected spacing and formatting issues in code for consistency.
  • Documentation

    • Expanded documentation comments for better clarity on certain methods and classes.
  • Tests

    • Added new test cases for variable type retrieval and symbol content generation.
    • Introduced test scripts and procedures for comprehensive testing of new features.

@ghost
Copy link

ghost commented Jul 24, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@github-actions
Copy link

Qodana Community for JVM

14 new problems were found

Inspection name Severity Problems
Statement with empty body 🔶 Warning 5
'@Serial' annotation could be used 🔶 Warning 4
Mismatched query and update of collection 🔶 Warning 2
Mismatched read and write of array 🔶 Warning 1
Nullability and data flow problems 🔶 Warning 1
Redundant character escape 🔶 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]

@theshadowco theshadowco force-pushed the feature/osClassModuleType branch from 88e8e3b to 7992f30 Compare December 26, 2023 12:02
Copy link
Contributor

coderabbitai bot commented Dec 26, 2023

Important

Auto Review Skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Walkthrough

The changes primarily involve code cleanliness and refactoring for readability and maintainability. Whitespace has been trimmed, annotations added to suppress deprecation warnings, and spacing issues have been addressed. New functionality includes the addition of a ModuleTypeComputer class and enhancements to variable symbol handling to incorporate type information. The codebase has also been updated to reflect new language features, such as replacing Collectors.toList() with toList(). Furthermore, diagnostic capabilities have been expanded with new checks and compatibility modes.

Changes

Files Change Summary
.../bsl/languageserver/BSLLanguageServer.java
.../IdenticalExpressionsDiagnostic.java
Removed trailing whitespace.
.../BSLWorkspaceService.java Added @SuppressWarnings annotation; simplified CompletableFuture return.
.../cfg/BasicBlockVertex.java
.../ControlFlowGraphWalker.java
.../cli/FormatCommand.java
.../diagnostics/IncorrectLineBreakDiagnostic.java
.../utils/Ranges.java
Fixed spacing issues around if conditions and other minor formatting adjustments.
.../codeactions/DisableDiagnosticTriggeringSupplier.java
.../QuickFixSupplier.java
.../databind/ParametersDeserializer.java
.../context/DocumentContext.java
.../context/symbol/AbstractVariableSymbol.java
.../context/symbol/IntBasedVariableSymbol.java
.../context/symbol/ShortBasedVariableSymbol.java
.../context/symbol/VariableSymbol.java
.../diagnostics/AllFunctionPathMustHaveReturnDiagnostic.java
.../diagnostics/CodeAfterAsyncCallDiagnostic.java
.../diagnostics/CodeOutOfRegionDiagnostic.java
.../diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java
.../diagnostics/IncorrectUseOfStrTemplateDiagnostic.java
.../diagnostics/MagicDateDiagnostic.java
.../diagnostics/MissingTemporaryFileDeletionDiagnostic.java
.../diagnostics/UnusedLocalVariableDiagnostic.java
.../diagnostics/UsingCancelParameterDiagnostic.java
.../diagnostics/metadata/DiagnosticCompatibilityMode.java
.../hover/VariableSymbolMarkupContentBuilder.java
.../providers/SelectionRangeProvider.java
.../references/OscriptReferenceFinder.java
.../references/ReferenceIndex.java
.../references/model/Location.java
.../references/model/SymbolOccurrence.java
.../utils/Modules.java
.../utils/Trees.java
.../utils/expressiontree/BslExpression.java
.../utils/expressiontree/ConstructorCallNode.java
.../utils/expressiontree/ExpressionParseTreeRewriter.java
.../utils/expressiontree/ExpressionTreeBuildingVisitor.java
.../utils/expressiontree/TernaryOperatorNode.java
.../utils/expressiontree/UnaryOperationNode.java
Refactoring for readability and maintainability; added new functionality and types handling.
.../context/computer/ModuleTypeComputer.java
.../context/computer/VariableSymbolComputer.java
Introduced ModuleTypeComputer class; enhanced variable handling with type information.
.../test/... Added various test methods and resources to ensure new functionality works as expected.

Poem

In the warren of the code, through the thicket, hop and leap,
A rabbit cleans and tidies up, no trailing whites to keep.
Refactoring with care, new types in fields do weave,
Celebrate the cleaner code, this CodeRabbit did achieve! 🐇✨

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.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @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 Dec 26, 2023

Qodana Community for JVM

17 new problems were found

Inspection name Severity Problems
Statement with empty body 🔶 Warning 5
'@Serial' annotation could be used 🔶 Warning 4
Constant values 🔶 Warning 3
Mismatched query and update of collection 🔶 Warning 2
Class is exposed outside of its visibility scope 🔶 Warning 1
Nullability and data flow problems 🔶 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]

@nixel2007
Copy link
Member Author

@coderabbitai review

Copy link
Contributor

@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.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 29390f1 and 7992f30.
Files selected for processing (52)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLWorkspaceService.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/BasicBlockVertex.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/cli/FormatCommand.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/codeactions/DisableDiagnosticTriggeringSupplier.java (6 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/codeactions/QuickFixSupplier.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/databind/ParametersDeserializer.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContext.java (6 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleTypeComputer.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java (7 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AbstractVariableSymbol.java (3 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/IntBasedVariableSymbol.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ShortBasedVariableSymbol.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AllFunctionPathMustHaveReturnDiagnostic.java (4 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CodeAfterAsyncCallDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CodeOutOfRegionDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java (3 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectLineBreakDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.java (3 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingCancelParameterDiagnostic.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/metadata/DiagnosticCompatibilityMode.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java (4 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/references/OscriptReferenceFinder.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java (5 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/Location.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Modules.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java (2 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ConstructorCallNode.java (2 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionParseTreeRewriter.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/TernaryOperatorNode.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/UnaryOperationNode.java (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContextTest.java (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolTest.java (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ForbiddenMetadataNameDiagnosticTest.java (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/references/OscriptReferenceFinderTest.java (1 hunks)
  • src/test/resources/context/symbol/variableSymbolTest.os (1 hunks)
  • src/test/resources/hover/variableSymbolMarkupContentBuilder.bsl (1 hunks)
  • src/test/resources/metadata/oscript/main.os (1 hunks)
  • src/test/resources/metadata/oscript/Классы/ТестовыйКласс.os (1 hunks)
  • src/test/resources/metadata/oscript/Модули/ТестовыйМодуль.os (1 hunks)
Files skipped from review due to trivial changes (24)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLWorkspaceService.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/BasicBlockVertex.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/codeactions/DisableDiagnosticTriggeringSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/codeactions/QuickFixSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AllFunctionPathMustHaveReturnDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CodeAfterAsyncCallDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IdenticalExpressionsDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Modules.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ConstructorCallNode.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionParseTreeRewriter.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/UnaryOperationNode.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContextTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ForbiddenMetadataNameDiagnosticTest.java
Additional comments: 50
src/test/resources/context/symbol/variableSymbolTest.os (1)
  • 1-3: The procedure В() correctly instantiates a new object of МойКласс. Ensure that МойКласс is defined and accessible within the scope.
src/test/resources/metadata/oscript/main.os (1)
  • 1-4: The instantiation of ТестовыйКласс and the subsequent operation are syntactically correct. Ensure that ТестовыйМодуль.ЭкспортныйМетод() is defined and returns a compatible type for addition with ТестовыйКласс.
src/test/resources/metadata/oscript/Классы/ТестовыйКласс.os (2)
  • 2-5: The procedure ПростоПроцедура() is declared correctly but is empty. If this is intentional (e.g., for a test stub), it's fine; otherwise, implementation is needed.

  • 7-10: The procedure ПриСозданииОбъекта() is also declared correctly but is empty. As with ПростоПроцедура(), ensure this is intentional.

src/test/resources/metadata/oscript/Модули/ТестовыйМодуль.os (2)
  • 2-4: The procedure ПростоМетод() is declared correctly. Ensure that it is intended to be private since it lacks the Экспорт keyword.

  • 6-8: The procedure ЭкспортныйМетод() is correctly declared as exported and calls ПростоМетод(). Ensure that ПростоМетод() is intended to be called here and that it exists.

src/test/resources/hover/variableSymbolMarkupContentBuilder.bsl (1)
  • 35-35: The instantiation of ИмяКласса is correct. Ensure that ИмяКласса is defined and that this instantiation is used as intended.
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/TernaryOperatorNode.java (1)
  • 40-40: The constructor access level change to private prevents subclassing. Ensure that TernaryOperatorNode is not intended to be subclassed and that instances are only created through the static create method.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java (1)
  • 54-54: The addition of the getType() method to the VariableSymbol interface requires all implementing classes to provide an implementation for this method. Verify that all implementations have been updated accordingly.
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/Location.java (1)
  • 72-72: The assignment of endCharacter is now unconditional, which could be a logical change. Verify that this change is intentional and that endCharacter should always be set to the value from range.getEnd().getCharacter() regardless of any conditions.
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java (1)
  • 58-69: The changes in the compareTo method alter the comparison logic. Ensure that the new comparison order (URI, range, occurrence type, and symbol) is intended and that it aligns with the desired sorting behavior.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/metadata/DiagnosticCompatibilityMode.java (1)
  • 51-51: The addition of COMPATIBILITY_MODE_8_3_21 to the DiagnosticCompatibilityMode enum is correct. Ensure that this new version is supported and handled correctly in all relevant parts of the application.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleTypeComputer.java (1)
  • 30-75: The ModuleTypeComputer class and its methods computeModuleType and computeTypeName are correctly implemented. Ensure that the logic within computeBSL and computeOS methods correctly determines the module types based on the document context and that the computeTypeName method correctly extracts the type name from the URI.
src/test/java/com/github/_1c_syntax/bsl/languageserver/references/OscriptReferenceFinderTest.java (2)
  • 43-55: The test testFindReferenceToClass is implemented correctly. Ensure that the test setup is correct and that the reference is indeed present at the specified position.

  • 57-73: The test testFindReferenceToModule is implemented correctly. It checks for the presence of a reference and the absence of a variable at specified positions. Ensure that the test setup is correct and that the assertions reflect the expected behavior.

src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/IntBasedVariableSymbol.java (1)
  • 78-81: The addition of the type parameter to the IntBasedVariableSymbol constructor is correct. Ensure that all instantiations of IntBasedVariableSymbol have been updated to pass the new type parameter and that the type is correctly handled in the superclass.
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/databind/ParametersDeserializer.java (1)
  • 81-82: The instantiation of TypeReference using an empty anonymous class is a common Java idiom to work around type erasure by subclassing. This change seems correct, but ensure that the TypeReference is used as intended and that the deserialization logic is not affected.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ShortBasedVariableSymbol.java (1)
  • 78-81: The addition of the type parameter to the ShortBasedVariableSymbol constructor is consistent with the changes in the AbstractVariableSymbol class. Ensure that all instantiations of ShortBasedVariableSymbol are updated to include the new type argument.
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/OscriptReferenceFinder.java (1)
  • 1-93: No specific changes are highlighted within the hunk for OscriptReferenceFinder. Assuming the class is provided for context, no action is required.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingCancelParameterDiagnostic.java (1)
  • 72-72: The change in the visitSub method adjusts the text extraction for the lValue in the cancelPattern.matcher call. Ensure that the new logic correctly matches the intended pattern and that the diagnostic's behavior remains consistent with its purpose.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.java (1)
  • 112-112: The modification in the checkExclAddDiagnostic method changes the conditional check for ctx instanceof BSLParser.CallParamListContext. Ensure that this change correctly reflects the intended logic and that the diagnostic's behavior is as expected.
src/main/java/com/github/_1c_syntax/bsl/languageserver/cli/FormatCommand.java (1)
  • 137-137: The change in the findFilesForFormatting method adds spaces around the condition check for directory existence. This is a code style improvement and should not affect the logic or control flow.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AbstractVariableSymbol.java (3)
  • 101-101: The addition of the type field in the AbstractVariableSymbol class is a significant change. Ensure that all subclasses and usages of AbstractVariableSymbol are updated to handle the new type field appropriately.

  • 197-198: The type parameter is now passed to the constructor of ShortBasedVariableSymbol in the build method. Verify that this change is reflected everywhere ShortBasedVariableSymbol is instantiated.

  • 217-217: The type parameter is also passed to the constructor of IntBasedVariableSymbol in the build method. Verify that this change is reflected everywhere IntBasedVariableSymbol is instantiated.

src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectLineBreakDiagnostic.java (1)
  • 107-107: Please ensure that the addition of a space before the opening brace in the if condition aligns with the project's coding standards.
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolTest.java (1)
  • 147-154: Ensure that the new test method testVariableNewOS is correctly asserting the expected behavior and that the file path and variable type are correct.
src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java (1)
  • 132-148: Ensure that the new test method testMethodVarContentFromDirectFile_type_from_new is correctly asserting the expected behavior and that the content generation for the variable symbol is accurate.
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java (3)
  • 28-38: The addition of imports for Token, TerminalNode, Tree, Position, and Positions suggests new functionality related to parsing and position handling. Ensure these imports are used within the file and are not extraneous to avoid unnecessary dependencies.

  • 83-83: The formatting change in the getSelectionRange method's parameters is cosmetic and should follow the project's style guide. If the project has a defined style for method parameters, ensure this change is consistent with it.

  • 218-266: New methods related to position handling within parse trees and tokens have been added. Verify that these methods are correctly implemented and performant, especially in the context of large parse trees where performance could be a concern.

src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java (5)
  • 65-68: The constructor now accepts a List<MethodSymbol> instead of a different type. Ensure that all instantiations of VariableSymbolComputer throughout the codebase have been updated to pass the correct type.

  • 159-171: The method visitLValue has been renamed to visitAssignment, and its logic has been adjusted. Ensure that this renaming and logic change is consistent with the intended functionality and that all references to the old method name have been updated.

  • 210-220: The getVariableScope method has been modified to handle different contexts. Verify that the new logic correctly resolves variable scopes in all scenarios where this method is used.

  • 252-262: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [255-271]

The updateVariablesCache method has been extended to include type information. Ensure that the handling of type information is correct and that the performance is acceptable, especially when dealing with a large number of variables.

  • 282-291: A new method typeName has been introduced to extract type information from assignment contexts. Verify that this method correctly extracts the type information and is used appropriately within the class.
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java (4)
  • 73-73: The use of the var keyword for local variable declarations is a Java best practice for enhancing code readability and maintainability, provided the type is clear from the context. Ensure that the use of var does not reduce code clarity in these instances.

  • 167-174: The addMethodCall method now uses the var keyword for local variables. Confirm that the types of these variables are clear from the context to maintain code readability.

  • 197-209: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [200-216]

The addVariableUsage method has been updated to use the var keyword. Ensure that the types of the variables are clear from the context and that the use of var does not obscure the code's intent.

  • 257-257: The getSourceDefinedSymbol method now uses the var keyword for local variables. Confirm that the types of these variables are clear from the context to maintain code readability.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContext.java (10)
  • 31-31: The import of ModuleTypeComputer is added to support the new functionality for computing module types.

  • 121-121: The addition of typeName as a Lazy object is intended to cache the computed type name. Ensure that the computeTypeName method is implemented correctly and that the cached value is used appropriately throughout the codebase.

Verification successful

The computeTypeName method is implemented in the DocumentContext.java file and it uses the ModuleTypeComputer to compute the type name. Additionally, the typeName.getOrCompute() method is used within the same file, indicating that the cached value is being accessed appropriately.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the implementation of computeTypeName method.
ast-grep --lang java --pattern $'private String computeTypeName() { $$$ }'

# Verify the usage of typeName in the codebase.
rg --type java 'typeName.getOrCompute()'

Length of output: 699

* 373-373: Refactoring of `computeModuleType` to use `ModuleTypeComputer` is noted. Ensure that the `ModuleTypeComputer` class is correctly implemented and that the refactoring maintains the intended functionality.
Verification successful

The ModuleTypeComputer class is implemented with methods to compute the module type based on the file type and the document's URI. The computeModuleType method delegates to computeBSL or computeOS depending on the file type, and computeTypeName computes the type name for OScript modules and classes. The refactoring appears to maintain the intended functionality.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the implementation of ModuleTypeComputer class.
ast-grep --lang java --pattern $'class ModuleTypeComputer { $$$ }'

# Verify the usage of computeModuleType in the codebase.
rg --type java 'computeModuleType()'

Length of output: 696



Script:

#!/bin/bash
# Inspect the contents of ModuleTypeComputer.java to verify the implementation.
cat src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleTypeComputer.java

Length of output: 2610

* 378-378: Refactoring of `computeTypeName` to use `ModuleTypeComputer` is noted. Similar to `computeModuleType`, ensure that the `computeTypeName` method is correctly implemented and that the refactoring maintains the intended functionality.
Verification successful

To verify the correct implementation of the computeTypeName method and its usage within DocumentContext.java, as well as to ensure that ModuleTypeComputer is properly implemented, I will generate additional shell scripts to:

  1. Check the full implementation of the computeTypeName method in DocumentContext.java.
  2. Inspect the ModuleTypeComputer class and its computeTypeName method to ensure it is correctly implemented.

To further investigate the implementation of the computeTypeName method within the ModuleTypeComputer class, I will adjust the ast-grep pattern to be less specific and also use cat to directly inspect the ModuleTypeComputer class file.


The ModuleTypeComputer class has been successfully located, and its contents have been provided. The computeTypeName method within this class is implemented to return the file name without the .os extension if the module type is OScriptModule or OScriptClass. Otherwise, it returns an empty string. This implementation seems to be consistent with the intended functionality of computing a type name based on the document context and module type.

The computeModuleType method is also present, and it contains logic to determine the module type based on the file type and path, which supports the functionality of the computeTypeName method by providing the necessary module type information.

Based on the provided context and the new information from the scripts, the refactoring of the computeTypeName method to use ModuleTypeComputer appears to be correctly implemented and maintains the intended functionality.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the implementation of computeTypeName method.
ast-grep --lang java --pattern $'private String computeTypeName() { $$$ }'

# Verify the usage of typeName in the codebase.
rg --type java 'typeName.getOrCompute()'

Length of output: 699



Script:

#!/bin/bash
# Check the full implementation of computeTypeName method in DocumentContext.java
cat src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContext.java | grep -A 10 "private String computeTypeName()"

# Inspect the ModuleTypeComputer class and its computeTypeName method
ast-grep --lang java --pattern $'class ModuleTypeComputer { $$$ computeTypeName() { $$$ } $$$ }'

Length of output: 617



Script:

#!/bin/bash
# Inspect the ModuleTypeComputer class and its computeTypeName method with a less specific pattern
ast-grep --lang java --pattern $'class ModuleTypeComputer { $$$ }'

# Directly inspect the ModuleTypeComputer class file
fd "ModuleTypeComputer.java" --exec cat {}

Length of output: 2621

* 162-162: The refactoring of `getTokensFromDefaultChannel` to use Java streams and the `toList` method is noted. Ensure that the stream operation correctly filters tokens from the default channel.
Verification successful

The verification of the getTokensFromDefaultChannel method shows that it is used in various places within the codebase, including diagnostics and context computation. The refactored method appears to be functioning correctly, as it is being utilized without any issues reported in the output.

Given that the method is used in several diagnostics and context computations, it is important that the refactoring to use Java streams and the toList method does not introduce any behavioral changes unless intended. The usage of toList is a standard practice in Java for collecting stream results into a list, and the filtering by the default channel is a common operation when dealing with token streams.

Since the output does not indicate any errors or issues with the usage of the refactored method, and the refactoring itself follows Java best practices, it seems that the changes are correctly implemented.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the usage of getTokensFromDefaultChannel in the codebase.
rg --type java 'getTokensFromDefaultChannel()'

Length of output: 985

* 168-168: The refactoring of `getComments` to use Java streams and the `toList` method is noted. Ensure that the stream operation correctly filters comment tokens.
Verification successful

The implementation of the getComments() method in DocumentContext.java has been successfully retrieved. The method filters tokens from the token stream, selecting only those of type BSLLexer.LINE_COMMENT, and collects them into a list.

Here is the relevant part of the implementation:

public List<Token> getComments() {
  return getTokens().stream()
    .filter(token -> token.getType() == BSLLexer.LINE_COMMENT)
    .toList();
}

This confirms that the getComments() method is indeed using a stream operation to filter out comment tokens, specifically those that are line comments. The use of filter with the condition token.getType() == BSLLexer.LINE_COMMENT ensures that only line comment tokens are included in the resulting list.

Based on this information, the refactoring of getComments to use Java streams and the toList method is correctly implemented to filter comment tokens as intended.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the usage of getComments in the codebase.
rg --type java 'getComments()'

Length of output: 3139



Script:

#!/bin/bash
# Retrieve the implementation of getComments() from DocumentContext.java
ast-grep --lang java --pattern $'class DocumentContext {
  $$$
  public List<Token> getComments() {
    $$$
  }
  $$$
}'

Length of output: 121



Script:

#!/bin/bash
# Extract the implementation of getComments() from DocumentContext.java using ripgrep
rg --type java --context 10 'public List<Token> getComments\(\)' src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContext.java

Length of output: 731

* 373-373: Refactoring of `computeModuleType` to use `ModuleTypeComputer` is noted. Ensure that the `ModuleTypeComputer` class is correctly implemented and that the refactoring maintains the intended functionality.
Verification successful

To verify the correct implementation of the ModuleTypeComputer class and the computeModuleType method, we need to view the contents of the ModuleTypeComputer.java file. Let's generate a script to do that.


The ModuleTypeComputer class has been successfully located and its contents displayed. The class contains a constructor that accepts a DocumentContext object and two methods: computeModuleType and computeTypeName. The computeModuleType method checks the file type and calls either computeBSL or computeOS to determine the module type, defaulting to ModuleType.UNKNOWN if the file type is neither BSL nor OS. The computeBSL and computeOS methods contain logic specific to each file type to compute the module type. The computeTypeName method computes the type name for OScript modules and classes by extracting the file name from the document's URI and removing the .os extension.

Based on the provided output, the refactoring to use the ModuleTypeComputer class appears to be correctly implemented, with the computeModuleType method in DocumentContext.java now creating an instance of ModuleTypeComputer and calling its computeModuleType method.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the implementation of ModuleTypeComputer class.
ast-grep --lang java --pattern $'class ModuleTypeComputer { $$$ }'

# Verify the usage of computeModuleType in the codebase.
rg --type java 'computeModuleType()'

Length of output: 696



Script:

#!/bin/bash
# Find the ModuleTypeComputer.java file and display its contents
fd 'ModuleTypeComputer.java' --exec cat {}

Length of output: 2553

* 378-378: Refactoring of `computeTypeName` to use `ModuleTypeComputer` is noted. Similar to `computeModuleType`, ensure that the `computeTypeName` method is correctly implemented and that the refactoring maintains the intended functionality.
Verification successful

The refactoring of the computeTypeName method to use ModuleTypeComputer has been verified, and it is correctly implemented in the DocumentContext.java file. Additionally, the usage of typeName.getOrCompute() within the same file suggests that the typeName is being computed and cached as expected.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the implementation of computeTypeName method.
ast-grep --lang java --pattern $'private String computeTypeName() { $$$ }'

# Verify the usage of typeName in the codebase.
rg --type java 'typeName.getOrCompute()'

Length of output: 699

* 162-162: The refactoring of `getTokensFromDefaultChannel` to use Java streams and the `toList` method is noted. Ensure that the stream operation correctly filters tokens from the default channel.
Verification successful

The refactoring of getTokensFromDefaultChannel to use Java streams and the toList method has been verified in the codebase. It is used in several diagnostic classes and within the DocumentContext class. The stream operation correctly filters tokens from the default channel, and the usage of the method in the codebase suggests that the refactoring should not alter its intended behavior.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the usage of getTokensFromDefaultChannel in the codebase.
rg --type java 'getTokensFromDefaultChannel()'

Length of output: 985

* 168-168: The refactoring of `getComments` to use Java streams and the `toList` method is noted. Ensure that the stream operation correctly filters comment tokens.
Verification successful

The provided script output shows the implementation of the getComments() method within DocumentContext.java. The method filters tokens from the token list, keeping only those where the token type is BSLLexer.LINE_COMMENT. The stream operation concludes with a toList() call, which collects the filtered tokens into a list.

The filtering condition checks if the token type is BSLLexer.LINE_COMMENT, which suggests that it is correctly identifying line comment tokens. There is no indication from the output that the stream operation is incorrect or that the filter condition is improperly set.

Based on the output, the refactoring of getComments to use Java streams and the toList method appears to be implemented correctly, as it filters the tokens to include only comments.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the usage of getComments in the codebase.
rg --type java 'getComments()'

Length of output: 3139



Script:

#!/bin/bash
# Examine the implementation of getComments() in DocumentContext.java
rg --type java --context 10 'public List<Token> getComments\(\)' src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContext.java

Length of output: 731

@@ -72,6 +72,7 @@
public ParseTree visitFile(BSLParser.FileContext ctx) {

// Для неизвестных модулей не будем требовать нахождения кода в области
// fix me bsl only
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment // fix me bsl only suggests that there may be outstanding work to be done here. Please address or remove this comment if it's no longer relevant.

Comment on lines +59 to +61
if (documentContext.getUri().getPath().contains("Модули")) {
return ModuleType.OScriptModule;
} else if (documentContext.getUri().getPath().contains("Классы")) {
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

sonarqubecloud bot commented Jan 3, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

11 New issues
0 Security Hotspots
93.9% Coverage on New Code
0.4% Duplication on New Code

See analysis details on SonarCloud

Copy link

github-actions bot commented Apr 7, 2024

Qodana Community for JVM

15 new problems were found

Inspection name Severity Problems
Statement with empty body 🔶 Warning 5
'@Serial' annotation could be used 🔶 Warning 4
Mismatched query and update of collection 🔶 Warning 2
Class is exposed outside of its visibility scope 🔶 Warning 1
Constant values 🔶 Warning 1
Nullability and data flow problems 🔶 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]

Copy link

sonarqubecloud bot commented Apr 7, 2024

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.

3 participants