Skip to content

Conversation

@theshadowco
Copy link
Member

@theshadowco theshadowco commented Nov 12, 2025

Описание

Корректирована проверка родительского каталога

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

Closes

Чеклист

Общие

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

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

Summary by CodeRabbit

  • Bug Fixes
    • Improved application stability by addressing a potential crash condition that could occur during file system operations.

@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

Walkthrough

A null-safety check was strengthened in the MDClasses.findFiles method. The code now validates both that the grandparent directory exists and its fileName is non-null before filtering results, preventing potential NullPointerException errors during parent directory traversal.

Changes

Cohort / File(s) Change Summary
Null-safety enhancement
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java
Narrowed null-safety checks in findFiles method to require grandparent directory existence and non-null fileName before filtering, preventing NullPointerException on file name access

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file modification with focused, localized change
  • Straightforward null-safety improvement with clear intent
  • Minimal logic density; defensive programming enhancement

Poem

A rabbit hops through code so tight,
Where nulls once lurked in directory's night—
Now checks run deep, both paths align,
No exceptions leap, the logic's fine! 🐰✨

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 title 'проверка родителя на null' (check parent for null) directly describes the main change: narrowing null-safety checks for parent directories in MDClasses.findFiles.
✨ 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/fixParentCheck

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.

Copy link

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (1)

277-277: Consider similar null-safety check for consistency.

Line 277 calls path.getParent().getFileName().toString() without checking if getFileName() returns null. While this is unlikely to be an issue in practice (source files are rarely directly under the filesystem root), for complete null-safety and consistency with the fix on line 279, you might consider adding a similar check here.

If you decide to add this check, the code could be:

-        var parentName = path.getParent().getFileName().toString();
+        var parent = path.getParent();
+        if (parent == null || parent.getFileName() == null) {
+          return false;
+        }
+        var parentName = parent.getFileName().toString();
📜 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 e6b59ab and f2625f2.

📒 Files selected for processing (1)
  • src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 524
File: src/main/java/com/github/_1c_syntax/bsl/mdo/support/UsePurposes.java:61-69
Timestamp: 2025-10-06T14:38:27.354Z
Learning: В проекте mdclasses используются JSR-305 аннотации (ParametersAreNonnullByDefault) на уровне пакетов для обеспечения null-safety через статический анализ, что является приемлемым подходом вместо явных runtime проверок.
📚 Learning: 2025-10-06T14:38:27.354Z
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 524
File: src/main/java/com/github/_1c_syntax/bsl/mdo/support/UsePurposes.java:61-69
Timestamp: 2025-10-06T14:38:27.354Z
Learning: В проекте mdclasses используются JSR-305 аннотации (ParametersAreNonnullByDefault) на уровне пакетов для обеспечения null-safety через статический анализ, что является приемлемым подходом вместо явных runtime проверок.

Applied to files:

  • src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java
⏰ 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). (16)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (23, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: QA
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: build (23, macOS-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (23, windows-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (23, ubuntu-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (17, windows-latest)
🔇 Additional comments (1)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (1)

279-281: LGTM! Proper null-safety check added.

The additional null check for getFileName() correctly prevents potential NPE when the grandparent is a root path. This is a valid defensive programming improvement. Based on learnings.

However, please ensure this change is covered by tests, as the PR checklist indicates test coverage is incomplete. Consider adding a test case that exercises this code path with a directory structure where the parent is close to the root.

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Test Results

  549 files  ±0    549 suites  ±0   8m 12s ⏱️ -3s
  239 tests ±0    239 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 187 runs  ±0  2 187 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit f2625f2. ± Comparison against base commit e6b59ab.

♻️ This comment has been updated with latest results.

@theshadowco theshadowco merged commit b735f51 into develop Nov 12, 2025
25 checks passed
@theshadowco theshadowco deleted the feature/fixParentCheck branch November 12, 2025 08:41
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