Замечания на уровне проекта + прогрессивный расчет #317
Conversation
WalkthroughThe project has made several changes, including updating a language server dependency, enhancing SonarQube integration, and improving syntax highlighting for a specific database language (SDBL). Changes in issue loading now support both files and projects, reflecting a more flexible analysis capability. Tests have been refined with modern Java features like Changes
Assessment against linked issues
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (13)
- build.gradle.kts (1 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensor.java (8 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighter.java (4 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/IssuesLoader.java (7 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/LanguageServerDiagnosticsLoaderSensor.java (2 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensorTest.java (3 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighterTest.java (5 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLPluginTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/IssuesLoaderTest.java (6 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/QualityProfileTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/RuleDefinitionTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageServerRuleDefinitionTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageTest.java (1 hunks)
Files skipped from review due to trivial changes (5)
- build.gradle.kts
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLPluginTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/QualityProfileTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/RuleDefinitionTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageTest.java
Additional comments: 24
src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageServerRuleDefinitionTest.java (1)
- 63-66: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-63]
The test class
BSLLanguageServerRuleDefinitionTestappears to be well-structured and follows Java best practices. The teststest_initandtestCheckTagParametersare checking the initialization of rules and the tag parameters respectively. It's good to see that assertions are being made to ensure that the rules repository is not empty and that rules with parameters have the expected tag.However, there is no change in this file that requires a specific comment. The file seems to be unchanged and is just provided for context.
src/main/java/com/github/_1c_syntax/bsl/sonar/LanguageServerDiagnosticsLoaderSensor.java (1)
- 30-30: The import of
org.eclipse.lsp4j.jsonrpc.messages.Eitheris correctly added to support the new functionality of creating issues at the project level.src/test/java/com/github/_1c_syntax/bsl/sonar/IssuesLoaderTest.java (4)
31-31: The import of
org.eclipse.lsp4j.jsonrpc.messages.Eitheris correctly added to support the new functionality of creating issues at the project level.72-72: The change in the
test_createExtIssuemethod to useEither.forLeft(inputFile)is consistent with the changes made in theLanguageServerDiagnosticsLoaderSensor.javafile. This ensures that the tests are aligned with the new method signature.121-121: The change in the
test_createIssuemethod to useEither.forLeft(inputFile)is consistent with the changes made in theLanguageServerDiagnosticsLoaderSensor.javafile. This ensures that the tests are aligned with the new method signature.171-171: The addition of the
issuesLoader.createIssue(Either.forRight(context.project()), diagnostic);line is a new test case to verify the creation of issues at the project level. This is a good addition to ensure that the new functionality is covered by tests.src/main/java/com/github/_1c_syntax/bsl/sonar/IssuesLoader.java (5)
35-35: The import of
org.eclipse.lsp4j.jsonrpc.messages.Eitheris correctly added to support the new functionality of creating issues at the project level.46-46: The import of
org.sonar.api.scanner.fs.InputProjectis correctly added to support the new functionality of creating issues at the project level.157-157: The modification of the
createIssuemethod to acceptEither<InputFile, InputProject>instead of justInputFileis a significant change. This change is necessary to support the new feature of creating issues at the project level. It's important to ensure that all calls to this method have been updated to use the new signature.* 188-188: The `processDiagnostic` method has been updated to handle the new `Either` type for issue creation. This change is consistent with the new functionality of creating issues at the project level.Verification successful
--- The search with `rg` has successfully identified several calls to the `createIssue` method in both test and main source files. The calls are using the new signature that accepts an `Either<InputFile, InputProject>` argument, as evidenced by the usage of `Either.forLeft` and `Either.forRight` in the arguments. This confirms that the method's calls have been updated to match the new method signature, which supports the creation of issues at both the file and project levels. Based on the provided output, the review comment regarding the necessity to ensure that all calls to the `createIssue` method have been updated to use the new signature is verified as correct. The calls in the `IssuesLoaderTest.java`, `LanguageServerDiagnosticsLoaderSensor.java`, and `BSLCoreSensor.java` reflect the updated usage. </details> <details> <summary>Scripts Executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Search for calls to createIssue to ensure they are using the new signature. ast-grep --lang java --pattern $'createIssue(Either.$_($_), $_)'Length of output: 65
Script:
#!/bin/bash # Search for calls to createIssue to ensure they are using the new signature. # The previous pattern may have been too specific, so let's try a broader search. ast-grep --lang java --pattern $'createIssue($_, $_)'Length of output: 54
Script:
#!/bin/bash # Use ripgrep to search for calls to createIssue in the codebase. rg 'createIssue\('Length of output: 1285
- 229-229: The
createExternalIssuemethod has been updated to handle the newEither<InputFile, InputProject>type for issue creation. This change is consistent with the new functionality of creating issues at the project level.src/test/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensorTest.java (1)
- 212-217: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-255]
The test class
BSLCoreSensorTestappears to be well-structured and follows Java best practices. The tests cover various scenarios, including descriptor setup, execution with different configurations, and coverage metrics. It's good to see that assertions are being made to ensure that the sensor behaves as expected.However, there is no change in this file that requires a specific comment. The file seems to be unchanged and is just provided for context.
src/main/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensor.java (5)
30-30: The import of
com.github._1c_syntax.bsl.languageserver.diagnostics.metadata.DiagnosticCodeis correctly added to support the new functionality of creating issues at the project level.40-40: The import of
org.eclipse.lsp4j.Diagnosticis correctly added to support the new functionality of creating issues at the project level.79-80: The addition of the
diagnosticsOnProjectset is a significant change. This set is used to store the diagnostic codes that can be located on the project level, which is a new feature introduced in this PR.188-194: The modification in the
executemethod to check if a diagnostic can be located on the project level and create an issue accordingly is a significant change. This change is necessary to support the new feature of creating issues at the project level. It's important to ensure that all diagnostics are correctly categorized and that issues are created at the appropriate level.348-350: The logic to add diagnostic codes to the
diagnosticsOnProjectset if they can be located on the project level is correctly implemented. This is necessary for the new feature of creating issues at the project level.src/test/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighterTest.java (3)
157-157: The use of
varfor local variable type inference is consistent with modern Java practices and enhances code readability by reducing verbosity where the type is obvious.385-408: New mathematical and string function tokens have been added to the
functionsset. This change seems to be in line with the PR's objective to enhance diagnostic capabilities and syntax highlighting.486-490: Introduction of a new set of string constants
edsis consistent with the PR's objective to handle new tokens related to project-level diagnostics. Ensure that the usage of these constants is consistent throughout the codebase.Verification successful
The search results indicate that the new string constants
EDS_CUBE,EDS_TABLE, andEDS_CUBE_DIMTABLEare indeed used in both the test fileBSLHighlighterTest.javaand the main source fileBSLHighlighter.java. This suggests that the constants are not only defined but also utilized within the codebase, which aligns with the reviewer's comment about ensuring consistent usage throughout the project.Based on the provided output, it appears that the constants are used in a manner consistent with the PR's objectives, and there are no indications of inconsistent usage or errors related to these constants in the provided context.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new EDS constants are used consistently in the codebase rg --type java "EDS_CUBE|EDS_TABLE|EDS_CUBE_DIMTABLE"Length of output: 613
src/main/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighter.java (4)
68-68: The addition of the
SDBL_EDSset is consistent with the changes in the test file and the PR's objectives to support new tokens for enhanced diagnostics.262-263: The
getTypeOfTextSDBLmethod has been updated to handle the newSDBL_EDSset. This change is necessary for the correct syntax highlighting of the new tokens.531-554: New tokens related to mathematical and string functions have been added to the
createSdblFunctionsmethod. This addition is necessary for the correct syntax highlighting of these functions and aligns with the PR's objectives.639-644: The
createSdblEDSmethod has been introduced to create a set of EDS-related tokens. This is a new addition and should be verified for consistent usage across the codebase.Verification successful
The verification process has confirmed that the
createSdblEDSmethod is used inBSLHighlighter.javato initialize a static final set namedSDBL_EDS. Additionally, theSDBL_EDSset is utilized within a conditional statement in the same file, indicating that the new tokens are indeed being used in the codebase logic.Based on the information gathered, the review comment about the introduction of the
createSdblEDSmethod and its consistent usage across the codebase is accurate. The new EDS-related tokens are defined, a set is created using these tokens, and this set is subsequently used in the code logic.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new EDS-related tokens are used consistently in the codebase rg --type java "EDS_CUBE|EDS_TABLE|EDS_CUBE_DIMTABLE"Length of output: 613
Script:
#!/bin/bash # Check for references to the `createSdblEDS` method to ensure the new set is being utilized rg --type java "createSdblEDS"Length of output: 274
Script:
#!/bin/bash # Check for usage of the `SDBL_EDS` set to ensure that the new tokens are being utilized in the codebase logic rg --type java "SDBL_EDS"Length of output: 268
src/main/java/com/github/_1c_syntax/bsl/sonar/LanguageServerDiagnosticsLoaderSensor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensor.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/sonar/LanguageServerDiagnosticsLoaderSensor.java
Outdated
Show resolved
Hide resolved
theshadowco
changed the title
- динамическое увеличение времени на исправление от сложности - фиксы для bslls 0.23 - фиксы зависимостей
theshadowco
force-pushed
the
feature/supportEdtStorage
branch
from
74beb14 to
641777d
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (13)
- build.gradle.kts (1 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensor.java (8 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighter.java (4 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/IssuesLoader.java (6 hunks)
- src/main/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageServerRuleDefinition.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensorTest.java (3 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighterTest.java (5 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLPluginTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/IssuesLoaderTest.java (3 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/QualityProfileTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/RuleDefinitionTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageServerRuleDefinitionTest.java (1 hunks)
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (12)
- build.gradle.kts
- src/main/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensor.java
- src/main/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighter.java
- src/main/java/com/github/_1c_syntax/bsl/sonar/IssuesLoader.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLCoreSensorTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLHighlighterTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/BSLPluginTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/IssuesLoaderTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/QualityProfileTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/ext_issues/RuleDefinitionTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageServerRuleDefinitionTest.java
- src/test/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageTest.java
Additional comments: 1
src/main/java/com/github/_1c_syntax/bsl/sonar/language/BSLLanguageServerRuleDefinition.java (1)
- 141-154: The changes to the debt remediation function setup in the
setUpNewRulemethod look correct. The conditional check based ondiagnosticInfo.getExtraMinForComplexity()and the subsequent setting of either a linear function with an offset or a constant per issue function align with the PR objectives. Ensure that thegetExtraMinForComplexityandgetMinutesToFixmethods return values in the expected format for this logic to work correctly.
|
closes #48
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Tests