Skip to content

анализ решения в целом, а не одной произвольной конфигурации#3534

Merged
nixel2007 merged 1 commit into
developfrom
feature/solution
Oct 20, 2025
Merged

анализ решения в целом, а не одной произвольной конфигурации#3534
nixel2007 merged 1 commit into
developfrom
feature/solution

Conversation

@theshadowco

@theshadowco theshadowco commented Oct 20, 2025

Copy link
Copy Markdown
Member

Описание

Реализовано чтение решения (проекта, solution) - всего анализируемого каталога как единое целого

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

Closes

Чеклист

Общие

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

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

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

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

Summary by CodeRabbit

  • Chores

    • Updated MD classes dependency version.
  • Refactor

    • Simplified internal configuration resolution logic.
    • Updated configuration metadata construction approach.

@coderabbitai

coderabbitai Bot commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The pull request updates the MD classes dependency version and refactors configuration discovery logic. Changes include removal of file-based configuration discovery patterns, substitution of a configuration metadata construction method, and introduction of type inference in module mapping.

Changes

Cohort / File(s) Summary
Dependency Update
build.gradle.kts
Updated MDClasses dependency from version 0.16.0.22-SNAPSHOT to 0.16.0.23-SNAPSHOT
Configuration Discovery Removal
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java
Removed static Pattern searchConfiguration field, private method getConfigurationFile(Path rootPath), and associated file-based configuration discovery logic; updated annotation formatting for accessors
Configuration Metadata Construction
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java
Changed computeConfigurationMetadata() to invoke MDClasses.createSolution(configurationRoot) instead of MDClasses.createConfiguration(configurationRoot) while preserving error handling and fallback behavior
Type Inference
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.java
Replaced explicit Map<ModuleType, URI> type annotation with var for modules variable assignment; removed corresponding unused imports

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes are predominantly straightforward: a dependency version bump, removal of clearly-scoped internal logic, a single method-call substitution with preserved error paths, and a type inference refactoring. All modifications follow consistent patterns with minimal semantic complexity.

Possibly related PRs

Poem

🐰 A rabbit hops through code so clean,
Old patterns fade, new solutions gleam—
createSolution now takes the lead,*
With trimmed discovery, we're freed!
Type inference makes the syntax sweet,
Dependencies dance—the PR's complete! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "анализ решения в целом, а не одной произвольной конфигурации" (analysis of the solution as a whole, not one arbitrary configuration) directly aligns with the main changes in the changeset. The key technical modifications—particularly the replacement of MDClasses.createConfiguration() with MDClasses.createSolution() in ServerContext and the removal of configuration discovery logic in LanguageServerConfiguration—implement exactly what the title describes: a shift from analyzing individual configurations to analyzing the entire solution/project as a unified entity. The title is concise, specific, and clearly conveys the primary architectural change to someone reviewing the commit history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/solution

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4400d74 and 95278ef.

📒 Files selected for processing (4)
  • build.gradle.kts (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: Analyse
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (23, windows-latest)
  • GitHub Check: build (23, ubuntu-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: Benchmark
  • GitHub Check: build
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (23, macOS-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (23, windows-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (23, ubuntu-latest)
🔇 Additional comments (3)
build.gradle.kts (1)

92-92: Verified: SNAPSHOT dependency is properly integrated with comprehensive error handling.

The code examination confirms the mdclasses SNAPSHOT dependency 0.16.0.23-SNAPSHOT supports the new createSolution API. The implementation uses this method as the primary approach with proper fallback mechanisms:

  • Primary method: MDClasses.createSolution(configurationRoot) executes in a ForkJoinPool with timeout capability
  • Fallback handlers: Two exception catches (ExecutionException, InterruptedException) and a null check all gracefully revert to MDClasses.createConfiguration()

The design pattern demonstrates the SNAPSHOT version has been tested and is compatible with the codebase.

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

292-292: Verify whether the fallback behavior is intentional graceful degradation or incomplete refactoring, and document accordingly.

The review comment correctly identifies an inconsistency in the code:

  • Main path (line 292): Attempts MDClasses.createSolution(configurationRoot) for whole-solution analysis
  • Exception handlers (lines 295, 298): Fall back to MDClasses.createConfiguration(), reverting to single-configuration mode

This fallback pattern is confirmed in the codebase. However, the design intent—whether this is intentional graceful degradation or an oversight—cannot be verified from the available code, as the MDClasses class definitions are not accessible in this environment (likely in a separate module or external dependency).

Required action: Confirm with the development team whether the fallback to createConfiguration() on solution-parsing errors is the intended design. If yes, add a comment explaining this graceful degradation. If no, consider updating lines 295 and 298 to use createSolution() or implement an alternative recovery strategy.

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

191-191: Code change is correct, but manual verification of external dependency required.

The change from Map<ModuleType, URI> to var is appropriate—the type is clearly inferred from configuration.mdoModuleTypes(mdoRef) and confirmed by the subsequent usage at line 192 (modules.keySet()). The var keyword maintains type safety while improving readability.

However, I cannot verify whether the return type of mdoModuleTypes() changed in mdclasses version 0.16.0.23-SNAPSHOT (the version in build.gradle.kts). The method is defined in the external mdclasses library, and its source is not accessible in this repository.

To confirm the change is safe:

  • Check the mdclasses 0.16.0.23-SNAPSHOT javadoc or GitHub repository for the return type signature of mdoModuleTypes()
  • Verify it still returns Map<ModuleType, URI> or compatible type

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants