Skip to content

feat: запрет использования ложных категорий в ВызватьИсключение (#1935)#4101

Merged
nixel2007 merged 2 commits into
1c-syntax:developfrom
erprivalov:feature/Issue-1935-clean
Jun 14, 2026
Merged

feat: запрет использования ложных категорий в ВызватьИсключение (#1935)#4101
nixel2007 merged 2 commits into
1c-syntax:developfrom
erprivalov:feature/Issue-1935-clean

Conversation

@erprivalov

@erprivalov erprivalov commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Описание

Реализована проверка, которая сообщает о нарушении, если во втором параметре оператора ВызватьИсключение передана одна из следующих запрещенных категорий:

  • ВсеОшибки / AllErrors
  • ПрочаяОшибка / OtherError
  • ОшибкаКомпиляцииВстроенногоЯзыка / ScriptCompilationError
  • ОшибкаВоВремяВыполненияВстроенногоЯзыка / ScriptRuntimeError
  • ИсключениеВызванноеИзВстроенногоЯзыка / ScriptRaisedException

Вместо общих групп рекомендуется использовать максимально точные прикладные категории ошибок.

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

Closes #1935

Чеклист

Общие

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

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

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

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

Summary by CodeRabbit

  • New Features

    • Added diagnostic rule that flags improper use of generic or system error categories when raising exceptions, promoting the use of specific application-level categories instead.
  • Documentation

    • Added comprehensive documentation for the new diagnostic rule with examples demonstrating correct vs. incorrect exception category usage.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a complete BadExceptionCategory diagnostic to the BSL language server that detects and reports when developers use overly generic system error categories in the functional form of exception raising via Raise/ВызватьИсключение. The implementation includes semantic checking via TypeService, configuration schema wiring, multilingual localization, comprehensive documentation with examples, and unit test coverage.

Changes

BadExceptionCategory Diagnostic

Layer / File(s) Summary
User documentation and specification
docs/diagnostics/BadExceptionCategory.md, docs/en/diagnostics/BadExceptionCategory.md
Russian and English documentation describing the forbidden categories (ВсеОшибки, ПрочаяОшибка, ОшибкаКомпиляцииВстроенногоЯзыка, ОшибкаВоВремяВыполненияВстроенногоЯзыка, ИсключениеВызванноеИзВстроенногоЯзыка), rationale, and correct vs. incorrect usage examples with reference to 1C documentation.
Diagnostic detection logic
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnostic.java
BadExceptionCategoryDiagnostic inspects RaiseStatement nodes, extracts the category identifier from the second parameter, checks if the name is forbidden (case-insensitive), and validates via TypeService that the category references a system error category before recording a diagnostic.
Configuration schema registration
src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json, src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json
JSON schema definitions register BadExceptionCategory as a configurable diagnostic parameter with default enabled state and dual boolean/object type support.
Diagnostic message localization
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnostic_en.properties, src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnostic_ru.properties
English and Russian properties define diagnosticName and diagnosticMessage for user-facing reporting of rule violations.
Unit test validation
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnosticTest.java
JUnit 5 test extends AbstractDiagnosticTest and validates that exactly 6 violations are detected at expected line ranges (lines 24–28 and line 31) in the diagnostic test fixture.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

hacktoberfest-accepted

Suggested reviewers

  • nixel2007
  • sfaqer

Poem

🐰 A rabbit bounces with glee,
For exceptions now category-free,
No more broad ВсеОшибки thrown,
Specificity's seeds are sown,
Better code for all to see! 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: implementing a diagnostic check for forbidden exception categories in the Raise function.
Linked Issues check ✅ Passed All requirements from issue #1935 are met: diagnostic detects forbidden categories, documentation provided in both languages, correct severity/tags configured, and test coverage included.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the BadExceptionCategory diagnostic as specified in issue #1935; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnosticTest.java (1)

37-37: ⚡ Quick win

Use a descriptive test method name instead of test.

Rename this method to reflect behavior under test (for example, shouldReportForbiddenExceptionCategories) so failures are self-explanatory.

Suggested rename
-  void test() {
+  void shouldReportForbiddenExceptionCategories() {

As per coding guidelines, "Use meaningful, descriptive names following Java naming conventions; keep class and method names concise but clear".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnosticTest.java`
at line 37, The test method named test in class
BadExceptionCategoryDiagnosticTest is non-descriptive; rename the method to a
meaningful name like shouldReportForbiddenExceptionCategories (or another name
reflecting the behavior under test) and update the method declaration (keeping
any `@Test` annotation intact) so test failures and reports are self-explanatory;
ensure any internal references to the old method name are updated accordingly.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnosticTest.java`:
- Line 37: The test method named test in class
BadExceptionCategoryDiagnosticTest is non-descriptive; rename the method to a
meaningful name like shouldReportForbiddenExceptionCategories (or another name
reflecting the behavior under test) and update the method declaration (keeping
any `@Test` annotation intact) so test failures and reports are self-explanatory;
ensure any internal references to the old method name are updated accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bea38d68-e746-41b0-927c-0405eee173cd

📥 Commits

Reviewing files that changed from the base of the PR and between 8ce9f2f and 8c4c3fb.

⛔ Files ignored due to path filters (1)
  • src/test/resources/diagnostics/BadExceptionCategoryDiagnostic.bsl is excluded by !src/test/resources/**
📒 Files selected for processing (8)
  • docs/diagnostics/BadExceptionCategory.md
  • docs/en/diagnostics/BadExceptionCategory.md
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnostic.java
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.json
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnostic_en.properties
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnostic_ru.properties
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadExceptionCategoryDiagnosticTest.java

@sonarqubecloud

Copy link
Copy Markdown

@nixel2007

Copy link
Copy Markdown
Member

Пока волью, пришли follow up плз

@nixel2007
nixel2007 merged commit 3c41a89 into 1c-syntax:develop Jun 14, 2026
26 checks passed
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.

[NEW] BadExceptionCategory

2 participants