Skip to content

fix: устранение ложных reliability-замечаний Sonar (S2583/S2637) - #3963

Merged
nixel2007 merged 2 commits into
developfrom
fix/sonar-reliability-gate
May 27, 2026
Merged

fix: устранение ложных reliability-замечаний Sonar (S2583/S2637)#3963
nixel2007 merged 2 commits into
developfrom
fix/sonar-reliability-gate

Conversation

@nixel2007

@nixel2007 nixel2007 commented May 27, 2026

Copy link
Copy Markdown
Member

Что и зачем

Quality gate на develop падал по единственному условию — new_reliability_rating = C (порог A). Причина — 38 reliability-замечаний в новом коде, почти все из которых ложные срабатывания или намеренные паттерны (Spring DI прототип-бинов, ANTLR-nullability при @NullMarked).

Этот PR устраняет две позиции, где в коде действительно стоит уточнить контракт nullability (а не править логику):

  1. DiagnosticsOptions.setIgnoredAuthors (java:S2583) — @JsonSetter, вызывается Jackson и реально может получить null ("ignoredAuthors": null). Проверка authors == null корректна; из-за @NullMarked Sonar считал её недостижимой. Параметр помечен @Nullable.
  2. BracketDocumentHighlightSupplier.findMatchingBracket (java:S2637) — приватный хелпер возвращает null, когда пара не найдена; вызывающий код уже проверяет результат. Возврат помечен @Nullable.

Обе правки — только аннотации, без изменения поведения (NullAway в сборке не используется).

Остальные замечания

Прочие reliability-замечания в новом коде разобраны индивидуально и являются ложными срабатываниями либо намеренными паттернами, которые нельзя чинить правкой кода без вреда дизайну:

  • java:S6832 ×23 — инъекция workspace/prototype-бинов (DocumentContext, LanguageServerConfiguration) в одноаргументный конструктор. Потребители — либо классы, создаваемые через new вручную (*Computer), либо сами prototype-бины (диагностики). Не синглтоны → реальной проблемы нет.
  • java:S6813 ×4 — намеренный field-injection (self-injection с @Lazy для кэша; разрыв цикла через @Qualifier/@Lazy; документированный обход циклической зависимости).
  • java:S2637/S2583 в cfg/expression tree и ParseTree-коде — ANTLR может вернуть null даже там, где сигнатура этого не заявляет; защитные проверки оставлены.

Они закрываются в SonarCloud как False Positive / Won't Fix (вне этого PR).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Internal code quality improvements for enhanced null-safety handling

Review Change Stack

nixel2007 and others added 2 commits May 27, 2026 23:15
Метод вызывается Jackson при десериализации и может получить null
(JSON `"ignoredAuthors": null`), поэтому защитная проверка `authors == null`
корректна. Из-за @NullMarked на пакете Sonar считал параметр non-null и
помечал проверку как недостижимую (java:S2583). Помечаем параметр @nullable,
отражая реальный контракт.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Метод возвращает null, когда парная скобка не найдена, и вызывающий код
уже проверяет результат на null. Из-за @NullMarked на пакете Sonar считал
возврат non-null и помечал `return null` как нарушение контракта
(java:S2637). Помечаем возврат @nullable, отражая реальное поведение.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 27, 2026 21:16
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4d295c1b-98d2-4b87-b58d-d5ebb98459a6

📥 Commits

Reviewing files that changed from the base of the PR and between 926aeb7 and 4e182d9.

📒 Files selected for processing (2)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/DiagnosticsOptions.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/BracketDocumentHighlightSupplier.java

📝 Walkthrough

Walkthrough

Two Java methods receive explicit JSpecify nullability annotations. setIgnoredAuthors in DiagnosticsOptions now declares its parameter as @Nullable, and findMatchingBracket in BracketDocumentHighlightSupplier marks its return type as @Nullable. No behavior changes; annotations document existing null-handling contracts.

Changes

Nullability Annotations

Layer / File(s) Summary
DiagnosticsOptions parameter nullability
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/DiagnosticsOptions.java
The setIgnoredAuthors method parameter is marked @Nullable to document that null input is accepted and results in an empty set initialization.
BracketDocumentHighlightSupplier return nullability
src/main/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/BracketDocumentHighlightSupplier.java
Import of org.jspecify.annotations.Nullable and annotation of findMatchingBracket return type as @Nullable document that the method can return null when no matching bracket is found.

Estimated Code Review Effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested Reviewers

  • theshadowco

Poem

🐰 Two methods now wear their null-safe vests,
JSpecify annotations pass the tests!
Parameter and return both plainly state:
"I might be null—that's totally great!" ✨

🚥 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 accurately describes the main change: fixing false Sonar reliability warnings (S2583/S2637) through nullability annotations in two files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sonar-reliability-gate

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.

Copilot AI 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.

Pull request overview

This PR refines nullability contracts to match existing runtime behavior and reduce false reliability findings without changing logic.

Changes:

  • Marks findMatchingBracket as nullable because it returns null when no matching bracket is found.
  • Marks DiagnosticsOptions.setIgnoredAuthors input as nullable because Jackson may pass null for the property.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/main/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/BracketDocumentHighlightSupplier.java Adds @Nullable to the private bracket matching helper return type.
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/DiagnosticsOptions.java Adds @Nullable to the Jackson setter parameter for ignored authors.

@nixel2007
nixel2007 merged commit a6474e9 into develop May 27, 2026
37 checks passed
@nixel2007
nixel2007 deleted the fix/sonar-reliability-gate branch May 27, 2026 21:22
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