Fix NPE in DefectMissing for range +unlint of absent lint (#841)

[bibonix]

Fixes #841.

When +unlint name:line-line (range form) references a lint name that did not fire, DefectMissing.apply() calls this.defects.get(name) which returns null, and then lines.stream() throws NullPointerException.

This PR:

• Adds two failing unit tests to DefectMissingTest covering the missing-key + range case (with and without other entries in the map).
• Adds an integration test catchesRangeUnlintOfAbsentLint to LtUnlintNonExistingDefectTest matching the reproducer in the issue.
• Fixes DefectMissing.apply() to treat a null lines list the same way the non-range branch does: the defect is reported as missing unless the lint name is in the excluded list.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved handling of unlint directives with line ranges that reference non-existent lint rules.

• Tests

  • Added test coverage for edge cases involving range-based unlint directives on absent lint types.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request fixes a NullPointerException in DefectMissing when unlint directives use range patterns (name:line-line) to reference non-existent lint definitions. The fix adds a null-safety check before invoking stream operations on potentially absent defect mappings.

Changes

Cohort / File(s)	Summary
Null-safety check for range unlints src/main/java/org/eolang/lints/DefectMissing.java	Added defensive null check to prevent NullPointerException when stream() is called on absent defect entries; computes missing as the negation of excluded.contains(name) when lines is null, maintaining consistent behavior for non-existent lints with range patterns.
DefectMissing test coverage src/test/java/org/eolang/lints/DefectMissingTest.java	Added two new JUnit tests to validate behavior when unlint range patterns reference absent lint keys — one with other entries in the defects map, one with an empty map; both assert matcher returns true.
Integration test for absent lint ranges src/test/java/org/eolang/lints/LtUnlintNonExistingDefectTest.java	Added new test case catchesRangeUnlintOfAbsentLint() to verify that unlint directives with line ranges referencing absent lints return defects instead of throwing NullPointerException, extending coverage beyond single-line non-existing unlint scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• PR #687: Modifies DefectMissing to handle unlints with line-range syntax and adds tests for unlints referencing non-existing defect keys, directly addressing the same null-safety concern.
• PR #684: Adds range-support infrastructure to LtUnlint and introduces a TODO marker in DefectMissing that this PR directly addresses with its defensive logic.

Suggested labels

core

Suggested reviewers

• maxonfjvipon
• yegor256

Poem

🐰 A null pointer once crept through the code,
When unlints and ranges both crossed the road,
But now with a check, both careful and kind,
The absent-key case no longer will bind! 🌿

🚥 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 and concisely summarizes the main change: fixing a NullPointerException in DefectMissing when processing range +unlint patterns for absent lint names, directly addressing issue #841.
Linked Issues check	✅ Passed	The pull request fully addresses all requirements from issue #841: it fixes the NPE by adding null-checks in DefectMissing.apply(), implements test coverage for missing-key + range cases in both unit and integration tests, and ensures correct behavior when +unlint references absent lint names.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #841: modifications to DefectMissing.apply() logic, new unit tests in DefectMissingTest and integration test in LtUnlintNonExistingDefectTest, with no unrelated alterations present.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

src/test/java/org/eolang/lints/DefectMissingTest.java (1)

84-101: Good coverage for the absent-key + range scenarios.

Both tests correctly exercise the new lines == null branch (one with other map entries, one with an empty map).

Optional: consider adding a third case where the absent lint name is in the excluded collection, asserting apply(...) returns false. That would pin down the !this.excluded.contains(name) half of the new branch, which is currently only implicitly covered.

🧪 Optional additional test

+    `@Test`
+    void returnsFalseWhenRangeReferencesExcludedAbsentLint() {
+        MatcherAssert.assertThat(
+            "Defect should not be missing when referenced lint is excluded",
+            new DefectMissing(new MapOf<>(), new ListOf<>("ascii-only"))
+                .apply("ascii-only:1-5"),
+            Matchers.equalTo(false)
+        );
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/eolang/lints/DefectMissingTest.java` around lines 84 - 101,
Add a third unit test that verifies the case where the referenced lint name is
absent from the defects map but present in the excluded collection: construct a
DefectMissing with an empty or non-matching MapOf for defects and a ListOf that
contains the lint name in the excluded parameter, then call
DefectMissing.apply("ascii-only:1-5") and assert it returns false; this will
exercise the "!this.excluded.contains(name)" path in DefectMissing.apply and
ensure the excluded list suppresses the missing-defect result.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/test/java/org/eolang/lints/DefectMissingTest.java`:
- Around line 84-101: Add a third unit test that verifies the case where the
referenced lint name is absent from the defects map but present in the excluded
collection: construct a DefectMissing with an empty or non-matching MapOf for
defects and a ListOf that contains the lint name in the excluded parameter, then
call DefectMissing.apply("ascii-only:1-5") and assert it returns false; this
will exercise the "!this.excluded.contains(name)" path in DefectMissing.apply
and ensure the excluded list suppresses the missing-defect result.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1958622-29a7-42c4-adc5-eddcb2ecbb2e

📥 Commits

Reviewing files that changed from the base of the PR and between 38bb5f1 and 3f7fca2.

📒 Files selected for processing (3)

• src/main/java/org/eolang/lints/DefectMissing.java
• src/test/java/org/eolang/lints/DefectMissingTest.java
• src/test/java/org/eolang/lints/LtUnlintNonExistingDefectTest.java

[bibonix]

Closing as duplicate of #843, which was opened yesterday for the same issue and has a green CI matrix. Sorry for the noise.