Fix MockitoMockMaker throws NPE on null object

[AndreasTu]

MockMakerRegistry.asMockOrNull() shall return null on null object and not call the mock makers.

Fixes #2337

Summary by CodeRabbit

• Bug Fixes

  • Prevented a NullPointerException when processing null objects in Mockito-backed mock handling, improving robustness and null-safety.

• Tests

  • Added regression and null-input tests to ensure stubbing or mock queries against null do not throw.

• Documentation

  • Noted the fix in the upcoming release notes under “Misc.”

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 68705309-de4a-4a2b-9bc5-c1e8215a04ce

📥 Commits

Reviewing files that changed from the base of the PR and between c0c2eaa and 012cd4e.

📒 Files selected for processing (5)

• docs/release_notes.adoc
• spock-core/src/main/java/org/spockframework/mock/runtime/IMockMaker.java
• spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java
• spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMaker.java
• spock-specs/src/test/groovy/org/spockframework/mock/runtime/mockito/MockitoMockMakerSpec.groovy

✅ Files skipped from review due to trivial changes (1)

• docs/release_notes.adoc

🚧 Files skipped from review as they are similar to previous changes (2)

• spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMaker.java
• spock-core/src/main/java/org/spockframework/mock/runtime/IMockMaker.java

---

📝 Walkthrough

Walkthrough

This PR prevents a NullPointerException when mock-makers receive a null object by adding @Nullable annotations and early null checks; it also adds tests and a release-note entry documenting the fix (issue 2337).

Changes

Cohort / File(s)	Summary
Nullability Annotations spock-core/src/main/java/org/spockframework/mock/runtime/IMockMaker.java, spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java, spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMaker.java	Added @Nullable to asMockOrNull parameters and return types to declare nullable contract.
Null-Input Guard Logic spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java, spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMaker.java	Added early null checks returning null when input object is null, avoiding downstream NPEs.
Tests & Cleanup spock-specs/src/test/groovy/org/spockframework/mock/runtime/mockito/MockitoMockMakerSpec.groovy, spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java	Added tests asserting asMockOrNull(null) returns null and a regression test for issue #2337; removed an unused import.
Docs docs/release_notes.adoc	Added release-note entry under 2.5 (tbd) → === Misc documenting the fix for spockIssue:2337.

Sequence Diagram(s)

sequenceDiagram
    participant Test
    participant MockMakerRegistry
    participant MockitoMockMaker
    participant MockitoImpl
    Test->>MockMakerRegistry: call asMockOrNull(object)
    alt object is null
        MockMakerRegistry-->>Test: return null
    else object non-null
        MockMakerRegistry->>MockitoMockMaker: asMockOrNull(object)
        alt impl is null
            MockitoMockMaker-->>MockMakerRegistry: return null
        else impl present
            MockitoMockMaker->>MockitoImpl: getHandler(object)
            MockitoImpl-->>MockitoMockMaker: handler / null
            MockitoMockMaker-->>MockMakerRegistry: IMockObject / null
        end
    end
    MockMakerRegistry-->>Test: IMockObject / null

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniffed a null upon the floor,
I paused, I guarded, then no more—
A tiny check, a marked return,
The NPE no longer burns.
Hoppity-hop, the tests all cheer—this rabbit’s fix is here! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 fix: preventing MockitoMockMaker from throwing NPE when handling null objects, which is the core issue addressed in the PR.
Linked Issues check	✅ Passed	The PR fully addresses issue #2337 by implementing null checks in MockMakerRegistry, MockitoMockMaker, and IMockMaker, preventing NPE when asMockOrNull receives null. Tests confirm the fix works as intended.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the null-handling bug: null checks in registry and makers, nullable annotations, removal of unused import, and test coverage. No extraneous changes 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[testlens-app]

✅ All tests passed ✅

⚠️ TestLens detected flakiness ⚠️

Test Summary

Check	Project/Task	Test	Runs
Verify Branches and PRs / Build and Verify (5.0, 11, macos-latest)	:spock-specs:test	TimeoutExtension > annotating spec class has same effect as annotating every feature method not already annotated with @timeout	⚠️ 🚫

🏷️ Commit: 012cd4e
▶️ Tests: 3789 executed
⚪️ Checks: 33/33 completed

---

Learn more about TestLens at testlens.app.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.16%. Comparing base (5f8de21) to head (012cd4e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2338      +/-   ##
============================================
- Coverage     82.22%   82.16%   -0.07%     
+ Complexity     4826     4824       -2     
============================================
  Files           472      472              
  Lines         15038    15040       +2     
  Branches       1907     1908       +1     
============================================
- Hits          12365    12357       -8     
- Misses         1985     1991       +6     
- Partials        688      692       +4     

Files with missing lines	Coverage Δ
...va/org/spockframework/mock/runtime/IMockMaker.java	100.00% <ø> (ø)
...spockframework/mock/runtime/MockMakerRegistry.java	97.29% <100.00%> (+0.04%)	⬆️
...amework/mock/runtime/mockito/MockitoMockMaker.java	100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Vampire]

LGTM