Skip to content

#280: Fix unicode max diameter#1228

Merged
yegor256 merged 4 commits into
cqfn:masterfrom
marinka-aa:fix-issue-280-unicode-max-diameter
Jun 25, 2026
Merged

#280: Fix unicode max diameter#1228
yegor256 merged 4 commits into
cqfn:masterfrom
marinka-aa:fix-issue-280-unicode-max-diameter

Conversation

@marinka-aa

@marinka-aa marinka-aa commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added support for detecting and reporting class inheritance in Java code.
    • Expanded the documented pattern catalog with a new inheritance example.
  • Bug Fixes

    • Improved text reading so files with unexpected or mixed encodings are handled more reliably.
    • Added safer fallback behavior when the initial encoding guess fails.
  • Tests

    • Added coverage for class inheritance cases, including nested classes and non-matching scenarios.
    • Added coverage for reading files with non-UTF-8 content.

@marinka-aa marinka-aa changed the title #280: Fix issue 280 unicode max diameter #280: Fix unicode max diameter Jun 25, 2026
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds the P34 Class inheritance pattern, registers it, documents it, and adds fixtures/tests for inheritance cases. It also updates autodetected text reading to try multiple encodings and adds non-UTF-8 coverage.

Changes

Class inheritance pattern

Layer / File(s) Summary
Pattern implementation and registration
aibolit/patterns/class_inheritance/class_inheritance.py, aibolit/config.py, aibolit/patterns/class_inheritance/__init__.py, PATTERNS.md
ClassInheritance scans CLASS_DECLARATION nodes for extends, the config registers it as P34, and PATTERNS.md adds the new pattern entry.
Fixtures and pattern tests
test/patterns/class_inheritance/SimpleInheritance.java, test/patterns/class_inheritance/OnlyImplements.java, test/patterns/class_inheritance/TypeParameterExtends.java, test/patterns/class_inheritance/NestedInheritance.java, test/patterns/class_inheritance/test_class_inheritance.py
Java fixtures cover inheritance, implements-only input, bounded type parameters, and nested classes, and the test module asserts the expected pattern matches.

Autodetected encoding fallback

Layer / File(s) Summary
Fallback decode strategy
aibolit/utils/encoding_detector.py
read_text_with_autodetected_encoding now tries detected encoding, UTF-8, and Latin-1 in order, skips decode failures, and falls back to replacement decoding.
Encoding detector coverage
test/metrics/maxDiameter/test_max_diameter.py, test/utils/test_encoding_detector.py
Tests add a non-UTF-8 Java file case for MaxDiameter and a mocked detector case for read_text_with_autodetected_encoding.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • cqfn/aibolit#814: Also updates aibolit/config.py pattern registration and related typing/tests around get_patterns_config().

Suggested reviewers

  • ivanovmg
  • yegor256

Poem

A bunny hopped through bytes so odd,
Yet found new paths with a happy nod.
P34 sprang up, neat and spry,
While UTF-8 took one more try. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% 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 matches a real major part of the changeset: fixing Unicode handling for max diameter tests and related encoding behavior.
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 unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PMD (7.25.0)
test/patterns/class_inheritance/NestedInheritance.java

[ERROR] Error at scripts/ruleset.xml:23:36
21|
22|
23|
^^^^^ Value should be 'countImports'

24|
25| : '|' should be 'countImports'
[ERROR] Error at scripts/ruleset.xml:19:3
17|
18|
19|
^^^^^ Error while parsing rule reference

20|
21| : Error at scripts/ruleset.xml:23:36
21|
22|
23|
^^^^^ Value should be 'countImports'

24|
25|
[ERROR] Cannot load ruleset scripts/ruleset.xml: 2 XML validation errors occurred
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

test/patterns/class_inheritance/TypeParameterExtends.java

[ERROR] Error at scripts/ruleset.xml:23:36
21|
22|
23|
^^^^^ Value should be 'countImports'

24|
25| : '|' should be 'countImports'
[ERROR] Error at scripts/ruleset.xml:19:3
17|
18|
19|
^^^^^ Error while parsing rule reference

20|
21| : Error at scripts/ruleset.xml:23:36
21|
22|
23|
^^^^^ Value should be 'countImports'

24|
25|
[ERROR] Cannot load ruleset scripts/ruleset.xml: 2 XML validation errors occurred
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

test/patterns/class_inheritance/SimpleInheritance.java

[ERROR] Error at scripts/ruleset.xml:23:36
21|
22|
23|
^^^^^ Value should be 'countImports'

24|
25| : '|' should be 'countImports'
[ERROR] Error at scripts/ruleset.xml:19:3
17|
18|
19|
^^^^^ Error while parsing rule reference

20|
21| : Error at scripts/ruleset.xml:23:36
21|
22|
23|
^^^^^ Value should be 'countImports'

24|
25|
[ERROR] Cannot load ruleset scripts/ruleset.xml: 2 XML validation errors occurred
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

  • 1 others

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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

Inline comments:
In `@aibolit/utils/encoding_detector.py`:
- Around line 24-31: The fallback in encoding detection is unreachable because
`detect_encoding_of_data`, `utf-8`, and `latin-1` are tried before the
replacement decode, and `latin-1` will always succeed. Update `decode_data` in
`aibolit/utils/encoding_detector.py` so the final `data.decode(...,
errors="replace")` path can actually run, either by removing `latin-1` from the
initial `encodings` list or by moving the replacement decode ahead of it while
keeping the existing trial order logic in `dict.fromkeys(...)`.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b9ab1e85-095e-407a-ac30-058f7d221c89

📥 Commits

Reviewing files that changed from the base of the PR and between a3ace02 and 9d43a78.

📒 Files selected for processing (12)
  • PATTERNS.md
  • aibolit/config.py
  • aibolit/patterns/class_inheritance/__init__.py
  • aibolit/patterns/class_inheritance/class_inheritance.py
  • aibolit/utils/encoding_detector.py
  • test/metrics/maxDiameter/test_max_diameter.py
  • test/patterns/class_inheritance/NestedInheritance.java
  • test/patterns/class_inheritance/OnlyImplements.java
  • test/patterns/class_inheritance/SimpleInheritance.java
  • test/patterns/class_inheritance/TypeParameterExtends.java
  • test/patterns/class_inheritance/test_class_inheritance.py
  • test/utils/test_encoding_detector.py

Comment on lines +24 to +31
encodings = [detect_encoding_of_data(data), 'utf-8', 'latin-1']
for encoding in dict.fromkeys(filter(None, encodings)):
try:
return data.decode(encoding)
except (LookupError, UnicodeDecodeError):
continue

return data.decode('utf-8', errors='replace')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

utf-8(errors="replace") fallback is unreachable with current order

Because Line 24 always includes latin-1, Line 31 will never execute (latin-1 decode succeeds for any bytes). If replacement-decoding is intended as a real last resort, remove latin-1 from the trial list or move replacement logic before it.

🤖 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 `@aibolit/utils/encoding_detector.py` around lines 24 - 31, The fallback in
encoding detection is unreachable because `detect_encoding_of_data`, `utf-8`,
and `latin-1` are tried before the replacement decode, and `latin-1` will always
succeed. Update `decode_data` in `aibolit/utils/encoding_detector.py` so the
final `data.decode(..., errors="replace")` path can actually run, either by
removing `latin-1` from the initial `encodings` list or by moving the
replacement decode ahead of it while keeping the existing trial order logic in
`dict.fromkeys(...)`.

@yegor256 yegor256 merged commit 766fefd into cqfn:master Jun 25, 2026
20 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.

2 participants